pprovenzano commented on code in PR #13374:
URL: https://github.com/apache/kafka/pull/13374#discussion_r1136329445
##########
core/src/test/scala/unit/kafka/tools/StorageToolTest.scala:
##########
@@ -221,4 +224,78 @@ Found problem:
assertThrows(classOf[IllegalArgumentException], () =>
parseMetadataVersion("--release-version", "0.0"))
}
+
+ @Test
+ def testAddScram():Unit = {
+ def parseAddScram(strings: String*):
Option[ArrayBuffer[UserScramCredentialRecord]] = {
+ var args = mutable.Seq("format", "-c", "config.props", "-t",
"XcZZOzUqS4yHOjhMQB6JLQ")
+ args ++= strings
+ val namespace = StorageTool.parseArguments(args.toArray)
+ StorageTool.getUserScramCredentialRecords(namespace)
+ }
+
+ var scramRecords = parseAddScram()
+ assertEquals(None, scramRecords)
+
+ // Validate we can add multiple SCRAM creds.
+ scramRecords = parseAddScram("-S",
+
"SCRAM-SHA-256=[name=alice,salt=\"MWx2NHBkbnc0ZndxN25vdGN4bTB5eTFrN3E=\",saltedpassword=\"mT0yyUUxnlJaC99HXgRTSYlbuqa4FSGtJCJfTMvjYCE=\",iterations=8192]",
+ "-S",
+
"SCRAM-SHA-256=[name=george,salt=\"MWx2NHBkbnc0ZndxN25vdGN4bTB5eTFrN3E=\",saltedpassword=\"mT0yyUUxnlJaC99HXgRTSYlbuqa4FSGtJCJfTMvjYCE=\",iterations=8192]")
+
+ assertEquals(2, scramRecords.get.size)
+
+ // Require name subfield.
+ try assertEquals(1, parseAddScram("-S",
+
"SCRAM-SHA-256=[salt=\"MWx2NHBkbnc0ZndxN25vdGN4bTB5eTFrN3E=\",saltedpassword=\"mT0yyUUxnlJaC99HXgRTSYlbuqa4FSGtJCJfTMvjYCE=\",iterations=8192]"))
catch {
+ case e: TerseFailure => assertEquals(s"You must supply 'name' to
add-scram", e.getMessage)
+ }
+
+ // Require password xor saltedpassword
+ try assertEquals(1, parseAddScram("-S",
+
"SCRAM-SHA-256=[name=alice,salt=\"MWx2NHBkbnc0ZndxN25vdGN4bTB5eTFrN3E=\",password=alice,saltedpassword=\"mT0yyUUxnlJaC99HXgRTSYlbuqa4FSGtJCJfTMvjYCE=\",iterations=8192]"))
+ catch {
+ case e: TerseFailure => assertEquals(s"You must only supply one of
'password' or 'saltedpassword' to add-scram", e.getMessage)
+ }
+
+ try assertEquals(1, parseAddScram("-S",
+
"SCRAM-SHA-256=[name=alice,salt=\"MWx2NHBkbnc0ZndxN25vdGN4bTB5eTFrN3E=\",iterations=8192]"))
+ catch {
+ case e: TerseFailure => assertEquals(s"You must supply one of 'password'
or 'saltedpassword' to add-scram", e.getMessage)
+ }
+
+ // Validate salt is required with saltedpassword
+ try assertEquals(1, parseAddScram("-S",
+
"SCRAM-SHA-256=[name=alice,saltedpassword=\"mT0yyUUxnlJaC99HXgRTSYlbuqa4FSGtJCJfTMvjYCE=\",iterations=8192]"))
+ catch {
+ case e: TerseFailure => assertEquals(s"You must supply 'salt' with
'saltedpassword' to add-scram", e.getMessage)
+ }
+
+ // Validate salt is optional with password
+ assertEquals(1, parseAddScram("-S",
"SCRAM-SHA-256=[name=alice,password=alice,iterations=4096]").get.size)
+
+ // Require 4096 <= iterations <= 16384
+ try assertEquals(1, parseAddScram("-S",
+
"SCRAM-SHA-256=[name=alice,salt=\"MWx2NHBkbnc0ZndxN25vdGN4bTB5eTFrN3E=\",password=alice,iterations=16385]"))
+ catch {
+ case e: TerseFailure => assertEquals(s"The 'iterations' value must be <=
16384 for add-scram", e.getMessage)
+ }
+
+ assertEquals(1, parseAddScram("-S",
+
"SCRAM-SHA-256=[name=alice,salt=\"MWx2NHBkbnc0ZndxN25vdGN4bTB5eTFrN3E=\",password=alice,iterations=16384]")
+ .get.size)
+
+ try assertEquals(1, parseAddScram("-S",
+
"SCRAM-SHA-256=[name=alice,salt=\"MWx2NHBkbnc0ZndxN25vdGN4bTB5eTFrN3E=\",password=alice,iterations=4095]"))
+ catch {
+ case e: TerseFailure => assertEquals(s"The 'iterations' value must be >=
4096 for add-scram", e.getMessage)
+ }
+
+ assertEquals(1, parseAddScram("-S",
+
"SCRAM-SHA-256=[name=alice,salt=\"MWx2NHBkbnc0ZndxN25vdGN4bTB5eTFrN3E=\",password=alice,iterations=4096]")
+ .get.size)
+
+ // Validate iterations is optional
+ assertEquals(1, parseAddScram("-S",
"SCRAM-SHA-256=[name=alice,password=alice]") .get.size)
Review Comment:
There is a check for metadata version in ScramImage and a test to validate
that check. I have manually tested that the formatter produces an error if the
IBP version is set to an earlier version and SCRAM records are added. Testing
this in a unit test is a lot of work and not worth it as the cluster will just
fail to come up if they bypass it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
