From 92b855d2e179cd622a68b2ad9198e45d647a2cfe Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Wed, 13 Sep 2023 10:32:37 -0400 Subject: [PATCH] Replaces DCF isInitialized with a countdown latch and waitForInit Signed-off-by: Darshit Chanpura --- .../TransportConfigUpdateAction.java | 9 +++- .../ConfigurationRepository.java | 44 +++++++++++++------ .../securityconf/DynamicConfigFactory.java | 13 +++--- .../security/securityconf/Initializable.java | 34 -------------- 4 files changed, 46 insertions(+), 54 deletions(-) delete mode 100644 src/main/java/org/opensearch/security/securityconf/Initializable.java diff --git a/src/main/java/org/opensearch/security/action/configupdate/TransportConfigUpdateAction.java b/src/main/java/org/opensearch/security/action/configupdate/TransportConfigUpdateAction.java index 1e5b5e4056..8cf75420cf 100644 --- a/src/main/java/org/opensearch/security/action/configupdate/TransportConfigUpdateAction.java +++ b/src/main/java/org/opensearch/security/action/configupdate/TransportConfigUpdateAction.java @@ -32,6 +32,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.opensearch.OpenSearchException; import org.opensearch.action.FailedNodeException; import org.opensearch.action.support.ActionFilters; import org.opensearch.action.support.nodes.TransportNodesAction; @@ -125,7 +126,13 @@ protected ConfigUpdateResponse newResponse( @Override protected ConfigUpdateNodeResponse nodeOperation(final NodeConfigUpdateRequest request) { - configurationRepository.reloadConfiguration(CType.fromStringValues((request.request.getConfigTypes()))); + try { + dynamicConfigFactory.waitForInit(); + } catch (InterruptedException e) { + // thread interrupted while waiting for DCF to be initialized + throw new OpenSearchException("Error while waiting for DynamicConfigFactory to be initialized: ", e); + } + configurationRepository.reloadConfiguration(CType.fromStringValues(request.request.getConfigTypes())); backendRegistry.get().invalidateCache(); return new ConfigUpdateNodeResponse(clusterService.localNode(), request.request.getConfigTypes(), null); } diff --git a/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java b/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java index 17ea48f46c..b441d8ab1c 100644 --- a/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java +++ b/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java @@ -215,20 +215,36 @@ private ConfigurationRepository( } } - while (!dynamicConfigFactory.isInitialized()) { + // while (!dynamicConfigFactory.isInitialized()) { + // try { + // LOGGER.debug("Try to load config ..."); + // reloadConfiguration(Arrays.asList(CType.values())); + // break; + // } catch (Exception e) { + // LOGGER.debug("Unable to load configuration due to {}", String.valueOf(ExceptionUtils.getRootCause(e))); + // try { + // Thread.sleep(3000); + // } catch (InterruptedException e1) { + // Thread.currentThread().interrupt(); + // LOGGER.debug("Thread was interrupted so we cancel initialization"); + // break; + // } + // } + // } + + // wait for DCF to be initialized + dynamicConfigFactory.waitForInit(); + + try { + LOGGER.debug("Try to load config ..."); + reloadConfiguration(Arrays.asList(CType.values())); + } catch (Exception e) { + LOGGER.debug("Unable to load configuration due to {}", String.valueOf(ExceptionUtils.getRootCause(e))); try { - LOGGER.debug("Try to load config ..."); - reloadConfiguration(Arrays.asList(CType.values())); - break; - } catch (Exception e) { - LOGGER.debug("Unable to load configuration due to {}", String.valueOf(ExceptionUtils.getRootCause(e))); - try { - Thread.sleep(3000); - } catch (InterruptedException e1) { - Thread.currentThread().interrupt(); - LOGGER.debug("Thread was interrupted so we cancel initialization"); - break; - } + Thread.sleep(3000); + } catch (InterruptedException e1) { + Thread.currentThread().interrupt(); + LOGGER.debug("Thread was interrupted so we cancel initialization"); } } @@ -381,7 +397,7 @@ public void reloadConfiguration(Collection configTypes) throws ConfigUpda LOCK.unlock(); } } else { - throw new ConfigUpdateAlreadyInProgressException("A config update is already imn progress"); + throw new ConfigUpdateAlreadyInProgressException("A config update is already in progress"); } } catch (InterruptedException e) { Thread.currentThread().interrupt(); diff --git a/src/main/java/org/opensearch/security/securityconf/DynamicConfigFactory.java b/src/main/java/org/opensearch/security/securityconf/DynamicConfigFactory.java index bcbe3aef57..53c553451b 100644 --- a/src/main/java/org/opensearch/security/securityconf/DynamicConfigFactory.java +++ b/src/main/java/org/opensearch/security/securityconf/DynamicConfigFactory.java @@ -33,6 +33,7 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicBoolean; import com.fasterxml.jackson.databind.JsonNode; @@ -73,8 +74,9 @@ import org.opensearch.security.support.WildcardMatcher; import org.opensearch.threadpool.ThreadPool; -public class DynamicConfigFactory implements Initializable, ConfigurationChangeListener { +public class DynamicConfigFactory implements ConfigurationChangeListener { + private final CountDownLatch initializationLatch = new CountDownLatch(1); public static final EventBusBuilder EVENT_BUS_BUILDER = EventBus.builder(); private static SecurityDynamicConfiguration staticRoles = SecurityDynamicConfiguration.empty(); private static SecurityDynamicConfiguration staticActionGroups = SecurityDynamicConfiguration.empty(); @@ -313,7 +315,7 @@ public void onChange(Map> typeToConfig) { } initialized.set(true); - + initializationLatch.countDown(); } private static ConfigV6 getConfigV6(SecurityDynamicConfiguration sdc) { @@ -328,9 +330,10 @@ private static ConfigV7 getConfigV7(SecurityDynamicConfiguration sdc) { return c.getCEntry("config"); } - @Override - public final boolean isInitialized() { - return initialized.get(); + public void waitForInit() throws InterruptedException { + // wait for DCF to completely initialized + initializationLatch.await(); + } public void registerDCFListener(Object listener) { diff --git a/src/main/java/org/opensearch/security/securityconf/Initializable.java b/src/main/java/org/opensearch/security/securityconf/Initializable.java deleted file mode 100644 index ab1a1ebd4a..0000000000 --- a/src/main/java/org/opensearch/security/securityconf/Initializable.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright 2015-2017 floragunn GmbH - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ -/* - * 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. - * - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.security.securityconf; - -public interface Initializable { - - boolean isInitialized(); - -}