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.

Sign up for GitHub

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

Jump to bottom

fix: improve error message on custom policies validation failure #9862

Merged
merged 1 commit into from Mar 8, 2022
Merged

fix: improve error message on custom policies validation failure #9862

merged 1 commit into from Mar 8, 2022

Conversation

edwardfoyle
Copy link
Member

Description of changes

Improve error message when custom-policies.json validation fails. The message will now include the path with the failure message.

Issue #, if available

fixes #9861

Description of how you validated changes

Tested manually. Message used to look like:

✖ An error occurred when pushing the resources to the cloud
🛑 An error occurred during the push operation: Invalid custom IAM policies in the envvartest function.

    Edit <project-dir>/amplify/backend/function/envvartest/custom-policies.json to fix
    Learn more about custom IAM policies for function: https://docs.amplify.aws/cli/function/#access-existing-aws-resource-from-lambda-function
should be array

and now looks like:

🛑 envvartest function custom-policies.json failed validation:
- [0].Resource should be array
✖ An error occurred when pushing the resources to the cloud
🛑 An error occurred during the push operation: 
      Invalid custom IAM policies for envvartest function.
      See details above and fix errors in <project-dir>/amplify/backend/function/envvartest/custom-policies.json.
      Learn more about custom IAM policies: https://docs.amplify.aws/cli/function/#access-existing-aws-resource-from-lam

Checklist

  • PR description included
  • yarn test passes

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

@edwardfoyle edwardfoyle requested a review from a team as a code owner February 25, 2022 19:21
@lgtm-com
Copy link

lgtm-com bot commented Feb 25, 2022

This pull request introduces 1 alert when merging 850680a into c4b7487 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

sachscode
sachscode reviewed Feb 25, 2022
View reviewed changes
Copy link
Contributor

@sachscode sachscode left a comment

Choose a reason for hiding this comment

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

LGTM , just one question

packages/amplify-cli-core/src/customPoliciesUtils.ts
printer.error(`${resourceName} ${categoryName} custom-policies.json failed validation:`);
formatter.list((validatePolicy?.errors || []).map(err => `${err.dataPath} ${err.message}`));
throw new CustomPoliciesFormatError(`
Invalid custom IAM policies for ${resourceName} ${categoryName}.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need an e2e test fix, since out messages have changed ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I searched for the original string in our test suite and didn't get any hits, so hopefully we're good

sachscode
sachscode approved these changes Feb 25, 2022
View reviewed changes
Copy link
Contributor

@sachscode sachscode left a comment

Choose a reason for hiding this comment

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

LGTM

@edwardfoyle edwardfoyle merged commit 60e05f7 into aws-amplify:master Mar 8, 2022
@github-actions
Copy link

👋 Hi, this pull request was referenced in the v7.6.24 release!

Check out the release notes here https://github.com/aws-amplify/amplify-cli/releases/tag/v7.6.24.

@github-actions github-actions bot added the referenced-in-release Issues referenced in a published release changelog label Mar 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danielleadams danielleadams danielleadams approved these changes

@sachscode sachscode sachscode approved these changes

Assignees
No one assigned
Labels
referenced-in-release Issues referenced in a published release changelog
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unclear error message when there is a validation error in custom-policies.json
3 participants
@edwardfoyle @danielleadams @sachscode