feat(extensions): enforce collision policy for duplicate entity identities

[hopehadfield]

Hey, I just made a Pull Request!

• Add collision handling for extensions catalog entities in the backend provider by keying on normalized kind/namespace/name.
• Keep the first definition when duplicates are equivalent, and fail fast when duplicates with the same identity have conflicting content.
• Add targeted unit tests covering equivalent duplicates, conflicting duplicates, and same-name entities across different namespaces.

This change is being made as a result of RHIDP-12376. Multi-catalog ingestion can introduce duplicate entity identities; this change makes behaviour deterministic and prevents silent shadowing of conflicting definitions.

Manually validated runtime behavior in three scenarios:

1. equivalent duplicate -> warning + continue
2. conflicting duplicate -> provider error
3. same name with different namespace -> both accepted

✔️ Checklist

• A changeset describing the change and affected packages. (more info)
• Added or Updated documentation
• Tests for new functionality and regression tests for bug fixes
• Screenshots attached (for UI changes)

How to Test

Create a directory /tmp/extensions-test, with subdirectories primary/catalog-entities/plugins and extra/community/catalog-entities/plugins

For case A (equivalent duplicate), create an identical plugin yaml file on each path, for example:
/tmp/extensions-collision-test/primary/catalog-entities/plugins/github.yaml and /tmp/extensions-collision-test/extra/community/catalog-entities/plugins/github.yaml both containing

apiVersion: extensions.backstage.io/v1alpha1
kind: Plugin
metadata:
  name: github
spec:
  owner: redhat

After running yarn start-backend you should see in the logs that the duplicate entity was skipped, with the first definition being kept. The backend continues successfully.

For case B (conflicting duplicate), update the spec.owner in /tmp/extensions-collision-test/extra/community/catalog-entities/plugins/github.yaml to community and restart the backend. You should see that the provider fails with a conflicting entities message surfaced in the logs.

[rhdh-qodo-merge]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (2)

1. Strict equality false conflicts 🐞 Bug ≡ Correctness

Description

BaseEntityProvider.getEntities keys collisions using a normalized identity (namespace defaults to
"default"), but treats duplicates as equivalent only when the raw parsed entities are
deep-strict-equal. This can throw a conflict for duplicates that represent the same identity but
differ only by harmless representation differences (e.g., one explicitly sets `metadata.namespace:
default` while the other omits it), stopping ingestion unnecessarily.

Code

workspaces/extensions/plugins/catalog-backend-module-extensions/src/providers/BaseEntityProvider.ts[R87-112]

+    for (const fileData of allEntities) {
+      if (fileData.content.kind !== this.getKind()) {
+        continue;
+      }
+
+      const identity = this.getEntityIdentity(fileData.content);
+      const existing = entitiesByIdentity.get(identity);
+      if (!existing) {
+        entitiesByIdentity.set(identity, {
+          entity: fileData.content,
+          filePath: fileData.filePath,
+        });
+        continue;
+      }
+
+      if (isDeepStrictEqual(existing.entity, fileData.content)) {
+        console.warn(
+          `Skipping duplicate Extensions entity '${identity}' from '${fileData.filePath}'. Keeping first definition from '${existing.filePath}'.`,
+        );
+        continue;
+      }
+
+      throw new Error(
+        `Conflicting Extensions entities detected for '${identity}' in '${existing.filePath}' and '${fileData.filePath}'.`,
+      );
+    }

Relevance

⭐⭐⭐ High

Likely correctness issue: normalized identity can collide but deepStrictEqual may falsely mark as
conflicting.

PR-#2671

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Identity normalization collapses missing namespace to "default", meaning entities with and without
an explicit default namespace collide on the same identity key; however, the duplicate-equivalence
check compares the raw YAML-loaded objects, so those representations are not considered equal and
will fall into the conflict throw path. Additionally, provider-managed annotations are overwritten
later, but still influence the deep-equality check if present in the YAML.

workspaces/extensions/plugins/catalog-backend-module-extensions/src/providers/BaseEntityProvider.ts[54-61]
workspaces/extensions/plugins/catalog-backend-module-extensions/src/providers/BaseEntityProvider.ts[63-75]
workspaces/extensions/plugins/catalog-backend-module-extensions/src/providers/BaseEntityProvider.ts[87-112]
workspaces/extensions/plugins/catalog-backend-module-extensions/src/utils/file-utils.ts[56-67]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`getEntities` uses a normalized identity (defaulting namespace to `default`) but uses `isDeepStrictEqual` on the raw parsed entities to decide whether duplicates are equivalent. This can incorrectly throw conflicts for duplicates that should be treated as equivalent under the provider’s own identity normalization (e.g., omitted vs explicit `metadata.namespace: default`, or differences only in annotations that the provider overwrites anyway).

