From bb835aaeb78e897850f612eec240bccc2678e1a0 Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Wed, 16 Sep 2026 00:08:10 +0000 Subject: [PATCH] Keep the pre-graduation resource sharing setting names working, deprecated Graduating resource sharing out of experimental (#6348) renamed the two feature-flag settings and dropped the old names outright: plugins.security.experimental.resource_sharing.enabled -> plugins.security.resource_sharing.enabled plugins.security.experimental.resource_sharing.protected_types -> plugins.security.resource_sharing.protected_types An existing cluster therefore had to be reconfigured before upgrading. A node whose opensearch.yml still carried an old key would not start, and a persistent cluster setting under an old key was archived on upgrade, silently returning the feature to its default of disabled. Consumers then fall back to their own access control without reporting an error, so shares stop being honored with no signal. This restores the old names as deprecated aliases instead, matching how the plugin already handles the opendistro-to-opensearch rename of ssl_dual_mode_enabled: - Each current setting now declares its pre-graduation setting as a fallback. The fallback resolves at read time against whichever settings instance is supplied, so it covers opensearch.yml, cluster settings and dynamic updates alike. The current name takes precedence when both are set, so a leftover old key cannot override a deliberate new one. - The old settings are registered and marked Property.Deprecated, which is also what lets the upgraders below resolve them. - A setting upgrader per setting rewrites an old key to the current one during cluster-state recovery and on any cluster settings update that still uses it, so an upgraded cluster stops carrying the deprecated key rather than keeping it indefinitely. - Because the deprecation warning core emits does not name a replacement by design, each setting logs an explicit warning naming the current key when the old one is in use. Users keep working through an upgrade, are told the old names are going away, and are pointed at the replacement. Testing: new ResourceSharingSettingMigrationTests covers fallback resolution for both settings, precedence when both keys are set, the upgraders' key mapping, and upgradeSettings rewriting the old keys while leaving current ones untouched. Verified green, then confirmed the fallback tests fail when the fallback wiring is removed. Signed-off-by: Darshit Chanpura --- RESOURCE_SHARING_AND_ACCESS_CONTROL.md | 14 +- .../security/OpenSearchSecurityPlugin.java | 14 ++ .../ResourceSharingFeatureFlagSetting.java | 56 ++++++- ...ourceSharingProtectedResourcesSetting.java | 51 ++++++- .../security/support/ConfigConstants.java | 16 ++ .../ResourceSharingSettingMigrationTests.java | 142 ++++++++++++++++++ 6 files changed, 286 insertions(+), 7 deletions(-) create mode 100644 src/test/java/org/opensearch/security/resources/settings/ResourceSharingSettingMigrationTests.java 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()); + } +}