Copilot commented on code in PR #10859:
URL: https://github.com/apache/ozone/pull/10859#discussion_r3648013106
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestBucketManagerImpl.java:
##########
@@ -540,4 +554,121 @@ public void testListBucketsResolvesFsoAndObsLinkLayouts()
throws Exception {
assertEquals(volume, listedObsLink.getSourceVolume());
assertEquals("obs-source", listedObsLink.getSourceBucket());
}
+
+ @Test
+ void testGetBucketInfoFailsWithoutSourceReadAccess() throws Exception {
+ String linkVolume = volumeName();
+ String sourceVolume = volumeName();
+ OmBucketInfo link = createLinkBucketInfo(linkVolume, sourceVolume);
+ OmBucketInfo source = createSourceBucketInfo(sourceVolume);
+ BucketManager bucketManager = mock(BucketManager.class);
+ when(bucketManager.getBucketInfo(linkVolume, "link")).thenReturn(link);
+ when(bucketManager.getBucketInfo(sourceVolume,
"source")).thenReturn(source);
+ OmMetadataReader metadataReader = mock(OmMetadataReader.class);
+ denySourceRead(metadataReader, sourceVolume);
+ OzoneManager omSpy = createAclEnabledOmSpy(bucketManager, metadataReader);
+
+ OMException exception = assertThrows(OMException.class,
+ () -> omSpy.getBucketInfo(linkVolume, "link"));
+
+ assertEquals(ResultCodes.PERMISSION_DENIED, exception.getResult());
+ verify(metadataReader).checkAcls(BUCKET, OZONE, READ, sourceVolume,
"source", null);
+ verify(bucketManager, times(1)).getBucketInfo(sourceVolume, "source");
+ }
+
+ @Test
+ void testListBucketsDoesNotCopySourcePropertiesWithoutReadAccess() throws
Exception {
+ String linkVolume = volumeName();
+ String sourceVolume = volumeName();
+ OmBucketInfo link = createLinkBucketInfo(linkVolume, sourceVolume);
+ OmBucketInfo source = createSourceBucketInfo(sourceVolume);
+ OmBucketInfo regular = OmBucketInfo.newBuilder()
+ .setVolumeName(linkVolume)
+ .setBucketName("regular")
+ .build();
+ BucketManager bucketManager = mock(BucketManager.class);
+ when(bucketManager.listBuckets(linkVolume, "", "", 100, false))
+ .thenReturn(new ArrayList<>(Arrays.asList(link, regular)));
+ when(bucketManager.getBucketInfo(sourceVolume,
"source")).thenReturn(source);
+ OmMetadataReader metadataReader = mock(OmMetadataReader.class);
+ denySourceRead(metadataReader, sourceVolume);
+ OzoneManager omSpy = createAclEnabledOmSpy(bucketManager, metadataReader);
+
+ List<OmBucketInfo> result = omSpy.listBuckets(linkVolume, "", "", 100,
false);
+
+ assertThat(result.get(0).getMetadata())
+ .containsEntry("linkKey", "linkValue")
+ .doesNotContainKey("sourceKey");
+ assertSame(regular, result.get(1));
+ verify(metadataReader).checkAcls(BUCKET, OZONE, READ, sourceVolume,
"source", null);
+ verify(bucketManager, times(1)).getBucketInfo(sourceVolume, "source");
+ }
+
+ @Test
+ void testListBucketsChecksSourceReadAccessOncePerSource() throws Exception {
+ String linkVolume = volumeName();
+ String sourceVolume = volumeName();
+ OmBucketInfo firstLink = createLinkBucketInfo(linkVolume, sourceVolume)
+ .toBuilder()
+ .setBucketName("link1")
+ .build();
+ OmBucketInfo secondLink = createLinkBucketInfo(linkVolume, sourceVolume)
+ .toBuilder()
+ .setBucketName("link2")
+ .build();
+ OmBucketInfo source = createSourceBucketInfo(sourceVolume);
+ BucketManager bucketManager = mock(BucketManager.class);
+ when(bucketManager.listBuckets(linkVolume, "", "", 100, false))
+ .thenReturn(new ArrayList<>(Arrays.asList(firstLink, secondLink)));
+ when(bucketManager.getBucketInfo(sourceVolume,
"source")).thenReturn(source);
+ OmMetadataReader metadataReader = mock(OmMetadataReader.class);
+ OzoneManager omSpy = createAclEnabledOmSpy(bucketManager, metadataReader);
+
+ List<OmBucketInfo> result = omSpy.listBuckets(linkVolume, "", "", 100,
false);
+
+ assertEquals("sourceValue", result.get(0).getMetadata().get("sourceKey"));
+ assertEquals("sourceValue", result.get(1).getMetadata().get("sourceKey"));
+ verify(metadataReader, times(1))
+ .checkAcls(BUCKET, OZONE, READ, sourceVolume, "source", null);
+ }
+
+ private static OmBucketInfo createLinkBucketInfo(String linkVolume, String
sourceVolume) {
+ return OmBucketInfo.newBuilder()
+ .setVolumeName(linkVolume)
+ .setBucketName("link")
+ .setSourceVolume(sourceVolume)
+ .setSourceBucket("source")
+ .addAllMetadata(singletonMap("linkKey", "linkValue"))
+ .build();
+ }
+
+ private static OmBucketInfo createSourceBucketInfo(String sourceVolume) {
+ return OmBucketInfo.newBuilder()
+ .setVolumeName(sourceVolume)
+ .setBucketName("source")
+ .addAllMetadata(singletonMap("sourceKey", "sourceValue"))
+ .build();
+ }
+
+ private static void denySourceRead(OmMetadataReader metadataReader, String
sourceVolume) throws IOException {
+ doAnswer(invocation -> {
+ if (sourceVolume.equals(invocation.getArgument(3)) &&
"source".equals(invocation.getArgument(4))) {
+ throw new OMException("denied", ResultCodes.PERMISSION_DENIED);
+ }
+ return null;
+ }).when(metadataReader).checkAcls(any(), any(), any(), any(), any(),
any());
+ }
+
+ private OzoneManager createAclEnabledOmSpy(BucketManager bucketManager,
OmMetadataReader metadataReader) {
+ OzoneManager omSpy = spy(omTestManagers.getOzoneManager());
+ HddsWhiteboxTestUtils.setInternalState(omSpy, "bucketManager",
bucketManager);
+ HddsWhiteboxTestUtils.setInternalState(omSpy, "omMetadataReader",
metadataReader);
+ when(omSpy.getAclsEnabled()).thenReturn(true);
+ AuditMessage auditMessage = mock(AuditMessage.class);
Review Comment:
Stubbing a method on a Mockito spy via `when(spy.method()).thenReturn(...)`
will invoke the real method during stubbing, which can introduce unintended
side effects / flakiness. Prefer `doReturn(...).when(spy)...` for spies
(consistent with the other stubs in this helper).
--
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]
