From b6229113505b33f3ebf8bfa533cb08a0d9a20be9 Mon Sep 17 00:00:00 2001 From: Brandon Palm Date: Wed, 6 May 2026 10:56:21 -0500 Subject: [PATCH] CM-1038: Replace istiocsr type-specific decoders with generic DecodeObjBytes Remove 8 type-specific decode*ObjBytes functions from the istiocsr package and replace all call sites with the existing generic common.DecodeObjBytes[T] helper, matching the pattern already used by the trustmanager package. Cache the ServiceAccount name at init time to avoid redundant YAML deserialization on every reconcile cycle. --- pkg/controller/istiocsr/certificates.go | 2 +- pkg/controller/istiocsr/deployments.go | 2 +- pkg/controller/istiocsr/rbacs.go | 14 +-- pkg/controller/istiocsr/serviceaccounts.go | 2 +- pkg/controller/istiocsr/services.go | 4 +- pkg/controller/istiocsr/test_utils.go | 23 ++--- pkg/controller/istiocsr/utils.go | 104 ++------------------- 7 files changed, 30 insertions(+), 121 deletions(-) diff --git a/pkg/controller/istiocsr/certificates.go b/pkg/controller/istiocsr/certificates.go index c57c2d944..3b959e112 100644 --- a/pkg/controller/istiocsr/certificates.go +++ b/pkg/controller/istiocsr/certificates.go @@ -56,7 +56,7 @@ func (r *Reconciler) createOrApplyCertificates(istiocsr *v1alpha1.IstioCSR, reso } func (r *Reconciler) getCertificateObject(istiocsr *v1alpha1.IstioCSR, resourceLabels map[string]string) (*certmanagerv1.Certificate, error) { - certificate := decodeCertificateObjBytes(assets.MustAsset(certificateAssetName)) + certificate := common.DecodeObjBytes[*certmanagerv1.Certificate](codecs, certmanagerv1.SchemeGroupVersion, assets.MustAsset(certificateAssetName)) common.UpdateNamespace(certificate, istiocsr.Spec.IstioCSRConfig.Istio.Namespace) // add custom label for identification on the object created in different namespace. diff --git a/pkg/controller/istiocsr/deployments.go b/pkg/controller/istiocsr/deployments.go index d709229a1..c4c82fe09 100644 --- a/pkg/controller/istiocsr/deployments.go +++ b/pkg/controller/istiocsr/deployments.go @@ -75,7 +75,7 @@ func (r *Reconciler) getDeploymentObject(istiocsr *v1alpha1.IstioCSR, resourceLa return nil, fmt.Errorf("failed to verify issuer in %s/%s: %w", istiocsr.GetNamespace(), istiocsr.GetName(), err) } - deployment := decodeDeploymentObjBytes(assets.MustAsset(deploymentAssetName)) + deployment := common.DecodeObjBytes[*appsv1.Deployment](codecs, appsv1.SchemeGroupVersion, assets.MustAsset(deploymentAssetName)) common.UpdateNamespace(deployment, istiocsr.GetNamespace()) common.UpdateResourceLabels(deployment, resourceLabels) diff --git a/pkg/controller/istiocsr/rbacs.go b/pkg/controller/istiocsr/rbacs.go index 1d77c1a63..da91242f0 100644 --- a/pkg/controller/istiocsr/rbacs.go +++ b/pkg/controller/istiocsr/rbacs.go @@ -18,7 +18,7 @@ const ( ) func (r *Reconciler) createOrApplyRBACResource(istiocsr *v1alpha1.IstioCSR, resourceLabels map[string]string, istioCSRCreateRecon bool) error { - serviceAccount := decodeServiceAccountObjBytes(assets.MustAsset(serviceAccountAssetName)).GetName() + serviceAccount := cachedServiceAccountName clusterRoleName, err := r.createOrApplyClusterRoles(istiocsr, resourceLabels, istioCSRCreateRecon) if err != nil { @@ -129,7 +129,7 @@ func (r *Reconciler) createOrApplyClusterRoles(istiocsr *v1alpha1.IstioCSR, reso } func (r *Reconciler) getClusterRoleObject(istioCSRNamespace string, resourceLabels map[string]string) *rbacv1.ClusterRole { - clusterRole := decodeClusterRoleObjBytes(assets.MustAsset(clusterRoleAssetName)) + clusterRole := common.DecodeObjBytes[*rbacv1.ClusterRole](codecs, rbacv1.SchemeGroupVersion, assets.MustAsset(clusterRoleAssetName)) updateToUseGenerateName(clusterRole) updateResourceLabelsWithIstioMapperLabels(clusterRole, istioCSRNamespace, resourceLabels) return clusterRole @@ -227,7 +227,7 @@ func (r *Reconciler) createOrApplyClusterRoleBindings(istiocsr *v1alpha1.IstioCS } func (r *Reconciler) getClusterRoleBindingObject(clusterRoleName, serviceAccount, istiocsrNamespace string, resourceLabels map[string]string) *rbacv1.ClusterRoleBinding { - clusterRoleBinding := decodeClusterRoleBindingObjBytes(assets.MustAsset(clusterRoleBindingAssetName)) + clusterRoleBinding := common.DecodeObjBytes[*rbacv1.ClusterRoleBinding](codecs, rbacv1.SchemeGroupVersion, assets.MustAsset(clusterRoleBindingAssetName)) clusterRoleBinding.RoleRef.Name = clusterRoleName updateToUseGenerateName(clusterRoleBinding) updateResourceLabelsWithIstioMapperLabels(clusterRoleBinding, istiocsrNamespace, resourceLabels) @@ -285,7 +285,7 @@ func (r *Reconciler) createOrApplyRoles(istiocsr *v1alpha1.IstioCSR, resourceLab } func (r *Reconciler) getRoleObject(istiocsrNamespace, roleNamespace string, resourceLabels map[string]string) *rbacv1.Role { - role := decodeRoleObjBytes(assets.MustAsset(roleAssetName)) + role := common.DecodeObjBytes[*rbacv1.Role](codecs, rbacv1.SchemeGroupVersion, assets.MustAsset(roleAssetName)) common.UpdateNamespace(role, roleNamespace) updateResourceLabelsWithIstioMapperLabels(role, istiocsrNamespace, resourceLabels) return role @@ -328,7 +328,7 @@ func (r *Reconciler) createOrApplyRoleBindings(istiocsr *v1alpha1.IstioCSR, serv } func (r *Reconciler) getRoleBindingObject(serviceAccount, istiocsrNamespace, roleNamespace string, resourceLabels map[string]string) *rbacv1.RoleBinding { - roleBinding := decodeRoleBindingObjBytes(assets.MustAsset(roleBindingAssetName)) + roleBinding := common.DecodeObjBytes[*rbacv1.RoleBinding](codecs, rbacv1.SchemeGroupVersion, assets.MustAsset(roleBindingAssetName)) common.UpdateNamespace(roleBinding, roleNamespace) updateResourceLabelsWithIstioMapperLabels(roleBinding, istiocsrNamespace, resourceLabels) updateServiceAccountNamespaceInRBACBindingObject[*rbacv1.RoleBinding](roleBinding, serviceAccount, istiocsrNamespace) @@ -372,7 +372,7 @@ func (r *Reconciler) createOrApplyRoleForLeases(istiocsr *v1alpha1.IstioCSR, res } func (r *Reconciler) getRoleForLeasesObject(istiocsrNamespace, roleNamespace string, resourceLabels map[string]string) *rbacv1.Role { - role := decodeRoleObjBytes(assets.MustAsset(roleLeasesAssetName)) + role := common.DecodeObjBytes[*rbacv1.Role](codecs, rbacv1.SchemeGroupVersion, assets.MustAsset(roleLeasesAssetName)) common.UpdateNamespace(role, roleNamespace) updateResourceLabelsWithIstioMapperLabels(role, istiocsrNamespace, resourceLabels) return role @@ -415,7 +415,7 @@ func (r *Reconciler) createOrApplyRoleBindingForLeases(istiocsr *v1alpha1.IstioC } func (r *Reconciler) getRoleBindingForLeasesObject(serviceAccount, istiocsrNamespace, roleNamespace string, resourceLabels map[string]string) *rbacv1.RoleBinding { - roleBinding := decodeRoleBindingObjBytes(assets.MustAsset(roleBindingLeasesAssetName)) + roleBinding := common.DecodeObjBytes[*rbacv1.RoleBinding](codecs, rbacv1.SchemeGroupVersion, assets.MustAsset(roleBindingLeasesAssetName)) common.UpdateNamespace(roleBinding, roleNamespace) updateResourceLabelsWithIstioMapperLabels(roleBinding, istiocsrNamespace, resourceLabels) updateServiceAccountNamespaceInRBACBindingObject[*rbacv1.RoleBinding](roleBinding, serviceAccount, istiocsrNamespace) diff --git a/pkg/controller/istiocsr/serviceaccounts.go b/pkg/controller/istiocsr/serviceaccounts.go index 6930dc66b..84fbce986 100644 --- a/pkg/controller/istiocsr/serviceaccounts.go +++ b/pkg/controller/istiocsr/serviceaccounts.go @@ -51,7 +51,7 @@ func (r *Reconciler) createOrApplyServiceAccounts(istiocsr *v1alpha1.IstioCSR, r } func (r *Reconciler) getServiceAccountObject(istiocsr *v1alpha1.IstioCSR, resourceLabels map[string]string) *corev1.ServiceAccount { - serviceAccount := decodeServiceAccountObjBytes(assets.MustAsset(serviceAccountAssetName)) + serviceAccount := common.DecodeObjBytes[*corev1.ServiceAccount](codecs, corev1.SchemeGroupVersion, assets.MustAsset(serviceAccountAssetName)) common.UpdateNamespace(serviceAccount, istiocsr.GetNamespace()) common.UpdateResourceLabels(serviceAccount, resourceLabels) return serviceAccount diff --git a/pkg/controller/istiocsr/services.go b/pkg/controller/istiocsr/services.go index bc49f01f9..9a6c34cab 100644 --- a/pkg/controller/istiocsr/services.go +++ b/pkg/controller/istiocsr/services.go @@ -66,7 +66,7 @@ func (r *Reconciler) createOrApplyService(istiocsr *v1alpha1.IstioCSR, svc *core } func (r *Reconciler) getServiceObject(istiocsr *v1alpha1.IstioCSR, resourceLabels map[string]string) *corev1.Service { - service := decodeServiceObjBytes(assets.MustAsset(serviceAssetName)) + service := common.DecodeObjBytes[*corev1.Service](codecs, corev1.SchemeGroupVersion, assets.MustAsset(serviceAssetName)) common.UpdateNamespace(service, istiocsr.GetNamespace()) common.UpdateResourceLabels(service, resourceLabels) if istiocsr.Spec.IstioCSRConfig.Server != nil { @@ -76,7 +76,7 @@ func (r *Reconciler) getServiceObject(istiocsr *v1alpha1.IstioCSR, resourceLabel } func (r *Reconciler) getMetricsServiceObject(istiocsr *v1alpha1.IstioCSR, resourceLabels map[string]string) *corev1.Service { - service := decodeServiceObjBytes(assets.MustAsset(metricsServiceAssetName)) + service := common.DecodeObjBytes[*corev1.Service](codecs, corev1.SchemeGroupVersion, assets.MustAsset(metricsServiceAssetName)) common.UpdateNamespace(service, istiocsr.GetNamespace()) common.UpdateResourceLabels(service, resourceLabels) return service diff --git a/pkg/controller/istiocsr/test_utils.go b/pkg/controller/istiocsr/test_utils.go index f2276b9f2..19d59e710 100644 --- a/pkg/controller/istiocsr/test_utils.go +++ b/pkg/controller/istiocsr/test_utils.go @@ -25,6 +25,7 @@ import ( certmanagermetav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" + "github.com/openshift/cert-manager-operator/pkg/controller/common" "github.com/openshift/cert-manager-operator/pkg/operator/assets" "github.com/openshift/cert-manager-operator/pkg/testutil" ) @@ -138,7 +139,7 @@ func testACMEIssuer() *certmanagerv1.ClusterIssuer { } func testCertificate() *certmanagerv1.Certificate { - cert := decodeCertificateObjBytes(assets.MustAsset(certificateAssetName)) + cert := common.DecodeObjBytes[*certmanagerv1.Certificate](codecs, certmanagerv1.SchemeGroupVersion, assets.MustAsset(certificateAssetName)) cert.SetNamespace(testIstiodNamespace) labels := make(map[string]string) maps.Copy(labels, controllerDefaultResourceLabels) @@ -154,14 +155,14 @@ func testCertificate() *certmanagerv1.Certificate { } func testClusterRole() *rbacv1.ClusterRole { - role := decodeClusterRoleObjBytes(assets.MustAsset(clusterRoleAssetName)) + role := common.DecodeObjBytes[*rbacv1.ClusterRole](codecs, rbacv1.SchemeGroupVersion, assets.MustAsset(clusterRoleAssetName)) role.SetName("cert-manager-istio-csr-sdghj") role.SetLabels(controllerDefaultResourceLabels) return role } func testClusterRoleBinding() *rbacv1.ClusterRoleBinding { - roleBinding := decodeClusterRoleBindingObjBytes(assets.MustAsset(clusterRoleBindingAssetName)) + roleBinding := common.DecodeObjBytes[*rbacv1.ClusterRoleBinding](codecs, rbacv1.SchemeGroupVersion, assets.MustAsset(clusterRoleBindingAssetName)) roleBinding.SetName("cert-manager-istio-csr-dfkhk") roleBinding.SetGenerateName("cert-manager-istio-csr-") roleBinding.SetLabels(controllerDefaultResourceLabels) @@ -169,7 +170,7 @@ func testClusterRoleBinding() *rbacv1.ClusterRoleBinding { } func testClusterRoleBindingExtra() *rbacv1.ClusterRoleBinding { - roleBinding := decodeClusterRoleBindingObjBytes(assets.MustAsset(clusterRoleBindingAssetName)) + roleBinding := common.DecodeObjBytes[*rbacv1.ClusterRoleBinding](codecs, rbacv1.SchemeGroupVersion, assets.MustAsset(clusterRoleBindingAssetName)) roleBinding.SetName("cert-manager-istio-csr-dfmfj") roleBinding.SetGenerateName("cert-manager-istio-csr-") roleBinding.SetLabels(controllerDefaultResourceLabels) @@ -177,7 +178,7 @@ func testClusterRoleBindingExtra() *rbacv1.ClusterRoleBinding { } func testDeployment() *appsv1.Deployment { - deployment := decodeDeploymentObjBytes(assets.MustAsset(deploymentAssetName)) + deployment := common.DecodeObjBytes[*appsv1.Deployment](codecs, appsv1.SchemeGroupVersion, assets.MustAsset(deploymentAssetName)) deployment.SetNamespace(testIstioCSRNamespace) deployment.SetLabels(controllerDefaultResourceLabels) deployment.Spec.Template.Labels = controllerDefaultResourceLabels @@ -186,42 +187,42 @@ func testDeployment() *appsv1.Deployment { } func testRole() *rbacv1.Role { - role := decodeRoleObjBytes(assets.MustAsset(roleAssetName)) + role := common.DecodeObjBytes[*rbacv1.Role](codecs, rbacv1.SchemeGroupVersion, assets.MustAsset(roleAssetName)) role.SetNamespace(testIstiodNamespace) role.SetLabels(controllerDefaultResourceLabels) return role } func testRoleBinding() *rbacv1.RoleBinding { - roleBinding := decodeRoleBindingObjBytes(assets.MustAsset(roleBindingAssetName)) + roleBinding := common.DecodeObjBytes[*rbacv1.RoleBinding](codecs, rbacv1.SchemeGroupVersion, assets.MustAsset(roleBindingAssetName)) roleBinding.SetNamespace(testIstiodNamespace) roleBinding.SetLabels(controllerDefaultResourceLabels) return roleBinding } func testRoleLeases() *rbacv1.Role { - role := decodeRoleObjBytes(assets.MustAsset(roleLeasesAssetName)) + role := common.DecodeObjBytes[*rbacv1.Role](codecs, rbacv1.SchemeGroupVersion, assets.MustAsset(roleLeasesAssetName)) role.SetNamespace(testIstiodNamespace) role.SetLabels(controllerDefaultResourceLabels) return role } func testRoleBindingLeases() *rbacv1.RoleBinding { - roleBinding := decodeRoleBindingObjBytes(assets.MustAsset(roleBindingLeasesAssetName)) + roleBinding := common.DecodeObjBytes[*rbacv1.RoleBinding](codecs, rbacv1.SchemeGroupVersion, assets.MustAsset(roleBindingLeasesAssetName)) roleBinding.SetNamespace(testIstiodNamespace) roleBinding.SetLabels(controllerDefaultResourceLabels) return roleBinding } func testService() *corev1.Service { - service := decodeServiceObjBytes(assets.MustAsset(serviceAssetName)) + service := common.DecodeObjBytes[*corev1.Service](codecs, corev1.SchemeGroupVersion, assets.MustAsset(serviceAssetName)) service.SetNamespace(testIstioCSRNamespace) service.SetLabels(controllerDefaultResourceLabels) return service } func testServiceAccount() *corev1.ServiceAccount { - serviceAccount := decodeServiceAccountObjBytes(assets.MustAsset(serviceAccountAssetName)) + serviceAccount := common.DecodeObjBytes[*corev1.ServiceAccount](codecs, corev1.SchemeGroupVersion, assets.MustAsset(serviceAccountAssetName)) serviceAccount.SetNamespace(testIstioCSRNamespace) serviceAccount.SetLabels(controllerDefaultResourceLabels) return serviceAccount diff --git a/pkg/controller/istiocsr/utils.go b/pkg/controller/istiocsr/utils.go index 34477b561..643e10485 100644 --- a/pkg/controller/istiocsr/utils.go +++ b/pkg/controller/istiocsr/utils.go @@ -22,11 +22,13 @@ import ( "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" "github.com/openshift/cert-manager-operator/pkg/controller/common" + "github.com/openshift/cert-manager-operator/pkg/operator/assets" ) var ( - scheme = runtime.NewScheme() - codecs = serializer.NewCodecFactory(scheme) + scheme = runtime.NewScheme() + codecs = serializer.NewCodecFactory(scheme) + cachedServiceAccountName string ) func init() { @@ -45,6 +47,8 @@ func init() { if err := certmanagerv1.AddToScheme(scheme); err != nil { panic(err) } + + cachedServiceAccountName = common.DecodeObjBytes[*corev1.ServiceAccount](codecs, corev1.SchemeGroupVersion, assets.MustAsset(serviceAccountAssetName)).GetName() } // updateStatus is for updating the status subresource of istiocsr.openshift.operator.io. @@ -133,102 +137,6 @@ func updateResourceLabelsWithIstioMapperLabels(obj client.Object, istiocsrNamesp obj.SetLabels(l) } -func decodeDeploymentObjBytes(objBytes []byte) *appsv1.Deployment { - obj, err := runtime.Decode(codecs.UniversalDecoder(appsv1.SchemeGroupVersion), objBytes) - if err != nil { - panic(err) - } - deployment, ok := obj.(*appsv1.Deployment) - if !ok { - panic("failed to convert to *appsv1.Deployment") - } - return deployment -} - -func decodeClusterRoleObjBytes(objBytes []byte) *rbacv1.ClusterRole { - obj, err := runtime.Decode(codecs.UniversalDecoder(rbacv1.SchemeGroupVersion), objBytes) - if err != nil { - panic(err) - } - clusterRole, ok := obj.(*rbacv1.ClusterRole) - if !ok { - panic("failed to convert to *rbacv1.ClusterRole") - } - return clusterRole -} - -func decodeClusterRoleBindingObjBytes(objBytes []byte) *rbacv1.ClusterRoleBinding { - obj, err := runtime.Decode(codecs.UniversalDecoder(rbacv1.SchemeGroupVersion), objBytes) - if err != nil { - panic(err) - } - clusterRoleBinding, ok := obj.(*rbacv1.ClusterRoleBinding) - if !ok { - panic("failed to convert to *rbacv1.ClusterRoleBinding") - } - return clusterRoleBinding -} - -func decodeRoleObjBytes(objBytes []byte) *rbacv1.Role { - obj, err := runtime.Decode(codecs.UniversalDecoder(rbacv1.SchemeGroupVersion), objBytes) - if err != nil { - panic(err) - } - role, ok := obj.(*rbacv1.Role) - if !ok { - panic("failed to convert to *rbacv1.Role") - } - return role -} - -func decodeRoleBindingObjBytes(objBytes []byte) *rbacv1.RoleBinding { - obj, err := runtime.Decode(codecs.UniversalDecoder(rbacv1.SchemeGroupVersion), objBytes) - if err != nil { - panic(err) - } - roleBinding, ok := obj.(*rbacv1.RoleBinding) - if !ok { - panic("failed to convert to *rbacv1.RoleBinding") - } - return roleBinding -} - -func decodeServiceObjBytes(objBytes []byte) *corev1.Service { - obj, err := runtime.Decode(codecs.UniversalDecoder(corev1.SchemeGroupVersion), objBytes) - if err != nil { - panic(err) - } - service, ok := obj.(*corev1.Service) - if !ok { - panic("failed to convert to *corev1.Service") - } - return service -} - -func decodeServiceAccountObjBytes(objBytes []byte) *corev1.ServiceAccount { - obj, err := runtime.Decode(codecs.UniversalDecoder(corev1.SchemeGroupVersion), objBytes) - if err != nil { - panic(err) - } - serviceAccount, ok := obj.(*corev1.ServiceAccount) - if !ok { - panic("failed to convert to *corev1.ServiceAccount") - } - return serviceAccount -} - -func decodeCertificateObjBytes(objBytes []byte) *certmanagerv1.Certificate { - obj, err := runtime.Decode(codecs.UniversalDecoder(certmanagerv1.SchemeGroupVersion), objBytes) - if err != nil { - panic(err) - } - certificate, ok := obj.(*certmanagerv1.Certificate) - if !ok { - panic("failed to convert to *certmanagerv1.Certificate") - } - return certificate -} - func hasObjectChanged(desired, fetched client.Object) bool { if reflect.TypeOf(desired) != reflect.TypeOf(fetched) { panic("both objects to be compared must be of same type")