From ab0caec8faa32e0e9f4a00b61b5c37105803bd1c Mon Sep 17 00:00:00 2001 From: Gaurav Narula Date: Wed, 27 May 2026 11:44:09 +0100 Subject: [PATCH 1/3] KAFKA-20617: add validation for cluster-id in Formatter This change adds validation for Uuids in Formatter which is used by `kafka-storage.sh` which allows users to fail fast before they inadvertently end up using an invalid cluster-id. --- .../apache/kafka/metadata/storage/Formatter.java | 11 +++++++++++ .../kafka/metadata/storage/FormatterTest.java | 14 ++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/metadata/src/main/java/org/apache/kafka/metadata/storage/Formatter.java b/metadata/src/main/java/org/apache/kafka/metadata/storage/Formatter.java index 1ccf0c3d5c05a..f4f024770d788 100644 --- a/metadata/src/main/java/org/apache/kafka/metadata/storage/Formatter.java +++ b/metadata/src/main/java/org/apache/kafka/metadata/storage/Formatter.java @@ -236,6 +236,17 @@ public void run() throws Exception { if (clusterId == null) { throw new FormatterException("You must specify the cluster id."); } + try { + if (clusterId.contains("=")) { + throw new FormatterException("The specified cluster id, " + clusterId + " contains padding and is invalid"); + } + Uuid uuid = Uuid.fromString(clusterId); + if (Uuid.RESERVED.contains(uuid)) { + throw new FormatterException("The specified cluster id, " + clusterId + " is reserved"); + } + } catch (IllegalArgumentException e) { + throw new FormatterException("The specified cluster id, " + clusterId + " is invalid", e); + } if (directories.isEmpty()) { throw new FormatterException("You must specify at least one directory to format"); } diff --git a/metadata/src/test/java/org/apache/kafka/metadata/storage/FormatterTest.java b/metadata/src/test/java/org/apache/kafka/metadata/storage/FormatterTest.java index dcfba4328aeb7..1c87c991fca87 100644 --- a/metadata/src/test/java/org/apache/kafka/metadata/storage/FormatterTest.java +++ b/metadata/src/test/java/org/apache/kafka/metadata/storage/FormatterTest.java @@ -587,4 +587,18 @@ public void testFormatWithNoInitialControllers() throws Exception { assertNotNull(logDirProps1); } } + + @ParameterizedTest + @ValueSource(strings = {"unrvTtQISjar0JUWGU/8Pg", "igNUVIdeSPO5JCZYFhOh7Q==", "AAAAAAAAAAAAAAAAAAAAAA", "AAAAAAAAAAAAAAAAAAAAAQ"}) + public void testFormatWithInvalidClusterId(String clusterId) throws Exception { + try (TestEnv testEnv = new TestEnv(2)) { + FormatterContext formatter1 = testEnv.newFormatter(); + formatter1.formatter.setClusterId(clusterId); + String expectedPrefix = "The specified cluster id, " + clusterId; + assertEquals(expectedPrefix, + assertThrows(FormatterException.class, + formatter1.formatter::run). + getMessage().substring(0, expectedPrefix.length())); + } + } } From 2a7007900afc6b45f8ffceee3ce3a321afacdfa0 Mon Sep 17 00:00:00 2001 From: Gaurav Narula Date: Fri, 29 May 2026 14:00:42 +0100 Subject: [PATCH 2/3] Address review comments --- .../kafka/metadata/storage/Formatter.java | 23 +++++++++++++------ .../kafka/metadata/storage/FormatterTest.java | 10 ++++++++ 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/metadata/src/main/java/org/apache/kafka/metadata/storage/Formatter.java b/metadata/src/main/java/org/apache/kafka/metadata/storage/Formatter.java index f4f024770d788..2d18344d70da7 100644 --- a/metadata/src/main/java/org/apache/kafka/metadata/storage/Formatter.java +++ b/metadata/src/main/java/org/apache/kafka/metadata/storage/Formatter.java @@ -229,17 +229,19 @@ public BootstrapMetadata bootstrapMetadata() { return bootstrapMetadata; } - public void run() throws Exception { - if (nodeId < 0) { - throw new RuntimeException("You must specify a valid non-negative node ID."); - } + /** + * Validates the correctness of the given cluster id. A valid cluster id is a base64, urlencoded, no padding + * representation of a {@link Uuid}. These checks do not validate the absence of - character as + * {@link Uuid#randomUuid()} avoids them only for convenience reasons. + */ + private void validateClusterId(String clusterId) { if (clusterId == null) { throw new FormatterException("You must specify the cluster id."); } + if (clusterId.contains("=")) { + throw new FormatterException("The specified cluster id, " + clusterId + " is invalid: contains padding"); + } try { - if (clusterId.contains("=")) { - throw new FormatterException("The specified cluster id, " + clusterId + " contains padding and is invalid"); - } Uuid uuid = Uuid.fromString(clusterId); if (Uuid.RESERVED.contains(uuid)) { throw new FormatterException("The specified cluster id, " + clusterId + " is reserved"); @@ -247,6 +249,13 @@ public void run() throws Exception { } catch (IllegalArgumentException e) { throw new FormatterException("The specified cluster id, " + clusterId + " is invalid", e); } + } + + public void run() throws Exception { + if (nodeId < 0) { + throw new RuntimeException("You must specify a valid non-negative node ID."); + } + validateClusterId(clusterId); if (directories.isEmpty()) { throw new FormatterException("You must specify at least one directory to format"); } diff --git a/metadata/src/test/java/org/apache/kafka/metadata/storage/FormatterTest.java b/metadata/src/test/java/org/apache/kafka/metadata/storage/FormatterTest.java index 1c87c991fca87..46d14202b0c4a 100644 --- a/metadata/src/test/java/org/apache/kafka/metadata/storage/FormatterTest.java +++ b/metadata/src/test/java/org/apache/kafka/metadata/storage/FormatterTest.java @@ -601,4 +601,14 @@ public void testFormatWithInvalidClusterId(String clusterId) throws Exception { getMessage().substring(0, expectedPrefix.length())); } } + + @ParameterizedTest + @ValueSource(strings = {"iIygKoNNSpGzNeAEr_QW7w", "-w_KF-snTmuEnKPqZ0RDzA", "dZMgZA7nTLqZTdzb0zbvSQ", "hxqHyN2OSSmPajVhm_DR-Q"}) + public void testFormatWithValidClusterId(String clusterId) throws Exception { + try (TestEnv testEnv = new TestEnv(2)) { + FormatterContext formatter1 = testEnv.newFormatter(); + formatter1.formatter.setClusterId(clusterId); + formatter1.formatter.run(); + } + } } From 55d3e00f738fc55bff82b68e124f64fd51d160e0 Mon Sep 17 00:00:00 2001 From: Gaurav Narula Date: Fri, 29 May 2026 14:33:28 +0100 Subject: [PATCH 3/3] Stress that validating `-` would break compatibility with old cids --- .../main/java/org/apache/kafka/metadata/storage/Formatter.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/metadata/src/main/java/org/apache/kafka/metadata/storage/Formatter.java b/metadata/src/main/java/org/apache/kafka/metadata/storage/Formatter.java index 2d18344d70da7..d6b32498a67d0 100644 --- a/metadata/src/main/java/org/apache/kafka/metadata/storage/Formatter.java +++ b/metadata/src/main/java/org/apache/kafka/metadata/storage/Formatter.java @@ -232,7 +232,8 @@ public BootstrapMetadata bootstrapMetadata() { /** * Validates the correctness of the given cluster id. A valid cluster id is a base64, urlencoded, no padding * representation of a {@link Uuid}. These checks do not validate the absence of - character as - * {@link Uuid#randomUuid()} avoids them only for convenience reasons. + * {@link Uuid#randomUuid()} avoids them only for convenience reasons and such a validation would break + * compatibility when attempting to format using an old cluster id. */ private void validateClusterId(String clusterId) { if (clusterId == null) {