### Issue Context
Entities are loaded via `js-yaml` without normalization. The provider itself normalizes namespace for identity, and later overwrites `ANNOTATION_LOCATION`/`ANNOTATION_ORIGIN_LOCATION`.

### Fix Focus Areas
- workspaces/extensions/plugins/catalog-backend-module-extensions/src/providers/BaseEntityProvider.ts[54-116]
- workspaces/extensions/plugins/catalog-backend-module-extensions/src/utils/file-utils.ts[56-67]

### Suggested fix
1. Introduce a small canonicalization function used only for duplicate comparison, e.g.:
  - Ensure `metadata.namespace` is set to `'default'` when missing.
  - Remove/ignore provider-managed annotation keys (`ANNOTATION_LOCATION`, `ANNOTATION_ORIGIN_LOCATION`) from the comparison payload.
  - Optionally normalize empty objects (e.g., missing `annotations` vs `{}`) if you consider them equivalent.
2. Compare canonicalized copies with `isDeepStrictEqual`.
3. Add a unit test for the specific false-conflict case (one entity omits namespace, the other sets `namespace: default`) and ensure it warns + keeps the first definition instead of throwing.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Console logging bypasses logger 🐞 Bug ◔ Observability

Description

Duplicate warnings (and related provider warnings) are emitted via console.warn instead of
Backstage’s LoggerService, making logs harder to configure, aggregate, and correlate in
production. The module already wires a logger for other components, but providers don’t use it.

Code

workspaces/extensions/plugins/catalog-backend-module-extensions/src/providers/BaseEntityProvider.ts[R102-105]

