feat: support annotations by cgpoh · Pull Request #111 · strimzi/kafka-access-operator

cgpoh · 2025-11-15T09:33:57Z

This PR adds support for customizing the metadata (annotations and labels) of Secrets generated by the Kafka Access operator. We are using Mittwald secret replicator to replicate secrets to other namespaces and requires annotation in order for it to work.

Copilot

Pull Request Overview

This PR adds support for customizing the metadata (annotations and labels) of Secrets generated by the KafkaAccess operator through a new template field in the KafkaAccess spec. This enables users to add custom annotations for tools like secret replicators and to apply custom labels for organization and filtering purposes.

Key changes:

Introduces a new template API structure (KafkaAccessTemplate, SecretTemplate, MetadataTemplate) for specifying custom Secret metadata
Updates the reconciler to merge template-specified annotations/labels with existing Secret metadata during updates
Provides example configurations demonstrating the use of annotations for secret replication tools

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`packaging/install/040-Crd-kafkaaccess.yaml`	Adds CRD schema definition for the new template field supporting secret metadata customization
`packaging/helm-charts/helm3/strimzi-access-operator/crds/040-Crd-kafkaaccess.yaml`	Adds CRD schema definition for Helm chart installation
`packaging/examples/kafka-access-with-user.yaml`	Adds example demonstrating custom annotation and label usage
`operator/src/main/java/io/strimzi/kafka/access/server/HealthServlet.java`	Adds explicit constructor and updates class documentation
`operator/src/main/java/io/strimzi/kafka/access/internal/KafkaParser.java`	Adds private constructor to prevent instantiation and improves documentation
`operator/src/main/java/io/strimzi/kafka/access/internal/KafkaAccessMapper.java`	Fixes spelling in javadoc ("Kuberentes" → "Kubernetes") and adds private constructor
`operator/src/main/java/io/strimzi/kafka/access/KafkaAccessReconciler.java`	Implements template handling logic for extracting and applying annotations/labels to Secrets
`operator/src/main/java/io/strimzi/kafka/access/KafkaAccessOperator.java`	Adds explicit constructor and enhances class documentation
`install/040-Crd-kafkaaccess.yaml`	Adds CRD schema definition for the template field
`helm-charts/helm3/strimzi-access-operator/crds/040-Crd-kafkaaccess.yaml`	Adds CRD schema definition for Helm chart
`examples/kafka-access-with-user.yaml`	Adds example usage of template field with custom metadata
`examples/kafka-access-with-reflector.yaml`	New example file showing secret replication tool integration patterns
`api/src/main/java/io/strimzi/kafka/access/model/SecretTemplate.java`	New model class for Secret template configuration
`api/src/main/java/io/strimzi/kafka/access/model/MetadataTemplate.java`	New model class for metadata (annotations/labels) template configuration
`api/src/main/java/io/strimzi/kafka/access/model/KafkaAccessTemplate.java`	New model class for top-level template configuration
`api/src/main/java/io/strimzi/kafka/access/model/KafkaAccessSpec.java`	Adds template field to KafkaAccess spec with getter/setter methods

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-15T09:38:22Z

+                    final Map<String, String> currentAnnotations = Optional.ofNullable(secret.getMetadata().getAnnotations()).orElse(new HashMap<>());
+                    final Map<String, String> currentLabels = Optional.ofNullable(secret.getMetadata().getLabels()).orElse(new HashMap<>());
+
+                    // Merge template annotations/labels with existing ones (template takes precedence)


The comment "Merge template annotations/labels with existing ones (template takes precedence)" is slightly misleading. The merge behavior actually preserves ALL existing annotations/labels and adds/overwrites only the template-specified ones. This means:

Manually added annotations/labels are preserved across reconciliations

Removing an annotation/label from the template will NOT remove it from an existing secret

Template values take precedence only for keys that exist in both maps

Consider clarifying the comment to explicitly state this behavior, as it has important implications for users who might expect the secret metadata to exactly match the template specification:

// Merge template annotations/labels with existing ones. // Template values override matching keys, but all other existing metadata is preserved. // Note: Removing a key from the template will not remove it from existing secrets.

Suggested change

// Merge template annotations/labels with existing ones (template takes precedence)

// Merge template annotations/labels with existing ones.

// Template values override matching keys, but all other existing metadata is preserved.

// Note: Removing a key from the template will not remove it from existing secrets.

Copilot · 2025-11-15T09:38:22Z

+    /**
+     * Creates a new HealthServlet.
+     * This explicit constructor documents the default servlet instantiation.
+     */
+    public HealthServlet() {
+        super();
+    }
+


