Skip to content

CM-867: Implement trust-manager resource reconcilers #379

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-bot merged 7 commits into from
Mar 24, 2026
Merged

CM-867: Implement trust-manager resource reconcilers #379

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3136225
Implement resource reconcilers for trust-manager
chiragkyal Mar 13, 2026
3803731
Update relatedImages and Makefile
chiragkyal Mar 4, 2026
4089ab8
Add unit tests for trust-manager
chiragkyal Mar 13, 2026
4f3c5f7
Add e2e tests for trust-manager reconcilers
chiragkyal Mar 12, 2026
88f97b2
Address review comments
chiragkyal Mar 18, 2026
de5b043
Update the controller predicate logic
chiragkyal Mar 18, 2026
219524a
Update the cache handling for Issuer and ClusterIssuer
chiragkyal Mar 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,8 +305,11 @@ local-run: build ## Run the operator locally against the cluster configured in ~
RELATED_IMAGE_CERT_MANAGER_CONTROLLER=quay.io/jetstack/cert-manager-controller:$(CERT_MANAGER_VERSION) \
RELATED_IMAGE_CERT_MANAGER_ACMESOLVER=quay.io/jetstack/cert-manager-acmesolver:$(CERT_MANAGER_VERSION) \
RELATED_IMAGE_CERT_MANAGER_ISTIOCSR=quay.io/jetstack/cert-manager-istio-csr:$(ISTIO_CSR_VERSION) \
RELATED_IMAGE_CERT_MANAGER_TRUST_MANAGER=quay.io/jetstack/trust-manager:$(TRUST_MANAGER_VERSION) \
OPERATOR_NAME=cert-manager-operator \
OPERAND_IMAGE_VERSION=$(BUNDLE_VERSION) \
ISTIOCSR_OPERAND_IMAGE_VERSION=$(ISTIO_CSR_VERSION) \
TRUSTMANAGER_OPERAND_IMAGE_VERSION=$(TRUST_MANAGER_VERSION) \
OPERATOR_IMAGE_VERSION=$(BUNDLE_VERSION) \
./cert-manager-operator start \
--config=./hack/local-run-config.yaml \
Expand Down
26 changes: 26 additions & 0 deletions bundle/manifests/cert-manager-operator.clusterserviceversion.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -681,6 +681,26 @@ spec:
- patch
- update
- watch
- apiGroups:
- trust.cert-manager.io
resources:
- bundles
verbs:
- get
- list
- watch
- apiGroups:
- trust.cert-manager.io
resources:
- bundles/finalizers
verbs:
- update
- apiGroups:
- trust.cert-manager.io
resources:
- bundles/status
verbs:
- patch
- apiGroups:
- authentication.k8s.io
resources:
Expand Down Expand Up @@ -762,10 +782,14 @@ spec:
value: quay.io/jetstack/cert-manager-acmesolver:v1.19.2
- name: RELATED_IMAGE_CERT_MANAGER_ISTIOCSR
value: quay.io/jetstack/cert-manager-istio-csr:v0.15.0
- name: RELATED_IMAGE_CERT_MANAGER_TRUST_MANAGER
value: quay.io/jetstack/trust-manager:v0.20.3
- name: OPERAND_IMAGE_VERSION
value: 1.19.2
- name: ISTIOCSR_OPERAND_IMAGE_VERSION
value: 0.15.0
- name: TRUSTMANAGER_OPERAND_IMAGE_VERSION
value: 0.20.3
- name: OPERATOR_IMAGE_VERSION
value: 1.19.0
- name: OPERATOR_LOG_LEVEL
Expand Down Expand Up @@ -880,5 +904,7 @@ spec:
name: cert-manager-acmesolver
- image: quay.io/jetstack/cert-manager-istio-csr:v0.15.0
name: cert-manager-istiocsr
- image: quay.io/jetstack/trust-manager:v0.20.3
name: cert-manager-trust-manager
replaces: cert-manager-operator.v1.18.0
version: 1.19.0
4 changes: 4 additions & 0 deletions config/manager/manager.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,14 @@ spec:
value: quay.io/jetstack/cert-manager-acmesolver:v1.19.2
- name: RELATED_IMAGE_CERT_MANAGER_ISTIOCSR
value: quay.io/jetstack/cert-manager-istio-csr:v0.15.0
- name: RELATED_IMAGE_CERT_MANAGER_TRUST_MANAGER
value: quay.io/jetstack/trust-manager:v0.20.3
- name: OPERAND_IMAGE_VERSION
value: 1.19.2
- name: ISTIOCSR_OPERAND_IMAGE_VERSION
value: 0.15.0
- name: TRUSTMANAGER_OPERAND_IMAGE_VERSION
value: 0.20.3
- name: OPERATOR_IMAGE_VERSION
value: 1.19.0
- name: OPERATOR_LOG_LEVEL
Expand Down
20 changes: 20 additions & 0 deletions config/rbac/role.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,3 +278,23 @@ rules:
- patch
- update
- watch
- apiGroups:
- trust.cert-manager.io
resources:
- bundles
verbs:
- get
- list
- watch
- apiGroups:
- trust.cert-manager.io
resources:
- bundles/finalizers
verbs:
- update
- apiGroups:
- trust.cert-manager.io
resources:
- bundles/status
verbs:
- patch
3 changes: 2 additions & 1 deletion ...sr/core_validation_helpers_duplication.go → ...troller/common/core_validation_helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package istiocsr
package common

import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -13,6 +13,7 @@ import (
* Methods here are the replicates of the methods defined in k8s.io/kubernetes/pkg/apis/core/validation package, which
* is done just because of the lack of better alternative to use private methods.
* TODO: Remove this source file when validateAffinity method is made public.
* Upstream tracker: https://github.com/kubernetes/kubernetes/issues/136422
*/

// validateAffinity checks if given affinities are valid.
Expand Down
7 changes: 6 additions & 1 deletion pkg/controller/common/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func NewMultipleInstanceError(err error) *ReconcileError {
}
return &ReconcileError{
Reason: MultipleInstanceError,
Message: fmt.Sprint(err.Error()),
Message: err.Error(),
Err: err,
}
}
Expand Down Expand Up @@ -121,3 +121,8 @@ func IsMultipleInstanceError(err error) bool {
func (e *ReconcileError) Error() string {
return fmt.Sprintf("%s: %s", e.Message, e.Err)
}

// Unwrap returns the underlying error for error chain traversal.
func (e *ReconcileError) Unwrap() error {
return e.Err
}
32 changes: 23 additions & 9 deletions pkg/controller/common/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,20 @@
package common

import (
"fmt"
"reflect"

"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/runtime/serializer"
"sigs.k8s.io/controller-runtime/pkg/client"
)

// UpdateName sets the name on the given object.
func UpdateName(obj client.Object, name string) {
obj.SetName(name)
}

// UpdateNamespace sets the namespace on the given object.
func UpdateNamespace(obj client.Object, newNamespace string) {
obj.SetNamespace(newNamespace)
Expand All @@ -16,15 +25,6 @@ func UpdateResourceLabels(obj client.Object, labels map[string]string) {
obj.SetLabels(labels)
}

// HasObjectChanged compares two objects of the same type and returns true if they differ.
// Returns false if the objects are not of the same type.
func HasObjectChanged(desired, fetched client.Object) bool {
if reflect.TypeOf(desired) != reflect.TypeOf(fetched) {
return false
}
return ObjectMetadataModified(desired, fetched)
}

// ObjectMetadataModified compares the labels of two objects and returns true if they differ.
func ObjectMetadataModified(desired, fetched client.Object) bool {
return !reflect.DeepEqual(desired.GetLabels(), fetched.GetLabels())
Expand All @@ -50,3 +50,17 @@ func AddAnnotation(obj client.Object, annotation, value string) bool {
}
return false
}

// DecodeObjBytes decodes raw YAML/JSON bytes into a typed Kubernetes object.
// Panics on decode failure or type mismatch.
func DecodeObjBytes[T runtime.Object](codecs serializer.CodecFactory, gv schema.GroupVersion, objBytes []byte) T {
obj, err := runtime.Decode(codecs.UniversalDecoder(gv), objBytes)
if err != nil {
panic(fmt.Sprintf("failed to decode object bytes for %T: %v", *new(T), err))
}
typed, ok := obj.(T)
if !ok {
panic(fmt.Sprintf("failed to convert decoded object to %T", *new(T)))
}
return typed
}
Comment on lines +54 to +66
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 17, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid panics in decode helper used by reconcilers.

DecodeObjBytes panics on decode/type mismatch, which can crash the operator process and cause crash loops instead of returning a reconcile error.

Proposed fix 
-func DecodeObjBytes[T runtime.Object](codecs serializer.CodecFactory, gv schema.GroupVersion, objBytes []byte) T {
+func DecodeObjBytes[T runtime.Object](codecs serializer.CodecFactory, gv schema.GroupVersion, objBytes []byte) (T, error) {
+	var zero T
 	obj, err := runtime.Decode(codecs.UniversalDecoder(gv), objBytes)
 	if err != nil {
-		panic(fmt.Sprintf("failed to decode object bytes for %T: %v", *new(T), err))
+		return zero, fmt.Errorf("failed to decode object bytes for %T: %w", *new(T), err)
 	}
 	typed, ok := obj.(T)
 	if !ok {
-		panic(fmt.Sprintf("failed to convert decoded object to %T", *new(T)))
+		return zero, fmt.Errorf("failed to convert decoded object to %T", *new(T))
 	}
-	return typed
+	return typed, nil
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// DecodeObjBytes decodes raw YAML/JSON bytes into a typed Kubernetes object.
// Panics on decode failure or type mismatch.
func DecodeObjBytes[T runtime.Object](codecs serializer.CodecFactory, gv schema.GroupVersion, objBytes []byte) T {
obj, err := runtime.Decode(codecs.UniversalDecoder(gv), objBytes)
if err != nil {
panic(fmt.Sprintf("failed to decode object bytes for %T: %v", *new(T), err))
}
typed, ok := obj.(T)
if !ok {
panic(fmt.Sprintf("failed to convert decoded object to %T", *new(T)))
}
return typed
}
// DecodeObjBytes decodes raw YAML/JSON bytes into a typed Kubernetes object.
// Returns an error on decode failure or type mismatch.
func DecodeObjBytes[T runtime.Object](codecs serializer.CodecFactory, gv schema.GroupVersion, objBytes []byte) (T, error) {
var zero T
obj, err := runtime.Decode(codecs.UniversalDecoder(gv), objBytes)
if err != nil {
return zero, fmt.Errorf("failed to decode object bytes for %T: %w", *new(T), err)
}
typed, ok := obj.(T)
if !ok {
return zero, fmt.Errorf("failed to convert decoded object to %T", *new(T))
}
return typed, nil
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/controller/common/utils.go` around lines 54 - 66, DecodeObjBytes
currently panics on decode or type-mismatch; change its signature from
DecodeObjBytes[T runtime.Object](codecs serializer.CodecFactory, gv
schema.GroupVersion, objBytes []byte) T to return (T, error) and replace the
panic calls with returned errors (e.g., fmt.Errorf with context about decode
failure or type assertion failure), returning the zero value of T on error;
update all callers (especially reconcilers) to handle the error instead of
letting it crash the process (propagate the error or convert to a reconcile
error/status) so no panics are possible from runtime.Decode or the type
assertion.

Copy link
Copy Markdown
Contributor

@mytreya-rh mytreya-rh Mar 20, 2026

Choose a reason for hiding this comment

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

This change is not needed as all the callers of this function pass static manifest data which is present in the binary. Inability to decode such static manifest should lead to panic.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 20, 2026

Choose a reason for hiding this comment

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

@mytreya-rh, understood — the panic is intentional here as a "fail-fast on programmer error" pattern. Since all callers supply statically embedded asset bytes baked into the binary, a decode failure would indicate a build-time bug rather than a runtime condition, making panic the correct signal. I'll update my understanding for future reviews.

📝 Learnings were identified and not saved (knowledge base disabled). Enable

54 changes: 54 additions & 0 deletions pkg/controller/common/validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package common

import (
"unsafe"

corev1 "k8s.io/api/core/v1"
apivalidation "k8s.io/apimachinery/pkg/api/validation"
metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/kubernetes/pkg/apis/core"
corevalidation "k8s.io/kubernetes/pkg/apis/core/validation"
)

// ValidateNodeSelectorConfig validates the NodeSelector configuration using
// the Kubernetes label validation rules.
func ValidateNodeSelectorConfig(nodeSelector map[string]string, fldPath *field.Path) error {
return metav1validation.ValidateLabels(nodeSelector, fldPath.Child("nodeSelector")).ToAggregate()
}

// ValidateTolerationsConfig validates the Tolerations configuration using
// the Kubernetes core toleration validation rules.
func ValidateTolerationsConfig(tolerations []corev1.Toleration, fldPath *field.Path) error {
// convert corev1.Tolerations to core.Tolerations, required for validation.
convTolerations := *(*[]core.Toleration)(unsafe.Pointer(&tolerations))
return corevalidation.ValidateTolerations(convTolerations, fldPath.Child("tolerations")).ToAggregate()
}

// ValidateResourceRequirements validates the ResourceRequirements configuration
// using the Kubernetes core resource requirements validation rules.
func ValidateResourceRequirements(requirements corev1.ResourceRequirements, fldPath *field.Path) error {
// convert corev1.ResourceRequirements to core.ResourceRequirements, required for validation.
convRequirements := *(*core.ResourceRequirements)(unsafe.Pointer(&requirements))
return corevalidation.ValidateContainerResourceRequirements(&convRequirements, nil, fldPath.Child("resources"), corevalidation.PodValidationOptions{}).ToAggregate()
}

// ValidateAffinityRules validates the Affinity configuration using
// the Kubernetes core affinity validation rules.
func ValidateAffinityRules(affinity *corev1.Affinity, fldPath *field.Path) error {
// convert corev1.Affinity to core.Affinity, required for validation.
convAffinity := (*core.Affinity)(unsafe.Pointer(affinity))
return validateAffinity(convAffinity, corevalidation.PodValidationOptions{}, fldPath.Child("affinity")).ToAggregate()
}

// ValidateLabelsConfig validates label keys and values using the Kubernetes
// metadata validation rules.
func ValidateLabelsConfig(labels map[string]string, fldPath *field.Path) error {
return metav1validation.ValidateLabels(labels, fldPath.Child("labels")).ToAggregate()
}

// ValidateAnnotationsConfig validates annotation keys and sizes using the
// Kubernetes metadata validation rules.
func ValidateAnnotationsConfig(annotations map[string]string, fldPath *field.Path) error {
return apivalidation.ValidateAnnotations(annotations, fldPath.Child("annotations")).ToAggregate()
}
2 changes: 2 additions & 0 deletions pkg/controller/istiocsr/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,8 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
Watches(&corev1.ConfigMap{}, handler.EnqueueRequestsFromMapFunc(mapFunc), controllerConfigMapWatchPredicates).
WatchesMetadata(&corev1.Secret{}, handler.EnqueueRequestsFromMapFunc(mapFunc), controllerWatchResourcePredicates).
Watches(&networkingv1.NetworkPolicy{}, handler.EnqueueRequestsFromMapFunc(mapFunc), controllerManagedResourcePredicates).
Watches(&certmanagerv1.Issuer{}, handler.EnqueueRequestsFromMapFunc(mapFunc), controllerWatchResourcePredicates).
Watches(&certmanagerv1.ClusterIssuer{}, handler.EnqueueRequestsFromMapFunc(mapFunc), controllerWatchResourcePredicates).
Complete(r)
}

Expand Down
39 changes: 8 additions & 31 deletions pkg/controller/istiocsr/deployments.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,12 @@ import (
"os"
"reflect"
"strings"
"unsafe"

appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/kubernetes/pkg/apis/core"
corevalidation "k8s.io/kubernetes/pkg/apis/core/validation"
"sigs.k8s.io/controller-runtime/pkg/client"

certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
Expand Down Expand Up @@ -188,7 +184,7 @@ func updateResourceRequirement(deployment *appsv1.Deployment, istiocsr *v1alpha1
if reflect.ValueOf(istiocsr.Spec.IstioCSRConfig.Resources).IsZero() {
return nil
}
if err := validateResourceRequirements(istiocsr.Spec.IstioCSRConfig.Resources,
if err := common.ValidateResourceRequirements(istiocsr.Spec.IstioCSRConfig.Resources,
field.NewPath("spec", "istioCSRConfig")); err != nil {
return err
}
Expand All @@ -202,7 +198,7 @@ func updateAffinityRules(deployment *appsv1.Deployment, istiocsr *v1alpha1.Istio
if istiocsr.Spec.IstioCSRConfig.Affinity == nil {
return nil
}
if err := validateAffinityRules(istiocsr.Spec.IstioCSRConfig.Affinity,
if err := common.ValidateAffinityRules(istiocsr.Spec.IstioCSRConfig.Affinity,
field.NewPath("spec", "istioCSRConfig")); err != nil {
return err
}
Expand All @@ -214,7 +210,7 @@ func updatePodTolerations(deployment *appsv1.Deployment, istiocsr *v1alpha1.Isti
if istiocsr.Spec.IstioCSRConfig.Tolerations == nil {
return nil
}
if err := validateTolerationsConfig(istiocsr.Spec.IstioCSRConfig.Tolerations,
if err := common.ValidateTolerationsConfig(istiocsr.Spec.IstioCSRConfig.Tolerations,
field.NewPath("spec", "istioCSRConfig")); err != nil {
return err
}
Expand All @@ -226,7 +222,7 @@ func updateNodeSelector(deployment *appsv1.Deployment, istiocsr *v1alpha1.IstioC
if istiocsr.Spec.IstioCSRConfig.NodeSelector == nil {
return nil
}
if err := validateNodeSelectorConfig(istiocsr.Spec.IstioCSRConfig.NodeSelector,
if err := common.ValidateNodeSelectorConfig(istiocsr.Spec.IstioCSRConfig.NodeSelector,
field.NewPath("spec", "istioCSRConfig")); err != nil {
return err
}
Expand Down Expand Up @@ -269,6 +265,10 @@ func (r *Reconciler) assertIssuerRefExists(istiocsr *v1alpha1.IstioCSR) error {
return common.NewIrrecoverableError(errInvalidIssuerRefConfig, "spec.istioCSRConfig.certManager.issuerRef uses unsupported ACME issuer")
}

if err := r.updateWatchLabel(obj, istiocsr); err != nil {
return common.FromClientError(err, "failed to update watch label on cert-manager issuer %s", istiocsr.Spec.IstioCSRConfig.CertManager.IssuerRef.Name)
}

return nil
}

Expand Down Expand Up @@ -641,26 +641,3 @@ func (r *Reconciler) updateWatchLabel(obj client.Object, istiocsr *v1alpha1.Isti
}
return nil
}

// validateNodeSelectorConfig validates the NodeSelector configuration.
func validateNodeSelectorConfig(nodeSelector map[string]string, fldPath *field.Path) error {
return metav1validation.ValidateLabels(nodeSelector, fldPath.Child("nodeSelector")).ToAggregate()
}

func validateTolerationsConfig(tolerations []corev1.Toleration, fldPath *field.Path) error {
// convert corev1.Tolerations to core.Tolerations, required for validation.
convTolerations := *(*[]core.Toleration)(unsafe.Pointer(&tolerations))
return corevalidation.ValidateTolerations(convTolerations, fldPath.Child("tolerations")).ToAggregate()
}

func validateResourceRequirements(requirements corev1.ResourceRequirements, fldPath *field.Path) error {
// convert corev1.ResourceRequirements to core.ResourceRequirements, required for validation.
convRequirements := *(*core.ResourceRequirements)(unsafe.Pointer(&requirements))
return corevalidation.ValidateContainerResourceRequirements(&convRequirements, nil, fldPath.Child("resources"), corevalidation.PodValidationOptions{}).ToAggregate()
}

func validateAffinityRules(affinity *corev1.Affinity, fldPath *field.Path) error {
// convert corev1.Affinity to core.Affinity, required for validation.
convAffinity := (*core.Affinity)(unsafe.Pointer(affinity))
return validateAffinity(convAffinity, corevalidation.PodValidationOptions{}, fldPath.Child("affinity")).ToAggregate()
}
Loading