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

feat: Add AWS Workload Identity Federation Support #414

Closed
wants to merge 17 commits into from
Closed

feat: Add AWS Workload Identity Federation Support #414

wants to merge 17 commits into from

Conversation

BigTailWolf
Copy link
Contributor

This is a re-catch on the closed pull request:
#408

rbclark and others added 13 commits October 28, 2022 16:34
@rbclark
feat: Add External Account Credential support as well as support for …
5dbc165 
…impersonated credential
@rbclark
chore: Add tests for ExternalAccount, add methods required to make te…
6935145 
…st compatible with apply_auth_examples. ExternalAccount was not fully compatible with the other providers, this brings it in line and moves some common methods between the other provides and ExternalAccount into a module which is now included by the AWSClient provider and Signet::Auth::Client
@rbclark
chore: Move AWS Credentials to their own file, do not pass role from …
ce07c11 
…external account, create dedicated Connection helper module, other misc PR cleanup
@rbclark
chore: Update BaseClient documentation comment
a89e880
@rbclark
chore: Ensure the subject token type matches AWS before building AwsC…
94e1935 
…redentials
@ScruffyProdigy
feat: perform validation and add tests
804b3dc
@ScruffyProdigy
Fixes to get manual testing working
d846561
@ScruffyProdigy
dates
2bdc8cc
@ScruffyProdigy
fixes
d1dead1
@ScruffyProdigy
Allow different versions of Faraday
997cee9
@ScruffyProdigy
ALlow different versions of Faraday
6237cfd
@ScruffyProdigy
Merge branch 'main' into aws
4ccec03
@bajajneha27 @BigTailWolf
chore: use SampleLoader for running sample tests (googleapis#411)
d85f872
@BigTailWolf BigTailWolf requested a review from a team as a code owner January 27, 2023 21:59
@rbclark rbclark mentioned this pull request Feb 4, 2023
Open
@BigTailWolf
Copy link
Contributor Author

@dazuma Would you please take a look at this. We need to have it approved and unblock the customer request.

@bajajneha27
Copy link
Contributor

@BigTailWolf I just noticed that the CLS is not signed. Can we please fix that?

bajajneha27
bajajneha27 reviewed Mar 13, 2023
View reviewed changes
lib/googleauth/base_client.rb Outdated Show resolved Hide resolved
lib/googleauth/base_client.rb Outdated Show resolved Hide resolved
lib/googleauth/external_account.rb Show resolved Hide resolved
lib/googleauth/external_account/aws_credentials.rb Show resolved Hide resolved
lib/googleauth/external_account/aws_credentials.rb Outdated Show resolved Hide resolved
lib/googleauth/external_account/aws_credentials.rb Show resolved Hide resolved
lib/googleauth/external_account/base_credentials.rb Show resolved Hide resolved
lib/googleauth/helpers/connection.rb Show resolved Hide resolved
spec/googleauth/apply_auth_examples.rb Show resolved Hide resolved
@BigTailWolf
Copy link
Contributor Author

BigTailWolf commented Mar 13, 2023 •
edited

@BigTailWolf I just noticed that the CLS is not signed. Can we please fix that?

Hi Neha, I think the security check failed due to Ryan's CLA status. Mine is under Google CLA.
Yet I don't know how to remove Ryan (Scuffypuppy) from the check.

Would you please try to remove him from the googleapis/google-auth-library-ruby settings?

@BigTailWolf BigTailWolf marked this pull request as draft March 14, 2023 00:01
@BigTailWolf BigTailWolf marked this pull request as ready for review March 14, 2023 00:01
BigTailWolf
BigTailWolf commented Mar 14, 2023
View reviewed changes
Copy link
Contributor Author

@BigTailWolf BigTailWolf left a comment

Choose a reason for hiding this comment

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

Comments addressed

bajajneha27
bajajneha27 reviewed Mar 14, 2023
View reviewed changes
Copy link
Contributor

@bajajneha27 bajajneha27 left a comment

Choose a reason for hiding this comment

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

Jin, I added a few more nit comments. Otherwise it looks good.

lib/googleauth/external_account.rb
@@ -0,0 +1,114 @@
# Copyright 2022 Google, Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit:

Suggested change
# Copyright 2022 Google, Inc.
# Copyright 2023 Google, Inc.

lib/googleauth/external_account.rb
# See the License for the specific language governing permissions and
# limitations under the License.

require "time"
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like we're not using time here

lib/googleauth/external_account.rb
module ExternalAccount
# Provides an entrypoint for all Exernal Account credential classes.
class Credentials
# The subject token type used for AWS external_account credentials.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: the comment should be above AWS_SUBJECT_TOKEN_TYPE

lib/googleauth/external_account/aws_credentials.rb
@@ -0,0 +1,381 @@
# Copyright 2022 Google, Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit:

Suggested change
# Copyright 2022 Google, Inc.
# Copyright 2023 Google, Inc.

lib/googleauth/external_account/base_credentials.rb
@@ -0,0 +1,140 @@
# Copyright 2022 Google, Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Copyright 2022 Google, Inc.
# Copyright 2023 Google, Inc.

lib/googleauth/oauth2/sts_client.rb
@@ -0,0 +1,99 @@
# Copyright 2022 Google LLC
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Copyright 2022 Google LLC
# Copyright 2023 Google LLC

spec/googleauth/apply_auth_examples.rb Show resolved Hide resolved
spec/googleauth/external_account/aws_credentials_spec.rb
@@ -0,0 +1,511 @@
# Copyright 2022 Google, Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Copyright 2022 Google, Inc.
# Copyright 2023 Google, Inc.

spec/googleauth/external_account_spec.rb
@@ -0,0 +1,228 @@
# Copyright 2022 Google, Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Copyright 2022 Google, Inc.
# Copyright 2023 Google, Inc.

spec/googleauth/oauth2/sts_client_spec.rb
@@ -0,0 +1,86 @@
# Copyright 2022 Google, Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Copyright 2022 Google, Inc.
# Copyright 2023 Google, Inc.

@BigTailWolf BigTailWolf mentioned this pull request Mar 17, 2023
Merged
@BigTailWolf BigTailWolf deleted the b260911343 branch June 20, 2023 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bajajneha27 bajajneha27 bajajneha27 left review comments

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

Successfully merging this pull request may close these issues.

None yet
4 participants
@BigTailWolf @bajajneha27 @rbclark @ScruffyProdigy