[nitpick] This explicit no-argument constructor that only calls super() is unnecessary and can be removed. Java automatically provides a default no-argument constructor that calls the superclass constructor when no constructors are explicitly defined. The Javadoc plugin warning (if any) should be configured to ignore classes that don't need explicit constructor documentation, rather than adding unnecessary constructors.

Suggested change

/**

* Creates a new HealthServlet.

* This explicit constructor documents the default servlet instantiation.

*/

public HealthServlet() {

super();

}

Copilot · 2025-11-15T09:38:22Z

+    /**
+     * Creates a new KafkaAccessOperator instance.
+     * Explicit constructor added to satisfy Javadoc plugin warning on default constructor.
+     */
+    public KafkaAccessOperator() {
+        // Intentionally empty.
+    }


[nitpick] This explicit no-argument constructor is unnecessary and can be removed. Java automatically provides a default no-argument constructor when no constructors are explicitly defined. Since this class only contains static methods, the constructor is never called directly. The Javadoc plugin warning (if any) should be configured to ignore classes that don't need explicit constructor documentation, rather than adding unnecessary constructors.

Suggested change

/**

* Creates a new KafkaAccessOperator instance.

* Explicit constructor added to satisfy Javadoc plugin warning on default constructor.

*/

public KafkaAccessOperator() {

// Intentionally empty.

}

scholzj

I think having some description explaining what this is, what it does, and why this is needed would be a good start. You should also not change anything in install, examples and helm-chart folders. Only their versions in packaging.

cgpoh · 2025-11-15T15:19:34Z

I think having some description explaining what this is, what it does, and why this is needed would be a good start. You should also not change anything in install, examples and helm-chart folders. Only their versions in packaging.

Thanks @scholzj , I removed the changes in those folders and added description of the PR.

scholzj · 2025-11-15T17:35:15Z

We are using Mittwald secret replicator to replicate secrets to other namespaces and requires annotation in order for it to work.

Can you please elaborate on this? The whole purpose of the Access Operator is that it copies the credentials where needed without any need for additional tooling. If you want to use secret replication tools, you can set the labels/annotations at the secrets at the source (In Strimzi Cluster and User Operators) any skip the whole Access Operator.

cgpoh · 2025-11-15T18:35:09Z

We are using Mittwald secret replicator to replicate secrets to other namespaces and requires annotation in order for it to work.

Can you please elaborate on this? The whole purpose of the Access Operator is that it copies the credentials where needed without any need for additional tooling. If you want to use secret replication tools, you can set the labels/annotations at the secrets at the source (In Strimzi Cluster and User Operators) any skip the whole Access Operator.

Instead of replicating and referring to 2 secrets (Kafka cluster CA cert secret and Kafka User secret), I just need to replicate 1 secret generated by Access operator and I can connect to my Kafka cluster with the correct credentials. Therefore, I find it very convenient if I can replicate the secret generated by Access operator via annotation.

scholzj · 2025-11-15T21:18:51Z

Instead of replicating and referring to 2 secrets (Kafka cluster CA cert secret and Kafka User secret), I just need to replicate 1 secret generated by Access operator and I can connect to my Kafka cluster with the correct credentials. Therefore, I find it very convenient if I can replicate the secret generated by Access operator via annotation.

I'm not really sure if that is worth the operating effort and running costs TBH. Nor the maintenance effort required of the feature. One Secret or two, seems to make a little difference. Let's see what the others think.

katheris · 2026-01-29T11:19:55Z

I think it's reasonable for users to be able to customise the labels and annotations on the Secret created by the Access Operator, in the same way that they can for the other Strimzi created resources. I don't understand the usecase here for being able to replicate, but generally I can see users wanting to have set labels or annotations across their Strimzi resources.

I'll add this as a discussion to the next Community call to see what others think and get consensus.

aeroyorch · 2026-03-10T18:12:27Z

Hi! Any update on this PR?

It would be really useful to support templating so we can add labels/annotations. For consistency, it would be great if it could align with how KafkaUser handles templates in Strimzi.

katheris · 2026-03-19T16:06:52Z

@cgpoh Are you planning to rebase this PR at some point? In the community call we agreed that we would be happy to accept this change, so once you've rebased and fixed the DCO signoff it can be reviewed. @aeroyorch if they don't come back you could also open your own PR for this

cgpoh · 2026-03-20T06:55:49Z

@cgpoh Are you planning to rebase this PR at some point? In the community call we agreed that we would be happy to accept this change, so once you've rebased and fixed the DCO signoff it can be reviewed. @aeroyorch if they don't come back you could also open your own PR for this

