fix(deployment): de-duplicate plugin config (#51)

gashcrumb · web-flow · commit d40c0b221d5e · 2026-03-11T14:10:35.000+01:00
Merge metadata + auth config by normalized plugin name so the same
plugin (OCI vs local path) is not listed twice. Add unit tests, docs,
and ESLint override for node:test describe/it.

Assisted-By: Cursor Desktop

rh-pre-commit.version: 2.3.2
rh-pre-commit.check-secrets: ENABLED
diff --git a/.github/workflows/pr-build-and-check.yml b/.github/workflows/pr-build-and-check.yml
@@ -29,3 +29,6 @@ jobs:
 
       - name: Build
         run: yarn build
+
+      - name: Run Tests
+        run: yarn test
diff --git a/README.md b/README.md
@@ -4,3 +4,10 @@ Documentation is available online:
 
 - Package documentation: https://redhat-developer.github.io/rhdh-e2e-test-utils/
 - Overlay testing documentation: https://redhat-developer.github.io/rhdh-e2e-test-utils/overlay/
+
+## Testing
+
+Unit tests use Node’s built-in test runner (`node:test`) and are discovered by pattern.
+
+- **Run tests:** `yarn test` (builds then runs all tests under `dist/` matching `**/*.test.js`).
+- **Add or change tests:** Add a `*.test.ts` file next to the code under `src/` (e.g. `src/utils/foo.test.ts`). It is compiled to `dist/` and picked up automatically; no need to update the test script.
diff --git a/docs/api/utils/plugin-metadata.md b/docs/api/utils/plugin-metadata.md
@@ -8,6 +8,7 @@ Utilities for loading and injecting plugin metadata from Package CRD files into
 import {
   shouldInjectPluginMetadata,
   extractPluginName,
+  getNormalizedPluginMergeKey,
   getMetadataDirectory,
   parseAllMetadataFiles,
   injectMetadataConfig,
@@ -77,6 +78,38 @@ const name = extractPluginName("oci://quay.io/rhdh/backstage-community-plugin-te
 
 ---
 
+### getNormalizedPluginMergeKey()
+
+Returns a stable merge key for a plugin entry so that OCI and local path for the same logical plugin match when merging dynamic-plugins configs. Strips a trailing `-dynamic` suffix so that e.g. `backstage-community-plugin-catalog-backend-module-keycloak-dynamic` (local) and `backstage-community-plugin-catalog-backend-module-keycloak` (from OCI) map to the same key.
+
+```typescript
+function getNormalizedPluginMergeKey(entry: { package?: string }): string
+```
+
+**Parameters:**
+| Parameter | Type | Description |
+|-----------|------|-------------|
+| `entry` | `{ package?: string }` | Plugin entry with optional package reference |
+
+**Returns:** Normalized key for merge deduplication, or empty string if `package` is missing.
+
+**Example:**
+
+```typescript
+// OCI and local path for the same plugin yield the same key
+getNormalizedPluginMergeKey({
+  package: "oci://ghcr.io/org/repo/backstage-community-plugin-catalog-backend-module-keycloak:tag!alias",
+});
+// Returns: "backstage-community-plugin-catalog-backend-module-keycloak"
+
+getNormalizedPluginMergeKey({
+  package: "./dynamic-plugins/dist/backstage-community-plugin-catalog-backend-module-keycloak-dynamic",
+});
+// Returns: "backstage-community-plugin-catalog-backend-module-keycloak"
+```
+
+---
+
 ### getMetadataDirectory()
 
 Gets the metadata directory path.
diff --git a/docs/changelog.md b/docs/changelog.md
@@ -2,6 +2,12 @@
 
 All notable changes to this project will be documented in this file.
 
+## [1.1.16] - Current
+
+### Fixed
+
+- **Duplicate plugin when no user `dynamic-plugins.yaml` (Keycloak auth, PR build)**: When the workspace had no `dynamic-plugins.yaml`, auto-generated config (with OCI URL) was merged with auth config (with local path). Because merge used exact `package` string match, the same plugin appeared twice and the backend failed with `ExtensionPoint with ID 'keycloak.transformer' is already registered`. The merge now uses a normalized plugin key so OCI and local path for the same logical plugin are deduplicated; the metadata-derived entry (e.g. OCI URL) wins.
+
 ## [1.1.15] - Current
 
 ### Added
diff --git a/docs/guide/configuration/config-files.md b/docs/guide/configuration/config-files.md
@@ -193,6 +193,9 @@ If your `dynamic-plugins.yaml` file doesn't exist, the package **auto-generates*
 1. Iterates through all metadata files in `../metadata/`
 2. Creates plugin entries with `disabled: false` (enabled)
 3. Uses `spec.appConfigExamples[0].content` as the plugin config
+4. Merges the result with package defaults and auth-specific plugins (e.g. Keycloak)
+
+Entries for the **same logical plugin** (e.g. keycloak from metadata and from auth) are **deduplicated**: the plugin appears once in the final config. When both metadata and auth list the same plugin, the metadata-derived entry wins (e.g. the OCI URL on PR builds is kept instead of the auth default local path).
 
 This is useful when you want to test all plugins with their default configurations without writing a `dynamic-plugins.yaml`.
 
diff --git a/docs/guide/utilities/plugin-metadata.md b/docs/guide/utilities/plugin-metadata.md
@@ -123,6 +123,8 @@ The `RHDHDeployment` class automatically uses these utilities during `deploy()`:
 
 2. If `dynamic-plugins.yaml` doesn't exist:
    - Auto-generates from all metadata files
+   - Merges with package defaults and auth-specific plugins (e.g. Keycloak)
+   - Deduplicates by normalized plugin name so the same logical plugin appears once (metadata/OCI wins)
    - All plugins enabled with default configurations
 
 See [Configuration Files](/guide/configuration/config-files#plugin-metadata-injection) for detailed behavior.
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@red-hat-developer-hub/e2e-test-utils",
-  "version": "1.1.15",
+  "version": "1.1.16",
   "description": "Test utilities for RHDH E2E tests",
   "license": "Apache-2.0",
   "repository": {
@@ -66,6 +66,7 @@
     "prepublishOnly": "yarn build",
     "prettier:check": "prettier --check . '!dist' '!README.md' '!docs' '!.github/workflows/deploy-docs.yml'",
     "prettier:fix": "prettier --write . '!dist' '!README.md' '!docs' '!.github/workflows/deploy-docs.yml'",
+    "test": "node --test \"dist/**/*.test.js\"",
     "typecheck": "tsc --noEmit"
   },
   "keywords": [
@@ -75,7 +76,7 @@
     "test-utils"
   ],
   "engines": {
-    "node": ">=22",
+    "node": ">=22.18.0",
     "yarn": ">=3"
   },
   "peerDependencies": {
diff --git a/src/deployment/rhdh/deployment.test.ts b/src/deployment/rhdh/deployment.test.ts
@@ -0,0 +1,52 @@
+import { describe, it } from "node:test";
+import assert from "node:assert";
+import { deepMerge } from "../../utils/merge-yamls.js";
+import { getNormalizedPluginMergeKey } from "../../utils/plugin-metadata.js";
+
+/**
+ * Tests the merge behavior used when user dynamic-plugins config does not exist:
+ * auth config (e.g. keycloak) is merged with metadata config using normalized plugin key.
+ * Result must have exactly one entry per logical plugin; metadata (source) wins so OCI URL is kept.
+ */
+describe("dynamic-plugins merge (no user config path)", () => {
+  it("yields one keycloak plugin with OCI package when auth has local path and metadata has OCI", () => {
+    const authPlugins: Record<string, unknown> = {
+      plugins: [
+        {
+          package:
+            "./dynamic-plugins/dist/backstage-community-plugin-catalog-backend-module-keycloak-dynamic",
+          disabled: false,
+          pluginConfig: {},
+        },
+      ],
+      includes: ["dynamic-plugins.default.yaml"],
+    };
+    const metadataConfig: Record<string, unknown> = {
+      plugins: [
+        {
+          package:
+            "oci://ghcr.io/redhat-developer/rhdh-plugin-export-overlays/backstage-community-plugin-catalog-backend-module-keycloak:pr_1980__3.16.0!backstage-community-plugin-catalog-backend-module-keycloak",
+          disabled: false,
+          pluginConfig: { catalog: { providers: { keycloakOrg: {} } } },
+        },
+      ],
+    };
+    const merged = deepMerge(authPlugins, metadataConfig, {
+      arrayMergeStrategy: {
+        byKey: "package",
+        normalizeKey: (item) =>
+          getNormalizedPluginMergeKey(item as Record<string, unknown>),
+      },
+    });
+    const plugins = merged.plugins as Array<{ package?: string }>;
+    assert.strictEqual(
+      plugins.length,
+      1,
+      "merged config must have exactly one keycloak plugin",
+    );
+    assert.ok(
+      plugins[0].package?.startsWith("oci://"),
+      "metadata (OCI) must win over auth local path",
+    );
+  });
+});
diff --git a/src/deployment/rhdh/deployment.ts b/src/deployment/rhdh/deployment.ts
@@ -6,6 +6,7 @@ import { mergeYamlFilesIfExists, deepMerge } from "../../utils/merge-yamls.js";
 import {
   loadAndInjectPluginMetadata,
   generateDynamicPluginsConfigFromMetadata,
+  getNormalizedPluginMergeKey,
 } from "../../utils/plugin-metadata.js";
 import { envsubst } from "../../utils/common.js";
 import { runOnce } from "../../playwright/run-once.js";
@@ -128,13 +129,19 @@ export class RHDHDeployment {
       );
       const metadataConfig = await generateDynamicPluginsConfigFromMetadata();
 
-      // Merge with package defaults and auth config
+      // Merge with package defaults and auth config. Use normalized plugin key so
+      // the same logical plugin (e.g. keycloak from metadata OCI + auth local path)
+      // is deduplicated; metadata (source) wins so OCI URL is kept on PR builds.
       const authPlugins = await mergeYamlFilesIfExists(
         [DEFAULT_CONFIG_PATHS.dynamicPlugins, authConfig.dynamicPlugins],
         { arrayMergeStrategy: { byKey: "package" } },
       );
-      return deepMerge(metadataConfig, authPlugins, {
-        arrayMergeStrategy: { byKey: "package" },
+      return deepMerge(authPlugins, metadataConfig, {
+        arrayMergeStrategy: {
+          byKey: "package",
+          normalizeKey: (item) =>
+            getNormalizedPluginMergeKey(item as Record<string, unknown>),
+        },
       });
     }
 
diff --git a/src/eslint/base.config.ts b/src/eslint/base.config.ts
@@ -214,5 +214,12 @@ export function createEslintConfig(tsconfigRootDir: string): Linter.Config[] {
         "check-file/filename-naming-convention": "off",
       },
     },
+    // Node test runner (*.test.ts) - describe/it return promises the runner handles
+    {
+      files: ["**/*.test.ts"],
+      rules: {
+        "@typescript-eslint/no-floating-promises": "off",
+      },
+    },
   ] as Linter.Config[];
 }
diff --git a/src/utils/merge-yamls.test.ts b/src/utils/merge-yamls.test.ts
@@ -0,0 +1,72 @@
+import { describe, it } from "node:test";
+import assert from "node:assert";
+import { deepMerge } from "./merge-yamls.js";
+import { getNormalizedPluginMergeKey } from "./plugin-metadata.js";
+
+describe("deepMerge with arrayMergeStrategy byKey", () => {
+  it("keeps two plugin entries when package values differ and no normalizeKey", () => {
+    const target: Record<string, unknown> = {
+      plugins: [
+        {
+          package:
+            "oci://ghcr.io/org/repo/backstage-community-plugin-catalog-backend-module-keycloak:tag!alias",
+          disabled: false,
+        },
+      ],
+    };
+    const source: Record<string, unknown> = {
+      plugins: [
+        {
+          package:
+            "./dynamic-plugins/dist/backstage-community-plugin-catalog-backend-module-keycloak-dynamic",
+          disabled: false,
+        },
+      ],
+    };
+    const result = deepMerge(target, source, {
+      arrayMergeStrategy: { byKey: "package" },
+    });
+    const plugins = result.plugins as unknown[];
+    assert.strictEqual(
+      plugins.length,
+      2,
+      "without normalizeKey both entries are kept",
+    );
+  });
+
+  it("merges into one plugin when normalizeKey maps both to same key and source wins", () => {
+    const target: Record<string, unknown> = {
+      plugins: [
+        {
+          package:
+            "./dynamic-plugins/dist/backstage-community-plugin-catalog-backend-module-keycloak-dynamic",
+          disabled: false,
+        },
+      ],
+    };
+    const source: Record<string, unknown> = {
+      plugins: [
+        {
+          package:
+            "oci://ghcr.io/org/repo/backstage-community-plugin-catalog-backend-module-keycloak:pr_1__1.0!keycloak",
+          disabled: false,
+        },
+      ],
+    };
+    const normalizeKey = (item: unknown) =>
+      getNormalizedPluginMergeKey(item as Record<string, unknown>);
+    const result = deepMerge(target, source, {
+      arrayMergeStrategy: { byKey: "package", normalizeKey },
+    });
+    const plugins = result.plugins as Array<{ package?: string }>;
+    assert.strictEqual(
+      plugins.length,
+      1,
+      "same normalized key yields one entry",
+    );
+    assert.ok(
+      plugins[0].package?.startsWith("oci://"),
+      "source (OCI) wins over target (local path)",
+    );
+  });
+});
diff --git a/src/utils/merge-yamls.ts b/src/utils/merge-yamls.ts
@@ -8,7 +8,11 @@ import mergeWith from "lodash.mergewith";
 export type ArrayMergeStrategy =
   | "replace" // Replace arrays entirely (default, Kustomize-style)
   | "concat" // Concatenate arrays
-  | { byKey: string }; // Merge arrays of objects by a specific key
+  | {
+      byKey: string;
+      /** Optional: normalize key for matching so different values (e.g. OCI vs local path) map to the same entry. Source wins when merging. */
+      normalizeKey?: (item: unknown) => string;
+    };
 
 /**
  * Options for YAML merging.
@@ -19,45 +23,61 @@ export interface MergeOptions {
    * - "replace": Replace arrays entirely (default)
    * - "concat": Concatenate arrays
    * - { byKey: "keyName" }: Merge arrays of objects by a specific key
+   * - { byKey: "keyName", normalizeKey }: Same, but match by normalized key (e.g. for plugin deduplication)
    */
   arrayMergeStrategy?: ArrayMergeStrategy;
 }
 
 /**
- * Merges two arrays of objects by a specific key.
- * Objects with matching keys are deeply merged, new objects are appended.
+ * Returns the merge key for an item: normalized if normalizeKey is provided, else raw key value.
+ * Returns null if the item is not an object or has no key (and no normalizer is provided).
+ */
+function getMergeKey(
+  item: unknown,
+  key: string,
+  normalizeKey?: (item: unknown) => string,
+): string | null {
+  if (typeof item !== "object" || item === null) {
+    return null;
+  }
+  if (normalizeKey) {
+    return normalizeKey(item);
+  }
+  if (key in (item as Record<string, unknown>)) {
+    return String((item as Record<string, unknown>)[key]);
+  }
+  return null;
+}
+
+/**
+ * Merges two arrays of objects by a specific key (optionally normalized).
+ * Objects with matching keys are deeply merged, new objects are appended. Source wins.
  */
 function mergeArraysByKey(
   target: unknown[],
   source: unknown[],
-  key: string,
+  keyStrategy: { byKey: string; normalizeKey?: (item: unknown) => string },
   mergeOptions: MergeOptions,
 ): unknown[] {
+  const { byKey: key, normalizeKey } = keyStrategy;
   const result = [...target];
 
   for (const srcItem of source) {
-    if (typeof srcItem === "object" && srcItem !== null && key in srcItem) {
-      const srcKeyValue = (srcItem as Record<string, unknown>)[key];
-      const existingIndex = result.findIndex(
-        (item) =>
-          typeof item === "object" &&
-          item !== null &&
-          (item as Record<string, unknown>)[key] === srcKeyValue,
+    const srcKeyValue = getMergeKey(srcItem, key, normalizeKey);
+    if (srcKeyValue === null) {
+      result.push(srcItem);
+      continue;
+    }
+    const existingIndex = result.findIndex(
+      (item) => getMergeKey(item, key, normalizeKey) === srcKeyValue,
+    );
+    if (existingIndex !== -1) {
+      result[existingIndex] = deepMerge(
+        result[existingIndex] as Record<string, unknown>,
+        srcItem as Record<string, unknown>,
+        mergeOptions,
       );
-
-      if (existingIndex !== -1) {
-        // Merge existing object with source object
-        result[existingIndex] = deepMerge(
-          result[existingIndex] as Record<string, unknown>,
-          srcItem as Record<string, unknown>,
-          mergeOptions,
-        );
-      } else {
-        // Append new object
-        result.push(srcItem);
-      }
     } else {
-      // Non-object or missing key, append as-is
       result.push(srcItem);
     }
   }
@@ -86,7 +106,7 @@ export function deepMerge(
         } else if (strategy === "concat") {
           return [...objValue, ...srcValue];
         } else if (typeof strategy === "object" && "byKey" in strategy) {
-          return mergeArraysByKey(objValue, srcValue, strategy.byKey, options);
+          return mergeArraysByKey(objValue, srcValue, strategy, options);
         }
       }
     },
diff --git a/src/utils/plugin-metadata.test.ts b/src/utils/plugin-metadata.test.ts
diff --git a/src/utils/plugin-metadata.ts b/src/utils/plugin-metadata.ts
