From efc0d893cd04315de7493155c53a1471879fd835 Mon Sep 17 00:00:00 2001 From: Jamie Klassen Date: Thu, 7 Sep 2023 09:53:12 -0400 Subject: [PATCH] rearrange clusterlocator tests and remove default authStrategy parameter Signed-off-by: Jamie Klassen --- .../src/auth/OidcStrategy.test.ts | 97 ++++++----- .../ConfigClusterLocator.test.ts | 159 +++++++----------- .../cluster-locator/ConfigClusterLocator.ts | 24 +-- 3 files changed, 117 insertions(+), 163 deletions(-) diff --git a/plugins/kubernetes-backend/src/auth/OidcStrategy.test.ts b/plugins/kubernetes-backend/src/auth/OidcStrategy.test.ts index 5650e0eccd..7843cc6544 100644 --- a/plugins/kubernetes-backend/src/auth/OidcStrategy.test.ts +++ b/plugins/kubernetes-backend/src/auth/OidcStrategy.test.ts @@ -15,47 +15,18 @@ */ import { ANNOTATION_KUBERNETES_OIDC_TOKEN_PROVIDER } from '@backstage/plugin-kubernetes-common'; +import { AuthMetadata } from '../types/types'; import { OidcStrategy } from './OidcStrategy'; -describe('OidcStrategy tests', () => { - const strategy = new OidcStrategy(); - - it('returns cluster details with auth token', async () => { - const details = await strategy.decorateClusterDetailsWithAuth( - { - name: 'test', - url: '', - authMetadata: { - authProvider: 'oidc', - [ANNOTATION_KUBERNETES_OIDC_TOKEN_PROVIDER]: 'okta', - }, - }, - { - oidc: { okta: 'fakeToken' }, - }, - ); - - expect(details.authMetadata.serviceAccountToken).toBe('fakeToken'); +describe('OidcStrategy', () => { + let strategy: OidcStrategy; + beforeEach(() => { + strategy = new OidcStrategy(); }); - it('returns error when oidcTokenProvider is not configured', async () => { - await expect( - strategy.decorateClusterDetailsWithAuth( - { - name: 'test', - url: '', - authMetadata: { authProvider: 'oidc' }, - }, - {}, - ), - ).rejects.toThrow( - 'oidc authProvider requires a configured oidcTokenProvider', - ); - }); - - it('returns error when token is not included in request body', async () => { - await expect( - strategy.decorateClusterDetailsWithAuth( + describe('decorateClusterDetailsWithAuth', () => { + it('returns cluster details with auth token', async () => { + const details = await strategy.decorateClusterDetailsWithAuth( { name: 'test', url: '', @@ -64,8 +35,54 @@ describe('OidcStrategy tests', () => { [ANNOTATION_KUBERNETES_OIDC_TOKEN_PROVIDER]: 'okta', }, }, - {}, - ), - ).rejects.toThrow('Auth token not found under oidc.okta in request body'); + { + oidc: { okta: 'fakeToken' }, + }, + ); + + expect(details.authMetadata.serviceAccountToken).toBe('fakeToken'); + }); + + it('fails when oidcTokenProvider is not configured', async () => { + await expect( + strategy.decorateClusterDetailsWithAuth( + { + name: 'test', + url: '', + authMetadata: { authProvider: 'oidc' }, + }, + {}, + ), + ).rejects.toThrow( + 'oidc authProvider requires a configured oidcTokenProvider', + ); + }); + + it('fails when token is not included in request body', async () => { + await expect( + strategy.decorateClusterDetailsWithAuth( + { + name: 'test', + url: '', + authMetadata: { + authProvider: 'oidc', + [ANNOTATION_KUBERNETES_OIDC_TOKEN_PROVIDER]: 'okta', + }, + }, + {}, + ), + ).rejects.toThrow('Auth token not found under oidc.okta in request body'); + }); + }); + + describe('validate', () => { + it('fails when token provider is not specified', () => { + const authMetadata: AuthMetadata = { + authProvider: 'oidc', + }; + expect(() => strategy.validate(authMetadata)).toThrow( + `Must specify a token provider for 'oidc' strategy`, + ); + }); }); }); diff --git a/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.test.ts b/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.test.ts index 58e21909f7..8c0d565f96 100644 --- a/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.test.ts +++ b/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.test.ts @@ -22,14 +22,24 @@ import { } from '@backstage/plugin-kubernetes-common'; import { ConfigClusterLocator } from './ConfigClusterLocator'; import { ClusterDetails } from '../types/types'; +import { AuthenticationStrategy } from '../auth'; describe('ConfigClusterLocator', () => { + let authStrategy: jest.Mocked; + + beforeEach(() => { + authStrategy = { + decorateClusterDetailsWithAuth: jest.fn(), + validate: jest.fn(), + }; + }); + it('empty clusters returns empty cluster details', async () => { const config: Config = new ConfigReader({ clusters: [], }); - const sut = ConfigClusterLocator.fromConfig(config); + const sut = ConfigClusterLocator.fromConfig(config, authStrategy); const result = await sut.getClusters(); @@ -47,7 +57,7 @@ describe('ConfigClusterLocator', () => { ], }); - const sut = ConfigClusterLocator.fromConfig(config); + const sut = ConfigClusterLocator.fromConfig(config, authStrategy); const result = await sut.getClusters(); @@ -86,7 +96,7 @@ describe('ConfigClusterLocator', () => { ], }); - const sut = ConfigClusterLocator.fromConfig(config); + const sut = ConfigClusterLocator.fromConfig(config, authStrategy); const result = await sut.getClusters(); @@ -116,75 +126,50 @@ describe('ConfigClusterLocator', () => { ]); }); - it('one aws cluster with assumeRole and one without', async () => { + it('copies "assumeRole" config value into metadata', async () => { const config: Config = new ConfigReader({ clusters: [ { name: 'cluster1', - serviceAccountToken: 'token', url: 'http://localhost:8080', authProvider: 'aws', - skipTLSVerify: false, - }, - { assumeRole: 'SomeRole', - name: 'cluster2', - url: 'http://localhost:8081', - authProvider: 'aws', - skipTLSVerify: true, - }, - { - assumeRole: 'SomeRole', - name: 'cluster2', - externalId: 'SomeExternalId', - url: 'http://localhost:8081', - authProvider: 'aws', - skipTLSVerify: true, }, ], }); - const sut = ConfigClusterLocator.fromConfig(config); - + const sut = ConfigClusterLocator.fromConfig(config, authStrategy); const result = await sut.getClusters(); - expect(result).toStrictEqual([ + expect(result).toMatchObject([ { - name: 'cluster1', - url: 'http://localhost:8080', - authMetadata: { - authProvider: 'aws', - serviceAccountToken: 'token', - }, - skipTLSVerify: false, - skipMetricsLookup: false, - caData: undefined, - caFile: undefined, - }, - { - name: 'cluster2', - url: 'http://localhost:8081', - authMetadata: { - authProvider: 'aws', + authMetadata: expect.objectContaining({ [ANNOTATION_KUBERNETES_AWS_ASSUME_ROLE]: 'SomeRole', - }, - skipTLSVerify: true, - skipMetricsLookup: false, - caData: undefined, - caFile: undefined, + }), }, - { - name: 'cluster2', - url: 'http://localhost:8081', - authMetadata: { + ]); + }); + + it('copies "externalId" config value into metadata', async () => { + const config: Config = new ConfigReader({ + clusters: [ + { + name: 'cluster1', + url: 'http://localhost:8080', authProvider: 'aws', - [ANNOTATION_KUBERNETES_AWS_ASSUME_ROLE]: 'SomeRole', + externalId: 'SomeExternalId', + }, + ], + }); + + const sut = ConfigClusterLocator.fromConfig(config, authStrategy); + const result = await sut.getClusters(); + + expect(result).toMatchObject([ + { + authMetadata: expect.objectContaining({ [ANNOTATION_KUBERNETES_AWS_EXTERNAL_ID]: 'SomeExternalId', - }, - skipTLSVerify: true, - skipMetricsLookup: false, - caData: undefined, - caFile: undefined, + }), }, ]); }); @@ -206,7 +191,7 @@ describe('ConfigClusterLocator', () => { ], }); - const sut = ConfigClusterLocator.fromConfig(config); + const sut = ConfigClusterLocator.fromConfig(config, authStrategy); const result = await sut.getClusters(); @@ -242,7 +227,7 @@ describe('ConfigClusterLocator', () => { ], }); - const sut = ConfigClusterLocator.fromConfig(config); + const sut = ConfigClusterLocator.fromConfig(config, authStrategy); const result = await sut.getClusters(); @@ -261,34 +246,6 @@ describe('ConfigClusterLocator', () => { ]); }); - it('supports aks authProvider', async () => { - const sut = ConfigClusterLocator.fromConfig( - new ConfigReader({ - clusters: [ - { - name: 'aks-cluster', - url: 'https://aks.test', - authProvider: 'aks', - }, - ], - }), - ); - - const result = await sut.getClusters(); - - expect(result).toStrictEqual([ - { - name: 'aks-cluster', - url: 'https://aks.test', - authMetadata: { authProvider: 'aks' }, - skipMetricsLookup: false, - skipTLSVerify: false, - caData: undefined, - caFile: undefined, - }, - ]); - }); - it('has cluster level defined customResources returns clusterDetails with those CRDs', async () => { const config: Config = new ConfigReader({ clusters: [ @@ -307,7 +264,7 @@ describe('ConfigClusterLocator', () => { ], }); - const sut = ConfigClusterLocator.fromConfig(config); + const sut = ConfigClusterLocator.fromConfig(config, authStrategy); const result = await sut.getClusters(); @@ -331,22 +288,22 @@ describe('ConfigClusterLocator', () => { ]); }); - // TODO move this to a test on OidcStrategy#validate - it('errors when authProvider is oidc but oidcTokenProvider is missing', async () => { - expect(() => - ConfigClusterLocator.fromConfig( - new ConfigReader({ - clusters: [ - { - name: 'oidc-cluster', - url: 'https://aks.test', - authProvider: 'oidc', - }, - ], - }), - ), - ).toThrow( - `Invalid cluster 'oidc-cluster': Must specify a token provider for 'oidc' strategy`, + it('wraps validation errors from auth strategy', () => { + const config: Config = new ConfigReader({ + clusters: [ + { + name: 'cluster1', + url: 'http://localhost:8080', + authProvider: 'authProvider', + }, + ], + }); + authStrategy.validate.mockImplementation(_ => { + throw new Error('mock error'); + }); + + expect(() => ConfigClusterLocator.fromConfig(config, authStrategy)).toThrow( + `Invalid cluster 'cluster1': mock error`, ); }); }); diff --git a/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.ts b/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.ts index bba9a69635..4bf6f44f79 100644 --- a/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.ts +++ b/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.ts @@ -21,25 +21,7 @@ import { ANNOTATION_KUBERNETES_OIDC_TOKEN_PROVIDER, } from '@backstage/plugin-kubernetes-common'; import { ClusterDetails, KubernetesClustersSupplier } from '../types/types'; -import { - AuthenticationStrategy, - AksStrategy, - DispatchStrategy, - GoogleStrategy, - GoogleServiceAccountStrategy, - NoopStrategy, - OidcStrategy, -} from '../auth'; - -const defaultAuthStrategies: Record = { - aks: new AksStrategy(), - aws: new NoopStrategy(), - google: new GoogleStrategy(), - googleServiceAccount: new GoogleServiceAccountStrategy(), - localKubectlProxy: new NoopStrategy(), - oidc: new OidcStrategy(), - serviceAccount: new NoopStrategy(), -}; +import { AuthenticationStrategy } from '../auth'; export class ConfigClusterLocator implements KubernetesClustersSupplier { private readonly clusterDetails: ClusterDetails[]; @@ -50,9 +32,7 @@ export class ConfigClusterLocator implements KubernetesClustersSupplier { static fromConfig( config: Config, - authStrategy: AuthenticationStrategy = new DispatchStrategy({ - authStrategyMap: defaultAuthStrategies, - }), + authStrategy: AuthenticationStrategy, ): ConfigClusterLocator { return new ConfigClusterLocator( config.getConfigArray('clusters').map(c => {