Phase 2 support for operator privileges: Cluster settings (#66684) (#68563)

Add a new OperatorDynamic enum to differentiate between operator-only and
regular dynamic cluster settings. The Setting constructor validates that Dynamic
and OperatorDynamic cannot be both specified. Operator-only settings behave
as the follows:

* When the feature is enabled, operator-only settings cannot be updated  with
   PUT cluster settings API unless the user is an operator.
* The restore behaviour for operator-only settings will be identical for either
  operator or non-operator user. That is, when operator privileges are enabled,
  operator-only settings will not be restored. Otherwise (if the feature is
  disabled), the behaviour is the same as of today.

Author: ywangd

server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java

@@ -56,6 +56,9 @@ public class RestoreSnapshotRequest extends MasterNodeRequest<RestoreSnapshotReq
     private Settings indexSettings = EMPTY_SETTINGS;
     private String[] ignoreIndexSettings = Strings.EMPTY_ARRAY;
 
+    // This field does not get serialised (except toString for debugging purpose) because it is always set locally by authz
+    private boolean skipOperatorOnlyState = false;
+
     @Nullable // if any snapshot UUID will do
     private String snapshotUuid;
 
@@ -455,6 +458,14 @@ public String snapshotUuid() {
         return snapshotUuid;
     }
 
+    public boolean skipOperatorOnlyState() {
+        return skipOperatorOnlyState;
+    }
+
+    public void skipOperatorOnlyState(boolean skipOperatorOnlyState) {
+        this.skipOperatorOnlyState = skipOperatorOnlyState;
+    }
+
     /**
      * Parses restore definition
      *
@@ -523,6 +534,12 @@ public RestoreSnapshotRequest source(Map<String, Object> source) {
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject();
+        toXContentFragment(builder, params);
+        builder.endObject();
+        return builder;
+    }
+
+    private void toXContentFragment(XContentBuilder builder, Params params) throws IOException {
         builder.startArray("indices");
         for (String index : indices) {
             builder.value(index);
@@ -552,8 +569,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
             builder.value(ignoreIndexSetting);
         }
         builder.endArray();
-        builder.endObject();
-        return builder;
     }
 
     @Override
@@ -578,20 +593,27 @@ public boolean equals(Object o) {
             Objects.equals(renameReplacement, that.renameReplacement) &&
             Objects.equals(indexSettings, that.indexSettings) &&
             Arrays.equals(ignoreIndexSettings, that.ignoreIndexSettings) &&
-            Objects.equals(snapshotUuid, that.snapshotUuid);
+            Objects.equals(snapshotUuid, that.snapshotUuid) &&
+            skipOperatorOnlyState == that.skipOperatorOnlyState;
     }
 
     @Override
     public int hashCode() {
         int result = Objects.hash(snapshot, repository, indicesOptions, renamePattern, renameReplacement, waitForCompletion,
-            includeGlobalState, partial, includeAliases, indexSettings, snapshotUuid);
+            includeGlobalState, partial, includeAliases, indexSettings, snapshotUuid, skipOperatorOnlyState);
         result = 31 * result + Arrays.hashCode(indices);
         result = 31 * result + Arrays.hashCode(ignoreIndexSettings);
         return result;
     }
 
     @Override
     public String toString() {
-        return Strings.toString(this);
+        return Strings.toString((ToXContentObject) (builder, params) -> {
+            builder.startObject();
+            toXContentFragment(builder, params);
+            builder.field("skipOperatorOnlyState", skipOperatorOnlyState);
+            builder.endObject();
+            return builder;
+        });
     }
 }

server/src/main/java/org/elasticsearch/common/settings/Setting.java

@@ -88,6 +88,11 @@ public enum Property {
          */
         Dynamic,
 
+        /**
+         * Operator only Dynamic setting
+         */
+        OperatorDynamic,
+
         /**
          * mark this setting as final, not updateable even when the context is not dynamic
          * ie. Setting this property on an index scoped setting will fail update when the index is closed
@@ -158,9 +163,13 @@ private Setting(Key key, @Nullable Setting<T> fallbackSetting, Function<Settings
             this.properties = EMPTY_PROPERTIES;
         } else {
             final EnumSet<Property> propertiesAsSet = EnumSet.copyOf(Arrays.asList(properties));
-            if (propertiesAsSet.contains(Property.Dynamic) && propertiesAsSet.contains(Property.Final)) {
+            if ((propertiesAsSet.contains(Property.Dynamic) || propertiesAsSet.contains(Property.OperatorDynamic))
+                && propertiesAsSet.contains(Property.Final)) {
                 throw new IllegalArgumentException("final setting [" + key + "] cannot be dynamic");
             }
+            if (propertiesAsSet.contains(Property.Dynamic) && propertiesAsSet.contains(Property.OperatorDynamic)) {
+                throw new IllegalArgumentException("setting [" + key + "] cannot be both dynamic and operator dynamic");
+            }
             checkPropertyRequiresIndexScope(propertiesAsSet, Property.NotCopyableOnResize);
             checkPropertyRequiresIndexScope(propertiesAsSet, Property.InternalIndex);
             checkPropertyRequiresIndexScope(propertiesAsSet, Property.PrivateIndex);
@@ -285,7 +294,14 @@ public final Key getRawKey() {
      * Returns <code>true</code> if this setting is dynamically updateable, otherwise <code>false</code>
      */
     public final boolean isDynamic() {
-        return properties.contains(Property.Dynamic);
+        return properties.contains(Property.Dynamic) || properties.contains(Property.OperatorDynamic);
+    }
+
+    /**
+     * Returns <code>true</code> if this setting is dynamically updateable by operators, otherwise <code>false</code>
+     */
+    public final boolean isOperatorOnly() {
+        return properties.contains(Property.OperatorDynamic);
     }
 
     /**

server/src/main/java/org/elasticsearch/snapshots/RestoreService.java

@@ -55,6 +55,7 @@
 import org.elasticsearch.common.lucene.Lucene;
 import org.elasticsearch.common.regex.Regex;
 import org.elasticsearch.common.settings.ClusterSettings;
+import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.Index;
 import org.elasticsearch.index.IndexSettings;
@@ -81,6 +82,7 @@
 import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import static java.util.Collections.unmodifiableSet;
 import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_AUTO_EXPAND_REPLICAS;
@@ -443,6 +445,22 @@ restoreUUID, snapshot, overallState(RestoreInProgress.State.INIT, shards),
                         if (request.includeGlobalState()) {
                             if (metadata.persistentSettings() != null) {
                                 Settings settings = metadata.persistentSettings();
+                                if (request.skipOperatorOnlyState()) {
+                                    // Skip any operator-only settings from the snapshot. This happens when operator privileges are enabled
+                                    Set<String> operatorSettingKeys = Stream.concat(
+                                        settings.keySet().stream(), currentState.metadata().persistentSettings().keySet().stream())
+                                        .filter(k -> {
+                                            final Setting<?> setting = clusterSettings.get(k);
+                                            return setting != null && setting.isOperatorOnly();
+                                        })
+                                        .collect(Collectors.toSet());
+                                    if (false == operatorSettingKeys.isEmpty()) {
+                                        settings = Settings.builder()
+                                            .put(settings.filter(k -> false == operatorSettingKeys.contains(k)))
+                                            .put(currentState.metadata().persistentSettings().filter(operatorSettingKeys::contains))
+                                            .build();
+                                    }
+                                }
                                 clusterSettings.validateUpdate(settings);
                                 mdBuilder.persistentSettings(settings);
                             }
@@ -457,6 +475,7 @@ restoreUUID, snapshot, overallState(RestoreInProgress.State.INIT, shards),
                                     if (RepositoriesMetadata.TYPE.equals(cursor.key) == false
                                             && DataStreamMetadata.TYPE.equals(cursor.key) == false
                                             && cursor.value instanceof Metadata.NonRestorableCustom == false) {
+                                        // TODO: Check request.skipOperatorOnly for Autoscaling policies (NonRestorableCustom)
                                         // Don't restore repositories while we are working with them
                                         // TODO: Should we restore them at the end?
                                         // Also, don't restore data streams here, we already added them to the metadata builder above

server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java

@@ -10,6 +10,7 @@
 
 import org.elasticsearch.action.support.IndicesOptions;
 import org.elasticsearch.common.bytes.BytesReference;
+import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
 import org.elasticsearch.common.xcontent.ToXContent;
@@ -29,6 +30,8 @@
 import java.util.List;
 import java.util.Map;
 
+import static org.hamcrest.Matchers.containsString;
+
 public class RestoreSnapshotRequestTests extends AbstractWireSerializingTestCase<RestoreSnapshotRequest> {
     private RestoreSnapshotRequest randomState(RestoreSnapshotRequest instance) {
         if (randomBoolean()) {
@@ -123,4 +126,37 @@ public void testSource() throws IOException {
 
         assertEquals(original, processed);
     }
+
+    public void testSkipOperatorOnlyWillNotBeSerialised() throws IOException {
+        RestoreSnapshotRequest original = createTestInstance();
+        assertFalse(original.skipOperatorOnlyState()); // default is false
+        if (randomBoolean()) {
+            original.skipOperatorOnlyState(true);
+        }
+        Map<String, Object> map = convertRequestToMap(original);
+        // It is not serialised as xcontent
+        assertFalse(map.containsKey("skip_operator_only"));
+
+        // Xcontent is not affected by the value of skipOperatorOnlyState
+        original.skipOperatorOnlyState(original.skipOperatorOnlyState() == false);
+        assertEquals(map, convertRequestToMap(original));
+
+        // Nor does it serialise to streamInput
+        final BytesStreamOutput streamOutput = new BytesStreamOutput();
+        original.writeTo(streamOutput);
+        final RestoreSnapshotRequest deserialized = new RestoreSnapshotRequest(streamOutput.bytes().streamInput());
+        assertFalse(deserialized.skipOperatorOnlyState());
+    }
+
+    public void testToStringWillIncludeSkipOperatorOnlyState() {
+        RestoreSnapshotRequest original = createTestInstance();
+        assertThat(original.toString(), containsString("skipOperatorOnlyState"));
+    }
+
+    private Map<String, Object> convertRequestToMap(RestoreSnapshotRequest request) throws IOException {
+        XContentBuilder builder = request.toXContent(XContentFactory.jsonBuilder(), new ToXContent.MapParams(Collections.emptyMap()));
+        XContentParser parser = XContentType.JSON.xContent().createParser(
+            NamedXContentRegistry.EMPTY, null, BytesReference.bytes(builder).streamInput());
+        return parser.mapOrdered();
+    }
 }

server/src/test/java/org/elasticsearch/common/settings/SettingTests.java

@@ -939,10 +939,16 @@ public void testRejectNullProperties() {
 
     public void testRejectConflictingDynamicAndFinalProperties() {
         IllegalArgumentException ex = expectThrows(IllegalArgumentException.class,
-            () -> Setting.simpleString("foo.bar", Property.Final, Property.Dynamic));
+            () -> Setting.simpleString("foo.bar", Property.Final, randomFrom(Property.Dynamic, Property.OperatorDynamic)));
         assertThat(ex.getMessage(), containsString("final setting [foo.bar] cannot be dynamic"));
     }
 
+    public void testRejectConflictingDynamicAndOperatorDynamicProperties() {
+        IllegalArgumentException ex = expectThrows(IllegalArgumentException.class,
+            () -> Setting.simpleString("foo.bar", Property.Dynamic, Property.OperatorDynamic));
+        assertThat(ex.getMessage(), containsString("setting [foo.bar] cannot be both dynamic and operator dynamic"));
+    }
+
     public void testRejectNonIndexScopedNotCopyableOnResizeSetting() {
         final IllegalArgumentException e = expectThrows(
                 IllegalArgumentException.class,
@@ -1233,4 +1239,11 @@ public boolean innerMatch(LogEvent event) {
             mockLogAppender.stop();
         }
     }
+
+    public void testDynamicTest() {
+        final Property property = randomFrom(Property.Dynamic, Property.OperatorDynamic);
+        final Setting<String> setting = Setting.simpleString("foo.bar", property);
+        assertTrue(setting.isDynamic());
+        assertEquals(setting.isOperatorOnly(), property == Property.OperatorDynamic);
+    }
 }

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java

@@ -401,7 +401,7 @@ public Set<DiscoveryNodeRole> getRoles() {
     public static final String MACHINE_MEMORY_NODE_ATTR = "ml.machine_memory";
     public static final String MAX_JVM_SIZE_NODE_ATTR = "ml.max_jvm_size";
     public static final Setting<Integer> CONCURRENT_JOB_ALLOCATIONS =
-            Setting.intSetting("xpack.ml.node_concurrent_job_allocations", 2, 0, Property.Dynamic, Property.NodeScope);
+            Setting.intSetting("xpack.ml.node_concurrent_job_allocations", 2, 0, Property.OperatorDynamic, Property.NodeScope);
     /**
      * The amount of memory needed to load the ML native code shared libraries. The assumption is that the first
      * ML job to run on a given node will do this, and then subsequent ML jobs on the same node will reuse the
@@ -415,7 +415,7 @@ public Set<DiscoveryNodeRole> getRoles() {
     // controls the types of jobs that can be created, and each job alone is considerably smaller than what each node
     // can handle.
     public static final Setting<Integer> MAX_MACHINE_MEMORY_PERCENT =
-           Setting.intSetting("xpack.ml.max_machine_memory_percent", 30, 5, 200, Property.Dynamic, Property.NodeScope);
+           Setting.intSetting("xpack.ml.max_machine_memory_percent", 30, 5, 200, Property.OperatorDynamic, Property.NodeScope);
     /**
      * This boolean value indicates if `max_machine_memory_percent` should be ignored and a automatic calculation is used instead.
      *
@@ -431,10 +431,10 @@ public Set<DiscoveryNodeRole> getRoles() {
     public static final Setting<Boolean> USE_AUTO_MACHINE_MEMORY_PERCENT = Setting.boolSetting(
         "xpack.ml.use_auto_machine_memory_percent",
         false,
-        Property.Dynamic,
+        Property.OperatorDynamic,
         Property.NodeScope);
     public static final Setting<Integer> MAX_LAZY_ML_NODES =
-            Setting.intSetting("xpack.ml.max_lazy_ml_nodes", 0, 0, Property.Dynamic, Property.NodeScope);
+            Setting.intSetting("xpack.ml.max_lazy_ml_nodes", 0, 0, Property.OperatorDynamic, Property.NodeScope);
 
     // Before 8.0.0 this needs to match the max allowed value for xpack.ml.max_open_jobs,
     // as the current node could be running in a cluster where some nodes are still using
@@ -449,7 +449,7 @@ public Set<DiscoveryNodeRole> getRoles() {
 
     public static final Setting<TimeValue> PROCESS_CONNECT_TIMEOUT =
         Setting.timeSetting("xpack.ml.process_connect_timeout", TimeValue.timeValueSeconds(10),
-            TimeValue.timeValueSeconds(5), Setting.Property.Dynamic, Setting.Property.NodeScope);
+            TimeValue.timeValueSeconds(5), Property.OperatorDynamic, Setting.Property.NodeScope);
 
     // Undocumented setting for integration test purposes
     public static final Setting<ByteSizeValue> MIN_DISK_SPACE_OFF_HEAP =
@@ -468,7 +468,7 @@ public Set<DiscoveryNodeRole> getRoles() {
                 }
                 return value;
             },
-            Property.Dynamic,
+            Property.OperatorDynamic,
             Property.NodeScope
         );
 
@@ -481,7 +481,7 @@ public Set<DiscoveryNodeRole> getRoles() {
     public static final Setting<ByteSizeValue> MAX_ML_NODE_SIZE = Setting.byteSizeSetting(
         "xpack.ml.max_ml_node_size",
         ByteSizeValue.ZERO,
-        Property.Dynamic,
+        Property.OperatorDynamic,
         Property.NodeScope);
 
     private static final Logger logger = LogManager.getLogger(MachineLearning.class);

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlConfigMigrationEligibilityCheck.java

@@ -27,7 +27,7 @@ public class MlConfigMigrationEligibilityCheck {
     private static final Version MIN_NODE_VERSION = Version.V_6_6_0;
 
     public static final Setting<Boolean> ENABLE_CONFIG_MIGRATION = Setting.boolSetting(
-        "xpack.ml.enable_config_migration", true, Setting.Property.Dynamic, Setting.Property.NodeScope);
+        "xpack.ml.enable_config_migration", true, Setting.Property.OperatorDynamic, Setting.Property.NodeScope);
 
     private volatile boolean isConfigMigrationEnabled;
 

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/utils/persistence/ResultsPersisterService.java

@@ -77,7 +77,7 @@ public class ResultsPersisterService {
         20,
         0,
         50,
-        Setting.Property.Dynamic,
+        Setting.Property.OperatorDynamic,
         Setting.Property.NodeScope);
     private static final int MAX_RETRY_SLEEP_MILLIS = (int)Duration.ofMinutes(15).toMillis();
     private static final int MIN_RETRY_SLEEP_MILLIS = 50;

x-pack/plugin/security/qa/operator-privileges-tests/build.gradle

@@ -16,6 +16,13 @@ dependencies {
   javaRestTestImplementation project.sourceSets.main.runtimeClasspath
 }
 
+File repoDir = file("$buildDir/testclusters/repo")
+
+javaRestTest {
+  /* To support taking snapshots, we have to set path.repo setting */
+  systemProperty 'tests.path.repo', repoDir
+}
+
 testClusters.all {
   testDistribution = 'DEFAULT'
   numberOfNodes = 3
@@ -27,6 +34,7 @@ testClusters.all {
   setting 'xpack.security.enabled', 'true'
   setting 'xpack.security.http.ssl.enabled', 'false'
   setting 'xpack.security.operator_privileges.enabled', "true"
+  setting 'path.repo', repoDir.absolutePath
 
   user username: "test_admin", password: 'x-pack-test-password', role: "superuser"
   user username: "test_operator", password: 'x-pack-test-password', role: "limited_operator"