Skip to content

Commit

Permalink
chore(custom-resource-handlers): lint (#33204)
Browse files Browse the repository at this point in the history
The linter in the custom-resource-handlers folder was never turned on, resulting in typos, inconsistencies, and potentially bugs. This PR does not fix any potential bugs. I have tracked them in separate issues.

There's more work to be done with the generated files that we are not linting right now, but this is better than nothing

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
kaizencc authored Jan 28, 2025
1 parent c76f668 commit 93df62a
Show file tree
Hide file tree
Showing 35 changed files with 117 additions and 85 deletions.
4 changes: 4 additions & 0 deletions packages/@aws-cdk/custom-resource-handlers/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
const baseConfig = require('@aws-cdk/cdk-build-tools/config/eslintrc');
baseConfig.parserOptions.project = __dirname + '/tsconfig.dev.json';
baseConfig.ignorePatterns = [
...baseConfig.ignorePatterns || [],
'test/custom-resources-framework/expected/**', // ignore generated files
];
baseConfig.rules['import/no-extraneous-dependencies'] = [
'error',
{
Expand Down
25 changes: 22 additions & 3 deletions packages/@aws-cdk/custom-resource-handlers/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,37 @@

This package contains the following custom resource handlers:

### Stable:
## Stable

- aws-s3/auto-delete-objects-handler
- aws-certificatemanager/dns-validated-certificate-handler
- aws-cloudfront/edge-function
- aws-dynamodb/replica-handler
- aws-ec2/restrict-default-security-group-handler
- aws-ecr/auto-delete-images-handler
- aws-ecs/lambda-source
- aws-eks/custom-resource-handler
- aws-eks/kubectl-handler
- aws-events-targets/aws-api-handler
- aws-iam/oidc-handler
- aws-logs/log-retention-handler
- aws-route53/cross-account-zone-delegation-handler
- aws-route53/delete-existing-record-set-handler
- aws-s3/auto-delete-objects-handler
- aws-s3/notifications-resource-handler
- aws-s3-deployment/bucket-deployment-handler
- aws-ses/drop-spam-handler
- aws-stepfunctions-tasks/cross-region-aws-sdk-handler
- aws-stepfunctions-tasks/eval-nodejs-handler
- aws-stepfunctions-tasks/role-policy-handler
- aws-synthetics/auto-delete-underlying-resources-handler
- custom-resources/aws-custom-resource-handler
- pipelines/approve-lambda
- triggers/lambda

These handlers are copied into `aws-cdk-lib/custom-resource-handlers` at build time
and included as part of the `aws-cdk-lib` package.

### Experimental:
## Experimental

- aws-amplify-alpha/asset-deployment-handler
- aws-redshift-alpha/asset-deployment-handler
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export interface IsCompleteResponse {
* Additional/changes to resource attributes.
*/
readonly Data?: { [name: string]: any };
};
}

export abstract class ResourceHandler {
protected readonly requestId: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export async function autoDeleteHandler(event: AWSLambda.CloudFormationCustomRes
case 'Delete':
return onDelete(event.ResourceProperties?.RepositoryName);
}
};
}

async function onUpdate(event: AWSLambda.CloudFormationCustomResourceEvent) {
const updateEvent = event as AWSLambda.CloudFormationCustomResourceUpdateEvent;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,14 +208,14 @@ export class ClusterResourceHandler extends ResourceHandler {
};
if (updates.updateLogging) {
config.logging = this.newProps.logging;
};
}
if (updates.updateAccess) {
config.resourcesVpcConfig = {
endpointPrivateAccess: this.newProps.resourcesVpcConfig?.endpointPrivateAccess,
endpointPublicAccess: this.newProps.resourcesVpcConfig?.endpointPublicAccess,
publicAccessCidrs: this.newProps.resourcesVpcConfig?.publicAccessCidrs,
};
};
}

if (updates.updateAuthMode) {
// the update path must be
Expand Down Expand Up @@ -259,7 +259,7 @@ export class ClusterResourceHandler extends ResourceHandler {
throw e;
}
config.accessConfig = this.newProps.accessConfig;
};
}

if (updates.updateVpc) {
config.resourcesVpcConfig = {
Expand Down Expand Up @@ -481,7 +481,7 @@ function getTagsToUpdate<T extends Record<string, string>>(oldTags: T, newTags:

function getTagsToRemove<T extends Record<string, string>>(oldTags: T, newTags: T): string[] {
const missingKeys: string[] = [];
//Get all tag keys to remove
// Get all tag keys to remove
for (const key in oldTags) {
if (oldTags.hasOwnProperty(key) && !newTags.hasOwnProperty(key)) {
missingKeys.push(key);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
export const CLUSTER_RESOURCE_TYPE = 'Custom::AWSCDK-EKS-Cluster';
export const FARGATE_PROFILE_RESOURCE_TYPE = 'Custom::AWSCDK-EKS-FargateProfile';
export const FARGATE_PROFILE_RESOURCE_TYPE = 'Custom::AWSCDK-EKS-FargateProfile';
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ async function rebootClusterIfRequired(clusterId: string, parameterGroupName: st
await redshift.rebootCluster({ ClusterIdentifier: clusterId });
} catch (err: any) {
if (err.name === 'InvalidClusterStateFault') {
return await executeActionForStatus(status, 30000);
return executeActionForStatus(status, 30000);
} else {
throw err;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ export type CrossAccountZoneDelegationEvent = AWSLambda.CloudFormationCustomReso
}

interface ResourceProperties {
AssumeRoleArn: string,
ParentZoneName?: string,
ParentZoneId?: string,
DelegatedZoneName: string,
DelegatedZoneNameServers: string[],
TTL: number,
AssumeRoleRegion?: string,
AssumeRoleArn: string;
ParentZoneName?: string;
ParentZoneId?: string;
DelegatedZoneName: string;
DelegatedZoneNameServers: string[];
TTL: number;
AssumeRoleRegion?: string;
}

export async function handler(event: CrossAccountZoneDelegationEvent) {
Expand Down Expand Up @@ -129,4 +129,4 @@ function route53Region(region: string) {

// Default for commercial partition
return 'us-east-1';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export async function autoDeleteHandler(event: AWSLambda.CloudFormationCustomRes
case 'Delete':
return onDelete(event.ResourceProperties?.BucketName);
}
};
}

async function onUpdate(event: AWSLambda.CloudFormationCustomResourceEvent) {
const updateEvent = event as AWSLambda.CloudFormationCustomResourceUpdateEvent;
Expand All @@ -30,8 +30,8 @@ async function onUpdate(event: AWSLambda.CloudFormationCustomResourceEvent) {
/* If the name of the bucket has changed, CloudFormation will try to delete the bucket
and create a new one with the new name. Returning a PhysicalResourceId that differs
from the event's PhysicalResourceId will trigger a `Delete` event for the custom
resource, note that this is default CFN behaviour. The `Delete` event will trigger
`onDelete` function which will empty the content of the bucket and then proceed to
resource, note that this is default CFN behaviour. The `Delete` event will trigger
`onDelete` function which will empty the content of the bucket and then proceed to
delete the bucket. */
return { PhysicalResourceId: newBucketName };
}
Expand All @@ -51,6 +51,8 @@ async function denyWrites(bucketName: string) {
Principal: '*',
Effect: 'Deny',
Action: ['s3:PutObject'],
// TODO: this is probably an error of some sort
// eslint-disable-next-line @cdklabs/no-literal-partition
Resource: [`arn:aws:s3:::${bucketName}/*`],
},
);
Expand Down Expand Up @@ -85,7 +87,7 @@ async function emptyBucket(bucketName: string) {

const records = contents.map((record) => ({ Key: record.Key, VersionId: record.VersionId }));
await s3.deleteObjects({ Bucket: bucketName, Delete: { Objects: records } });
} while (listedObjects?.IsTruncated)
} while (listedObjects?.IsTruncated);
}

async function onDelete(bucketName?: string) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export async function autoDeleteHandler(event: AWSLambda.CloudFormationCustomRes
case 'Delete':
return onDelete(event.ResourceProperties?.CanaryName);
}
};
}

async function onUpdate(event: AWSLambda.CloudFormationCustomResourceEvent) {
const updateEvent = event as AWSLambda.CloudFormationCustomResourceUpdateEvent;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
/*eslint-disable no-console*/
/* eslint-disable no-console*/
/* eslint-disable import/no-extraneous-dependencies */
import { SSM } from '@aws-sdk/client-ssm';

import { ExportReaderCRProps, CrossRegionExports } from '../types';
// Must use a require() otherwise esbuild complains
// eslint-disable-next-line @typescript-eslint/no-require-imports
const pLimit: typeof import('p-limit') = require('p-limit');
import { ExportReaderCRProps, CrossRegionExports } from '../types';

export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent) {
const props: ExportReaderCRProps = event.ResourceProperties.ReaderProps;
Expand Down Expand Up @@ -44,7 +43,7 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent
return {
Data: imports,
};
};
}

/**
* Add tag to parameters for existing exports
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*eslint-disable no-console*/
/* eslint-disable no-console*/
/* eslint-disable import/no-extraneous-dependencies */
import { SSM } from '@aws-sdk/client-ssm';
import { CrossRegionExports, ExportWriterCRProps } from '../types';
Expand Down Expand Up @@ -53,12 +53,14 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent
console.error('Error processing event: ', e);
throw e;
}
};
}

