KeycloakService and friends #13355

oleiman · 2023-09-09T01:41:33Z

As work adding OIDC authentication to Redpanda gets underway, we would like to have
access to a locally installable/configurable identity provider service for the purposes of
experimentation and integration testing. This PR introduces a new ducktape service
(KeycloakService) to this end, built around the open source Keycloak
IAM application.

Additionally, this PR integrates OAuth/OIDC support into the existing PythonLibrdkafka
ducktape client and adds a redpanda_oauth_test.py. The latter contains a simple test
to spin up a keycloak service and demonstrate that it is alive and servicing requests.

Fixes: https://github.com/redpanda-data/core-internal/issues/748 https://github.com/redpanda-data/core-internal/issues/755

Backports Required

Release Notes

dotnwat · 2023-09-11T01:19:03Z

tests/rptest/clients/oauth_producer.py

+
+from confluent_kafka import Producer
+
+class OAuthProducer:


do we plan to integrate oauth support into other kafka clients we use in ducktape (at least the ones that support it)?

Good question. There's a line item on the epic to add oauth support to a Java client, but that's it afaik.

tests/rptest/clients/oauth_producer.py

tests/rptest/services/keycloak.py

BenPope

Looks great.

It might be worth integrating this with https://github.com/redpanda-data/redpanda/blob/dev/tests/rptest/clients/python_librdkafka.py

tests/docker/ducktape-deps/keycloak

michael-redpanda

This is looking pretty good! Can you maybe reorder/squash up your commits? I can't speak for everyone else, but I like to review PRs one commit at a time, and if there are substantial changes in architecture between commits (i.e. you dropped oauth_producer in favor of using python_librdkafka, which is good!), it makes it a little bit confusing to do that.

tests/rptest/services/keycloak.py

michael-redpanda · 2023-09-11T18:07:52Z

tests/rptest/services/keycloak.py

+        return len(self.pids(node)) > 0
+
+    def start_node(self, node, **kwargs):
+        self.logger.warn("Starting Keycloak service")


nit: info level I think is the typical level we use for these sort of messages (if not debug)

if not debug

So it is. Must have misread kerberos.py

tests/rptest/clients/oauth_producer.py

tests/rptest/tests/redpanda_oauth_test.py

oleiman · 2023-09-11T20:32:06Z

reorder/squash up...commits

@michael-redpanda - yeah, sorry. made those structural changes after Noah and maybe Ben had already taken a look. I'll squash 'em up now.

BenPope · 2023-09-12T12:19:06Z

tests/rptest/tests/redpanda_oauth_test.py

+            self.keycloak.clean_node(kc_node)
+            assert False, "Keycloak failed to start"
+
+        self.rpk.create_topic('foo')


You'll also need to create an ACL for the principal over this topic. The default mapping from the token to a principal is that the principal is extracted from the sub claim of the access token (until that becomes more configurable).

In this case that might be service-account-{CLIENT_ID}, depending on how Keycloak works.

michael-redpanda

Looking great! Couple of nits

tests/setup.py

tests/rptest/services/keycloak.py

michael-redpanda · 2023-09-12T12:42:02Z

tests/rptest/services/keycloak.py

+            # 'standardFlowEnabled': True,  # authorization flow grant
+            # 'directAccessGrantsEnabled': True, # resource owner password grant
+            # 'implicitFlowEnabled': True,  # implicit grant


nit: Any reason these should remain here? For code clarity can we remove commented out code.

These are the flags to enable different grant types on the client; left them in for pseudo-documentation purposes because they're slightly obtuse. PRD calls exclusively for client-credentials though; will remove.

tests/rptest/tests/redpanda_oauth_test.py

stale

dotnwat

plz add a PR description

oleiman · 2023-09-13T22:11:37Z

plz add a PR description

oops, not sure what happened there. done

tests/rptest/tests/redpanda_oauth_test.py

oleiman · 2023-09-19T21:17:52Z

/cdt
tests/rptest/tests/redpanda_oauth_test.py

oleiman · 2023-09-19T22:19:17Z

/cdt
dt-repeat=3
tests/rptest/tests/redpanda_oauth_test.py

oleiman · 2023-09-20T01:16:49Z

/cdt
tests/rptest/tests/redpanda_oauth_test.py

