Factor User into a top-level repeatable annotation #8
Conversation
The idea being that @user could also be used to support a @SaslScramSha annotation, and maybe @SaslOauthBearer too.
There was a problem hiding this comment.
Given the move to express users independently of the auth mechanism how would one express a test using multiple mechanisms? I'm thinking of cases trying to test that each mechanism is available in parallel?
The simplest way to me would be to express it as:
SASL_PLAIN->user1withpassword1SASL_SCRAM_SHA->user2withpassword2
Then assert that each user only works with the appropriate mechanism.
| @Retention(RetentionPolicy.RUNTIME) | ||
| @Repeatable(User.List.class) | ||
| @KafkaClusterConstraint | ||
| public @interface User { |
There was a problem hiding this comment.
How about Credentials rather than User?
|
|
||
| @Test | ||
| public void saslPlainAuthenticatingClusterParameterTwoUsers( | ||
| @BrokerCluster @SaslPlainAuth @User(user = "alice", password = "foo") @User(user = "bob", password = "bar") KafkaCluster cluster) |
There was a problem hiding this comment.
I found the old version easier to think about.
@SaslPlainAuth({
@SaslPlainAuth.UserPassword(user = "alice", password = "foo"),
@SaslPlainAuth.UserPassword(user = "bob", password = "bar")
}Clearly says to me that these users are authenticated via Sasl Plain. Where as the new version you can assume the same but it's less explicit.
How much do we care? I mean, there are two ways you could implement that: Via two listeners with a different mechanism for each one, or a single listener with multiple mechanisms. We could allow expressing both of those via annotations. But should we? How many test cases are there where that's something you actually care about? I think 90% of the value is being able to execute test cases with a particular mechanism. If you need to test multiple mechanisms then you can use |
|
+1 from me. I like the separation of concerns - the declaration of of the user/password is separated from the means used to send those over the wire (the SASL mechanism). |
|
SonarCloud Quality Gate failed.
|
|
To follow |
The idea being that @user could also be used to support a @SaslScramSha annotation, and maybe @SaslOauthBearer too.