Skip to content

Update Security Version Handling experimental feature to only track diffs between changes to security configuration #5790

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
cwperks wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Update Security Version Handling experimental feature to only track diffs between changes to security configuration #5790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3cab091
Update Security Version Handling experimental feature to only track d…
cwperks Nov 14, 2025
655e369
Create with mappings
cwperks Nov 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 0 additions & 122 deletions src/integrationTest/java/org/opensearch/security/api/RollbackVersionApiIntegrationTest.java
View file
Open in desktop

This file was deleted.

32 changes: 22 additions & 10 deletions src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,12 @@
import org.opensearch.security.configuration.AdminDNs;
import org.opensearch.security.configuration.ClusterInfoHolder;
import org.opensearch.security.configuration.CompatConfig;
import org.opensearch.security.configuration.ConfigurationChangeListener;
import org.opensearch.security.configuration.ConfigurationMap;
import org.opensearch.security.configuration.ConfigurationRepository;
import org.opensearch.security.configuration.DlsFlsRequestValve;
import org.opensearch.security.configuration.DlsFlsValveImpl;
import org.opensearch.security.configuration.PrivilegesInterceptorImpl;
import org.opensearch.security.configuration.SecurityConfigVersionHandler;
import org.opensearch.security.configuration.SecurityFlsDlsIndexSearcherWrapper;
import org.opensearch.security.dlic.rest.api.Endpoint;
import org.opensearch.security.dlic.rest.api.SecurityRestApiActions;
Expand Down Expand Up @@ -1163,7 +1164,12 @@ public Collection<Object> createComponents(
final XFFResolver xffResolver = new XFFResolver(threadPool);
backendRegistry = new BackendRegistry(settings, adminDns, xffResolver, auditLog, threadPool, cih);
backendRegistry.registerClusterSettingsChangeListener(clusterService.getClusterSettings());
cr.subscribeOnChange(configMap -> { backendRegistry.invalidateCache(); });
cr.subscribeOnChange(new ConfigurationChangeListener() {
@Override
public void onChange(ConfigurationMap configMap) {
backendRegistry.invalidateCache();
}
});
tokenManager = new SecurityTokenManager(cs, threadPool, userService);

final CompatConfig compatConfig = new CompatConfig(environment, transportPassiveAuthSetting);
Expand Down Expand Up @@ -1200,7 +1206,13 @@ public Collection<Object> createComponents(
resourcePluginInfo,
resourceSharingEnabledSetting
);
cr.subscribeOnChange(configMap -> { ((DlsFlsValveImpl) dlsFlsValve).updateConfiguration(cr.getConfiguration(CType.ROLES)); });
cr.subscribeOnChange(new ConfigurationChangeListener() {
@Override
public void onChange(ConfigurationMap configMap) {
((DlsFlsValveImpl) dlsFlsValve).updateConfiguration(cr.getConfiguration(CType.ROLES));
;
}
});
}

resourceAccessHandler = new ResourceAccessHandler(threadPool, rsIndexHandler, adminDns, evaluator, resourcePluginInfo);
Expand Down Expand Up @@ -2365,13 +2377,13 @@ public Collection<SystemIndexDescriptor> getSystemIndexDescriptors(Settings sett
systemIndexDescriptors.add(resourceSharingIndexDescriptor);
}

if (SecurityConfigVersionHandler.isVersionIndexEnabled(settings)) {
final String securityVersionsIndexPattern = settings.get(
ConfigConstants.SECURITY_CONFIG_VERSIONS_INDEX_NAME,
ConfigConstants.OPENSEARCH_SECURITY_DEFAULT_CONFIG_VERSIONS_INDEX
);
systemIndexDescriptors.add(new SystemIndexDescriptor(securityVersionsIndexPattern, "Security config versions index"));
}
// if (SecurityConfigVersionHandler.isVersionIndexEnabled(settings)) {
// final String securityVersionsIndexPattern = settings.get(
// ConfigConstants.SECURITY_CONFIG_VERSIONS_INDEX_NAME,
// ConfigConstants.OPENSEARCH_SECURITY_DEFAULT_CONFIG_VERSIONS_INDEX
// );
// systemIndexDescriptors.add(new SystemIndexDescriptor(securityVersionsIndexPattern, "Security config versions index"));
// }

return ImmutableList.copyOf(systemIndexDescriptors);
}
Expand Down
12 changes: 9 additions & 3 deletions src/main/java/org/opensearch/security/configuration/ConfigurationChangeListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,20 @@

package org.opensearch.security.configuration;

import com.fasterxml.jackson.databind.JsonNode;