@katheris, Thank you! I've rebased and fixed the DCO signoff.

katheris

Thanks @cgpoh, I've added two nits but otherwise I'm happy with these changes

katheris · 2026-03-23T10:54:34Z

@cgpoh what steps did you do to rebase as your branch history now seems to have commits from the main branch twice? You should be using git rebase to get a clean history

Signed-off-by: CG <cgpoh76@gmail.com>

… folders Signed-off-by: CG <cgpoh76@gmail.com>

Signed-off-by: CG <cgpoh76@gmail.com>

cgpoh · 2026-03-23T14:06:15Z

@cgpoh what steps did you do to rebase as your branch history now seems to have commits from the main branch twice? You should be using git rebase to get a clean history

Thanks @katheris, I've fixed the branch history

cgpoh · 2026-03-23T14:07:48Z

Thanks @cgpoh, I've added two nits but otherwise I'm happy with these changes

@katheris, Thank you for the approval. I've resolved the nits.

katheris

@cgpoh thanks for the updates, I'm happy with this now

scholzj · 2026-03-26T09:12:30Z

I do not think these changes belong here and are in any way related to the core change of the PR. Can you please remove them (and open a separate PR if you want to have that in the code?)

The same applies to all the other files where the comment and constructor changes are unrelated.

Thanks @scholzj , I've removed the changes

scholzj · 2026-03-26T09:19:54Z

+                    final Map<String, String> mergedAnnotations = new HashMap<>(currentAnnotations);
+                    mergedAnnotations.putAll(templateAnnotations);
+
+                    final Map<String, String> mergedLabels = new HashMap<>(currentLabels);
+                    mergedLabels.putAll(templateLabels);


Thsi seems wrong. The template should never rewrite the regular labels and annotations.

I made the changes to not rewrite the regular labels and annotations.

scholzj · 2026-03-26T09:20:30Z

    apiGroup: kafka.strimzi.io
    name: my-user
    namespace: kafka
+  # Optional: template to customize the generated Secret


Should there be an example for this, it should be a separate file. But I think no exaple is needed. We do not have it inother subprojects either.

I've removed the example

… existing secret metadata on reconcile updates Signed-off-by: CG <cgpoh76@gmail.com>

scholzj

I left some more comments. Maybe Kate needs to comment on some of them as well, as I'm not that familair with this source code.

scholzj · 2026-03-27T10:55:54Z

+        Map<String, String> currentData = Optional.ofNullable(secret.getData()).orElse(new HashMap<>());
+        Map<String, String> currentAnnotations = Optional.ofNullable(secret.getMetadata().getAnnotations()).orElse(new HashMap<>());
+        Map<String, String> currentLabels = Optional.ofNullable(secret.getMetadata().getLabels()).orElse(new HashMap<>());
+
+        Map<String, String> mergedAnnotations = mergeWithoutOverwritingCurrent(currentAnnotations, templateAnnotations);
+        Map<String, String> mergedLabels = mergeWithoutOverwritingCurrent(currentLabels, templateLabels);


You seem to use somewhere final and somewhere not. Most of the Strimzi code does not use final for variables used inside methods. Only for object variables. But not sure what does access operato use. I think either these should be final. Or in the other method you should not use it?

@katheris What is the access operator code style?

In the Access Operator we use final for the variables inside methods, sorry I missed that in my review so these should have final also to be consistent

Thanks @katheris , I've changed the code to use final

scholzj · 2026-03-27T10:56:15Z

