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

Add iam_token_only parameter #381

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kccox
Copy link
Contributor

@kccox kccox commented Feb 21, 2025

Description

Event Streams added a provision parameter iam_token_only, which when true disables plain SASL authentication and requires IAM token authorization. This adds the parameter to the module.

Release required?

  • No release
  • Patch release (x.x.X)
  • Minor release (x.X.x) (minor new feature)
  • Major release (X.x.x)
Release notes content

Adds an iam_token_only boolean variable which when true disables plain SASL authentication and requires IAM token authorization. This is only allowed for enterprise plans.

iam_token_only defaults to true in the Enterprise solution (solutions/enterprise). Note that this is changes the default authentication method, and requires customers to configure their Kafka clients differently, so is a breaking change. Consumers of solutions/enterprise may want to override the default and set iam_token_only to false until they are able to make these changes.

See the IBM Cloud documentation for using_sasl_oauthbearer for more information.

Run the pipeline

If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.

Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:

/run pipeline

Checklist for reviewers

  • If relevant, a test for the change is included or updated with this PR.
  • If relevant, documentation for the change is included or updated with this PR.

For mergers

  • Use a conventional commit message to set the release level. Follow the guidelines.
  • Include information that users need to know about the PR in the commit message. The commit message becomes part of the GitHub release notes.
  • Use the Squash and merge option.

@kccox kccox requested review from Ak-sky and akocbek as code owners February 21, 2025 17:21
@kccox kccox force-pushed the add-iam_token_only branch from 7525aa2 to d0ef0e3 Compare February 21, 2025 17:39
JunliWang
JunliWang previously approved these changes Feb 21, 2025
@kccox
Copy link
Contributor Author

kccox commented Feb 21, 2025

/run pipeline

1 similar comment
@shemau
Copy link
Contributor

shemau commented Feb 23, 2025

/run pipeline

Copy link
Contributor

@shemau shemau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the deployable architecture needs updating, specifically the enterprise variant in solutions/enterprise/. Given the nature of this feature, it might be appropriate to enable it by default in this architecture. This would also give test coverage, otherwise consider changing the test (tests/pr_test.go) to enable it.

@kccox kccox marked this pull request as draft February 24, 2025 17:22
@kccox
Copy link
Contributor Author

kccox commented Feb 24, 2025

Converting to draft. The request to use this in the enterprise variant exposes a test problem. To use the iam_token_only feature, the customer will need to provide IAM tokens as IAM_TOKEN and IAM_REFRESH_TOKEN. These are used for authentication when the module creates topics. However this repo's CI and test framework doesn't provide these tokens, instead requiring the IBMCLOUD_API_KEY.

We are looking at a solution (in terraform-provider-ibm) which will create the IAM tokens from the apikey if they are not provided. That will allow the test framework to run with just the IBMCLOUD_API_KEY. Once that's available we can add the iam_token_only feature.

(Note, this would not be a problem for customers, as they can generate tokens and set the token environment variables before running terraform. The planned fix will also allow customers to run with only an apikey.)

@kccox
Copy link
Contributor Author

kccox commented Mar 4, 2025

/run pipeline

JunliWang
JunliWang previously approved these changes Mar 4, 2025
@ocofaigh
Copy link
Member

ocofaigh commented Mar 5, 2025

I'm going to create a new PR to add an upgrade test on the enterprise DA. Once merged, Ill rebase this and then I can see in the test pipeline logs what the exact upgrade behaviour will be for consumers

@ocofaigh
Copy link
Member

ocofaigh commented Mar 5, 2025

PR to add upgrade test #384

@kccox
Copy link
Contributor Author

kccox commented Mar 7, 2025

/run pipeline

1 similar comment
@ocofaigh
Copy link
Member

ocofaigh commented Mar 7, 2025

/run pipeline

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.

4 participants