[#1118] Register the shipped Referential Integrity plugin for the pre-operation types check-references needs, and refuse check-references without them

[vharseko]

Summary

The shipped cn=Referential Integrity,cn=Plugins,cn=config entry registered the plugin for postOperationDelete, postOperationModifyDN, subordinateModifyDN and subordinateDelete only. check-references runs in the plugin's preOperationAdd and preOperationModify hooks, so on the shipped entry check-references:true was accepted and checked nothing (the behaviour reported in #579).

Changes

• config.ldif: the shipped entry lists preOperationAdd and preOperationModify, as the definition's default for plugin-type already does.
• ReferentialIntegrityPlugin.isConfigurationAcceptable: refuses check-references:true when plugin-type lacks either type, with one ERR_PLUGIN_REFERENT_CHECK_REFERENCES_WITHOUT_PLUGIN_TYPE reason per missing type. The plugin manager runs this check when the plugin is enabled, and when its configuration is changed while it is enabled. The message says to disable and re-enable the plugin after adding the types, because of Changing plugin-type on an enabled plugin has no effect until the plugin is re-enabled, and the server does not report it #1120.
• ReferentialIntegrityPlugin.initializePlugin: a configuration already stored without the types is still loaded when the server starts, with one WARN_PLUGIN_REFERENT_CHECK_REFERENCES_WITHOUT_PLUGIN_TYPE warning per missing type. Refusing it there would also stop the delete and modify DN clean-up, which does not need these types and ran before this change. The upgrade task heals every 5.1.x instance. The warning covers the rest: an instance already on a 5.2.0 snapshot, and a hand-edited or restored config.ldif.
• Upgrade task keyed at 5.2.0: adds both types to every entry with objectClass=ds-cfg-referential-integrity-plugin, so instances upgraded from earlier versions are healed and do not start failing the new check. The add is permissive, so it is idempotent.
• UpgradeUtils.getUpgradeSchema(): declares ds-cfg-plugin-type with caseIgnoreMatch. Without it the upgrade schema matches the attribute case-exactly, and the task would add preOperationAdd next to a preoperationadd an administrator already added with dsconfig, which writes plugin types in lower case.
• Developer guide, "Configuring Referential Integrity": check-references needs the two plugin types. The refusal applies to an enabled plugin, and to enabling one. A plugin already configured without the types when the server starts is loaded with a warning. Plugin types take effect when the plugin is enabled.

Tests

• ReferentialIntegrityPluginTestCase
  • checkReferencesWithoutPreOperationTypes: three configurations, the pre-fix shipped types and each pre-operation type missing on its own, each with the types it lacks;
  • testCheckReferencesWithoutPreOperationTypesIsNotAcceptable: each one is refused, with exactly one reason per missing type;
  • testCheckReferencesWithoutPreOperationTypesIsLoadedWithWarning: each one still initializes, and a warning is logged for each missing type;
  • testShippedEntryAcceptsCheckReferences: the entry read from the fresh-install config.ldif initializes with check-references:true;
  • testCheckReferencesWithoutPreOperationTypesIsRejected: turning check-references on for the running plugin without the types is refused, and the reason names both types;
  • testEnablingCheckReferencesWithoutPreOperationTypesIsRejected: on a disabled plugin check-references:true without the types is accepted, and enabling the plugin is then refused, with both types named.
• UpgradeUtilsTestCase
  • testAddReferentialIntegrityPluginTypesMirrorsFreshInstallTemplate: applied twice to the pre-fix entry, and to one where preoperationadd was already added in lower case, the task leaves exactly the plugin types of the fresh-install template;
  • testAddReferentialIntegrityPluginTypesTaskRunsOnUpgradeTo520: Upgrade.getUpgradeTasks (now package-private) from 5.1.2 to 5.2.0 includes the task.

Before the fix the new cases failed; in the lower-case case the upgrade produced 7 plugin types instead of 6. After it: 58/58 and 5/5. Each of these mutants is caught: no check, a check for preOperationAdd only, a case-exact upgrade schema, a refusal at startup, no startup warning, no refusal in isConfigurationAcceptable, the register call removed, and the task registered at 5.3.0.

Related

• Using OpenDJ version 5.0.1, after deleting a user using ldapmodify when I tried to add that deleted user to a group it allows without throwing the error #579: the same problem reported on 5.0.1.
• Changing plugin-type on an enabled plugin has no effect until the plugin is re-enabled, and the server does not report it #1120: changing plugin-type on a plugin that is already enabled has no effect until it is re-enabled, and the server does not report it. Not addressed here; [#1120] Apply a change to plugin-type on an enabled plugin without disabling and enabling it again #1127 fixes it. Whichever of [#1118] Register the shipped Referential Integrity plugin for the pre-operation types check-references needs, and refuse check-references without them #1123 and [#1120] Apply a change to plugin-type on an enabled plugin without disabling and enabling it again #1127 is merged second drops the "disable and re-enable" advice from the refusal message and the developer guide.

Fixes #1118

[maximthomas]

praise: The fix is made where the bug is, and the upgrade path covers the case-sensitivity trap.

• resource/config/config.ldif: the shipped cn=Referential Integrity entry now lists preOperationAdd and preOperationModify, matching the definition's default. ReferentialIntegrityPlugin.isConfigurationAcceptable gives one ERR_PLUGIN_REFERENT_CHECK_REFERENCES_WITHOUT_PLUGIN_TYPE reason for each missing type.
• UpgradeUtils.getUpgradeSchema() declares ds-cfg-plugin-type with caseIgnoreMatch, so the 5.2.0 task does not add a second preOperationAdd next to the preoperationadd that dsconfig writes. UpgradeUtilsTestCase row 2 pins this.
• Run locally at f4f2fc9: ReferentialIntegrityPluginTestCase 54/54 and UpgradeUtilsTestCase 4/4. All new cases are green.

---

question (non-blocking): Is it intended that at boot, an enabled RI plugin with check-references: true and no pre-operation types is no longer loaded at all?

opendj-server-legacy/src/main/java/org/opends/server/plugins/ReferentialIntegrityPlugin.java:176, :285-296

initializePlugin:176 runs the new check and throws ConfigException. PluginConfigManager.initializeUserPlugins logs the error and continues (:349-352), so the post-operation delete/modifyDN and subordinate cleanup of member/uniqueMember also stops. At BASE that cleanup ran and only the check was inert. The 5.2.0 task heals every 5.1.x → 5.2.0 upgrade. It does not heal an instance already on a 5.2.0-SNAPSHOT build: getUpgradeTasks uses TASKS.subMap(from, false, to, true) and the version compare ignores the revision. A hand-edited or restored config.ldif is not healed either. If unloading is intended, this matches how other invalid RI configs already fail at boot, and nothing needs to change. If it is not intended, the change would log the check-references reasons as warnings in initializePlugin and load the plugin, and keep the refusal for configuration changes.

---

suggestion (non-blocking): No test runs the register("5.2.0", …) call. The test applies only the task's constants.

opendj-server-legacy/src/test/java/org/opends/server/tools/upgrade/UpgradeUtilsTestCase.java:141-145, opendj-server-legacy/src/main/java/org/opends/server/tools/upgrade/Upgrade.java:652-655

testAddReferentialIntegrityPluginTypesMirrorsFreshInstallTemplate passes REFERENTIAL_INTEGRITY_PLUGIN_FILTER and ADD_REFERENTIAL_INTEGRITY_PRE_OPERATION_PLUGIN_TYPES directly to UpgradeUtils.updateConfigFile, and no test reads TASKS or getUpgradeTasks. Deleting the register call, or keying it at a version a 5.1.x → 5.2.0 upgrade never reaches, keeps every test green. Upgraded instances would then keep the four old types, and the new check would refuse check-references: true. The registration at the head is correct by reading. Only the pin is missing.

// Upgrade.java:782 — package-private for the test
static List<UpgradeTask> getUpgradeTasks(final BuildVersion fromVersion, final BuildVersion toVersion)

// UpgradeUtilsTestCase
@Test
public void testAddReferentialIntegrityPluginTypesTaskRunsOnUpgradeTo520() throws Exception
{
  final List<String> summaries = new ArrayList<>();
  for (final UpgradeTask task : Upgrade.getUpgradeTasks(BuildVersion.valueOf("5.1.2"), BuildVersion.valueOf("5.2.0")))
  {
    summaries.add(task.toString());
  }
  assertTrue(summaries.contains(INFO_UPGRADE_TASK_ADD_REFERENTIAL_INTEGRITY_PRE_OPERATION_PLUGIN_TYPES.get().toString()),
      summaries.toString());
}

Pin: this turns red when the register call is deleted or re-versioned. updateConfigEntry's task returns its summary from toString() (UpgradeTasks.java:1159-1161). A different filter literal inside the call would still go uncaught.

---

issue (non-blocking): The guide says OpenDJ refuses the set itself. On a disabled plugin, which is how the entry ships, the refusal comes only when the plugin is enabled.

opendj-doc-generated-ref/src/main/asciidoc/server-dev-guide/chap-groups.adoc:542

PluginConfigManager calls the plugin's isConfigurationAcceptable only if (configuration.isEnabled()) (:4357, :4439). cn=Referential Integrity ships with ds-cfg-enabled: false. On it, set-plugin-prop --set check-references:true is accepted even when a type is missing, and --set enabled:true is what gets refused. The misconfiguration still cannot become active, so only the sentence is wrong.

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.

---

suggestion (non-blocking): The refusal's remedy does not work on a running plugin that was registered without the types.

opendj-server-legacy/src/messages/org/opends/messages/plugin.properties:357-360, opendj-server-legacy/src/main/java/org/opends/server/plugins/ReferentialIntegrityPlugin.java:294

Take a plugin that was enabled with the four old types. An admin follows the message: adds preoperationadd and preoperationmodify, then sets check-references: true. Every change is accepted. But PluginConfigManager.applyConfigurationChange does not re-register plugin types for an existing plugin, so adds and modifies are still not checked until the plugin is re-enabled. That is the #1118 symptom again (the root is #1120). The guide says to re-enable; the message does not.

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

[vharseko]

@maximthomas all four points are taken in 8701e0c. The branch is rebased onto master d0ce1d7.

question: unloading at boot: this was not intended. Refusing the configuration in initializePlugin also stopped the delete and modify DN clean-up, which does not need the pre-operation types and ran at BASE. The check is now split out of isConfigurationAcceptable:

• isConfigurationAcceptable still refuses check-references:true without the types. The plugin manager calls it on a new instance with initialize=false before it enables the plugin or applies a change, so enabling and changing are still refused.
• initializePlugin runs the other checks as before. For each missing type it logs WARN_PLUGIN_REFERENT_CHECK_REFERENCES_WITHOUT_PLUGIN_TYPE_132 and then loads the plugin. The warning names the plugin DN and the type. It says the clean-up still runs and how to fix the configuration.

The three configurations moved from invalidConfigs to a checkReferencesWithoutPreOperationTypes provider that also gives the types each one lacks. Two tests use it:

• testCheckReferencesWithoutPreOperationTypesIsNotAcceptable: isConfigurationAcceptable returns false with exactly one reason per missing type;
• testCheckReferencesWithoutPreOperationTypesIsLoadedWithWarning: initializePlugin succeeds and logs the warning for each missing type.

suggestion: pin for register("5.2.0", …): Upgrade.getUpgradeTasks is package-private now, and testAddReferentialIntegrityPluginTypesTaskRunsOnUpgradeTo520 is your snippet. It is red with the register call removed and with the task registered at 5.3.0.

issue: the guide sentence: replaced with your wording. The guide also says that a plugin already configured that way at startup is loaded with a warning. testEnablingCheckReferencesWithoutPreOperationTypesIsRejected pins the sentence: on the disabled plugin, check-references:true without the types is accepted, and enabled:true is then refused with both types named.

suggestion: the refusal's remedy: _131 now says "then disable and re-enable the plugin", as the guide does. The root cause is fixed in #1127 (#1120). Whichever of #1123 and #1127 is merged second drops the re-enable advice from both the message and the guide.

Run locally at 8701e0c: ReferentialIntegrityPluginTestCase 58/58 and UpgradeUtilsTestCase 5/5. Five mutants are each caught by the new tests:

• refusing at startup;
• no warning;
• no refusal in isConfigurationAcceptable;
• no register call;
• register("5.3.0").

[maximthomas]

praise: The round-1 points are fixed where they arise, and the boot road keeps the clean-up.

• ReferentialIntegrityPlugin.initializePlugin now runs isConfigurationAcceptableIgnoringCheckReferencesPluginTypes and logs one WARN_PLUGIN_REFERENT_CHECK_REFERENCES_WITHOUT_PLUGIN_TYPE per missing type (:176-187). A stored pre-#1118 entry therefore keeps its delete and modify DN clean-up.
• testAddReferentialIntegrityPluginTypesTaskRunsOnUpgradeTo520 pins the 5.2.0 registration through the now package-private Upgrade.getUpgradeTasks.
• Run locally at 8701e0c: ReferentialIntegrityPluginTestCase 58/58 and UpgradeUtilsTestCase 5/5. Every new case is green.