Fix #5857: Add cert-manager auto-discovery to the Ingress trait by Thundercloud12 · Pull Request #6818 · apache/camel-k

Thundercloud12 · 2026-09-16T04:47:50Z

Summary

Adds cert-manager auto-discovery to the Ingress trait. When enabled, Camel K detects whether cert-manager is installed, resolves a ClusterIssuer or Issuer (either forced by name or auto-discovered), and annotates the generated Ingress so cert-manager's own ingress-shim handles certificate issuance and renewal.

Camel K never creates Certificate resources itself — this keeps certificate lifecycle management entirely inside cert-manager, where it belongs.

Fixes #5857

New Trait Options

tls-cert-manager-auto — enable cert-manager auto-discovery
tls-issuer-name — force a specific issuer, bypassing auto-discovery
tls-issuer-kind — Issuer or ClusterIssuer (defaults to ClusterIssuer)

Design Notes / Tradeoffs for Reviewers

Multiple issuers: When auto-discovery finds more than one ClusterIssuer/Issuer, the first one is selected rather than requiring exactly one unambiguous match. This was chosen for simplicity over correctness-by-refusal. Happy to switch to an "only when exactly one issuer is found" behavior if reviewers prefer.
t.Host fallback: The cert-manager path also activates when only ingress.host is set. This is intentional because ingress.host is the field most users actually configure. This fallback is scoped strictly to the cert-manager path (only when tlsSecretName is empty) and does not change the existing manual-TLS behavior covered by TestConfigureTLSWithoutHostsIngressTraitDoesSucceed.
RBAC widening: Adds get/list/watch permissions for issuers and clusterissuers (cert-manager.io) to the operator's ClusterRole/Role. These permissions are read-only. clusterissuers are cluster-scoped even in the namespaced installation mode; this is flagged for review against the project's RBAC threat model.
E2E scope: Rather than performing a live HTTPS round-trip through a real ingress controller—which would require additional infrastructure such as DNS, a tunnel, or an ingress addon—the E2E test verifies the issued certificate cryptographically. It parses the TLS secret and checks the DNS SAN and certificate expiry after confirming that cert-manager populated it.

Implementation

pkg/util/certmanager/ — new cert-manager detection package (IsInstalled, ListClusterIssuers, GetClusterIssuer, GetIssuer), built on unstructured without introducing a cert-manager Go client dependency. Absence of cert-manager degrades cleanly to "not found" rather than returning an error.
pkg/trait/ingress.go — adds resolveCertManagerIssuer() and reworks the TLS block in Apply() to merge the manual-TLS and cert-manager paths.
RBAC: Adds the required ClusterRole/Role/RoleBinding permissions for issuers and clusterissuers.
e2e/cert-manager/ — new cert-manager E2E workflow with its own Makefile target, mirroring the structure of the existing e2e/gateway suite.

Test Plan

Unit tests: pkg/util/certmanager — 8 tests covering installation detection and all three "absent" failure modes
Unit tests: pkg/trait/ingress_test.go — 6 additional tests covering:
- cert-manager not installed → no-op
- no issuer found → no-op
- successful auto-discovery
- successful forced-issuer resolution
- forced issuer missing → error
- forced issuer configured but cert-manager not installed → error
Existing Ingress trait tests — 12 tests pass unmodified, confirming no regression to manual TLS behavior
go build ./...
gofmt
./script/check_licenses.sh
E2E suite (make test-e2e) — not run in this environment; reviewers/CI to confirm

🤖 Generated with Claude Code on behalf of Thundercloud12

Thundercloud12 · 2026-09-16T04:51:29Z

@squakez this is the fix, there are two assumptions i should say that i have taken while building this written above too but writing it here so you can deal a good focus on it:

If clientissuer finds more than one issuer it falls back to the first issuer taking precendence alphabetically
The cert manager path will also activate WHEN ONLY t.Host is set, it is strictly done when tlssecretname is not there
If you find anything wrong or not upto mark with your architecture or codebase please do let me know, Thank you for giving time to review this pr!!

squakez

Thanks for the work. It's a good ground we can use to reason about the feature. The logic seems to be fine overall, beside a few optimization. However there are important design decision we need to take to make it more performant and reliable. See each comment and we can add more information in each thread.

squakez · 2026-09-16T13:49:16Z

