Skip to content

Conversation

madebydna
Copy link

This adds support for using MSK IAM authentication with the rdkafka2 output type. Authentication and authorization with an MSK cluster are facilitated through a base64-encoded signed URL, which is generated by the aws-msk-iam-sasl-signer-ruby library.

@madebydna
Copy link
Author

Reopening it based on conversation here.

@madebydna madebydna reopened this Dec 4, 2024
@madebydna
Copy link
Author

I fixed the dependencies that require Ruby > 3.0, so that the CI tests using Ruby 2.7 now also pass. I also added rdkafka to the gemspec file instead of the Gemfile, but leaving the USE_RDKAFKA conditional intact. Was there a particular reason that dependency was only in the Gemfile?

If there is interest at all in merging it in, I'd be happy to continue making improvements to the code with some input. Please let me know if this feature is not something that is considered useful and I can close the PR.

In terms of possible improvements, I was wondering if it would be better to have a config param such as use_aws_msk_iam_auth and not require the two rdkafka_options of security.protocol and sasl.mechanisms to be set explicitly. So instead of

<match *>
    @type rdkafka2
    # other options
    
    # AWS Region (required)
    aws_msk_region us-east-1
    
    # MSK IAM authentication settings (required)
    rdkafka_options {
      "security.protocol": "sasl_ssl",
      "sasl.mechanisms": "OAUTHBEARER"
    }
</match>

the config could look like this:

<match *>
    @type rdkafka2
    # ... other options ....

    use_aws_msk_iam_auth true
    aws_msk_region us-east-1
</match>

Also, I was wondering if there is a way to not use the @shared_producer but use the per-thread producers instead, since that is the default. Currently, there is a deadlock when calling get_producer from the oauthbearer_token_refresh_callback and the the share_producer setting is not set to true.

Copy link

github-actions bot commented Sep 8, 2025

This PR has been automatically marked as stale because it has been open 90 days with no activity. Remove stale label or comment or this PR will be closed in 30 days

@github-actions github-actions bot added the stale label Sep 8, 2025
@yogrr
Copy link

yogrr commented Sep 9, 2025

Any updates?

@daipom daipom removed the stale label Sep 10, 2025
@madebydna
Copy link
Author

I'd be happy to continue to work on it to get this merged. Is the DCO the reason this PR has not received any attention, @daipom?

@daipom
Copy link
Contributor

daipom commented Sep 10, 2025

Sorry I haven’t been able to make time for this PR.

I'd be happy to continue to work on it to get this merged. Is the DCO the reason this PR has not received any attention, @daipom?

Thanks!!
Yes! We need to add DCO.
In addition, we need to fix the conflict.

madebydna and others added 18 commits September 10, 2025 19:39
Signed-off-by: Andrea Singh <[email protected]>
Signed-off-by: Andrea Singh <[email protected]>
It helps to track new issues by aggregating ones from Project view.

Signed-off-by: Kentaro Hayashi <[email protected]>
Signed-off-by: Kentaro Hayashi <[email protected]>
  /opt/hostedtoolcache/Ruby/3.4.2/x64/lib/ruby/gems/3.4.0/gems/ruby-kafka-1.5.0/lib/kafka/protocol/record_batch.rb:1:
  warning: bigdecimal was loaded from the standard library, but is not
  part of the default gems starting from Ruby 3.4.0.  You can add
  bigdecimal to your Gemfile or gemspec to silence this warning.

Signed-off-by: Kentaro Hayashi <[email protected]>
Signed-off-by: Kentaro Hayashi <[email protected]>
Signed-off-by: Shizuo Fujita <[email protected]>
Bumps [actions/checkout](https://github.com/actions/checkout) from 4 to 5.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v4...v5)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-version: '5'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/stale](https://github.com/actions/stale) from 9 to 10.
- [Release notes](https://github.com/actions/stale/releases)
- [Changelog](https://github.com/actions/stale/blob/main/CHANGELOG.md)
- [Commits](actions/stale@v9...v10)

---
updated-dependencies:
- dependency-name: actions/stale
  dependency-version: '10'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@madebydna madebydna force-pushed the aws_msk_with_iam_support branch from 27922c4 to e23d33f Compare September 11, 2025 02:46
@madebydna
Copy link
Author

@daipom I have fixed the gemspec conflicts and signed off my commits. However, there is still a DCO issue that I don't know how to fix:

Commit sha: [632faa2]..., Author: Kentaro Hayashi, Committer: Andrea Singh; 
Expected "Kentaro Hayashi [[email protected]](mailto:[email protected])", 
but got "Kentaro Hayashi [[email protected]](mailto:[email protected])".

@madebydna
Copy link
Author

Closing this PR in favor of this one with a cleaner git history.

@madebydna madebydna closed this Sep 12, 2025
@daipom
Copy link
Contributor

daipom commented Sep 12, 2025

Sorry for my late response.
Thanks!
It looks like the issue was that the Git history had become complicated.
So, it looks better to either rebase and force push or recreating the PR.
#537 looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants