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

feat: Added '-enable_iam_login' flag for IAM db authentication #583

Merged
merged 6 commits into from Feb 21, 2021

Conversation

shawnhuang-gg
Copy link
Contributor

Change Description

This PR introduces the client side changes needed for utilizing the access token for cloudsql proxy IAM database authentication. The feature is gated by newly introduced flag --enable_iam_login. When enabled it includes the access token in the sqladmin.SslCertsCreateEphemeralRequest and send the request to create a ephemeral cert with access token info in it, which will later be used to perform IAM authentication. The access token has TTL of 1 hour and the ssl cert will be refreshed whenever access token expires or cert expires.

@google-cla google-cla bot added the cla: yes label Jan 8, 2021
@shawnhuang-gg shawnhuang-gg changed the title Add access token info in ephemeral ssl cert for proxy IAM db authentication BREAKING CHANGE: Add access token info in ephemeral ssl cert for proxy IAM db authentication Jan 8, 2021
@shawnhuang-gg shawnhuang-gg changed the title BREAKING CHANGE: Add access token info in ephemeral ssl cert for proxy IAM db authentication feat: Add access token info in ephemeral ssl cert for proxy IAM db authentication Jan 9, 2021
Copy link
Contributor

@kurtisvg kurtisvg left a comment

Choose a reason for hiding this comment

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

If I'm interpreting this correctly, it looks like we're relying on Gcloud to create the oauth2 token for us. Any reason why we can't do something in process, and not limited to having gcloud commands?

proxy/certs/certs.go Outdated Show resolved Hide resolved
cmd/cloud_sql_proxy/cloud_sql_proxy.go Outdated Show resolved Hide resolved
cmd/cloud_sql_proxy/cloud_sql_proxy.go Outdated Show resolved Hide resolved
cmd/cloud_sql_proxy/cloud_sql_proxy.go Outdated Show resolved Hide resolved
@kurtisvg kurtisvg assigned kurtisvg and unassigned shubha-rajan Jan 18, 2021
@kurtisvg
Copy link
Contributor

@shawnhuang-gg It looks like you are still working on this, but when you are ready for another review please just click the "request review" button to make sure it hits my inbox:
image

Copy link
Contributor

@kurtisvg kurtisvg left a comment

Choose a reason for hiding this comment

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

A few nits documentation wise, otherwise LGTM.

README.md Outdated Show resolved Hide resolved
cmd/cloud_sql_proxy/cloud_sql_proxy.go Outdated Show resolved Hide resolved
cmd/cloud_sql_proxy/cloud_sql_proxy.go Outdated Show resolved Hide resolved
@kurtisvg kurtisvg requested a review from enocom January 23, 2021 03:34
Copy link
Member

@enocom enocom left a comment

Choose a reason for hiding this comment

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

LGTM

@kurtisvg kurtisvg changed the title feat: Add access token info in ephemeral ssl cert for proxy IAM db authentication feat: Add support for IAM DB authentication with the '-enable_iam_login' flag Jan 26, 2021
@kurtisvg
Copy link
Contributor

@shawnhuang-gg This is good to go on our end. Once the changes have been rolled out we can trigger the tests and merge.

Otherwise we can merge sooner if we have the ability to allow-list the testing project early.

Copy link
Contributor

@kurtisvg kurtisvg left a comment

Choose a reason for hiding this comment

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

@shawnhuang-gg This LGTM on our end. Will follow up via email regarding testing.

Base automatically changed from master to main February 17, 2021 18:05
@google-cla
Copy link

google-cla bot commented Feb 20, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added cla: no and removed cla: yes labels Feb 20, 2021
shawnhuang-gg and others added 5 commits February 20, 2021 13:15
Co-authored-by: Kurtis Van Gent <31518063+kurtisvg@users.noreply.github.com>
Co-authored-by: Kurtis Van Gent <31518063+kurtisvg@users.noreply.github.com>
Co-authored-by: Kurtis Van Gent <31518063+kurtisvg@users.noreply.github.com>
@google-cla google-cla bot added cla: yes and removed cla: no labels Feb 20, 2021
@kurtisvg kurtisvg changed the title feat: Add support for IAM DB authentication with the '-enable_iam_login' flag feat: Added '-enable_iam_login' flag for using IAM db authentication Feb 20, 2021
@kurtisvg kurtisvg changed the title feat: Added '-enable_iam_login' flag for using IAM db authentication feat: Added '-enable_iam_login' flag for IAM db authentication Feb 20, 2021
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.

None yet

5 participants