Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: aws-sdk still used in EKS custom resources #26756

Merged
merged 3 commits into from
Aug 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
/* eslint-disable no-console */

// eslint-disable-next-line import/no-extraneous-dependencies
import { ResourceNotFoundException } from '@aws-sdk/client-eks';
// eslint-disable-next-line import/no-extraneous-dependencies
import * as aws from 'aws-sdk';
import * as EKS from '@aws-sdk/client-eks';
import { EksClient, ResourceEvent, ResourceHandler } from './common';
import { compareLoggingProps } from './compareLogging';
import { IsCompleteResponse, OnEventResponse } from '../../../custom-resources/lib/provider-framework/types';
Expand All @@ -19,16 +17,16 @@ export class ClusterResourceHandler extends ResourceHandler {
return this.physicalResourceId;
}

private readonly newProps: aws.EKS.CreateClusterRequest;
private readonly oldProps: Partial<aws.EKS.CreateClusterRequest>;
private readonly newProps: EKS.CreateClusterCommandInput;
private readonly oldProps: Partial<EKS.CreateClusterCommandInput>;

constructor(eks: EksClient, event: ResourceEvent) {
super(eks, event);

this.newProps = parseProps(this.event.ResourceProperties);
this.oldProps = event.RequestType === 'Update' ? parseProps(event.OldResourceProperties) : {};
// compare newProps and oldProps and update the newProps by appending disabled LogSetup if any
const compared: Partial<aws.EKS.CreateClusterRequest> = compareLoggingProps(this.oldProps, this.newProps);
const compared: Partial<EKS.CreateClusterCommandInput> = compareLoggingProps(this.oldProps, this.newProps);
this.newProps.logging = compared.logging;
}

