Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider a general @User annotation #13

Open
tombentley opened this issue Dec 21, 2022 · 1 comment
Open

Consider a general @User annotation #13

tombentley opened this issue Dec 21, 2022 · 1 comment

Comments

@tombentley
Copy link
Contributor

(This is some reasoning for why #8 might be a good idea).

Currently @User is explicitly associated with @SaslPlain. That's a pattern that we could continue (e.g. @SaslScram might have its own user, and @SaslOauthBearer too). But that might be unrealistically restrictive. i.e. we're currently assuming that a provider can actually provision a user with a particular password. Some KaaS might allow to provision a user using some SASL mechanism, but not allow you to pick your own password.

So we should consider separating the declaration of a user from the credentials required to authenticate as that user. The author of a test typically doesn't care what the credentials actually are anyway. What they care about is having a number of distinct accounts (and possibly clients authenticated using those accounts). Having a bare @User annotation would also allow us to re-use it on client declarations, so specify that the client should be configured to authenticate as that user, with the provider responsible for generating passwords and supplying the JAAS config fragment needed for configuring the clients. i.e. That would allow us to express a test configuration like this:

@Test myTest(@User("alice") @User("bob") KafkaCluster, // a cluster with users alice and bob
             @User("alice") Producer<String, String> aliceProducer, // a client authenticated as alice
             @User("bob") Consumer<String, String> bobProducer) // a client authenticated as bob

The value of this is somewhat lessened unless we're also able to specify the authorization for users. And that gets verbose to do using annotations. i.e. to do this properly we end up needing to have declarative topics and ACLs, something like:

@Test myTest(
             @User("alice") 
             @User("bob") 
             @Topic("myTopic") 
             @TopicAcl(topic="myTopic", principal="alice", operation=WRITE) 
             @TopicAcl(topic="myTopic", principal="bob", operation=READ) 
             KafkaCluster, // a cluster with users alice and bob
             @User("alice") Producer<String, String> aliceProducer, // a client authenticated as alice
             @User("bob") Consumer<String, String> bobProducer) // a client authenticated as bob

OTOH if we didn't have something like this, then we end up having to support methods on KafkaCluster to let people create users and ACLs in order to support tests like this.

It is probably the case that this level of abstraction is not necessary for Kroxylicious (we don't need to write lots of tests like this, where the Kafka cluster comes preconfigured with a bunch of authorization rules). So the question is really: "Do we want to leave this direction as a possibility in the future, or are we happy with stick with the existing per-mechanism users configuration, even though that might not be suitable for all providers"

@SamBarker
Copy link
Member

TBH I see the sample with ACL's test and I want to scream! I feel like the abstraction is wrong somehow...

But then I don't end up with any better ideas.

I feel like KafkaCluster having providers for users, clients, topics and ACL might be a good place to start and then look at evolving the set of annotations based on that experience. Or maybe we want a @ClusterBuilder so that users can programatically configure a cluster that the extension takes then takes care of making that a reality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

2 participants