oleiman · 2023-09-20T01:58:30Z

/cdt
tests/rptest/tests/redpanda_oauth_test.py

oleiman · 2023-09-20T03:17:20Z

/cdt
tests/rptest/tests/redpanda_oauth_test.py

oleiman · 2023-09-20T21:14:51Z

/cdt
tests/rptest/tests/redpanda_oauth_test.py

oleiman · 2023-09-20T21:40:30Z

force push to (actually) fix CDT finally

michael-redpanda · 2023-09-21T14:17:31Z

tests/rptest/tests/redpanda_oauth_test.py

+        # assert len(self.produce_messages) == 3, f"Expected 3 messages, got {len(self.produce_messages)}"
+        # assert len(self.produce_errors) == 0, f"Expected 0 errors, got {len(self.produce_errors)}"


THe idea is that these will be uncommented once paired with a working OIDC implementation in RP?

So my feeling with this is, uncommented them, mark the test @ok_to_fail and then remove it once OIDC is integrated. Thoughts @BenPope ?

Eh, maybe better to remove them? I still think the other assertions here have some value in terms of testing the test.

Other option would be to just have this test be a simple service test (does this service actually work) and remove any comms with Redpanda and add them back later.

Ripped a bunch of it out. Left a few things in to demonstrate service user manipulation and that librdkafka does the correct thing when passed the OAUTHBEARER algorithm, but the rest, I agree, was just noise.

- Makes use of keycloak-python - Requires a lightly patched fork to get around a urllib3 version mismatch. - Methods for creating and updating users, clients

- Spins up a redpanda cluster and a KeycloakService - Creates a user under the demo realm, with realm-admin priv - Registers a client with keycloak - Update client's service account to include an email address - This will appear in the access token as the grant includes the 'email' scope. - Initializes PythonLibrdkafka with appropriate info (client ID, secret) - Sends some messages (this will fail without backend support)

oleiman · 2023-09-21T22:04:03Z

force pushed a rebase on dev - I think the branch fell a bit too far behind for CI

oleiman · 2023-09-22T03:29:44Z

/ci-repeat 1

BenPope

Looks great!

oleiman · 2023-09-22T14:14:22Z

/cdt
tests/rptest/tests/redpanda_oauth_test.py

michael-redpanda

lgtm

oleiman requested a review from BenPope September 9, 2023 01:41

oleiman requested a review from a team as a code owner September 9, 2023 01:41

oleiman requested review from gousteris and removed request for a team September 9, 2023 01:41

oleiman marked this pull request as draft September 9, 2023 01:42

dotnwat reviewed Sep 11, 2023

View reviewed changes

oleiman force-pushed the keycloak-DT branch from d7424b0 to 67c73ef Compare September 11, 2023 06:53

BenPope reviewed Sep 11, 2023

View reviewed changes

tests/docker/ducktape-deps/keycloak Outdated Show resolved Hide resolved

oleiman force-pushed the keycloak-DT branch from 67c73ef to 4769ceb Compare September 11, 2023 15:40

oleiman marked this pull request as ready for review September 11, 2023 17:02

oleiman requested a review from michael-redpanda September 11, 2023 17:17

michael-redpanda previously requested changes Sep 11, 2023

View reviewed changes

oleiman force-pushed the keycloak-DT branch from ef0c5b1 to 03c4ab9 Compare September 11, 2023 21:38

oleiman self-assigned this Sep 12, 2023

BenPope reviewed Sep 12, 2023

View reviewed changes

michael-redpanda self-requested a review September 12, 2023 12:29

michael-redpanda reviewed Sep 12, 2023

View reviewed changes

oleiman force-pushed the keycloak-DT branch 2 times, most recently from f47a0fa to 9141ded Compare September 13, 2023 15:12

oleiman requested review from dotnwat, BenPope and michael-redpanda September 13, 2023 15:46

dotnwat reviewed Sep 13, 2023

View reviewed changes

tests/rptest/tests/redpanda_oauth_test.py Outdated Show resolved Hide resolved

oleiman force-pushed the keycloak-DT branch 3 times, most recently from e931538 to baaaa30 Compare September 14, 2023 18:56

BenPope mentioned this pull request Sep 15, 2023

security/oidc: Initial OIDC Support for Kafka API #13476

Closed

7 tasks

