chihsuan commented on code in PR #11479:
URL: https://github.com/apache/ozone/pull/11479#discussion_r4326047940
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOMMetadataReader.java:
##########
@@ -1045,4 +1057,79 @@ public void
getKeyInfoServesObjectStoreLayoutWhenNotRequested(boolean assumeS3Co
verify(keyManager).getKeyInfo(any(OmKeyArgs.class),
any(ResolvedBucket.class), any());
}
+
+ @Test
+ public void getBucketInfoResolvesS3VolumeWhenAssumeS3Context() throws
Exception {
+ BucketManager bucketManager = mock(BucketManager.class);
+ OmMetadataReader reader = mock(OmMetadataReader.class);
+ OMMetrics metrics = mock(OMMetrics.class);
+ OzoneManager ozoneManager = newBucketInfoOzoneManager(bucketManager,
reader, metrics);
+ OmBucketInfo bucket = bucketInfo(TENANT_VOLUME_NAME);
+ when(bucketManager.getBucketInfo(TENANT_VOLUME_NAME,
BUCKET_NAME)).thenReturn(bucket);
+
+ BucketInfoWithVolumeContext result =
ozoneManager.getBucketInfo(VOLUME_NAME, BUCKET_NAME, true);
+
+ assertSame(bucket, result.getBucketInfo());
+ assertEquals(Optional.of("user"), result.getUserPrincipal());
+ verify(ozoneManager).getS3VolumeContext(true);
+ verify(reader).checkAcls(BUCKET, OZONE, READ, TENANT_VOLUME_NAME,
BUCKET_NAME, null);
+ verify(reader, never()).checkAcls(eq(VOLUME), any(), any(), any(), any(),
any());
+ verify(metrics).incNumBucketInfos();
+ }
+
+ @Test
+ public void getBucketInfoUsesRequestedVolumeWithoutAssumeS3Context() throws
Exception {
+ BucketManager bucketManager = mock(BucketManager.class);
+ OmMetadataReader reader = mock(OmMetadataReader.class);
+ OzoneManager ozoneManager = newBucketInfoOzoneManager(bucketManager,
reader, mock(OMMetrics.class));
+ OmBucketInfo bucket = bucketInfo(VOLUME_NAME);
+ when(bucketManager.getBucketInfo(VOLUME_NAME,
BUCKET_NAME)).thenReturn(bucket);
+
+ BucketInfoWithVolumeContext result =
ozoneManager.getBucketInfo(VOLUME_NAME, BUCKET_NAME, false);
+
+ assertSame(bucket, result.getBucketInfo());
+ assertFalse(result.getUserPrincipal().isPresent());
+ verify(ozoneManager, never()).getS3VolumeContext(anyBoolean());
+ verify(reader).checkAcls(BUCKET, OZONE, READ, VOLUME_NAME, BUCKET_NAME,
null);
+ }
+
+ @Test
+ public void getBucketInfoReturnsBucketNotFoundWithoutVolumeCheck() throws
Exception {
Review Comment:
nit: Would it be worth dropping the `never()` checks on VOLUME `checkAcls`?
`getBucketInfo` doesn't check volume ACLs before or after this change, so they
can't fail.
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -499,6 +499,7 @@ message OMResponse {
optional StartFinalizeUpgradeResponse startFinalizeUpgradeResponse =
159;
optional QueryUpgradeStatusResponse queryUpgradeStatusResponse =
160;
optional CompleteFinalizeUpgradeResponse completeFinalizeUpgradeResponse =
161;
+ optional string userPrincipal = 162;
Review Comment:
Just curious, are we planning to put `userPrincipal` on `OMResponse` for the
other subtasks too? `GetKeyInfoResponse` has it in its own response, so it'd be
good to pick one way before this ships.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
