MINOR: Remove duplicate renewTimePeriodOpt in DelegationTokenCommand validation (#20177)

dalaoqi · web-flow · commit 3f1d830174df · 2025-08-03T16:40:18.000+08:00
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 &gt; /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.&lt;init&gt;(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 &lt;jhenyunghsu@gmail.com&gt;, TengYao Chi
 &lt;frankvicky@apache.org&gt;, Ken Huang &lt;s7133700@gmail.com&gt;, PoAn Yang
 &lt;payang@apache.org&gt;, Chia-Ping Tsai &lt;chia7712@gmail.com&gt;
diff --git 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 void checkArgs() {
             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
@@ -19,6 +19,7 @@
 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 @@ private DelegationTokenCommand.DelegationTokenCommandOptions getExpireOpts(Strin
         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();
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -300,7 +300,7 @@ public void checkArgs() {`
`300`	`300`	`CommandLineUtils.checkInvalidArgs(parser, options, createOpt, Set.of(hmacOpt, renewTimePeriodOpt, expiryTimePeriodOpt));`
`301`	`301`	`CommandLineUtils.checkInvalidArgs(parser, options, renewOpt, Set.of(renewPrincipalsOpt, maxLifeTimeOpt, expiryTimePeriodOpt, ownerPrincipalsOpt));`
`302`	`302`	`CommandLineUtils.checkInvalidArgs(parser, options, expiryOpt, Set.of(renewOpt, maxLifeTimeOpt, renewTimePeriodOpt, ownerPrincipalsOpt));`
`303`		`- CommandLineUtils.checkInvalidArgs(parser, options, describeOpt, Set.of(renewTimePeriodOpt, maxLifeTimeOpt, hmacOpt, renewTimePeriodOpt, expiryTimePeriodOpt));`
	`303`	`+ CommandLineUtils.checkInvalidArgs(parser, options, describeOpt, Set.of(renewTimePeriodOpt, maxLifeTimeOpt, hmacOpt, expiryTimePeriodOpt));`
`304`	`304`	`}`
`305`	`305`	`}`
`306`	`306`	`}`