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

Extract AWS Account ID from IAM access key ids (again) #8138

Merged
merged 4 commits into from
Apr 24, 2023

Conversation

dfangl
Copy link
Member

@dfangl dfangl commented Apr 13, 2023

Reintroduces changes made with #7045

Motivation

We still need to properly extract the account id from the IAM access key id, and its already encoded properly in there.
On the first try, we encountered issues with users having real AWS credentials in their environment, which led to several problems, as suddenly they were using multi account.

Changes

  • Access key IDs returned by LocalStack return, if not explicitly disabled, start with an L instead of an A. So, for account 000000000000 a user access key would be LKIAQAAAAAAALILNVHFS instead of AKIAQAAAAAAALILNVHFS. This allows us to distinguish AWS credentials from LocalStack credentials, while leaving the rest of the access key id format identical.
  • These access key IDs are changed back to start with an A before moto processes them, as moto does a lookup by equality of the whole string

This depends on getmoto/moto#6210 for a bugfix affecting the tests.

@dfangl dfangl requested a review from thrau as a code owner April 13, 2023 14:20
@github-actions
Copy link

github-actions bot commented Apr 13, 2023

LocalStack Community integration with Pro

1 910 tests  +4   1 705 ✔️ +4   1h 11m 2s ⏱️ - 10m 55s
       2 suites ±0      205 💤 ±0 
       2 files   ±0          0 ±0 

Results for commit 67e98d0. ± Comparison against base commit 66dd118.

♻️ This comment has been updated with latest results.

Copy link
Member

@viren-nadkarni viren-nadkarni left a comment

Choose a reason for hiding this comment

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

LGTM!

I currently have #8130 open, but will re-release moto-ext with your patches :)

Edit: New moto-ext bump #8153

@localstack-bot localstack-bot force-pushed the release/v2.1 branch 11 times, most recently from f4b05b5 to 7384da3 Compare April 18, 2023 06:47
@dfangl dfangl changed the base branch from release/v2.1 to master April 18, 2023 11:01
@coveralls
Copy link

Coverage Status

Coverage: 82.003% (-0.002%) from 82.006% when pulling 67e98d0 on iam-account-extraction into 66dd118 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. Awesome we can now revert the revert 👍

@dfangl dfangl merged commit a4a2216 into master Apr 24, 2023
@dfangl dfangl deleted the iam-account-extraction branch April 24, 2023 12:31
@joebowbeer
Copy link

Why are 5 files changed to revert a change to 2 files? 😄

It looks like this adds a new config PARITY_AWS_ACCESS_KEY_ID

Is it possible now to force localstack to use the 0's default id in all cases and not complain about receiving a real "production" id?

It looks like PARITY_AWS_ACCESS_KEY_ID provides a way to use the real id and not complain about it...

Where are these different options documented?

@viren-nadkarni
Copy link
Member

@joebowbeer: Thanks for highlighting the missing docs, it is being added with localstack/docs#613

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants