[Abhayanthk]

Description

Adds saslClientCallbackHandlerClass and saslLoginCallbackHandlerClass to KafkaAuthenticationSpec so Microcks' Kafka clients can authenticate to Amazon MSK using IAM. They map to Kafka's sasl.client.callback.handler.class / sasl.login.callback.handler.class settings, and are applied to both components that talk to Kafka — the async-minion and the main web app.

Changes

Area	File	Change
Spec	KafkaAuthenticationSpec.java	Added saslClientCallbackHandlerClass and saslLoginCallbackHandlerClass (both optional)
Minion template	AsyncMinionConfigMapDependentResource/application.properties	Map both properties
Web app template	MicrocksConfigMapDependentResource/application.properties	Same mapping, so the web app matches the minion
CRD	/deploy/crd/microckses.microcks.io-v1.yml + Helm chart copy	Regenerated, kept only the lines for the two new fields (same approach as 8e8bc6f), both copies in sync.
Docs	microcks-cr.md	Added the client callback handler row

Two pre-existing bugs

Both were already on main; neither is a regression from this PR.(More details in the comments)

Bug	What was wrong	Effect	Found via
A	The auth-type condition compared the enum against a string, so it never matched: type == "SSL" → type.name == "SSL"	SSL / SASL_SSL auth block never rendered in either template	First local test run
B	The web app's install guard was inverted vs the Helm chart	Web app only emitted Kafka auth when the operator installs its own Kafka, never for an external broker	Comparing the web app template against the Helm chart

Behaviour change

Scenario	Before	After
External broker (install: false) with SSL / SASL_SSL	No auth config rendered (Bugs A and B in the web app; Bug A in the minion)	Auth config rendered in both components
Operator-installed Kafka (install: true)	No auth lines	No change (Strimzi internal listener, no auth)

Tests

Test	Result	Covers
AsyncMinionConfigMapDependentResourceTest	4/4	External broker, SASL_SSL, 1.14: handlers on all three Kafka clients, and absent when unset
MicrocksConfigMapDependentResourceTest (new)	4/4	External + Strimzi brokers, both version layouts (1.13 and 1.14)
Full operator suite	18/18	—

Run locally against k3s. Each commit builds and passes on its own. Not covered: the SSL branch, and CI hasn't run yet.

Related issue(s)

Fixes #305. Replicates the Helm chart changes of microcks/microcks#2280; see also microcks/microcks#2232 for background.

AI usage

AI assisted with investigating the root cause, writing the code and docs, and drafting the tests. I've reviewed and understood every change and am fully accountable for them.

[github-actions]

👋 @Abhayanthk

Welcome to the Microcks community! 💖

Thanks and congrats 🎉 for opening your first pull request here! Be sure to follow the pull request template or please update it accordingly.

Hope you have a great time there!

[Abhayanthk]

What I found:

• I looked at the upstream fix #2232,
  and I see they fixed the worker by telling the Kafka producers to use the client IAM class so they could successfully authenticate and write to Amazon MSK.
• But in our Operator repo, it looks like we never fully added IAM support to the CRD.
• Because both the client and login classes are missing from KafkaAuthenticationSpec.java, the Operator just ignores them when you try to use them!

What I'm going to do:

• Add both of those missing instructions to the CRD class and the application.properties template, so the Operator can finally pass them down to the worker.

Backward compatibility:

This change is fully backward-compatible. Both callback handler fields are completely optional in the CRD. In the Qute template, they are wrapped in standard {#if} blocks. If a user doesn't provide these classes, the Operator simply skips those lines, meaning existing deployments will see zero changes to their application.properties.

This is my first contribution here and I'm still learning the codebase, so if I've put something in the wrong place or missed an existing pattern for SASL config, I'd appreciate any guidance.

[Abhayanthk]

While working on this, I noticed saslClientCallbackHandlerClass was missing from the microcks-cr.md table, even though saslLoginCallbackHandlerClass was already listed. So I added it in a second commit to keep the docs in sync.

I only added the new row and matched the existing style/wording — I didn't touch or rewrite any of the existing documentation. Happy to change the phrasing if you'd prefer it worded differently.

[Abhayanthk]

Hi again! I noticed the upstream PR (microcks/microcks#2232) included a unit test for the mapping, so I've added the equivalent coverage here.

I added a new test method to AsyncMinionConfigMapDependentResourceTest that verifies the Operator renders the new saslClientCallbackHandlerClass property into the application.properties ConfigMap.

This should round out the change — happy to adjust if you'd like the test structured differently.

[Abhayanthk]

Hi @lbroudoux 👋 — this is ready for review whenever you get a chance. I’ve added the full thought process, the fix, and my reasoning in the comments above, so you can read through it there.

[lbroudoux]

Thanks for your patience. This looks great but you should also apply the configuration changes to the main web app ConfigMap for the component that is producing Kafka messages.

As this PR also updates the KafkaAuthenticationSpec.java, it must also update the /deploy/crd/microckses.microcks.io-v1.yml file. This CRD is typically generated when you run mvn quarkus:dev on the operator module - you just have to extract it from the target/kubernetes folder.

Let me know if you can push the changes. Thanks,

[Abhayanthk]

Thanks! Addressed both points in 2fb491c:

• Applied the saslClientCallbackHandlerClass / saslLoginCallbackHandlerClass conditional mappings to MicrocksConfigMapDependentResource/application.properties, so the main web app producer matches the async-minion config.
• Regenerated /deploy/crd/microckses.microcks.io-v1.yml from the build output (target/kubernetes).

[Abhayanthk]

One thing I noticed: there's no test covering the main web app ConfigMap auth rendering (the MicrocksConfigMapDependentResource path), unlike the async-minion side.

To be clear, I mean a rendering unit test — asserting that the operator emits the right SASL properties into the generated ConfigMap (same style as the async-minion test), not an actual MSK/IAM integration test. Happy to add it here or as a follow-up — whichever you prefer.

[Abhayanthk]

Hi @lbroudoux, a follow-up on your review. Both points were addressed in 2fb491c, but when I first ran the tests locally I found two pre-existing bugs that meant the new properties — and all Kafka SSL / SASL_SSL auth config — never actually rendered. Both are fixed in this PR (5d9d2f6 for A, 7c50a10 for B) with tests in 8d5db8e. The PR description has the full summary.

Corrections to my earlier comments

• I said existing deployments would see zero changes. That's no longer true: external-broker users with SSL / SASL_SSL now get their auth config rendered (see Behaviour change in the description).
• I said my test verified the rendering. It was actually failing because of bug A; CI hadn't run, so it went unnoticed:

microcks-operator/operator/src/test/java/io/github/microcks/operator/base/resources/AsyncMinionConfigMapDependentResourceTest.java

Lines 101 to 102 in 49ab8f6

	Assertions.assertTrue(applicationProperties.contains("kafka.sasl.client.callback.handler.class=software.amazon.msk.auth.iam.IAMClientCallbackHandler"),
	"sasl client callback handler class property must be present");

• I said 2fb491c gave the web app the same config as the minion. The lines were there, but they only take effect with A and B fixed.
• The web app rendering test I offered is added in 8d5db8e.

Bug A — enum compared with a String

authentication.type is the KafkaAuthenticationType enum, and comparing it to the "SSL" / "SASL_SSL" literals never evaluates to true:

microcks-operator/operator/src/main/resources/templates/AsyncMinionConfigMapDependentResource/application.properties

Line 36 in 422f8e2

{#if spec.features.async.kafka.authentication.type == "SSL"}

Same pattern at minion L73 and web app L52, L64, L80, L99. A standalone Qute check gives no match for {#if t == "SASL_SSL"} with t = KafkaAuthenticationType.SASL_SSL, and a match for {#if t.name == "SASL_SSL"}. Before the fix, rendering the minion ConfigMap with SSL or SASL_SSL produced no security.protocol line at all. Fixed by comparing type.name.

Bug B — web app guard inverted vs the Helm chart

The Helm chart renders the web app's Kafka auth for an external broker (install == false):

https://github.com/microcks/microcks/blob/4a9ab4c00632814ea45004d6ab38290f39fba0fc/install/kubernetes/microcks/templates/configmap.yaml#L103

The operator had the opposite guard (also at L79), with no {#else}:

microcks-operator/operator/src/main/resources/templates/MicrocksConfigMapDependentResource/application.properties

Lines 51 to 52 in 422f8e2

	{#if spec.features.async.kafka.install}
	{#if spec.features.async.kafka.authentication.type == "SSL"}

So the web app only got auth config when the operator installs Strimzi — where both components use the plain listener, which has no authentication — and never for an external broker like MSK. It went unnoticed because bug A meant nothing rendered in either case.

Also worth flagging

• CRD regen: I kept only the generated lines for the two new fields, the same way imagePullSecrets was added in 8e8bc6f. A full regeneration with the current Quarkus / operator-sdk versions adds 58 more lines (additionalProperties, request, seLinuxChangePolicy, supplementalGroupsPolicy) — the same happens on unmodified main. Happy to include them if you'd prefer a full regen.
• Pre-1.14 branch: the handlers are also mapped in the spring.kafka.producer.* branch, which the operator still renders for Microcks < 1.14. The Helm chart has no equivalent path.
• Same bug, not touched here: web app L121 compares spec.commonExpositions.type (the ExpositionType enum) to "INGRESS", so the gRPC TLS cert paths behind it can't render either.

Questions

1. Should I open a separate issue for the commonExpositions.type comparison?
2. Could you approve the build-verify workflow run? It hasn't run on this PR yet.

[Abhayanthk]

Hi @lbroudoux, a gentle ping when you have time. Both review points are addressed in 2fb491c, and the two questions above are still open.

[lbroudoux]

Thanks for your patience and sorry for the delay...
I have now found time to review what looks like great work from you!

Should I open a separate issue for the commonExpositions.type comparison?

Yes please do so.

Happy to add it here or as a follow-up — whichever you prefer.

It would be awesome to add this in a follow-up issue and PR!

[lbroudoux]

It is great! Thanks a lot for taking the time to push this and to nudge me on the review!

[github-actions]

🎉 @Abhayanthk

You are now a Microcks community contributor! 💖

Thanks and congrats 🚀 on merging your first pull request! We are delighted and very proud of you! 👏

📢 If you're using Microcks in your organization, please add your company name to this list. 🙏 It really helps the project to gain momentum and credibility. It's a small contribution back to the project with a big impact.

If you need to know why and how to add yourself to the list, please read the blog post "Join the Microcks Adopters list and Empower the vibrant open source Community 🙌"

Kudos and please keep going, we need you 🙌