Skip to content

Commit

Permalink
chore: add aws-sdk as dev dependency for unit tests and satisfy `Aw…
Browse files Browse the repository at this point in the history 
…sSdkCall` in unit tests (#29860)

### Reason for this change

#29648 introduced a change to the `AwsSdkCall` representation used in the v2 and v3 handler code. Our handler unit tests use `satisfies` to validate that the event object satisfies `AwsSdkCall`. All unit tests and the build still pass, but the linter calls out that the event object doesn't actually satisfy `AwsSdkCall`.

#29845 removed the dependency `@aws-cdk/custom-resource-handlers` had on `aws-sdk`. We should add this as devDependency since we're using `aws-sdk` in v2 handler mocks.

### Description of changes

I added `logApiResponseData` property to the event objects being tested to make the event satisfy `AwsSdkCall`. I added `aws-sdk` as a dev dependency. We will remove this as part of the v2 handler removal.

### 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
@colifran
colifran authored Apr 17, 2024
1 parent 39755fd commit 41d2c52
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 2 deletions.
3 changes: 2 additions & 1 deletion packages/@aws-cdk/custom-resource-handlers/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@
"sinon": "^9.2.4",
"nock": "^13.5.4",
"fs-extra": "^11.2.0",
"esbuild": "^0.20.2"
"esbuild": "^0.20.2",
"aws-sdk": "^2.1596.0"
},
"dependencies": {
"@aws-cdk/asset-node-proxy-agent-v6": "^2.0.3",
Expand Down
11 changes: 11 additions & 0 deletions ...ers/test/custom-resources/aws-custom-resource/aws-custom-resource-provider-sdk-v2.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ test('create event with physical resource id path', async () => {
Bucket: 'my-bucket',
},
physicalResourceId: { responsePath: 'Contents.1.ETag' },
logApiResponseData: true,
} satisfies AwsSdkCall),
},
};
Expand Down Expand Up @@ -107,6 +108,7 @@ test('update event with physical resource id', async () => {
TopicArn: 'topicarn',
},
physicalResourceId: { id: 'topicarn' },
logApiResponseData: true,
} satisfies AwsSdkCall),
},
};
Expand Down Expand Up @@ -139,6 +141,7 @@ test('delete event', async () => {
Bucket: 'my-bucket',
},
physicalResourceId: { responsePath: 'Contents.1.ETag' },
logApiResponseData: true,
} satisfies AwsSdkCall),
},
};
Expand Down Expand Up @@ -173,6 +176,7 @@ test('delete event with Delete call and no physical resource id in call', async
parameters: {
Name: 'my-param',
},
logApiResponseData: true,
} satisfies AwsSdkCall),
},
};
Expand Down Expand Up @@ -207,6 +211,7 @@ test('create event with Delete call only', async () => {
parameters: {
Name: 'my-param',
},
logApiResponseData: true,
} satisfies AwsSdkCall),
},
};
Expand Down Expand Up @@ -243,6 +248,7 @@ test('catch errors', async () => {
},
physicalResourceId: { id: 'physicalResourceId' },
ignoreErrorCodesMatching: 'NoSuchBucket',
logApiResponseData: true,
} satisfies AwsSdkCall),
},
};
Expand Down Expand Up @@ -287,6 +293,7 @@ test('restrict output path', async () => {
},
physicalResourceId: { id: 'id' },
outputPath: 'Contents.0',
logApiResponseData: true,
} satisfies AwsSdkCall),
},
};
Expand Down Expand Up @@ -332,6 +339,7 @@ test('restrict output paths', async () => {
},
physicalResourceId: { id: 'id' },
outputPaths: ['Contents.0.Key', 'Contents.1.Key'],
logApiResponseData: true,
} satisfies AwsSdkCall),
},
};
Expand Down Expand Up @@ -370,6 +378,7 @@ test('can specify apiVersion and region', async () => {
apiVersion: '2010-03-31',
region: 'eu-west-1',
physicalResourceId: { id: 'id' },
logApiResponseData: true,
} satisfies AwsSdkCall),
},
};
Expand Down Expand Up @@ -413,6 +422,7 @@ test('installs the latest SDK', async () => {
TopicArn: 'topic',
},
physicalResourceId: { id: 'id' },
logApiResponseData: true,
} satisfies AwsSdkCall),
InstallLatestAwsSdk: 'true',
},
Expand Down Expand Up @@ -452,6 +462,7 @@ test('invalid service name throws explicit error', async () => {
TopicArn: 'topic',
},
physicalResourceId: { id: 'id' },
logApiResponseData: true,
} satisfies AwsSdkCall),
},
};
Expand Down
5 changes: 5 additions & 0 deletions ...om-resource-handlers/test/custom-resources/aws-custom-resource/aws-sdk-v2-handler.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable import/no-extraneous-dependencies */
import * as AWS from 'aws-sdk';
import { v2handler as handler } from '../../../lib/custom-resources/aws-custom-resource-handler';
import { AwsSdkCall } from '../../../lib/custom-resources/aws-custom-resource-handler/construct-types';
Expand Down Expand Up @@ -57,6 +58,7 @@ test('SDK global credentials are never set', async () => {
},
physicalResourceId: { id: 'id' },
service: 'SSM',
logApiResponseData: true,
} satisfies AwsSdkCall),
ServiceToken: 'serviceToken',
},
Expand Down Expand Up @@ -89,6 +91,7 @@ test('SDK credentials are not persisted across subsequent invocations', async ()
},
physicalResourceId: { id: 'id' },
service: 'SSM',
logApiResponseData: true,
} satisfies AwsSdkCall),
ServiceToken: 'serviceToken',
},
Expand All @@ -111,6 +114,7 @@ test('SDK credentials are not persisted across subsequent invocations', async ()
},
physicalResourceId: { id: 'id' },
service: 'SSM',
logApiResponseData: true,
} satisfies AwsSdkCall),
ServiceToken: 'serviceToken',
},
Expand All @@ -132,6 +136,7 @@ test('SDK credentials are not persisted across subsequent invocations', async ()
},
physicalResourceId: { id: 'id' },
service: 'SSM',
logApiResponseData: true,
} satisfies AwsSdkCall),
ServiceToken: 'serviceToken',
},
Expand Down
19 changes: 18 additions & 1 deletion ...om-resource-handlers/test/custom-resources/aws-custom-resource/aws-sdk-v3-handler.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
/* eslint-disable import/no-extraneous-dependencies */
process.env.AWS_REGION = 'us-east-1';

