feat: Update Regional Access Boundaries#8099
feat: Update Regional Access Boundaries#8099vverman wants to merge 2 commits intogoogleapis:regional-access-boundariesfrom
Conversation
…gate; updated tests.
There was a problem hiding this comment.
Code Review
This pull request promotes the Regional Access Boundary (RAB) feature to a permanent state by removing experimental flags and updating lookup endpoints to production. Feedback highlights potential breaking changes resulting from the removal of public API members, suggesting the use of deprecation instead to maintain backward compatibility. Additionally, several test files were flagged for Sinon sandbox re-assignments in nested hooks, which can lead to resource leaks and test interference.
I am having trouble creating individual review comments. Click here to see my feedback.
core/packages/google-auth-library-nodejs/src/auth/authclient.ts (240)
Removing the regionalAccessBoundaryEnabled property from the AuthClient class is a breaking change for consumers who might be relying on this public property. Since this is a library, it is recommended to keep the property and mark it as @deprecated to maintain backward compatibility.
/** @deprecated This feature is now always enabled. */
regionalAccessBoundaryEnabled = true;
core/packages/google-auth-library-nodejs/src/auth/regionalaccessboundary.ts (66-77)
Removing the exported function isRegionalAccessBoundaryEnabled is a breaking change for external consumers. To maintain backward compatibility, consider keeping the function as deprecated and having it return true now that the feature is always enabled.
/** @deprecated This feature is now always enabled. */
export function isRegionalAccessBoundaryEnabled() {
return true;
}core/packages/google-auth-library-nodejs/test/test.compute.ts (310-315)
Re-assigning the sandbox variable in this nested beforeEach block overwrites the reference to the sandbox created in the outer scope. This leads to a leak because the outer afterEach will only restore the new sandbox instance, leaving the original one's stubs active. You should use the existing sandbox instead of creating a new one here.
beforeEach(() => {
(
Compute.prototype.getRegionalAccessBoundaryUrl as sinon.SinonStub
).restore();
});core/packages/google-auth-library-nodejs/test/test.jwt.ts (1281-1284)
Similar to other test files, re-assigning the sandbox variable here causes a leak of the outer sandbox instance. Use the existing sandbox to restore the prototype method.
beforeEach(() => {
(JWT.prototype.getRegionalAccessBoundaryUrl as sinon.SinonStub).restore();
});core/packages/google-auth-library-nodejs/test/test.impersonated.ts (630-635)
Re-assigning the sandbox variable here causes a leak of the outer sandbox instance. Use the existing sandbox to restore the prototype method.
beforeEach(() => {
(
Impersonated.prototype.getRegionalAccessBoundaryUrl as sinon.SinonStub
).restore();
});…pute.ts, test.jwt.ts, and test.impersonated.ts.
|
Duplicate sinon.createSandBox comment by gemini has been addressed. |
Following are the salient fixes:
Environment variable gate for RAB refresh has been removed.
RAB lookup endpoints have been fixed (previously they were pointing to staging)
Tests fixed.