From 762809def515faf01f05fbe40faca5885a9efd29 Mon Sep 17 00:00:00 2001 From: Jamie Klassen Date: Fri, 12 Jan 2024 10:50:07 -0500 Subject: [PATCH 1/4] clean up KubernetesBuilder tests * reduce whitespace * reduce duplication * simplify stubs (mostly using `mockResolvedValue`) * non-exhaustive attempt to ensure test backends get cleaned up Signed-off-by: Jamie Klassen --- .../src/service/KubernetesBuilder.test.ts | 453 +++++------------- 1 file changed, 125 insertions(+), 328 deletions(-) diff --git a/plugins/kubernetes-backend/src/service/KubernetesBuilder.test.ts b/plugins/kubernetes-backend/src/service/KubernetesBuilder.test.ts index 5719a9392a..a35e488b50 100644 --- a/plugins/kubernetes-backend/src/service/KubernetesBuilder.test.ts +++ b/plugins/kubernetes-backend/src/service/KubernetesBuilder.test.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import { Entity } from '@backstage/catalog-model'; import { ANNOTATION_KUBERNETES_AUTH_PROVIDER, ANNOTATION_KUBERNETES_OIDC_TOKEN_PROVIDER, @@ -24,10 +23,8 @@ import { import request from 'supertest'; import { ClusterDetails, - FetchResponseWrapper, KubernetesFetcher, KubernetesServiceLocator, - ObjectFetchParams, } from '../types/types'; import { KubernetesCredential } from '../auth/types'; import { @@ -64,95 +61,65 @@ describe('KubernetesBuilder', () => { let app: ExtendedHttpServer; let objectsProviderMock: KubernetesObjectsProvider; const happyK8SResult = { - items: [ - { - clusterOne: { - pods: [ - { - metadata: { - name: 'pod1', - }, - }, - ], - }, - }, - ], - } as any; - const policyMock: jest.Mocked = { - authorize: jest.fn(), - authorizeConditional: jest.fn(), + items: [{ clusterOne: { pods: [{ metadata: { name: 'pod1' } }] } }], }; const permissionsMock: ServiceMock = - mockServices.permissions.mock(policyMock); + mockServices.permissions.mock({ + authorize: jest.fn(), + authorizeConditional: jest.fn(), + }); + const minimalValidConfigService = mockServices.rootConfig.factory({ + data: { + kubernetes: { + serviceLocatorMethod: { type: 'multiTenant' }, + clusterLocatorMethods: [], + }, + }, + }); + const withClusters = (clusters: ClusterDetails[]) => + createBackendModule({ + pluginId: 'kubernetes', + moduleId: 'testClusterSupplier', + register(env) { + env.registerInit({ + deps: { extension: kubernetesClusterSupplierExtensionPoint }, + async init({ extension }) { + extension.addClusterSupplier({ + getClusters: jest.fn().mockResolvedValue(clusters), + }); + }, + }); + }, + }); beforeEach(async () => { jest.resetAllMocks(); objectsProviderMock = { - getKubernetesObjectsByEntity: jest.fn().mockImplementation(_ => { - return Promise.resolve(happyK8SResult); - }), - getCustomResourcesByEntity: jest.fn().mockImplementation(_ => { - return Promise.resolve(happyK8SResult); - }), - }; - - const clusterSupplierMock = { - getClusters: jest.fn().mockImplementation(_ => { - return Promise.resolve([ - { - name: 'some-cluster', - url: 'https://localhost:1234', - authMetadata: { - [ANNOTATION_KUBERNETES_AUTH_PROVIDER]: 'serviceAccount', - }, - }, - { - name: 'some-other-cluster', - url: 'https://localhost:1235', - authMetadata: { - [ANNOTATION_KUBERNETES_AUTH_PROVIDER]: 'oidc', - [ANNOTATION_KUBERNETES_OIDC_TOKEN_PROVIDER]: 'google', - }, - }, - ]); - }), + getKubernetesObjectsByEntity: jest.fn().mockResolvedValue(happyK8SResult), + getCustomResourcesByEntity: jest.fn().mockResolvedValue(happyK8SResult), }; jest.mock('@backstage/catalog-client', () => ({ - CatalogClient: jest.fn().mockImplementation(() => ({ - getEntityByRef: jest.fn().mockImplementation(entityRef => { + CatalogClient: jest.fn().mockReturnValue({ + getEntityByRef: jest.fn().mockImplementation(async entityRef => { if (entityRef.name === 'noentity') { - return Promise.resolve(undefined); + return undefined; } - return Promise.resolve({ + return { kind: entityRef.kind, metadata: { name: entityRef.name, namespace: entityRef.namespace, }, - } as Entity); + }; }), - })), + }), })); const { server } = await startTestBackend({ features: [ - mockServices.rootConfig.factory({ - data: { - kubernetes: { - serviceLocatorMethod: { - type: 'multiTenant', - }, - clusterLocatorMethods: [ - { - type: 'config', - clusters: [], - }, - ], - }, - }, - }), + minimalValidConfigService, import('@backstage/plugin-kubernetes-backend/alpha'), import('@backstage/plugin-permission-backend/alpha'), import('@backstage/plugin-permission-backend-module-allow-all-policy'), @@ -168,24 +135,33 @@ describe('KubernetesBuilder', () => { }); }, }), - createBackendModule({ - pluginId: 'kubernetes', - moduleId: 'testClusterSupplier', - register(env) { - env.registerInit({ - deps: { extension: kubernetesClusterSupplierExtensionPoint }, - async init({ extension }) { - extension.addClusterSupplier(clusterSupplierMock); - }, - }); + withClusters([ + { + name: 'some-cluster', + url: 'https://localhost:1234', + authMetadata: { + [ANNOTATION_KUBERNETES_AUTH_PROVIDER]: 'serviceAccount', + }, }, - }), + { + name: 'some-other-cluster', + url: 'https://localhost:1235', + authMetadata: { + [ANNOTATION_KUBERNETES_AUTH_PROVIDER]: 'oidc', + [ANNOTATION_KUBERNETES_OIDC_TOKEN_PROVIDER]: 'google', + }, + }, + ]), ], }); app = server; }); + afterEach(() => { + app.stop(); + }); + describe('get /clusters', () => { it('happy path: lists clusters', async () => { const response = await request(app).get('/api/kubernetes/clusters'); @@ -219,20 +195,16 @@ describe('KubernetesBuilder', () => { it('happy path: lists kubernetes objects with auth in request body', async () => { const response = await request(app) .post('/api/kubernetes/services/test-service') - .send({ - auth: { - google: 'google_token_123', - }, - }) + .send({ auth: { google: 'google_token_123' } }) .set('Content-Type', 'application/json'); expect(response.status).toEqual(200); expect(response.body).toEqual(happyK8SResult); }); it('internal error: lists kubernetes objects', async () => { - objectsProviderMock.getKubernetesObjectsByEntity = jest - .fn() - .mockRejectedValue(Error('some internal error')); + objectsProviderMock.getKubernetesObjectsByEntity.mockRejectedValue( + Error('some internal error'), + ); const response = await request(app).post( '/api/kubernetes/services/test-service', @@ -260,96 +232,29 @@ describe('KubernetesBuilder', () => { }, ]; - const clusterSupplierMock = { - getClusters: jest.fn().mockImplementation(_ => { - return Promise.resolve(clusters); + const pod = { metadata: { name: 'pod1' } }; + + const mockServiceLocator: jest.Mocked = { + getClustersByEntity: jest.fn().mockResolvedValue({ + clusters: [someCluster], }), }; - const pod = { - metadata: { - name: 'pod1', - }, - }; - const result: ObjectsByEntityResponse = { - items: [ - { - cluster: { - name: someCluster.name, - }, - errors: [], - podMetrics: [], - resources: [ - { - type: 'pods', - resources: [pod], - }, - ], - }, - ], - }; - - const mockServiceLocator: KubernetesServiceLocator = { - getClustersByEntity( - _entity: Entity, - ): Promise<{ clusters: ClusterDetails[] }> { - return Promise.resolve({ clusters: [someCluster] }); - }, - }; - - const mockFetcher: KubernetesFetcher = { - fetchPodMetricsByNamespaces( - _clusterDetails: ClusterDetails, - _credential: KubernetesCredential, - _namespaces: Set, - ): Promise { - return Promise.resolve({ errors: [], responses: [] }); - }, - fetchObjectsForService( - _params: ObjectFetchParams, - ): Promise { - return Promise.resolve({ - errors: [], - responses: [ - { - type: 'pods', - resources: [pod], - }, - ], - }); - }, + const mockFetcher: jest.Mocked = { + fetchPodMetricsByNamespaces: jest + .fn() + .mockResolvedValue({ errors: [], responses: [] }), + fetchObjectsForService: jest.fn().mockResolvedValue({ + errors: [], + responses: [{ type: 'pods', resources: [pod] }], + }), }; const { server } = await startTestBackend({ features: [ - mockServices.rootConfig.factory({ - data: { - kubernetes: { - serviceLocatorMethod: { - type: 'multiTenant', - }, - clusterLocatorMethods: [ - { - type: 'config', - clusters: [], - }, - ], - }, - }, - }), + minimalValidConfigService, import('@backstage/plugin-kubernetes-backend/alpha'), - createBackendModule({ - pluginId: 'kubernetes', - moduleId: 'testClusterSupplier', - register(env) { - env.registerInit({ - deps: { extension: kubernetesClusterSupplierExtensionPoint }, - async init({ extension }) { - extension.addClusterSupplier(clusterSupplierMock); - }, - }); - }, - }), + withClusters(clusters), createBackendModule({ pluginId: 'kubernetes', moduleId: 'testFetcher', @@ -376,20 +281,22 @@ describe('KubernetesBuilder', () => { }), ], }); - app = server; const response = await request(app) .post('/api/kubernetes/services/test-service') - .send({ - entity: { - metadata: { - name: 'thing', - }, - }, - }); + .send({ entity: { metadata: { name: 'thing' } } }); - expect(response.body).toEqual(result); + expect(response.body).toEqual({ + items: [ + { + cluster: { name: someCluster.name }, + errors: [], + podMetrics: [], + resources: [{ type: 'pods', resources: [pod] }], + }, + ], + }); expect(response.status).toEqual(200); }); @@ -406,9 +313,11 @@ describe('KubernetesBuilder', () => { }), }; - const clusterSupplierMock = { - getClusters: jest.fn().mockImplementation(_ => { - return Promise.resolve([ + const { server } = await startTestBackend({ + features: [ + minimalValidConfigService, + import('@backstage/plugin-kubernetes-backend/alpha'), + withClusters([ { name: 'custom-cluster', url: 'http://my.cluster.url', @@ -416,40 +325,7 @@ describe('KubernetesBuilder', () => { [ANNOTATION_KUBERNETES_AUTH_PROVIDER]: 'custom', }, }, - ]); - }), - }; - - const { server } = await startTestBackend({ - features: [ - mockServices.rootConfig.factory({ - data: { - kubernetes: { - serviceLocatorMethod: { - type: 'multiTenant', - }, - clusterLocatorMethods: [ - { - type: 'config', - clusters: [], - }, - ], - }, - }, - }), - import('@backstage/plugin-kubernetes-backend/alpha'), - createBackendModule({ - pluginId: 'kubernetes', - moduleId: 'testClusterSupplier', - register(env) { - env.registerInit({ - deps: { extension: kubernetesClusterSupplierExtensionPoint }, - async init({ extension }) { - extension.addClusterSupplier(clusterSupplierMock); - }, - }); - }, - }), + ]), createBackendModule({ pluginId: 'kubernetes', moduleId: 'testAuthStrategy', @@ -493,11 +369,7 @@ describe('KubernetesBuilder', () => { await request(app) .post('/api/kubernetes/services/test-service') .send({ - entity: { - metadata: { - name: 'thing', - }, - }, + entity: { metadata: { name: 'thing' } }, auth: { custom: 'custom-token' }, }); @@ -534,30 +406,26 @@ describe('KubernetesBuilder', () => { ); }); - it('returns the given request body with permission set to allow', async () => { - const requestBody = { + it('forwards request body to k8s', async () => { + const namespaceManifest = { kind: 'Namespace', apiVersion: 'v1', - metadata: { - name: 'new-ns', - }, + metadata: { name: 'new-ns' }, }; const proxyEndpointRequest = request(app) .post('/api/kubernetes/proxy/api/v1/namespaces') .set(HEADER_KUBERNETES_CLUSTER, 'some-cluster') .set(HEADER_KUBERNETES_AUTH, 'randomtoken') - .send(requestBody); - + .send(namespaceManifest); worker.use(rest.all(proxyEndpointRequest.url, req => req.passthrough())); - const response = await proxyEndpointRequest; - expect(response.body).toStrictEqual(requestBody); + expect(response.body).toStrictEqual(namespaceManifest); }); - it('supports yaml content type with permission set to allow', async () => { - const requestBody = `--- + it('supports yaml content type', async () => { + const yamlManifest = `--- kind: Namespace apiVersion: v1 metadata: @@ -569,55 +437,37 @@ metadata: .set(HEADER_KUBERNETES_CLUSTER, 'some-cluster') .set(HEADER_KUBERNETES_AUTH, 'randomtoken') .set('content-type', 'application/yaml') - .send(requestBody); + .send(yamlManifest); worker.use(rest.all(proxyEndpointRequest.url, req => req.passthrough())); const response = await proxyEndpointRequest; - expect(response.text).toEqual(requestBody); + expect(response.text).toEqual(yamlManifest); }); - it('returns a 403 response if Permission Policy is in place that blocks endpoint', async () => { - const requestBody = { - kind: 'Namespace', - apiVersion: 'v1', - metadata: { - name: 'new-ns', - }, - }; - + it('returns 403 response when permission blocks endpoint', async () => { permissionsMock.authorize.mockResolvedValue([ { result: AuthorizeResult.DENY }, ]); const { server } = await startTestBackend({ features: [ - mockServices.rootConfig.factory({ - data: { - kubernetes: { - serviceLocatorMethod: { - type: 'multiTenant', - }, - clusterLocatorMethods: [ - { - type: 'config', - clusters: [], - }, - ], - }, - }, - }), + minimalValidConfigService, permissionsMock.factory, import('@backstage/plugin-kubernetes-backend/alpha'), - // import('@backstage/plugin-permission-backend/alpha'), ], }); + app = server; - const proxyEndpointRequest = request(server) + const proxyEndpointRequest = request(app) .post('/api/kubernetes/proxy/api/v1/namespaces') .set(HEADER_KUBERNETES_CLUSTER, 'some-cluster') .set(HEADER_KUBERNETES_AUTH, 'randomtoken') - .send(requestBody); + .send({ + kind: 'Namespace', + apiVersion: 'v1', + metadata: { name: 'new-ns' }, + }); worker.use(rest.all(proxyEndpointRequest.url, req => req.passthrough())); @@ -636,62 +486,17 @@ metadata: }), ); - const clusterSupplierMock = { - getClusters: jest.fn().mockImplementation(_ => { - return Promise.resolve([ + const { server } = await startTestBackend({ + features: [ + minimalValidConfigService, + import('@backstage/plugin-kubernetes-backend/alpha'), + withClusters([ { name: 'custom-cluster', url: 'http://my.cluster.url', - authMetadata: { - [ANNOTATION_KUBERNETES_AUTH_PROVIDER]: 'custom', - }, + authMetadata: { [ANNOTATION_KUBERNETES_AUTH_PROVIDER]: 'custom' }, }, - ]); - }), - }; - - const { server } = await startTestBackend({ - features: [ - mockServices.rootConfig.factory({ - data: { - kubernetes: { - serviceLocatorMethod: { - type: 'multiTenant', - }, - clusterLocatorMethods: [ - { - type: 'config', - clusters: [], - }, - ], - }, - }, - }), - import('@backstage/plugin-kubernetes-backend/alpha'), - createBackendModule({ - pluginId: 'kubernetes', - moduleId: 'testObjectsProvider', - register(env) { - env.registerInit({ - deps: { extension: kubernetesObjectsProviderExtensionPoint }, - async init({ extension }) { - extension.addObjectsProvider(objectsProviderMock); - }, - }); - }, - }), - createBackendModule({ - pluginId: 'kubernetes', - moduleId: 'testClusterSupplier', - register(env) { - env.registerInit({ - deps: { extension: kubernetesClusterSupplierExtensionPoint }, - async init({ extension }) { - extension.addClusterSupplier(clusterSupplierMock); - }, - }); - }, - }), + ]), createBackendModule({ pluginId: 'kubernetes', moduleId: 'testAuthStrategy', @@ -715,9 +520,7 @@ metadata: ], }); - app = server; - - const proxyEndpointRequest = request(app) + const proxyEndpointRequest = request(server) .get('/api/kubernetes/proxy/api/v1/namespaces') .set(HEADER_KUBERNETES_CLUSTER, 'custom-cluster') .set(HEADER_KUBERNETES_AUTH, 'custom-token'); @@ -727,9 +530,9 @@ metadata: expect(response.body).toStrictEqual({ items: [] }); }); - it('should not permit custom auth strategies with dashes', async () => { - const throwError = async () => { - await startTestBackend({ + it('should not permit custom auth strategies with dashes', () => { + const throwError = () => + startTestBackend({ features: [ import('@backstage/plugin-kubernetes-backend/alpha'), createBackendModule({ @@ -754,9 +557,7 @@ metadata: }), ], }); - }; - - await expect(throwError).rejects.toThrow( + return expect(throwError).rejects.toThrow( 'Strategy name can not include dashes', ); }); @@ -771,11 +572,7 @@ metadata: expect(response.status).toEqual(200); expect(response.body).toMatchObject({ permissions: [ - { - type: 'basic', - name: 'kubernetes.proxy', - attributes: {}, - }, + { type: 'basic', name: 'kubernetes.proxy', attributes: {} }, ], rules: [], }); From 7f6ff255f1b2af507941577a8dfc4de1f9e7b874 Mon Sep 17 00:00:00 2001 From: Jamie Klassen Date: Fri, 12 Jan 2024 12:40:40 -0500 Subject: [PATCH 2/4] enable custom auth metadata in config Signed-off-by: Jamie Klassen --- .changeset/ten-numbers-happen.md | 5 + plugins/kubernetes-backend/config.d.ts | 11 +- .../ConfigClusterLocator.test.ts | 61 +++++++ .../cluster-locator/ConfigClusterLocator.ts | 8 +- .../src/service/KubernetesBuilder.test.ts | 150 ++++++++++++------ 5 files changed, 185 insertions(+), 50 deletions(-) create mode 100644 .changeset/ten-numbers-happen.md diff --git a/.changeset/ten-numbers-happen.md b/.changeset/ten-numbers-happen.md new file mode 100644 index 0000000000..73397d4568 --- /dev/null +++ b/.changeset/ten-numbers-happen.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-kubernetes-backend': patch +--- + +Custom per-cluster auth metadata (mainly for use with custom `AuthenticationStrategy` implementations) can now be specified in the `authMetadata` property of clusters in the app-config. diff --git a/plugins/kubernetes-backend/config.d.ts b/plugins/kubernetes-backend/config.d.ts index e9b5878920..9c5417a00e 100644 --- a/plugins/kubernetes-backend/config.d.ts +++ b/plugins/kubernetes-backend/config.d.ts @@ -46,13 +46,16 @@ export interface Config { /** @visibility secret */ serviceAccountToken?: string; /** @visibility frontend */ - authProvider: + authProvider?: + | 'aks' | 'aws' - | 'google' - | 'serviceAccount' | 'azure' + | 'google' + | 'googleServiceAccount' | 'oidc' - | 'googleServiceAccount'; + | 'serviceAccount'; + /** @visibility secret */ + authMetadata?: object; /** @visibility frontend */ oidcTokenProvider?: string; /** @visibility frontend */ diff --git a/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.test.ts b/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.test.ts index 258cddcd66..cca2b28fcf 100644 --- a/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.test.ts +++ b/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.test.ts @@ -177,6 +177,67 @@ describe('ConfigClusterLocator', () => { ]); }); + it('reads custom authMetadata', async () => { + const config: Config = new ConfigReader({ + clusters: [ + { + name: 'cluster', + url: 'http://url', + authProvider: 'authProvider', + authMetadata: { 'custom-key': 'custom-value' }, + }, + ], + }); + + const result = await ConfigClusterLocator.fromConfig( + config, + authStrategy, + ).getClusters(); + + expect(result).toMatchObject([ + { + authMetadata: expect.objectContaining({ 'custom-key': 'custom-value' }), + }, + ]); + }); + + it('reads authProvider from metadata block', async () => { + const config: Config = new ConfigReader({ + clusters: [ + { + name: 'cluster', + url: 'http://url', + authMetadata: { + [ANNOTATION_KUBERNETES_AUTH_PROVIDER]: 'serviceAccount', + }, + }, + ], + }); + + const result = await ConfigClusterLocator.fromConfig( + config, + authStrategy, + ).getClusters(); + + expect(result).toMatchObject([ + { + authMetadata: { + [ANNOTATION_KUBERNETES_AUTH_PROVIDER]: 'serviceAccount', + }, + }, + ]); + }); + + it('forbids cluster without auth provider', () => { + const config: Config = new ConfigReader({ + clusters: [{ name: 'cluster', url: 'http://url' }], + }); + + expect(() => ConfigClusterLocator.fromConfig(config, authStrategy)).toThrow( + 'Missing required config value', + ); + }); + it('one cluster with dashboardParameters', async () => { const config: Config = new ConfigReader({ clusters: [ diff --git a/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.ts b/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.ts index 7899d7762d..816a9da0a6 100644 --- a/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.ts +++ b/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.ts @@ -37,7 +37,12 @@ export class ConfigClusterLocator implements KubernetesClustersSupplier { ): ConfigClusterLocator { return new ConfigClusterLocator( config.getConfigArray('clusters').map(c => { - const authProvider = c.getString('authProvider'); + const authMetadataBlock = c.getOptional<{ + [ANNOTATION_KUBERNETES_AUTH_PROVIDER]?: string; + }>('authMetadata'); + const authProvider = + authMetadataBlock?.[ANNOTATION_KUBERNETES_AUTH_PROVIDER] ?? + c.getString('authProvider'); const clusterDetails: ClusterDetails = { name: c.getString('name'), url: c.getString('url'), @@ -48,6 +53,7 @@ export class ConfigClusterLocator implements KubernetesClustersSupplier { authMetadata: { [ANNOTATION_KUBERNETES_AUTH_PROVIDER]: authProvider, ...ConfigClusterLocator.parseAuthMetadata(c), + ...authMetadataBlock, }, }; diff --git a/plugins/kubernetes-backend/src/service/KubernetesBuilder.test.ts b/plugins/kubernetes-backend/src/service/KubernetesBuilder.test.ts index a35e488b50..42f953a2fe 100644 --- a/plugins/kubernetes-backend/src/service/KubernetesBuilder.test.ts +++ b/plugins/kubernetes-backend/src/service/KubernetesBuilder.test.ts @@ -17,7 +17,6 @@ import { ANNOTATION_KUBERNETES_AUTH_PROVIDER, ANNOTATION_KUBERNETES_OIDC_TOKEN_PROVIDER, - ObjectsByEntityResponse, KubernetesRequestAuth, } from '@backstage/plugin-kubernetes-common'; import request from 'supertest'; @@ -39,10 +38,7 @@ import { startTestBackend, } from '@backstage/backend-test-utils'; import { rest } from 'msw'; -import { - AuthorizeResult, - PermissionEvaluator, -} from '@backstage/plugin-permission-common'; +import { AuthorizeResult } from '@backstage/plugin-permission-common'; import { PermissionsService, createBackendModule, @@ -57,9 +53,9 @@ import { } from '@backstage/plugin-kubernetes-node'; import { ExtendedHttpServer } from '@backstage/backend-app-api'; -describe('KubernetesBuilder', () => { +describe('API integration tests', () => { let app: ExtendedHttpServer; - let objectsProviderMock: KubernetesObjectsProvider; + let objectsProviderMock: jest.Mocked; const happyK8SResult = { items: [{ clusterOne: { pods: [{ metadata: { name: 'pod1' } }] } }], }; @@ -530,52 +526,116 @@ metadata: expect(response.body).toStrictEqual({ items: [] }); }); - it('should not permit custom auth strategies with dashes', () => { - const throwError = () => - startTestBackend({ - features: [ - import('@backstage/plugin-kubernetes-backend/alpha'), - createBackendModule({ - pluginId: 'kubernetes', - moduleId: 'testAuthStrategy', - register(env) { - env.registerInit({ - deps: { extension: kubernetesAuthStrategyExtensionPoint }, - async init({ extension }) { - extension.addAuthStrategy('custom-strategy', { - getCredential: jest - .fn< - Promise, - [ClusterDetails, KubernetesRequestAuth] - >() - .mockResolvedValue({ type: 'anonymous' }), - validateCluster: jest.fn().mockReturnValue([]), - }); + it('reads custom auth metadata from config', async () => { + const authStrategy = { + getCredential: jest.fn().mockResolvedValue({ type: 'anonymous' }), + validateCluster: jest.fn().mockReturnValue([]), + }; + worker.use( + rest.get('http://my.cluster/api', (_req, res, ctx) => + res(ctx.json({})), + ), + ); + const { server } = await startTestBackend({ + features: [ + mockServices.rootConfig.factory({ + data: { + kubernetes: { + serviceLocatorMethod: { type: 'multiTenant' }, + clusterLocatorMethods: [ + { + type: 'config', + clusters: [ + { + name: 'cluster', + url: 'http://my.cluster', + authProvider: 'custom', + authMetadata: { 'custom-key': 'custom-value' }, + }, + ], }, - }); + ], }, - }), - ], - }); - return expect(throwError).rejects.toThrow( - 'Strategy name can not include dashes', + }, + }), + import('@backstage/plugin-kubernetes-backend/alpha'), + createBackendModule({ + pluginId: 'kubernetes', + moduleId: 'testAuthStrategy', + register(env) { + env.registerInit({ + deps: { extension: kubernetesAuthStrategyExtensionPoint }, + async init({ extension }) { + extension.addAuthStrategy('custom', authStrategy); + }, + }); + }, + }), + ], + }); + app = server; + + const proxyEndpointRequest = request(app).get( + '/api/kubernetes/proxy/api', + ); + worker.use(rest.all(proxyEndpointRequest.url, req => req.passthrough())); + const response = await proxyEndpointRequest; + + expect(response.body).toStrictEqual({}); + expect(authStrategy.getCredential).toHaveBeenCalledWith( + expect.objectContaining({ + authMetadata: expect.objectContaining({ + 'custom-key': 'custom-value', + }), + }), + expect.anything(), ); }); }); - describe('get /.well-known/backstage/permissions/metadata', () => { - it('lists permissions supported by the kubernetes plugin', async () => { - const response = await request(app).get( - '/api/kubernetes/.well-known/backstage/permissions/metadata', - ); - - expect(response.status).toEqual(200); - expect(response.body).toMatchObject({ - permissions: [ - { type: 'basic', name: 'kubernetes.proxy', attributes: {} }, + it('forbids custom auth strategies with dashes', () => { + const throwError = () => + startTestBackend({ + features: [ + import('@backstage/plugin-kubernetes-backend/alpha'), + createBackendModule({ + pluginId: 'kubernetes', + moduleId: 'testAuthStrategy', + register(env) { + env.registerInit({ + deps: { extension: kubernetesAuthStrategyExtensionPoint }, + async init({ extension }) { + extension.addAuthStrategy('custom-strategy', { + getCredential: jest + .fn< + Promise, + [ClusterDetails, KubernetesRequestAuth] + >() + .mockResolvedValue({ type: 'anonymous' }), + validateCluster: jest.fn().mockReturnValue([]), + }); + }, + }); + }, + }), ], - rules: [], }); + return expect(throwError).rejects.toThrow( + 'Strategy name can not include dashes', + ); + }); + + it('serves permission integration endpoint', async () => { + const response = await request(app).get( + '/api/kubernetes/.well-known/backstage/permissions/metadata', + ); + + expect(response.status).toEqual(200); + expect(response.body).toMatchObject({ + permissions: [ + { type: 'basic', name: 'kubernetes.proxy', attributes: {} }, + ], + rules: [], }); }); From 7125d9c4329542544cc9413c3a435573dec4691a Mon Sep 17 00:00:00 2001 From: Jamie Klassen Date: Fri, 12 Jan 2024 17:41:49 -0500 Subject: [PATCH 3/4] regression test for parameter precedence Signed-off-by: Jamie Klassen --- .../ConfigClusterLocator.test.ts | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.test.ts b/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.test.ts index cca2b28fcf..6fb33d8b3a 100644 --- a/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.test.ts +++ b/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.test.ts @@ -228,6 +228,34 @@ describe('ConfigClusterLocator', () => { ]); }); + it('prefers authMetadata block to top-level keys', async () => { + const sut = ConfigClusterLocator.fromConfig( + new ConfigReader({ + clusters: [ + { + name: 'cluster', + url: 'http://url', + authProvider: 'aws', + authMetadata: { + [ANNOTATION_KUBERNETES_AUTH_PROVIDER]: 'serviceAccount', + }, + }, + ], + }), + authStrategy, + ); + + const result = await sut.getClusters(); + + expect(result).toMatchObject([ + { + authMetadata: { + [ANNOTATION_KUBERNETES_AUTH_PROVIDER]: 'serviceAccount', + }, + }, + ]); + }); + it('forbids cluster without auth provider', () => { const config: Config = new ConfigReader({ clusters: [{ name: 'cluster', url: 'http://url' }], From 413f08ddcc5dd156eb77c22b4ab7e69300cce70d Mon Sep 17 00:00:00 2001 From: Jamie Klassen Date: Fri, 12 Jan 2024 17:42:23 -0500 Subject: [PATCH 4/4] improve error message on missing authProvider Signed-off-by: Jamie Klassen --- .../src/cluster-locator/ConfigClusterLocator.test.ts | 4 +++- .../src/cluster-locator/ConfigClusterLocator.ts | 12 ++++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.test.ts b/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.test.ts index 6fb33d8b3a..1720ef5ed6 100644 --- a/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.test.ts +++ b/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.test.ts @@ -262,7 +262,9 @@ describe('ConfigClusterLocator', () => { }); expect(() => ConfigClusterLocator.fromConfig(config, authStrategy)).toThrow( - 'Missing required config value', + `cluster 'cluster' has no auth provider configured; this must be specified` + + ` via the 'authProvider' or ` + + `'authMetadata.${ANNOTATION_KUBERNETES_AUTH_PROVIDER}' parameter`, ); }); diff --git a/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.ts b/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.ts index 816a9da0a6..19ba8634ad 100644 --- a/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.ts +++ b/plugins/kubernetes-backend/src/cluster-locator/ConfigClusterLocator.ts @@ -40,11 +40,19 @@ export class ConfigClusterLocator implements KubernetesClustersSupplier { const authMetadataBlock = c.getOptional<{ [ANNOTATION_KUBERNETES_AUTH_PROVIDER]?: string; }>('authMetadata'); + const name = c.getString('name'); const authProvider = authMetadataBlock?.[ANNOTATION_KUBERNETES_AUTH_PROVIDER] ?? - c.getString('authProvider'); + c.getOptionalString('authProvider'); + if (!authProvider) { + throw new Error( + `cluster '${name}' has no auth provider configured; this must be ` + + `specified via the 'authProvider' or ` + + `'authMetadata.${ANNOTATION_KUBERNETES_AUTH_PROVIDER}' parameter`, + ); + } const clusterDetails: ClusterDetails = { - name: c.getString('name'), + name, url: c.getString('url'), skipTLSVerify: c.getOptionalBoolean('skipTLSVerify') ?? false, skipMetricsLookup: c.getOptionalBoolean('skipMetricsLookup') ?? false,