sachinnn99 commented on code in PR #10699:
URL: https://github.com/apache/gravitino/pull/10699#discussion_r3049978223
##########
iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergNamespaceOperations.java:
##########
@@ -257,4 +265,63 @@ void testUpdateNamespace() {
dummyEventListener.clearEvent();
verifyUpdateNamespaceSucc(Namespace.of("update_foo3", "a"));
}
+
+ @Test
+ void testRegisterTableWithCredentialVending() {
+ // register without credential vending — no credentials in response
+ verifyRegisterTableSucc("register_cred_foo1",
Namespace.of("register_cred_ns"));
+
+ // register with credential vending but local location — should NOT vend
+ Response response =
+ doRegisterTableWithCredentialVending(
+ "register_cred_foo2", Namespace.of("register_cred_ns2"), "mock");
+ Assertions.assertEquals(Status.OK.getStatusCode(), response.getStatus());
+ LoadTableResponse loadTableResponse =
response.readEntity(LoadTableResponse.class);
+
Assertions.assertTrue(!loadTableResponse.config().containsKey(Credential.CREDENTIAL_TYPE));
Review Comment:
Fixed — replaced `assertTrue(!...)` with `assertFalse(...)`.
##########
iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergTableOperations.java:
##########
@@ -603,7 +603,7 @@ private static boolean etagMatches(String ifNoneMatch,
EntityTag etag) {
return etag.getValue().equals(clientEtag);
}
- private boolean isCredentialVending(String accessDelegation) {
+ static boolean isCredentialVending(String accessDelegation) {
Review Comment:
Good catch on the visibility change being undocumented, but the change is
actually for production reuse rather than testing —
`IcebergNamespaceOperations#registerTable` (in the same package) now calls this
to parse the same `X-Iceberg-Access-Delegation` header that
`IcebergTableOperations#createTable` and `#loadTable` already parse.
`@VisibleForTesting` would be misleading because no test depends on its
visibility. I've added a JavaDoc explaining the production-reuse rationale
instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
