Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions workspaces/extensions/.changeset/clean-news-itch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@red-hat-developer-hub/backstage-plugin-catalog-backend-module-extensions': minor
---

Enforce collision policy for duplicate entity identities
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ apiVersion: extensions.backstage.io/v1alpha1
kind: Plugin
metadata:
namespace: extensions-plugin-demo
name: certified-plugin-1-by-vendor-a
name: certified-plugin-2-by-vendor-a
title: Certified Plugin 2 by Vendor A
description: This is a certified plugin example
annotations:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,14 @@ extensions:
directory: /path/to/custom/extensions
```

### Collision behavior

When multiple YAML sources define the same entity identity (`kind:namespace/name`), the provider handles collisions as follows:

- If definitions are equivalent, it keeps the first definition and logs a warning.
- If definitions conflict, it logs a warning and skips the conflicting definition.
- Entities with the same `kind`/`name` but different namespaces are treated as distinct entities and are both ingested.

## Plugin configuration YAML Guide:

This YAML file is used to add extensions plugin to the Software catalog in your backstage application.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,11 @@ import { SchedulerServiceTaskRunner } from '@backstage/backend-plugin-api';
export abstract class BaseEntityProvider<T extends Entity>
implements EntityProvider
{
constructor(taskRunner: SchedulerServiceTaskRunner, config?: Config);
constructor(
taskRunner: SchedulerServiceTaskRunner,
config?: Config,
logger?: LoggerService,
);
// (undocumented)
connect(connection: EntityProviderConnection): Promise<void>;
// (undocumented)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,10 @@ export const catalogModuleExtensions = createBackendModule({
const catalogApi = new CatalogClient({ discoveryApi: discovery });

catalog.addEntityProvider(
new ExtensionsPackageProvider(taskRunner, config),
new ExtensionsPackageProvider(taskRunner, config, logger),
);
catalog.addEntityProvider(
new ExtensionsPluginProvider(delayedTaskRunner, config),
new ExtensionsPluginProvider(delayedTaskRunner, config, logger),
);
// Disabling the collection provider as collections/all.yaml is already commented in RHDH 1.5 image.
// catalog.addEntityProvider(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
/*
* Copyright Red Hat, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { Entity } from '@backstage/catalog-model';
import {
LoggerService,
SchedulerServiceTaskRunner,
} from '@backstage/backend-plugin-api';
import { BaseEntityProvider } from './BaseEntityProvider';
import { JsonFileData } from '../types';

class TestEntityProvider extends BaseEntityProvider<Entity> {
getProviderName(): string {
return 'test-entity-provider';
}

getKind(): string {
return 'Plugin';
}
}

const taskRunner: SchedulerServiceTaskRunner = {
run: jest.fn(async ({ fn }) => fn(new AbortController().signal)),
};
const logger: LoggerService = {
warn: jest.fn(),
error: jest.fn(),
info: jest.fn(),
debug: jest.fn(),
child: jest.fn(),
};

const createEntity = (overrides?: Partial<Entity>): Entity => ({
apiVersion: 'extensions.backstage.io/v1alpha1',
kind: 'Plugin',
metadata: {
name: 'duplicate-plugin',
...overrides?.metadata,
},
spec: {
owner: 'test-owner',
...(overrides?.spec as object),
},
...overrides,
});

const createFileData = (
filePath: string,
entity: Entity,
): JsonFileData<Entity> => ({
filePath,
content: entity,
});

describe('BaseEntityProvider collision policy', () => {
beforeEach(() => {
jest.clearAllMocks();
});

afterEach(() => {
jest.restoreAllMocks();
});

it('keeps first definition when duplicate entities are equivalent', () => {
const provider = new TestEntityProvider(taskRunner, undefined, logger);
const duplicate = createEntity();

const entities = provider.getEntities([
createFileData('/extensions/primary/plugin.yaml', duplicate),
createFileData('/extensions/extra/community/plugin.yaml', duplicate),
]);

expect(entities).toHaveLength(1);
expect(logger.warn).toHaveBeenCalledWith(
expect.stringContaining(
"Skipping duplicate Extensions entity 'plugin:default/duplicate-plugin'",
),
);
});

it('warns and skips when duplicate entities have conflicting definitions', () => {
const provider = new TestEntityProvider(taskRunner, undefined, logger);
const firstEntity = createEntity({
spec: { owner: 'owner-a' },
});
const secondEntity = createEntity({
spec: { owner: 'owner-b' },
});

const entities = provider.getEntities([
createFileData('/extensions/primary/plugin.yaml', firstEntity),
createFileData('/extensions/extra/community/plugin.yaml', secondEntity),
]);

expect(entities).toHaveLength(1);
expect(logger.warn).toHaveBeenCalledWith(
expect.stringContaining(
"Conflicting Extensions entities detected for 'plugin:default/duplicate-plugin'",
),
);
});

it('keeps entities with same name when namespaces differ', () => {
const provider = new TestEntityProvider(taskRunner, undefined, logger);
const defaultNamespaceEntity = createEntity({
metadata: { name: 'shared-name' },
});
const customNamespaceEntity = createEntity({
metadata: { name: 'shared-name', namespace: 'community' },
});

const entities = provider.getEntities([
createFileData(
'/extensions/primary/plugin-default.yaml',
defaultNamespaceEntity,
),
createFileData(
'/extensions/extra/community/plugin-custom.yaml',
customNamespaceEntity,
),
]);

expect(entities).toHaveLength(2);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,15 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { SchedulerServiceTaskRunner } from '@backstage/backend-plugin-api';
import {
LoggerService,
SchedulerServiceTaskRunner,
} from '@backstage/backend-plugin-api';
import {
ANNOTATION_LOCATION,
ANNOTATION_ORIGIN_LOCATION,
Entity,
stringifyEntityRef,
} from '@backstage/catalog-model';
import {
EntityProvider,
Expand All @@ -28,6 +32,7 @@ import { readYamlFiles } from '../utils/file-utils';
import { JsonFileData } from '../types';
import path from 'path';
import fs from 'fs';
import { isDeepStrictEqual } from 'node:util';

/**
* @public
Expand All @@ -38,35 +43,82 @@ export abstract class BaseEntityProvider<T extends Entity>
private connection?: EntityProviderConnection;
private taskRunner: SchedulerServiceTaskRunner;
private config?: Config;
private readonly logger?: LoggerService;

private static readonly EXTENSIONS_DIRECTORY = '/extensions';
private static readonly DEPRECATED_MARKETPLACE_DIRECTORY = '/marketplace';

constructor(taskRunner: SchedulerServiceTaskRunner, config?: Config) {
constructor(
taskRunner: SchedulerServiceTaskRunner,
config?: Config,
logger?: LoggerService,
) {
this.taskRunner = taskRunner;
this.config = config;
this.logger = logger;
}

abstract getProviderName(): string;
abstract getKind(): string;

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()}`,
},
Comment on lines +69 to +73

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be filename instead?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

},
};
}

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 entitiesByEntityRef = new Map<
string,
{ entity: T; filePath: string }
>();

