Skip to content

Commit 568189b

Browse files
authored
Add new setting property 'Sensitive' for tiering dynamic settings (opensearch-project#20901)
Signed-off-by: Craig Perkins <cwperx@amazon.com>
1 parent 6272fe0 commit 568189b

6 files changed

Lines changed: 67 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
2121
- WLM group custom search settings - groundwork and timeout ([#20536](https://github.com/opensearch-project/OpenSearch/issues/20536))
2222
- Expose JVM runtime metrics via telemetry framework ([#20844](https://github.com/opensearch-project/OpenSearch/pull/20844))
2323
- Add intra segment support for single-value metric aggregations ([#20503](https://github.com/opensearch-project/OpenSearch/pull/20503))
24+
- Add new setting property 'Sensitive' for tiering dynamic settings ([#20901](https://github.com/opensearch-project/OpenSearch/pull/20901))
2425
- Add ref_path support for package-based hunspell dictionary loading ([#20840](https://github.com/opensearch-project/OpenSearch/pull/20840))
2526
- Add support for enabling pluggable data formats, starting with phase-1 of decoupling shard from engine, and introducing basic abstractions ([#20675](https://github.com/opensearch-project/OpenSearch/pull/20675))
2627

server/src/main/java/org/opensearch/action/admin/cluster/settings/SettingsUpdater.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ synchronized ClusterState updateSettings(
7676
final Settings persistentToApply,
7777
final Logger logger
7878
) {
79+
validateNoTransientSensitiveSettings(transientToApply);
7980
boolean changed = false;
8081

8182
/*
@@ -212,4 +213,12 @@ private void logInvalidSetting(
212213
);
213214
}
214215

216+
private void validateNoTransientSensitiveSettings(final Settings transientSettings) {
217+
for (String key : transientSettings.keySet()) {
218+
if (clusterSettings.isSensitiveSetting(key)) {
219+
throw new IllegalArgumentException("sensitive setting [" + key + "] must be updated using persistent settings");
220+
}
221+
}
222+
}
223+
215224
}

server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -775,6 +775,15 @@ public boolean isUnmodifiableOnRestoreSetting(String key) {
775775
return setting != null && setting.isUnmodifiableOnRestore();
776776
}
777777

778+
/**
779+
* Returns <code>true</code> if the setting for the given key is sensitive, meaning it requires
780+
* security admin privileges to be updated dynamically. Otherwise <code>false</code>.
781+
*/
782+
public boolean isSensitiveSetting(String key) {
783+
final Setting<?> setting = get(key);
784+
return setting != null && setting.isSensitive();
785+
}
786+
778787
/**
779788
* Returns a settings object that contains all settings that are not
780789
* already set in the given source. The diff contains either the default value for each

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

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,15 @@ public enum Property {
177177
* Mark this setting as immutable on snapshot restore
178178
* i.e. the setting will not be allowed to be removed or modified during restore
179179
*/
180-
UnmodifiableOnRestore
180+
UnmodifiableOnRestore,
181+
182+
/**
183+
* Marks a setting as sensitive. Can only be applied to dynamic settings.
184+
* The Sensitive property has default enforcement but enables plugins to implement
185+
* different policies for these settings. In practice the security plugin will
186+
* require higher privileges for modifying sensitive settings.
187+
*/
188+
Sensitive
181189
}
182190

183191
private final Key key;
@@ -217,6 +225,9 @@ private Setting(
217225
} else if (propertiesAsSet.contains(Property.UnmodifiableOnRestore) && propertiesAsSet.contains(Property.Dynamic)) {
218226
throw new IllegalArgumentException("UnmodifiableOnRestore setting [" + key + "] cannot be dynamic");
219227
}
228+
if (propertiesAsSet.contains(Property.Sensitive) && propertiesAsSet.contains(Property.Dynamic) == false) {
229+
throw new IllegalArgumentException("sensitive setting [" + key + "] must be dynamic");
230+
}
220231
checkPropertyRequiresIndexScope(propertiesAsSet, Property.NotCopyableOnResize);
221232
checkPropertyRequiresIndexScope(propertiesAsSet, Property.InternalIndex);
222233
checkPropertyRequiresIndexScope(propertiesAsSet, Property.PrivateIndex);
@@ -361,6 +372,14 @@ public final boolean isUnmodifiableOnRestore() {
361372
return properties.contains(Property.UnmodifiableOnRestore);
362373
}
363374

375+
/**
376+
* Returns <code>true</code> if this setting is sensitive, meaning it requires security admin
377+
* privileges to be updated dynamically. Otherwise <code>false</code>.
378+
*/
379+
public final boolean isSensitive() {
380+
return properties.contains(Property.Sensitive);
381+
}
382+
364383
public final boolean isInternalIndex() {
365384
return properties.contains(Property.InternalIndex);
366385
}

server/src/test/java/org/opensearch/action/admin/cluster/settings/SettingsUpdaterTests.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -670,4 +670,24 @@ public void testUpdateOfValidationDependentSettings() {
670670
assertThat(exception.getMessage(), equalTo("[high]=2 is lower than [low]=5"));
671671
}
672672

673+
public void testRejectSensitiveSettingInTransient() {
674+
Setting<String> sensitiveSetting = Setting.simpleString(
675+
"sensitive.setting",
676+
Property.Dynamic,
677+
Property.NodeScope,
678+
Property.Sensitive
679+
);
680+
final Set<Setting<?>> settingsSet = Stream.concat(ClusterSettings.BUILT_IN_CLUSTER_SETTINGS.stream(), Stream.of(sensitiveSetting))
681+
.collect(Collectors.toSet());
682+
ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, settingsSet);
683+
SettingsUpdater updater = new SettingsUpdater(clusterSettings);
684+
ClusterState state = ClusterState.builder(new ClusterName("foo")).metadata(Metadata.builder().build()).build();
685+
686+
IllegalArgumentException ex = expectThrows(
687+
IllegalArgumentException.class,
688+
() -> updater.updateSettings(state, Settings.builder().put("sensitive.setting", "value").build(), Settings.EMPTY, logger)
689+
);
690+
assertThat(ex.getMessage(), equalTo("sensitive setting [sensitive.setting] must be updated using persistent settings"));
691+
}
692+
673693
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1447,6 +1447,14 @@ public void testRejectConflictingDynamicAndUnmodifiableOnRestoreProperties() {
14471447
assertThat(ex.getMessage(), containsString("UnmodifiableOnRestore setting [foo.bar] cannot be dynamic"));
14481448
}
14491449

1450+
public void testRejectSensitiveWithoutDynamic() {
1451+
IllegalArgumentException ex = expectThrows(
1452+
IllegalArgumentException.class,
1453+
() -> Setting.simpleString("foo.bar", Property.Sensitive, Property.NodeScope)
1454+
);
1455+
assertThat(ex.getMessage(), containsString("sensitive setting [foo.bar] must be dynamic"));
1456+
}
1457+
14501458
public void testRejectNonIndexScopedUnmodifiableOnRestoreSetting() {
14511459
final IllegalArgumentException e = expectThrows(
14521460
IllegalArgumentException.class,

0 commit comments

Comments
 (0)