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.

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

Handle STS Credentials Region #267

Merged
merged 15 commits into from
Sep 12, 2023
Merged

Handle STS Credentials Region #267

merged 15 commits into from
Sep 12, 2023

Conversation

ben-taussig-solo
Copy link
Contributor

@ben-taussig-solo ben-taussig-solo commented Aug 22, 2023 •
edited by nfuden
Loading

Description

  • Partially resolves an issue where assuming a secondary role using an EKS ServiceAccount would fail when the region of the EKS cluster/OIDC provider used to authenticate the assumeRole request was not us-east-1
    • Prior to this PR, all STS requests were directed to sts.us-east-1.amazonaws.com
    • When a user has configured IAM Roles for EKS ServiceAccounts, the OIDC provider associated with that cluster will be scoped to the same region that the cluster is in. Subsequent IAM requests made after assuming credentials from that provider must be sent to the AWS STS endpoint that matches the region of the cluster, i.e. sts.us-east-2.amazonaws.com for a cluster/OIDC provider in us-east-2
    • For this reason, users with EKS clusters outside of us-east-1 were unable to use the Gloo Edge IRSA role-chaining workflow before these changes after aws shored up its cross region access rights
  • The Gloo OSS PR here provides helm changes that, along with the changes in this PR resolve this issue

@solo-changelog-bot
Copy link

Issues linked to changelog:
solo-io/gloo#8578

@ben-taussig-solo ben-taussig-solo mentioned this pull request Aug 22, 2023
Merged
@ben-taussig-solo
Copy link
Contributor Author

/kick build instantly failed

@ben-taussig-solo
Copy link
Contributor Author

I made a change here: e11ac28 which handles the error seen in the issue when we receive a response from AWS, with testing here: c48ddb6

looking for input as to whether I should add an explicit log message in this case

@soloio-bulldozer soloio-bulldozer bot merged commit 1d40d7b into main Sep 12, 2023
2 checks passed
@soloio-bulldozer soloio-bulldozer bot deleted the handle-sts-credentials-region branch September 12, 2023 19:56
ben-taussig-solo added a commit that referenced this pull request Sep 13, 2023
@ben-taussig-solo
Handle STS Credentials Region (#267)
2434651 
* add new field to ServiceAccountCredentials for sts region

* support new field

* support new setting in tests

* add changelog entry

* remove typo ffrom sts_credentials_provider_test.cc

* updae connection pool test

* update lambda integration test

* remove stray log messages

* relocate changelog entry

* allow region to be unset

* log credential scope mismatch on failure

* add testing against credentialscopemismatch

* update proto comment for new field
@ben-taussig-solo ben-taussig-solo mentioned this pull request Sep 13, 2023
Merged
ben-taussig-solo added a commit that referenced this pull request Sep 13, 2023
@ben-taussig-solo
Handle STS Credentials Region (#267)
1d0bcd2 
* add new field to ServiceAccountCredentials for sts region

* support new field

* support new setting in tests

* add changelog entry

* remove typo ffrom sts_credentials_provider_test.cc

* updae connection pool test

* update lambda integration test

* remove stray log messages

* relocate changelog entry

* allow region to be unset

* log credential scope mismatch on failure

* add testing against credentialscopemismatch

* update proto comment for new field
@ben-taussig-solo ben-taussig-solo mentioned this pull request Sep 13, 2023
Merged
soloio-bulldozer bot pushed a commit that referenced this pull request Sep 13, 2023
@ben-taussig-solo
[v1.23] Backport Handle STS Credentials Region (#267) (#270)
6fc4dcd 
* Handle STS Credentials Region (#267)

* add new field to ServiceAccountCredentials for sts region

* support new field

* support new setting in tests

* add changelog entry

* remove typo ffrom sts_credentials_provider_test.cc

* updae connection pool test

* update lambda integration test

* remove stray log messages

* relocate changelog entry

* allow region to be unset

* log credential scope mismatch on failure

* add testing against credentialscopemismatch

* update proto comment for new field

* correct typo in sts_credentials_provider_test.cc
soloio-bulldozer bot pushed a commit that referenced this pull request Sep 13, 2023
@ben-taussig-solo
[v1.25] Backport Handle STS Credentials Region (#267) (#269)
bf024d9 
* add new field to ServiceAccountCredentials for sts region

* support new field

* support new setting in tests

* add changelog entry

* remove typo ffrom sts_credentials_provider_test.cc

* updae connection pool test

* update lambda integration test

* remove stray log messages

* relocate changelog entry

* allow region to be unset

* log credential scope mismatch on failure

* add testing against credentialscopemismatch

* update proto comment for new field
@ben-taussig-solo ben-taussig-solo mentioned this pull request Sep 28, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nfuden nfuden nfuden approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ben-taussig-solo @nfuden