From 3f1d830174df26a2459a0317275ba5b1b6e4eac9 Mon Sep 17 00:00:00 2001 From: "Tsung-Han Ho (Miles Ho)" <13413575+dalaoqi@users.noreply.github.com> Date: Sun, 3 Aug 2025 16:40:18 +0800 Subject: [PATCH] MINOR: Remove duplicate renewTimePeriodOpt in DelegationTokenCommand validation (#20177) The bug was a duplicate parameter validation in the `DelegationTokenCommand` class. The `checkInvalidArgs` method for the `describeOpt` was incorrectly including `renewTimePeriodOpt` twice in the set of invalid arguments. This bug caused unexpected command errors during E2E testing. ### Before the fix: The following command would fail due to the duplicate validation logic: ``` TC_PATHS="tests/kafkatest/tests/core/delegation_token_test.py::DelegationTokenTest" /bin/bash tests/docker/run_tests.sh ``` ### Error output: ``` ducktape.cluster.remoteaccount.RemoteCommandError: ducker@ducker03: Command 'KAFKA_OPTS="-Djava.security.auth.login.config=/mnt/security/jaas.conf -Djava.security.krb5.conf=/mnt/security/krb5.conf" /opt/kafka-dev/bin/kafka-delegation-tokens.sh --bootstrap-server ducker03:9094 --create --max-life-time-period -1 --command-config /mnt/kafka/client.properties > /mnt/kafka/delegation_token.out' returned non-zero exit status 1. Remote error message: b'duplicate element: [renew-time-period]\njava.lang.IllegalArgumentException: duplicate element: [renew-time-period]\n\tat java.base/java.util.ImmutableCollections$SetN.(ImmutableCollections.java:918)\n\tat java.base/java.util.Set.of(Set.java:544)\n\tat org.apache.kafka.tools.DelegationTokenCommand$DelegationTokenCommandOptions.checkArgs(DelegationTokenCommand.java:304)\n\tat org.apache.kafka.tools.DelegationTokenCommand.execute(DelegationTokenCommand.java:79)\n\tat org.apache.kafka.tools.DelegationTokenCommand.mainNoExit(DelegationTokenCommand.java:57)\n\tat org.apache.kafka.tools.DelegationTokenCommand.main(DelegationTokenCommand.java:52)\n\n' [INFO:2025-07-31 11:27:25,531]: RunnerClient: kafkatest.tests.core.delegation_token_test.DelegationTokenTest.test_delegation_token_lifecycle.metadata_quorum=ISOLATED_KRAFT: Data: None ================================================================================ SESSION REPORT (ALL TESTS) ducktape version: 0.12.0 session_id: 2025-07-31--002 run time: 33.213 seconds tests run: 1 passed: 0 flaky: 0 failed: 1 ignored: 0 ================================================================================ test_id: kafkatest.tests.core.delegation_token_test.DelegationTokenTest.test_delegation_token_lifecycle.metadata_quorum=ISOLATED_KRAFT status: FAIL run time: 33.090 seconds ``` ### After the fix: The same command now executes successfully: ``` TC_PATHS="tests/kafkatest/tests/core/delegation_token_test.py::DelegationTokenTest" /bin/bash tests/docker/run_tests.sh ``` ### Success output: ``` ================================================================================ SESSION REPORT (ALL TESTS) ducktape version: 0.12.0 session_id: 2025-07-31--001 run time: 35.488 seconds tests run: 1 passed: 1 flaky: 0 failed: 0 ignored: 0 ================================================================================ test_id: kafkatest.tests.core.delegation_token_test.DelegationTokenTest.test_delegation_token_lifecycle.metadata_quorum=ISOLATED_KRAFT status: PASS run time: 35.363 seconds -------------------------------------------------------------------------------- ``` Reviewers: Jhen-Yung Hsu , TengYao Chi , Ken Huang , PoAn Yang , Chia-Ping Tsai --- .../kafka/tools/DelegationTokenCommand.java | 2 +- .../tools/DelegationTokenCommandTest.java | 153 ++++++++++++++++++ 2 files changed, 154 insertions(+), 1 deletion(-) diff --git a/tools/src/main/java/org/apache/kafka/tools/DelegationTokenCommand.java b/tools/src/main/java/org/apache/kafka/tools/DelegationTokenCommand.java index dad388fee6b..da1c1cfcb78 100644 --- a/tools/src/main/java/org/apache/kafka/tools/DelegationTokenCommand.java +++ b/tools/src/main/java/org/apache/kafka/tools/DelegationTokenCommand.java @@ -300,7 +300,7 @@ public class DelegationTokenCommand { CommandLineUtils.checkInvalidArgs(parser, options, createOpt, Set.of(hmacOpt, renewTimePeriodOpt, expiryTimePeriodOpt)); CommandLineUtils.checkInvalidArgs(parser, options, renewOpt, Set.of(renewPrincipalsOpt, maxLifeTimeOpt, expiryTimePeriodOpt, ownerPrincipalsOpt)); CommandLineUtils.checkInvalidArgs(parser, options, expiryOpt, Set.of(renewOpt, maxLifeTimeOpt, renewTimePeriodOpt, ownerPrincipalsOpt)); - CommandLineUtils.checkInvalidArgs(parser, options, describeOpt, Set.of(renewTimePeriodOpt, maxLifeTimeOpt, hmacOpt, renewTimePeriodOpt, expiryTimePeriodOpt)); + CommandLineUtils.checkInvalidArgs(parser, options, describeOpt, Set.of(renewTimePeriodOpt, maxLifeTimeOpt, hmacOpt, expiryTimePeriodOpt)); } } } diff --git a/tools/src/test/java/org/apache/kafka/tools/DelegationTokenCommandTest.java b/tools/src/test/java/org/apache/kafka/tools/DelegationTokenCommandTest.java index f0ac5a7a8ed..be0b744a7a4 100644 --- a/tools/src/test/java/org/apache/kafka/tools/DelegationTokenCommandTest.java +++ b/tools/src/test/java/org/apache/kafka/tools/DelegationTokenCommandTest.java @@ -19,6 +19,7 @@ package org.apache.kafka.tools; import org.apache.kafka.clients.admin.Admin; import org.apache.kafka.clients.admin.MockAdminClient; import org.apache.kafka.common.security.token.delegation.DelegationToken; +import org.apache.kafka.common.utils.Exit; import org.junit.jupiter.api.Test; @@ -109,4 +110,156 @@ public class DelegationTokenCommandTest { String[] args = {"--bootstrap-server", "localhost:9092", "--command-config", "testfile", "--expire", "--expiry-time-period", "-1", "--hmac", hmac}; return new DelegationTokenCommand.DelegationTokenCommandOptions(args); } + + + @Test + public void testCheckArgsMissingRequiredArgs() { + Exit.setExitProcedure((exitCode, message) -> { + throw new RuntimeException("Exit with code " + exitCode + ": " + message); + }); + try { + String[] argsCreateMissingBootstrap = {"--command-config", "testfile", "--create", "--max-life-time-period", "604800000"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsCreateMissingBootstrap = new DelegationTokenCommand.DelegationTokenCommandOptions(argsCreateMissingBootstrap); + assertThrows(RuntimeException.class, optsCreateMissingBootstrap::checkArgs); + + String[] argsCreateMissingConfig = {"--bootstrap-server", "localhost:9092", "--create", "--max-life-time-period", "604800000"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsCreateMissingConfig = new DelegationTokenCommand.DelegationTokenCommandOptions(argsCreateMissingConfig); + assertThrows(RuntimeException.class, optsCreateMissingConfig::checkArgs); + + String[] argsCreateMissingMaxLife = {"--bootstrap-server", "localhost:9092", "--command-config", "testfile", "--create"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsCreateMissingMaxLife = new DelegationTokenCommand.DelegationTokenCommandOptions(argsCreateMissingMaxLife); + assertThrows(RuntimeException.class, optsCreateMissingMaxLife::checkArgs); + + String[] argsRenewMissingBootstrap = {"--command-config", "testfile", "--renew", "--hmac", "test-hmac", "--renew-time-period", "604800000"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsRenewMissingBootstrap = new DelegationTokenCommand.DelegationTokenCommandOptions(argsRenewMissingBootstrap); + assertThrows(RuntimeException.class, optsRenewMissingBootstrap::checkArgs); + + String[] argsRenewMissingConfig = {"--bootstrap-server", "localhost:9092", "--renew", "--hmac", "test-hmac", "--renew-time-period", "604800000"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsRenewMissingConfig = new DelegationTokenCommand.DelegationTokenCommandOptions(argsRenewMissingConfig); + assertThrows(RuntimeException.class, optsRenewMissingConfig::checkArgs); + + String[] argsRenewMissingHmac = {"--bootstrap-server", "localhost:9092", "--command-config", "testfile", "--renew", "--renew-time-period", "604800000"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsRenewMissingHmac = new DelegationTokenCommand.DelegationTokenCommandOptions(argsRenewMissingHmac); + assertThrows(RuntimeException.class, optsRenewMissingHmac::checkArgs); + + String[] argsRenewMissingRenewTime = {"--bootstrap-server", "localhost:9092", "--command-config", "testfile", "--renew", "--hmac", "test-hmac"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsRenewMissingRenewTime = new DelegationTokenCommand.DelegationTokenCommandOptions(argsRenewMissingRenewTime); + assertThrows(RuntimeException.class, optsRenewMissingRenewTime::checkArgs); + + String[] argsExpireMissingBootstrap = {"--command-config", "testfile", "--expire", "--hmac", "test-hmac", "--expiry-time-period", "604800000"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsExpireMissingBootstrap = new DelegationTokenCommand.DelegationTokenCommandOptions(argsExpireMissingBootstrap); + assertThrows(RuntimeException.class, optsExpireMissingBootstrap::checkArgs); + + String[] argsExpireMissingConfig = {"--bootstrap-server", "localhost:9092", "--expire", "--hmac", "test-hmac", "--expiry-time-period", "604800000"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsExpireMissingConfig = new DelegationTokenCommand.DelegationTokenCommandOptions(argsExpireMissingConfig); + assertThrows(RuntimeException.class, optsExpireMissingConfig::checkArgs); + + String[] argsExpireMissingHmac = {"--bootstrap-server", "localhost:9092", "--command-config", "testfile", "--expire", "--expiry-time-period", "604800000"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsExpireMissingHmac = new DelegationTokenCommand.DelegationTokenCommandOptions(argsExpireMissingHmac); + assertThrows(RuntimeException.class, optsExpireMissingHmac::checkArgs); + + String[] argsExpireMissingExpiryTime = {"--bootstrap-server", "localhost:9092", "--command-config", "testfile", "--expire", "--hmac", "test-hmac"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsExpireMissingExpiryTime = new DelegationTokenCommand.DelegationTokenCommandOptions(argsExpireMissingExpiryTime); + assertThrows(RuntimeException.class, optsExpireMissingExpiryTime::checkArgs); + + String[] argsDescribeMissingBootstrap = {"--command-config", "testfile", "--describe"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsDescribeMissingBootstrap = new DelegationTokenCommand.DelegationTokenCommandOptions(argsDescribeMissingBootstrap); + assertThrows(RuntimeException.class, optsDescribeMissingBootstrap::checkArgs); + + String[] argsDescribeMissingConfig = {"--bootstrap-server", "localhost:9092", "--describe"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsDescribeMissingConfig = new DelegationTokenCommand.DelegationTokenCommandOptions(argsDescribeMissingConfig); + assertThrows(RuntimeException.class, optsDescribeMissingConfig::checkArgs); + } finally { + Exit.resetExitProcedure(); + } + } + + @Test + public void testCheckArgsInvalidArgs() { + Exit.setExitProcedure((exitCode, message) -> { + throw new RuntimeException("Exit with code " + exitCode + ": " + message); + }); + try { + String[] argsCreateWithHmac = {"--bootstrap-server", "localhost:9092", "--command-config", "testfile", "--create", "--max-life-time-period", "604800000", "--hmac", "test-hmac"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsCreateWithHmac = new DelegationTokenCommand.DelegationTokenCommandOptions(argsCreateWithHmac); + assertThrows(RuntimeException.class, optsCreateWithHmac::checkArgs); + + String[] argsCreateWithRenewTime = {"--bootstrap-server", "localhost:9092", "--command-config", "testfile", "--create", "--max-life-time-period", "604800000", "--renew-time-period", "604800000"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsCreateWithRenewTime = new DelegationTokenCommand.DelegationTokenCommandOptions(argsCreateWithRenewTime); + assertThrows(RuntimeException.class, optsCreateWithRenewTime::checkArgs); + + String[] argsCreateWithExpiryTime = {"--bootstrap-server", "localhost:9092", "--command-config", "testfile", "--create", "--max-life-time-period", "604800000", "--expiry-time-period", "604800000"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsCreateWithExpiryTime = new DelegationTokenCommand.DelegationTokenCommandOptions(argsCreateWithExpiryTime); + assertThrows(RuntimeException.class, optsCreateWithExpiryTime::checkArgs); + + String[] argsRenewWithRenewPrincipals = {"--bootstrap-server", "localhost:9092", "--command-config", "testfile", "--renew", "--hmac", "test-hmac", "--renew-time-period", "604800000", "--renewer-principal", "User:renewer"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsRenewWithRenewPrincipals = new DelegationTokenCommand.DelegationTokenCommandOptions(argsRenewWithRenewPrincipals); + assertThrows(RuntimeException.class, optsRenewWithRenewPrincipals::checkArgs); + + String[] argsRenewWithMaxLife = {"--bootstrap-server", "localhost:9092", "--command-config", "testfile", "--renew", "--hmac", "test-hmac", "--renew-time-period", "604800000", "--max-life-time-period", "604800000"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsRenewWithMaxLife = new DelegationTokenCommand.DelegationTokenCommandOptions(argsRenewWithMaxLife); + assertThrows(RuntimeException.class, optsRenewWithMaxLife::checkArgs); + + String[] argsRenewWithExpiryTime = {"--bootstrap-server", "localhost:9092", "--command-config", "testfile", "--renew", "--hmac", "test-hmac", "--renew-time-period", "604800000", "--expiry-time-period", "604800000"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsRenewWithExpiryTime = new DelegationTokenCommand.DelegationTokenCommandOptions(argsRenewWithExpiryTime); + assertThrows(RuntimeException.class, optsRenewWithExpiryTime::checkArgs); + + String[] argsRenewWithOwner = {"--bootstrap-server", "localhost:9092", "--command-config", "testfile", "--renew", "--hmac", "test-hmac", "--renew-time-period", "604800000", "--owner-principal", "User:owner"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsRenewWithOwner = new DelegationTokenCommand.DelegationTokenCommandOptions(argsRenewWithOwner); + assertThrows(RuntimeException.class, optsRenewWithOwner::checkArgs); + + String[] argsExpireWithRenew = {"--bootstrap-server", "localhost:9092", "--command-config", "testfile", "--expire", "--renew", "--hmac", "test-hmac", "--expiry-time-period", "604800000"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsExpireWithRenew = new DelegationTokenCommand.DelegationTokenCommandOptions(argsExpireWithRenew); + assertThrows(RuntimeException.class, optsExpireWithRenew::checkArgs); + + String[] argsExpireWithMaxLife = {"--bootstrap-server", "localhost:9092", "--command-config", "testfile", "--expire", "--hmac", "test-hmac", "--expiry-time-period", "604800000", "--max-life-time-period", "604800000"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsExpireWithMaxLife = new DelegationTokenCommand.DelegationTokenCommandOptions(argsExpireWithMaxLife); + assertThrows(RuntimeException.class, optsExpireWithMaxLife::checkArgs); + + String[] argsExpireWithRenewTime = {"--bootstrap-server", "localhost:9092", "--command-config", "testfile", "--expire", "--hmac", "test-hmac", "--expiry-time-period", "604800000", "--renew-time-period", "604800000"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsExpireWithRenewTime = new DelegationTokenCommand.DelegationTokenCommandOptions(argsExpireWithRenewTime); + assertThrows(RuntimeException.class, optsExpireWithRenewTime::checkArgs); + + String[] argsExpireWithOwner = {"--bootstrap-server", "localhost:9092", "--command-config", "testfile", "--expire", "--hmac", "test-hmac", "--expiry-time-period", "604800000", "--owner-principal", "User:owner"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsExpireWithOwner = new DelegationTokenCommand.DelegationTokenCommandOptions(argsExpireWithOwner); + assertThrows(RuntimeException.class, optsExpireWithOwner::checkArgs); + + String[] argsDescribeWithRenewTime = {"--bootstrap-server", "localhost:9092", "--command-config", "testfile", "--describe", "--renew-time-period", "604800000"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsDescribeWithRenewTime = new DelegationTokenCommand.DelegationTokenCommandOptions(argsDescribeWithRenewTime); + assertThrows(RuntimeException.class, optsDescribeWithRenewTime::checkArgs); + + String[] argsDescribeWithExpiryTime = {"--bootstrap-server", "localhost:9092", "--command-config", "testfile", "--describe", "--expiry-time-period", "604800000"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsDescribeWithExpiryTime = new DelegationTokenCommand.DelegationTokenCommandOptions(argsDescribeWithExpiryTime); + assertThrows(RuntimeException.class, optsDescribeWithExpiryTime::checkArgs); + + String[] argsDescribeWithMaxLife = {"--bootstrap-server", "localhost:9092", "--command-config", "testfile", "--describe", "--max-life-time-period", "604800000"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsDescribeWithMaxLife = new DelegationTokenCommand.DelegationTokenCommandOptions(argsDescribeWithMaxLife); + assertThrows(RuntimeException.class, optsDescribeWithMaxLife::checkArgs); + + String[] argsDescribeWithHmac = {"--bootstrap-server", "localhost:9092", "--command-config", "testfile", "--describe", "--hmac", "test-hmac"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsDescribeWithHmac = new DelegationTokenCommand.DelegationTokenCommandOptions(argsDescribeWithHmac); + assertThrows(RuntimeException.class, optsDescribeWithHmac::checkArgs); + } finally { + Exit.resetExitProcedure(); + } + } + + @Test + public void testCheckArgsValidOperations() { + String[] argsCreate = {"--bootstrap-server", "localhost:9092", "--command-config", "testfile", "--create", "--max-life-time-period", "604800000"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsCreate = new DelegationTokenCommand.DelegationTokenCommandOptions(argsCreate); + optsCreate.checkArgs(); + + String[] argsRenew = {"--bootstrap-server", "localhost:9092", "--command-config", "testfile", "--renew", "--hmac", "test-hmac", "--renew-time-period", "604800000"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsRenew = new DelegationTokenCommand.DelegationTokenCommandOptions(argsRenew); + optsRenew.checkArgs(); + + String[] argsExpire = {"--bootstrap-server", "localhost:9092", "--command-config", "testfile", "--expire", "--hmac", "test-hmac", "--expiry-time-period", "604800000"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsExpire = new DelegationTokenCommand.DelegationTokenCommandOptions(argsExpire); + optsExpire.checkArgs(); + + String[] argsDescribe = {"--bootstrap-server", "localhost:9092", "--command-config", "testfile", "--describe"}; + DelegationTokenCommand.DelegationTokenCommandOptions optsDescribe = new DelegationTokenCommand.DelegationTokenCommandOptions(argsDescribe); + optsDescribe.checkArgs(); + } }