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: detect deploymentType from Stack Tags #2116

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Amplifiyer
Copy link
Contributor

Problem

listBackends calls tryGetDeploymentType to load the entire stack metadata and then infer deployment type from it.

This creates issue if the stack is being created or deleted and the metadata is not available causing the fetchBackendOutput to throw causing the listBackends call to fail.

Issue number, if available:

Changes

Infer deploymentType from stack tags instead. If we can't we don't include that stack/backend in the result.

Corresponding docs PR, if applicable:

Validation

Checklist

  • If this PR includes a functional change to the runtime behavior of the code, I have added or updated automated test coverage for this change.
  • If this PR requires a change to the Project Architecture README, I have included that update in this PR.
  • If this PR requires a docs update, I have linked to that docs PR above.
  • If this PR modifies E2E tests, makes changes to resource provisioning, or makes SDK calls, I have run the PR checks with the run-e2e label set.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link

changeset-bot bot commented Oct 16, 2024

🦋 Changeset detected

Latest commit: eb32eb4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@aws-amplify/deployed-backend-client Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Amplifiyer Amplifiyer added the run-e2e Label that will include e2e tests in PR checks workflow label Oct 16, 2024
Comment on lines +157 to +163
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;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only functional change, rest are all test changes.

@@ -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 () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test was duplicate of the one above.

assert.equal(listStacksMockFn.mock.callCount(), 2);
});

void it('paginates listBackends when one page contains stacks, but it gets filtered due to sandbox deploymentType', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same, this test was duplicate of the one above.

@Amplifiyer Amplifiyer marked this pull request as ready for review October 16, 2024 15:48
@Amplifiyer Amplifiyer requested a review from a team as a code owner October 16, 2024 15:48
@Amplifiyer Amplifiyer requested review from johnpc, bzsurbhi and ArseniyKotov and removed request for johnpc October 16, 2024 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-e2e Label that will include e2e tests in PR checks workflow
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants