code review

Signed-off-by: goenning <me@goenning.net>
This commit is contained in:
goenning
2022-05-20 16:13:31 +01:00
parent f3b42c2cdb
commit 63aa03cc82
5 changed files with 54 additions and 23 deletions
@@ -72,7 +72,7 @@ describe('AzureIdentityKubernetesAuthTranslator tests', () => {
it('should issue new token 15 minutes befory expiry', async () => {
const authTranslator = new AzureIdentityKubernetesAuthTranslator(
logger,
new StaticTokenCredential(16 * 60 * 1000), // token expires in 16m
new StaticTokenCredential(16 * 60 * 1000), // token expires in 16min
);
const response = await authTranslator.decorateClusterDetailsWithAuth(cd);
@@ -87,25 +87,44 @@ describe('AzureIdentityKubernetesAuthTranslator tests', () => {
it('should re-use existing token if there is afailure', async () => {
const authTranslator = new AzureIdentityKubernetesAuthTranslator(
logger,
new StaticTokenCredential(16 * 60 * 1000), // new tokens expires in 16m
new StaticTokenCredential(16 * 60 * 1000), // new tokens expires in 16min
);
const response = await authTranslator.decorateClusterDetailsWithAuth(cd);
expect(response.serviceAccountToken).toEqual('MY_TOKEN_1');
jest.useFakeTimers().setSystemTime(Date.now() + 2 * 60 * 1000); // advance time by 2mins
jest.useFakeTimers().setSystemTime(Date.now() + 2 * 60 * 1000); // advance time by 2min
const response2 = await authTranslator.decorateClusterDetailsWithAuth(cd);
expect(response2.serviceAccountToken).toEqual('MY_TOKEN_2');
jest.useFakeTimers().setSystemTime(Date.now() + 2 * 60 * 1000); // advance time by 2mins
jest.useFakeTimers().setSystemTime(Date.now() + 2 * 60 * 1000); // advance time by 2min
const response3 = await authTranslator.decorateClusterDetailsWithAuth(cd);
expect(response3.serviceAccountToken).toEqual('MY_TOKEN_2');
jest.useFakeTimers().setSystemTime(Date.now() + 2 * 60 * 1000); // advance time by 2mins
const response4 = await authTranslator.decorateClusterDetailsWithAuth(cd);
expect(response4.serviceAccountToken).toEqual('MY_TOKEN_4');
});
it('should throw if existing token expired and failed to fetch a new one', async () => {
const authTranslator = new AzureIdentityKubernetesAuthTranslator(
logger,
new StaticTokenCredential(16 * 60 * 1000), // new tokens expires in 16min
);
const response = await authTranslator.decorateClusterDetailsWithAuth(cd);
expect(response.serviceAccountToken).toEqual('MY_TOKEN_1');
jest.useFakeTimers().setSystemTime(Date.now() + 2 * 60 * 1000); // advance time by 2min
const response2 = await authTranslator.decorateClusterDetailsWithAuth(cd);
expect(response2.serviceAccountToken).toEqual('MY_TOKEN_2');
jest.useFakeTimers().setSystemTime(Date.now() + 17 * 60 * 1000); // advance time by 17min
await expect(
authTranslator.decorateClusterDetailsWithAuth(cd),
).rejects.toThrow();
});
});
@@ -29,7 +29,7 @@ export class AzureIdentityKubernetesAuthTranslator
implements KubernetesAuthTranslator
{
private accessToken: AccessToken = { token: '', expiresOnTimestamp: 0 };
private newToken: Promise<string> | undefined;
private newTokenPromise: Promise<string> | undefined;
constructor(
private readonly logger: Logger,
@@ -49,15 +49,15 @@ export class AzureIdentityKubernetesAuthTranslator
}
private async getToken(): Promise<string> {
if (this.isTokenValid()) {
if (!this.tokenRequiresRefresh()) {
return this.accessToken.token;
}
if (!this.newToken) {
this.newToken = this.fetchNewToken();
if (!this.newTokenPromise) {
this.newTokenPromise = this.fetchNewToken();
}
return this.newToken;
return this.newTokenPromise;
}
private async fetchNewToken(): Promise<string> {
@@ -74,16 +74,24 @@ export class AzureIdentityKubernetesAuthTranslator
this.accessToken = newAccessToken;
} catch (err) {
this.logger.error('Unable to fetch Azure token', err);
// don't throw the error, so the existing token will be re-used until we're able to fetch a new token
// only throw the error if the token has already expired, otherwise re-use existing until we're able to fetch a new token
if (this.tokenExpired()) {
throw err;
}
}
this.newToken = undefined;
this.newTokenPromise = undefined;
return this.accessToken.token;
}
private isTokenValid(): boolean {
private tokenRequiresRefresh(): boolean {
// Set tokens to expire 15 minutes before its actual expiry time
const expiresOn = this.accessToken.expiresOnTimestamp - 15 * 60 * 1000;
return expiresOn >= Date.now();
return Date.now() >= expiresOn;
}
private tokenExpired(): boolean {
return Date.now() >= this.accessToken.expiresOnTimestamp;
}
}
@@ -29,19 +29,19 @@ describe('getKubernetesAuthTranslatorInstance', () => {
it('can return an auth translator for google auth', () => {
const authTranslator: KubernetesAuthTranslator =
sut.getKubernetesAuthTranslatorInstance(logger, 'google');
sut.getKubernetesAuthTranslatorInstance('google', { logger });
expect(authTranslator instanceof GoogleKubernetesAuthTranslator).toBe(true);
});
it('can return an auth translator for aws auth', () => {
const authTranslator: KubernetesAuthTranslator =
sut.getKubernetesAuthTranslatorInstance(logger, 'aws');
sut.getKubernetesAuthTranslatorInstance('aws', { logger });
expect(authTranslator instanceof AwsIamKubernetesAuthTranslator).toBe(true);
});
it('can return an auth translator for serviceAccount auth', () => {
const authTranslator: KubernetesAuthTranslator =
sut.getKubernetesAuthTranslatorInstance(logger, 'serviceAccount');
sut.getKubernetesAuthTranslatorInstance('serviceAccount', { logger });
expect(
authTranslator instanceof ServiceAccountKubernetesAuthTranslator,
).toBe(true);
@@ -49,13 +49,13 @@ describe('getKubernetesAuthTranslatorInstance', () => {
it('can return an auth translator for oidc auth', () => {
const authTranslator: KubernetesAuthTranslator =
sut.getKubernetesAuthTranslatorInstance(logger, 'oidc');
sut.getKubernetesAuthTranslatorInstance('oidc', { logger });
expect(authTranslator instanceof OidcKubernetesAuthTranslator).toBe(true);
});
it('throws an error when asked for an auth translator for an unsupported auth type', () => {
expect(() =>
sut.getKubernetesAuthTranslatorInstance(logger, 'linode'),
sut.getKubernetesAuthTranslatorInstance('linode', { logger }),
).toThrow(
'authProvider "linode" has no KubernetesAuthTranslator associated with it',
);
@@ -25,8 +25,10 @@ import { OidcKubernetesAuthTranslator } from './OidcKubernetesAuthTranslator';
export class KubernetesAuthTranslatorGenerator {
static getKubernetesAuthTranslatorInstance(
logger: Logger,
authProvider: string,
options: {
logger: Logger;
},
): KubernetesAuthTranslator {
switch (authProvider) {
case 'google': {
@@ -36,7 +38,7 @@ export class KubernetesAuthTranslatorGenerator {
return new AwsIamKubernetesAuthTranslator();
}
case 'azure': {
return new AzureIdentityKubernetesAuthTranslator(logger);
return new AzureIdentityKubernetesAuthTranslator(options.logger);
}
case 'serviceAccount': {
return new ServiceAccountKubernetesAuthTranslator();
@@ -293,8 +293,10 @@ export class KubernetesFanOutHandler {
this.authTranslators[provider] =
KubernetesAuthTranslatorGenerator.getKubernetesAuthTranslatorInstance(
this.logger,
provider,
{
logger: this.logger,
},
);
return this.authTranslators[provider];
}