-
Notifications
You must be signed in to change notification settings - Fork 62
feat: show early validation errors on deploy #970
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
base: main
Are you sure you want to change the base?
Conversation
Additional changes, done by hand: - Added `IMAGE_ARCHIVED` to `ScanStatus` to reflect the latest version from the SDK - `renderYargs` output now has an additional line break. Fix the test
packages/@aws-cdk/toolkit-lib/lib/api/deployments/early-validation.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/toolkit-lib/lib/api/deployments/early-validation.ts
Outdated
Show resolved
Hide resolved
| const failures = (eventsOutput.OperationEvents ?? []) | ||
| .filter((event) => event.ValidationStatus === ValidationStatus.FAILED) | ||
| .map((event) => ` - ${event.ValidationStatusReason} (at ${event.ValidationPath})`) | ||
| .join('\n'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might include events from multiple deployments, as long as they fit on one page.
We should only check events from the last deployment (perhaps even locate them based on an identifier, if that's feasible at all)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The toolkit deletes the old change set before creating a new one. And when a change set is deleted, all events are also deleted. So when a new one is created, and we are looking at its description here, it will contain only the events for this deployment.
packages/@aws-cdk/toolkit-lib/lib/api/deployments/early-validation.ts
Outdated
Show resolved
Hide resolved
# Conflicts: # packages/@aws-cdk/integ-runner/THIRD_PARTY_LICENSES # packages/aws-cdk/THIRD_PARTY_LICENSES # packages/aws-cdk/test/commands/init.test.ts # packages/cdk-assets/THIRD_PARTY_LICENSES
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #970 +/- ##
=======================================
Coverage 87.38% 87.39%
=======================================
Files 71 71
Lines 10010 10010
Branches 1311 1310 -1
=======================================
+ Hits 8747 8748 +1
+ Misses 1240 1239 -1
Partials 23 23
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
CloudFormation has recently launched early validation. When creating the change set, CloudFormation validates a few things such as whether a resource with that physical ID already exists. More validations to come. But the specific information about the error (other than it's an early validation error) is not returned by the
DescribeChangeSetAPI. The consumer is supposed to call a new API,DescribeEventsto get that information.Introduce a new class,
EarlyValidationReporterthat checks whether such an error occurred and call theDescribeChangeSetAPI. The bootstrap stack was also updated to include permission to this new API. If the user's bootstrap stack is older than that, throw a special error to instruct the user to re-bootstrap.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license