From e0b8def8c9ebc8fe8df1401054c3d4030995ad8b Mon Sep 17 00:00:00 2001 From: thanicz Date: Fri, 29 May 2026 08:42:32 +0200 Subject: [PATCH 1/4] KNOX-3332: New gatewayconfig change listener --- .../apache/knox/gateway/GatewayServer.java | 15 +++ .../services/ldap/KnoxLDAPServerManager.java | 67 ++++++++--- .../services/ldap/KnoxLDAPService.java | 28 +---- .../gateway/services/ldap/LdapMessages.java | 8 ++ .../knox/gateway/GatewayServerTest.java | 32 +++++ .../ldap/KnoxLDAPServerManagerTest.java | 110 +++++++++++++----- .../config/GatewayConfigChangeListener.java | 22 ++++ 7 files changed, 213 insertions(+), 69 deletions(-) create mode 100644 gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfigChangeListener.java diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServer.java b/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServer.java index cf1e8f9988..d08240ae2a 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServer.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServer.java @@ -28,6 +28,7 @@ import org.apache.knox.gateway.audit.api.ResourceType; import org.apache.knox.gateway.audit.log4j.audit.AuditConstants; import org.apache.knox.gateway.config.GatewayConfig; +import org.apache.knox.gateway.config.GatewayConfigChangeListener; import org.apache.knox.gateway.config.GatewayConfigurationException; import org.apache.knox.gateway.config.impl.GatewayConfigImpl; import org.apache.knox.gateway.deploy.DeploymentException; @@ -127,6 +128,7 @@ import java.util.ServiceLoader; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; @@ -148,6 +150,8 @@ public class GatewayServer { private static GatewayServer server; private static GatewayServices services; + private static final List configChangeListeners = new CopyOnWriteArrayList<>(); + private static Properties buildProperties; private static final ScheduledExecutorService exec = Executors.newSingleThreadScheduledExecutor(); @@ -239,6 +243,14 @@ public static synchronized GatewayServices getGatewayServices() { return services; } + public static void registerConfigChangeListener(GatewayConfigChangeListener listener) { + configChangeListeners.add(listener); + } + + public static void unregisterConfigChangeListener(GatewayConfigChangeListener listener) { + configChangeListeners.remove(listener); + } + private static void setupGatewayConfigRefresh(GatewayConfigImpl config) { Path resourcePath = Paths.get(config.getGatewayConfDir(), RELOADABLE_CONFIG_FILENAME); int refreshInterval = config.getConfigRefreshInterval(); @@ -261,6 +273,9 @@ private static synchronized void refreshGatewayConfig(GatewayConfigImpl config, lastReloadTime = lastModifiedTime; config.reloadConfiguration(); log.refreshedGatewayConfig(); + for (GatewayConfigChangeListener listener : configChangeListeners) { + listener.onGatewayConfigChanged(); + } } } } catch (IOException e) { diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManager.java b/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManager.java index 300e6f936f..c31edb168f 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManager.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManager.java @@ -30,6 +30,9 @@ import org.apache.directory.server.core.partition.ldif.LdifPartition; import org.apache.directory.server.ldap.LdapServer; import org.apache.directory.server.protocol.shared.transport.TcpTransport; +import org.apache.knox.gateway.GatewayServer; +import org.apache.knox.gateway.config.GatewayConfig; +import org.apache.knox.gateway.config.GatewayConfigChangeListener; import org.apache.knox.gateway.i18n.messages.MessagesFactory; import org.apache.knox.gateway.services.ldap.backend.BackendFactory; import org.apache.knox.gateway.services.ldap.backend.LdapBackend; @@ -42,9 +45,10 @@ /** * Manages the ApacheDS LDAP server instance with pluggable backends */ -public class KnoxLDAPServerManager { +public class KnoxLDAPServerManager implements GatewayConfigChangeListener { private static final LdapMessages LOG = MessagesFactory.get(LdapMessages.class); + private GatewayConfig config; private DirectoryService directoryService; private LdapServer ldapServer; private LdapBackend backend; @@ -56,21 +60,38 @@ public class KnoxLDAPServerManager { /** * Initialize the LDAP server with the given configuration * - * @param workDir Directory for LDAP data storage - * @param port Port for LDAP server to listen on - * @param baseDn Base DN for LDAP entries in the proxy server - * @param backendType Type of backend to use - * @param backendConfig Backend-specific configuration - * @param remoteBaseDn Base DN of the remote LDAP server (for proxy backends) + * @param config Gateway configuration */ - public void initialize(File workDir, int port, String baseDn, String backendType, Map backendConfig, String remoteBaseDn) throws Exception { - this.workDir = workDir; - this.port = port; - this.baseDn = baseDn; - this.remoteBaseDn = remoteBaseDn; + public void initialize(GatewayConfig config) throws Exception { + if (this.config == null) { + GatewayServer.registerConfigChangeListener(this); + } + this.config = config; - // Initialize backend + // Prepare work directory for LDAP data + File gatewayDataDir = new File(config.getGatewayDataDir()); + this.workDir = new File(gatewayDataDir, "ldap-server"); + + // Get configuration + this.port = config.getLDAPPort(); + this.baseDn = config.getLDAPBaseDN(); + String backendType = config.getLDAPBackendType(); + + // Get backend-specific configuration using prefixed properties + Map backendConfig = config.getLDAPBackendConfig(backendType); + + // Add common configuration backendConfig.put("baseDn", baseDn); + + // Add legacy dataFile property for backwards compatibility with file backend + if ("file".equalsIgnoreCase(backendType) && !backendConfig.containsKey("dataFile")) { + backendConfig.put("dataFile", config.getLDAPBackendDataFile()); + } + + // For proxy backends, extract remoteBaseDn if present + this.remoteBaseDn = backendConfig.get("remoteBaseDn"); + + // Initialize backend backend = BackendFactory.createBackend(backendType, backendConfig); // Clean up previous run if it didn't shut down cleanly @@ -83,6 +104,26 @@ public void initialize(File workDir, int port, String baseDn, String backendType workDir.mkdirs(); } + @Override + public void onGatewayConfigChanged() { + LOG.ldapReloadingConfig(); + try { + boolean wasRunning = isRunning(); + if (wasRunning) { + stop(); + } + // Re-initialize and start if it was running or if it's now enabled + if (config.isLDAPEnabled()) { + initialize(config); + if (wasRunning) { + start(); + } + } + } catch (Exception e) { + LOG.ldapServiceReloadFailed(e); + } + } + /** * Start the LDAP server */ diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPService.java b/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPService.java index 1ec748da9d..bdf6af2779 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPService.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPService.java @@ -22,7 +22,6 @@ import org.apache.knox.gateway.services.Service; import org.apache.knox.gateway.services.ServiceLifecycleException; -import java.io.File; import java.util.Map; /** @@ -46,32 +45,7 @@ public void init(GatewayConfig config, Map options) throws Servi try { // Initialize the LDAP server manager with configuration ldapServerManager = new KnoxLDAPServerManager(); - - // Prepare work directory for LDAP data - File gatewayDataDir = new File(config.getGatewayDataDir()); - File ldapWorkDir = new File(gatewayDataDir, "ldap-server"); - - // Get configuration - int port = config.getLDAPPort(); - String baseDn = config.getLDAPBaseDN(); - String backendType = config.getLDAPBackendType(); - - // Get backend-specific configuration using prefixed properties - Map backendConfig = config.getLDAPBackendConfig(backendType); - - // Add common configuration - backendConfig.put("baseDn", baseDn); - - // Add legacy dataFile property for backwards compatibility with file backend - if ("file".equalsIgnoreCase(backendType) && !backendConfig.containsKey("dataFile")) { - backendConfig.put("dataFile", config.getLDAPBackendDataFile()); - } - - // For proxy backends, extract remoteBaseDn if present - String remoteBaseDn = backendConfig.get("remoteBaseDn"); - - // Initialize but don't start yet - ldapServerManager.initialize(ldapWorkDir, port, baseDn, backendType, backendConfig, remoteBaseDn); + ldapServerManager.initialize(config); } catch (Exception e) { throw new ServiceLifecycleException("Failed to initialize LDAP service", e); diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/LdapMessages.java b/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/LdapMessages.java index 4845cda79e..cc92a099db 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/LdapMessages.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/LdapMessages.java @@ -102,4 +102,12 @@ public interface LdapMessages { @Message(level = MessageLevel.WARN, text = "LDAP authentication failed for user: {0}") void ldapAuthFailed(String user, @StackTrace(level = MessageLevel.INFO) Throwable cause); + + @Message(level = MessageLevel.INFO, + text = "Reloading LDAP configuration") + void ldapReloadingConfig(); + + @Message(level = MessageLevel.ERROR, + text = "Failed to reload LDAP service: {0}") + void ldapServiceReloadFailed(@StackTrace(level = MessageLevel.DEBUG) Exception e); } diff --git a/gateway-server/src/test/java/org/apache/knox/gateway/GatewayServerTest.java b/gateway-server/src/test/java/org/apache/knox/gateway/GatewayServerTest.java index 33d3481904..d587ec1297 100644 --- a/gateway-server/src/test/java/org/apache/knox/gateway/GatewayServerTest.java +++ b/gateway-server/src/test/java/org/apache/knox/gateway/GatewayServerTest.java @@ -17,6 +17,7 @@ */ package org.apache.knox.gateway; +import org.apache.knox.gateway.config.GatewayConfigChangeListener; import org.apache.knox.gateway.config.impl.GatewayConfigImpl; import org.easymock.EasyMock; import org.junit.Rule; @@ -32,6 +33,7 @@ import java.nio.file.attribute.FileTime; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; public class GatewayServerTest { @@ -95,4 +97,34 @@ public void testRefreshGatewayConfig() throws Exception { refreshMethod.invoke(null, config, configFile.toPath()); EasyMock.verify(config); } + + @Test + public void testGatewayConfigChangeListener() throws Exception { + GatewayConfigImpl config = EasyMock.createNiceMock(GatewayConfigImpl.class); + File configFile = folder.newFile("gateway-reloadable-listener.xml"); + try (PrintWriter writer = new PrintWriter(configFile, StandardCharsets.UTF_8)) { + writer.println(""); + } + + final boolean[] notified = {false}; + GatewayConfigChangeListener listener = () -> notified[0] = true; + + GatewayServer.registerConfigChangeListener(listener); + + try { + Method refreshMethod = GatewayServer.class.getDeclaredMethod("refreshGatewayConfig", GatewayConfigImpl.class, Path.class); + refreshMethod.setAccessible(true); + + // Reset lastReloadTime to ensure refresh happens + Field lastReloadTimeField = GatewayServer.class.getDeclaredField("lastReloadTime"); + lastReloadTimeField.setAccessible(true); + lastReloadTimeField.set(null, null); + + refreshMethod.invoke(null, config, configFile.toPath()); + + assertTrue("Listener should have been notified", notified[0]); + } finally { + GatewayServer.unregisterConfigChangeListener(listener); + } + } } diff --git a/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManagerTest.java b/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManagerTest.java index a9450a96ff..7ae2130cd9 100644 --- a/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManagerTest.java +++ b/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManagerTest.java @@ -17,6 +17,8 @@ */ package org.apache.knox.gateway.services.ldap; +import org.apache.knox.gateway.config.GatewayConfig; +import org.easymock.EasyMock; import org.junit.Test; import org.junit.Before; import org.junit.After; @@ -25,6 +27,8 @@ import java.util.HashMap; import java.util.Map; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.replay; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertFalse; @@ -71,9 +75,16 @@ public void tearDown() throws Exception { @Test public void testInitializeWithFileBackend() throws Exception { - Map backendConfig = createFileBackendConfig(); + GatewayConfig mockConfig = EasyMock.createNiceMock(GatewayConfig.class); + expect(mockConfig.getGatewayDataDir()).andReturn(tempWorkDir.getParent()).anyTimes(); + expect(mockConfig.getLDAPPort()).andReturn(3890).anyTimes(); + expect(mockConfig.getLDAPBaseDN()).andReturn("dc=test,dc=com").anyTimes(); + expect(mockConfig.getLDAPBackendType()).andReturn("file").anyTimes(); + expect(mockConfig.getLDAPBackendDataFile()).andReturn(tempLdapFile.getAbsolutePath()).anyTimes(); + expect(mockConfig.getLDAPBackendConfig("file")).andReturn(new HashMap<>()).anyTimes(); + replay(mockConfig); - serverManager.initialize(tempWorkDir, 3890, "dc=test,dc=com", "file", backendConfig, null); + serverManager.initialize(mockConfig); assertEquals("Port should be set correctly", 3890, serverManager.getPort()); assertEquals("Base DN should be set correctly", "dc=test,dc=com", serverManager.getBaseDn()); @@ -82,9 +93,22 @@ public void testInitializeWithFileBackend() throws Exception { @Test public void testInitializeWithLdapBackend() throws Exception { - Map backendConfig = createLdapBackendConfig(); + GatewayConfig mockConfig = EasyMock.createNiceMock(GatewayConfig.class); + expect(mockConfig.getGatewayDataDir()).andReturn(tempWorkDir.getParent()).anyTimes(); + expect(mockConfig.getLDAPPort()).andReturn(3891).anyTimes(); + expect(mockConfig.getLDAPBaseDN()).andReturn("dc=proxy,dc=com").anyTimes(); + expect(mockConfig.getLDAPBackendType()).andReturn("ldap").anyTimes(); - serverManager.initialize(tempWorkDir, 3891, "dc=proxy,dc=com", "ldap", backendConfig, "dc=hadoop,dc=apache,dc=org"); + Map backendConfig = new HashMap<>(); + backendConfig.put("url", "ldap://localhost:33389"); + backendConfig.put("remoteBaseDn", "dc=hadoop,dc=apache,dc=org"); + backendConfig.put("systemUsername", "cn=admin,dc=hadoop,dc=apache,dc=org"); + backendConfig.put("systemPassword", "admin-password"); + + expect(mockConfig.getLDAPBackendConfig("ldap")).andReturn(backendConfig).anyTimes(); + replay(mockConfig); + + serverManager.initialize(mockConfig); assertEquals("Port should be set correctly", 3891, serverManager.getPort()); assertEquals("Base DN should be set correctly", "dc=proxy,dc=com", serverManager.getBaseDn()); @@ -93,23 +117,36 @@ public void testInitializeWithLdapBackend() throws Exception { @Test(expected = Exception.class) public void testInitializeWithInvalidBackendType() throws Exception { - Map backendConfig = new HashMap<>(); - backendConfig.put("baseDn", "dc=test,dc=com"); + GatewayConfig mockConfig = EasyMock.createNiceMock(GatewayConfig.class); + expect(mockConfig.getGatewayDataDir()).andReturn(tempWorkDir.getParent()).anyTimes(); + expect(mockConfig.getLDAPBackendType()).andReturn("invalid").anyTimes(); + expect(mockConfig.getLDAPBackendConfig("invalid")).andReturn(new HashMap<>()).anyTimes(); + replay(mockConfig); - serverManager.initialize(tempWorkDir, 3890, "dc=test,dc=com", "invalid", backendConfig, null); + serverManager.initialize(mockConfig); } @Test public void testLockFileCleanup() throws Exception { + GatewayConfig mockConfig = EasyMock.createNiceMock(GatewayConfig.class); + expect(mockConfig.getGatewayDataDir()).andReturn(tempWorkDir.getParent()).anyTimes(); + expect(mockConfig.getLDAPBackendType()).andReturn("file").anyTimes(); + expect(mockConfig.getLDAPBackendDataFile()).andReturn(tempLdapFile.getAbsolutePath()).anyTimes(); + expect(mockConfig.getLDAPBackendConfig("file")).andReturn(new HashMap<>()).anyTimes(); + replay(mockConfig); + + // The manager will use tempWorkDir.getParent()/ldap-server as workDir + File actualWorkDir = new File(tempWorkDir.getParent(), "ldap-server"); + actualWorkDir.mkdirs(); + // Create a lock file to simulate previous unclean shutdown - File runDir = new File(tempWorkDir, "run"); + File runDir = new File(actualWorkDir, "run"); runDir.mkdirs(); File lockFile = new File(runDir, "instance.lock"); lockFile.createNewFile(); assertTrue("Lock file should exist before initialization", lockFile.exists()); - Map backendConfig = createFileBackendConfig(); - serverManager.initialize(tempWorkDir, 3890, "dc=test,dc=com", "file", backendConfig, null); + serverManager.initialize(mockConfig); assertFalse("Lock file should be cleaned up during initialization", lockFile.exists()); } @@ -123,8 +160,14 @@ public void testGettersBeforeInitialization() { @Test public void testStopBeforeStart() throws Exception { - Map backendConfig = createFileBackendConfig(); - serverManager.initialize(tempWorkDir, 3890, "dc=test,dc=com", "file", backendConfig, null); + GatewayConfig mockConfig = EasyMock.createNiceMock(GatewayConfig.class); + expect(mockConfig.getGatewayDataDir()).andReturn(tempWorkDir.getParent()).anyTimes(); + expect(mockConfig.getLDAPBackendType()).andReturn("file").anyTimes(); + expect(mockConfig.getLDAPBackendDataFile()).andReturn(tempLdapFile.getAbsolutePath()).anyTimes(); + expect(mockConfig.getLDAPBackendConfig("file")).andReturn(new HashMap<>()).anyTimes(); + replay(mockConfig); + + serverManager.initialize(mockConfig); // Should not throw exception when stopping before starting serverManager.stop(); @@ -132,8 +175,14 @@ public void testStopBeforeStart() throws Exception { @Test public void testMultipleStopCalls() throws Exception { - Map backendConfig = createFileBackendConfig(); - serverManager.initialize(tempWorkDir, 3890, "dc=test,dc=com", "file", backendConfig, null); + GatewayConfig mockConfig = EasyMock.createNiceMock(GatewayConfig.class); + expect(mockConfig.getGatewayDataDir()).andReturn(tempWorkDir.getParent()).anyTimes(); + expect(mockConfig.getLDAPBackendType()).andReturn("file").anyTimes(); + expect(mockConfig.getLDAPBackendDataFile()).andReturn(tempLdapFile.getAbsolutePath()).anyTimes(); + expect(mockConfig.getLDAPBackendConfig("file")).andReturn(new HashMap<>()).anyTimes(); + replay(mockConfig); + + serverManager.initialize(mockConfig); // Multiple stop calls should not throw exceptions serverManager.stop(); @@ -147,21 +196,24 @@ public void testStartWithoutInitialize() throws Exception { serverManager.start(); } - private Map createFileBackendConfig() { - Map config = new HashMap<>(); - config.put("baseDn", "dc=test,dc=com"); - config.put("dataFile", tempLdapFile.getAbsolutePath()); - return config; - } - - private Map createLdapBackendConfig() { - Map config = new HashMap<>(); - config.put("baseDn", "dc=proxy,dc=com"); - config.put("url", "ldap://localhost:33389"); - config.put("remoteBaseDn", "dc=hadoop,dc=apache,dc=org"); - config.put("systemUsername", "cn=admin,dc=hadoop,dc=apache,dc=org"); - config.put("systemPassword", "admin-password"); - return config; + @Test + public void testOnGatewayConfigChanged() throws Exception { + GatewayConfig mockConfig = EasyMock.createNiceMock(GatewayConfig.class); + expect(mockConfig.getGatewayDataDir()).andReturn(tempWorkDir.getParent()).anyTimes(); + expect(mockConfig.getLDAPPort()).andReturn(3890).anyTimes(); + expect(mockConfig.getLDAPBaseDN()).andReturn("dc=test,dc=com").anyTimes(); + expect(mockConfig.getLDAPBackendType()).andReturn("file").anyTimes(); + expect(mockConfig.getLDAPBackendDataFile()).andReturn(tempLdapFile.getAbsolutePath()).anyTimes(); + expect(mockConfig.getLDAPBackendConfig("file")).andReturn(new HashMap<>()).anyTimes(); + expect(mockConfig.isLDAPEnabled()).andReturn(true).anyTimes(); + replay(mockConfig); + + serverManager.initialize(mockConfig); + + // Test reload + serverManager.onGatewayConfigChanged(); + + assertEquals("Port should be maintained after reload", 3890, serverManager.getPort()); } private void cleanupTempFiles() { diff --git a/gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfigChangeListener.java b/gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfigChangeListener.java new file mode 100644 index 0000000000..bbbde512f6 --- /dev/null +++ b/gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfigChangeListener.java @@ -0,0 +1,22 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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. + */ +package org.apache.knox.gateway.config; + +public interface GatewayConfigChangeListener { + void onGatewayConfigChanged(); +} From e507d5c926721b24c4e01cae61fbf2c06898641f Mon Sep 17 00:00:00 2001 From: thanicz Date: Fri, 29 May 2026 12:06:33 +0200 Subject: [PATCH 2/4] KNOX-3332: Improve testOnGatewayConfigChanged test --- .../gateway/services/ldap/KnoxLDAPServerManagerTest.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManagerTest.java b/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManagerTest.java index 7ae2130cd9..70fbc45922 100644 --- a/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManagerTest.java +++ b/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManagerTest.java @@ -200,7 +200,7 @@ public void testStartWithoutInitialize() throws Exception { public void testOnGatewayConfigChanged() throws Exception { GatewayConfig mockConfig = EasyMock.createNiceMock(GatewayConfig.class); expect(mockConfig.getGatewayDataDir()).andReturn(tempWorkDir.getParent()).anyTimes(); - expect(mockConfig.getLDAPPort()).andReturn(3890).anyTimes(); + expect(mockConfig.getLDAPPort()).andReturn(3890).times(1).andReturn(3891).anyTimes(); expect(mockConfig.getLDAPBaseDN()).andReturn("dc=test,dc=com").anyTimes(); expect(mockConfig.getLDAPBackendType()).andReturn("file").anyTimes(); expect(mockConfig.getLDAPBackendDataFile()).andReturn(tempLdapFile.getAbsolutePath()).anyTimes(); @@ -209,11 +209,12 @@ public void testOnGatewayConfigChanged() throws Exception { replay(mockConfig); serverManager.initialize(mockConfig); + assertEquals("Initial port should be 3890", 3890, serverManager.getPort()); - // Test reload + // Test reload with new port serverManager.onGatewayConfigChanged(); - assertEquals("Port should be maintained after reload", 3890, serverManager.getPort()); + assertEquals("Port should be updated to 3891 after reload", 3891, serverManager.getPort()); } private void cleanupTempFiles() { From be2a6d1b770364b4fdaff5fc84a4fd6cd5deac30 Mon Sep 17 00:00:00 2001 From: thanicz Date: Mon, 1 Jun 2026 09:20:09 +0200 Subject: [PATCH 3/4] KNOX-3332: Moved the listener into KnoxLDAPService --- .../apache/knox/gateway/GatewayServer.java | 2 +- .../services/DefaultGatewayServices.java | 2 ++ .../services/ldap/KnoxLDAPServerManager.java | 30 +----------------- .../services/ldap/KnoxLDAPService.java | 31 ++++++++++++++++++- .../knox/gateway/GatewayServerTest.java | 2 +- .../ldap/KnoxLDAPServerManagerTest.java | 21 ------------- .../services/ldap/KnoxLDAPServiceTest.java | 27 +++++++++++++++- .../config/GatewayConfigChangeListener.java | 2 +- 8 files changed, 62 insertions(+), 55 deletions(-) diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServer.java b/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServer.java index d08240ae2a..a39e5e6c16 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServer.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServer.java @@ -274,7 +274,7 @@ private static synchronized void refreshGatewayConfig(GatewayConfigImpl config, config.reloadConfiguration(); log.refreshedGatewayConfig(); for (GatewayConfigChangeListener listener : configChangeListeners) { - listener.onGatewayConfigChanged(); + listener.onGatewayConfigChanged(config); } } } diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/services/DefaultGatewayServices.java b/gateway-server/src/main/java/org/apache/knox/gateway/services/DefaultGatewayServices.java index 06ce95d93a..3f03599765 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/services/DefaultGatewayServices.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/services/DefaultGatewayServices.java @@ -21,6 +21,7 @@ import java.util.Map; import org.apache.knox.gateway.GatewayMessages; +import org.apache.knox.gateway.GatewayServer; import org.apache.knox.gateway.config.GatewayConfig; import org.apache.knox.gateway.deploy.DeploymentContext; import org.apache.knox.gateway.descriptor.FilterParamDescriptor; @@ -88,6 +89,7 @@ public void init(GatewayConfig config, Map options) throws Servic if (config.isLDAPEnabled()) { KnoxLDAPService ldapService = new KnoxLDAPService(); ldapService.init(config, options); + GatewayServer.registerConfigChangeListener(ldapService); addService(ServiceType.LDAP_SERVICE, ldapService); } } diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManager.java b/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManager.java index c31edb168f..cd7bb1a803 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManager.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManager.java @@ -30,9 +30,7 @@ import org.apache.directory.server.core.partition.ldif.LdifPartition; import org.apache.directory.server.ldap.LdapServer; import org.apache.directory.server.protocol.shared.transport.TcpTransport; -import org.apache.knox.gateway.GatewayServer; import org.apache.knox.gateway.config.GatewayConfig; -import org.apache.knox.gateway.config.GatewayConfigChangeListener; import org.apache.knox.gateway.i18n.messages.MessagesFactory; import org.apache.knox.gateway.services.ldap.backend.BackendFactory; import org.apache.knox.gateway.services.ldap.backend.LdapBackend; @@ -45,10 +43,9 @@ /** * Manages the ApacheDS LDAP server instance with pluggable backends */ -public class KnoxLDAPServerManager implements GatewayConfigChangeListener { +public class KnoxLDAPServerManager { private static final LdapMessages LOG = MessagesFactory.get(LdapMessages.class); - private GatewayConfig config; private DirectoryService directoryService; private LdapServer ldapServer; private LdapBackend backend; @@ -63,11 +60,6 @@ public class KnoxLDAPServerManager implements GatewayConfigChangeListener { * @param config Gateway configuration */ public void initialize(GatewayConfig config) throws Exception { - if (this.config == null) { - GatewayServer.registerConfigChangeListener(this); - } - this.config = config; - // Prepare work directory for LDAP data File gatewayDataDir = new File(config.getGatewayDataDir()); this.workDir = new File(gatewayDataDir, "ldap-server"); @@ -104,26 +96,6 @@ public void initialize(GatewayConfig config) throws Exception { workDir.mkdirs(); } - @Override - public void onGatewayConfigChanged() { - LOG.ldapReloadingConfig(); - try { - boolean wasRunning = isRunning(); - if (wasRunning) { - stop(); - } - // Re-initialize and start if it was running or if it's now enabled - if (config.isLDAPEnabled()) { - initialize(config); - if (wasRunning) { - start(); - } - } - } catch (Exception e) { - LOG.ldapServiceReloadFailed(e); - } - } - /** * Start the LDAP server */ diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPService.java b/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPService.java index bdf6af2779..478a41b43a 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPService.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPService.java @@ -18,6 +18,7 @@ package org.apache.knox.gateway.services.ldap; import org.apache.knox.gateway.config.GatewayConfig; +import org.apache.knox.gateway.config.GatewayConfigChangeListener; import org.apache.knox.gateway.i18n.messages.MessagesFactory; import org.apache.knox.gateway.services.Service; import org.apache.knox.gateway.services.ServiceLifecycleException; @@ -28,7 +29,7 @@ * Knox LDAP Service - provides an embedded LDAP server with pluggable backends * for user and group lookups. */ -public class KnoxLDAPService implements Service { +public class KnoxLDAPService implements Service, GatewayConfigChangeListener { private static final LdapMessages LOG = MessagesFactory.get(LdapMessages.class); private KnoxLDAPServerManager ldapServerManager; @@ -81,6 +82,34 @@ public void stop() throws ServiceLifecycleException { } } + @Override + public void onGatewayConfigChanged(GatewayConfig config) { + LOG.ldapReloadingConfig(); + try { + this.enabled = config.isLDAPEnabled(); + + if (ldapServerManager != null) { + boolean wasRunning = ldapServerManager.isRunning(); + if (wasRunning) { + ldapServerManager.stop(); + } + // Re-initialize and start if it's now enabled + if (this.enabled) { + ldapServerManager.initialize(config); + if (wasRunning) { + ldapServerManager.start(); + } + } + } else if (this.enabled) { + // If it wasn't there, but now it's enabled, we need to create and initialize it. + ldapServerManager = new KnoxLDAPServerManager(); + ldapServerManager.initialize(config); + ldapServerManager.start(); + } + } catch (Exception e) { + LOG.ldapServiceReloadFailed(e); + } + } /** * Get the port the LDAP server is listening on */ diff --git a/gateway-server/src/test/java/org/apache/knox/gateway/GatewayServerTest.java b/gateway-server/src/test/java/org/apache/knox/gateway/GatewayServerTest.java index d587ec1297..a5938d7cd9 100644 --- a/gateway-server/src/test/java/org/apache/knox/gateway/GatewayServerTest.java +++ b/gateway-server/src/test/java/org/apache/knox/gateway/GatewayServerTest.java @@ -107,7 +107,7 @@ public void testGatewayConfigChangeListener() throws Exception { } final boolean[] notified = {false}; - GatewayConfigChangeListener listener = () -> notified[0] = true; + GatewayConfigChangeListener listener = (c) -> notified[0] = true; GatewayServer.registerConfigChangeListener(listener); diff --git a/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManagerTest.java b/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManagerTest.java index 70fbc45922..4528c8b4a8 100644 --- a/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManagerTest.java +++ b/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManagerTest.java @@ -196,27 +196,6 @@ public void testStartWithoutInitialize() throws Exception { serverManager.start(); } - @Test - public void testOnGatewayConfigChanged() throws Exception { - GatewayConfig mockConfig = EasyMock.createNiceMock(GatewayConfig.class); - expect(mockConfig.getGatewayDataDir()).andReturn(tempWorkDir.getParent()).anyTimes(); - expect(mockConfig.getLDAPPort()).andReturn(3890).times(1).andReturn(3891).anyTimes(); - expect(mockConfig.getLDAPBaseDN()).andReturn("dc=test,dc=com").anyTimes(); - expect(mockConfig.getLDAPBackendType()).andReturn("file").anyTimes(); - expect(mockConfig.getLDAPBackendDataFile()).andReturn(tempLdapFile.getAbsolutePath()).anyTimes(); - expect(mockConfig.getLDAPBackendConfig("file")).andReturn(new HashMap<>()).anyTimes(); - expect(mockConfig.isLDAPEnabled()).andReturn(true).anyTimes(); - replay(mockConfig); - - serverManager.initialize(mockConfig); - assertEquals("Initial port should be 3890", 3890, serverManager.getPort()); - - // Test reload with new port - serverManager.onGatewayConfigChanged(); - - assertEquals("Port should be updated to 3891 after reload", 3891, serverManager.getPort()); - } - private void cleanupTempFiles() { if (tempLdapFile != null && tempLdapFile.exists()) { tempLdapFile.delete(); diff --git a/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServiceTest.java b/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServiceTest.java index dbdb081dac..fe045194bb 100644 --- a/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServiceTest.java +++ b/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServiceTest.java @@ -165,6 +165,31 @@ public void testGettersWhenNotInitialized() { assertFalse("Should not be enabled when not initialized", ldapService.isEnabled()); } + @Test + public void testOnGatewayConfigChanged() throws Exception { + expect(mockConfig.isLDAPEnabled()).andReturn(true).anyTimes(); + expect(mockConfig.getGatewayDataDir()).andReturn(tempDataDir.getAbsolutePath()).anyTimes(); + expect(mockConfig.getLDAPPort()).andReturn(3890).times(1).andReturn(3891).anyTimes(); + expect(mockConfig.getLDAPBaseDN()).andReturn("dc=test,dc=com").anyTimes(); + expect(mockConfig.getLDAPBackendType()).andReturn("file").anyTimes(); + + Map fileBackendConfig = new HashMap<>(); + fileBackendConfig.put("dataFile", tempLdapFile.getAbsolutePath()); + expect(mockConfig.getLDAPBackendConfig("file")).andReturn(fileBackendConfig).anyTimes(); + + replay(mockConfig); + + ldapService.init(mockConfig, new HashMap<>()); + assertEquals("Initial port should be 3890", 3890, ldapService.getLdapPort()); + + // Test reload with new port + ldapService.onGatewayConfigChanged(mockConfig); + + assertEquals("Port should be updated to 3891 after reload", 3891, ldapService.getLdapPort()); + + verify(mockConfig); + } + private void setupMockConfigForFileBackend() { expect(mockConfig.isLDAPEnabled()).andReturn(true); expect(mockConfig.getGatewayDataDir()).andReturn(tempDataDir.getAbsolutePath()); @@ -191,4 +216,4 @@ private void setupMockConfigForLdapBackend() { ldapBackendConfig.put("systemPassword", "admin-password"); expect(mockConfig.getLDAPBackendConfig("ldap")).andReturn(ldapBackendConfig); } -} \ No newline at end of file +} diff --git a/gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfigChangeListener.java b/gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfigChangeListener.java index bbbde512f6..5dfd0fbf23 100644 --- a/gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfigChangeListener.java +++ b/gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfigChangeListener.java @@ -18,5 +18,5 @@ package org.apache.knox.gateway.config; public interface GatewayConfigChangeListener { - void onGatewayConfigChanged(); + void onGatewayConfigChanged(GatewayConfig config); } From 0565fbc8d4e4871efb15090bfb39a9d895ec3bad Mon Sep 17 00:00:00 2001 From: thanicz Date: Mon, 1 Jun 2026 11:44:08 +0200 Subject: [PATCH 4/4] KNOX-3332: Simplify restart logic --- .../services/ldap/KnoxLDAPService.java | 22 ++++++------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPService.java b/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPService.java index 478a41b43a..a6f66a67e8 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPService.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPService.java @@ -88,28 +88,20 @@ public void onGatewayConfigChanged(GatewayConfig config) { try { this.enabled = config.isLDAPEnabled(); - if (ldapServerManager != null) { - boolean wasRunning = ldapServerManager.isRunning(); - if (wasRunning) { - ldapServerManager.stop(); - } - // Re-initialize and start if it's now enabled - if (this.enabled) { - ldapServerManager.initialize(config); - if (wasRunning) { - ldapServerManager.start(); - } - } - } else if (this.enabled) { - // If it wasn't there, but now it's enabled, we need to create and initialize it. - ldapServerManager = new KnoxLDAPServerManager(); + if (this.enabled) { + this.ldapServerManager = this.ldapServerManager == null ? new KnoxLDAPServerManager() : this.ldapServerManager; + ldapServerManager.stop(); ldapServerManager.initialize(config); ldapServerManager.start(); + } else if (ldapServerManager != null) { + ldapServerManager.stop(); + ldapServerManager = null; } } catch (Exception e) { LOG.ldapServiceReloadFailed(e); } } + /** * Get the port the LDAP server is listening on */