diff --git a/.github/workflows/pr-build-and-check.yml b/.github/workflows/pr-build-and-check.yml index 80ce4fc..d4a6a02 100644 --- 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 index 6d01e31..beb32e6 100644 --- 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 index 44cc421..2d2e084 100644 --- 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 index f7a970d..8c3ae72 100644 --- 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 index 46d9cff..ca5c61f 100644 --- 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 index 01a8544..5f06f95 100644 --- 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 index c9c4ab0..df1d155 100644 --- 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 new file mode 100644 index 0000000..19b1521 --- /dev/null +++ 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 = { + plugins: [ + { + package: + "./dynamic-plugins/dist/backstage-community-plugin-catalog-backend-module-keycloak-dynamic", + disabled: false, + pluginConfig: {}, + }, + ], + includes: ["dynamic-plugins.default.yaml"], + }; + const metadataConfig: Record = { + 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), + }, + }); + 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 index 1d66f5c..12f45e0 100644 --- 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), + }, }); } diff --git a/src/eslint/base.config.ts b/src/eslint/base.config.ts index 331cfc6..683a136 100644 --- 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 new file mode 100644 index 0000000..72d9ca9 --- /dev/null +++ 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 = { + plugins: [ + { + package: + "oci://ghcr.io/org/repo/backstage-community-plugin-catalog-backend-module-keycloak:tag!alias", + disabled: false, + }, + ], + }; + const source: Record = { + 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 = { + plugins: [ + { + package: + "./dynamic-plugins/dist/backstage-community-plugin-catalog-backend-module-keycloak-dynamic", + disabled: false, + }, + ], + }; + const source: Record = { + 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); + 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 index 322f345..3556070 100644 --- 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)) { + return String((item as Record)[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)[key]; - const existingIndex = result.findIndex( - (item) => - typeof item === "object" && - item !== null && - (item as Record)[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, + srcItem as Record, + mergeOptions, ); - - if (existingIndex !== -1) { - // Merge existing object with source object - result[existingIndex] = deepMerge( - result[existingIndex] as Record, - srcItem as Record, - 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 new file mode 100644 index 0000000..196f904 --- /dev/null +++ b/src/utils/plugin-metadata.test.ts @@ -0,0 +1,64 @@ +import { describe, it } from "node:test"; +import assert from "node:assert"; +import { + extractPluginName, + getNormalizedPluginMergeKey, +} from "./plugin-metadata.js"; + +describe("getNormalizedPluginMergeKey", () => { + it("returns same key for OCI keycloak and local keycloak-dynamic (same logical plugin)", () => { + const oci = getNormalizedPluginMergeKey({ + 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", + }); + const local = getNormalizedPluginMergeKey({ + package: + "./dynamic-plugins/dist/backstage-community-plugin-catalog-backend-module-keycloak-dynamic", + }); + assert.strictEqual(oci, local, "same logical plugin has same merge key"); + assert.strictEqual( + oci, + "backstage-community-plugin-catalog-backend-module-keycloak", + ); + }); + + it("returns different keys for different plugins", () => { + const keycloak = getNormalizedPluginMergeKey({ + package: + "./dynamic-plugins/dist/backstage-community-plugin-catalog-backend-module-keycloak-dynamic", + }); + const techRadar = getNormalizedPluginMergeKey({ + package: + "./dynamic-plugins/dist/backstage-community-plugin-tech-radar-dynamic", + }); + assert.notStrictEqual(keycloak, techRadar); + }); + + it("returns empty string for missing or empty package", () => { + assert.strictEqual(getNormalizedPluginMergeKey({}), ""); + assert.strictEqual(getNormalizedPluginMergeKey({ package: "" }), ""); + assert.strictEqual(getNormalizedPluginMergeKey({ package: undefined }), ""); + }); +}); + +describe("extractPluginName", () => { + it("extracts name from OCI URL with tag and alias", () => { + const name = extractPluginName( + "oci://ghcr.io/org/repo/backstage-community-plugin-catalog-backend-module-keycloak:pr_1__1.0!alias", + ); + assert.strictEqual( + name, + "backstage-community-plugin-catalog-backend-module-keycloak", + ); + }); + + it("extracts name from local path with -dynamic suffix", () => { + const name = extractPluginName( + "./dynamic-plugins/dist/backstage-community-plugin-catalog-backend-module-keycloak-dynamic", + ); + assert.strictEqual( + name, + "backstage-community-plugin-catalog-backend-module-keycloak-dynamic", + ); + }); +}); diff --git a/src/utils/plugin-metadata.ts b/src/utils/plugin-metadata.ts index be5c8a1..997b5d7 100644 --- a/src/utils/plugin-metadata.ts +++ b/src/utils/plugin-metadata.ts @@ -203,6 +203,25 @@ export function extractPluginName(packageRef: string): string { return match?.[1] || packageRef; } +/** + * Returns a stable merge key for a plugin entry so OCI and local path for the same + * logical plugin match when merging dynamic-plugins configs. Strips a trailing + * "-dynamic" so e.g. backstage-community-plugin-catalog-backend-module-keycloak-dynamic + * and ...-keycloak (from OCI) map to the same key. + * + * @param entry Plugin entry with optional package reference + * @returns Normalized key for merge deduplication, or empty string if package is missing + */ +export function getNormalizedPluginMergeKey(entry: { + package?: string; +}): string { + const pkg = entry?.package; + if (pkg === undefined || pkg === "") { + return ""; + } + return extractPluginName(pkg).replace(/-dynamic$/, ""); +} + /** * Default metadata directory path relative to the e2e-tests directory. * Follows the same pattern as user config paths (e.g., tests/config/dynamic-plugins.yaml).