feat: Skip RAB lookup in case of non-email response from MDS.#13331
feat: Skip RAB lookup in case of non-email response from MDS.#13331vverman wants to merge 2 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates ComputeEngineCredentials to return null for the Regional Access Boundary (RAB) URL if the service account is null or does not contain '@', which can happen with default service accounts in GKE environments. It also updates RegionalAccessBoundaryManager to handle a null URL gracefully, and adds a unit test to verify this behavior. There are no review comments, so I have no feedback to provide.
| // In GKE environments, the default service account might return a non-email placeholder. | ||
| // Since RAB lookup requires a valid email-based service account, we skip RAB lookup | ||
| // in non-email scenarios by returning null. | ||
| if (account == null || !account.contains("@")) { |
There was a problem hiding this comment.
I know the document may have changed since this PR was raised, but I think the the standard email pattern is a regex and we will need to confirm to that.
| if (account == null || !account.contains("@")) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
I think we will also need to follow the recommendation for a one-time log message that the RAB instance is skipped (maybe here, maybe in the RABManager)
| if (url == null) { | ||
| future.set(null); | ||
| return; | ||
| } |
There was a problem hiding this comment.
qq, can you remind me again why we need to do future.set(null);?
2 participants
In ComputeEngineCredentials when running on GKE platform, the getAccount() call may return a value which isn't an email.
In this case the right behaviour is to skip RAB lookup which is what this PR does.
Added tests.