/**
* Create parameters for existing exports
*/
async function putParameters(ssm: SSM, parameters: CrossRegionExports): Promise<void> {
// This linter exemption could be wrong. It is added into enable linting after it was turned off for some time
// eslint-disable-next-line @cdklabs/promiseall-no-unbounded-parallelism
await Promise.all(Array.from(Object.entries(parameters), ([name, value]) => {
return ssm.putParameter({
Name: name,
Expand Down Expand Up @@ -94,6 +96,8 @@ async function deleteParameters(ssm: SSM, names: string[]) {
*/
async function throwIfAnyInUse(ssm: SSM, parameters: CrossRegionExports): Promise<void> {
const tagResults: Map<string, Set<string>> = new Map();
// This linter exemption could be wrong. It is added into enable linting after it was turned off for some time
// eslint-disable-next-line @cdklabs/promiseall-no-unbounded-parallelism
await Promise.all(Object.keys(parameters).map(async (name: string) => {
const result = await isInUse(ssm, name);
if (result.size > 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { Lambda, InvocationResponse, InvocationType } from '@aws-sdk/client-lamb
import { NodeHttpHandler } from '@smithy/node-http-handler';

export type DecodedInvocationResponse = Omit<InvocationResponse, 'Payload'> & {
Payload?: string
Payload?: string;
}

export type InvokeFunction = (functionName: string, invocationType: InvocationType, timeout: number) => Promise<DecodedInvocationResponse>;
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/custom-resource-handlers/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"private": true,
"version": "0.0.0",
"scripts": {
"build": "tsc -b && node scripts/generate.js",
"build": "tsc -b && node scripts/generate.js && cdk-lint",
"integ": "integ-runner --language javascript",
"lint": "cdk-lint",
"package": "cdk-package",
Expand Down Expand Up @@ -48,7 +48,7 @@
"@types/jest": "^29.5.14",
"aws-sdk-client-mock": "4.1.0",
"aws-sdk-client-mock-jest": "4.1.0",
"@cdklabs/typewriter": "^0.0.3",
"@cdklabs/typewriter": "^0.0.5",
"jest": "^29.7.0",
"sinon": "^9.2.4",
"nock": "^13.5.5",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ describe('handler', () => {
branchName: 'branchNameValue',
maxResults: 1,
});
expect(listJobsRequest).toBeCalled();
expect(listJobsRequest).toHaveBeenCalled();
expect(mockGetSignedUrlResponse).toHaveBeenCalledWith(mockS3, {
Bucket: 's3BucketNameValue',
Key: 's3ObjectKeyValue',
Expand All @@ -103,7 +103,7 @@ describe('handler', () => {
branchName: 'branchNameValue',
sourceUrl: 'signedUrlValue',
});
expect(startDeploymentRequest).toBeCalled();
expect(startDeploymentRequest).toHaveBeenCalled();
});

it('onEvent CREATE pending job', async () => {
Expand Down Expand Up @@ -137,7 +137,7 @@ describe('handler', () => {
branchName: 'branchNameValue',
maxResults: 1,
});
expect(listJobsRequest).toBeCalled();
expect(listJobsRequest).toHaveBeenCalled();
expect(mockGetSignedUrlResponse).not.toHaveBeenCalled();
expect(startDeploymentRequest).not.toHaveBeenCalled();
expect(startDeploymentRequest).not.toHaveBeenCalled();
Expand Down Expand Up @@ -184,7 +184,7 @@ describe('handler', () => {
branchName: 'branchNameValue',
jobId: 'amplifyJobIdValue',
});
expect(getJobRequest).toBeCalled();
expect(getJobRequest).toHaveBeenCalled();
});

it('isComplete CREATE pending', async () => {
Expand Down Expand Up @@ -224,7 +224,7 @@ describe('handler', () => {
branchName: 'branchNameValue',
jobId: 'amplifyJobIdValue',
});
expect(getJobRequest).toBeCalled();
expect(getJobRequest).toHaveBeenCalled();
});

it('isComplete CREATE failed', async () => {
Expand Down Expand Up @@ -259,7 +259,7 @@ describe('handler', () => {
branchName: 'branchNameValue',
jobId: 'amplifyJobIdValue',
});
expect(getJobRequest).toBeCalled();
expect(getJobRequest).toHaveBeenCalled();
});

it('isComplete CREATE cancelled', async () => {
Expand Down Expand Up @@ -295,7 +295,7 @@ describe('handler', () => {
branchName: 'branchNameValue',
jobId: 'amplifyJobIdValue',
});
expect(getJobRequest).toBeCalled();
expect(getJobRequest).toHaveBeenCalled();
});

it('isComplete CREATE no JobId', async () => {
Expand Down Expand Up @@ -375,7 +375,7 @@ describe('handler', () => {
branchName: 'branchNameValue',
maxResults: 1,
});
expect(listJobsRequest).toBeCalled();
expect(listJobsRequest).toHaveBeenCalled();
expect(mockGetSignedUrlResponse).toHaveBeenCalledWith(mockS3, {
Bucket: 's3BucketNameValue',
Key: 's3ObjectKeyValue',
Expand All @@ -385,7 +385,7 @@ describe('handler', () => {
branchName: 'branchNameValue',
sourceUrl: 'signedUrlValue',
});
expect(startDeploymentRequest).toBeCalled();
expect(startDeploymentRequest).toHaveBeenCalled();
});

it('onEvent UPDATE pending job', async () => {
Expand Down Expand Up @@ -422,7 +422,7 @@ describe('handler', () => {
branchName: 'branchNameValue',
maxResults: 1,
});
expect(listJobsRequest).toBeCalled();
expect(listJobsRequest).toHaveBeenCalled();
expect(mockGetSignedUrlResponse).not.toHaveBeenCalled();
expect(startDeploymentRequest).not.toHaveBeenCalled();
expect(startDeploymentRequest).not.toHaveBeenCalled();
Expand Down Expand Up @@ -471,7 +471,7 @@ describe('handler', () => {
branchName: 'branchNameValue',
jobId: 'amplifyJobIdValue',
});
expect(getJobRequest).toBeCalled();
expect(getJobRequest).toHaveBeenCalled();
});

it('isComplete UPDATE pending', async () => {
Expand Down Expand Up @@ -513,7 +513,7 @@ describe('handler', () => {
branchName: 'branchNameValue',
jobId: 'amplifyJobIdValue',
});
expect(getJobRequest).toBeCalled();
expect(getJobRequest).toHaveBeenCalled();
});

it('isComplete UPDATE failed', async () => {
Expand Down Expand Up @@ -550,7 +550,7 @@ describe('handler', () => {
branchName: 'branchNameValue',
jobId: 'amplifyJobIdValue',
});
expect(getJobRequest).toBeCalled();
expect(getJobRequest).toHaveBeenCalled();
});

it('isComplete UPDATE cancelled', async () => {
Expand Down Expand Up @@ -588,7 +588,7 @@ describe('handler', () => {
branchName: 'branchNameValue',
jobId: 'amplifyJobIdValue',
});
expect(getJobRequest).toBeCalled();
expect(getJobRequest).toHaveBeenCalled();
});

it('isComplete UPDATE no JobId', async () => {
Expand Down
Loading

0 comments on commit 93df62a

Please sign in to comment.