From 1c3cb3b2e53ca24a02782bf3057eae1536653198 Mon Sep 17 00:00:00 2001 From: Jamie Klassen Date: Mon, 29 Jan 2024 14:43:29 -0500 Subject: [PATCH] log warning on duplicate cluster names Signed-off-by: Jamie Klassen --- .changeset/tiny-donuts-drive.md | 6 ++ .../src/cluster-locator/index.test.ts | 57 +++++++++++++++++++ .../src/cluster-locator/index.ts | 30 ++++++++-- .../src/service/KubernetesBuilder.ts | 1 + 4 files changed, 90 insertions(+), 4 deletions(-) create mode 100644 .changeset/tiny-donuts-drive.md diff --git a/.changeset/tiny-donuts-drive.md b/.changeset/tiny-donuts-drive.md new file mode 100644 index 0000000000..aadac7a276 --- /dev/null +++ b/.changeset/tiny-donuts-drive.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-kubernetes-backend': patch +--- + +Backstage will log a warning whenever duplicate cluster names are detected -- +even if clusters sharing the same name come from separate locators. diff --git a/plugins/kubernetes-backend/src/cluster-locator/index.test.ts b/plugins/kubernetes-backend/src/cluster-locator/index.test.ts index beb9fc9cfe..fcdac32ecf 100644 --- a/plugins/kubernetes-backend/src/cluster-locator/index.test.ts +++ b/plugins/kubernetes-backend/src/cluster-locator/index.test.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { getVoidLogger } from '@backstage/backend-common'; import { Config, ConfigReader } from '@backstage/config'; import { CatalogApi } from '@backstage/catalog-client'; import { ANNOTATION_KUBERNETES_AUTH_PROVIDER } from '@backstage/plugin-kubernetes-common'; @@ -60,6 +61,7 @@ describe('getCombinedClusterSupplier', () => { config, catalogApi, mockStrategy, + getVoidLogger(), ); const result = await clusterSupplier.getClusters(); @@ -99,9 +101,64 @@ describe('getCombinedClusterSupplier', () => { config, catalogApi, new DispatchStrategy({ authStrategyMap: {} }), + getVoidLogger(), ), ).toThrow( new Error('Unsupported kubernetes.clusterLocatorMethods: "magic"'), ); }); + + it('logs a warning when two clusters have the same name', async () => { + const logger = getVoidLogger(); + const warn = jest.spyOn(logger, 'warn'); + const config: Config = new ConfigReader( + { + kubernetes: { + clusterLocatorMethods: [ + { + type: 'config', + clusters: [ + { name: 'cluster', url: 'url', authProvider: 'authProvider' }, + ], + }, + { type: 'catalog' }, + ], + }, + }, + 'ctx', + ); + const mockStrategy: jest.Mocked = { + getCredential: jest.fn(), + validateCluster: jest.fn().mockReturnValue([]), + presentAuthMetadata: jest.fn(), + }; + catalogApi = { + getEntities: jest.fn().mockResolvedValue({ + items: [{ metadata: { annotations: {}, name: 'cluster' } }], + }), + getEntitiesByRefs: jest.fn(), + queryEntities: jest.fn(), + getEntityAncestors: jest.fn(), + getEntityByRef: jest.fn(), + removeEntityByUid: jest.fn(), + refreshEntity: jest.fn(), + getEntityFacets: jest.fn(), + getLocationById: jest.fn(), + getLocationByRef: jest.fn(), + addLocation: jest.fn(), + removeLocationById: jest.fn(), + getLocationByEntity: jest.fn(), + validateEntity: jest.fn(), + }; + + const clusterSupplier = getCombinedClusterSupplier( + config, + catalogApi, + mockStrategy, + logger, + ); + await clusterSupplier.getClusters(); + + expect(warn).toHaveBeenCalledWith(`Duplicate cluster name 'cluster'`); + }); }); diff --git a/plugins/kubernetes-backend/src/cluster-locator/index.ts b/plugins/kubernetes-backend/src/cluster-locator/index.ts index 03a459b634..2dfd3178f7 100644 --- a/plugins/kubernetes-backend/src/cluster-locator/index.ts +++ b/plugins/kubernetes-backend/src/cluster-locator/index.ts @@ -14,21 +14,25 @@ * limitations under the License. */ +import { CatalogApi } from '@backstage/catalog-client'; import { Config } from '@backstage/config'; import { Duration } from 'luxon'; +import { Logger } from 'winston'; import { ClusterDetails, KubernetesClustersSupplier } from '../types/types'; import { AuthenticationStrategy } from '../auth/types'; import { ConfigClusterLocator } from './ConfigClusterLocator'; import { GkeClusterLocator } from './GkeClusterLocator'; import { CatalogClusterLocator } from './CatalogClusterLocator'; -import { CatalogApi } from '@backstage/catalog-client'; import { LocalKubectlProxyClusterLocator } from './LocalKubectlProxyLocator'; class CombinedClustersSupplier implements KubernetesClustersSupplier { - constructor(readonly clusterSuppliers: KubernetesClustersSupplier[]) {} + constructor( + readonly clusterSuppliers: KubernetesClustersSupplier[], + readonly logger: Logger, + ) {} async getClusters(): Promise { - return await Promise.all( + const clusters = await Promise.all( this.clusterSuppliers.map(supplier => supplier.getClusters()), ) .then(res => { @@ -37,6 +41,23 @@ class CombinedClustersSupplier implements KubernetesClustersSupplier { .catch(e => { throw e; }); + return this.warnDuplicates(clusters); + } + + private warnDuplicates(clusters: ClusterDetails[]): ClusterDetails[] { + const clusterNames = new Set(); + const duplicatedNames = new Set(); + for (const clusterName of clusters.map(c => c.name)) { + if (clusterNames.has(clusterName)) { + duplicatedNames.add(clusterName); + } else { + clusterNames.add(clusterName); + } + } + for (const clusterName of duplicatedNames) { + this.logger.warn(`Duplicate cluster name '${clusterName}'`); + } + return clusters; } } @@ -44,6 +65,7 @@ export const getCombinedClusterSupplier = ( rootConfig: Config, catalogClient: CatalogApi, authStrategy: AuthenticationStrategy, + logger: Logger, refreshInterval: Duration | undefined = undefined, ): KubernetesClustersSupplier => { const clusterSuppliers = rootConfig @@ -72,5 +94,5 @@ export const getCombinedClusterSupplier = ( } }); - return new CombinedClustersSupplier(clusterSuppliers); + return new CombinedClustersSupplier(clusterSuppliers, logger); }; diff --git a/plugins/kubernetes-backend/src/service/KubernetesBuilder.ts b/plugins/kubernetes-backend/src/service/KubernetesBuilder.ts index 81cc3d471d..8cc9b8a8b6 100644 --- a/plugins/kubernetes-backend/src/service/KubernetesBuilder.ts +++ b/plugins/kubernetes-backend/src/service/KubernetesBuilder.ts @@ -243,6 +243,7 @@ export class KubernetesBuilder { config, this.env.catalogApi, new DispatchStrategy({ authStrategyMap: this.getAuthStrategyMap() }), + this.env.logger, refreshInterval, );