From 63d6f623955a8ed5589738ef128fa89536ede881 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Wed, 30 Sep 2026 13:01:34 +0300 Subject: [PATCH 1/3] [#1120] Apply a change to plugin-type on an enabled plugin without disabling and enabling it again When plugin-type changes on an enabled plugin, the plugin config manager now initializes a new instance of the plugin for the new plugin types and then replaces the registered instance with it. If the plugin refuses the new types in initializePlugin(), the change fails and the running instance stays registered for its old types. plugin-type no longer declares component-restart, and the plugin developer guide describes how a running plugin is re-created. --- .../chap-writing-plugins.adoc | 2 + .../server/config/PluginConfiguration.xml | 6 +- .../server/core/PluginConfigManager.java | 52 ++++++++ .../core/PluginConfigManagerTestCase.java | 120 ++++++++++++++++++ .../plugins/PluginTypeTrackingPlugin.java | 99 +++++++++++++++ .../ReferentialIntegrityPluginTestCase.java | 48 +++++++ 6 files changed, 323 insertions(+), 4 deletions(-) create mode 100644 opendj-server-legacy/src/test/java/org/opends/server/plugins/PluginTypeTrackingPlugin.java diff --git a/opendj-doc-generated-ref/src/main/asciidoc/server-dev-guide/chap-writing-plugins.adoc b/opendj-doc-generated-ref/src/main/asciidoc/server-dev-guide/chap-writing-plugins.adoc index 77b43c7236..daeeadcb00 100644 --- a/opendj-doc-generated-ref/src/main/asciidoc/server-dev-guide/chap-writing-plugins.adoc +++ b/opendj-doc-generated-ref/src/main/asciidoc/server-dev-guide/chap-writing-plugins.adoc @@ -373,6 +373,8 @@ Although the example plugin's `isConfigurationChangeAcceptable()` method always In the `applyConfigurationChange()` method the plugin must modify its configuration as necessary. The example plugin can handle configuration changes without further intervention by the administrator. Other plugins might require administrative intervention because changes can be made that can only be taken into account at plugin initialization. +The plugin types are the exception: a plugin receives them only when it is initialized, so the server does not pass a change to `plugin-type` to the running plugin. When `plugin-type` changes on an enabled plugin, the server creates a new instance of the plugin, initializes it for the new plugin types, and then deregisters the running instance and calls its `finalizePlugin()` method. If `initializePlugin()` refuses the new plugin types, the change fails and the running instance stays registered for the plugin types it had. State that the plugin keeps in memory does not carry over to the new instance. + In the example plugin, the method that extends the server's behavior is the `doStartup()` method. Which method is implemented depends on what class the plugin extends. For example, a password validator extending link:../javadoc/index.html?org/opends/server/api/PasswordValidator.html[PasswordValidator, window=\_blank] would implement a `passwordIsAcceptable()` method. diff --git a/opendj-maven-plugin/src/main/resources/config/xml/org/forgerock/opendj/server/config/PluginConfiguration.xml b/opendj-maven-plugin/src/main/resources/config/xml/org/forgerock/opendj/server/config/PluginConfiguration.xml index 6ad393f6ce..1c90647da4 100644 --- a/opendj-maven-plugin/src/main/resources/config/xml/org/forgerock/opendj/server/config/PluginConfiguration.xml +++ b/opendj-maven-plugin/src/main/resources/config/xml/org/forgerock/opendj/server/config/PluginConfiguration.xml @@ -14,6 +14,7 @@ Copyright 2007-2010 Sun Microsystems, Inc. Portions Copyright 2011 ForgeRock AS. + Portions Copyright 2026 3A Systems, LLC. ! --> - Specifies the set of plug-in types for the plug-in, which specifies the times at which the plug-in is invoked. + Specifies the set of plug-in types for the plug-in, which specifies the times at which the plug-in is invoked. - - - diff --git a/opendj-server-legacy/src/main/java/org/opends/server/core/PluginConfigManager.java b/opendj-server-legacy/src/main/java/org/opends/server/core/PluginConfigManager.java index 73382497f6..5cacd02f1c 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/core/PluginConfigManager.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/core/PluginConfigManager.java @@ -4485,6 +4485,8 @@ public ConfigChangeResult applyConfigurationChange( // required. If the mapper is disabled, then instantiate the class and // initialize and register it as an identity mapper. Also, update the // plugin to indicate whether it should be invoked for internal operations. + // If only the plugin types have changed, then replace the plugin with one + // initialized for the new types. String className = configuration.getJavaClass(); if (existingPlugin != null) { @@ -4492,6 +4494,15 @@ public ConfigChangeResult applyConfigurationChange( { ccr.setAdminActionRequired(true); } + else + { + HashSet pluginTypes = getPluginTypes(configuration); + if (!pluginTypes.equals(existingPlugin.getPluginTypes())) + { + replacePlugin(configuration, pluginTypes, ccr); + return ccr; + } + } existingPlugin.setInvokeForInternalOperations( configuration.isInvokeForInternalOperations()); @@ -4521,6 +4532,47 @@ public ConfigChangeResult applyConfigurationChange( return ccr; } + /** + * Replaces the registered instance of an enabled plugin with a new one initialized for the + * plugin types of its new configuration. A plugin receives its plugin types, and may refuse + * them, only when it is initialized, so the registered instance cannot be moved to other types + * in place. The new instance is initialized first: if that fails, the registered one stays in + * use for the plugin types it was initialized for. + * + * @param configuration + * The new configuration of the plugin. + * @param pluginTypes + * The plugin types of the new configuration. + * @param ccr + * The result of the configuration change, which receives the failure, if any. + */ + private void replacePlugin(PluginCfg configuration, Set pluginTypes, + ConfigChangeResult ccr) + { + DirectoryServerPlugin plugin; + try + { + plugin = loadPlugin(configuration.getJavaClass(), pluginTypes, configuration, true); + } + catch (InitializationException ie) + { + ccr.setResultCodeIfSuccess(serverContext.getCoreConfigManager().getServerErrorResultCode()); + ccr.addMessage(ie.getMessageObject()); + return; + } + + pluginLock.lock(); + try + { + deregisterPlugin(configuration.dn()); + registerPlugin(plugin, configuration.dn(), pluginTypes); + } + finally + { + pluginLock.unlock(); + } + } + private HashSet getPluginTypes(PluginCfg configuration) { HashSet pluginTypes = new HashSet<>(); diff --git a/opendj-server-legacy/src/test/java/org/opends/server/core/PluginConfigManagerTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/core/PluginConfigManagerTestCase.java index 7e6ddf7174..9ed91d4fec 100644 --- a/opendj-server-legacy/src/test/java/org/opends/server/core/PluginConfigManagerTestCase.java +++ b/opendj-server-legacy/src/test/java/org/opends/server/core/PluginConfigManagerTestCase.java @@ -13,20 +13,29 @@ * * Copyright 2006-2008 Sun Microsystems, Inc. * Portions Copyright 2014-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.opends.server.core; import java.util.ArrayList; +import java.util.EnumSet; import org.opends.server.TestCaseUtils; import org.opends.server.api.plugin.DirectoryServerPlugin; import org.opends.server.api.plugin.PluginType; +import org.opends.server.plugins.PluginTypeTrackingPlugin; import org.forgerock.opendj.ldap.DN; +import org.forgerock.opendj.ldap.ResultCode; +import org.forgerock.opendj.ldap.requests.ModifyRequest; import org.testng.annotations.BeforeClass; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; +import static org.forgerock.opendj.ldap.ModificationType.*; +import static org.forgerock.opendj.ldap.requests.Requests.*; +import static org.opends.server.api.plugin.PluginType.*; +import static org.opends.server.protocols.internal.InternalClientConnection.*; import static org.opends.server.util.ServerConstants.*; import static org.testng.Assert.*; @@ -636,5 +645,116 @@ public void testPluginOrder(String pluginOrderString, EOL + "Expected order: " + expectedOrder + EOL + "Actual order: " + actualOrder); } + + + + /** + * A change to the plugin types of an enabled plugin takes effect at once: the plugin is + * re-created for the new types, and the instance registered for the old ones is finalized. + */ + @Test + public void testPluginTypeChangeAppliesToEnabledPlugin() throws Exception + { + TestCaseUtils.initializeTestBackend(true); + DN pluginDN = addTrackingPlugin(); + try + { + PluginTypeTrackingPlugin original = getTrackingPlugin(pluginDN); + int changeListeners = countChangeListeners(pluginDN); + assertEquals(modifyTestEntryAndCountPreOperation(), 0); + + assertEquals(setPluginTypes(pluginDN, "postOperationModify", "preOperationModify"), ResultCode.SUCCESS); + + PluginTypeTrackingPlugin replacement = getTrackingPlugin(pluginDN); + assertNotSame(replacement, original); + assertTrue(original.isFinalized(), "The instance registered for the old plugin types is not finalized"); + assertFalse(replacement.isFinalized()); + assertEquals(replacement.getPluginTypes(), EnumSet.of(POST_OPERATION_MODIFY, PRE_OPERATION_MODIFY)); + assertEquals(countChangeListeners(pluginDN), changeListeners); + assertEquals(modifyTestEntryAndCountPreOperation(), 1); + + assertEquals(setPluginTypes(pluginDN, "postOperationModify"), ResultCode.SUCCESS); + + assertEquals(getTrackingPlugin(pluginDN).getPluginTypes(), EnumSet.of(POST_OPERATION_MODIFY)); + assertTrue(replacement.isFinalized()); + assertEquals(countChangeListeners(pluginDN), changeListeners); + assertEquals(modifyTestEntryAndCountPreOperation(), 0); + } + finally + { + TestCaseUtils.deleteEntry(pluginDN); + } + } + + /** + * When the plugin cannot be initialized for the new plugin types, the change fails and the + * instance registered for the old plugin types stays registered and in use. + */ + @Test + public void testPluginTypeChangeRefusedByPluginKeepsRunningPlugin() throws Exception + { + TestCaseUtils.initializeTestBackend(true); + DN pluginDN = addTrackingPlugin(); + try + { + assertEquals(setPluginTypes(pluginDN, "postOperationModify", "preOperationModify"), ResultCode.SUCCESS); + PluginTypeTrackingPlugin running = getTrackingPlugin(pluginDN); + + ResultCode resultCode = setPluginTypes(pluginDN, "postOperationModify", "preOperationDelete"); + + assertNotEquals(resultCode, ResultCode.SUCCESS); + assertSame(getTrackingPlugin(pluginDN), running); + assertFalse(running.isFinalized()); + assertEquals(running.getPluginTypes(), EnumSet.of(POST_OPERATION_MODIFY, PRE_OPERATION_MODIFY)); + assertEquals(modifyTestEntryAndCountPreOperation(), 1); + } + finally + { + TestCaseUtils.deleteEntry(pluginDN); + } + } + + private static DN addTrackingPlugin() throws Exception + { + DN pluginDN = DN.valueOf("cn=Plugin Type Tracking Plugin,cn=Plugins,cn=config"); + TestCaseUtils.addEntry( + "dn: " + pluginDN, + "objectClass: top", + "objectClass: ds-cfg-plugin", + "cn: Plugin Type Tracking Plugin", + "ds-cfg-java-class: " + PluginTypeTrackingPlugin.class.getName(), + "ds-cfg-enabled: true", + "ds-cfg-plugin-type: postOperationModify", + "ds-cfg-invoke-for-internal-operations: true"); + return pluginDN; + } + + private static PluginTypeTrackingPlugin getTrackingPlugin(DN pluginDN) + { + DirectoryServerPlugin plugin = DirectoryServer.getPluginConfigManager().getRegisteredPlugin(pluginDN); + assertNotNull(plugin, "The " + pluginDN + " plugin is not registered with the server"); + return (PluginTypeTrackingPlugin) plugin; + } + + private static int countChangeListeners(DN pluginDN) + { + return TestCaseUtils.getServerContext().getConfigurationHandler().getChangeListeners(pluginDN).size(); + } + + private static ResultCode setPluginTypes(DN pluginDN, String... pluginTypes) + { + ModifyRequest request = newModifyRequest(pluginDN).addModification(REPLACE, "ds-cfg-plugin-type", pluginTypes); + return getRootConnection().processModify(request).getResultCode(); + } + + /** Modifies the test entry and returns how many times the tracking plugin was invoked before it. */ + private static int modifyTestEntryAndCountPreOperation() throws Exception + { + DN testEntryDN = DN.valueOf(TestCaseUtils.TEST_ROOT_DN_STRING); + PluginTypeTrackingPlugin.takePreOperationModifyCount(testEntryDN); + ModifyRequest request = newModifyRequest(testEntryDN).addModification(REPLACE, "description", "modified"); + assertEquals(getRootConnection().processModify(request).getResultCode(), ResultCode.SUCCESS); + return PluginTypeTrackingPlugin.takePreOperationModifyCount(testEntryDN); + } } diff --git a/opendj-server-legacy/src/test/java/org/opends/server/plugins/PluginTypeTrackingPlugin.java b/opendj-server-legacy/src/test/java/org/opends/server/plugins/PluginTypeTrackingPlugin.java new file mode 100644 index 0000000000..6ee232cb92 --- /dev/null +++ b/opendj-server-legacy/src/test/java/org/opends/server/plugins/PluginTypeTrackingPlugin.java @@ -0,0 +1,99 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package org.opends.server.plugins; + +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicInteger; + +import org.forgerock.i18n.LocalizableMessage; +import org.forgerock.opendj.config.server.ConfigException; +import org.forgerock.opendj.ldap.DN; +import org.forgerock.opendj.server.config.server.PluginCfg; +import org.opends.server.api.plugin.DirectoryServerPlugin; +import org.opends.server.api.plugin.PluginResult; +import org.opends.server.api.plugin.PluginType; +import org.opends.server.types.operation.PostOperationModifyOperation; +import org.opends.server.types.operation.PreOperationModifyOperation; + +/** + * A plugin which records the modify operations it is invoked for, per target entry, and whether + * it has been finalized. It refuses {@link #REFUSED_TYPE} in {@link #initializePlugin} only, the + * way a plugin which checks its plugin types there and not in + * {@link #isConfigurationAcceptable} does: such a configuration passes the acceptance phase. + */ +public class PluginTypeTrackingPlugin extends DirectoryServerPlugin +{ + /** The plugin type which {@link #initializePlugin} refuses. */ + public static final PluginType REFUSED_TYPE = PluginType.PRE_OPERATION_DELETE; + + private static final ConcurrentHashMap PRE_OPERATION_MODIFY_COUNTS = new ConcurrentHashMap<>(); + + private volatile boolean finalized; + + /** + * Returns how many times a pre-operation modify of the given entry has invoked a plugin of this + * class, and resets that count. + * + * @param entryDN + * The DN of the modified entry. + * @return The number of invocations since the previous call. + */ + public static int takePreOperationModifyCount(DN entryDN) + { + AtomicInteger count = PRE_OPERATION_MODIFY_COUNTS.remove(entryDN); + return count != null ? count.get() : 0; + } + + @Override + public void initializePlugin(Set pluginTypes, PluginCfg configuration) throws ConfigException + { + if (pluginTypes.contains(REFUSED_TYPE)) + { + throw new ConfigException(LocalizableMessage.raw("Plugin type " + REFUSED_TYPE + " is not supported")); + } + } + + @Override + public PluginResult.PreOperation doPreOperation(PreOperationModifyOperation modifyOperation) + { + PRE_OPERATION_MODIFY_COUNTS.computeIfAbsent(modifyOperation.getEntryDN(), dn -> new AtomicInteger()) + .incrementAndGet(); + return PluginResult.PreOperation.continueOperationProcessing(); + } + + @Override + public PluginResult.PostOperation doPostOperation(PostOperationModifyOperation modifyOperation) + { + return PluginResult.PostOperation.continueOperationProcessing(); + } + + @Override + public void finalizePlugin() + { + finalized = true; + } + + /** + * Indicates whether the plugin manager has finalized this instance. + * + * @return {@code true} once {@link #finalizePlugin()} has been called. + */ + public boolean isFinalized() + { + return finalized; + } +} diff --git a/opendj-server-legacy/src/test/java/org/opends/server/plugins/ReferentialIntegrityPluginTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/plugins/ReferentialIntegrityPluginTestCase.java index 580f328097..e80f130287 100644 --- a/opendj-server-legacy/src/test/java/org/opends/server/plugins/ReferentialIntegrityPluginTestCase.java +++ b/opendj-server-legacy/src/test/java/org/opends/server/plugins/ReferentialIntegrityPluginTestCase.java @@ -1692,6 +1692,54 @@ public void testEnforceIntegrityAddGroupWithMissingMember() throws Exception assertEquals(addOperation.getResultCode(), ResultCode.CONSTRAINT_VIOLATION); } + /** + * Test case: + * - the plugin is enabled for the post-operation and subordinate plugin + * types only + * - while it stays enabled, the pre-operation add and modify plugin types + * are added and integrity is enforced on the attribute 'member' + * - add a group 'referent group' to the 'dc=example,dc=com' with the + * 'member' attribute pointing to the existing user entries and one missing + * - CONSTRAINT VIOLATION: the new plugin types take effect without the + * plugin being disabled and enabled again + * @throws Exception + */ + @Test + public void testEnforceIntegrityAfterPluginTypesAddedToEnabledPlugin() throws Exception + { + replaceAttrEntry(configDN, "ds-cfg-enabled", "false"); + replaceAttrEntry(configDN, dsConfigPluginType, + "postoperationdelete", + "postoperationmodifydn", + "subordinatemodifydn", + "subordinatedelete"); + addAttrEntry(configDN, dsConfigBaseDN, "dc=example,dc=com"); + replaceAttrEntry(configDN, dsConfigAttrType, "member"); + replaceAttrEntry(configDN, "ds-cfg-enabled", "true"); + + assertEquals(replaceAttrEntry(configDN, dsConfigPluginType, + "postoperationdelete", + "postoperationmodifydn", + "subordinatemodifydn", + "subordinatedelete", + "preoperationadd", + "preoperationmodify").getResultCode(), ResultCode.SUCCESS); + assertEquals(replaceAttrEntry(configDN, dsConfigEnforceIntegrity, "true").getResultCode(), + ResultCode.SUCCESS); + + Entry entry = TestCaseUtils.makeEntry( + "dn: cn=referent group,ou=groups,dc=example,dc=com", + "objectclass: top", + "objectclass: groupofnames", + "cn: refetent group", + "member: uid=user.1,ou=people,ou=dept,dc=example,dc=com", + "member: uid=bad,ou=people,ou=dept,dc=example,dc=com" + ); + + AddOperation addOperation = getRootConnection().processAdd(entry); + assertEquals(addOperation.getResultCode(), ResultCode.CONSTRAINT_VIOLATION); + } + /** * Test case: * - integrity is enforced on the attribute 'member' From 2c485e3aae21899a020ecd94c0db406ea514b2b3 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Wed, 30 Sep 2026 15:17:59 +0300 Subject: [PATCH 2/3] [#1120] Register the replacement plugin before finalizing the old one, and finalize a plugin that fails to initialize Review round 1 of #1127: - replacePlugin() registers the new instance in place of the old one and only then finalizes the old one, outside pluginLock, and an exception from that finalizePlugin() is logged instead of leaving the plugin unregistered. - loadPlugin() finalizes a plugin whose initializePlugin() fails, so a change listener it registered first does not stay behind. This covers startup, add, enable and the replacement. - invoke-for-internal-operations is applied to the running instance also when the plugin refuses the new plugin types. - The pre-operation invoke methods read the plugin array once, so a concurrent registration change cannot skip a plugin or make the index run past the end of the array. - SambaPasswordPlugin and PasswordPolicyImportPlugin remove their change listener in finalizePlugin(). - Tests: the test plugin registers a change listener and counts post-operation modifies; new cases for a failing finalizePlugin() and for a Java class change together with the plugin types; listener counts for the Samba and password policy import plugins. --- .../chap-writing-plugins.adoc | 2 +- .../server/core/PluginConfigManager.java | 160 +++++++++++++----- .../plugins/PasswordPolicyImportPlugin.java | 9 + .../server/plugins/SambaPasswordPlugin.java | 6 + .../core/PluginConfigManagerTestCase.java | 101 ++++++++++- .../OtherPluginTypeTrackingPlugin.java | 24 +++ .../PasswordPolicyImportPluginTestCase.java | 41 +++++ .../plugins/PluginTypeTrackingPlugin.java | 100 ++++++++++- .../plugins/SambaPasswordPluginTestCase.java | 34 ++++ 9 files changed, 423 insertions(+), 54 deletions(-) create mode 100644 opendj-server-legacy/src/test/java/org/opends/server/plugins/OtherPluginTypeTrackingPlugin.java diff --git a/opendj-doc-generated-ref/src/main/asciidoc/server-dev-guide/chap-writing-plugins.adoc b/opendj-doc-generated-ref/src/main/asciidoc/server-dev-guide/chap-writing-plugins.adoc index daeeadcb00..49fda8e2b6 100644 --- a/opendj-doc-generated-ref/src/main/asciidoc/server-dev-guide/chap-writing-plugins.adoc +++ b/opendj-doc-generated-ref/src/main/asciidoc/server-dev-guide/chap-writing-plugins.adoc @@ -373,7 +373,7 @@ Although the example plugin's `isConfigurationChangeAcceptable()` method always In the `applyConfigurationChange()` method the plugin must modify its configuration as necessary. The example plugin can handle configuration changes without further intervention by the administrator. Other plugins might require administrative intervention because changes can be made that can only be taken into account at plugin initialization. -The plugin types are the exception: a plugin receives them only when it is initialized, so the server does not pass a change to `plugin-type` to the running plugin. When `plugin-type` changes on an enabled plugin, the server creates a new instance of the plugin, initializes it for the new plugin types, and then deregisters the running instance and calls its `finalizePlugin()` method. If `initializePlugin()` refuses the new plugin types, the change fails and the running instance stays registered for the plugin types it had. State that the plugin keeps in memory does not carry over to the new instance. +The plugin types are the exception: a plugin receives them only when it is initialized, so the server does not pass a change to `plugin-type` to the running plugin. When `plugin-type` changes on an enabled plugin, the server creates a new instance of the plugin, initializes it for the new plugin types, registers it in place of the running instance, and then calls the `finalizePlugin()` method of the running instance. If `initializePlugin()` refuses the new plugin types, the change fails and the running instance stays registered for the plugin types it had. The server then calls `finalizePlugin()` of the refused instance, so `finalizePlugin()` must release whatever `initializePlugin()` acquired before it failed, such as a change listener. Until `plugin-type` is changed to types the plugin accepts, every later change of the plugin configuration also fails with the same reason, although the other changes are applied to the running instance. State that the plugin keeps in memory does not carry over to the new instance. In the example plugin, the method that extends the server's behavior is the `doStartup()` method. Which method is implemented depends on what class the plugin extends. For example, a password validator extending link:../javadoc/index.html?org/opends/server/api/PasswordValidator.html[PasswordValidator, window=\_blank] would implement a `passwordIsAcceptable()` method. diff --git a/opendj-server-legacy/src/main/java/org/opends/server/core/PluginConfigManager.java b/opendj-server-legacy/src/main/java/org/opends/server/core/PluginConfigManager.java index 5cacd02f1c..9215730043 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/core/PluginConfigManager.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/core/PluginConfigManager.java @@ -396,7 +396,18 @@ public void initializeUserPlugins(Set pluginTypes) { plugin.initializeInternal(serverContext, configuration.dn(), pluginTypes, configuration.isInvokeForInternalOperations()); - plugin.initializePlugin(pluginTypes, configuration); + try + { + plugin.initializePlugin(pluginTypes, configuration); + } + catch (Exception e) + { + // The plugin may have registered a change listener or started a + // thread before it failed, and it is never registered, so nothing + // else would finalize it. + finalizePluginQuietly(plugin); + throw e; + } } else { @@ -419,6 +430,24 @@ public void initializeUserPlugins(Set pluginTypes) } } + /** + * Finalizes the provided plugin, logging rather than throwing anything its + * {@code finalizePlugin()} method throws. + * + * @param plugin The plugin to finalize. + */ + private void finalizePluginQuietly(DirectoryServerPlugin plugin) + { + try + { + plugin.finalizePlugin(); + } + catch (Exception e) + { + logger.traceException(e); + } + } + /** * Gets the OpenDS plugin type object that corresponds to the configuration * counterpart. @@ -2322,9 +2351,12 @@ public PluginResult.PreOperation invokePreOperationAddPlugins( throws CanceledOperationException { PluginResult.PreOperation result = null; - for (int i = 0; i < preOperationAddPlugins.length; i++) + // Read the array once: it is replaced when a plugin is registered or + // deregistered, and the index must stay within the one being iterated. + DirectoryServerPlugin[] plugins = preOperationAddPlugins; + for (int i = 0; i < plugins.length; i++) { - DirectoryServerPlugin p = preOperationAddPlugins[i]; + DirectoryServerPlugin p = plugins[i]; if (isInternalOperation(addOperation, p)) { continue; @@ -2340,18 +2372,18 @@ public PluginResult.PreOperation invokePreOperationAddPlugins( } catch (Exception e) { - return handlePreOperationException(e, i, preOperationAddPlugins, + return handlePreOperationException(e, i, plugins, addOperation, p); } if (result == null) { - return handlePreOperationResult(addOperation, i, preOperationAddPlugins, + return handlePreOperationResult(addOperation, i, plugins, p); } else if (!result.continuePluginProcessing()) { - registerSkippedPreOperationPlugins(i, preOperationAddPlugins, + registerSkippedPreOperationPlugins(i, plugins, addOperation); return result; } @@ -2381,9 +2413,12 @@ public PluginResult.PreOperation invokePreOperationBindPlugins( { PluginResult.PreOperation result = null; - for (int i = 0; i < preOperationBindPlugins.length; i++) + // Read the array once: it is replaced when a plugin is registered or + // deregistered, and the index must stay within the one being iterated. + DirectoryServerPlugin[] plugins = preOperationBindPlugins; + for (int i = 0; i < plugins.length; i++) { - DirectoryServerPlugin p = preOperationBindPlugins[i]; + DirectoryServerPlugin p = plugins[i]; if (isInternalOperation(bindOperation, p)) { continue; @@ -2395,18 +2430,18 @@ public PluginResult.PreOperation invokePreOperationBindPlugins( } catch (Exception e) { - return handlePreOperationException(e, i, preOperationBindPlugins, + return handlePreOperationException(e, i, plugins, bindOperation, p); } if (result == null) { return handlePreOperationResult(bindOperation, i, - preOperationBindPlugins, p); + plugins, p); } else if (!result.continuePluginProcessing()) { - registerSkippedPreOperationPlugins(i, preOperationBindPlugins, + registerSkippedPreOperationPlugins(i, plugins, bindOperation); return result; @@ -2439,9 +2474,12 @@ public PluginResult.PreOperation invokePreOperationComparePlugins( throws CanceledOperationException { PluginResult.PreOperation result = null; - for (int i = 0; i < preOperationComparePlugins.length; i++) + // Read the array once: it is replaced when a plugin is registered or + // deregistered, and the index must stay within the one being iterated. + DirectoryServerPlugin[] plugins = preOperationComparePlugins; + for (int i = 0; i < plugins.length; i++) { - DirectoryServerPlugin p = preOperationComparePlugins[i]; + DirectoryServerPlugin p = plugins[i]; if (isInternalOperation(compareOperation, p)) { continue; @@ -2457,14 +2495,14 @@ public PluginResult.PreOperation invokePreOperationComparePlugins( } catch (Exception e) { - return handlePreOperationException(e, i, preOperationComparePlugins, + return handlePreOperationException(e, i, plugins, compareOperation, p); } if (result == null) { return handlePreOperationResult(compareOperation, i, - preOperationComparePlugins, p); + plugins, p); } else if (!result.continuePluginProcessing()) { @@ -2498,9 +2536,12 @@ public PluginResult.PreOperation invokePreOperationDeletePlugins( throws CanceledOperationException { PluginResult.PreOperation result = null; - for (int i = 0; i < preOperationDeletePlugins.length; i++) + // Read the array once: it is replaced when a plugin is registered or + // deregistered, and the index must stay within the one being iterated. + DirectoryServerPlugin[] plugins = preOperationDeletePlugins; + for (int i = 0; i < plugins.length; i++) { - DirectoryServerPlugin p = preOperationDeletePlugins[i]; + DirectoryServerPlugin p = plugins[i]; if (isInternalOperation(deleteOperation, p)) { continue; @@ -2516,18 +2557,18 @@ public PluginResult.PreOperation invokePreOperationDeletePlugins( } catch (Exception e) { - return handlePreOperationException(e, i, preOperationDeletePlugins, + return handlePreOperationException(e, i, plugins, deleteOperation, p); } if (result == null) { return handlePreOperationResult(deleteOperation, i, - preOperationDeletePlugins, p); + plugins, p); } else if (!result.continuePluginProcessing()) { - registerSkippedPreOperationPlugins(i, preOperationDeletePlugins, + registerSkippedPreOperationPlugins(i, plugins, deleteOperation); return result; @@ -2596,9 +2637,12 @@ public PluginResult.PreOperation invokePreOperationExtendedPlugins( throws CanceledOperationException { PluginResult.PreOperation result = null; - for (int i = 0; i < preOperationExtendedPlugins.length; i++) + // Read the array once: it is replaced when a plugin is registered or + // deregistered, and the index must stay within the one being iterated. + DirectoryServerPlugin[] plugins = preOperationExtendedPlugins; + for (int i = 0; i < plugins.length; i++) { - DirectoryServerPlugin p = preOperationExtendedPlugins[i]; + DirectoryServerPlugin p = plugins[i]; if (isInternalOperation(extendedOperation, p)) { registerSkippedPreOperationPlugin(p, extendedOperation); @@ -2615,18 +2659,18 @@ public PluginResult.PreOperation invokePreOperationExtendedPlugins( } catch (Exception e) { - return handlePreOperationException(e, i, preOperationExtendedPlugins, + return handlePreOperationException(e, i, plugins, extendedOperation, p); } if (result == null) { return handlePreOperationResult(extendedOperation, i, - preOperationExtendedPlugins, p); + plugins, p); } else if (!result.continuePluginProcessing()) { - registerSkippedPreOperationPlugins(i, preOperationExtendedPlugins, + registerSkippedPreOperationPlugins(i, plugins, extendedOperation); return result; @@ -2659,9 +2703,12 @@ public PluginResult.PreOperation invokePreOperationModifyPlugins( throws CanceledOperationException { PluginResult.PreOperation result = null; - for (int i = 0; i < preOperationModifyPlugins.length; i++) + // Read the array once: it is replaced when a plugin is registered or + // deregistered, and the index must stay within the one being iterated. + DirectoryServerPlugin[] plugins = preOperationModifyPlugins; + for (int i = 0; i < plugins.length; i++) { - DirectoryServerPlugin p = preOperationModifyPlugins[i]; + DirectoryServerPlugin p = plugins[i]; if (isInternalOperation(modifyOperation, p)) { continue; @@ -2677,18 +2724,18 @@ public PluginResult.PreOperation invokePreOperationModifyPlugins( } catch (Exception e) { - return handlePreOperationException(e, i, preOperationModifyPlugins, + return handlePreOperationException(e, i, plugins, modifyOperation, p); } if (result == null) { return handlePreOperationResult(modifyOperation, i, - preOperationModifyPlugins, p); + plugins, p); } else if (!result.continuePluginProcessing()) { - registerSkippedPreOperationPlugins(i, preOperationModifyPlugins, + registerSkippedPreOperationPlugins(i, plugins, modifyOperation); return result; @@ -2721,9 +2768,12 @@ public PluginResult.PreOperation invokePreOperationModifyDNPlugins( throws CanceledOperationException { PluginResult.PreOperation result = null; - for (int i = 0; i < preOperationModifyDNPlugins.length; i++) + // Read the array once: it is replaced when a plugin is registered or + // deregistered, and the index must stay within the one being iterated. + DirectoryServerPlugin[] plugins = preOperationModifyDNPlugins; + for (int i = 0; i < plugins.length; i++) { - DirectoryServerPlugin p = preOperationModifyDNPlugins[i]; + DirectoryServerPlugin p = plugins[i]; if (isInternalOperation(modifyDNOperation, p)) { continue; @@ -2739,18 +2789,18 @@ public PluginResult.PreOperation invokePreOperationModifyDNPlugins( } catch (Exception e) { - return handlePreOperationException(e, i, preOperationModifyDNPlugins, + return handlePreOperationException(e, i, plugins, modifyDNOperation, p); } if (result == null) { return handlePreOperationResult(modifyDNOperation, i, - preOperationModifyDNPlugins, p); + plugins, p); } else if (!result.continuePluginProcessing()) { - registerSkippedPreOperationPlugins(i, preOperationModifyDNPlugins, + registerSkippedPreOperationPlugins(i, plugins, modifyDNOperation); return result; @@ -2783,9 +2833,12 @@ public PluginResult.PreOperation invokePreOperationSearchPlugins( throws CanceledOperationException { PluginResult.PreOperation result = null; - for (int i = 0; i < preOperationSearchPlugins.length; i++) + // Read the array once: it is replaced when a plugin is registered or + // deregistered, and the index must stay within the one being iterated. + DirectoryServerPlugin[] plugins = preOperationSearchPlugins; + for (int i = 0; i < plugins.length; i++) { - DirectoryServerPlugin p = preOperationSearchPlugins[i]; + DirectoryServerPlugin p = plugins[i]; if (isInternalOperation(searchOperation, p)) { continue; @@ -2801,18 +2854,18 @@ public PluginResult.PreOperation invokePreOperationSearchPlugins( } catch (Exception e) { - return handlePreOperationException(e, i, preOperationSearchPlugins, + return handlePreOperationException(e, i, plugins, searchOperation, p); } if (result == null) { return handlePreOperationResult(searchOperation, i, - preOperationSearchPlugins, p); + plugins, p); } else if (!result.continuePluginProcessing()) { - registerSkippedPreOperationPlugins(i, preOperationSearchPlugins, + registerSkippedPreOperationPlugins(i, plugins, searchOperation); return result; @@ -4490,6 +4543,11 @@ public ConfigChangeResult applyConfigurationChange( String className = configuration.getJavaClass(); if (existingPlugin != null) { + // Update the running instance first, so that it has the new value even + // when it stays in use because a replacement cannot be initialized. + existingPlugin.setInvokeForInternalOperations( + configuration.isInvokeForInternalOperations()); + if (! className.equals(existingPlugin.getClass().getName())) { ccr.setAdminActionRequired(true); @@ -4500,13 +4558,9 @@ public ConfigChangeResult applyConfigurationChange( if (!pluginTypes.equals(existingPlugin.getPluginTypes())) { replacePlugin(configuration, pluginTypes, ccr); - return ccr; } } - existingPlugin.setInvokeForInternalOperations( - configuration.isInvokeForInternalOperations()); - return ccr; } @@ -4537,7 +4591,11 @@ public ConfigChangeResult applyConfigurationChange( * plugin types of its new configuration. A plugin receives its plugin types, and may refuse * them, only when it is initialized, so the registered instance cannot be moved to other types * in place. The new instance is initialized first: if that fails, the registered one stays in - * use for the plugin types it was initialized for. + * use for the plugin types it was initialized for. Otherwise the registered instance is + * deregistered, the new one is registered in its place, and only then is the old one + * finalized, so that the plugin is missing only for the time the arrays are rewritten, not for + * the time the old instance takes to finalize, and a failure to finalize does not leave the + * plugin unregistered. * * @param configuration * The new configuration of the plugin. @@ -4561,16 +4619,26 @@ private void replacePlugin(PluginCfg configuration, Set pluginTypes, return; } + DirectoryServerPlugin oldPlugin; pluginLock.lock(); try { - deregisterPlugin(configuration.dn()); + oldPlugin = registeredPlugins.remove(configuration.dn()); + if (oldPlugin != null) + { + deregisterPlugin0(oldPlugin); + } registerPlugin(plugin, configuration.dn(), pluginTypes); } finally { pluginLock.unlock(); } + + if (oldPlugin != null) + { + finalizePluginQuietly(oldPlugin); + } } private HashSet getPluginTypes(PluginCfg configuration) diff --git a/opendj-server-legacy/src/main/java/org/opends/server/plugins/PasswordPolicyImportPlugin.java b/opendj-server-legacy/src/main/java/org/opends/server/plugins/PasswordPolicyImportPlugin.java index e69ff72370..7ac2c6b524 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/plugins/PasswordPolicyImportPlugin.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/plugins/PasswordPolicyImportPlugin.java @@ -74,6 +74,8 @@ public final class PasswordPolicyImportPlugin { private static final LocalizedLogger logger = LocalizedLogger.getLoggerForThisClass(); + /** The configuration which this plugin is registered with as a change listener. */ + private PasswordPolicyImportPluginCfg currentConfig; /** The attribute type used to specify the password policy for an entry. */ private AttributeType customPolicyAttribute; /** The set of attribute types defined in the schema with the auth password syntax. */ @@ -107,6 +109,7 @@ public final void initializePlugin(Set pluginTypes, throws ConfigException { configuration.addPasswordPolicyImportChangeListener(this); + currentConfig = configuration; Schema schema = DirectoryServer.getInstance().getServerContext().getSchema(); customPolicyAttribute = schema.getAttributeType(OP_ATTR_PWPOLICY_POLICY_DN); @@ -223,6 +226,12 @@ else if (! defaultAuthPasswordSchemes[i].supportsAuthPasswordSyntax()) processImportBegin(null, null); } + @Override + public final void finalizePlugin() + { + currentConfig.removePasswordPolicyImportChangeListener(this); + } + @Override public void processImportBegin(LocalBackend backend, LDIFImportConfig config) { diff --git a/opendj-server-legacy/src/main/java/org/opends/server/plugins/SambaPasswordPlugin.java b/opendj-server-legacy/src/main/java/org/opends/server/plugins/SambaPasswordPlugin.java index 0c1b191acb..30c5d1bcde 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/plugins/SambaPasswordPlugin.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/plugins/SambaPasswordPlugin.java @@ -865,6 +865,12 @@ public void initializePlugin(final Set pluginTypes, this.config = configuration; } + @Override + public void finalizePlugin() + { + config.removeSambaPasswordChangeListener(this); + } + /** diff --git a/opendj-server-legacy/src/test/java/org/opends/server/core/PluginConfigManagerTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/core/PluginConfigManagerTestCase.java index 9ed91d4fec..7b6efe3327 100644 --- a/opendj-server-legacy/src/test/java/org/opends/server/core/PluginConfigManagerTestCase.java +++ b/opendj-server-legacy/src/test/java/org/opends/server/core/PluginConfigManagerTestCase.java @@ -23,6 +23,7 @@ import org.opends.server.TestCaseUtils; import org.opends.server.api.plugin.DirectoryServerPlugin; import org.opends.server.api.plugin.PluginType; +import org.opends.server.plugins.OtherPluginTypeTrackingPlugin; import org.opends.server.plugins.PluginTypeTrackingPlugin; import org.forgerock.opendj.ldap.DN; import org.forgerock.opendj.ldap.ResultCode; @@ -650,7 +651,8 @@ public void testPluginOrder(String pluginOrderString, /** * A change to the plugin types of an enabled plugin takes effect at once: the plugin is - * re-created for the new types, and the instance registered for the old ones is finalized. + * re-created for the new types, and the instance registered for the old ones is finalized once + * the new one is registered in its place. */ @Test public void testPluginTypeChangeAppliesToEnabledPlugin() throws Exception @@ -668,6 +670,8 @@ public void testPluginTypeChangeAppliesToEnabledPlugin() throws Exception PluginTypeTrackingPlugin replacement = getTrackingPlugin(pluginDN); assertNotSame(replacement, original); assertTrue(original.isFinalized(), "The instance registered for the old plugin types is not finalized"); + assertSame(original.getRegisteredWhenFinalized(), replacement, + "The old instance was finalized before the new one was registered in its place"); assertFalse(replacement.isFinalized()); assertEquals(replacement.getPluginTypes(), EnumSet.of(POST_OPERATION_MODIFY, PRE_OPERATION_MODIFY)); assertEquals(countChangeListeners(pluginDN), changeListeners); @@ -678,6 +682,8 @@ public void testPluginTypeChangeAppliesToEnabledPlugin() throws Exception assertEquals(getTrackingPlugin(pluginDN).getPluginTypes(), EnumSet.of(POST_OPERATION_MODIFY)); assertTrue(replacement.isFinalized()); assertEquals(countChangeListeners(pluginDN), changeListeners); + assertEquals(PluginTypeTrackingPlugin.takeChangesSeenWhenFinalized(), 0, + "A finalized instance received a change of its configuration entry"); assertEquals(modifyTestEntryAndCountPreOperation(), 0); } finally @@ -688,7 +694,9 @@ public void testPluginTypeChangeAppliesToEnabledPlugin() throws Exception /** * When the plugin cannot be initialized for the new plugin types, the change fails and the - * instance registered for the old plugin types stays registered and in use. + * instance registered for the old plugin types stays registered and in use. The rejected + * instance leaves nothing behind, and later changes of the entry still reach the running + * instance. */ @Test public void testPluginTypeChangeRefusedByPluginKeepsRunningPlugin() throws Exception @@ -699,6 +707,7 @@ public void testPluginTypeChangeRefusedByPluginKeepsRunningPlugin() throws Excep { assertEquals(setPluginTypes(pluginDN, "postOperationModify", "preOperationModify"), ResultCode.SUCCESS); PluginTypeTrackingPlugin running = getTrackingPlugin(pluginDN); + int changeListeners = countChangeListeners(pluginDN); ResultCode resultCode = setPluginTypes(pluginDN, "postOperationModify", "preOperationDelete"); @@ -706,6 +715,55 @@ public void testPluginTypeChangeRefusedByPluginKeepsRunningPlugin() throws Excep assertSame(getTrackingPlugin(pluginDN), running); assertFalse(running.isFinalized()); assertEquals(running.getPluginTypes(), EnumSet.of(POST_OPERATION_MODIFY, PRE_OPERATION_MODIFY)); + assertEquals(countChangeListeners(pluginDN), changeListeners, + "The instance which refused the new plugin types is still listening to the entry"); + assertEquals(modifyTestEntryAndCountPreOperation(), 1); + + // The stored plugin types are still the refused ones, so the plugin is re-created again, and + // refuses again, but the other properties are applied to the running instance. + resultCode = getRootConnection().processModify(newModifyRequest(pluginDN) + .addModification(REPLACE, "ds-cfg-invoke-for-internal-operations", "false")).getResultCode(); + + assertNotEquals(resultCode, ResultCode.SUCCESS); + assertSame(getTrackingPlugin(pluginDN), running); + assertFalse(running.invokeForInternalOperations()); + assertEquals(countChangeListeners(pluginDN), changeListeners); + } + finally + { + TestCaseUtils.deleteEntry(pluginDN); + } + } + + /** + * When the instance registered for the old plugin types fails to finalize, the new instance is + * already registered in its place and stays in use. + */ + @Test + public void testPluginTypeChangeWhenOldInstanceFailsToFinalize() throws Exception + { + TestCaseUtils.initializeTestBackend(true); + DN pluginDN = addTrackingPlugin(); + try + { + PluginTypeTrackingPlugin original = getTrackingPlugin(pluginDN); + + PluginTypeTrackingPlugin.setFailToFinalize(true); + ResultCode resultCode; + try + { + resultCode = setPluginTypes(pluginDN, "postOperationModify", "preOperationModify"); + } + finally + { + PluginTypeTrackingPlugin.setFailToFinalize(false); + } + + assertEquals(resultCode, ResultCode.SUCCESS); + assertTrue(original.isFinalized()); + PluginTypeTrackingPlugin replacement = getTrackingPlugin(pluginDN); + assertNotSame(replacement, original); + assertEquals(replacement.getPluginTypes(), EnumSet.of(POST_OPERATION_MODIFY, PRE_OPERATION_MODIFY)); assertEquals(modifyTestEntryAndCountPreOperation(), 1); } finally @@ -714,6 +772,35 @@ public void testPluginTypeChangeRefusedByPluginKeepsRunningPlugin() throws Excep } } + /** + * A change of the Java class needs administrative action, so a change of the plugin types which + * comes with it is not applied: the running instance stays registered for its plugin types. + */ + @Test + public void testPluginTypeChangeWithJavaClassChangeKeepsRunningPlugin() throws Exception + { + TestCaseUtils.initializeTestBackend(true); + DN pluginDN = addTrackingPlugin(); + try + { + PluginTypeTrackingPlugin original = getTrackingPlugin(pluginDN); + + ModifyRequest request = newModifyRequest(pluginDN) + .addModification(REPLACE, "ds-cfg-java-class", OtherPluginTypeTrackingPlugin.class.getName()) + .addModification(REPLACE, "ds-cfg-plugin-type", "postOperationModify", "preOperationModify"); + assertEquals(getRootConnection().processModify(request).getResultCode(), ResultCode.SUCCESS); + + assertSame(getTrackingPlugin(pluginDN), original); + assertFalse(original.isFinalized()); + assertEquals(original.getPluginTypes(), EnumSet.of(POST_OPERATION_MODIFY)); + assertEquals(modifyTestEntryAndCountPreOperation(), 0); + } + finally + { + TestCaseUtils.deleteEntry(pluginDN); + } + } + private static DN addTrackingPlugin() throws Exception { DN pluginDN = DN.valueOf("cn=Plugin Type Tracking Plugin,cn=Plugins,cn=config"); @@ -726,6 +813,7 @@ private static DN addTrackingPlugin() throws Exception "ds-cfg-enabled: true", "ds-cfg-plugin-type: postOperationModify", "ds-cfg-invoke-for-internal-operations: true"); + PluginTypeTrackingPlugin.takeChangesSeenWhenFinalized(); return pluginDN; } @@ -747,14 +835,19 @@ private static ResultCode setPluginTypes(DN pluginDN, String... pluginTypes) return getRootConnection().processModify(request).getResultCode(); } - /** Modifies the test entry and returns how many times the tracking plugin was invoked before it. */ + /** + * Modifies the test entry and returns how many times the tracking plugin was invoked before it. + * The plugin always has the post-operation modify type, so it must be invoked once after it. + */ private static int modifyTestEntryAndCountPreOperation() throws Exception { DN testEntryDN = DN.valueOf(TestCaseUtils.TEST_ROOT_DN_STRING); PluginTypeTrackingPlugin.takePreOperationModifyCount(testEntryDN); + PluginTypeTrackingPlugin.takePostOperationModifyCount(testEntryDN); ModifyRequest request = newModifyRequest(testEntryDN).addModification(REPLACE, "description", "modified"); assertEquals(getRootConnection().processModify(request).getResultCode(), ResultCode.SUCCESS); + assertEquals(PluginTypeTrackingPlugin.takePostOperationModifyCount(testEntryDN), 1, + "The tracking plugin was not invoked once after the modify of the test entry"); return PluginTypeTrackingPlugin.takePreOperationModifyCount(testEntryDN); } } - diff --git a/opendj-server-legacy/src/test/java/org/opends/server/plugins/OtherPluginTypeTrackingPlugin.java b/opendj-server-legacy/src/test/java/org/opends/server/plugins/OtherPluginTypeTrackingPlugin.java new file mode 100644 index 0000000000..43c4d13900 --- /dev/null +++ b/opendj-server-legacy/src/test/java/org/opends/server/plugins/OtherPluginTypeTrackingPlugin.java @@ -0,0 +1,24 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package org.opends.server.plugins; + +/** + * A {@link PluginTypeTrackingPlugin} of another class, so that a test can change the Java class of + * a plugin to one which the server can load. + */ +public class OtherPluginTypeTrackingPlugin extends PluginTypeTrackingPlugin +{ +} diff --git a/opendj-server-legacy/src/test/java/org/opends/server/plugins/PasswordPolicyImportPluginTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/plugins/PasswordPolicyImportPluginTestCase.java index b9e457312b..642745ee24 100644 --- a/opendj-server-legacy/src/test/java/org/opends/server/plugins/PasswordPolicyImportPluginTestCase.java +++ b/opendj-server-legacy/src/test/java/org/opends/server/plugins/PasswordPolicyImportPluginTestCase.java @@ -13,13 +13,21 @@ * * Copyright 2006-2008 Sun Microsystems, Inc. * Portions Copyright 2014-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.opends.server.plugins; +import static org.forgerock.opendj.ldap.ModificationType.*; +import static org.forgerock.opendj.ldap.requests.Requests.*; +import static org.opends.server.protocols.internal.InternalClientConnection.*; +import static org.testng.Assert.*; + import java.io.ByteArrayInputStream; import java.util.ArrayList; import java.util.List; +import org.forgerock.opendj.ldap.ResultCode; +import org.forgerock.opendj.ldap.requests.ModifyRequest; import org.opends.server.TestCaseUtils; import org.forgerock.opendj.server.config.meta.PasswordPolicyImportPluginCfgDefn; import org.opends.server.api.plugin.PluginType; @@ -291,5 +299,38 @@ public void testDoLDIFImport() plugin.doLDIFImport(importConfig, e); } } + + /** + * Disabling the plugin finalizes it, and the finalized instance no longer listens to the + * configuration entry, so enabling the plugin again adds no listener. + */ + @Test + public void testDisableAndEnableLeavesNoChangeListenerBehind() + throws Exception + { + DN dn = DN.valueOf("cn=Password Policy Import,cn=plugins,cn=config"); + int changeListeners = countChangeListeners(dn); + try + { + assertEquals(setEnabled(dn, false), ResultCode.SUCCESS); + } + finally + { + assertEquals(setEnabled(dn, true), ResultCode.SUCCESS); + } + assertNotNull(DirectoryServer.getPluginConfigManager().getRegisteredPlugin(dn)); + assertEquals(countChangeListeners(dn), changeListeners); + } + + private static int countChangeListeners(DN dn) + { + return TestCaseUtils.getServerContext().getConfigurationHandler().getChangeListeners(dn).size(); + } + + private static ResultCode setEnabled(DN dn, boolean enabled) + { + ModifyRequest request = newModifyRequest(dn).addModification(REPLACE, "ds-cfg-enabled", String.valueOf(enabled)); + return getRootConnection().processModify(request).getResultCode(); + } } diff --git a/opendj-server-legacy/src/test/java/org/opends/server/plugins/PluginTypeTrackingPlugin.java b/opendj-server-legacy/src/test/java/org/opends/server/plugins/PluginTypeTrackingPlugin.java index 6ee232cb92..90809a00b2 100644 --- a/opendj-server-legacy/src/test/java/org/opends/server/plugins/PluginTypeTrackingPlugin.java +++ b/opendj-server-legacy/src/test/java/org/opends/server/plugins/PluginTypeTrackingPlugin.java @@ -15,17 +15,21 @@ */ package org.opends.server.plugins; +import java.util.List; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicInteger; import org.forgerock.i18n.LocalizableMessage; +import org.forgerock.opendj.config.server.ConfigChangeResult; import org.forgerock.opendj.config.server.ConfigException; +import org.forgerock.opendj.config.server.ConfigurationChangeListener; import org.forgerock.opendj.ldap.DN; import org.forgerock.opendj.server.config.server.PluginCfg; import org.opends.server.api.plugin.DirectoryServerPlugin; import org.opends.server.api.plugin.PluginResult; import org.opends.server.api.plugin.PluginType; +import org.opends.server.core.DirectoryServer; import org.opends.server.types.operation.PostOperationModifyOperation; import org.opends.server.types.operation.PreOperationModifyOperation; @@ -34,6 +38,8 @@ * it has been finalized. It refuses {@link #REFUSED_TYPE} in {@link #initializePlugin} only, the * way a plugin which checks its plugin types there and not in * {@link #isConfigurationAcceptable} does: such a configuration passes the acceptance phase. + * Like most plugins, it registers a change listener on its configuration before it checks its + * plugin types, and removes it in {@link #finalizePlugin()}. */ public class PluginTypeTrackingPlugin extends DirectoryServerPlugin { @@ -41,8 +47,32 @@ public class PluginTypeTrackingPlugin extends DirectoryServerPlugin public static final PluginType REFUSED_TYPE = PluginType.PRE_OPERATION_DELETE; private static final ConcurrentHashMap PRE_OPERATION_MODIFY_COUNTS = new ConcurrentHashMap<>(); + private static final ConcurrentHashMap POST_OPERATION_MODIFY_COUNTS = new ConcurrentHashMap<>(); + private static final AtomicInteger CHANGES_SEEN_WHEN_FINALIZED = new AtomicInteger(); + private static volatile boolean failToFinalize; + private final ConfigurationChangeListener listener = new ConfigurationChangeListener() + { + @Override + public boolean isConfigurationChangeAcceptable(PluginCfg configuration, List reasons) + { + return true; + } + + @Override + public ConfigChangeResult applyConfigurationChange(PluginCfg configuration) + { + if (finalized) + { + CHANGES_SEEN_WHEN_FINALIZED.incrementAndGet(); + } + return new ConfigChangeResult(); + } + }; + + private volatile PluginCfg configuration; private volatile boolean finalized; + private volatile DirectoryServerPlugin registeredWhenFinalized; /** * Returns how many times a pre-operation modify of the given entry has invoked a plugin of this @@ -54,13 +84,55 @@ public class PluginTypeTrackingPlugin extends DirectoryServerPlugin */ public static int takePreOperationModifyCount(DN entryDN) { - AtomicInteger count = PRE_OPERATION_MODIFY_COUNTS.remove(entryDN); + return take(PRE_OPERATION_MODIFY_COUNTS, entryDN); + } + + /** + * Returns how many times a post-operation modify of the given entry has invoked a plugin of this + * class, and resets that count. + * + * @param entryDN + * The DN of the modified entry. + * @return The number of invocations since the previous call. + */ + public static int takePostOperationModifyCount(DN entryDN) + { + return take(POST_OPERATION_MODIFY_COUNTS, entryDN); + } + + private static int take(ConcurrentHashMap counts, DN entryDN) + { + AtomicInteger count = counts.remove(entryDN); return count != null ? count.get() : 0; } + /** + * Returns how many configuration changes the change listeners of finalized plugins of this class + * have received, and resets that count. + * + * @return The number of changes since the previous call. + */ + public static int takeChangesSeenWhenFinalized() + { + return CHANGES_SEEN_WHEN_FINALIZED.getAndSet(0); + } + + /** + * Makes {@link #finalizePlugin()} throw once it has done its work, or stop throwing. + * + * @param fail + * Whether {@link #finalizePlugin()} throws. + */ + public static void setFailToFinalize(boolean fail) + { + failToFinalize = fail; + } + @Override public void initializePlugin(Set pluginTypes, PluginCfg configuration) throws ConfigException { + this.configuration = configuration; + configuration.addChangeListener(listener); if (pluginTypes.contains(REFUSED_TYPE)) { throw new ConfigException(LocalizableMessage.raw("Plugin type " + REFUSED_TYPE + " is not supported")); @@ -70,21 +142,32 @@ public void initializePlugin(Set pluginTypes, PluginCfg configuratio @Override public PluginResult.PreOperation doPreOperation(PreOperationModifyOperation modifyOperation) { - PRE_OPERATION_MODIFY_COUNTS.computeIfAbsent(modifyOperation.getEntryDN(), dn -> new AtomicInteger()) - .incrementAndGet(); + count(PRE_OPERATION_MODIFY_COUNTS, modifyOperation.getEntryDN()); return PluginResult.PreOperation.continueOperationProcessing(); } @Override public PluginResult.PostOperation doPostOperation(PostOperationModifyOperation modifyOperation) { + count(POST_OPERATION_MODIFY_COUNTS, modifyOperation.getEntryDN()); return PluginResult.PostOperation.continueOperationProcessing(); } + private static void count(ConcurrentHashMap counts, DN entryDN) + { + counts.computeIfAbsent(entryDN, dn -> new AtomicInteger()).incrementAndGet(); + } + @Override public void finalizePlugin() { + configuration.removeChangeListener(listener); + registeredWhenFinalized = DirectoryServer.getPluginConfigManager().getRegisteredPlugin(getPluginEntryDN()); finalized = true; + if (failToFinalize) + { + throw new IllegalStateException("The test makes finalizePlugin() fail"); + } } /** @@ -96,4 +179,15 @@ public boolean isFinalized() { return finalized; } + + /** + * Returns the plugin which the plugin manager had registered for the configuration entry of this + * instance when it finalized this instance. + * + * @return The registered plugin, or {@code null} if there was none. + */ + public DirectoryServerPlugin getRegisteredWhenFinalized() + { + return registeredWhenFinalized; + } } diff --git a/opendj-server-legacy/src/test/java/org/opends/server/plugins/SambaPasswordPluginTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/plugins/SambaPasswordPluginTestCase.java index 177f5874b0..7fa59db052 100644 --- a/opendj-server-legacy/src/test/java/org/opends/server/plugins/SambaPasswordPluginTestCase.java +++ b/opendj-server-legacy/src/test/java/org/opends/server/plugins/SambaPasswordPluginTestCase.java @@ -13,6 +13,7 @@ * * Copyright 2011-2012 profiq s.r.o. * Portions Copyright 2011-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.opends.server.plugins; @@ -794,4 +795,37 @@ public long getCurrentTime() plugin.setTimeStampProvider(null); } } + + /** + * A change of the plugin types re-creates the plugin, and the finalized instance no longer + * listens to the configuration entry. + */ + @Test + public void testPluginTypeChangeLeavesNoChangeListenerBehind() throws Exception + { + DN pluginDN = DN.valueOf("cn=samba password,cn=Plugins,cn=config"); + int changeListeners = countChangeListeners(pluginDN); + try + { + assertEquals(setPluginTypes(pluginDN, "preoperationmodify"), ResultCode.SUCCESS); + assertEquals(countChangeListeners(pluginDN), changeListeners); + } + finally + { + assertEquals(setPluginTypes(pluginDN, "postoperationextended", "preoperationmodify"), ResultCode.SUCCESS); + } + assertEquals(countChangeListeners(pluginDN), changeListeners); + } + + private static int countChangeListeners(DN pluginDN) + { + return TestCaseUtils.getServerContext().getConfigurationHandler().getChangeListeners(pluginDN).size(); + } + + private static ResultCode setPluginTypes(DN pluginDN, String... pluginTypes) + { + ModifyRequest request = Requests.newModifyRequest(pluginDN) + .addModification(REPLACE, "ds-cfg-plugin-type", pluginTypes); + return getRootConnection().processModify(request).getResultCode(); + } } From d4b074945a02b645899a898e85b653003bfc5319 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Wed, 30 Sep 2026 16:42:33 +0300 Subject: [PATCH 3/3] [#1120] Leave no change listener behind a referential integrity plugin refused at startup, and remove the MSAD plugin's listener on finalize Review round 2 of #1127: - ReferentialIntegrityPlugin registers its change listener only after its configuration check, and finalizePlugin() returns early when the check refused the configuration. Before, the listener was registered first and finalizePlugin() failed with a NullPointerException before it removed it, so an instance refused at startup, where there is no acceptance phase, kept listening to its entry. - MsadPlugin removes its change listener in finalizePlugin(), so neither a plugin type change nor a refused plugin type leaves a listener behind. SambaPasswordPlugin.finalizePlugin() no longer fails on an instance that refused its configuration. - The DirectoryServerPlugin.finalizePlugin() Javadoc says that it is also called on an instance whose initializePlugin() threw. - #1123 told the administrator to disable and re-enable the referential integrity plugin after adding plugin types, in chap-groups.adoc and in the check-references error and warning. Added plugin types now take effect at once, so the sentence is replaced and the advice dropped from both messages. - Tests: a change that leaves the plugin types alone keeps the running instance; a referential integrity configuration refused by initializePlugin() leaves no change listener behind. --- .../server-dev-guide/chap-groups.adoc | 2 +- .../api/plugin/DirectoryServerPlugin.java | 7 +- .../plugins/ReferentialIntegrityPlugin.java | 10 ++- .../server/plugins/SambaPasswordPlugin.java | 6 +- .../org/opends/messages/plugin.properties | 5 +- .../core/PluginConfigManagerTestCase.java | 18 ++++- .../ReferentialIntegrityPluginTestCase.java | 77 +++++++++++++++++++ .../src/main/java/opendj/MsadPlugin.java | 21 +++++ 8 files changed, 137 insertions(+), 9 deletions(-) diff --git a/opendj-doc-generated-ref/src/main/asciidoc/server-dev-guide/chap-groups.adoc b/opendj-doc-generated-ref/src/main/asciidoc/server-dev-guide/chap-groups.adoc index e60b8751a8..1e7bdb3b80 100644 --- a/opendj-doc-generated-ref/src/main/asciidoc/server-dev-guide/chap-groups.adoc +++ b/opendj-doc-generated-ref/src/main/asciidoc/server-dev-guide/chap-groups.adoc @@ -539,6 +539,6 @@ member: uid=tmorris,ou=People,dc=example,dc=com ---- By default, the referential integrity plugin is configured to manage `member` and `uniqueMember` attributes. These attributes take values that are DNs, and are indexed for equality by default for the default backend. Before you add an additional attribute to manage, make sure that it has DN syntax and that it is indexed for equality. OpenDJ directory server requires that the attribute be indexed because an unindexed search for integrity would potentially consume too many of the server's resources. Attribute syntax is explained in xref:../admin-guide/chap-schema.adoc#chap-schema["Managing Schema"] in the __Administration Guide__. For instructions on indexing attributes, see xref:../admin-guide/chap-indexing.adoc#configure-indexes["Configuring and Rebuilding Indexes"] in the __Administration Guide__. -You can also configure the referential integrity plugin to check that new entries added to groups actually exist in the directory by setting the `check-references` property to `true`. You can specify additional criteria once you have activated the check. To ensure that entries added must match a filter, set the `check-references-filter-criteria` to identify the attribute and the filter. For example, you can specify that group members must be person entries by setting `check-references-filter-criteria` to `member:(objectclass=person)`. To ensure that entries must be located in the same naming context, set `check-references-scope-criteria` to `naming-context`. The check runs when entries are added and modified, so the plugin must be registered for the `preOperationAdd` and `preOperationModify` plugin types, as the default configuration is. When the plugin is enabled, OpenDJ refuses to set `check-references` to `true` if `plugin-type` lacks either of them, and it refuses to enable a plugin configured that way. A plugin already configured that way when the server starts is loaded with a warning, and it does not check references until the types are added. Plugin types take effect when the plugin is enabled, so add them before enabling the plugin, or disable and re-enable it afterwards. +You can also configure the referential integrity plugin to check that new entries added to groups actually exist in the directory by setting the `check-references` property to `true`. You can specify additional criteria once you have activated the check. To ensure that entries added must match a filter, set the `check-references-filter-criteria` to identify the attribute and the filter. For example, you can specify that group members must be person entries by setting `check-references-filter-criteria` to `member:(objectclass=person)`. To ensure that entries must be located in the same naming context, set `check-references-scope-criteria` to `naming-context`. The check runs when entries are added and modified, so the plugin must be registered for the `preOperationAdd` and `preOperationModify` plugin types, as the default configuration is. When the plugin is enabled, OpenDJ refuses to set `check-references` to `true` if `plugin-type` lacks either of them, and it refuses to enable a plugin configured that way. A plugin already configured that way when the server starts is loaded with a warning, and it does not check references until the types are added. Added plugin types take effect at once, and the plugin does not need to be disabled and enabled again. diff --git a/opendj-server-legacy/src/main/java/org/opends/server/api/plugin/DirectoryServerPlugin.java b/opendj-server-legacy/src/main/java/org/opends/server/api/plugin/DirectoryServerPlugin.java index cdc0b8b864..0c5f4daf2d 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/api/plugin/DirectoryServerPlugin.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/api/plugin/DirectoryServerPlugin.java @@ -13,6 +13,7 @@ * * Copyright 2006-2010 Sun Microsystems, Inc. * Portions Copyright 2014-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.opends.server.api.plugin; @@ -167,7 +168,11 @@ public abstract void initializePlugin(Set pluginTypes, /** * Performs any necessary finalization for this plugin. This will * be called just after the plugin has been deregistered with the - * server but before it has been unloaded. + * server but before it has been unloaded. It is also called when + * {@link #initializePlugin} throws, on an instance that was never + * registered: it must then release whatever + * {@code initializePlugin} acquired before it failed, and must not + * assume that it completed. */ public void finalizePlugin() { diff --git a/opendj-server-legacy/src/main/java/org/opends/server/plugins/ReferentialIntegrityPlugin.java b/opendj-server-legacy/src/main/java/org/opends/server/plugins/ReferentialIntegrityPlugin.java index f42d845141..34ac476f93 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/plugins/ReferentialIntegrityPlugin.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/plugins/ReferentialIntegrityPlugin.java @@ -170,7 +170,6 @@ public final void initializePlugin(Set pluginTypes, ReferentialIntegrityPluginCfg pluginCfg) throws ConfigException { - pluginCfg.addReferentialIntegrityChangeListener(this); LinkedList unacceptableReasons = new LinkedList<>(); if (!isConfigurationAcceptableIgnoringCheckReferencesPluginTypes(pluginCfg, unacceptableReasons)) @@ -178,6 +177,10 @@ public final void initializePlugin(Set pluginTypes, throw new ConfigException(unacceptableReasons.getFirst()); } + // Only after the check: finalizePlugin() cannot remove a listener registered by a refused + // configuration, because the configuration it removes it from is set in applyConfigurationChange(). + pluginCfg.addReferentialIntegrityChangeListener(this); + // Enabling the plugin or changing its configuration is refused without these types, but a configuration // already stored without them (the entry shipped before issue #1118) is loaded with a warning: refusing // it would also stop the delete and modify DN clean-up, which does not need them. @@ -928,6 +931,11 @@ public String getShutdownListenerName() { @Override public final void finalizePlugin() { + if (currentConfiguration == null) + { + // initializePlugin() refused the configuration before registering anything. + return; + } currentConfiguration.removeReferentialIntegrityChangeListener(this); if(interval > 0) { diff --git a/opendj-server-legacy/src/main/java/org/opends/server/plugins/SambaPasswordPlugin.java b/opendj-server-legacy/src/main/java/org/opends/server/plugins/SambaPasswordPlugin.java index 30c5d1bcde..1d9d92a57b 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/plugins/SambaPasswordPlugin.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/plugins/SambaPasswordPlugin.java @@ -868,7 +868,11 @@ public void initializePlugin(final Set pluginTypes, @Override public void finalizePlugin() { - config.removeSambaPasswordChangeListener(this); + // Null when initializePlugin() refused the configuration before registering the listener. + if (config != null) + { + config.removeSambaPasswordChangeListener(this); + } } diff --git a/opendj-server-legacy/src/messages/org/opends/messages/plugin.properties b/opendj-server-legacy/src/messages/org/opends/messages/plugin.properties index e114683693..d0aa7633dd 100644 --- a/opendj-server-legacy/src/messages/org/opends/messages/plugin.properties +++ b/opendj-server-legacy/src/messages/org/opends/messages/plugin.properties @@ -357,10 +357,9 @@ ERR_PLUGIN_REFERENT_EXCEPTION_129=The opration could not be processed \ ERR_PLUGIN_REFERENT_CHECK_REFERENCES_WITHOUT_PLUGIN_TYPE_131=The property \ 'check-references' is set to true, but the property 'plugin-type' does not list \ '%s', so the references added by that operation would not be checked. Add '%s' to \ - 'plugin-type', then disable and re-enable the plugin, or set 'check-references' to false + 'plugin-type', or set 'check-references' to false WARN_PLUGIN_REFERENT_CHECK_REFERENCES_WITHOUT_PLUGIN_TYPE_132=The Referential \ Integrity plugin %s has 'check-references' set to true, but its property \ 'plugin-type' does not list '%s', so the references added by that operation are \ not checked. The plugin is loaded and still removes the references to deleted and \ - renamed entries. Add '%s' to 'plugin-type', then disable and re-enable the plugin, \ - or set 'check-references' to false + renamed entries. Add '%s' to 'plugin-type', or set 'check-references' to false diff --git a/opendj-server-legacy/src/test/java/org/opends/server/core/PluginConfigManagerTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/core/PluginConfigManagerTestCase.java index 7b6efe3327..d72c376d9a 100644 --- a/opendj-server-legacy/src/test/java/org/opends/server/core/PluginConfigManagerTestCase.java +++ b/opendj-server-legacy/src/test/java/org/opends/server/core/PluginConfigManagerTestCase.java @@ -677,6 +677,14 @@ public void testPluginTypeChangeAppliesToEnabledPlugin() throws Exception assertEquals(countChangeListeners(pluginDN), changeListeners); assertEquals(modifyTestEntryAndCountPreOperation(), 1); + // A change that leaves the plugin types alone keeps the running instance. + assertEquals(setInvokeForInternalOperations(pluginDN, false), ResultCode.SUCCESS); + assertSame(getTrackingPlugin(pluginDN), replacement); + assertFalse(replacement.isFinalized()); + assertFalse(replacement.invokeForInternalOperations()); + assertEquals(setInvokeForInternalOperations(pluginDN, true), ResultCode.SUCCESS); + assertSame(getTrackingPlugin(pluginDN), replacement); + assertEquals(setPluginTypes(pluginDN, "postOperationModify"), ResultCode.SUCCESS); assertEquals(getTrackingPlugin(pluginDN).getPluginTypes(), EnumSet.of(POST_OPERATION_MODIFY)); @@ -721,8 +729,7 @@ public void testPluginTypeChangeRefusedByPluginKeepsRunningPlugin() throws Excep // The stored plugin types are still the refused ones, so the plugin is re-created again, and // refuses again, but the other properties are applied to the running instance. - resultCode = getRootConnection().processModify(newModifyRequest(pluginDN) - .addModification(REPLACE, "ds-cfg-invoke-for-internal-operations", "false")).getResultCode(); + resultCode = setInvokeForInternalOperations(pluginDN, false); assertNotEquals(resultCode, ResultCode.SUCCESS); assertSame(getTrackingPlugin(pluginDN), running); @@ -835,6 +842,13 @@ private static ResultCode setPluginTypes(DN pluginDN, String... pluginTypes) return getRootConnection().processModify(request).getResultCode(); } + private static ResultCode setInvokeForInternalOperations(DN pluginDN, boolean invoke) + { + ModifyRequest request = newModifyRequest(pluginDN) + .addModification(REPLACE, "ds-cfg-invoke-for-internal-operations", String.valueOf(invoke)); + return getRootConnection().processModify(request).getResultCode(); + } + /** * Modifies the test entry and returns how many times the tracking plugin was invoked before it. * The plugin always has the post-operation modify type, so it must be invoked once after it. diff --git a/opendj-server-legacy/src/test/java/org/opends/server/plugins/ReferentialIntegrityPluginTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/plugins/ReferentialIntegrityPluginTestCase.java index e80f130287..379b3ae15c 100644 --- a/opendj-server-legacy/src/test/java/org/opends/server/plugins/ReferentialIntegrityPluginTestCase.java +++ b/opendj-server-legacy/src/test/java/org/opends/server/plugins/ReferentialIntegrityPluginTestCase.java @@ -37,6 +37,7 @@ import org.opends.server.TestCaseUtils; import org.forgerock.opendj.server.config.meta.PluginCfgDefn.PluginType; import org.forgerock.opendj.server.config.meta.ReferentialIntegrityPluginCfgDefn; +import org.forgerock.opendj.server.config.server.ReferentialIntegrityPluginCfg; import org.opends.server.api.Group; import org.opends.server.controls.SubtreeDeleteControl; import org.opends.server.core.AddOperation; @@ -788,6 +789,82 @@ public void testInitializeWithInValidConfigs(Entry e) plugin.finalizePlugin(); } + /** + * Configurations that the plugin refuses in {@code initializePlugin()}: one in its configuration check, one when it + * sets up the log file. Their DN is not the one of the running plugin, so that an instance left listening does not + * take part in the changes the other cases make. + */ + @DataProvider(name = "configsRefusedByInitializePlugin") + public Object[][] createConfigsRefusedByInitializePlugin() throws Exception + { + List entries = TestCaseUtils.makeEntries( + "dn: cn=Refused Referential Integrity,cn=Plugins,cn=config", + "objectClass: top", + "objectClass: ds-cfg-plugin", + "objectClass: ds-cfg-referential-integrity-plugin", + "cn: Refused Referential Integrity", + "ds-cfg-java-class: org.opends.server.plugins.ReferentialIntegrityPlugin", + "ds-cfg-enabled: true", + "ds-cfg-plugin-type: postOperationDelete", + "ds-cfg-plugin-type: postOperationModifyDN", + "ds-cfg-plugin-type: subordinateModifyDN", + "ds-cfg-attribute-type: cn", + "", + "dn: cn=Refused Referential Integrity,cn=Plugins,cn=config", + "objectClass: top", + "objectClass: ds-cfg-plugin", + "objectClass: ds-cfg-referential-integrity-plugin", + "cn: Refused Referential Integrity", + "ds-cfg-java-class: org.opends.server.plugins.ReferentialIntegrityPlugin", + "ds-cfg-enabled: true", + "ds-cfg-plugin-type: postOperationDelete", + "ds-cfg-plugin-type: postOperationModifyDN", + "ds-cfg-plugin-type: subordinateModifyDN", + "ds-cfg-attribute-type: member", + "ds-cfg-update-interval: 300 seconds", + "ds-cfg-log-file: /hopefully/doesn't/file/exist"); + Object[][] array = new Object[entries.size()][]; + for (int i = 0; i < array.length; i++) + { + array[i] = new Object[] { entries.get(i) }; + } + return array; + } + + /** + * An instance whose {@code initializePlugin()} refused its configuration is finalized by the plugin manager, which + * never registers it. That must remove whatever change listener it registered, and must not fail. When the server + * starts there is no acceptance phase, so this is the only thing that stops a refused instance from listening to + * its configuration entry. + */ + @Test(dataProvider = "configsRefusedByInitializePlugin") + public void testConfigurationRefusedByInitializePluginLeavesNoChangeListenerBehind(Entry e) throws Exception + { + ReferentialIntegrityPluginCfg configuration = + InitializationUtils.getConfiguration(ReferentialIntegrityPluginCfgDefn.getInstance(), e); + int changeListeners = countChangeListeners(configuration.dn()); + + ReferentialIntegrityPlugin plugin = new ReferentialIntegrityPlugin(); + try + { + plugin.initializePlugin(TestCaseUtils.getPluginTypes(e), configuration); + fail("The plugin accepted a configuration it must refuse: " + e); + } + catch (ConfigException expected) + { + // As expected. + } + plugin.finalizePlugin(); + + assertEquals(countChangeListeners(configuration.dn()), changeListeners, + "The refused instance is still listening to its configuration entry"); + } + + private static int countChangeListeners(DN dn) + { + return TestCaseUtils.getServerContext().getConfigurationHandler().getChangeListeners(dn).size(); + } + /** * Issue #1118: configurations with {@code check-references} set to true that lack a pre-operation plugin type, * with the plugin types each one lacks. diff --git a/opendj-server-msad-plugin/src/main/java/opendj/MsadPlugin.java b/opendj-server-msad-plugin/src/main/java/opendj/MsadPlugin.java index d6264aef11..627fac6f28 100644 --- a/opendj-server-msad-plugin/src/main/java/opendj/MsadPlugin.java +++ b/opendj-server-msad-plugin/src/main/java/opendj/MsadPlugin.java @@ -1,3 +1,18 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2025-2026 3A Systems, LLC. + */ package opendj; import java.util.List; @@ -91,6 +106,12 @@ public void initializePlugin(Set pluginTypes, MsadPluginCfg config) logger.info(LocalizableMessage.raw("initialized MSAD plugin")); } + @Override + public void finalizePlugin() { + // Also called when initializePlugin() refused the plugin types after registering the listener. + config.removeMsadChangeListener(this); + } + @Override public PreOperation doPreOperation(PreOperationBindOperation bindOperation) { DN bindDN = bindOperation.getBindDN();