Skip to content

Ensure CA key Secret is removed when using CertManagerCaProvider - #13194

Open
katheris wants to merge 1 commit into
strimzi:mainfrom
katheris:cleanUpOldCaKeySecret
Open

katheris wants to merge 1 commit into
strimzi:mainfrom
katheris:cleanUpOldCaKeySecret

Conversation

@katheris

Copy link
Copy Markdown
Member

Type of change

Select the type of your PR and delete the other items

  • Enhancement / new feature

Description

Currently if a user starts with Strimzi managing the CA and then switches to using cert-manager the Secret containing the CA private key isn't cleaned up.

Add logic to CertManagerCaProvider to make sure this Secret is removed.

Checklist

Please go through this checklist and make sure all applicable tasks have been done

  • Update documentation
  • Update CHANGELOG.md (if present)
  • Reference relevant issue(s) and close them after merging
  • Write tests
  • Make sure all tests pass
  • Try your changes inside a Kubernetes cluster, not just from unit tests
  • AI assistance was used to create this PR (see the Strimzi AI policy)

Signed-off-by: Kate Stanley <11195226+katheris@users.noreply.github.com>
@snyk-io

snyk-io Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

✅ Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
✅ Open Source Security 0 0 0 0 0 issues
✅ Licenses 0 0 0 0 0 issues
✅ Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@codecov

codecov Bot commented Sep 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.77%. Comparing base (d8b42c6) to head (1f83bbd).

Additional details and impacted files
@@             Coverage Diff              @@
##               main   #13194      +/-   ##
============================================
+ Coverage     80.75%   80.77%   +0.01%     
- Complexity     6806     6809       +3     
============================================
  Files           360      360              
  Lines         23352    23357       +5     
  Branches       3174     3175       +1     
============================================
+ Hits          18858    18866       +8     
  Misses         3254     3254              
+ Partials       1240     1237       -3     
Files with missing lines Coverage Δ
...uster/operator/assembly/CertManagerCaProvider.java 92.45% <100.00%> (+0.78%) ⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@scholzj scholzj added this to the 1.3.0 milestone Sep 28, 2026
@scholzj

scholzj commented Sep 28, 2026

Copy link
Copy Markdown
Member

/gha run pipeline=regression

@github-actions

github-actions Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

⏳ System test verification started: link
Internal cluster security: encryption tls, authentication mtls

The following 8 job(s) will be executed:

  • regression-brokers-amd64 (cncf-ubuntu-8-32-x86)
  • regression-security-amd64 (cncf-ubuntu-8-32-x86)
  • regression-operators-amd64 (cncf-ubuntu-8-32-x86)
  • regression-operands-amd64 (cncf-ubuntu-8-32-x86)
  • regression-brokers-arm64 (cncf-ubuntu-8-32-arm)
  • regression-security-arm64 (cncf-ubuntu-8-32-arm)
  • regression-operators-arm64 (cncf-ubuntu-8-32-arm)
  • regression-operands-arm64 (cncf-ubuntu-8-32-arm)

Tests will start after successful build completion.

@github-actions

Copy link
Copy Markdown

🎉 System test verification passed: link

@ppatierno ppatierno left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this behaviour would deserve at least one line to warn users when they do the migration from Strimzi CA to cert-manager. You never know what's their expectation. Also why deleting the private key only? Moving to cert-manager doesn't imply that you will use the same Secret from Strimzi CA as the place to copy the public key you are using with cert-manager. It could be a different Secret, right?

@tinaselenge

Copy link
Copy Markdown
Contributor

I think this behaviour would deserve at least one line to warn users when they do the migration from Strimzi CA to cert-manager. You never know what's their expectation. Also why deleting the private key only? Moving to cert-manager doesn't imply that you will use the same Secret from Strimzi CA as the place to copy the public key you are using with cert-manager. It could be a different Secret, right?

With cert-manager, the operator still uses the same internal secret for the CA certificate to track generation annotations on it. When user references a secret in the Kafka CR, the operator validates and copies the certificate from it into the internal secret.

@ppatierno

ppatierno commented Sep 29, 2026 •

Copy link
Copy Markdown
Member

With cert-manager, the operator still uses the same internal secret for the CA certificate to track generation annotations on it. When user references a secret in the Kafka CR, the operator validates and copies the certificate from it into the internal secret.

So maybe I missed that during the cert-manager integration, then the flow is:

Secret (for cert-manager) with tls.crt and tls.key ---> tls.crt copied (by user) into the ca.crt field of the Secret specified within the Kafka CR --> copied (by operator) into the ca.crt of the internal Secret -ca-cert

Is the flow correct? I missed the last copy, I thought the operator was going to use directly the Secret specified within the Kafka CR. Why did we need the additional copy?

Btw I think that the deletion of the Secret private key would still need a mention within the documentation.

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