K8SPS-649 support ClusterIssuer#1421
Merged
Merged
Conversation
Signed-off-by: Oksana Grishchenko <oksana.grishchenko@percona.com>
Signed-off-by: Oksana Grishchenko <oksana.grishchenko@percona.com>
This reverts commit 405a6f9.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for cert-manager ClusterIssuer references in .spec.tls.issuerConf, along with RBAC and e2e coverage updates to validate the behavior.
Changes:
- Update TLS issuer lookup logic to fetch either namespaced
Issueror cluster-scopedClusterIssuerbased on.spec.tls.issuerConf.kind. - Add controller/operator RBAC rules for
clusterissuers(get/list/watch). - Extend unit/e2e tests to exercise
ClusterIssuerusage and validate certificates after switching issuer kind.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/controller/ps/tls.go | Adds kind-based lookup to support ClusterIssuer in checkTLSIssuer; uses cert-manager kind constants. |
| pkg/controller/ps/tls_test.go | Adds a unit test intended to validate ClusterIssuer handling (currently has a type issue). |
| pkg/controller/ps/controller.go | Adds kubebuilder RBAC marker for clusterissuers. |
| e2e-tests/tests/tls-cert-manager/07-switch-to-cluster-issuer.yaml | Adds step to grant cluster-wide RBAC + create a ClusterIssuer + patch the CR to use it (namespace handling needs adjustment). |
| e2e-tests/tests/tls-cert-manager/07-assert-cluster-issuer.yaml | Asserts certificate issuerRef fields and cluster readiness after switching to ClusterIssuer. |
| e2e-tests/tests/tls-cert-manager/08-check-cert-after-cluster-issuer.yaml | Verifies certificate SANs after switching issuer kind. |
| e2e-tests/tests/tls-cert-manager/97-cleanup-cluster-issuer-rbac.yaml | Cleans up ClusterIssuer and associated cluster-wide RBAC created by the test. |
| deploy/rbac.yaml | Adds clusterissuers rule to a namespaced Role (does not actually grant access to cluster-scoped resources). |
| deploy/cw-rbac.yaml | Adds clusterissuers rule to the cluster-wide RBAC manifest. |
| deploy/bundle.yaml | Adds clusterissuers rule inside a Role in the standard bundle (won’t authorize cluster-scoped access). |
| deploy/cw-bundle.yaml | Adds clusterissuers rule in the cluster-wide bundle. |
| config/rbac/role.yaml | Adds clusterissuers permissions to the generated ClusterRole. |
| config/rbac/cluster/role.yaml | Adds clusterissuers permissions to the generated cluster-scoped role. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Oksana Grishchenko <oksana.grishchenko@percona.com>
12 tasks
oksana-grishchenko
requested review from
egegunes,
eleo007,
gkech,
hors,
jvpasinatto,
mayankshah1607,
nmarukovich,
pooknull and
valmiranogueira
as code owners
Signed-off-by: Oksana Grishchenko <oksana.grishchenko@percona.com>
Signed-off-by: Oksana Grishchenko <oksana.grishchenko@percona.com>
Signed-off-by: Oksana Grishchenko <oksana.grishchenko@percona.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Comment on lines
+127
to
+131
| err := r.Get(ctx, nn, obj) | ||
| if kind == cm.ClusterIssuerKind && k8serrors.IsForbidden(err) { | ||
| // In namespaced installs we may not have cluster-scoped read permissions. | ||
| // Ignore only RBAC-denied errors and let cert-manager handle issuer resolution. | ||
| return nil |
Comment on lines
+234
to
+238
| BeforeAll(func() { | ||
| _, err := envtest.InstallCRDs(cfg, envtest.CRDInstallOptions{ | ||
| Paths: []string{filepath.Join("testdata", "cert-manager.yaml")}, | ||
| }) | ||
| Expect(err).NotTo(HaveOccurred()) |
mayankshah1607
left a comment
mayankshah1607
left a comment
Member
There was a problem hiding this comment.
LGTM
One thing to check is that the cm.ObjectReference is deprecated, and we can use cm.IssuerReference instead.. (not so important because ObjectReference is internally aliased to IssuerReference anyway)
mayankshah1607
previously approved these changes
Jun 26, 2026
Signed-off-by: Oksana Grishchenko <oksana.grishchenko@percona.com>
Comment on lines
+127
to
+134
| err := r.Get(ctx, nn, obj) | ||
| if kind == cm.ClusterIssuerKind && k8serrors.IsForbidden(err) { | ||
| // In namespaced installs we may not have cluster-scoped read permissions. | ||
| // Ignore only RBAC-denied errors and let cert-manager handle issuer resolution. | ||
| return nil | ||
| } | ||
|
|
||
| return err |
gkech
reviewed
Jun 29, 2026
egegunes
approved these changes
Jul 2, 2026
mayankshah1607
approved these changes
Jul 2, 2026
Collaborator
commit: f21ba3f |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
K8SPS-649 support ClusterIssuer
Problem:
Operator does not support using ClusterIssuer although it's stated in docs
Cause:
In the codebase we check explicitly if the Issuer exists in the cluster. However the ClusterIssuer is a different kind so it failed the check
Solution:
Check the kind of the resource referenced in
.spec.tlsand get the resource accordingly. Ignore RBAC-denied errors and let cert-manager handle issuer resolution. In case the configuredClusterIssuerdoes not exist indeed, the Certificate that references it will stay not ready, thewaitForCert()eventually times out and returns reconciliation errortimeout: tls certificate from certmanager is not readyUpdate e2e tests to check if ClusterIssuer works.
We can mention it in docs that in order to use
ClusterIssueryou need additionally to apply matching RBAC.CHECKLIST
Jira
Needs Doc) and QA (Needs QA)?Tests
Config/Logging/Testability