Skip to content

Commit

Permalink
fix: aws-sdk still used in EKS custom resources (#26756)
Browse files Browse the repository at this point in the history 
Removes usage of aws-sdk in eks custom resources. The remaining usage was only type references that appear to be forward compatible but this cleans up the code and makes it possible to remove aws-sdk as a dev dependency to aws-cdk-lib once the rout53 cross account zone delegation handler is updated.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
@MrArnoldPalmer
MrArnoldPalmer committed Aug 18, 2023
1 parent 52b43fc commit e78e355
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 52 deletions.
28 changes: 13 additions & 15 deletions packages/aws-cdk-lib/aws-eks/lib/cluster-resource-handler/cluster.ts
View file
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
18 changes: 9 additions & 9 deletions packages/aws-cdk-lib/aws-eks/lib/cluster-resource-handler/common.ts
View file
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>;
}
12 changes: 7 additions & 5 deletions packages/aws-cdk-lib/aws-eks/lib/cluster-resource-handler/compareLogging.ts
View file
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
13 changes: 6 additions & 7 deletions packages/aws-cdk-lib/aws-eks/lib/cluster-resource-handler/fargate.ts
View file
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
View file
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

0 comments on commit e78e355

Please sign in to comment.