/**
* Callback function on change particular configuration
*/
@FunctionalInterface
public interface ConfigurationChangeListener {

/**
* @param configuration not null updated configuration on that was subscribe current listener
* @param typeToConfig not null updated configuration on that was subscribe current listener
*/
default void onChange(ConfigurationMap typeToConfig) {}

/**
* @param diff diff between old and new configuration
*/
void onChange(ConfigurationMap typeToConfig);
default void onChange(JsonNode diff) {}
}
44 changes: 39 additions & 5 deletions src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@
import com.google.common.cache.CacheBuilder;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

Expand Down Expand Up @@ -80,6 +82,7 @@
import org.opensearch.env.Environment;
import org.opensearch.index.shard.IndexEventListener;
import org.opensearch.index.shard.IndexShard;
import org.opensearch.security.DefaultObjectMapper;
import org.opensearch.security.auditlog.AuditLog;
import org.opensearch.security.auditlog.config.AuditConfig;
import org.opensearch.security.securityconf.DynamicConfigFactory;
Expand All @@ -93,6 +96,8 @@
import org.opensearch.threadpool.ThreadPool;
import org.opensearch.transport.client.Client;

import com.flipkart.zjsonpatch.JsonDiff;

import static org.opensearch.security.support.ConfigConstants.SECURITY_ALLOW_DEFAULT_INIT_USE_CLUSTER_STATE;
import static org.opensearch.security.support.SnapshotRestoreHelper.isSecurityIndexRestoredFromSnapshot;

Expand Down Expand Up @@ -409,7 +414,7 @@ Future<Void> executeConfigurationInitialization(final SecurityMetadata securityM
if (initializationInProcess.compareAndSet(false, true)) {
return threadPool.generic().submit(() -> {
securityIndexHandler.loadConfiguration(securityMetadata.configuration(), ActionListener.wrap(cTypeConfigs -> {
notifyConfigurationListeners(cTypeConfigs);
notifyConfigurationListeners(cTypeConfigs, CType.values());
final var auditConfigDocPresent = cTypeConfigs.containsKey(CType.AUDIT) && cTypeConfigs.get(CType.AUDIT).notEmpty();
setupAuditConfigurationIfAny(auditConfigDocPresent);
auditHotReloadingEnabled.getAndSet(auditConfigDocPresent);
Expand Down Expand Up @@ -545,23 +550,52 @@ public void reloadConfiguration(Collection<CType<?>> configTypes, ActionListener
*/
private void doReload(Set<CType<?>> configTypes) {
ConfigurationMap loaded = getConfigurationsFromIndex(configTypes, false, acceptInvalid);
notifyConfigurationListeners(loaded);
notifyConfigurationListeners(loaded, configTypes);
}

private void notifyConfigurationListeners(ConfigurationMap configuration) {
private void notifyConfigurationListeners(ConfigurationMap configuration, Set<CType<?>> configTypes) {
JsonNode diff = null;
// diff only computed on active cluster manager
if (clusterService.state().nodes().isLocalNodeElectedClusterManager()) {
diff = computeConfigDiff(configuration, configTypes);
System.out.println("computed diff: " + diff);
}
configCache.putAll(configuration.rawMap());
notifyAboutChanges(configuration);
notifyAboutChanges(configuration, diff);
}

private JsonNode computeConfigDiff(ConfigurationMap newConfiguration, Set<CType<?>> configTypes) {
try {
Map<CType<?>, SecurityDynamicConfiguration<?>> filteredOldConfig = configCache.asMap()
.entrySet()
.stream()
.filter(e -> configTypes.contains(e.getKey()))
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
JsonNode oldNode = DefaultObjectMapper.objectMapper.valueToTree(filteredOldConfig);
JsonNode newNode = DefaultObjectMapper.objectMapper.valueToTree(newConfiguration.rawMap());
JsonNode forwardDiff = JsonDiff.asJson(oldNode, newNode);
JsonNode reverseDiff = JsonDiff.asJson(newNode, oldNode);
ObjectNode result = DefaultObjectMapper.objectMapper.createObjectNode();
result.set("forwardDiff", forwardDiff);
result.set("reverseDiff", reverseDiff);

return result;
} catch (Exception e) {
LOGGER.error("Failed to compute config diff", e);
return DefaultObjectMapper.objectMapper.createArrayNode();
}
}

public synchronized void subscribeOnChange(ConfigurationChangeListener listener) {
configurationChangedListener.add(listener);
}

private synchronized void notifyAboutChanges(ConfigurationMap typeToConfig) {
private synchronized void notifyAboutChanges(ConfigurationMap typeToConfig, JsonNode diff) {
for (ConfigurationChangeListener listener : configurationChangedListener) {
try {
LOGGER.debug("Notify {} listener about change configuration with type {}", listener, typeToConfig);
listener.onChange(typeToConfig);
listener.onChange(diff);
} catch (Exception e) {
LOGGER.error("{} listener errored: " + e, listener, e);
throw ExceptionsHelper.convertToOpenSearchException(e);
Expand Down
Loading
Loading