[#3514] Add support for AWS MSK IAM authentication using SASL OAUTHBEARER

[qswinson]

Fixes #3514

Proposed Changes

Add support for AWS MSK IAM authentication using the SASL OAUTHBEARER mechanism. This PR supports using either the container's default credentials or supplying a Role ARN to be assumed.

The control plane configurations are managed through properties in the kafka-auth secret.

apiVersion: v1
kind: Secret
metadata:
  name: kafka-auth
  namespace: knative-eventing
type: Opaque
stringData:
  protocol: SASL_SSL
  sasl.mechanism: OAUTHBEARER
  type: OAUTHBEARER
  tokenProvider: MSKAccessTokenProvider

or

apiVersion: v1
kind: Secret
metadata:
  name: kafka-auth
  namespace: knative-eventing
type: Opaque
stringData:
  protocol: SASL_SSL
  sasl.mechanism: OAUTHBEARER
  type: OAUTHBEARER
  tokenProvider: MSKRoleAccessTokenProvider
  roleARN: arn:aws:iam::123456789012:role/role-that-can-access-msk

The data plane configuration is managed through sasl java properties. These must be added to each config-kafka-*-producer.properties and config-kafka-*-consumer.properties configmap property string.

security.protocol=SASL_SSL
sasl.mechanism=OAUTHBEARER
sasl.jaas.config=org.apache.kafka.common.security.oauthbearer.OAuthBearerLoginModule required  awsStsRegion="REGION";
sasl.login.callback.handler.class=software.amazon.msk.auth.iam.IAMOAuthBearerLoginCallbackHandler
sasl.client.callback.handler.class=software.amazon.msk.auth.iam.IAMOAuthBearerLoginCallbackHandler

or

security.protocol=SASL_SSL
sasl.mechanism=OAUTHBEARER
sasl.jaas.config=org.apache.kafka.common.security.oauthbearer.OAuthBearerLoginModule required  awsStsRegion="REGION" awsRoleArn="arn:aws:iam::123456789012:role/role-that-can-access-msk";
sasl.login.callback.handler.class=software.amazon.msk.auth.iam.IAMOAuthBearerLoginCallbackHandler
sasl.client.callback.handler.class=software.amazon.msk.auth.iam.IAMOAuthBearerLoginCallbackHandler

Release Note

Added support for AWS MSK IAM authentication using SASL mechanism OAUTHBEARER.

Docs
knative/docs#6365

[knative-prow]

Welcome @qswinson! It looks like this is your first PR to knative-extensions/eventing-kafka-broker 🎉

[knative-prow]

Hi @qswinson. Thanks for your PR.

I'm waiting for a knative-extensions member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: qswinson / name: Quenby Mitchell (e781313, 4eeaa51, 095833c, ccac58d, 3b0e459, 487d0dc, 943c016, aae9ec0, 491975e, 598e215, 56f5548, c696d5f, f7f07a7, 4c5a285, a751f26, 43b2924, 4e12348, c5f9f7a, 0dff494, 55e8cde, b98f8ed, f1edfb5, 36ea131, 6bed236, 6370dba)

[matzew]

Thanks for the contribution!
I think that our kafka distribution strimzi is also supporting OAUTHBEARER.

Would it make sense to also add integration tests for that, and see if the feature is also possible in a generic way?

[dprotaso]

/ok-to-test

Thanks for the contribution! I think that our kafka distribution strimzi is also supporting OAUTHBEARER.

Would it make sense to also add integration tests for that, and see if the feature is also possible in a generic way?

That might be beyond the scope of this PR. Maybe a follow up? The way I'm reading the strimzi docs it says it has OAUTHBEARER support but it doesn't include an OAuth server. @matzew Does the e2e testing already set one up?

[matzew]

Does the e2e testing already set one up?
no, that would be something like Keycloak, I guess

That might be beyond the scope of this PR. Maybe a follow up?
yeah, possible

[twoGiants]

Hi @qswinson!

Thank you for addressing my review and improving the architecture of this contribution. That is a great improvement and you did it much better than I had proposed. I am so happy about that! 😸 👍

I have a few more comments and they address:

• encapsulation => set to private what is not used outside the package
• putting strings into constants => removes duplication
• two forgotten comments from the previous review
• file names

I hope you're ok with one last round. We nearly have it and it'll be a really nicely polished PR.

[knative-prow]

@twoGiants: changing LGTM is restricted to collaborators

In response to this:

Hi @qswinson!

Thank you for addressing my review and improving the architecture of this contribution. That is a great improvement and you did it much better than I had proposed. I am so happy about that! 😸 👍

I have a few more comments and they address:

• encapsulation => set to private what is not used outside the package
• putting strings into constants => removes duplication
• two forgotten comments from the previous review
• file names

I hope you're ok with one last round. We nearly have it and it'll be a really nicely polished PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[dprotaso]

feature.go:71: Failed to get sources.knative.dev/v1, Resource=kafkasources test-oxtycyyq/kafka-source-neqcygnv: %!w(*errors.StatusError=&{{{Status v1} { } Failure conversion webhook for sources.knative.dev/v1beta1, Kind=KafkaSource failed: Post "https://kafka-webhook-eventing.knative-eventing.svc:443/resource-conversion?timeout=30s": EOF 500}})

looks like a flake

/retest

[qswinson]

/retest

[dprotaso]

bump @twoGiants @matzew @creydr for a final pass

[twoGiants]

bump @twoGiants @matzew @creydr for a final pass

Taking a final look 🔍 👀 😸

[twoGiants]

Hi @qswinson,

Thank you for the updates.

I noticed there are still a few small inconsistencies left, like incomplete encapsulation, test case names extracted into constants, and unused test case variables. To simplify things and get this PR merged, I've created a patch with the final fixes.

Please apply the attached patch, and we should be all set.

pr-4516-rework.patch

In the future, please ensure that contributions are fully cleaned up, proofread and all review comments are properly addressed before submitting them for review.

Thanks.

[dprotaso]

hey @twoGiants there's a resolve button so you don't have to comment on every thread when things are done

Also your patch file doesn't download. It returns a 404. I would recommend using the GitHub UI to create commit suggestions.

See: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/commenting-on-a-pull-request#adding-comments-to-a-pull-request

[dprotaso]

since we're using t.Setenv now we don't need the setup/cleanup funcs anymore

[dprotaso]

/lgtm

[dprotaso]

/retest

[matzew]

/lgtm
/approve

[knative-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: matzew, qswinson

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [matzew]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[twoGiants]

hey @twoGiants there's a resolve button so you don't have to comment on every thread when things are done

See: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/commenting-on-a-pull-request#adding-comments-to-a-pull-request

Hi @dprotaso!

I don't have a resolve button on my review in this PR. I do not have the permissions yet to resolve review comments. Maybe one needs to be Approver or Maintainer. I am only org "Member". I put the "resolved" comments there to not lose track of what is resolved while reviewing. (Edit: Yes, I need write access in the repo to resolve conversations.)

Also your patch file doesn't download. It returns a 404.

Very unfortunate that the download didn't work. I am able to download the file when I click on the link. But @creydr was also not able to download it, so it looks like it works only for me.

I would recommend using the GitHub UI to create commit suggestions.

See: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/commenting-on-a-pull-request#adding-comments-to-a-pull-request

Yes, that was one of the options. I was considering those options:

• push directly in this PR => couldn't do it, not enough permissions as "Member"
• approve and submit an additional PR with the cleanup because the functionality was implemented and tested, i.e. good enough
• create a PR into the branch of the contributor
• create comment suggestions in this branch
• create a patch of the cleanup and attach it the review
• push the locally checked out branch of this PR in my fork, reference the cleanup commit in the last review and ask to cherry-pick

I picked the fifth option because it was the fastest for me as I already had cleanup the code and had a commit ready. You can checkout my cleanup commit here. My commit contains your suggestions form above and a few more things.

I will open a separate small cleanup PR and @creydr will support in the review. It won't be much.

@dprotaso you can also find me on the CNCF slack. Happy to connect with you there! 😸 👍

---

P.S.: Edit on 08.10.25, 10:45: small cleanup PR #4553.

[dprotaso]

thanks @qswinson 🎉