From 0c6677ef54c434cc732b4ca2d2865363b9d5345f Mon Sep 17 00:00:00 2001 From: Kevin Rathbun Date: Tue, 15 Apr 2025 15:19:31 -0400 Subject: [PATCH 1/3] Prevents offline of all system tables Previously, could offline any table that was not ROOT. This commit prevents offline of any system table --- .../core/clientImpl/TableOperationsImpl.java | 4 ++-- .../apache/accumulo/manager/FateServiceHandler.java | 12 ++++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java index 17727b2b8f1..30b8622b1d5 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java @@ -1463,7 +1463,7 @@ public void offline(String tableName, boolean wait) EXISTING_TABLE_NAME.validate(tableName); TableId tableId = context.getTableId(tableName); - List args = Arrays.asList(ByteBuffer.wrap(tableId.canonical().getBytes(UTF_8))); + List args = Arrays.asList(ByteBuffer.wrap(tableName.getBytes(UTF_8))); Map opts = new HashMap<>(); try { @@ -1510,7 +1510,7 @@ public void online(String tableName, boolean wait) return; } - List args = Arrays.asList(ByteBuffer.wrap(tableId.canonical().getBytes(UTF_8))); + List args = Arrays.asList(ByteBuffer.wrap(tableName.getBytes(UTF_8))); Map opts = new HashMap<>(); try { diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java b/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java index c9ecd1d4d87..b7c444dfcbf 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java @@ -382,7 +382,11 @@ public void executeFateOperation(TInfo tinfo, TCredentials c, long opid, FateOpe case TABLE_ONLINE: { TableOperation tableOp = TableOperation.ONLINE; validateArgumentCount(arguments, tableOp, 1); - final var tableId = validateTableIdArgument(arguments.get(0), tableOp, NOT_ROOT_TABLE_ID); + String tableName = + validateName(arguments.get(0), tableOp, EXISTING_TABLE_NAME.and(NOT_BUILTIN_TABLE)); + + final TableId tableId = + ClientServiceHandler.checkTableId(manager.getContext(), tableName, tableOp); NamespaceId namespaceId = getNamespaceIdFromTableId(tableOp, tableId); final boolean canOnlineOfflineTable; @@ -410,7 +414,11 @@ public void executeFateOperation(TInfo tinfo, TCredentials c, long opid, FateOpe case TABLE_OFFLINE: { TableOperation tableOp = TableOperation.OFFLINE; validateArgumentCount(arguments, tableOp, 1); - final var tableId = validateTableIdArgument(arguments.get(0), tableOp, NOT_ROOT_TABLE_ID); + String tableName = + validateName(arguments.get(0), tableOp, EXISTING_TABLE_NAME.and(NOT_BUILTIN_TABLE)); + + final TableId tableId = + ClientServiceHandler.checkTableId(manager.getContext(), tableName, tableOp); NamespaceId namespaceId = getNamespaceIdFromTableId(tableOp, tableId); final boolean canOnlineOfflineTable; From 49932e09a3c9a47ec95b10461f0f0f6eff8b126b Mon Sep 17 00:00:00 2001 From: Kevin Rathbun Date: Wed, 16 Apr 2025 15:42:31 -0400 Subject: [PATCH 2/3] avoid RPC changes, adds new Validator --- .../core/clientImpl/TableOperationsImpl.java | 4 ++-- .../org/apache/accumulo/core/util/Validators.java | 15 +++++++++++++++ .../accumulo/manager/FateServiceHandler.java | 13 +++++-------- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java index 30b8622b1d5..17727b2b8f1 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java @@ -1463,7 +1463,7 @@ public void offline(String tableName, boolean wait) EXISTING_TABLE_NAME.validate(tableName); TableId tableId = context.getTableId(tableName); - List args = Arrays.asList(ByteBuffer.wrap(tableName.getBytes(UTF_8))); + List args = Arrays.asList(ByteBuffer.wrap(tableId.canonical().getBytes(UTF_8))); Map opts = new HashMap<>(); try { @@ -1510,7 +1510,7 @@ public void online(String tableName, boolean wait) return; } - List args = Arrays.asList(ByteBuffer.wrap(tableName.getBytes(UTF_8))); + List args = Arrays.asList(ByteBuffer.wrap(tableId.canonical().getBytes(UTF_8))); Map opts = new HashMap<>(); try { diff --git a/core/src/main/java/org/apache/accumulo/core/util/Validators.java b/core/src/main/java/org/apache/accumulo/core/util/Validators.java index 3c756ea0834..b2fc673a683 100644 --- a/core/src/main/java/org/apache/accumulo/core/util/Validators.java +++ b/core/src/main/java/org/apache/accumulo/core/util/Validators.java @@ -227,4 +227,19 @@ public static Validator sameNamespaceAs(String oldTableName) { return Validator.OK; }); + public static final Validator NOT_METADATA_TABLEID = new Validator<>(id -> { + if (id == null) { + return Optional.of("Table id must not be null"); + } + if (RootTable.ID.equals(id)) { + return Optional + .of("Table must not be the " + RootTable.NAME + "(Id: " + RootTable.ID + ") table"); + } + if (MetadataTable.ID.equals(id)) { + return Optional.of( + "Table must not be the " + MetadataTable.NAME + "(Id: " + MetadataTable.ID + ") table"); + } + return Validator.OK; + }); + } diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java b/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java index b7c444dfcbf..73fee37f6f4 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java @@ -27,6 +27,7 @@ import static org.apache.accumulo.core.util.Validators.NOT_BUILTIN_NAMESPACE; import static org.apache.accumulo.core.util.Validators.NOT_BUILTIN_TABLE; import static org.apache.accumulo.core.util.Validators.NOT_METADATA_TABLE; +import static org.apache.accumulo.core.util.Validators.NOT_METADATA_TABLEID; import static org.apache.accumulo.core.util.Validators.NOT_ROOT_TABLE_ID; import static org.apache.accumulo.core.util.Validators.VALID_TABLE_ID; import static org.apache.accumulo.core.util.Validators.sameNamespaceAs; @@ -382,11 +383,9 @@ public void executeFateOperation(TInfo tinfo, TCredentials c, long opid, FateOpe case TABLE_ONLINE: { TableOperation tableOp = TableOperation.ONLINE; validateArgumentCount(arguments, tableOp, 1); - String tableName = - validateName(arguments.get(0), tableOp, EXISTING_TABLE_NAME.and(NOT_BUILTIN_TABLE)); + final var tableId = + validateTableIdArgument(arguments.get(0), tableOp, NOT_METADATA_TABLEID); - final TableId tableId = - ClientServiceHandler.checkTableId(manager.getContext(), tableName, tableOp); NamespaceId namespaceId = getNamespaceIdFromTableId(tableOp, tableId); final boolean canOnlineOfflineTable; @@ -414,11 +413,9 @@ public void executeFateOperation(TInfo tinfo, TCredentials c, long opid, FateOpe case TABLE_OFFLINE: { TableOperation tableOp = TableOperation.OFFLINE; validateArgumentCount(arguments, tableOp, 1); - String tableName = - validateName(arguments.get(0), tableOp, EXISTING_TABLE_NAME.and(NOT_BUILTIN_TABLE)); + final var tableId = + validateTableIdArgument(arguments.get(0), tableOp, NOT_METADATA_TABLEID); - final TableId tableId = - ClientServiceHandler.checkTableId(manager.getContext(), tableName, tableOp); NamespaceId namespaceId = getNamespaceIdFromTableId(tableOp, tableId); final boolean canOnlineOfflineTable; From 781151e16f1d8c0f9e1a7e6fe9f327aab35d1905 Mon Sep 17 00:00:00 2001 From: Kevin Rathbun Date: Wed, 16 Apr 2025 15:51:29 -0400 Subject: [PATCH 3/3] fix potentially confusing validator --- .../java/org/apache/accumulo/core/util/Validators.java | 6 +----- .../apache/accumulo/manager/FateServiceHandler.java | 10 +++++----- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/util/Validators.java b/core/src/main/java/org/apache/accumulo/core/util/Validators.java index b2fc673a683..2e3df5f3e93 100644 --- a/core/src/main/java/org/apache/accumulo/core/util/Validators.java +++ b/core/src/main/java/org/apache/accumulo/core/util/Validators.java @@ -227,14 +227,10 @@ public static Validator sameNamespaceAs(String oldTableName) { return Validator.OK; }); - public static final Validator NOT_METADATA_TABLEID = new Validator<>(id -> { + public static final Validator NOT_METADATA_TABLE_ID = new Validator<>(id -> { if (id == null) { return Optional.of("Table id must not be null"); } - if (RootTable.ID.equals(id)) { - return Optional - .of("Table must not be the " + RootTable.NAME + "(Id: " + RootTable.ID + ") table"); - } if (MetadataTable.ID.equals(id)) { return Optional.of( "Table must not be the " + MetadataTable.NAME + "(Id: " + MetadataTable.ID + ") table"); diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java b/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java index 73fee37f6f4..52320225bab 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java @@ -27,7 +27,7 @@ import static org.apache.accumulo.core.util.Validators.NOT_BUILTIN_NAMESPACE; import static org.apache.accumulo.core.util.Validators.NOT_BUILTIN_TABLE; import static org.apache.accumulo.core.util.Validators.NOT_METADATA_TABLE; -import static org.apache.accumulo.core.util.Validators.NOT_METADATA_TABLEID; +import static org.apache.accumulo.core.util.Validators.NOT_METADATA_TABLE_ID; import static org.apache.accumulo.core.util.Validators.NOT_ROOT_TABLE_ID; import static org.apache.accumulo.core.util.Validators.VALID_TABLE_ID; import static org.apache.accumulo.core.util.Validators.sameNamespaceAs; @@ -383,8 +383,8 @@ public void executeFateOperation(TInfo tinfo, TCredentials c, long opid, FateOpe case TABLE_ONLINE: { TableOperation tableOp = TableOperation.ONLINE; validateArgumentCount(arguments, tableOp, 1); - final var tableId = - validateTableIdArgument(arguments.get(0), tableOp, NOT_METADATA_TABLEID); + final var tableId = validateTableIdArgument(arguments.get(0), tableOp, + NOT_ROOT_TABLE_ID.and(NOT_METADATA_TABLE_ID)); NamespaceId namespaceId = getNamespaceIdFromTableId(tableOp, tableId); @@ -413,8 +413,8 @@ public void executeFateOperation(TInfo tinfo, TCredentials c, long opid, FateOpe case TABLE_OFFLINE: { TableOperation tableOp = TableOperation.OFFLINE; validateArgumentCount(arguments, tableOp, 1); - final var tableId = - validateTableIdArgument(arguments.get(0), tableOp, NOT_METADATA_TABLEID); + final var tableId = validateTableIdArgument(arguments.get(0), tableOp, + NOT_ROOT_TABLE_ID.and(NOT_METADATA_TABLE_ID)); NamespaceId namespaceId = getNamespaceIdFromTableId(tableOp, tableId);