+	// secret name. It may fall back to t.Host so that auto-discovery also works for the
+	// common single-host case, without changing the pre-existing manual TLS behavior
+	// (which requires TLSHosts to be set explicitly and never considers t.Host).
+	if secretName == "" {


I think we miss a check on the auto parameter. If disabled on purpose, we should not proceed. Ideally the check has to be done in the Configure. And in fact, you can use in general the Configure func to compute any configuration that you will later use in Apply by storing private scoped variables.

…on of concerns clear among configure and apply

github-actions · 2026-09-16T15:10:11Z

✔️ Unit test coverage report - coverage increased from 63.8% to 64% (+0.2%)

…nabled by default and added a function to disable it

…e test client, matching the Knative default, and exclude cert-manager duck types from the generic fake Kubernetes clientset (they are only ever accessed via the typed controller-runtime client).added types to prevent unstructured, ingress_test made it enabled by default and added a function to disable it

Thundercloud12 · 2026-09-16T17:50:53Z

@squakez thanks for the compehensive comment going carefully through this code, after your comments i decided that a seperation of concerns was a must thereby seperating into configure and apply as would be visible and seconly tookl up on your untyped fileds and followed the example elsewhere shown in this repo, thirdly the very last one i should say i have followed up on your advice, downside being cert-manager detection static for the operator's lifetime if someone installs cert-manager into a running cluster, the operator won't notice until it restarts.

squakez · 2026-09-17T06:31:38Z

downside being cert-manager detection static for the operator's lifetime if someone installs cert-manager into a running cluster, the operator won't notice until it restarts.

This is certainly a side effect that we can document. We have a similar "problem" with Knative and it's documented as well.

Thundercloud12 · 2026-09-17T08:32:41Z

@squakez mirroring the knative's approach i have added the required docs

squakez

Nice. Only a minor check on a potential bootstrap error, but the rest LGTM.

github-actions · 2026-09-17T12:34:01Z

✔️ Unit test coverage report - coverage increased from 63.8% to 63.9% (+0.1%)

…orced-issuer branch of resolveCertManagerIssuer() into its own resolveForcedCertManagerIssuer() method to bring nesting complexity under golangci-lint's nestif threshold, and switch to plain (unnamed) return values to satisfy nonamedreturns.

github-actions · 2026-09-17T14:26:04Z

✔️ Unit test coverage report - coverage increased from 63.8% to 63.9% (+0.1%)

Thundercloud12 · 2026-09-18T04:21:35Z

@squakez i have also fixed the issue that were failing some checks

Thundercloud12 added 2 commits September 16, 2026 00:04

Fix apache#5857: Add cert-manager auto-discovery to the Ingress trait

28e47b4

Fix apache#5857: Add cert-manager auto-discovery to the Ingress trait

f549a79

squakez reviewed Sep 16, 2026

View reviewed changes

refactor: refactored according to the advices, received made seperati…

4a6dca9

…on of concerns clear among configure and apply

Thundercloud12 added 2 commits September 16, 2026 21:20

refactor: added types to prevent unstructured, ingress_test made it e…

e5ff8ce

…nabled by default and added a function to disable it

Document cert-manager auto-discovery install-order caveat

3c7485e

squakez approved these changes Sep 17, 2026

View reviewed changes

Comment thread pkg/cmd/operator/operator.go Outdated

Thundercloud12 added 3 commits September 17, 2026 18:42

added error handling

36cf578

pushed formatted fix

7626931

squakez merged commit e6c5cc0 into apache:main Sep 18, 2026
14 checks passed

Thundercloud12 deleted the feature/5857-cert-manager-auto-discovery branch September 18, 2026 07:16

Thundercloud12 restored the feature/5857-cert-manager-auto-discovery branch September 18, 2026 07:16

Conversation

Thundercloud12 commented Sep 16, 2026

Summary

New Trait Options

Design Notes / Tradeoffs for Reviewers

Implementation

Test Plan

Uh oh!

Thundercloud12 commented Sep 16, 2026

Uh oh!

squakez left a comment

Choose a reason for hiding this comment

Uh oh!

squakez Sep 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Sep 16, 2026

Uh oh!

Thundercloud12 commented Sep 16, 2026

Uh oh!

squakez commented Sep 17, 2026

Uh oh!

Thundercloud12 commented Sep 17, 2026

Uh oh!

squakez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Sep 17, 2026

Uh oh!

github-actions Bot commented Sep 17, 2026

Uh oh!

Thundercloud12 commented Sep 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants