From f33bd8e78bf6d68d88efaeb2ba7c7f7256b62eb9 Mon Sep 17 00:00:00 2001 From: Nagaraj G Date: Thu, 31 Jul 2025 15:12:42 +0530 Subject: [PATCH] Security init improvements Signed-off-by: Nagaraj G --- .../ConfigurationRepository.java | 39 +++++++++++++------ .../security/support/ConfigConstants.java | 3 ++ .../security/support/ConfigHelper.java | 2 + 3 files changed, 32 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java b/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java index 737f86ba74..cd96d8ddf4 100644 --- a/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java +++ b/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java @@ -96,11 +96,15 @@ import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.client.Client; +import static org.opensearch.security.support.ConfigConstants.DEFAULT_TIMEOUT; import static org.opensearch.security.support.ConfigConstants.SECURITY_ALLOW_DEFAULT_INIT_USE_CLUSTER_STATE; import static org.opensearch.security.support.SnapshotRestoreHelper.isSecurityIndexRestoredFromSnapshot; public class ConfigurationRepository implements ClusterStateListener, IndexEventListener { private static final Logger LOGGER = LogManager.getLogger(ConfigurationRepository.class); + private static final int INITIALIZATION_RETRY_INTERVAL_MS = 3000; + private static final int INITIALIZATION_MAX_DURATION_MS = 5 * 60 * 1000; + private static final int MAX_RETRIES = INITIALIZATION_MAX_DURATION_MS / INITIALIZATION_RETRY_INTERVAL_MS; private final String securityIndex; private final Client client; @@ -273,26 +277,33 @@ private void initalizeClusterConfiguration(final boolean installDefaultConfig) { LOGGER.error("{} does not exist", confFile.getAbsolutePath()); } } catch (Exception e) { - LOGGER.error("Cannot apply default config (this is maybe not an error!)", e); + LOGGER.error("Cannot apply default config", e); + throw e; } } - while (!dynamicConfigFactory.isInitialized()) { + for (int retryCount = 0; retryCount < MAX_RETRIES && !dynamicConfigFactory.isInitialized(); retryCount++) { try { - LOGGER.debug("Try to load config ..."); + LOGGER.info("Try to load config ..."); reloadConfiguration(CType.values(), true); break; } catch (Exception e) { - LOGGER.debug("Unable to load configuration due to {}", String.valueOf(ExceptionUtils.getRootCause(e))); + LOGGER.error("Unable to load configuration due to {}", String.valueOf(ExceptionUtils.getRootCause(e))); try { - TimeUnit.MILLISECONDS.sleep(3000); + TimeUnit.MILLISECONDS.sleep(INITIALIZATION_RETRY_INTERVAL_MS); } catch (InterruptedException e1) { Thread.currentThread().interrupt(); - LOGGER.debug("Thread was interrupted so we cancel initialization"); + LOGGER.error("Thread was interrupted so we cancel initialization"); break; } } } + + if (!dynamicConfigFactory.isInitialized()) { + LOGGER.error("Node '{}' failed to initialize", clusterService.localNode().getName()); + throw new IllegalStateException(String.format("Node '%s' failed to initialize", clusterService.localNode().getName())); + } + setupAuditConfigurationIfAny(cl.isAuditConfigDocPresentInIndex()); LOGGER.info("Node '{}' initialized", clusterService.localNode().getName()); @@ -325,7 +336,8 @@ private void setupAuditConfigurationIfAny(final boolean auditConfigDocPresent) { private boolean createSecurityIndexIfAbsent() { try { final Map indexSettings = ImmutableMap.of("index.number_of_shards", 1, "index.auto_expand_replicas", "0-all"); - final CreateIndexRequest createIndexRequest = new CreateIndexRequest(securityIndex).settings(indexSettings); + final CreateIndexRequest createIndexRequest = new CreateIndexRequest(securityIndex).timeout(DEFAULT_TIMEOUT) + .settings(indexSettings); final boolean ok = client.admin().indices().create(createIndexRequest).actionGet().isAcknowledged(); LOGGER.info("Index {} created?: {}", securityIndex, ok); return ok; @@ -341,14 +353,14 @@ private void waitForSecurityIndexToBeAtLeastYellow() { try { response = client.admin() .cluster() - .health(new ClusterHealthRequest(securityIndex).waitForActiveShards(1).waitForYellowStatus()) + .health(new ClusterHealthRequest(securityIndex).waitForActiveShards(1).waitForYellowStatus().timeout(DEFAULT_TIMEOUT)) .actionGet(); } catch (Exception e) { - LOGGER.debug("Caught a {} but we just try again ...", e.toString()); + LOGGER.error("Caught a {} but we just try again ...", e.toString()); } while (response == null || response.isTimedOut() || response.getStatus() == ClusterHealthStatus.RED) { - LOGGER.debug( + LOGGER.error( "index '{}' not healthy yet, we try again ... (Reason: {})", securityIndex, response == null ? "no response" : (response.isTimedOut() ? "timeout" : "other, maybe red cluster") @@ -360,9 +372,12 @@ private void waitForSecurityIndexToBeAtLeastYellow() { Thread.currentThread().interrupt(); } try { - response = client.admin().cluster().health(new ClusterHealthRequest(securityIndex).waitForYellowStatus()).actionGet(); + response = client.admin() + .cluster() + .health(new ClusterHealthRequest(securityIndex).waitForYellowStatus().timeout(DEFAULT_TIMEOUT)) + .actionGet(); } catch (Exception e) { - LOGGER.debug("Caught again a {} but we just try again ...", e.toString()); + LOGGER.error("Caught again a {} but we just try again ...", e.toString()); } } } diff --git a/src/main/java/org/opensearch/security/support/ConfigConstants.java b/src/main/java/org/opensearch/security/support/ConfigConstants.java index 81202e47fe..b51279ab8b 100644 --- a/src/main/java/org/opensearch/security/support/ConfigConstants.java +++ b/src/main/java/org/opensearch/security/support/ConfigConstants.java @@ -36,6 +36,7 @@ import com.google.common.collect.ImmutableSet; import org.opensearch.common.settings.Settings; +import org.opensearch.common.unit.TimeValue; import org.opensearch.security.auditlog.impl.AuditCategory; import com.password4j.types.Hmac; @@ -426,6 +427,8 @@ public enum RolesMappingResolution { public static final String OPENSEARCH_RESOURCE_SHARING_ENABLED = "plugins.security.experimental.resource_sharing.enabled"; public static final boolean OPENSEARCH_RESOURCE_SHARING_ENABLED_DEFAULT = false; + public static final TimeValue DEFAULT_TIMEOUT = TimeValue.timeValueSeconds(30); + public static Set getSettingAsSet( final Settings settings, final String key, diff --git a/src/main/java/org/opensearch/security/support/ConfigHelper.java b/src/main/java/org/opensearch/security/support/ConfigHelper.java index d2cb6fc8ff..bde4281785 100644 --- a/src/main/java/org/opensearch/security/support/ConfigHelper.java +++ b/src/main/java/org/opensearch/security/support/ConfigHelper.java @@ -56,6 +56,7 @@ import org.opensearch.transport.client.Client; import static org.opensearch.core.xcontent.DeprecationHandler.THROW_UNSUPPORTED_OPERATION; +import static org.opensearch.security.support.ConfigConstants.DEFAULT_TIMEOUT; @Deprecated public class ConfigHelper { @@ -95,6 +96,7 @@ public static void uploadFile( final IndexRequest indexRequest = new IndexRequest(index).id(configType) .opType(OpType.CREATE) .setRefreshPolicy(RefreshPolicy.IMMEDIATE) + .timeout(DEFAULT_TIMEOUT) .source(configType, readXContent(reader, XContentType.YAML)); final String res = tc.index(indexRequest).actionGet().getId();