better name + signature for validation method

Signed-off-by: Jamie Klassen <jklassen@vmware.com>
This commit is contained in:
Jamie Klassen
2023-09-14 17:51:35 -04:00
parent 0ad36158d9
commit 9212955b35
19 changed files with 63 additions and 45 deletions
+10 -10
View File
@@ -29,7 +29,7 @@ export class AksStrategy implements AuthenticationStrategy {
requestAuth: KubernetesRequestAuth,
): Promise<KubernetesCredential>;
// (undocumented)
validate(_: AuthMetadata): void;
validateCluster(): Error[];
}
// @public (undocumented)
@@ -37,7 +37,7 @@ export class AnonymousStrategy implements AuthenticationStrategy {
// (undocumented)
getCredential(): Promise<KubernetesCredential>;
// (undocumented)
validate(): void;
validateCluster(): Error[];
}
// @public (undocumented)
@@ -48,7 +48,7 @@ export interface AuthenticationStrategy {
authConfig: KubernetesRequestAuth,
): Promise<KubernetesCredential>;
// (undocumented)
validate(authMetadata: AuthMetadata): void;
validateCluster(authMetadata: AuthMetadata): Error[];
}
// @public
@@ -60,7 +60,7 @@ export class AwsIamStrategy implements AuthenticationStrategy {
// (undocumented)
getCredential(clusterDetails: ClusterDetails): Promise<KubernetesCredential>;
// (undocumented)
validate(): void;
validateCluster(): Error[];
}
// @public (undocumented)
@@ -69,7 +69,7 @@ export class AzureIdentityStrategy implements AuthenticationStrategy {
// (undocumented)
getCredential(): Promise<KubernetesCredential>;
// (undocumented)
validate(): void;
validateCluster(): Error[];
}
// @public (undocumented)
@@ -119,7 +119,7 @@ export class DispatchStrategy implements AuthenticationStrategy {
auth: KubernetesRequestAuth,
): Promise<KubernetesCredential>;
// (undocumented)
validate(authMetadata: AuthMetadata): void;
validateCluster(authMetadata: AuthMetadata): Error[];
}
// @public (undocumented)
@@ -142,7 +142,7 @@ export class GoogleServiceAccountStrategy implements AuthenticationStrategy {
// (undocumented)
getCredential(): Promise<KubernetesCredential>;
// (undocumented)
validate(): void;
validateCluster(): Error[];
}
// @public (undocumented)
@@ -153,7 +153,7 @@ export class GoogleStrategy implements AuthenticationStrategy {
requestAuth: KubernetesRequestAuth,
): Promise<KubernetesCredential>;
// (undocumented)
validate(_: AuthMetadata): void;
validateCluster(): Error[];
}
// @public
@@ -441,7 +441,7 @@ export class OidcStrategy implements AuthenticationStrategy {
authConfig: KubernetesRequestAuth,
): Promise<KubernetesCredential>;
// (undocumented)
validate(authMetadata: AuthMetadata): void;
validateCluster(authMetadata: AuthMetadata): Error[];
}
// @public (undocumented)
@@ -465,7 +465,7 @@ export class ServiceAccountStrategy implements AuthenticationStrategy {
// (undocumented)
getCredential(clusterDetails: ClusterDetails): Promise<KubernetesCredential>;
// (undocumented)
validate(): void;
validateCluster(): Error[];
}
// @public (undocumented)
@@ -13,7 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { AuthMetadata, ClusterDetails } from '../types/types';
import { ClusterDetails } from '../types/types';
import { AuthenticationStrategy, KubernetesCredential } from './types';
import { KubernetesRequestAuth } from '@backstage/plugin-kubernetes-common';
@@ -31,5 +31,7 @@ export class AksStrategy implements AuthenticationStrategy {
? { type: 'bearer token', token: token as string }
: { type: 'anonymous' };
}
public validate(_: AuthMetadata) {}
public validateCluster(): Error[] {
return [];
}
}
@@ -25,5 +25,7 @@ export class AnonymousStrategy implements AuthenticationStrategy {
return { type: 'anonymous' };
}
public validate() {}
public validateCluster(): Error[] {
return [];
}
}
@@ -64,7 +64,9 @@ export class AwsIamStrategy implements AuthenticationStrategy {
};
}
public validate() {}
public validateCluster(): Error[] {
return [];
}
private async getBearerToken(
clusterName: string,
@@ -51,7 +51,9 @@ export class AzureIdentityStrategy implements AuthenticationStrategy {
: { type: 'anonymous' };
}
public validate() {}
public validateCluster(): Error[] {
return [];
}
private async fetchNewToken(): Promise<string> {
try {
@@ -30,7 +30,7 @@ describe('getCredential', () => {
beforeEach(() => {
mockStrategy = {
getCredential: jest.fn(),
validate: jest.fn(),
validateCluster: jest.fn(),
};
strategy = new DispatchStrategy({
authStrategyMap: { google: mockStrategy },
@@ -55,14 +55,16 @@ export class DispatchStrategy implements AuthenticationStrategy {
);
}
public validate(authMetadata: AuthMetadata) {
public validateCluster(authMetadata: AuthMetadata): Error[] {
const authProvider = authMetadata[ANNOTATION_KUBERNETES_AUTH_PROVIDER];
const strategy = this.strategyMap[authProvider];
if (!strategy) {
throw new Error(
`authProvider "${authProvider}" has no config associated with it`,
);
return [
new Error(
`authProvider "${authProvider}" has no config associated with it`,
),
];
}
strategy.validate(authMetadata);
return strategy.validateCluster(authMetadata);
}
}
@@ -33,5 +33,7 @@ export class GoogleServiceAccountStrategy implements AuthenticationStrategy {
return { type: 'bearer token', token };
}
public validate() {}
public validateCluster(): Error[] {
return [];
}
}
@@ -15,7 +15,7 @@
*/
import { AuthenticationStrategy, KubernetesCredential } from './types';
import { AuthMetadata, ClusterDetails } from '../types/types';
import { ClusterDetails } from '../types/types';
import { KubernetesRequestAuth } from '@backstage/plugin-kubernetes-common';
/**
@@ -35,5 +35,7 @@ export class GoogleStrategy implements AuthenticationStrategy {
}
return { type: 'bearer token', token: token as string };
}
public validate(_: AuthMetadata) {}
public validateCluster(): Error[] {
return [];
}
}
@@ -75,10 +75,10 @@ describe('OidcStrategy', () => {
});
});
describe('validate', () => {
describe('validateCluster', () => {
it('fails when token provider is not specified', () => {
expect(() => strategy.validate({})).toThrow(
`Must specify a token provider for 'oidc' strategy`,
expect(strategy.validateCluster({})).toContainEqual(
new Error(`Must specify a token provider for 'oidc' strategy`),
);
});
});
@@ -49,11 +49,12 @@ export class OidcStrategy implements AuthenticationStrategy {
return { type: 'bearer token', token: token as string };
}
public validate(authMetadata: AuthMetadata) {
public validateCluster(authMetadata: AuthMetadata): Error[] {
const oidcTokenProvider =
authMetadata[ANNOTATION_KUBERNETES_OIDC_TOKEN_PROVIDER];
if (!oidcTokenProvider || oidcTokenProvider === '') {
throw new Error(`Must specify a token provider for 'oidc' strategy`);
return [new Error(`Must specify a token provider for 'oidc' strategy`)];
}
return [];
}
}
@@ -42,5 +42,7 @@ export class ServiceAccountStrategy implements AuthenticationStrategy {
};
}
public validate() {}
public validateCluster(): Error[] {
return [];
}
}
+1 -1
View File
@@ -34,5 +34,5 @@ export interface AuthenticationStrategy {
clusterDetails: ClusterDetails,
authConfig: KubernetesRequestAuth,
): Promise<KubernetesCredential>;
validate(authMetadata: AuthMetadata): void;
validateCluster(authMetadata: AuthMetadata): Error[];
}
@@ -31,7 +31,7 @@ describe('ConfigClusterLocator', () => {
beforeEach(() => {
authStrategy = {
getCredential: jest.fn(),
validate: jest.fn(),
validateCluster: jest.fn().mockReturnValue([]),
};
});
@@ -307,9 +307,7 @@ describe('ConfigClusterLocator', () => {
},
],
});
authStrategy.validate.mockImplementation(_ => {
throw new Error('mock error');
});
authStrategy.validateCluster.mockReturnValue([new Error('mock error')]);
expect(() => ConfigClusterLocator.fromConfig(config, authStrategy)).toThrow(
`Invalid cluster 'cluster1': mock error`,
@@ -74,11 +74,14 @@ export class ConfigClusterLocator implements KubernetesClustersSupplier {
clusterDetails.dashboardParameters = c.get('dashboardParameters');
}
try {
authStrategy.validate(clusterDetails.authMetadata);
} catch (e) {
const validationErrors = authStrategy.validateCluster(
clusterDetails.authMetadata,
);
if (validationErrors.length !== 0) {
throw new Error(
`Invalid cluster '${clusterDetails.name}': ${e.message}`,
`Invalid cluster '${clusterDetails.name}': ${validationErrors
.map(e => e.message)
.join(', ')}`,
);
}
return clusterDetails;
@@ -52,7 +52,7 @@ describe('getCombinedClusterSupplier', () => {
);
const mockStrategy: jest.Mocked<AuthenticationStrategy> = {
getCredential: jest.fn(),
validate: jest.fn(),
validateCluster: jest.fn().mockReturnValue([]),
};
const clusterSupplier = getCombinedClusterSupplier(
@@ -328,7 +328,7 @@ describe('KubernetesBuilder', () => {
type: 'bearer token',
token: requestAuth.custom as string,
})),
validate: jest.fn(),
validateCluster: jest.fn().mockReturnValue([]),
})
.setClusterSupplier({
getClusters: jest
@@ -493,7 +493,7 @@ metadata:
[ClusterDetails, KubernetesRequestAuth]
>()
.mockResolvedValue({ type: 'anonymous' }),
validate: jest.fn(),
validateCluster: jest.fn().mockReturnValue([]),
})
.setClusterSupplier({
getClusters: jest
@@ -195,7 +195,7 @@ describe('KubernetesFanOutHandler', () => {
[ClusterDetails, KubernetesRequestAuth]
>()
.mockResolvedValue({ type: 'anonymous' }),
validate: jest.fn(),
validateCluster: jest.fn().mockReturnValue([]),
},
config,
});
@@ -1146,7 +1146,7 @@ describe('KubernetesFanOutHandler', () => {
[ClusterDetails, KubernetesRequestAuth]
>()
.mockResolvedValue({ type: 'bearer token', token: 'token' }),
validate: jest.fn(),
validateCluster: jest.fn().mockReturnValue([]),
},
config,
});
@@ -132,7 +132,7 @@ describe('KubernetesProxy', () => {
[ClusterDetails, KubernetesRequestAuth]
>()
.mockResolvedValue({ type: 'anonymous' }),
validate: jest.fn(),
validateCluster: jest.fn(),
};
proxy = new KubernetesProxy({ logger, clusterSupplier, authStrategy });
permissionApi.authorize.mockResolvedValue([