Skip to content

aws: STS WebIdentity credentials provider#31004

Merged
mattklein123 merged 14 commits intoenvoyproxy:mainfrom
suniltheta:aws_sts
Dec 6, 2023
Merged

aws: STS WebIdentity credentials provider#31004
mattklein123 merged 14 commits intoenvoyproxy:mainfrom
suniltheta:aws_sts

Conversation

@suniltheta
Copy link
Copy Markdown
Contributor

@suniltheta suniltheta commented Nov 22, 2023

Co-authored-by: Scott LaVigne lavignes@amazon.com

Commit Message: aws: STS WebIdentity credentials provider
Additional Description: Envoy support for k8s IAM Roles for Service Accounts aws/aws-app-mesh-roadmap#182. This is an AWS internal patch that is upstreamed after updating it to adopt to use http async client way of fetching metadata credentials.
Note: This change enables SSL on curl dependency which is on deprecation path #30731. The SSL on curl is required to establish https connection with STS service.

Risk Level: Low
Testing: Unit testing
Docs Changes: Yes
Release Notes: Yes
Platform Specific Features: NA

Sunil Narasimhamurthy added 3 commits November 20, 2023 04:24
Signed-off-by: Sunil Narasimhamurthy <sunnrs@amazon.com>
Signed-off-by: Sunil Narasimhamurthy <sunnrs@amazon.com>
Signed-off-by: Sunil Narasimhamurthy <sunnrs@amazon.com>
Signed-off-by: Sunil Narasimhamurthy <sunnrs@amazon.com>
@suniltheta
Copy link
Copy Markdown
Contributor Author

Looks like can't enable SSL on curl without failing windows build for reasons mentioned in

# SSL (via Envoy's SSL dependency) is disabled, curl's CMake uses
# FindOpenSSL.cmake which fails at what looks like version parsing
# (the libraries are found ok).

So no option but to remove the code path that would enable WebIdentity credentials provider for deprecated curl path.

Sunil Narasimhamurthy added 2 commits November 22, 2023 05:30
Signed-off-by: Sunil Narasimhamurthy <sunnrs@amazon.com>
… disabled at first

Signed-off-by: Sunil Narasimhamurthy <sunnrs@amazon.com>
Sunil Narasimhamurthy added 2 commits November 22, 2023 07:31
Signed-off-by: Sunil Narasimhamurthy <sunnrs@amazon.com>
Signed-off-by: Sunil Narasimhamurthy <sunnrs@amazon.com>
@KBaichoo
Copy link
Copy Markdown
Contributor

Can you take a look as codeowner @lavignes?

Thank you

/assign @lavignes

Signed-off-by: Sunil Narasimhamurthy <sunnrs@amazon.com>
Sunil Narasimhamurthy added 2 commits November 27, 2023 03:21
* Fix STS json response credentials expiration format which is integer and not string
* Sanitize the path before logging, which might contain sensitive info
* Properly log cluster type
* Fix garbage value in uri when adding new cluster

Signed-off-by: Sunil Narasimhamurthy <sunnrs@amazon.com>
Signed-off-by: Sunil Narasimhamurthy <sunnrs@amazon.com>
@suniltheta
Copy link
Copy Markdown
Contributor Author

Has merge conflicts with #31135. Once the other change is merged I will update this change and will be good for review.

Sunil Narasimhamurthy added 3 commits December 1, 2023 23:31
Signed-off-by: Sunil Narasimhamurthy <sunnrs@amazon.com>
Signed-off-by: Sunil Narasimhamurthy <sunnrs@amazon.com>
@suniltheta
Copy link
Copy Markdown
Contributor Author

@soulxu can you please help identify someone for a first pass review?

@mattklein123
Copy link
Copy Markdown
Member

@suniltheta is there someone else from AWS that can review for correctness or has it already been reviewed? I can skim after that. Thanks.

/wait-any

@suniltheta
Copy link
Copy Markdown
Contributor Author

Thanks Matt for your help. This change was from an internal patch that was maintained for 4 years pending contribution to upstream Envoy until the deprecation of libcurl.
The internal patch is working as expected and all the functional aspects remain the same in this PR except the unit testing code required modification to adapt to using http async client. Also, did some change to get STS response in json format to avoid adding an external dependency for xml parsing (pugixml or tinyxml2).

I have manually tested that this change works for AWS Request Signing http filter extension to fetch the credentials properly from AWS STS.


Let me quickly try to find if anyone from internal AWS cpp sdk interest slack channel can help with giving some feedback. And get back here to check with you again.

@mattklein123
Copy link
Copy Markdown
Member

If it's already being used that is fine. I can give it a quick skim.

Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

LGTM with a question, thanks.

/wait-any

Comment thread source/extensions/common/aws/utility.cc
Comment thread test/extensions/common/aws/BUILD
@michaelfinch
Copy link
Copy Markdown
Contributor

Thank you for supporting this!

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.

5 participants