+        }
+    }
+
+    private Map<String, String> mergeWithoutOverwritingCurrent(Map<String, String> current, Map<String, String> template) {


Could it be static?

Thanks @scholzj , I've changed it to static function

Signed-off-by: CG <cgpoh76@gmail.com>

scholzj · 2026-03-30T19:04:09Z

+        final Map<String, String> currentAnnotations = Optional.ofNullable(secret.getMetadata().getAnnotations()).orElse(new HashMap<>());
+        final Map<String, String> currentLabels = Optional.ofNullable(secret.getMetadata().getLabels()).orElse(new HashMap<>());


Maybe we can use Map.of() here as we don't modify it later but create a new map in the merge method?

Does it even make sense to create these final maps that are only passed into the method? Why not just do something like:

final Map<String, String> mergedAnnotations = mergeWithoutOverwritingCurrent(Optional.ofNullable(secret.getMetadata().getAnnotations()).orElse(Map.of()), templateAnnotations);

scholzj · 2026-03-30T19:10:38Z

+
+    /**
+     * Extracts labels from the KafkaAccess spec template that should be applied to the Secret.
+     * These are merged with the common secret labels.


This does not seem to apply anymore.

Thanks, I pushed my changes to reword this

…e without overwriting existing labels/annotations Signed-off-by: CG <cgpoh76@gmail.com>

Signed-off-by: CG <cgpoh76@gmail.com>

scholzj

Thanks for the PR and for implementing the review comments.

cgpoh · 2026-03-31T07:40:39Z

Thanks for the PR and for implementing the review comments.

@scholzj , @katheris thanks for the review and approval :)

Copilot AI review requested due to automatic review settings November 15, 2025 09:33

Copilot started reviewing on behalf of cgpoh November 15, 2025 09:34 View session

Copilot finished reviewing on behalf of cgpoh November 15, 2025 09:37

Copilot AI reviewed Nov 15, 2025

View reviewed changes

scholzj requested changes Nov 15, 2025

View reviewed changes

cgpoh force-pushed the annotations branch 3 times, most recently from dc40ab1 to 67130bb Compare March 20, 2026 06:48

katheris added this to the 0.3.0 milestone Mar 23, 2026

katheris approved these changes Mar 23, 2026

View reviewed changes

Comment thread api/src/main/java/io/strimzi/kafka/access/model/SecretTemplate.java Outdated

Comment thread operator/src/main/java/io/strimzi/kafka/access/KafkaAccessOperator.java Outdated

katheris requested review from katheris and scholzj March 23, 2026 10:54

cgpoh added 2 commits March 23, 2026 21:48

feat: support annotations

4582a44

Signed-off-by: CG <cgpoh76@gmail.com>

chore: remove changes to contents in examples, install and helm-chart…

c869568

… folders Signed-off-by: CG <cgpoh76@gmail.com>

cgpoh force-pushed the annotations branch from f543fff to c869568 Compare March 23, 2026 13:50

chore: remove extra description

5e7718f

Signed-off-by: CG <cgpoh76@gmail.com>

scholzj requested review from a team and removed request for scholzj March 23, 2026 15:36

katheris approved these changes Mar 23, 2026

View reviewed changes

scholzj requested a review from a team March 23, 2026 15:45

scholzj requested changes Mar 26, 2026

View reviewed changes

chore: add test verifying template annotations/labels are merged with…

698f377

… existing secret metadata on reconcile updates Signed-off-by: CG <cgpoh76@gmail.com>

scholzj reviewed Mar 27, 2026

View reviewed changes

chore: use final for variables in method

1179adb

Signed-off-by: CG <cgpoh76@gmail.com>

scholzj reviewed Mar 30, 2026

View reviewed changes

cgpoh added 2 commits March 31, 2026 11:53

chore: clarify java doc, use Map.of fallbacks in secret metadata merg…

fb3ad4f

…e without overwriting existing labels/annotations Signed-off-by: CG <cgpoh76@gmail.com>

chore: rename function for clarity

cf9b5ad

Signed-off-by: CG <cgpoh76@gmail.com>

scholzj approved these changes Mar 31, 2026

View reviewed changes

scholzj merged commit fffa11c into strimzi:main Mar 31, 2026
22 checks passed

cgpoh deleted the annotations branch March 31, 2026 11:20

-                    // Merge template annotations/labels with existing ones (template takes precedence)
+                    // Merge template annotations/labels with existing ones.
+                    // Template values override matching keys, but all other existing metadata is preserved.
+                    // Note: Removing a key from the template will not remove it from existing secrets.

		final Map<String, String> currentAnnotations = Optional.ofNullable(secret.getMetadata().getAnnotations()).orElse(new HashMap<>());
		final Map<String, String> currentLabels = Optional.ofNullable(secret.getMetadata().getLabels()).orElse(new HashMap<>());

Conversation

cgpoh commented Nov 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

scholzj left a comment

Choose a reason for hiding this comment

Uh oh!

cgpoh commented Nov 15, 2025

Uh oh!

scholzj commented Nov 15, 2025

Uh oh!

cgpoh commented Nov 15, 2025

Uh oh!

scholzj commented Nov 15, 2025

Uh oh!

katheris commented Jan 29, 2026

Uh oh!

aeroyorch commented Mar 10, 2026

Uh oh!

katheris commented Mar 19, 2026

Uh oh!

cgpoh commented Mar 20, 2026

Uh oh!

katheris left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

katheris commented Mar 23, 2026

Uh oh!

cgpoh commented Mar 23, 2026

Uh oh!

cgpoh commented Mar 23, 2026

Uh oh!

katheris left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scholzj left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

cgpoh commented Nov 15, 2025 •

edited

Loading