8000 Add tests to test urlencoded IAM policies by dfangl · Pull Request #11580 · localstack/localstack · GitHub
[go: up one dir, main page]
Skip to content

Add tests to test urlencoded IAM policies #11580

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
Merged
merged 3 commits into from
Nov 4, 2024
Merged

Add tests to test urlencoded IAM policies #11580

merged 3 commits into from
Nov 4, 2024

Conversation

dfangl
Copy link
Member
@dfangl dfangl commented Sep 25, 2024 •
edited
Loading

Motivation

During investigation of an IAM issue, I noticed our IAM implementation not returning exactly the same policies as submitted.

This happens, when parts of the policy are already urlencoded, and the response removes this encoding, returning the actual value.
This behavior however does not match AWS, which does return the policies as they were submitted.

A fix for this behavior is in getmoto/moto#8157, this PR only contains AWS validated tests for this behavior. The tests will fail, as long as the PR in moto is not merged and moto updated in LocalStack.

Changes

  • Add AWS validated tests to verify the policies being unchanged, even if they contain urlencoded parts

@dfangl dfangl added the semver: patch Non-breaking changes which can be included in patch releases label Sep 25, 2024
@dfangl dfangl self-assigned this Sep 25, 2024
@dfangl dfangl added this to the 3.8 milestone Sep 25, 2024
@github-actions GitHub Actions
Copy link
github-actions bot commented Sep 25, 2024 •
edited
Loading

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 43m 7s ⏱️ -38s
3 512 tests +3  3 099 ✅ +3  413 💤 ±0  0 ❌ ±0 
3 514 runs  +3  3 099 ✅ +3  415 💤 ±0  0 ❌ ±0 

Results for commit bfb5d55. ± Comparison against base commit 3a54f21.

♻️ This comment has been updated with latest results.

@dfangl dfangl modified the milestones: 3.8, 4.0 Sep 30, 2024
dominikschubert
dominikschubert approved these changes Oct 16, 2024
View reviewed changes
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! Tests should also now pass here after a rebase since we've upgraded our moto version since then 👍

@dfangl
Copy link
Member Author
dfangl commented Oct 21, 2024

We need to wait for #11710 before rebasing and merging 👍

@dfangl dfangl force-pushed the iam/policy-encoding branch from 2162ee1 to bfb5d55 Compare October 22, 2024 10:45
pinzon
pinzon approved these changes Oct 30, 2024
View reviewed changes
Copy link
Member
@pinzon pinzon left a comment

Choose a reason for hiding this comment

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

The PR you were waiting for is merged. Maybe is time to merge this one too 👍

@dfangl dfangl merged commit 1db9b8f into master Nov 4, 2024
34 checks passed
@dfangl dfangl deleted the iam/policy-encoding branch November 4, 2024 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dominikschubert dominikschubert dominikschubert approved these changes

@pinzon pinzon pinzon approved these changes

@steffyP steffyP Awaiting requested review from steffyP steffyP is a code owner

Assignees

@dfangl dfangl

Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Milestone
4.0
Development

Successfully merging this pull request may close these issues.
3 participants
@dfangl @dominikschubert @pinzon
0