diff --git a/RESOURCE_SHARING_AND_ACCESS_CONTROL.md b/RESOURCE_SHARING_AND_ACCESS_CONTROL.md index 45146e4b94..4c6322172b 100644 --- a/RESOURCE_SHARING_AND_ACCESS_CONTROL.md +++ b/RESOURCE_SHARING_AND_ACCESS_CONTROL.md @@ -493,12 +493,16 @@ This feature is controlled by the following flag: ```yaml plugins.security.resource_sharing.enabled: true ``` -> **Upgrading from a version that used the experimental flag (breaking change)** +> **Upgrading from a version that used the experimental flag (deprecated, not yet removed)** > -> Prior to graduation, these settings were named `plugins.security.experimental.resource_sharing.enabled` and `plugins.security.experimental.resource_sharing.protected_types`. The `experimental.` segment has been **removed with no fallback**, so the old keys no longer work. Before upgrading: -> - **`opensearch.yml`:** rename the keys to the new names on every node. A node that still has an old `plugins.security.experimental.resource_sharing.*` key will **fail to start** (`unknown setting`). -> - **Persistent cluster settings:** re-apply the setting under the new key after upgrading. On upgrade the old key is no longer recognized and is archived (`archived.plugins.security.experimental.resource_sharing.*`), so the feature reverts to its default (**disabled**) until you re-apply it. -> - **During a rolling upgrade**, enforcement is inconsistent until all nodes are on the new version: the new key is rejected by not-yet-upgraded nodes and the old key by upgraded nodes. Plan for resource sharing to be effectively disabled in this window and re-apply the setting once the upgrade completes. +> Prior to graduation these settings were named `plugins.security.experimental.resource_sharing.enabled` and `plugins.security.experimental.resource_sharing.protected_types`. **The old names are deprecated and will be removed in a future major version. Move to the new names.** +> +> Until then the old names keep working, so an upgrade does not require any change before it starts: +> - **`opensearch.yml`:** a node that still has an old key starts normally and the value is honored. The log records that the setting is deprecated and names its replacement. +> - **Cluster settings:** an existing `plugins.security.experimental.resource_sharing.*` cluster setting is honored, and is rewritten to the new name during cluster-state recovery so the deprecated key does not linger in `GET _cluster/settings`. A dynamic update that still uses the old name is also accepted and rewritten. +> - **Precedence:** if both names are set, the new name wins. A leftover old key cannot override a deliberate new one. +> +> Rename the keys at your convenience. Once renamed, the deprecation warnings stop. ### **List protected types** diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 9d4dff0f90..a4798ad2d6 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -90,6 +90,7 @@ import org.opensearch.common.settings.IndexScopedSettings; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Setting.Property; +import org.opensearch.common.settings.SettingUpgrader; import org.opensearch.common.settings.Settings; import org.opensearch.common.settings.SettingsFilter; import org.opensearch.common.util.BigArrays; @@ -2709,6 +2710,11 @@ public List> getSettings() { // Defaults to no resources as protected settings.add(resourceSharingProtectedResourceTypesSetting.getDynamicSetting()); + // Pre-graduation names of the two settings above. Registered so that an existing configuration is + // still understood and so the setting upgraders below can resolve the old keys. + settings.add(ResourceSharingFeatureFlagSetting.LEGACY_RESOURCE_SHARING_ENABLED); + settings.add(ResourceSharingProtectedResourcesSetting.LEGACY_PROTECTED_TYPES); + settings.add(UserFactory.Caching.MAX_SIZE); settings.add(UserFactory.Caching.EXPIRE_AFTER_ACCESS); @@ -2752,6 +2758,14 @@ public List> getSettings() { return settings; } + @Override + public List> getSettingUpgraders() { + return List.of( + ResourceSharingFeatureFlagSetting.RESOURCE_SHARING_ENABLED_UPGRADER, + ResourceSharingProtectedResourcesSetting.PROTECTED_TYPES_UPGRADER + ); + } + @Override public List getSettingsFilter() { List settingsFilter = new ArrayList<>(); diff --git a/src/main/java/org/opensearch/security/resources/settings/ResourceSharingFeatureFlagSetting.java b/src/main/java/org/opensearch/security/resources/settings/ResourceSharingFeatureFlagSetting.java index cbb64e1b4d..12edf2aaf2 100644 --- a/src/main/java/org/opensearch/security/resources/settings/ResourceSharingFeatureFlagSetting.java +++ b/src/main/java/org/opensearch/security/resources/settings/ResourceSharingFeatureFlagSetting.java @@ -13,6 +13,7 @@ import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Setting; +import org.opensearch.common.settings.SettingUpgrader; import org.opensearch.common.settings.Settings; import org.opensearch.security.resources.ResourcePluginInfo; import org.opensearch.security.setting.OpensearchDynamicSetting; @@ -22,18 +23,71 @@ public class ResourceSharingFeatureFlagSetting extends OpensearchDynamicSetting { private static final Logger logger = LogManager.getLogger(ResourceSharingFeatureFlagSetting.class); + /** + * Pre-graduation name of {@link #RESOURCE_SHARING_ENABLED}, kept registered so that an existing + * configuration is still understood. Registering it also lets {@link #RESOURCE_SHARING_ENABLED_UPGRADER} + * resolve the key, since the upgrade path looks the old setting up before rewriting it. + */ + @Deprecated + public static final Setting LEGACY_RESOURCE_SHARING_ENABLED = Setting.boolSetting( + ConfigConstants.OPENSEARCH_LEGACY_RESOURCE_SHARING_ENABLED, + ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED_DEFAULT, + Setting.Property.NodeScope, + Setting.Property.Dynamic, + Setting.Property.Deprecated + ); + + /** + * Falls back to {@link #LEGACY_RESOURCE_SHARING_ENABLED} when the current key is absent. The fallback + * resolves at read time against whichever settings instance is supplied, so it covers node settings from + * {@code opensearch.yml} as well as cluster settings, and a dynamic update to the old key still moves the + * resolved value and therefore still fires the update consumer. + */ public static final Setting RESOURCE_SHARING_ENABLED = Setting.boolSetting( ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED, - ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED_DEFAULT, + LEGACY_RESOURCE_SHARING_ENABLED, Setting.Property.NodeScope, Setting.Property.Dynamic ); + /** + * Rewrites the pre-graduation key to the current one in the cluster state, so an upgraded cluster stops + * carrying the deprecated key instead of keeping it indefinitely. Applies during cluster-state recovery + * and to any cluster settings update that still uses the old name. + */ + public static final SettingUpgrader RESOURCE_SHARING_ENABLED_UPGRADER = new SettingUpgrader() { + @Override + public Setting getSetting() { + return LEGACY_RESOURCE_SHARING_ENABLED; + } + + @Override + public String getKey(final String key) { + return RESOURCE_SHARING_ENABLED.getKey(); + } + }; + private final ResourcePluginInfo resourcePluginInfo; public ResourceSharingFeatureFlagSetting(final Settings settings, final ResourcePluginInfo resourcePluginInfo) { super(RESOURCE_SHARING_ENABLED, RESOURCE_SHARING_ENABLED.get(settings)); this.resourcePluginInfo = resourcePluginInfo; + warnIfLegacyKeyInUse(settings); + } + + /** + * The generic deprecation warning that {@code Setting.Property.Deprecated} produces names the old key but + * not its replacement, so log the replacement explicitly. Only fires when the old key is actually set. + */ + static void warnIfLegacyKeyInUse(final Settings settings) { + if (LEGACY_RESOURCE_SHARING_ENABLED.exists(settings)) { + logger.warn( + "Resource sharing is configured with [{}], which is deprecated. The setting is still honored, " + + "but support for it will be removed in a future major version. Use [{}] instead.", + ConfigConstants.OPENSEARCH_LEGACY_RESOURCE_SHARING_ENABLED, + ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED + ); + } } @Override diff --git a/src/main/java/org/opensearch/security/resources/settings/ResourceSharingProtectedResourcesSetting.java b/src/main/java/org/opensearch/security/resources/settings/ResourceSharingProtectedResourcesSetting.java index 3afe137432..e5c05727ed 100644 --- a/src/main/java/org/opensearch/security/resources/settings/ResourceSharingProtectedResourcesSetting.java +++ b/src/main/java/org/opensearch/security/resources/settings/ResourceSharingProtectedResourcesSetting.java @@ -16,6 +16,7 @@ import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Setting; +import org.opensearch.common.settings.SettingUpgrader; import org.opensearch.common.settings.Settings; import org.opensearch.security.resources.ResourcePluginInfo; import org.opensearch.security.setting.OpensearchDynamicSetting; @@ -24,19 +25,67 @@ public class ResourceSharingProtectedResourcesSetting extends OpensearchDynamicSetting> { private static final Logger logger = LogManager.getLogger(ResourceSharingProtectedResourcesSetting.class); + /** + * Pre-graduation name of {@link #PROTECTED_TYPES}. See + * {@link ResourceSharingFeatureFlagSetting#LEGACY_RESOURCE_SHARING_ENABLED}. + */ + @Deprecated + public static final Setting> LEGACY_PROTECTED_TYPES = Setting.listSetting( + ConfigConstants.OPENSEARCH_LEGACY_RESOURCE_SHARING_PROTECTED_TYPES, + ConfigConstants.OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES_DEFAULT, + Function.identity(), + Setting.Property.NodeScope, + Setting.Property.Dynamic, + Setting.Property.Deprecated + ); + + /** + * Falls back to {@link #LEGACY_PROTECTED_TYPES} when the current key is absent. + */ public static final Setting> PROTECTED_TYPES = Setting.listSetting( ConfigConstants.OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES, - ConfigConstants.OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES_DEFAULT, + LEGACY_PROTECTED_TYPES, Function.identity(), Setting.Property.NodeScope, Setting.Property.Dynamic ); + /** + * Rewrites the pre-graduation key to the current one in the cluster state. Only the key moves, so the + * inherited list-value passthrough is what we want. + */ + public static final SettingUpgrader> PROTECTED_TYPES_UPGRADER = new SettingUpgrader>() { + @Override + public Setting> getSetting() { + return LEGACY_PROTECTED_TYPES; + } + + @Override + public String getKey(final String key) { + return PROTECTED_TYPES.getKey(); + } + }; + private final ResourcePluginInfo resourcePluginInfo; public ResourceSharingProtectedResourcesSetting(final Settings settings, final ResourcePluginInfo resourcePluginInfo) { super(PROTECTED_TYPES, PROTECTED_TYPES.get(settings)); this.resourcePluginInfo = resourcePluginInfo; + warnIfLegacyKeyInUse(settings); + } + + /** + * See {@link ResourceSharingFeatureFlagSetting#warnIfLegacyKeyInUse(Settings)}. + */ + static void warnIfLegacyKeyInUse(final Settings settings) { + if (LEGACY_PROTECTED_TYPES.exists(settings)) { + logger.warn( + "Resource sharing protected types are configured with [{}], which is deprecated. The setting is " + + "still honored, but support for it will be removed in a future major version. Use [{}] instead.", + ConfigConstants.OPENSEARCH_LEGACY_RESOURCE_SHARING_PROTECTED_TYPES, + ConfigConstants.OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES + ); + } } @Override diff --git a/src/main/java/org/opensearch/security/support/ConfigConstants.java b/src/main/java/org/opensearch/security/support/ConfigConstants.java index 93185e62de..7aae9d08d6 100644 --- a/src/main/java/org/opensearch/security/support/ConfigConstants.java +++ b/src/main/java/org/opensearch/security/support/ConfigConstants.java @@ -457,9 +457,25 @@ public class ConfigConstants { public static final String OPENSEARCH_RESOURCE_SHARING_ENABLED = "plugins.security.resource_sharing.enabled"; public static final boolean OPENSEARCH_RESOURCE_SHARING_ENABLED_DEFAULT = false; + /** + * Pre-graduation name of {@link #OPENSEARCH_RESOURCE_SHARING_ENABLED}. Retained so that a cluster + * configured before the feature graduated keeps working; {@code RESOURCE_SHARING_ENABLED} falls back + * to it and a setting upgrader rewrites it in the cluster state. + */ + @Deprecated + public static final String OPENSEARCH_LEGACY_RESOURCE_SHARING_ENABLED = "plugins.security.experimental.resource_sharing.enabled"; + // Protected resource types // Resource sharing will only apply to these types public static final String OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES = "plugins.security.resource_sharing.protected_types"; + + /** + * Pre-graduation name of {@link #OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES}. See + * {@link #OPENSEARCH_LEGACY_RESOURCE_SHARING_ENABLED}. + */ + @Deprecated + public static final String OPENSEARCH_LEGACY_RESOURCE_SHARING_PROTECTED_TYPES = + "plugins.security.experimental.resource_sharing.protected_types"; public static final List OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES_DEFAULT = List.of(); // defaults to no registered types as // protected diff --git a/src/test/java/org/opensearch/security/resources/settings/ResourceSharingSettingMigrationTests.java b/src/test/java/org/opensearch/security/resources/settings/ResourceSharingSettingMigrationTests.java new file mode 100644 index 0000000000..1b11832fd1 --- /dev/null +++ b/src/test/java/org/opensearch/security/resources/settings/ResourceSharingSettingMigrationTests.java @@ -0,0 +1,142 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.security.resources.settings; + +import java.util.List; +import java.util.Set; + +import org.junit.Test; + +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Setting; +import org.opensearch.common.settings.SettingUpgrader; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.settings.SettingsException; +import org.opensearch.security.support.ConfigConstants; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +/** + * Covers the migration path from the pre-graduation resource sharing setting names to the current ones: + * the current settings fall back to the old keys, and the setting upgraders rewrite the old keys in the + * cluster state. + */ +public class ResourceSharingSettingMigrationTests { + + private static final String LEGACY_ENABLED = ConfigConstants.OPENSEARCH_LEGACY_RESOURCE_SHARING_ENABLED; + private static final String CURRENT_ENABLED = ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED; + private static final String LEGACY_TYPES = ConfigConstants.OPENSEARCH_LEGACY_RESOURCE_SHARING_PROTECTED_TYPES; + private static final String CURRENT_TYPES = ConfigConstants.OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES; + + private ClusterSettings clusterSettings() { + final Set> registered = Set.of( + ResourceSharingFeatureFlagSetting.RESOURCE_SHARING_ENABLED, + ResourceSharingFeatureFlagSetting.LEGACY_RESOURCE_SHARING_ENABLED, + ResourceSharingProtectedResourcesSetting.PROTECTED_TYPES, + ResourceSharingProtectedResourcesSetting.LEGACY_PROTECTED_TYPES + ); + final Set> upgraders = Set.of( + ResourceSharingFeatureFlagSetting.RESOURCE_SHARING_ENABLED_UPGRADER, + ResourceSharingProtectedResourcesSetting.PROTECTED_TYPES_UPGRADER + ); + return new ClusterSettings(Settings.EMPTY, registered, upgraders); + } + + @Test + public void testFeatureFlagDefaultsToDisabled() { + assertFalse(ResourceSharingFeatureFlagSetting.RESOURCE_SHARING_ENABLED.get(Settings.EMPTY)); + } + + @Test + public void testFeatureFlagReadsCurrentKey() { + final Settings settings = Settings.builder().put(CURRENT_ENABLED, true).build(); + assertTrue(ResourceSharingFeatureFlagSetting.RESOURCE_SHARING_ENABLED.get(settings)); + } + + @Test + public void testFeatureFlagFallsBackToLegacyKey() { + final Settings settings = Settings.builder().put(LEGACY_ENABLED, true).build(); + assertTrue(ResourceSharingFeatureFlagSetting.RESOURCE_SHARING_ENABLED.get(settings)); + } + + @Test + public void testCurrentKeyWinsOverLegacyKey() { + final Settings settings = Settings.builder().put(LEGACY_ENABLED, true).put(CURRENT_ENABLED, false).build(); + assertFalse(ResourceSharingFeatureFlagSetting.RESOURCE_SHARING_ENABLED.get(settings)); + } + + @Test + public void testProtectedTypesFallsBackToLegacyKey() { + final Settings settings = Settings.builder().putList(LEGACY_TYPES, List.of("sample-resource", "ml-model-group")).build(); + assertEquals(List.of("sample-resource", "ml-model-group"), ResourceSharingProtectedResourcesSetting.PROTECTED_TYPES.get(settings)); + } + + @Test + public void testProtectedTypesCurrentKeyWinsOverLegacyKey() { + final Settings settings = Settings.builder() + .putList(LEGACY_TYPES, List.of("stale-type")) + .putList(CURRENT_TYPES, List.of("sample-resource")) + .build(); + assertEquals(List.of("sample-resource"), ResourceSharingProtectedResourcesSetting.PROTECTED_TYPES.get(settings)); + } + + @Test + public void testUpgradersTargetTheLegacySettings() { + assertEquals(LEGACY_ENABLED, ResourceSharingFeatureFlagSetting.RESOURCE_SHARING_ENABLED_UPGRADER.getSetting().getKey()); + assertEquals(CURRENT_ENABLED, ResourceSharingFeatureFlagSetting.RESOURCE_SHARING_ENABLED_UPGRADER.getKey(LEGACY_ENABLED)); + assertEquals(LEGACY_TYPES, ResourceSharingProtectedResourcesSetting.PROTECTED_TYPES_UPGRADER.getSetting().getKey()); + assertEquals(CURRENT_TYPES, ResourceSharingProtectedResourcesSetting.PROTECTED_TYPES_UPGRADER.getKey(LEGACY_TYPES)); + } + + @Test + public void testUpgradeSettingsRewritesLegacyKeys() { + final Settings upgraded = clusterSettings().upgradeSettings( + Settings.builder().put(LEGACY_ENABLED, true).putList(LEGACY_TYPES, List.of("sample-resource")).build() + ); + + assertFalse("legacy feature flag key should not survive the upgrade", upgraded.hasValue(LEGACY_ENABLED)); + assertFalse("legacy protected types key should not survive the upgrade", upgraded.hasValue(LEGACY_TYPES)); + assertEquals("true", upgraded.get(CURRENT_ENABLED)); + assertEquals(List.of("sample-resource"), upgraded.getAsList(CURRENT_TYPES)); + } + + @Test + public void testUpgradeSettingsLeavesCurrentKeysAlone() { + final Settings original = Settings.builder().put(CURRENT_ENABLED, true).putList(CURRENT_TYPES, List.of("sample-resource")).build(); + + assertEquals(original, clusterSettings().upgradeSettings(original)); + } + + @Test + public void testLegacyKeysPassNodeSettingValidation() { + // SettingsModule validates node settings from opensearch.yml against the registered settings, and an + // unrecognized key is what stops a node from starting. Registering the pre-graduation settings is what + // keeps such a node startable. + clusterSettings().validate( + Settings.builder().put(LEGACY_ENABLED, true).putList(LEGACY_TYPES, List.of("sample-resource")).build(), + true + ); + } + + @Test + public void testGenuinelyUnknownKeyStillFailsValidation() { + // Negative control for the check above: validation must still reject a key nothing registers. + final Settings settings = Settings.builder().put("plugins.security.experimental.resource_sharing.nonexistent", true).build(); + assertThrows(SettingsException.class, () -> clusterSettings().validate(settings, true)); + } + + @Test + public void testLegacySettingsAreMarkedDeprecated() { + assertTrue(ResourceSharingFeatureFlagSetting.LEGACY_RESOURCE_SHARING_ENABLED.isDeprecated()); + assertTrue(ResourceSharingProtectedResourcesSetting.LEGACY_PROTECTED_TYPES.isDeprecated()); + } +}