+      if (isDeepStrictEqual(existing.entity, fileData.content)) {
+        console.warn(
+          `Skipping duplicate Extensions entity '${identity}' from '${fileData.filePath}'. Keeping first definition from '${existing.filePath}'.`,
+        );

Relevance

⭐⭐ Medium

Mixed history: console log removal accepted, but switching to Backstage logger was rejected before.

PR-#2597
PR-#2512

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The provider logs duplicate handling via console.warn. Elsewhere in this module, the Backstage
logger service is part of init deps and is used by processors, indicating a consistent structured
logging approach is expected/available.

workspaces/extensions/plugins/catalog-backend-module-extensions/src/providers/BaseEntityProvider.ts[102-106]
workspaces/extensions/plugins/catalog-backend-module-extensions/src/module.ts[37-80]
workspaces/extensions/plugins/catalog-backend-module-extensions/src/processors/PluginInstallStatusProcessor.ts[137-152]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`BaseEntityProvider` emits important operational signals (duplicate collisions, config fallback warnings) through `console.warn`/`console.error`. In Backstage backends, this bypasses the configured `LoggerService`, reducing consistency and operational control.

### Issue Context
`catalogModuleExtensions` already receives `logger: coreServices.logger` during init, and other processors use `LoggerService`.

### Fix Focus Areas
- workspaces/extensions/plugins/catalog-backend-module-extensions/src/providers/BaseEntityProvider.ts[36-220]
- workspaces/extensions/plugins/catalog-backend-module-extensions/src/module.ts[37-103]
- workspaces/extensions/plugins/catalog-backend-module-extensions/src/providers/BaseEntityProvider.test.ts[1-125]

### Suggested fix
1. Add an optional `logger: LoggerService` dependency to `BaseEntityProvider` (and constructors of concrete providers), defaulting to a minimal fallback if not provided.
2. Replace `console.warn`/`console.error` in `BaseEntityProvider` with `this.logger.warn`/`this.logger.error`.
3. Update `module.ts` to pass the module `logger` into each provider.
4. Update the unit test to spy on the injected mock logger (or keep a fallback path but prefer testing structured logging).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. getEntities() collision policy undocumented 📎 Requirement gap ⚙ Maintainability

Description

This PR introduces deterministic collision handling for duplicate entity identities (skip
equivalent, throw on conflicts) but does not add documentation describing expected multi-source
behavior, collision scenarios, or namespace implications. This violates the checklist requirements
to document multi-source implications, collision behavior, and single-source backwards compatibility
considerations.

Code

workspaces/extensions/plugins/catalog-backend-module-extensions/src/providers/BaseEntityProvider.ts[R77-116]

  getEntities(allEntities: JsonFileData<T>[]): T[] {
    if (allEntities.length === 0) {
      return [];
    }
-    return allEntities
-      .filter(d => d.content.kind === this.getKind())
-      .map(file => ({
-        ...file.content,
-        metadata: {
-          ...file.content.metadata,
-          annotations: {
-            ...file.content.metadata.annotations,
-            [ANNOTATION_LOCATION]: `file:${this.getProviderName()}`,
-            [ANNOTATION_ORIGIN_LOCATION]: `file:${this.getProviderName()}`,
-          },
-        },
-      }));
+
+    const entitiesByIdentity = new Map<
+      string,
+      { entity: T; filePath: string }
+    >();
+
+    for (const fileData of allEntities) {
+      if (fileData.content.kind !== this.getKind()) {
+        continue;
+      }
+
+      const identity = this.getEntityIdentity(fileData.content);
+      const existing = entitiesByIdentity.get(identity);
+      if (!existing) {
+        entitiesByIdentity.set(identity, {
+          entity: fileData.content,
+          filePath: fileData.filePath,
+        });
+        continue;
+      }
+
+      if (isDeepStrictEqual(existing.entity, fileData.content)) {
+        console.warn(
+          `Skipping duplicate Extensions entity '${identity}' from '${fileData.filePath}'. Keeping first definition from '${existing.filePath}'.`,
+        );
+        continue;
+      }
+
+      throw new Error(
+        `Conflicting Extensions entities detected for '${identity}' in '${existing.filePath}' and '${fileData.filePath}'.`,
+      );
+    }
+
+    return Array.from(entitiesByIdentity.values()).map(({ entity }) =>
+      this.addProviderAnnotations(entity),
+    );

Relevance

⭐ Low

Historical pattern: reviewers often reject requests to document behavior/compatibility unless
required.

PR-#2711
PR-#2712

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new getEntities() implementation enforces a collision policy based on normalized
kind/namespace/name, including warning on equivalent duplicates and throwing on conflicting
duplicates, but the PR only adds a changeset entry and no documentation updates describing
multi-source collision behavior or compatibility impact.

Document implications of multi-source plugin catalogs for extensions backend module
Assess and document entity identity collision behavior across multiple sources
Confirm backwards compatibility impact for single-source deployments
workspaces/extensions/plugins/catalog-backend-module-extensions/src/providers/BaseEntityProvider.ts[77-116]
workspaces/extensions/.changeset/clean-news-itch.md[1-5]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR changes runtime behavior for entity identity collisions (normalized `kind/namespace/name`) but does not document expected multi-source behavior, the collision policy, or backwards compatibility considerations.

## Issue Context
`BaseEntityProvider.getEntities()` now de-duplicates equivalent entities and fails fast on conflicting duplicates, which is important operational behavior for multi-catalog ingestion.

## Fix Focus Areas
- workspaces/extensions/plugins/catalog-backend-module-extensions/src/providers/BaseEntityProvider.ts[77-116]
- workspaces/extensions/plugins/catalog-backend-module-extensions/README.md[38-61]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. ANNOTATION_ORIGIN_LOCATION strategy undocumented 📎 Requirement gap ⚙ Maintainability

Description

This PR adds provider origin annotations (ANNOTATION_LOCATION and ANNOTATION_ORIGIN_LOCATION)
but does not document or recommend a source-identification strategy for filtering/troubleshooting.
This violates the requirement to evaluate and document a strategy for origin/source identification.

Code

workspaces/extensions/plugins/catalog-backend-module-extensions/src/providers/BaseEntityProvider.ts[R63-75]

+  private addProviderAnnotations(entity: T): T {
+    return {
+      ...entity,
+      metadata: {
+        ...entity.metadata,
+        annotations: {
+          ...entity.metadata.annotations,
+          [ANNOTATION_LOCATION]: `file:${this.getProviderName()}`,
+          [ANNOTATION_ORIGIN_LOCATION]: `file:${this.getProviderName()}`,
+        },
+      },
+    };
+  }

Relevance

⭐ Low

Similar “add/update docs” review suggestions often rejected; not enforced for extensions module
changes.

PR-#2711
PR-#2712

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The PR sets origin-related annotations to file:${this.getProviderName()} for all entities, which
is a source identification mechanism, but provides no accompanying documentation/recommendation
describing what the origin represents and how it should be used.

Evaluate and recommend a strategy for source identification (origin annotation) on entities
workspaces/extensions/plugins/catalog-backend-module-extensions/src/providers/BaseEntityProvider.ts[63-72]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Origin/source identification is implemented via entity annotations, but the PR does not document what the origin value means or recommend how it should be used for filtering/troubleshooting.

## Issue Context
`BaseEntityProvider.addProviderAnnotations()` sets `ANNOTATION_LOCATION` and `ANNOTATION_ORIGIN_LOCATION` to `file:${getProviderName()}` for ingested entities.

## Fix Focus Areas
- workspaces/extensions/plugins/catalog-backend-module-extensions/src/providers/BaseEntityProvider.ts[63-72]
- workspaces/extensions/plugins/catalog-backend-module-extensions/README.md[90-139]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[rhdh-gh-app]

Important

This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior.

Changed Packages

Package Name	Package Path	Changeset Bump	Current Version
@red-hat-developer-hub/backstage-plugin-catalog-backend-module-extensions	workspaces/extensions/plugins/catalog-backend-module-extensions	minor	v0.16.0

[rhdh-qodo-merge]

Review Summary by Qodo

Enforce collision policy for duplicate entity identities

✨ Enhancement 🧪 Tests

Walkthroughs

Description

• Enforce collision policy for duplicate entity identities in extensions catalog
• Keep first definition when duplicates are equivalent, warn user
• Fail fast with error when duplicates have conflicting content
• Add comprehensive unit tests for collision scenarios

Diagram

flowchart LR
  A["Entity Processing"] --> B["Normalize Identity"]
  B --> C["Check for Duplicates"]
  C --> D{Duplicate Found?}
  D -->|No| E["Add to Map"]
  D -->|Yes| F{Content Equal?}
  F -->|Yes| G["Warn & Skip"]
  F -->|No| H["Throw Error"]
  E --> I["Apply Annotations"]
  G --> I
  I --> J["Return Entities"]

Loading

File Changes

1. workspaces/extensions/plugins/catalog-backend-module-extensions/src/providers/BaseEntityProvider.ts ✨ Enhancement +60/-13

Implement collision detection and policy enforcement

• Added getEntityIdentity() method to normalize entity identity by kind, namespace, and name
• Refactored getEntities() to detect and handle duplicate entities using a Map
• Implements collision policy: warn on equivalent duplicates, throw error on conflicting duplicates
• Extracted annotation logic into separate addProviderAnnotations() method
• Added import for isDeepStrictEqual utility for deep equality comparison

workspaces/extensions/plugins/catalog-backend-module-extensions/src/providers/BaseEntityProvider.ts

---

2. workspaces/extensions/plugins/catalog-backend-module-extensions/src/providers/BaseEntityProvider.test.ts 🧪 Tests +125/-0

Add collision policy unit tests

• Added comprehensive test suite for collision policy with three test cases
• Test for equivalent duplicates: verifies warning is logged and only first definition kept
• Test for conflicting duplicates: verifies error is thrown with descriptive message
• Test for same-name entities with different namespaces: verifies both are accepted
• Uses test helper functions to create entities and file data with flexible overrides

workspaces/extensions/plugins/catalog-backend-module-extensions/src/providers/BaseEntityProvider.test.ts

---

3. workspaces/extensions/.changeset/clean-news-itch.md 📝 Documentation +5/-0

Add changeset for collision policy feature

• Created changeset documenting the collision policy enforcement feature
• Marked as minor version bump for the catalog backend module extensions package

workspaces/extensions/.changeset/clean-news-itch.md

---

[debsmita1]

@hopehadfield Could you add some examples to test your PR ?

[hopehadfield]

@debsmita1 I included a "How to Test" section in the PR, please let me know if you have any questions

[dzemanov]

THank you @hopehadfield for these changes, I left some small comments.

[dzemanov]

Can you use logger.warn?

[hopehadfield]

Do you want me to also migrate the existing console.warn/error calls in this provider in the same PR?

[dzemanov]

Would be good to update it now if you have time

[hopehadfield]

Thanks for the review @dzemanov! I've addressed your comments, please let me know if there's anything else required before merging

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[codecov]

⚠️ JUnit XML file not found

The CLI was unable to find any JUnit XML files to upload.
For more help, visit our troubleshooting guide.

[dzemanov]

Should this be filename instead?

[dzemanov]

Oh, I see it being used the same way here, so it's correct.

[dzemanov]

Tested that this works fine, thank you @hopehadfield!

Tested with:

• duplicate entities
• conflicting entities

setup:

extensions:
  directory: /tmp/extensions-test

I see this optional configuration is not in config.d.ts, would be good to mention it there 😅. I can create a small following PR or we can add it in this PR.

It would be good to fix this test entity invalid name before merging this PR (I noticed it now thanks to catalog warn log).

If you have time, there are some sonar issues flagged that would be good to fix.

Otherwise, good to go!