for (const fileData of allEntities) {
if (fileData.content.kind !== this.getKind()) {
continue;
}

const identity = stringifyEntityRef({
kind: fileData.content.kind,
namespace: fileData.content.metadata.namespace ?? 'default',
name: fileData.content.metadata.name,
}).toLocaleLowerCase('en-US');
const existing = entitiesByEntityRef.get(identity);
if (!existing) {
entitiesByEntityRef.set(identity, {
entity: fileData.content,
filePath: fileData.filePath,
});
continue;
}

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

this.logger?.warn(
`Conflicting Extensions entities detected for '${identity}' in '${existing.filePath}' and '${fileData.filePath}'. Skipping conflicting definition from '${fileData.filePath}'.`,
);
}

return Array.from(entitiesByEntityRef.values()).map(({ entity }) =>
this.addProviderAnnotations(entity),
);
}

async connect(connection: EntityProviderConnection): Promise<void> {
Expand Down Expand Up @@ -119,7 +171,7 @@ export abstract class BaseEntityProvider<T extends Entity>
}
}
} catch (error) {
console.warn(
this.logger?.warn(
'Failed to read extensions directory from config, falling back to hardcoded fallbacks',
error,
);
Expand All @@ -139,7 +191,7 @@ export abstract class BaseEntityProvider<T extends Entity>
}
}

console.warn(
this.logger?.warn(
`Extensions directory not found. Checked: configured directory "${BaseEntityProvider.EXTENSIONS_DIRECTORY}" and "${BaseEntityProvider.DEPRECATED_MARKETPLACE_DIRECTORY}"`,
);
return null;
Expand All @@ -157,7 +209,7 @@ export abstract class BaseEntityProvider<T extends Entity>
try {
yamlData = readYamlFiles(extensionsFilePath);
} catch (error) {
console.error(error.message);
this.logger?.error(error.message);
}
}

Expand Down
Loading