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 error with non-existent policy for AttachRolePolicy operation #8615

Merged
merged 3 commits into from Jul 12, 2023

Conversation

dfangl
Copy link
Member

@dfangl dfangl commented Jul 4, 2023

Motivation

Currently, we do not properly verify the policy arn in the attach_*_policy methods, and have not tests for this behavior.

Depens on getmoto/moto#6482 , fixes #7934 .

Ext Test run (due to snapshot transform changes): https://github.com/localstack/localstack-ext/actions/runs/5522309201

Changes

  • Add tests for role/user attach policies and errors of the operation
  • Add verification of the policy arn
  • Depends on the linked moto PR for correct error handling for the correct arn but policy does not exist case.
  • Adds two more transform statements to iam_api transformer.

@dfangl dfangl changed the title Fix error with non-existent policy for AttachRolePolicy Fix error with non-existent policy for AttachRolePolicy operation Jul 4, 2023
@dfangl dfangl added the semver: patch Non-breaking changes which can be included in patch releases label Jul 4, 2023
@github-actions
Copy link

github-actions bot commented Jul 4, 2023

LocalStack Community integration with Pro

       2 files         2 suites   1h 29m 22s ⏱️
2 220 tests 1 885 ✔️ 335 💤 0
2 221 runs  1 885 ✔️ 336 💤 0

Results for commit de72fa9.

♻️ This comment has been updated with latest results.

@dfangl dfangl added this to the 2.2 milestone Jul 11, 2023
@coveralls
Copy link

Coverage Status

coverage: 82.197% (+0.007%) from 82.19% when pulling de72fa9 on fix-iam-key-error into 9fb6be1 on master.

Copy link
Member

@dominikschubert dominikschubert left a comment

Choose a reason for hiding this comment

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

LGTM!

@dfangl dfangl merged commit 0ff2710 into master Jul 12, 2023
27 checks passed
@dfangl dfangl deleted the fix-iam-key-error branch July 12, 2023 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: key error causes internal server exception rather than failing gracefully
3 participants