fix(cli): IAM Policy changes not deploying with --hotswap-fallback #28185
Conversation
mrgrain
temporarily deployed
to
test-pipeline
GitHub Actions
Inactive
There was a problem hiding this comment.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.
aws-cdk-automation
added
the
pr/needs-cli-test-run
mrgrain
temporarily deployed
to
test-pipeline
GitHub Actions
Inactive
mrgrain
force-pushed
the
mrgrain/fix/iam-policy-ignored-in-hotswap
branch
from
c1543ff to
aca2ec8
Compare
mrgrain
had a problem deploying
to
test-pipeline
GitHub Actions
Failure
mrgrain
force-pushed
the
mrgrain/fix/iam-policy-ignored-in-hotswap
branch
from
aca2ec8 to
d61a1fc
Compare
mrgrain
temporarily deployed
to
test-pipeline
GitHub Actions
Inactive
| const deploymentLambda2Old = { | ||
| Type: 'AWS::Lambda::Function', | ||
| Role: { | ||
| 'Fn::GetAtt': [ | ||
| 'ServiceRole', | ||
| 'Arn', | ||
| ], | ||
| }, | ||
| }; | ||
|
|
||
| const deploymentLambda2New = { | ||
| Type: 'AWS::Lambda::Function', | ||
| Role: { | ||
| 'Fn::GetAtt': [ | ||
| 'ServiceRole2', | ||
| 'Arn', | ||
| ], | ||
| }, | ||
| }; | ||
|
|
There was a problem hiding this comment.
I believe this existing test was unintentionally relying on undefined behavior.
Previously, the stack under would receive a change that changes Policy2 from being a policy that is used exclusively by S3Deployment, to a "free-floating" policy. This was because policy2Old -> policy2New is changing the role from ServiceRole -> ServiceRole2. However ServiceRole2 is not used by the S3Deployment.
Without adjusting the test case, it now would have failed because this new "free-floating" policy would have been detected as non-hotswappable and also not skippable since it is not connected to an S3Deployment anymore.
I believe the intent of the test was to test the skipping (based on test case description and assertions) and this accidentally worked. So I adjusted the test case.
|
➡️ PR build request submitted to A maintainer must now check the pipeline and add the |
mrgrain
added
the
pr-linter/cli-integ-tested
aws-cdk-automation
removed
the
pr/needs-cli-test-run
mrgrain
added
the
pr-linter/exempt-integ-test
aws-cdk-automation
dismissed
their stale review
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
aws-cdk-automation
added
the
pr/needs-maintainer-review
| const deploymentLambda2Old = { | ||
| Type: 'AWS::Lambda::Function', | ||
| Role: { | ||
| 'Fn::GetAtt': [ | ||
| 'ServiceRole', | ||
| 'Arn', | ||
| ], | ||
| }, | ||
| }; | ||
|
|
||
| const deploymentLambda2New = { | ||
| Type: 'AWS::Lambda::Function', | ||
| Role: { | ||
| 'Fn::GetAtt': [ | ||
| 'ServiceRole2', | ||
| 'Arn', | ||
| ], | ||
| }, | ||
| }; | ||
|
|
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
aws-cdk-automation
removed
the
pr/needs-maintainer-review
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
…ws#28185) The hotswappable resource detectors failed to correctly identify `AWS::IAM::Policy` resources as not-hotswappable. When `--hotswap-fallback` was used and the only change to the stack was with `AWS::IAM::Policy`, this caused the deploy command to first report IAM changes, and then report `no changes` on the stack. <img width="1076" alt="image" src="https://github.com/aws/aws-cdk/assets/379814/d77320bc-fc8d-4b70-b710-2c28467d07e5"> ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
The hotswappable resource detectors failed to correctly identify
AWS::IAM::Policyresources as not-hotswappable.When
--hotswap-fallbackwas used and the only change to the stack was withAWS::IAM::Policy, this caused the deploy command to first report IAM changes, and then reportno changeson the stack.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license