import { CloudWatchClient, GetMetricDataCommand } from '@aws-sdk/client-cloudwatch';
import { EncryptCommand, KMSClient } from '@aws-sdk/client-kms';
import * as S3 from '@aws-sdk/client-s3';
import { CloudWatchClient, GetMetricDataCommand } from '@aws-sdk/client-cloudwatch';
import { mockClient } from 'aws-sdk-client-mock';
import * as fs from 'fs-extra';
import * as nock from 'nock';
Expand Down Expand Up @@ -108,6 +108,7 @@ test('create event with physical resource id path', async () => {
Bucket: 'my-bucket',
},
physicalResourceId: { responsePath: 'Contents.1.ETag' },
logApiResponseData: true,
} satisfies AwsSdkCall),
},
};
Expand Down Expand Up @@ -145,6 +146,7 @@ test('update event with physical resource id', async () => {
Key: 'key',
},
physicalResourceId: { id: 'key' },
logApiResponseData: true,
} satisfies AwsSdkCall),
},
};
Expand Down Expand Up @@ -175,6 +177,7 @@ test('delete event', async () => {
Bucket: 'my-bucket',
},
physicalResourceId: { responsePath: 'Contents.1.ETag' },
logApiResponseData: true,
} satisfies AwsSdkCall),
},
};
Expand Down Expand Up @@ -209,6 +212,7 @@ test('delete event with Delete call and no physical resource id in call', async
Bucket: 'my-bucket',
Key: 'my-object',
},
logApiResponseData: true,
} satisfies AwsSdkCall),
},
};
Expand Down Expand Up @@ -244,6 +248,7 @@ test('create event with Delete call only', async () => {
Bucket: 'my-bucket',
Key: 'my-object',
},
logApiResponseData: true,
} satisfies AwsSdkCall),
},
};
Expand Down Expand Up @@ -279,6 +284,7 @@ test('catch errors - name property', async () => {
},
physicalResourceId: { id: 'physicalResourceId' },
ignoreErrorCodesMatching: 'NoSuchBucket',
logApiResponseData: true,
} satisfies AwsSdkCall),
},
};
Expand Down Expand Up @@ -316,6 +322,7 @@ test('catch errors - constructor name', async () => {
},
physicalResourceId: { id: 'physicalResourceId' },
ignoreErrorCodesMatching: 'S3ServiceException',
logApiResponseData: true,
} satisfies AwsSdkCall),
},
};
Expand Down Expand Up @@ -358,6 +365,7 @@ test('restrict output path', async () => {
},
physicalResourceId: { id: 'id' },
outputPath: 'Contents.0',
logApiResponseData: true,
} satisfies AwsSdkCall),
},
};
Expand Down Expand Up @@ -401,6 +409,7 @@ test('restrict output paths', async () => {
},
physicalResourceId: { id: 'id' },
outputPaths: ['Contents.0.Key', 'Contents.1.Key'],
logApiResponseData: true,
} satisfies AwsSdkCall),
},
};
Expand Down Expand Up @@ -437,6 +446,7 @@ test('can specify apiVersion and region', async () => {
apiVersion: '2010-03-31',
region: 'eu-west-1',
physicalResourceId: { id: 'id' },
logApiResponseData: true,
} satisfies AwsSdkCall),
},
};
Expand Down Expand Up @@ -480,6 +490,7 @@ test('installs the latest SDK', async () => {
Key: 'key',
},
physicalResourceId: { id: 'id' },
logApiResponseData: true,
} satisfies AwsSdkCall),
InstallLatestAwsSdk: 'true',
},
Expand Down Expand Up @@ -521,6 +532,7 @@ test('falls back to installed sdk if installation fails', async () => {
Key: 'key',
},
physicalResourceId: { id: 'id' },
logApiResponseData: true,
} satisfies AwsSdkCall),
InstallLatestAwsSdk: 'true',
},
Expand Down Expand Up @@ -635,6 +647,7 @@ test('Being able to call the AWS SDK v2 format', async () => {
Bucket: 'foo',
Key: 'bar',
},
logApiResponseData: true,
} satisfies AwsSdkCall),
},
};
Expand Down Expand Up @@ -669,6 +682,7 @@ test('invalid v3 package name throws explicit error', async () => {
Key: 'key',
},
physicalResourceId: { id: 'id' },
logApiResponseData: true,
} satisfies AwsSdkCall),
},
};
Expand Down Expand Up @@ -699,6 +713,7 @@ test('invalid v2 service name throws explicit error', async () => {
Key: 'key',
},
physicalResourceId: { id: 'id' },
logApiResponseData: true,
} satisfies AwsSdkCall),
},
};
Expand Down Expand Up @@ -733,6 +748,7 @@ test('automatic Uint8Array conversion when necessary', async () => {
KeyId: 'key-id',
Plaintext: 'dummy-data',
},
logApiResponseData: true,
} satisfies AwsSdkCall),
},
}, {} as AWSLambda.Context);
Expand Down Expand Up @@ -792,6 +808,7 @@ test('automatic Date conversion when necessary', async () => {
StartTime: new Date('2023-01-01'),
EndTime: new Date('2023-01-02'),
},
logApiResponseData: true,
} satisfies AwsSdkCall),
},
}, {} as AWSLambda.Context);
Expand Down

0 comments on commit 41d2c52

Please sign in to comment.