Skip to content

Commit b251d62

Browse files
committed
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.
1 parent e8a646d commit b251d62

7 files changed

Lines changed: 30 additions & 121 deletions

File tree

pkg/controller/istiocsr/certificates.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func (r *Reconciler) createOrApplyCertificates(istiocsr *v1alpha1.IstioCSR, reso
5656
}
5757

5858
func (r *Reconciler) getCertificateObject(istiocsr *v1alpha1.IstioCSR, resourceLabels map[string]string) (*certmanagerv1.Certificate, error) {
59-
certificate := decodeCertificateObjBytes(assets.MustAsset(certificateAssetName))
59+
certificate := common.DecodeObjBytes[*certmanagerv1.Certificate](codecs, certmanagerv1.SchemeGroupVersion, assets.MustAsset(certificateAssetName))
6060

6161
common.UpdateNamespace(certificate, istiocsr.Spec.IstioCSRConfig.Istio.Namespace)
6262
// add custom label for identification on the object created in different namespace.

pkg/controller/istiocsr/deployments.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ func (r *Reconciler) getDeploymentObject(istiocsr *v1alpha1.IstioCSR, resourceLa
7575
return nil, fmt.Errorf("failed to verify issuer in %s/%s: %w", istiocsr.GetNamespace(), istiocsr.GetName(), err)
7676
}
7777

78-
deployment := decodeDeploymentObjBytes(assets.MustAsset(deploymentAssetName))
78+
deployment := common.DecodeObjBytes[*appsv1.Deployment](codecs, appsv1.SchemeGroupVersion, assets.MustAsset(deploymentAssetName))
7979

8080
common.UpdateNamespace(deployment, istiocsr.GetNamespace())
8181
common.UpdateResourceLabels(deployment, resourceLabels)

pkg/controller/istiocsr/rbacs.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ const (
1818
)
1919

2020
func (r *Reconciler) createOrApplyRBACResource(istiocsr *v1alpha1.IstioCSR, resourceLabels map[string]string, istioCSRCreateRecon bool) error {
21-
serviceAccount := decodeServiceAccountObjBytes(assets.MustAsset(serviceAccountAssetName)).GetName()
21+
serviceAccount := cachedServiceAccountName
2222

2323
clusterRoleName, err := r.createOrApplyClusterRoles(istiocsr, resourceLabels, istioCSRCreateRecon)
2424
if err != nil {
@@ -129,7 +129,7 @@ func (r *Reconciler) createOrApplyClusterRoles(istiocsr *v1alpha1.IstioCSR, reso
129129
}
130130

131131
func (r *Reconciler) getClusterRoleObject(istioCSRNamespace string, resourceLabels map[string]string) *rbacv1.ClusterRole {
132-
clusterRole := decodeClusterRoleObjBytes(assets.MustAsset(clusterRoleAssetName))
132+
clusterRole := common.DecodeObjBytes[*rbacv1.ClusterRole](codecs, rbacv1.SchemeGroupVersion, assets.MustAsset(clusterRoleAssetName))
133133
updateToUseGenerateName(clusterRole)
134134
updateResourceLabelsWithIstioMapperLabels(clusterRole, istioCSRNamespace, resourceLabels)
135135
return clusterRole
@@ -227,7 +227,7 @@ func (r *Reconciler) createOrApplyClusterRoleBindings(istiocsr *v1alpha1.IstioCS
227227
}
228228

229229
func (r *Reconciler) getClusterRoleBindingObject(clusterRoleName, serviceAccount, istiocsrNamespace string, resourceLabels map[string]string) *rbacv1.ClusterRoleBinding {
230-
clusterRoleBinding := decodeClusterRoleBindingObjBytes(assets.MustAsset(clusterRoleBindingAssetName))
230+
clusterRoleBinding := common.DecodeObjBytes[*rbacv1.ClusterRoleBinding](codecs, rbacv1.SchemeGroupVersion, assets.MustAsset(clusterRoleBindingAssetName))
231231
clusterRoleBinding.RoleRef.Name = clusterRoleName
232232
updateToUseGenerateName(clusterRoleBinding)
233233
updateResourceLabelsWithIstioMapperLabels(clusterRoleBinding, istiocsrNamespace, resourceLabels)
@@ -285,7 +285,7 @@ func (r *Reconciler) createOrApplyRoles(istiocsr *v1alpha1.IstioCSR, resourceLab
285285
}
286286

287287
func (r *Reconciler) getRoleObject(istiocsrNamespace, roleNamespace string, resourceLabels map[string]string) *rbacv1.Role {
288-
role := decodeRoleObjBytes(assets.MustAsset(roleAssetName))
288+
role := common.DecodeObjBytes[*rbacv1.Role](codecs, rbacv1.SchemeGroupVersion, assets.MustAsset(roleAssetName))
289289
common.UpdateNamespace(role, roleNamespace)
290290
updateResourceLabelsWithIstioMapperLabels(role, istiocsrNamespace, resourceLabels)
291291
return role
@@ -328,7 +328,7 @@ func (r *Reconciler) createOrApplyRoleBindings(istiocsr *v1alpha1.IstioCSR, serv
328328
}
329329

330330
func (r *Reconciler) getRoleBindingObject(serviceAccount, istiocsrNamespace, roleNamespace string, resourceLabels map[string]string) *rbacv1.RoleBinding {
331-
roleBinding := decodeRoleBindingObjBytes(assets.MustAsset(roleBindingAssetName))
331+
roleBinding := common.DecodeObjBytes[*rbacv1.RoleBinding](codecs, rbacv1.SchemeGroupVersion, assets.MustAsset(roleBindingAssetName))
332332
common.UpdateNamespace(roleBinding, roleNamespace)
333333
updateResourceLabelsWithIstioMapperLabels(roleBinding, istiocsrNamespace, resourceLabels)
334334
updateServiceAccountNamespaceInRBACBindingObject[*rbacv1.RoleBinding](roleBinding, serviceAccount, istiocsrNamespace)
@@ -372,7 +372,7 @@ func (r *Reconciler) createOrApplyRoleForLeases(istiocsr *v1alpha1.IstioCSR, res
372372
}
373373

374374
func (r *Reconciler) getRoleForLeasesObject(istiocsrNamespace, roleNamespace string, resourceLabels map[string]string) *rbacv1.Role {
375-
role := decodeRoleObjBytes(assets.MustAsset(roleLeasesAssetName))
375+
role := common.DecodeObjBytes[*rbacv1.Role](codecs, rbacv1.SchemeGroupVersion, assets.MustAsset(roleLeasesAssetName))
376376
common.UpdateNamespace(role, roleNamespace)
377377
updateResourceLabelsWithIstioMapperLabels(role, istiocsrNamespace, resourceLabels)
378378
return role
@@ -415,7 +415,7 @@ func (r *Reconciler) createOrApplyRoleBindingForLeases(istiocsr *v1alpha1.IstioC
415415
}
416416

417417
func (r *Reconciler) getRoleBindingForLeasesObject(serviceAccount, istiocsrNamespace, roleNamespace string, resourceLabels map[string]string) *rbacv1.RoleBinding {
418-
roleBinding := decodeRoleBindingObjBytes(assets.MustAsset(roleBindingLeasesAssetName))
418+
roleBinding := common.DecodeObjBytes[*rbacv1.RoleBinding](codecs, rbacv1.SchemeGroupVersion, assets.MustAsset(roleBindingLeasesAssetName))
419419
common.UpdateNamespace(roleBinding, roleNamespace)
420420
updateResourceLabelsWithIstioMapperLabels(roleBinding, istiocsrNamespace, resourceLabels)
421421
updateServiceAccountNamespaceInRBACBindingObject[*rbacv1.RoleBinding](roleBinding, serviceAccount, istiocsrNamespace)

pkg/controller/istiocsr/serviceaccounts.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func (r *Reconciler) createOrApplyServiceAccounts(istiocsr *v1alpha1.IstioCSR, r
5151
}
5252

5353
func (r *Reconciler) getServiceAccountObject(istiocsr *v1alpha1.IstioCSR, resourceLabels map[string]string) *corev1.ServiceAccount {
54-
serviceAccount := decodeServiceAccountObjBytes(assets.MustAsset(serviceAccountAssetName))
54+
serviceAccount := common.DecodeObjBytes[*corev1.ServiceAccount](codecs, corev1.SchemeGroupVersion, assets.MustAsset(serviceAccountAssetName))
5555
common.UpdateNamespace(serviceAccount, istiocsr.GetNamespace())
5656
common.UpdateResourceLabels(serviceAccount, resourceLabels)
5757
return serviceAccount

pkg/controller/istiocsr/services.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ func (r *Reconciler) createOrApplyService(istiocsr *v1alpha1.IstioCSR, svc *core
6666
}
6767

6868
func (r *Reconciler) getServiceObject(istiocsr *v1alpha1.IstioCSR, resourceLabels map[string]string) *corev1.Service {
69-
service := decodeServiceObjBytes(assets.MustAsset(serviceAssetName))
69+
service := common.DecodeObjBytes[*corev1.Service](codecs, corev1.SchemeGroupVersion, assets.MustAsset(serviceAssetName))
7070
common.UpdateNamespace(service, istiocsr.GetNamespace())
7171
common.UpdateResourceLabels(service, resourceLabels)
7272
if istiocsr.Spec.IstioCSRConfig.Server != nil {
@@ -76,7 +76,7 @@ func (r *Reconciler) getServiceObject(istiocsr *v1alpha1.IstioCSR, resourceLabel
7676
}
7777

7878
func (r *Reconciler) getMetricsServiceObject(istiocsr *v1alpha1.IstioCSR, resourceLabels map[string]string) *corev1.Service {
79-
service := decodeServiceObjBytes(assets.MustAsset(metricsServiceAssetName))
79+
service := common.DecodeObjBytes[*corev1.Service](codecs, corev1.SchemeGroupVersion, assets.MustAsset(metricsServiceAssetName))
8080
common.UpdateNamespace(service, istiocsr.GetNamespace())
8181
common.UpdateResourceLabels(service, resourceLabels)
8282
return service

pkg/controller/istiocsr/test_utils.go

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
certmanagermetav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1"
2626

2727
"github.com/openshift/cert-manager-operator/api/operator/v1alpha1"
28+
"github.com/openshift/cert-manager-operator/pkg/controller/common"
2829
"github.com/openshift/cert-manager-operator/pkg/operator/assets"
2930
"github.com/openshift/cert-manager-operator/pkg/testutil"
3031
)
@@ -138,7 +139,7 @@ func testACMEIssuer() *certmanagerv1.ClusterIssuer {
138139
}
139140

140141
func testCertificate() *certmanagerv1.Certificate {
141-
cert := decodeCertificateObjBytes(assets.MustAsset(certificateAssetName))
142+
cert := common.DecodeObjBytes[*certmanagerv1.Certificate](codecs, certmanagerv1.SchemeGroupVersion, assets.MustAsset(certificateAssetName))
142143
cert.SetNamespace(testIstiodNamespace)
143144
labels := make(map[string]string)
144145
maps.Copy(labels, controllerDefaultResourceLabels)
@@ -154,30 +155,30 @@ func testCertificate() *certmanagerv1.Certificate {
154155
}
155156

156157
func testClusterRole() *rbacv1.ClusterRole {
157-
role := decodeClusterRoleObjBytes(assets.MustAsset(clusterRoleAssetName))
158+
role := common.DecodeObjBytes[*rbacv1.ClusterRole](codecs, rbacv1.SchemeGroupVersion, assets.MustAsset(clusterRoleAssetName))
158159
role.SetName("cert-manager-istio-csr-sdghj")
159160
role.SetLabels(controllerDefaultResourceLabels)
160161
return role
161162
}
162163

163164
func testClusterRoleBinding() *rbacv1.ClusterRoleBinding {
164-
roleBinding := decodeClusterRoleBindingObjBytes(assets.MustAsset(clusterRoleBindingAssetName))
165+
roleBinding := common.DecodeObjBytes[*rbacv1.ClusterRoleBinding](codecs, rbacv1.SchemeGroupVersion, assets.MustAsset(clusterRoleBindingAssetName))
165166
roleBinding.SetName("cert-manager-istio-csr-dfkhk")
166167
roleBinding.SetGenerateName("cert-manager-istio-csr-")
167168
roleBinding.SetLabels(controllerDefaultResourceLabels)
168169
return roleBinding
169170
}
170171

171172
func testClusterRoleBindingExtra() *rbacv1.ClusterRoleBinding {
172-
roleBinding := decodeClusterRoleBindingObjBytes(assets.MustAsset(clusterRoleBindingAssetName))
173+
roleBinding := common.DecodeObjBytes[*rbacv1.ClusterRoleBinding](codecs, rbacv1.SchemeGroupVersion, assets.MustAsset(clusterRoleBindingAssetName))
173174
roleBinding.SetName("cert-manager-istio-csr-dfmfj")
174175
roleBinding.SetGenerateName("cert-manager-istio-csr-")
175176
roleBinding.SetLabels(controllerDefaultResourceLabels)
176177
return roleBinding
177178
}
178179

179180
func testDeployment() *appsv1.Deployment {
180-
deployment := decodeDeploymentObjBytes(assets.MustAsset(deploymentAssetName))
181+
deployment := common.DecodeObjBytes[*appsv1.Deployment](codecs, appsv1.SchemeGroupVersion, assets.MustAsset(deploymentAssetName))
181182
deployment.SetNamespace(testIstioCSRNamespace)
182183
deployment.SetLabels(controllerDefaultResourceLabels)
183184
deployment.Spec.Template.Labels = controllerDefaultResourceLabels
@@ -186,42 +187,42 @@ func testDeployment() *appsv1.Deployment {
186187
}
187188

188189
func testRole() *rbacv1.Role {
189-
role := decodeRoleObjBytes(assets.MustAsset(roleAssetName))
190+
role := common.DecodeObjBytes[*rbacv1.Role](codecs, rbacv1.SchemeGroupVersion, assets.MustAsset(roleAssetName))
190191
role.SetNamespace(testIstiodNamespace)
191192
role.SetLabels(controllerDefaultResourceLabels)
192193
return role
193194
}
194195

195196
func testRoleBinding() *rbacv1.RoleBinding {
196-
roleBinding := decodeRoleBindingObjBytes(assets.MustAsset(roleBindingAssetName))
197+
roleBinding := common.DecodeObjBytes[*rbacv1.RoleBinding](codecs, rbacv1.SchemeGroupVersion, assets.MustAsset(roleBindingAssetName))
197198
roleBinding.SetNamespace(testIstiodNamespace)
198199
roleBinding.SetLabels(controllerDefaultResourceLabels)
199200
return roleBinding
200201
}
201202

202203
func testRoleLeases() *rbacv1.Role {
203-
role := decodeRoleObjBytes(assets.MustAsset(roleLeasesAssetName))
204+
role := common.DecodeObjBytes[*rbacv1.Role](codecs, rbacv1.SchemeGroupVersion, assets.MustAsset(roleLeasesAssetName))
204205
role.SetNamespace(testIstiodNamespace)
205206
role.SetLabels(controllerDefaultResourceLabels)
206207
return role
207208
}
208209

209210
func testRoleBindingLeases() *rbacv1.RoleBinding {
210-
roleBinding := decodeRoleBindingObjBytes(assets.MustAsset(roleBindingLeasesAssetName))
211+
roleBinding := common.DecodeObjBytes[*rbacv1.RoleBinding](codecs, rbacv1.SchemeGroupVersion, assets.MustAsset(roleBindingLeasesAssetName))
211212
roleBinding.SetNamespace(testIstiodNamespace)
212213
roleBinding.SetLabels(controllerDefaultResourceLabels)
213214
return roleBinding
214215
}
215216

216217
func testService() *corev1.Service {
217-
service := decodeServiceObjBytes(assets.MustAsset(serviceAssetName))
218+
service := common.DecodeObjBytes[*corev1.Service](codecs, corev1.SchemeGroupVersion, assets.MustAsset(serviceAssetName))
218219
service.SetNamespace(testIstioCSRNamespace)
219220
service.SetLabels(controllerDefaultResourceLabels)
220221
return service
221222
}
222223

223224
func testServiceAccount() *corev1.ServiceAccount {
224-
serviceAccount := decodeServiceAccountObjBytes(assets.MustAsset(serviceAccountAssetName))
225+
serviceAccount := common.DecodeObjBytes[*corev1.ServiceAccount](codecs, corev1.SchemeGroupVersion, assets.MustAsset(serviceAccountAssetName))
225226
serviceAccount.SetNamespace(testIstioCSRNamespace)
226227
serviceAccount.SetLabels(controllerDefaultResourceLabels)
227228
return serviceAccount

pkg/controller/istiocsr/utils.go

Lines changed: 6 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,13 @@ import (
2323

2424
"github.com/openshift/cert-manager-operator/api/operator/v1alpha1"
2525
"github.com/openshift/cert-manager-operator/pkg/controller/common"
26+
"github.com/openshift/cert-manager-operator/pkg/operator/assets"
2627
)
2728

2829
var (
29-
scheme = runtime.NewScheme()
30-
codecs = serializer.NewCodecFactory(scheme)
30+
scheme = runtime.NewScheme()
31+
codecs = serializer.NewCodecFactory(scheme)
32+
cachedServiceAccountName string
3133
)
3234

3335
func init() {
@@ -46,6 +48,8 @@ func init() {
4648
if err := certmanagerv1.AddToScheme(scheme); err != nil {
4749
panic(err)
4850
}
51+
52+
cachedServiceAccountName = common.DecodeObjBytes[*corev1.ServiceAccount](codecs, corev1.SchemeGroupVersion, assets.MustAsset(serviceAccountAssetName)).GetName()
4953
}
5054

5155
// updateStatus is for updating the status subresource of istiocsr.openshift.operator.io.
@@ -134,102 +138,6 @@ func updateResourceLabelsWithIstioMapperLabels(obj client.Object, istiocsrNamesp
134138
obj.SetLabels(l)
135139
}
136140

137-
func decodeDeploymentObjBytes(objBytes []byte) *appsv1.Deployment {
138-
obj, err := runtime.Decode(codecs.UniversalDecoder(appsv1.SchemeGroupVersion), objBytes)
139-
if err != nil {
140-
panic(err)
141-
}
142-
deployment, ok := obj.(*appsv1.Deployment)
143-
if !ok {
144-
panic("failed to convert to *appsv1.Deployment")
145-
}
146-
return deployment
147-
}
148-
149-
func decodeClusterRoleObjBytes(objBytes []byte) *rbacv1.ClusterRole {
150-
obj, err := runtime.Decode(codecs.UniversalDecoder(rbacv1.SchemeGroupVersion), objBytes)
151-
if err != nil {
152-
panic(err)
153-
}
154-
clusterRole, ok := obj.(*rbacv1.ClusterRole)
155-
if !ok {
156-
panic("failed to convert to *rbacv1.ClusterRole")
157-
}
158-
return clusterRole
159-
}
160-
161-
func decodeClusterRoleBindingObjBytes(objBytes []byte) *rbacv1.ClusterRoleBinding {
162-
obj, err := runtime.Decode(codecs.UniversalDecoder(rbacv1.SchemeGroupVersion), objBytes)
163-
if err != nil {
164-
panic(err)
165-
}
166-
clusterRoleBinding, ok := obj.(*rbacv1.ClusterRoleBinding)
167-
if !ok {
168-
panic("failed to convert to *rbacv1.ClusterRoleBinding")
169-
}
170-
return clusterRoleBinding
171-
}
172-
173-
func decodeRoleObjBytes(objBytes []byte) *rbacv1.Role {
174-
obj, err := runtime.Decode(codecs.UniversalDecoder(rbacv1.SchemeGroupVersion), objBytes)
175-
if err != nil {
176-
panic(err)
177-
}
178-
role, ok := obj.(*rbacv1.Role)
179-
if !ok {
180-
panic("failed to convert to *rbacv1.Role")
181-
}
182-
return role
183-
}
184-
185-
func decodeRoleBindingObjBytes(objBytes []byte) *rbacv1.RoleBinding {
186-
obj, err := runtime.Decode(codecs.UniversalDecoder(rbacv1.SchemeGroupVersion), objBytes)
187-
if err != nil {
188-
panic(err)
189-
}
190-
roleBinding, ok := obj.(*rbacv1.RoleBinding)
191-
if !ok {
192-
panic("failed to convert to *rbacv1.RoleBinding")
193-
}
194-
return roleBinding
195-
}
196-
197-
func decodeServiceObjBytes(objBytes []byte) *corev1.Service {
198-
obj, err := runtime.Decode(codecs.UniversalDecoder(corev1.SchemeGroupVersion), objBytes)
199-
if err != nil {
200-
panic(err)
201-
}
202-
service, ok := obj.(*corev1.Service)
203-
if !ok {
204-
panic("failed to convert to *corev1.Service")
205-
}
206-
return service
207-
}
208-
209-
func decodeServiceAccountObjBytes(objBytes []byte) *corev1.ServiceAccount {
210-
obj, err := runtime.Decode(codecs.UniversalDecoder(corev1.SchemeGroupVersion), objBytes)
211-
if err != nil {
212-
panic(err)
213-
}
214-
serviceAccount, ok := obj.(*corev1.ServiceAccount)
215-
if !ok {
216-
panic("failed to convert to *corev1.ServiceAccount")
217-
}
218-
return serviceAccount
219-
}
220-
221-
func decodeCertificateObjBytes(objBytes []byte) *certmanagerv1.Certificate {
222-
obj, err := runtime.Decode(codecs.UniversalDecoder(certmanagerv1.SchemeGroupVersion), objBytes)
223-
if err != nil {
224-
panic(err)
225-
}
226-
certificate, ok := obj.(*certmanagerv1.Certificate)
227-
if !ok {
228-
panic("failed to convert to *certmanagerv1.Certificate")
229-
}
230-
return certificate
231-
}
232-
233141
func hasObjectChanged(desired, fetched client.Object) bool {
234142
if reflect.TypeOf(desired) != reflect.TypeOf(fetched) {
235143
panic("both objects to be compared must be of same type")

0 commit comments

Comments
 (0)