[WIP] HBASE-29435 Add RestoreSnapshotHFileArchiver to guard against accidental data loss

hbase-server/src/main/java/org/apache/hadoop/hbase/backup/RestoreSnapshotHFileArchiver.java

@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.backup;
+
+import java.io.IOException;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.hadoop.hbase.util.CommonFSUtils;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Safe wrapper around {@link HFileArchiver} that guards against accidental operations on the
+ * production HBase root directory.
+ * <p>
+ * This class is intended for use by {@link org.apache.hadoop.hbase.snapshot.RestoreSnapshotHelper}
+ * and other callers that operate on temporary/restore directories rather than the live data
+ * directory. Before delegating to {@link HFileArchiver}, every method validates that the target
+ * paths do <b>not</b> resolve to the production root directory (as configured by
+ * {@code hbase.rootdir}). If they do, the operation is refused with an {@link IOException} and a
+ * prominent log message.
+ * <p>
+ * This is a defense-in-depth measure introduced by
+ * <a href="https://issues.apache.org/jira/browse/HBASE-29435">HBASE-29435</a> to prevent the class
+ * of bugs demonstrated in HBASE-29346, where a MapReduce snapshot restore accidentally archived
+ * live HFiles from the production root directory.
+ *
+ * @see HFileArchiver
+ * @see <a href="https://issues.apache.org/jira/browse/HBASE-29435">HBASE-29435</a>
+ */
+@InterfaceAudience.Private
+public final class RestoreSnapshotHFileArchiver {
+  private static final Logger LOG = LoggerFactory.getLogger(RestoreSnapshotHFileArchiver.class);
+
+  private RestoreSnapshotHFileArchiver() {
+    // Utility class — no instantiation.
+  }
+
+  /**
+   * Archive a region, after verifying the operation does not target the production root directory.
+   * <p>
+   * Delegates to {@link HFileArchiver#archiveRegion(Configuration, FileSystem, RegionInfo, Path,
+   * Path)} after safety validation.
+   * @param conf     the configuration (used to resolve production root dir for the safety check)
+   * @param fs       the file system
+   * @param hri      region to archive
+   * @param rootDir  root directory of the table tree (should be a temp/restore dir, not production)
+   * @param tableDir table directory under {@code rootDir}
+   * @throws IOException if the target is the production root dir, or if archival fails
+   */
+  public static void archiveRegion(Configuration conf, FileSystem fs, RegionInfo hri, Path rootDir,
+    Path tableDir) throws IOException {
+    validateNotProductionRootDir(conf, rootDir, "archiveRegion",
+      "region=" + hri.getEncodedName());
+    HFileArchiver.archiveRegion(conf, fs, hri, rootDir, tableDir);
+  }
+
+  /**
+   * Archive all files in a column family directory, after verifying the operation does not target
+   * the production root directory.
+   * <p>
+   * Delegates to
+   * {@link HFileArchiver#archiveFamilyByFamilyDir(FileSystem, Configuration, RegionInfo, Path,
+   * byte[])} after safety validation.
+   * @param fs        the file system
+   * @param conf      the configuration (used to resolve production root dir for the safety check)
+   * @param parent    region hosting the family
+   * @param familyDir path to the family directory to archive
+   * @param family    column family name
+   * @throws IOException if the target is the production root dir, or if archival fails
+   */
+  public static void archiveFamilyByFamilyDir(FileSystem fs, Configuration conf, RegionInfo parent,
+    Path familyDir, byte[] family) throws IOException {
+    validateNotProductionRootDir(conf, familyDir, "archiveFamilyByFamilyDir",
+      "region=" + parent.getEncodedName() + ", family=" + new String(family));
+    HFileArchiver.archiveFamilyByFamilyDir(fs, conf, parent, familyDir, family);
+  }
+
+  /**
+   * Validates that the given target path does not fall under the production HBase root directory.
+   * <p>
+   * The production root directory is resolved from {@code conf} via
+   * {@link CommonFSUtils#getRootDir(Configuration)} (i.e., the {@code hbase.rootdir} setting). If
+   * {@code targetPath} starts with (is a child of) the production root, this method throws an
+   * {@link IOException} and logs an ERROR — the operation must not proceed.
+   * @param conf       configuration to resolve the production root directory
+   * @param targetPath the path being operated on
+   * @param operation  name of the operation (for logging)
+   * @param detail     additional context (region, file, family — for logging)
+   * @throws IOException if {@code targetPath} is under the production root directory
+   */
+  static void validateNotProductionRootDir(Configuration conf, Path targetPath, String operation,
+    String detail) throws IOException {
+    Path productionRootDir = CommonFSUtils.getRootDir(conf);
+    Path qualifiedTarget = targetPath.getFileSystem(conf).makeQualified(targetPath);
+    Path qualifiedRoot = productionRootDir.getFileSystem(conf).makeQualified(productionRootDir);
+
+    String targetStr = qualifiedTarget.toUri().getPath();
+    String rootStr = qualifiedRoot.toUri().getPath();
+
+    if (targetStr.equals(rootStr) || targetStr.startsWith(rootStr + "/")) {
+      String message = "BLOCKED: " + operation + " attempted on production root directory! "
+        + "targetPath=" + targetPath + ", productionRootDir=" + productionRootDir + ", " + detail
+        + ". This operation has been refused to prevent accidental data loss. "
+        + "See HBASE-29435 / HBASE-29346.";
+      LOG.error(message);
+      throw new IOException(message);
+    }
+  }
+}

hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/RestoreSnapshotHelper.java

@@ -39,7 +39,7 @@
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hbase.MetaTableAccessor;
 import org.apache.hadoop.hbase.TableName;
-import org.apache.hadoop.hbase.backup.HFileArchiver;
+import org.apache.hadoop.hbase.backup.RestoreSnapshotHFileArchiver;
 import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
 import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
 import org.apache.hadoop.hbase.client.Connection;
@@ -415,7 +415,7 @@ private void removeHdfsRegions(final ThreadPoolExecutor exec, final List<RegionI
     ModifyRegionUtils.editRegions(exec, regions, new ModifyRegionUtils.RegionEditTask() {
       @Override
       public void editRegion(final RegionInfo hri) throws IOException {
-        HFileArchiver.archiveRegion(conf, fs, hri, rootDir, tableDir);
+        RestoreSnapshotHFileArchiver.archiveRegion(conf, fs, hri, rootDir, tableDir);
       }
     });
   }
@@ -556,7 +556,7 @@ private void restoreRegion(final RegionInfo regionInfo,
           + " from region=" + regionInfo.getEncodedName() + " table=" + tableName);
         LOG.debug("Removing family=" + Bytes.toString(family) + " in snapshot=" + snapshotName
           + " from region=" + regionInfo.getEncodedName() + " table=" + tableName);
-        HFileArchiver.archiveFamilyByFamilyDir(fs, conf, regionInfo, familyDir, family);
+        RestoreSnapshotHFileArchiver.archiveFamilyByFamilyDir(fs, conf, regionInfo, familyDir, family);
         fs.delete(familyDir, true);
       }
 

hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestRestoreSnapshotHFileArchiver.java

@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.backup;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
+
+import java.io.IOException;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.HBaseConfiguration;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.testclassification.MiscTests;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests for {@link RestoreSnapshotHFileArchiver} — validates that the root-directory safety check
+ * correctly blocks operations targeting the production root directory and allows operations on temp
+ * directories.
+ */
+@Tag(MiscTests.TAG)
+@Tag(SmallTests.TAG)
+public class TestRestoreSnapshotHFileArchiver {
+
+  private static Configuration createConf(String rootDir) {
+    Configuration conf = HBaseConfiguration.create();
+    conf.set("fs.defaultFS", "file:///");
+    conf.set(HConstants.HBASE_DIR, rootDir);
+    return conf;
+  }
+
+  /**
+   * When the target path IS the production root dir, validation must throw.
+   */
+  @Test
+  public void testBlocksExactRootDir() throws IOException {
+    Configuration conf = createConf("/hbase");
+    Path targetPath = new Path("/hbase");
+
+    try {
+      RestoreSnapshotHFileArchiver.validateNotProductionRootDir(conf, targetPath, "testOp",
+        "detail");
+      fail("Expected IOException when target equals production root dir");
+    } catch (IOException e) {
+      assertTrue(e.getMessage().contains("BLOCKED"),
+        "Error message should mention BLOCKED");
+      assertTrue(e.getMessage().contains("HBASE-29435"),
+        "Error message should reference HBASE-29435");
+    }
+  }
+
+  /**
+   * When the target path is a child of the production root dir, validation must throw.
+   */
+  @Test
+  public void testBlocksChildOfRootDir() throws IOException {
+    Configuration conf = createConf("/hbase");
+    Path targetPath = new Path("/hbase/data/default/MY_TABLE");
+
+    try {
+      RestoreSnapshotHFileArchiver.validateNotProductionRootDir(conf, targetPath, "archiveRegion",
+        "region=abc123");
+      fail("Expected IOException when target is under production root dir");
+    } catch (IOException e) {
+      assertTrue(e.getMessage().contains("BLOCKED"),
+        "Error message should mention BLOCKED");
+    }
+  }
+
+  /**
+   * When the target path is a temp/restore dir outside the root, validation must pass.
+   */
+  @Test
+  public void testAllowsTempDir() throws IOException {
+    Configuration conf = createConf("/hbase");
+    Path targetPath = new Path("/tmp/snapshot-restore/data/default/MY_TABLE");
+
+    // Should not throw
+    RestoreSnapshotHFileArchiver.validateNotProductionRootDir(conf, targetPath, "archiveRegion",
+      "region=abc123");
+  }
+
+  /**
+   * A path that is a sibling of the root dir (shares a prefix string but is not a child) must be
+   * allowed. For example, /hbase-staging is not a child of /hbase.
+   */
+  @Test
+  public void testAllowsSiblingOfRootDir() throws IOException {
+    Configuration conf = createConf("/hbase");
+    Path targetPath = new Path("/hbase-staging/data/default/MY_TABLE");
+
+    // Should not throw — /hbase-staging is NOT under /hbase
+    RestoreSnapshotHFileArchiver.validateNotProductionRootDir(conf, targetPath, "archiveRegion",
+      "region=abc123");
+  }
+
+  /**
+   * The archive subdirectory under root (/hbase/archive) should also be blocked, since it is a
+   * child of the production root.
+   */
+  @Test
+  public void testBlocksArchiveSubdir() throws IOException {
+    Configuration conf = createConf("/hbase");
+    Path targetPath = new Path("/hbase/archive/data/default/MY_TABLE");
+
+    try {
+      RestoreSnapshotHFileArchiver.validateNotProductionRootDir(conf, targetPath,
+        "archiveStoreFile", "file=hfile123");
+      fail("Expected IOException when target is under production root dir (archive subdir)");
+    } catch (IOException e) {
+      assertTrue(e.getMessage().contains("BLOCKED"),
+        "Error message should mention BLOCKED");
+    }
+  }
+
+  /**
+   * Verify the check works with a custom (non-default) root directory.
+   */
+  @Test
+  public void testBlocksCustomRootDir() throws IOException {
+    Configuration conf = createConf("/custom/hbase/root");
+    Path targetPath = new Path("/custom/hbase/root/data/default/MY_TABLE");
+
+    try {
+      RestoreSnapshotHFileArchiver.validateNotProductionRootDir(conf, targetPath, "archiveRegion",
+        "region=xyz");
+      fail("Expected IOException when target is under custom production root dir");
+    } catch (IOException e) {
+      assertTrue(e.getMessage().contains("BLOCKED"));
+    }
+  }
+
+  /**
+   * A temp dir with a completely different root must be allowed even with a custom root.
+   */
+  @Test
+  public void testAllowsTempDirWithCustomRoot() throws IOException {
+    Configuration conf = createConf("/custom/hbase/root");
+    Path targetPath = new Path("/tmp/restore/data/default/MY_TABLE");
+
+    // Should not throw
+    RestoreSnapshotHFileArchiver.validateNotProductionRootDir(conf, targetPath, "archiveRegion",
+      "region=xyz");
+  }
+}