oleiman requested a review from gousteris September 19, 2023 00:38

BenPope reviewed Sep 19, 2023

View reviewed changes

tests/rptest/tests/redpanda_oauth_test.py Outdated Show resolved Hide resolved

oleiman force-pushed the keycloak-DT branch from b9f7021 to 1182e7d Compare September 20, 2023 01:57

oleiman force-pushed the keycloak-DT branch from 1182e7d to 7a971a1 Compare September 20, 2023 03:16

oleiman force-pushed the keycloak-DT branch 2 times, most recently from b22a158 to a8552fe Compare September 20, 2023 21:14

michael-redpanda reviewed Sep 21, 2023

View reviewed changes

michael-redpanda requested a review from BenPope September 21, 2023 14:32

oleiman force-pushed the keycloak-DT branch from a8552fe to 08b2399 Compare September 21, 2023 20:07

oleiman requested a review from michael-redpanda September 21, 2023 20:41

oleiman added 4 commits September 21, 2023 15:02

tests/docker: Add keycloak deps and install in base ducktape image

431876b

rptest/services: Introduce KeycloakService

eb4aae3

- Makes use of keycloak-python - Requires a lightly patched fork to get around a urllib3 version mismatch. - Methods for creating and updating users, clients

rptest/clients/python_librdkafka: Introduce OAUTH flow

cb98484

oleiman force-pushed the keycloak-DT branch from 08b2399 to 8660699 Compare September 21, 2023 22:03

BenPope approved these changes Sep 22, 2023

View reviewed changes

michael-redpanda approved these changes Sep 22, 2023

View reviewed changes

oleiman merged commit 3a8ffdd into redpanda-data:dev Sep 22, 2023
9 checks passed

oleiman deleted the keycloak-DT branch September 22, 2023 18:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KeycloakService and friends #13355

KeycloakService and friends #13355

oleiman commented Sep 9, 2023 •

edited

Loading

dotnwat Sep 11, 2023

oleiman Sep 11, 2023

BenPope left a comment

michael-redpanda left a comment

michael-redpanda Sep 11, 2023

oleiman Sep 11, 2023

oleiman commented Sep 11, 2023

BenPope Sep 12, 2023

michael-redpanda left a comment

michael-redpanda Sep 12, 2023

oleiman Sep 12, 2023

dotnwat left a comment

oleiman commented Sep 13, 2023

oleiman commented Sep 19, 2023

oleiman commented Sep 19, 2023

oleiman commented Sep 20, 2023

oleiman commented Sep 20, 2023

oleiman commented Sep 20, 2023

oleiman commented Sep 20, 2023

oleiman commented Sep 20, 2023

michael-redpanda Sep 21, 2023

oleiman Sep 21, 2023

michael-redpanda Sep 21, 2023

oleiman Sep 21, 2023

michael-redpanda Sep 21, 2023

oleiman Sep 21, 2023

oleiman commented Sep 21, 2023

oleiman commented Sep 22, 2023

BenPope left a comment

oleiman commented Sep 22, 2023

michael-redpanda left a comment

		# assert len(self.produce_messages) == 3, f"Expected 3 messages, got {len(self.produce_messages)}"
		# assert len(self.produce_errors) == 0, f"Expected 0 errors, got {len(self.produce_errors)}"

KeycloakService and friends #13355

KeycloakService and friends #13355

Conversation

oleiman commented Sep 9, 2023 • edited Loading

Backports Required

Release Notes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenPope left a comment

Choose a reason for hiding this comment

michael-redpanda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oleiman commented Sep 11, 2023

Choose a reason for hiding this comment

michael-redpanda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dotnwat left a comment

Choose a reason for hiding this comment

oleiman commented Sep 13, 2023

oleiman commented Sep 19, 2023

oleiman commented Sep 19, 2023

oleiman commented Sep 20, 2023

oleiman commented Sep 20, 2023

oleiman commented Sep 20, 2023

oleiman commented Sep 20, 2023

oleiman commented Sep 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oleiman commented Sep 21, 2023

oleiman commented Sep 22, 2023

BenPope left a comment

Choose a reason for hiding this comment

oleiman commented Sep 22, 2023

michael-redpanda left a comment

Choose a reason for hiding this comment

oleiman commented Sep 9, 2023 •

edited

Loading