fix(auth): Skip RAB lookup in case of non-email response from MDS.#13331
Conversation
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.
…olean for ease of reading.
| // 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. |
There was a problem hiding this comment.
nit: Since this is a temp change, can you link to the internal ticket tracking this (b/XXXX)
There was a problem hiding this comment.
This actualy isn't a temp change. MDS already returns a non-email value for certain GKE instances so this PR is actually to handle that scenario.
There was a problem hiding this comment.
Apologies, I meant that the skip RAB is a temp change for now. Can you link the internal tracking ticket in the PR description or as a comment in the code TODO(b/xxxx): ..?
There was a problem hiding this comment.
Ah I see, the correct and expected behaviour is for RAB lookup to be skipped when a non-email account is returned from the MDS.
This won't be a temp change rather this is the fix.
|
Some errors in the CI: Hmm, this may be related to some changes we made into GDCH to resolve a customer issue. Can you try and pull in the latest changes into the RAB feature branch? |
|
Plan to raise a PR with the rebased main later as it'd introduce a huge delta in this PR. |
52c6e1c
into
googleapis:regional-access-boundaries
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.