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(custom-resource-handlers): better fallback for require failures #30469

Closed
wants to merge 22 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
d678efc
Fix AWS SDK fallback
glitchassassin May 6, 2024
aa55b7d
Fixing tests
glitchassassin May 7, 2024
034f582
Cleaning up debugging
glitchassassin May 7, 2024
5aac71e
Simplifying require logic
glitchassassin May 7, 2024
ef15e7d
updating integration tests
glitchassassin May 13, 2024
77bbb00
update integ tests
pahud May 23, 2024
ad5687f
Merge pull request #2 from pahud/pahud-fix
glitchassassin May 24, 2024
814dc1d
Updating integration tests
glitchassassin May 28, 2024
637364c
more integration tests
glitchassassin May 28, 2024
edd79db
fixing bucket-auto-delete-objects test
glitchassassin May 28, 2024
84bad7e
updating integration tests
glitchassassin May 28, 2024
30e59ed
more integration tests
glitchassassin May 29, 2024
26a472e
opensearch integration tests
glitchassassin May 29, 2024
15a0b97
fixing remaining integration tests
glitchassassin May 29, 2024
f234e00
Merge branch 'main' of github.com:aws/aws-cdk into patch-1
glitchassassin Jun 6, 2024
c585290
Merge branch 'main' of github.com:aws/aws-cdk into patch-1
glitchassassin Jun 13, 2024
606d0a2
updating some integration tests
glitchassassin Jun 13, 2024
85318ee
Merge branch 'main' of github.com:aws/aws-cdk into patch-1
glitchassassin Jun 24, 2024
fd9c56a
updating integration snapshots
glitchassassin Jun 25, 2024
e257509
Merge branch 'main' of github.com:aws/aws-cdk into patch-1
glitchassassin Aug 23, 2024
39ac239
Replacing with latest from aws/main branch
glitchassassin Aug 23, 2024
5c0960f
Merge branch 'main' into patch-1
glitchassassin Aug 28, 2024
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
Expand Up @@ -40,34 +40,33 @@ interface AwsSdk {
async function loadAwsSdk(
packageName: string,
installLatestAwsSdk?: 'true' | 'false',
) {
let awsSdk: AwsSdk;
): Promise<AwsSdk> {
try {
if (!installedSdk[packageName] && installLatestAwsSdk === 'true') {
// Try to install the latest version
try {
installLatestSdk(packageName);
// MUST use require here. Dynamic import() do not support importing from directories
// esbuild-disable unsupported-require-call -- not esbuildable but that's fine
awsSdk = require(`/tmp/node_modules/${packageName}`);
} catch (e) {
console.log(`Failed to install latest AWS SDK v3. Falling back to pre-installed version. Error: ${e}`);
// MUST use require as dynamic import() does not support importing from directories
// esbuild-disable unsupported-require-call -- not esbuildable but that's fine
return require(packageName); // Fallback to pre-installed version
}
}

} else if (installedSdk[packageName]) {
// MUST use require here. Dynamic import() do not support importing from directories
// esbuild-disable unsupported-require-call -- not esbuildable but that's fine
awsSdk = require(`/tmp/node_modules/${packageName}`);
} else {
// esbuild-disable unsupported-require-call -- not esbuildable but that's fine
awsSdk = require(packageName);
if (installedSdk[packageName]) {
// Try to load the installed version
try {
// MUST use require here. Dynamic import() do not support importing from directories
// esbuild-disable unsupported-require-call -- not esbuildable but that's fine
return require(`/tmp/node_modules/${packageName}`);
} catch (e) {
console.log(`Failed to load latest AWS SDK v3. Falling back to pre-installed version. Error: ${e}`);
}
}
// MUST use require as dynamic import() does not support importing from directories
// esbuild-disable unsupported-require-call -- not esbuildable but that's fine
return require(packageName); // Fallback to pre-installed version
} catch (error) {
throw Error(`Package ${packageName} does not exist.`);
}
return awsSdk;
}

/* eslint-disable @typescript-eslint/no-require-imports, import/no-extraneous-dependencies */
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* eslint-disable import/no-extraneous-dependencies */
process.env.AWS_REGION = 'us-east-1';

import { execSync } from 'child_process';
import { CloudWatchClient, GetMetricDataCommand } from '@aws-sdk/client-cloudwatch';
import { EncryptCommand, KMSClient } from '@aws-sdk/client-kms';
import * as S3 from '@aws-sdk/client-s3';
Expand All @@ -16,14 +17,12 @@ import 'aws-sdk-client-mock-jest' ;
// 5s timeout
jest.setTimeout(60_000);

const mockExecSync = jest.fn();
const mockExecSync = jest.mocked(execSync);
jest.mock('child_process', () => {
return jest.fn().mockImplementation(() => {
return {
...(jest.requireActual('child_process')),
execSync: mockExecSync,
};
});
return {
...(jest.requireActual('child_process')),
execSync: jest.fn(),
};
});

beforeEach(() => {
Expand Down Expand Up @@ -493,6 +492,9 @@ test('installs the latest SDK', async () => {
// Now remove the symlink and let the handler install it
await fs.unlink(tmpPath);

// unmock execSync for this test only
mockExecSync.mockImplementationOnce(jest.requireActual('child_process').execSync);

localS3MockClient.on(localAwsSdk.GetObjectCommand).resolves({});

const event: AWSLambda.CloudFormationCustomResourceCreateEvent = {
Expand Down Expand Up @@ -523,6 +525,9 @@ test('installs the latest SDK', async () => {

expect(request.isDone()).toBeTruthy();

// require cache may succeed even if the path doesn't exist,
// so we check both
expect(await fs.pathExists(tmpPath)).toBeTruthy();
expect(() => require.resolve(tmpPath)).not.toThrow();

// clean up aws-sdk install
Expand Down Expand Up @@ -565,6 +570,68 @@ test('falls back to installed sdk if installation fails', async () => {
expect(request.isDone()).toBeTruthy();
});

test('falls back to installed sdk if require fails', async () => {
s3MockClient.on(S3.GetObjectCommand).resolves({});

const firstEvent: AWSLambda.CloudFormationCustomResourceCreateEvent = {
...eventCommon,
RequestType: 'Create',
ResourceProperties: {
ServiceToken: 'token',
Create: JSON.stringify({
service: '@aws-sdk/client-s3',
action: 'GetObjectCommand',
parameters: {
Bucket: 'my-bucket',
Key: 'key',
},
physicalResourceId: { id: 'id' },
logApiResponseData: true,
} satisfies AwsSdkCall),
InstallLatestAwsSdk: 'true',
},
};

// handler will invoke execSync mock to install @aws-sdk/client-s3
// the mock will not throw an error, so it will mark it as installed
// the require will fail, because the mock didn't *actually* install
// the package, and will fall back to the pre-installed version
createRequest(body =>
body.Status === 'SUCCESS',
);
await handler(firstEvent, {} as AWSLambda.Context);

const secondEvent: AWSLambda.CloudFormationCustomResourceCreateEvent = {
...eventCommon,
RequestType: 'Create',
ResourceProperties: {
ServiceToken: 'token',
Create: JSON.stringify({
service: '@aws-sdk/client-s3',
action: 'GetObjectCommand',
parameters: {
Bucket: 'my-bucket',
Key: 'key',
},
physicalResourceId: { id: 'id' },
logApiResponseData: true,
} satisfies AwsSdkCall),
InstallLatestAwsSdk: 'true',
},
};

// second handler will see that install had been successful, but its
// require will fail and fall back to the pre-installed version
const request = createRequest(body =>
body.Status === 'SUCCESS',
);
await handler(secondEvent, {} as AWSLambda.Context);

// Reset to 'false' so that the next run will reinstall aws-sdk
forceSdkInstallation();
expect(request.isDone()).toBeTruthy();
});

test('SDK credentials are not persisted across subsequent invocations', async () => {
// GIVEN
s3MockClient.on(S3.GetObjectCommand).resolves({});
Expand Down
Loading