From 227905348a6035ce8137f0d2e8e94d4d5bca8601 Mon Sep 17 00:00:00 2001 From: Praveen Gupta Date: Wed, 16 Oct 2024 15:17:10 +0200 Subject: [PATCH 1/2] fix: detect deploymentType from Stack Tags --- .changeset/purple-dodos-kneel.md | 5 + .../src/deployed_backend_client.ts | 31 +--- ...d_client_list_delete_failed_stacks.test.ts | 140 ++---------------- ...oyed_backend_client_list_sandboxes.test.ts | 100 ++----------- 4 files changed, 36 insertions(+), 240 deletions(-) create mode 100644 .changeset/purple-dodos-kneel.md diff --git a/.changeset/purple-dodos-kneel.md b/.changeset/purple-dodos-kneel.md new file mode 100644 index 0000000000..71d24222b5 --- /dev/null +++ b/.changeset/purple-dodos-kneel.md @@ -0,0 +1,5 @@ +--- +'@aws-amplify/deployed-backend-client': patch +--- + +fix: detect deploymentType from Stack Tags diff --git a/packages/deployed-backend-client/src/deployed_backend_client.ts b/packages/deployed-backend-client/src/deployed_backend_client.ts index 94b18def4c..2ff69efe31 100644 --- a/packages/deployed-backend-client/src/deployed_backend_client.ts +++ b/packages/deployed-backend-client/src/deployed_backend_client.ts @@ -15,11 +15,7 @@ import { ListBackendsResponse, } from './deployed_backend_client_factory.js'; import { BackendIdentifierConversions } from '@aws-amplify/platform-core'; -import { - BackendOutputClient, - BackendOutputClientError, - BackendOutputClientErrorType, -} from './backend_output_client_factory.js'; +import { BackendOutputClient } from './backend_output_client_factory.js'; import { CloudFormationClient, DeleteStackCommand, @@ -158,26 +154,13 @@ export class DefaultDeployedBackendClient implements DeployedBackendClient { private tryGetDeploymentType = async ( stackSummary: StackSummary ): Promise => { - const backendIdentifier = { - stackName: stackSummary.StackName as string, - }; + const stackDescription = await this.cfnClient.send( + new DescribeStacksCommand({ StackName: stackSummary.StackName }) + ); - try { - const backendOutput: BackendOutput = - await this.backendOutputClient.getOutput(backendIdentifier); - - return backendOutput[platformOutputKey].payload - .deploymentType as DeploymentType; - } catch (error) { - if ( - (error as BackendOutputClientError).code === - BackendOutputClientErrorType.METADATA_RETRIEVAL_ERROR - ) { - // Ignore stacks where metadata cannot be retrieved. These are not Amplify stacks, or not compatible with this library. - return; - } - throw error; - } + return stackDescription.Stacks?.[0].Tags?.find( + (tag) => tag.Key === 'amplify:deployment-type' + )?.Value as DeploymentType; }; private listStacks = async ( diff --git a/packages/deployed-backend-client/src/deployed_backend_client_list_delete_failed_stacks.test.ts b/packages/deployed-backend-client/src/deployed_backend_client_list_delete_failed_stacks.test.ts index 96afbc73c2..116042ad1c 100644 --- a/packages/deployed-backend-client/src/deployed_backend_client_list_delete_failed_stacks.test.ts +++ b/packages/deployed-backend-client/src/deployed_backend_client_list_delete_failed_stacks.test.ts @@ -6,15 +6,9 @@ import { ListStacksCommand, StackStatus, } from '@aws-sdk/client-cloudformation'; -import { platformOutputKey } from '@aws-amplify/backend-output-schemas'; import { DefaultBackendOutputClient } from './backend_output_client.js'; import { DefaultDeployedBackendClient } from './deployed_backend_client.js'; import { BackendStatus } from './deployed_backend_client_factory.js'; -import { - BackendOutputClientError, - BackendOutputClientErrorType, - StackIdentifier, -} from './index.js'; import { AmplifyClient } from '@aws-sdk/client-amplify'; import { S3 } from '@aws-sdk/client-s3'; import { DeployedResourcesEnumerator } from './deployed-backend-client/deployed_resources_enumerator.js'; @@ -34,14 +28,6 @@ const listStacksMock = { ], }; -const getOutputMockResponse = { - [platformOutputKey]: { - payload: { - deploymentType: 'branch', - }, - }, -}; - void describe('Deployed Backend Client list delete failed stacks', () => { const mockCfnClient = new CloudFormation(); const mockS3Client = new S3(); @@ -56,9 +42,19 @@ void describe('Deployed Backend Client list delete failed stacks', () => { const matchingStack = listStacksMock.StackSummaries.find((stack) => { return stack.StackName === request.input.StackName; }); - const stack = matchingStack; + // Add tags that are used to detect deployment type return { - Stacks: [stack], + Stacks: [ + { + ...matchingStack, + Tags: [ + { + Key: 'amplify:deployment-type', + Value: 'branch', + }, + ], + }, + ], }; } throw request; @@ -83,23 +79,6 @@ void describe('Deployed Backend Client list delete failed stacks', () => { mockCfnClient, new AmplifyClient() ); - const getOutputMock = mock.method( - mockBackendOutputClient, - 'getOutput', - (backendIdentifier: StackIdentifier) => { - if (backendIdentifier.stackName === 'amplify-test-not-a-sandbox') { - return { - ...getOutputMockResponse, - [platformOutputKey]: { - payload: { - deploymentType: 'branch', - }, - }, - }; - } - return getOutputMockResponse; - } - ); const returnedDeleteFailedStacks = [ { deploymentType: 'branch', @@ -116,7 +95,6 @@ void describe('Deployed Backend Client list delete failed stacks', () => { ]; beforeEach(() => { - getOutputMock.mock.resetCalls(); listStacksMockFn.mock.resetCalls(); cfnClientSendMock.mock.resetCalls(); const deployedResourcesEnumerator = new DeployedResourcesEnumerator( @@ -171,98 +149,4 @@ void describe('Deployed Backend Client list delete failed stacks', () => { assert.equal(listStacksMockFn.mock.callCount(), 2); }); - - void it('paginates listBackends when one page contains stacks, but it gets filtered due to not deleted failed status', async () => { - listStacksMockFn.mock.mockImplementationOnce(() => { - return { - StackSummaries: [], - NextToken: 'abc', - }; - }); - const failedStacks = deployedBackendClient.listBackends({ - deploymentType: 'branch', - backendStatusFilters: [BackendStatus.DELETE_FAILED], - }); - assert.deepEqual( - (await failedStacks.getBackendSummaryByPage().next()).value, - returnedDeleteFailedStacks - ); - - assert.equal(listStacksMockFn.mock.callCount(), 2); - }); - - void it('paginates listBackends when one page contains stacks, but it gets filtered due to sandbox deploymentType', async () => { - listStacksMockFn.mock.mockImplementationOnce(() => { - return { - StackSummaries: [], - NextToken: 'abc', - }; - }); - const failedStacks = deployedBackendClient.listBackends({ - deploymentType: 'branch', - backendStatusFilters: [BackendStatus.DELETE_FAILED], - }); - assert.deepEqual( - (await failedStacks.getBackendSummaryByPage().next()).value, - returnedDeleteFailedStacks - ); - - assert.equal(listStacksMockFn.mock.callCount(), 2); - }); - - void it('paginates listBackends when one page contains a stack, but it gets filtered due to not having gen2 outputs', async () => { - getOutputMock.mock.mockImplementationOnce(() => { - throw new BackendOutputClientError( - BackendOutputClientErrorType.METADATA_RETRIEVAL_ERROR, - 'Test metadata retrieval error' - ); - }); - listStacksMockFn.mock.mockImplementationOnce(() => { - return { - StackSummaries: [ - { - StackName: 'amplify-123-name-branch-testHash', - StackStatus: StackStatus.DELETE_FAILED, - CreationTime: new Date(0), - LastUpdatedTime: new Date(1), - }, - ], - NextToken: 'abc', - }; - }); - const failedStacks = deployedBackendClient.listBackends({ - deploymentType: 'branch', - backendStatusFilters: [BackendStatus.DELETE_FAILED], - }); - assert.deepEqual( - (await failedStacks.getBackendSummaryByPage().next()).value, - returnedDeleteFailedStacks - ); - - assert.equal(listStacksMockFn.mock.callCount(), 2); - }); - - void it('does not paginate listBackends when one page throws an unexpected error fetching gen2 outputs', async () => { - getOutputMock.mock.mockImplementationOnce(() => { - throw new Error('Unexpected Error!'); - }); - listStacksMockFn.mock.mockImplementationOnce(() => { - return { - StackSummaries: [ - { - StackName: 'amplify-123-name-branch-testHash', - StackStatus: StackStatus.DELETE_FAILED, - CreationTime: new Date(0), - LastUpdatedTime: new Date(1), - }, - ], - NextToken: 'abc', - }; - }); - const listBackendsPromise = deployedBackendClient.listBackends({ - deploymentType: 'branch', - backendStatusFilters: [BackendStatus.DELETE_FAILED], - }); - await assert.rejects(listBackendsPromise.getBackendSummaryByPage().next()); - }); }); diff --git a/packages/deployed-backend-client/src/deployed_backend_client_list_sandboxes.test.ts b/packages/deployed-backend-client/src/deployed_backend_client_list_sandboxes.test.ts index 12a9e024f9..c1a8ee8bb5 100644 --- a/packages/deployed-backend-client/src/deployed_backend_client_list_sandboxes.test.ts +++ b/packages/deployed-backend-client/src/deployed_backend_client_list_sandboxes.test.ts @@ -7,14 +7,8 @@ import { StackStatus, } from '@aws-sdk/client-cloudformation'; import { BackendDeploymentStatus } from './deployed_backend_client_factory.js'; -import { platformOutputKey } from '@aws-amplify/backend-output-schemas'; import { DefaultBackendOutputClient } from './backend_output_client.js'; import { DefaultDeployedBackendClient } from './deployed_backend_client.js'; -import { - BackendOutputClientError, - BackendOutputClientErrorType, - StackIdentifier, -} from './index.js'; import { AmplifyClient } from '@aws-sdk/client-amplify'; import { S3 } from '@aws-sdk/client-s3'; import { DeployedResourcesEnumerator } from './deployed-backend-client/deployed_resources_enumerator.js'; @@ -34,14 +28,6 @@ const listStacksMock = { ], }; -const getOutputMockResponse = { - [platformOutputKey]: { - payload: { - deploymentType: 'sandbox', - }, - }, -}; - void describe('Deployed Backend Client list sandboxes', () => { const mockCfnClient = new CloudFormation(); const mockS3Client = new S3(); @@ -56,9 +42,18 @@ void describe('Deployed Backend Client list sandboxes', () => { const matchingStack = listStacksMock.StackSummaries.find((stack) => { return stack.StackName === request.input.StackName; }); - const stack = matchingStack; return { - Stacks: [stack], + Stacks: [ + { + ...matchingStack, + Tags: [ + { + Key: 'amplify:deployment-type', + Value: 'sandbox', + }, + ], + }, + ], }; } throw request; @@ -84,23 +79,7 @@ void describe('Deployed Backend Client list sandboxes', () => { mockCfnClient, new AmplifyClient() ); - const getOutputMock = mock.method( - mockBackendOutputClient, - 'getOutput', - (backendIdentifier: StackIdentifier) => { - if (backendIdentifier.stackName === 'amplify-test-not-a-sandbox') { - return { - ...getOutputMockResponse, - [platformOutputKey]: { - payload: { - deploymentType: 'branch', - }, - }, - }; - } - return getOutputMockResponse; - } - ); + const returnedSandboxes = [ { deploymentType: 'sandbox', @@ -117,7 +96,6 @@ void describe('Deployed Backend Client list sandboxes', () => { ]; beforeEach(() => { - getOutputMock.mock.resetCalls(); listStacksMockFn.mock.resetCalls(); cfnClientSendMock.mock.resetCalls(); const deployedResourcesEnumerator = new DeployedResourcesEnumerator( @@ -208,58 +186,4 @@ void describe('Deployed Backend Client list sandboxes', () => { assert.equal(listStacksMockFn.mock.callCount(), 2); }); - - void it('paginates listBackends when one page contains a stack, but it gets filtered due to not having gen2 outputs', async () => { - getOutputMock.mock.mockImplementationOnce(() => { - throw new BackendOutputClientError( - BackendOutputClientErrorType.METADATA_RETRIEVAL_ERROR, - 'Test metadata retrieval error' - ); - }); - listStacksMockFn.mock.mockImplementationOnce(() => { - return { - StackSummaries: [ - { - StackName: 'amplify-test-name-sandbox-testHash', - StackStatus: StackStatus.CREATE_COMPLETE, - CreationTime: new Date(0), - LastUpdatedTime: new Date(1), - }, - ], - NextToken: 'abc', - }; - }); - const sandboxes = deployedBackendClient.listBackends({ - deploymentType: 'sandbox', - }); - assert.deepEqual( - (await sandboxes.getBackendSummaryByPage().next()).value, - returnedSandboxes - ); - - assert.equal(listStacksMockFn.mock.callCount(), 2); - }); - - void it('does not paginate listBackends when one page throws an unexpected error fetching gen2 outputs', async () => { - getOutputMock.mock.mockImplementationOnce(() => { - throw new Error('Unexpected Error!'); - }); - listStacksMockFn.mock.mockImplementationOnce(() => { - return { - StackSummaries: [ - { - StackName: 'amplify-test-name-sandbox-testHash', - StackStatus: StackStatus.CREATE_COMPLETE, - CreationTime: new Date(0), - LastUpdatedTime: new Date(1), - }, - ], - NextToken: 'abc', - }; - }); - const listBackendsPromise = deployedBackendClient.listBackends({ - deploymentType: 'sandbox', - }); - await assert.rejects(listBackendsPromise.getBackendSummaryByPage().next()); - }); }); From eb32eb4b15d6d4fa071b5b46c2bf2051661b7fb5 Mon Sep 17 00:00:00 2001 From: Praveen Gupta Date: Wed, 16 Oct 2024 15:47:35 +0200 Subject: [PATCH 2/2] add another test --- ...oyed_backend_client_list_sandboxes.test.ts | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/packages/deployed-backend-client/src/deployed_backend_client_list_sandboxes.test.ts b/packages/deployed-backend-client/src/deployed_backend_client_list_sandboxes.test.ts index c1a8ee8bb5..fb8d690037 100644 --- a/packages/deployed-backend-client/src/deployed_backend_client_list_sandboxes.test.ts +++ b/packages/deployed-backend-client/src/deployed_backend_client_list_sandboxes.test.ts @@ -186,4 +186,37 @@ void describe('Deployed Backend Client list sandboxes', () => { assert.equal(listStacksMockFn.mock.callCount(), 2); }); + + void it('filter stacks that do not have deploymentType tag in it', async () => { + cfnClientSendMock.mock.mockImplementation( + (request: ListStacksCommand | DescribeStacksCommand) => { + if (request instanceof ListStacksCommand) { + return listStacksMockFn(request.input); + } + if (request instanceof DescribeStacksCommand) { + const matchingStack = listStacksMock.StackSummaries.find((stack) => { + return stack.StackName === request.input.StackName; + }); + return { + Stacks: [ + { + ...matchingStack, + Tags: [], + }, + ], + }; + } + throw request; + } + ); + const sandboxes = deployedBackendClient.listBackends({ + deploymentType: 'sandbox', + }); + assert.deepEqual( + (await sandboxes.getBackendSummaryByPage().next()).done, + true + ); + + assert.equal(listStacksMockFn.mock.callCount(), 1); + }); });