Expand Down Expand Up @@ -71,7 +69,7 @@ export class ClusterResourceHandler extends ResourceHandler {
try {
await this.eks.deleteCluster({ name: this.clusterName });
} catch (e: any) {
if (!(e instanceof ResourceNotFoundException)) {
if (!(e instanceof EKS.ResourceNotFoundException)) {
throw e;
} else {
console.log(`cluster ${this.clusterName} not found, idempotently succeeded`);
Expand All @@ -90,7 +88,7 @@ export class ClusterResourceHandler extends ResourceHandler {
console.log('describeCluster returned:', JSON.stringify(resp, undefined, 2));
} catch (e: any) {
// see https://aws.amazon.com/blogs/developer/service-error-handling-modular-aws-sdk-js/
if (e instanceof ResourceNotFoundException) {
if (e instanceof EKS.ResourceNotFoundException) {
console.log('received ResourceNotFoundException, this means the cluster has been deleted (or never existed)');
return { IsComplete: true };
}
Expand Down Expand Up @@ -147,7 +145,7 @@ export class ClusterResourceHandler extends ResourceHandler {
}

if (updates.updateLogging || updates.updateAccess) {
const config: aws.EKS.UpdateClusterConfigRequest = {
const config: EKS.UpdateClusterConfigCommandInput = {
name: this.clusterName,
};
if (updates.updateLogging) {
Expand All @@ -158,9 +156,9 @@ export class ClusterResourceHandler extends ResourceHandler {
// https://awscli.amazonaws.com/v2/documentation/api/latest/reference/eks/update-cluster-config.html)
// will fail, therefore we take only the access fields explicitly
config.resourcesVpcConfig = {
endpointPrivateAccess: this.newProps.resourcesVpcConfig.endpointPrivateAccess,
endpointPublicAccess: this.newProps.resourcesVpcConfig.endpointPublicAccess,
publicAccessCidrs: this.newProps.resourcesVpcConfig.publicAccessCidrs,
endpointPrivateAccess: this.newProps.resourcesVpcConfig?.endpointPrivateAccess,
endpointPublicAccess: this.newProps.resourcesVpcConfig?.endpointPublicAccess,
publicAccessCidrs: this.newProps.resourcesVpcConfig?.publicAccessCidrs,
};
}
const updateResponse = await this.eks.updateClusterConfig(config);
Expand Down Expand Up @@ -241,7 +239,7 @@ export class ClusterResourceHandler extends ResourceHandler {
OpenIdConnectIssuer: cluster.identity?.oidc?.issuer?.substring(8) ?? '', // Strips off https:// from the issuer url

// We can safely return the first item from encryption configuration array, because it has a limit of 1 item
// https://docs.aws.amazon.com/eks/latest/APIReference/API_CreateCluster.html#AmazonEKS-CreateCluster-request-encryptionConfig
// https://docs.amazon.com/eks/latest/APIReference/API_CreateCluster.html#AmazonEKS-CreateCluster-request-encryptionConfig
EncryptionConfigKeyArn: cluster.encryptionConfig?.shift()?.provider?.keyArn ?? '',
},
};
Expand Down Expand Up @@ -283,7 +281,7 @@ export class ClusterResourceHandler extends ResourceHandler {
}
}

function parseProps(props: any): aws.EKS.CreateClusterRequest {
function parseProps(props: any): EKS.CreateClusterCommandInput {

const parsed = props?.Config ?? {};

Expand Down Expand Up @@ -317,7 +315,7 @@ interface UpdateMap {
updateAccess: boolean; // resourcesVpcConfig.endpointPrivateAccess and endpointPublicAccess
}

function analyzeUpdate(oldProps: Partial<aws.EKS.CreateClusterRequest>, newProps: aws.EKS.CreateClusterRequest): UpdateMap {
function analyzeUpdate(oldProps: Partial<EKS.CreateClusterCommandInput>, newProps: EKS.CreateClusterCommandInput): UpdateMap {
console.log('old props: ', JSON.stringify(oldProps));
console.log('new props: ', JSON.stringify(newProps));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,13 @@ export abstract class ResourceHandler {

export interface EksClient {
configureAssumeRole(request: sts.AssumeRoleCommandInput): void;
createCluster(request: _eks.CreateClusterCommandInput): Promise<_eks.CreateClusterResponse>;
deleteCluster(request: _eks.DeleteClusterCommandInput): Promise<_eks.DeleteClusterResponse>;
describeCluster(request: _eks.DescribeClusterCommandInput): Promise<_eks.DescribeClusterResponse>;
updateClusterConfig(request: _eks.UpdateClusterConfigCommandInput): Promise<_eks.UpdateClusterConfigResponse>;
updateClusterVersion(request: _eks.UpdateClusterVersionCommandInput): Promise<_eks.UpdateClusterVersionResponse>;
describeUpdate(req: _eks.DescribeUpdateCommandInput): Promise<_eks.DescribeUpdateResponse>;
createFargateProfile(request: _eks.CreateFargateProfileCommandInput): Promise<_eks.CreateFargateProfileResponse>;
describeFargateProfile(request: _eks.DescribeFargateProfileCommandInput): Promise<_eks.DescribeFargateProfileResponse>;
deleteFargateProfile(request: _eks.DeleteFargateProfileCommandInput): Promise<_eks.DeleteFargateProfileResponse>;
createCluster(request: _eks.CreateClusterCommandInput): Promise<_eks.CreateClusterCommandOutput>;
deleteCluster(request: _eks.DeleteClusterCommandInput): Promise<_eks.DeleteClusterCommandOutput>;
describeCluster(request: _eks.DescribeClusterCommandInput): Promise<_eks.DescribeClusterCommandOutput>;
updateClusterConfig(request: _eks.UpdateClusterConfigCommandInput): Promise<_eks.UpdateClusterConfigCommandOutput>;
updateClusterVersion(request: _eks.UpdateClusterVersionCommandInput): Promise<_eks.UpdateClusterVersionCommandOutput>;
describeUpdate(req: _eks.DescribeUpdateCommandInput): Promise<_eks.DescribeUpdateCommandOutput>;
createFargateProfile(request: _eks.CreateFargateProfileCommandInput): Promise<_eks.CreateFargateProfileCommandOutput>;
describeFargateProfile(request: _eks.DescribeFargateProfileCommandInput): Promise<_eks.DescribeFargateProfileCommandOutput>;
deleteFargateProfile(request: _eks.DeleteFargateProfileCommandInput): Promise<_eks.DeleteFargateProfileCommandOutput>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@
* @param newProps new properties
* @returns result with LogSet with enabled:false if any
*/
// eslint-disable-next-line import/no-extraneous-dependencies
import * as EKS from '@aws-sdk/client-eks';

export function compareLoggingProps(oldProps: Partial<AWS.EKS.CreateClusterRequest>,
newProps: Partial<AWS.EKS.CreateClusterRequest>): Partial<AWS.EKS.CreateClusterRequest> {
const result: Partial<AWS.EKS.CreateClusterRequest> = { logging: {} };
let enabledTypes: AWS.EKS.LogType[] = [];
let disabledTypes: AWS.EKS.LogType[] = [];
export function compareLoggingProps(oldProps: Partial<EKS.CreateClusterCommandInput>,
newProps: Partial<EKS.CreateClusterCommandInput>): Partial<EKS.CreateClusterCommandInput> {
const result: Partial<EKS.CreateClusterCommandInput> = { logging: {} };
let enabledTypes: (EKS.LogType | string)[] = [];
let disabledTypes: (EKS.LogType | string)[] = [];

if (newProps.logging?.clusterLogging === undefined && oldProps.logging?.clusterLogging === undefined) {
return newProps;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// eslint-disable-next-line import/no-extraneous-dependencies
import { ResourceNotFoundException } from '@aws-sdk/client-eks';
import * as aws from 'aws-sdk'; // eslint-disable-line import/no-extraneous-dependencies
import * as EKS from '@aws-sdk/client-eks';
import { ResourceHandler } from './common';

const MAX_NAME_LEN = 63;
Expand All @@ -9,7 +8,7 @@ export class FargateProfileResourceHandler extends ResourceHandler {
protected async onCreate() {
const fargateProfileName = this.event.ResourceProperties.Config.fargateProfileName ?? this.generateProfileName();

const createFargateProfile: aws.EKS.CreateFargateProfileRequest = {
const createFargateProfile: EKS.CreateFargateProfileCommandInput = {
fargateProfileName,
...this.event.ResourceProperties.Config,
};
Expand All @@ -35,7 +34,7 @@ export class FargateProfileResourceHandler extends ResourceHandler {
throw new Error('Cannot delete a profile without a physical id');
}

const deleteFargateProfile: aws.EKS.DeleteFargateProfileRequest = {
const deleteFargateProfile: EKS.DeleteFargateProfileCommandInput = {
clusterName: this.event.ResourceProperties.Config.clusterName,
fargateProfileName: this.physicalResourceId,
};
Expand Down Expand Up @@ -86,12 +85,12 @@ export class FargateProfileResourceHandler extends ResourceHandler {
* Queries the Fargate profile's current status and returns the status or
* NOT_FOUND if the profile doesn't exist (i.e. it has been deleted).
*/
private async queryStatus(): Promise<aws.EKS.FargateProfileStatus | 'NOT_FOUND' | undefined> {
private async queryStatus(): Promise<EKS.FargateProfileStatus | 'NOT_FOUND' | string | undefined> {
if (!this.physicalResourceId) {
throw new Error('Unable to determine status for fargate profile without a resource name');
}

const describeFargateProfile: aws.EKS.DescribeFargateProfileRequest = {
const describeFargateProfile: EKS.DescribeFargateProfileCommandInput = {
clusterName: this.event.ResourceProperties.Config.clusterName,
fargateProfileName: this.physicalResourceId,
};
Expand All @@ -109,7 +108,7 @@ export class FargateProfileResourceHandler extends ResourceHandler {

return status;
} catch (describeFargateProfileError: any) {
if (describeFargateProfileError instanceof ResourceNotFoundException) {
if (describeFargateProfileError instanceof EKS.ResourceNotFoundException) {
this.log('received ResourceNotFoundException, this means the profile has been deleted (or never existed)');
return 'NOT_FOUND';
}
Expand Down
37 changes: 21 additions & 16 deletions packages/aws-cdk-lib/aws-eks/test/cluster-resource-handler-mocks.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import * as eks from '@aws-sdk/client-eks';
import * as sts from '@aws-sdk/client-sts';
import * as sdk from 'aws-sdk';
import { EksClient } from '../lib/cluster-resource-handler/common';

/**
Expand All @@ -9,15 +8,15 @@ import { EksClient } from '../lib/cluster-resource-handler/common';
*/
export let actualRequest: {
configureAssumeRoleRequest?: sts.AssumeRoleRequest;
createClusterRequest?: eks.CreateClusterRequest;
describeClusterRequest?: eks.DescribeClusterRequest;
describeUpdateRequest?: eks.DescribeUpdateRequest;
deleteClusterRequest?: eks.DeleteClusterRequest;
updateClusterConfigRequest?: eks.UpdateClusterConfigRequest;
updateClusterVersionRequest?: eks.UpdateClusterVersionRequest;
createFargateProfile?: eks.CreateFargateProfileRequest;
describeFargateProfile?: eks.DescribeFargateProfileRequest;
deleteFargateProfile?: eks.DeleteFargateProfileRequest;
createClusterRequest?: eks.CreateClusterCommandInput;
describeClusterRequest?: eks.DescribeClusterCommandInput;
describeUpdateRequest?: eks.DescribeUpdateCommandInput;
deleteClusterRequest?: eks.DeleteClusterCommandInput;
updateClusterConfigRequest?: eks.UpdateClusterConfigCommandInput;
updateClusterVersionRequest?: eks.UpdateClusterVersionCommandInput;
createFargateProfile?: eks.CreateFargateProfileCommandInput;
describeFargateProfile?: eks.DescribeFargateProfileCommandInput;
deleteFargateProfile?: eks.DeleteFargateProfileCommandInput;
} = { };

/**
Expand All @@ -26,7 +25,7 @@ export let actualRequest: {
export let simulateResponse: {
describeClusterResponseMockStatus?: string;
describeUpdateResponseMockStatus?: string;
describeUpdateResponseMockErrors?: sdk.EKS.ErrorDetails;
describeUpdateResponseMockErrors?: eks.ErrorDetail[];
deleteClusterError?: Error;
describeClusterException?: Error;
} = { };
Expand All @@ -47,6 +46,7 @@ export const client: EksClient = {
createCluster: async req => {
actualRequest.createClusterRequest = req;
return {
$metadata: {},
cluster: {
name: req.name,
roleArn: req.roleArn,
Expand All @@ -64,6 +64,7 @@ export const client: EksClient = {
throw simulateResponse.deleteClusterError;
}
return {
$metadata: {},
cluster: {
name: req.name,
},
Expand All @@ -78,6 +79,7 @@ export const client: EksClient = {
}

return {
$metadata: {},
cluster: {
name: req.name,
version: '1.0',
Expand All @@ -94,6 +96,7 @@ export const client: EksClient = {
actualRequest.describeUpdateRequest = req;

return {
$metadata: {},
update: {
id: req.updateId,
errors: simulateResponse.describeUpdateResponseMockErrors,
Expand All @@ -105,6 +108,7 @@ export const client: EksClient = {
updateClusterConfig: async req => {
actualRequest.updateClusterConfigRequest = req;
return {
$metadata: {},
update: {
id: MOCK_UPDATE_STATUS_ID,
},
Expand All @@ -114,6 +118,7 @@ export const client: EksClient = {
updateClusterVersion: async req => {
actualRequest.updateClusterVersionRequest = req;
return {
$metadata: {},
update: {
id: MOCK_UPDATE_STATUS_ID,
},
Expand All @@ -122,17 +127,17 @@ export const client: EksClient = {

createFargateProfile: async req => {
actualRequest.createFargateProfile = req;
return { };
return { $metadata: {} };
},

describeFargateProfile: async req => {
actualRequest.describeFargateProfile = req;
return { };
return { $metadata: {} };
},

deleteFargateProfile: async req => {
actualRequest.deleteFargateProfile = req;
return { };
return { $metadata: {} };
},
};

Expand All @@ -148,8 +153,8 @@ export const MOCK_ASSUME_ROLE_ARN = 'assume:role:arn';

export function newRequest<T extends 'Create' | 'Update' | 'Delete'>(
requestType: T,
props?: Partial<sdk.EKS.CreateClusterRequest>,
oldProps?: Partial<sdk.EKS.CreateClusterRequest>) {
props?: Partial<eks.CreateClusterCommandInput>,
oldProps?: Partial<eks.CreateClusterCommandInput>) {
return {
StackId: 'fake-stack-id',
RequestId: 'fake-request-id',
Expand Down