omkreddy commented on code in PR #13114:
URL: https://github.com/apache/kafka/pull/13114#discussion_r1106751131
##########
core/src/main/scala/kafka/server/KafkaApis.scala:
##########
@@ -3296,17 +3296,23 @@ class KafkaApis(val requestChannel: RequestChannel,
}
def handleDescribeUserScramCredentialsRequest(request:
RequestChannel.Request): Unit = {
Review Comment:
As I mentioned above, ControllerAPI also should support
DescribeUserScramCredentialsRequest API
##########
core/src/test/scala/unit/kafka/server/AlterUserScramCredentialsRequestTest.scala:
##########
@@ -260,11 +271,13 @@ class AlterUserScramCredentialsRequestTest extends
BaseRequestTest {
.setSalt(saltBytes)
.setSaltedPassword(saltedPasswordBytes),
))).build()
- val results1 = sendAlterUserScramCredentialsRequest(request1).data.results
- assertEquals(2, results1.size)
- checkNoErrorsAlteringCredentials(results1)
- checkUserAppearsInAlterResults(results1, user1)
- checkUserAppearsInAlterResults(results1, user2)
+ val results1_1 =
sendAlterUserScramCredentialsRequest(request1_1).data.results
+ assertEquals(2, results1_1.size)
+ checkNoErrorsAlteringCredentials(results1_1)
+ checkUserAppearsInAlterResults(results1_1, user1)
+ checkUserAppearsInAlterResults(results1_1, user2)
+
+ Thread.sleep(10000)
Review Comment:
we generally use 'TestUtils.waitForCondition` in tests
##########
metadata/src/main/java/org/apache/kafka/image/ScramImage.java:
##########
@@ -50,6 +58,63 @@ public void write(ImageWriter writer, ImageWriterOptions
options) {
}
}
+ public DescribeUserScramCredentialsResponseData
describe(DescribeUserScramCredentialsRequestData request) {
+
+ List<UserName> users = request.users();
+ Map<String, Boolean> uniqueUsers = new HashMap<String, Boolean>();
+
+ if ((users == null) || (users.size() == 0)) {
+ System.out.println("Describe : get all the users");
+ // If there are no users listed then get all the users
+ for (Map<String, ScramCredentialData> scramCredentialDataSet :
mechanisms.values()) {
+ for (String user : scramCredentialDataSet.keySet()) {
+ uniqueUsers.put(user, false);
+ }
+ }
+ } else {
+ // Filter out duplicates
+ for (UserName user : users) {
+ if (uniqueUsers.containsKey(user.name())) {
+ uniqueUsers.put(user.name(), true);
+ } else {
+ uniqueUsers.put(user.name(), false);
+ }
+ }
+ }
+
+ DescribeUserScramCredentialsResponseData retval = new
DescribeUserScramCredentialsResponseData();
+
+ for (Map.Entry<String, Boolean> user : uniqueUsers.entrySet()) {
+ DescribeUserScramCredentialsResult result =
+ new DescribeUserScramCredentialsResult().setUser(user.getKey());
+
+ if (user.getValue() == false) {
+ List<CredentialInfo> credentialInfos = new
ArrayList<CredentialInfo>();
+
+ boolean datafound = false;
+ for (Map.Entry<ScramMechanism, Map<String,
ScramCredentialData>> mechanismsEntry : mechanisms.entrySet()) {
+ Map<String, ScramCredentialData> credentialDataSet =
mechanismsEntry.getValue();
+ if (credentialDataSet.containsKey(user.getKey())) {
+ credentialInfos.add(new
CredentialInfo().setMechanism(mechanismsEntry.getKey().type())
+
.setIterations(credentialDataSet.get(user.getKey()).iterations()));
+ datafound = true;
+ }
+ }
+ if (datafound) {
+ result.setCredentialInfos(credentialInfos);
+ } else {
+ result.setErrorCode(Errors.RESOURCE_NOT_FOUND.code())
+
.setErrorMessage("attemptToDescribeUserThatDoesNotExist: " + user.getKey());
Review Comment:
Can we use same error message as
https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/server/ZkAdminManager.scala#L802
##########
metadata/src/main/java/org/apache/kafka/image/ScramImage.java:
##########
@@ -50,6 +58,63 @@ public void write(ImageWriter writer, ImageWriterOptions
options) {
}
}
+ public DescribeUserScramCredentialsResponseData
describe(DescribeUserScramCredentialsRequestData request) {
+
+ List<UserName> users = request.users();
+ Map<String, Boolean> uniqueUsers = new HashMap<String, Boolean>();
+
+ if ((users == null) || (users.size() == 0)) {
+ System.out.println("Describe : get all the users");
+ // If there are no users listed then get all the users
+ for (Map<String, ScramCredentialData> scramCredentialDataSet :
mechanisms.values()) {
+ for (String user : scramCredentialDataSet.keySet()) {
+ uniqueUsers.put(user, false);
+ }
+ }
+ } else {
+ // Filter out duplicates
+ for (UserName user : users) {
+ if (uniqueUsers.containsKey(user.name())) {
+ uniqueUsers.put(user.name(), true);
+ } else {
+ uniqueUsers.put(user.name(), false);
+ }
+ }
+ }
+
+ DescribeUserScramCredentialsResponseData retval = new
DescribeUserScramCredentialsResponseData();
+
+ for (Map.Entry<String, Boolean> user : uniqueUsers.entrySet()) {
+ DescribeUserScramCredentialsResult result =
+ new DescribeUserScramCredentialsResult().setUser(user.getKey());
+
+ if (user.getValue() == false) {
+ List<CredentialInfo> credentialInfos = new
ArrayList<CredentialInfo>();
+
+ boolean datafound = false;
+ for (Map.Entry<ScramMechanism, Map<String,
ScramCredentialData>> mechanismsEntry : mechanisms.entrySet()) {
+ Map<String, ScramCredentialData> credentialDataSet =
mechanismsEntry.getValue();
+ if (credentialDataSet.containsKey(user.getKey())) {
+ credentialInfos.add(new
CredentialInfo().setMechanism(mechanismsEntry.getKey().type())
+
.setIterations(credentialDataSet.get(user.getKey()).iterations()));
+ datafound = true;
+ }
+ }
+ if (datafound) {
+ result.setCredentialInfos(credentialInfos);
+ } else {
+ result.setErrorCode(Errors.RESOURCE_NOT_FOUND.code())
Review Comment:
Do we have separate error message for "Username must not be empty"
##########
metadata/src/main/java/org/apache/kafka/image/ScramImage.java:
##########
@@ -50,6 +58,63 @@ public void write(ImageWriter writer, ImageWriterOptions
options) {
}
}
+ public DescribeUserScramCredentialsResponseData
describe(DescribeUserScramCredentialsRequestData request) {
+
+ List<UserName> users = request.users();
+ Map<String, Boolean> uniqueUsers = new HashMap<String, Boolean>();
+
+ if ((users == null) || (users.size() == 0)) {
+ System.out.println("Describe : get all the users");
+ // If there are no users listed then get all the users
+ for (Map<String, ScramCredentialData> scramCredentialDataSet :
mechanisms.values()) {
+ for (String user : scramCredentialDataSet.keySet()) {
+ uniqueUsers.put(user, false);
+ }
+ }
+ } else {
+ // Filter out duplicates
+ for (UserName user : users) {
+ if (uniqueUsers.containsKey(user.name())) {
+ uniqueUsers.put(user.name(), true);
+ } else {
+ uniqueUsers.put(user.name(), false);
+ }
+ }
+ }
+
+ DescribeUserScramCredentialsResponseData retval = new
DescribeUserScramCredentialsResponseData();
+
+ for (Map.Entry<String, Boolean> user : uniqueUsers.entrySet()) {
+ DescribeUserScramCredentialsResult result =
+ new DescribeUserScramCredentialsResult().setUser(user.getKey());
+
+ if (user.getValue() == false) {
+ List<CredentialInfo> credentialInfos = new
ArrayList<CredentialInfo>();
+
+ boolean datafound = false;
+ for (Map.Entry<ScramMechanism, Map<String,
ScramCredentialData>> mechanismsEntry : mechanisms.entrySet()) {
Review Comment:
Dont we need to check for ScramMechanism.UNKNOWN
similar to
https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/server/ZkAdminManager.scala#L812
and
https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/server/ZkAdminManager.scala#L816
##########
core/src/main/scala/kafka/server/KafkaApis.scala:
##########
@@ -3611,12 +3617,4 @@ object KafkaApis {
private def unsupported(text: String): Exception = {
new UnsupportedVersionException(s"Unsupported when using a Raft-based
metadata quorum: $text")
}
-
- private def notYetSupported(request: RequestChannel.Request): Exception = {
Review Comment:
maybe we can keep them for now
##########
core/src/main/scala/kafka/server/ControllerApis.scala:
##########
@@ -99,6 +99,7 @@ class ControllerApis(val requestChannel: RequestChannel,
case ApiKeys.INCREMENTAL_ALTER_CONFIGS =>
handleIncrementalAlterConfigs(request)
case ApiKeys.ALTER_PARTITION_REASSIGNMENTS =>
handleAlterPartitionReassignments(request)
case ApiKeys.LIST_PARTITION_REASSIGNMENTS =>
handleListPartitionReassignments(request)
+ case ApiKeys.ALTER_USER_SCRAM_CREDENTIALS =>
handleAlterUserScramCredentials(request)
Review Comment:
I think, Similar to other APIs (CREATE_ACLS, DESCRIBE_ACLS, etc), we should
also support DescribeUserScramCredentialsRequest API here.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
