diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/AbstractRootedOzoneFileSystemTest.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/AbstractRootedOzoneFileSystemTest.java index e479cbb661eb..52a44e85297c 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/AbstractRootedOzoneFileSystemTest.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/AbstractRootedOzoneFileSystemTest.java @@ -1835,16 +1835,74 @@ void testBucketDefaultsShouldBeInheritedToFileForEC() assertEquals(ReplicationType.EC.name(), key.getReplicationConfig().getReplicationType().name()); } + @Test + void testGetFileStatusUsesSingleOmRpc() throws Exception { + String keyName = "single-rpc-" + RandomStringUtils.secure().nextAlphabetic(5); + Path filePath = new Path(bucketPath, keyName); + ContractTestUtils.touch(fs, filePath); + + OMMetrics metrics = getOMMetrics(); + long bucketInfosBefore = metrics.getNumBucketInfos(); + long getFileStatusBefore = metrics.getNumGetFileStatus(); + + FileStatus status = fs.getFileStatus(filePath); + assertTrue(status.isFile()); + + assertEquals(bucketInfosBefore, metrics.getNumBucketInfos(), + "getFileStatus must not trigger InfoBucket"); + assertEquals(getFileStatusBefore + 1, metrics.getNumGetFileStatus()); + + long getFileStatusAfterFirst = metrics.getNumGetFileStatus(); + fs.getFileStatus(filePath); + assertEquals(bucketInfosBefore, metrics.getNumBucketInfos()); + assertEquals(getFileStatusAfterFirst + 1, metrics.getNumGetFileStatus()); + } + + @Test + void testGetFileStatusRejectsObsBucket() throws Exception { + OzoneBucket obsBucket = + TestDataUtil.createVolumeAndBucket(client, BucketLayout.OBJECT_STORE); + Path obsBucketPath = new Path( + new Path(OZONE_URI_DELIMITER, obsBucket.getVolumeName()), + obsBucket.getName()); + String keyName = "obs-key-" + RandomStringUtils.secure().nextAlphabetic(5); + TestDataUtil.createKey(obsBucket, keyName, + "data".getBytes(StandardCharsets.UTF_8)); + Path keyPath = new Path(obsBucketPath, keyName); + + OMMetrics metrics = getOMMetrics(); + long bucketInfosBefore = metrics.getNumBucketInfos(); + + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> fs.getFileStatus(keyPath)); + assertThat(exception.getMessage()).contains(obsBucket.getName()); + assertThat(exception.getMessage()).contains("OBJECT_STORE"); + assertEquals(bucketInfosBefore, metrics.getNumBucketInfos(), + "getFileStatus must not trigger InfoBucket"); + } + @Test void testGetFileStatus() throws Exception { String volumeNameLocal = getRandomNonExistVolumeName(); String bucketNameLocal = RandomStringUtils.secure().nextNumeric(5); Path volume = new Path("/" + volumeNameLocal); fs.mkdirs(volume); - assertThrows(OMException.class, - () -> fs.getFileStatus(new Path(volume, bucketNameLocal))); - // Cleanup - fs.delete(volume, false); + try { + FileNotFoundException exception = assertThrows(FileNotFoundException.class, + () -> fs.getFileStatus(new Path(volume, bucketNameLocal))); + assertThat(exception.getMessage()).contains("Bucket doesn't exist"); + } finally { + fs.delete(volume, false); + } + } + + @Test + void testGetFileStatusMissingFile() throws Exception { + Path missingFile = new Path(bucketPath, "missing-file-" + + RandomStringUtils.secure().nextAlphanumeric(5)); + FileNotFoundException exception = assertThrows(FileNotFoundException.class, + () -> fs.getFileStatus(missingFile)); + assertThat(exception.getMessage()).contains("No such file or directory"); } @Test diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmAcls.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmAcls.java index 8faf7d973cff..c61ca25a8b4e 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmAcls.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmAcls.java @@ -46,6 +46,7 @@ import org.apache.hadoop.ozone.client.OzoneClient; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes; +import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; import org.apache.hadoop.ozone.security.acl.IOzoneObj; import org.apache.hadoop.ozone.security.acl.OzoneObj; @@ -193,7 +194,8 @@ public void testReadKeyPermissionDenied() throws Exception { @Test public void testGetFileStatusPermissionDenied() throws Exception { - OzoneBucket bucket = TestDataUtil.createVolumeAndBucket(client); + OzoneBucket bucket = TestDataUtil.createVolumeAndBucket(client, + BucketLayout.FILE_SYSTEM_OPTIMIZED); TestDataUtil.createKey(bucket, "testKey", "testcontent".getBytes(StandardCharsets.UTF_8)); authorizer.keyAclAllow = false; diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotTests.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotTests.java index 44131d81c88f..f3223bbbbfe3 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotTests.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotTests.java @@ -65,6 +65,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; +import static org.junit.jupiter.api.Assumptions.assumeFalse; import static org.junit.jupiter.api.Assumptions.assumeTrue; import com.google.common.collect.Lists; @@ -525,6 +526,7 @@ public void checkKey() throws Exception { KeyInfoWithVolumeContext fileInfo = writeClient.getKeyInfo(keyArgs, false); assertEquals(fileInfo.getKeyInfo().getKeyName(), snapshotKeyPrefix + key1); + assumeFalse(bucketLayout.equals(BucketLayout.OBJECT_STORE)); OzoneFileStatus ozoneFileStatus = writeClient.getFileStatus(keyArgs); assertEquals(ozoneFileStatus.getKeyInfo().getKeyName(), snapshotKeyPrefix + key1); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataReader.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataReader.java index 64f46089c066..3c5db0eb56aa 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataReader.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataReader.java @@ -52,6 +52,7 @@ import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.OmKeyArgs; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; +import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils; import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus; import org.apache.hadoop.ozone.om.helpers.OzoneFileStatusLight; import org.apache.hadoop.ozone.om.helpers.S3VolumeContext; @@ -277,6 +278,10 @@ public OzoneFileStatus getFileStatus(OmKeyArgs args) throws IOException { args = bucket.update(args); try { + if (bucket.bucketLayout() != null) { + OzoneFSUtils.validateBucketLayout(bucket.requestedBucket(), + bucket.bucketLayout()); + } if (isAclEnabled) { checkAcls(getResourceType(args), StoreType.OZONE, ACLType.READ, bucket, args.getKeyName()); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOMMetadataReader.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOMMetadataReader.java index 903b0720943d..4cb0a9871e9d 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOMMetadataReader.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOMMetadataReader.java @@ -18,12 +18,22 @@ package org.apache.hadoop.ozone.om; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import io.grpc.Context; import org.apache.hadoop.ipc_.Server; +import org.apache.hadoop.ozone.audit.AuditLogger; +import org.apache.hadoop.ozone.om.helpers.BucketLayout; +import org.apache.hadoop.ozone.om.helpers.OmKeyArgs; +import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus; import org.junit.jupiter.api.Test; import org.mockito.MockedStatic; @@ -69,4 +79,62 @@ public void testGetClientAddress() { } } + @Test + public void getFileStatusRejectsObjectStoreLayout() throws Exception { + OzoneManager ozoneManager = mock(OzoneManager.class); + KeyManager keyManager = mock(KeyManager.class); + when(ozoneManager.getAclsEnabled()).thenReturn(false); + when(ozoneManager.getBucketManager()).thenReturn(mock(BucketManager.class)); + when(ozoneManager.getVolumeManager()).thenReturn(mock(VolumeManager.class)); + when(ozoneManager.getPerfMetrics()).thenReturn(mock(OMPerformanceMetrics.class)); + when(ozoneManager.resolveBucketLink(any(OmKeyArgs.class))) + .thenReturn(new ResolvedBucket("vol", "obs-bucket", "vol", "obs-bucket", + "owner", BucketLayout.OBJECT_STORE)); + + OmMetadataReader reader = new OmMetadataReader(keyManager, + mock(PrefixManager.class), ozoneManager, mock(org.slf4j.Logger.class), + mock(AuditLogger.class), mock(OmMetadataReaderMetrics.class), null); + + OmKeyArgs keyArgs = new OmKeyArgs.Builder() + .setVolumeName("vol") + .setBucketName("obs-bucket") + .setKeyName("key1") + .build(); + + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> reader.getFileStatus(keyArgs)); + assertTrue(exception.getMessage().contains("obs-bucket")); + assertTrue(exception.getMessage().contains("OBJECT_STORE")); + verify(keyManager, never()).getFileStatus(any(), anyString()); + } + + @Test + public void getFileStatusAllowsLegacyLayout() throws Exception { + OzoneManager ozoneManager = mock(OzoneManager.class); + KeyManager keyManager = mock(KeyManager.class); + when(ozoneManager.getAclsEnabled()).thenReturn(false); + when(ozoneManager.getBucketManager()).thenReturn(mock(BucketManager.class)); + when(ozoneManager.getVolumeManager()).thenReturn(mock(VolumeManager.class)); + when(ozoneManager.getPerfMetrics()).thenReturn(mock(OMPerformanceMetrics.class)); + when(ozoneManager.resolveBucketLink(any(OmKeyArgs.class))) + .thenReturn(new ResolvedBucket("vol", "legacy-bucket", "vol", + "legacy-bucket", "owner", BucketLayout.LEGACY)); + OzoneFileStatus expectedStatus = new OzoneFileStatus(); + when(keyManager.getFileStatus(any(OmKeyArgs.class), anyString())) + .thenReturn(expectedStatus); + + OmMetadataReader reader = new OmMetadataReader(keyManager, + mock(PrefixManager.class), ozoneManager, mock(org.slf4j.Logger.class), + mock(AuditLogger.class), mock(OmMetadataReaderMetrics.class), null); + + OmKeyArgs keyArgs = new OmKeyArgs.Builder() + .setVolumeName("vol") + .setBucketName("legacy-bucket") + .setKeyName("key1") + .build(); + + OzoneFileStatus status = reader.getFileStatus(keyArgs); + assertEquals(expectedStatus, status); + verify(keyManager).getFileStatus(any(OmKeyArgs.class), anyString()); + } } diff --git a/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java b/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java index 50842949af2c..2f585e41a549 100644 --- a/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java +++ b/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java @@ -345,6 +345,12 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr, } // Try get bucket again bucket = proxy.getBucketDetails(volumeStr, bucketStr); + + BucketLayout resolvedBucketLayout = + OzoneClientUtils.resolveLinkBucketLayout(bucket, objectStore, + new HashSet<>()); + + OzoneFSUtils.validateBucketLayout(bucket.getName(), resolvedBucketLayout); } else { throw ex; } @@ -690,19 +696,24 @@ public FileStatusAdapter getFileStatus(String path, URI uri, * Return FileStatusAdapter based on OFSPath being a * valid bucket path or valid snapshot path. * Throws exception in case of failure. + * + *

Non-snapshot paths call OM GetFileStatus directly (HDDS-15925) without a + * prior InfoBucket RPC. OBJECT_STORE buckets are rejected by OM GetFileStatus. + * Mutating OFS operations still validate layout via {@link #getBucket(OFSPath, boolean)}. */ private FileStatusAdapter getFileStatusForKeyOrSnapshot( OFSPath ofsPath, URI uri, Path qualifiedPath, String userName, boolean headOp) throws IOException { String key = ofsPath.getKeyName(); try { - OzoneBucket bucket = getBucket(ofsPath, false); if (ofsPath.isSnapshotPath()) { + OzoneBucket bucket = getBucket(ofsPath, false); OzoneVolume volume = objectStore.getVolume(ofsPath.getVolumeName()); return getFileStatusAdapterWithSnapshotIndicator( volume, bucket, uri); } else { - OzoneFileStatus status = bucket.getFileStatus(key, headOp); + OzoneFileStatus status = proxy.getOzoneFileStatus(ofsPath.getVolumeName(), + ofsPath.getBucketName(), key, headOp); return toFileStatusAdapter(status, userName, uri, qualifiedPath, ofsPath.getNonKeyPath()); } @@ -712,6 +723,10 @@ private FileStatusAdapter getFileStatusForKeyOrSnapshot( } else if (e.getResult() == OMException.ResultCodes.BUCKET_NOT_FOUND) { throw new FileNotFoundException(key + ": Bucket doesn't exist!"); } + String message = e.getMessage(); + if (message != null && message.contains("does not support file system semantics")) { + throw new IllegalArgumentException(message); + } throw e; } } diff --git a/hadoop-ozone/ozonefs-common/src/test/java/org/apache/hadoop/fs/ozone/TestBasicRootedOzoneClientAdapterHeadOp.java b/hadoop-ozone/ozonefs-common/src/test/java/org/apache/hadoop/fs/ozone/TestBasicRootedOzoneClientAdapterHeadOp.java index 8f67adef1346..7b5f20e30192 100644 --- a/hadoop-ozone/ozonefs-common/src/test/java/org/apache/hadoop/fs/ozone/TestBasicRootedOzoneClientAdapterHeadOp.java +++ b/hadoop-ozone/ozonefs-common/src/test/java/org/apache/hadoop/fs/ozone/TestBasicRootedOzoneClientAdapterHeadOp.java @@ -43,6 +43,7 @@ import org.apache.hadoop.ozone.client.ObjectStore; import org.apache.hadoop.ozone.client.OzoneBucket; import org.apache.hadoop.ozone.client.OzoneVolume; +import org.apache.hadoop.ozone.client.protocol.ClientProtocol; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus; @@ -62,15 +63,20 @@ public class TestBasicRootedOzoneClientAdapterHeadOp { private BasicRootedOzoneClientAdapterImpl adapter; private OzoneBucket bucket; + private ClientProtocol proxy; @BeforeEach public void setUp() throws Exception { adapter = mock(BasicRootedOzoneClientAdapterImpl.class, CALLS_REAL_METHODS); bucket = mock(OzoneBucket.class); + proxy = mock(ClientProtocol.class); doReturn(bucket).when(adapter).getBucket(any(OFSPath.class), eq(false)); - // Inject a mock object store so the volume/snapshot dispatch branches can - // run without a live OM connection. + Field proxyField = + BasicRootedOzoneClientAdapterImpl.class.getDeclaredField("proxy"); + proxyField.setAccessible(true); + proxyField.set(adapter, proxy); + OzoneVolume volume = mock(OzoneVolume.class); when(volume.getName()).thenReturn("vol"); when(volume.getOwner()).thenReturn("user"); @@ -101,25 +107,26 @@ private static OzoneFileStatus fileStatus(boolean isDir) { @Test public void keyPathThreadsHeadOp() throws IOException { - when(bucket.getFileStatus(anyString(), anyBoolean())) + when(proxy.getOzoneFileStatus(anyString(), anyString(), anyString(), anyBoolean())) .thenReturn(fileStatus(false)); assertFalse(adapter.getFileStatus("/vol/bucket/key", URI_OFS, WORKING_DIR, "user", true).isDir()); ArgumentCaptor headOp = ArgumentCaptor.forClass(Boolean.class); - verify(bucket).getFileStatus(anyString(), headOp.capture()); + verify(proxy).getOzoneFileStatus(eq("vol"), eq("bucket"), eq("key"), + headOp.capture()); assertTrue(headOp.getValue()); } @Test public void fourArgOverloadDoesNotUseHeadOp() throws IOException { - when(bucket.getFileStatus(anyString(), anyBoolean())) + when(proxy.getOzoneFileStatus(anyString(), anyString(), anyString(), anyBoolean())) .thenReturn(fileStatus(true)); assertTrue(adapter.getFileStatus("/vol/bucket/key", URI_OFS, WORKING_DIR, "user").isDir()); - verify(bucket).getFileStatus(anyString(), eq(false)); + verify(proxy).getOzoneFileStatus(eq("vol"), eq("bucket"), eq("key"), eq(false)); } @Test @@ -130,7 +137,7 @@ public void rootPathReturnsDirectory() throws IOException { @Test public void fileNotFoundMappedToFileNotFoundException() throws IOException { - when(bucket.getFileStatus(anyString(), anyBoolean())) + when(proxy.getOzoneFileStatus(anyString(), anyString(), anyString(), anyBoolean())) .thenThrow(new OMException("missing", OMException.ResultCodes.FILE_NOT_FOUND)); assertThrows(FileNotFoundException.class, @@ -140,7 +147,7 @@ public void fileNotFoundMappedToFileNotFoundException() throws IOException { @Test public void otherOMExceptionPropagates() throws IOException { - when(bucket.getFileStatus(anyString(), anyBoolean())) + when(proxy.getOzoneFileStatus(anyString(), anyString(), anyString(), anyBoolean())) .thenThrow(new OMException("boom", OMException.ResultCodes.INTERNAL_ERROR)); assertThrows(OMException.class, @@ -150,7 +157,7 @@ public void otherOMExceptionPropagates() throws IOException { @Test public void bucketNotFoundMappedToFileNotFoundException() throws IOException { - when(bucket.getFileStatus(anyString(), anyBoolean())) + when(proxy.getOzoneFileStatus(anyString(), anyString(), anyString(), anyBoolean())) .thenThrow(new OMException("no bucket", OMException.ResultCodes.BUCKET_NOT_FOUND)); assertThrows(FileNotFoundException.class, @@ -169,7 +176,6 @@ public void snapshotIndicatorPathReturnsDirectory() throws IOException { when(bucket.getVolumeName()).thenReturn("vol"); when(bucket.getName()).thenReturn("bucket"); when(bucket.getCreationTime()).thenReturn(Instant.EPOCH); - // keyName == ".snapshot" is the snapshot indicator path. assertTrue(adapter.getFileStatus("/vol/bucket/.snapshot", URI_OFS, WORKING_DIR, "user", true).isDir()); }