Update AIP-4115.#1022
Conversation
matthewstevenson88
left a comment
matthewstevenson88
left a comment
There was a problem hiding this comment.
Thanks for the PR Hang! I've left some comments, mostly nits. :)
matthewstevenson88
left a comment
matthewstevenson88
left a comment
There was a problem hiding this comment.
LGTM, with a few remaining micro-nits. Thanks for the PR Hang! :)
shinfan
requested review from
andyrzhao
and removed request for
tavishvaidya
TimurSadykov
left a comment
TimurSadykov
left a comment
There was a problem hiding this comment.
This AIP, by the title, supposed to clarify default credentials for VMs. With this edit it is mostly focused on mTLS, which might not even be supported
Added comments to the doc
theodorehu95
requested review from
TimurSadykov
and removed request for
andyrzhao
| - Google Cloud Functions | ||
| - Cloud Run | ||
| - Workload Identity on Google Kubernetes Engine | ||
| ## Prerequisites |
There was a problem hiding this comment.
This paragraph is not really a "prerequisite" but rather a description of the fallback logic in the absence of mTLS support. Furthermore, mTLS support detection mechanism is not fleshed out in the current state of things, and the ramifications extend beyond the scope of this AIP (as you stated.) I would suggest we move the contents of this paragraph into the guidance section and also frame it in the context of existing mTLS support which is enabled through ECP and SecureConnect, which is applicable even in Google Cloud VMs. For example, the PR https://github.com/googleapis/google-api-go-client/pull/1874/files gives precedent to clientCertSource as discovered by ADC. Along those lines, I would also add back the link to [Application Default Credential][2] and maybe even reference AIP-4114 in this doc. WDYT?
There was a problem hiding this comment.
Yeah. I moved the "prerequisite" to be part of the guidance and mentioned briefly that it should give priority to DCA. I am a bit hesitant to add more content related to ECP and SecureConnect, since IIUC these things are using DCA and as long as DCA takes precedence, they should be included with more details in DCA AIP if appropriate. WDYT?
andyrzhao
left a comment
andyrzhao
left a comment
There was a problem hiding this comment.
LGTM, thanks for the updates!
6 participants
Update AIP-4115 go/default-credentials-aip.