Keep the pre-graduation resource sharing setting names working, deprecated

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**
 

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<Setting<?>> 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<Setting<?>> getSettings() {
         return settings;
     }
 
+    @Override
+    public List<SettingUpgrader<?>> getSettingUpgraders() {
+        return List.of(
+            ResourceSharingFeatureFlagSetting.RESOURCE_SHARING_ENABLED_UPGRADER,
+            ResourceSharingProtectedResourcesSetting.PROTECTED_TYPES_UPGRADER
+        );
+    }
+
     @Override
     public List<String> getSettingsFilter() {
         List<String> settingsFilter = new ArrayList<>();

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<Boolean> {
     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<Boolean> 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<Boolean> 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<Boolean> RESOURCE_SHARING_ENABLED_UPGRADER = new SettingUpgrader<Boolean>() {
+        @Override
+        public Setting<Boolean> 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

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<List<String>> {
     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<List<String>> 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<List<String>> 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<List<String>> PROTECTED_TYPES_UPGRADER = new SettingUpgrader<List<String>>() {
+        @Override
+        public Setting<List<String>> 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

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<String> OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES_DEFAULT = List.of(); // defaults to no registered types as
                                                                                                       // protected
 

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<Setting<?>> registered = Set.of(
+            ResourceSharingFeatureFlagSetting.RESOURCE_SHARING_ENABLED,
+            ResourceSharingFeatureFlagSetting.LEGACY_RESOURCE_SHARING_ENABLED,
+            ResourceSharingProtectedResourcesSetting.PROTECTED_TYPES,
+            ResourceSharingProtectedResourcesSetting.LEGACY_PROTECTED_TYPES
+        );
+        final Set<SettingUpgrader<?>> 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());
+    }
+}