-
Notifications
You must be signed in to change notification settings - Fork 6.6k
chore(cloud-sql): update GKE sample comments #10451
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing for general sample workflow
# If you are not connecting with Automatic IAM, you can delete | ||
# the following flag. | ||
- "--auto-iam-authn" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just confirming you want this to be on by default? It conflicts with the advice above that using Automatic IAM is recommended but it's on by default here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes this being on is fine, enabling it still allows users to connect via regular user and password authn, the flag just enables the additional ability for auto IAM authN and doesn't strictly force it.
@dandhlee this is ready for another pass when you have a second |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing for Samples
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please merge main to this branch to fix Python2.7 testing issue.
Adding comments to GKE Cloud SQL sample to highlight recommended use.