feat: add allowNonDefaultServiceAccount option for DirectPath#1433
Conversation
vam-google
left a comment
There was a problem hiding this comment.
LGTM, with a minor comment
| if (allowNonDefaultServiceAccount != null) { | ||
| return allowNonDefaultServiceAccount; | ||
| } | ||
| return credentials instanceof ComputeEngineCredentials; |
There was a problem hiding this comment.
Do we need this? I see there is already the isOnComputeEngine() method, so ideally we want to keep compute-engine specific check in one place. Also, this method (isNonDefaultServiceAccountAllowed()) is used only in conjunction with isOnComputeEngine() on line 324 of this file).
There was a problem hiding this comment.
Sorry the name might be misleading:
-
isOnComputeEngine()checks the environment of the VM as DirectPath is allowed on Compute Engine/GKE but not App Engine. -
isNonDefaultServiceAccountAllowed()checks which service account the VM is using to access DirectPath.credentialis instance ofComputeEngineCredentialsguarantees that the VM is using the default service account (because the credential is retrieved from the metadata server), but it can not guarantee the environment (i.e.,ComputeEngineCredentialscan also be used on App Engine).
Based on the above understanding I think the two checks are different and should be kept.
8135915 to
41c56d8
Compare
|
I updated the code. If the option is not set, or set to false, the code behavior should be the same as currently it is. |
🤖 I have created a release \*beep\* \*boop\* --- ## [2.1.0](https://www.github.com/googleapis/gax-java/compare/v2.0.0...v2.1.0) (2021-08-11) ### Features * add allowNonDefaultServiceAccount option for DirectPath ([#1433](https://www.github.com/googleapis/gax-java/issues/1433)) ([209b494](https://www.github.com/googleapis/gax-java/commit/209b4944feba1c62be2c9de4545e3b01a806b738)) ### Bug Fixes * fix httpjson executor ([#1448](https://www.github.com/googleapis/gax-java/issues/1448)) ([8f48b70](https://www.github.com/googleapis/gax-java/commit/8f48b7027b95e8e75872d1f9dac537ea697d0acc)) * make closeAsync don't interrupt running thread ([#1446](https://www.github.com/googleapis/gax-java/issues/1446)) ([7c6c298](https://www.github.com/googleapis/gax-java/commit/7c6c29824487346d444730388ea6967408692696)) ### Dependencies * update dependency com.google.api:api-common to v2.0.1 ([#1452](https://www.github.com/googleapis/gax-java/issues/1452)) ([a52f16f](https://www.github.com/googleapis/gax-java/commit/a52f16f6cef8340357acb374ff31c8a6f248403c)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Add an option allowNonDefaultServiceAccount for cloud services to bypass the check that credential must be retrieved from the metadata server, which implicitly requires using the default service account. In this way, non-default service account can be used for DirectPath.