Enable additional golangci-lint linters and fix duplicate code

- Enabled linters: dupl, err113, gocognit, maintidx, revive, gocyclo, cyclop
- Fixed all duplicate code (dupl) issues by extracting common patterns:
  - Added reconcileNamespacedRBACObject helper in istiocsr/rbacs.go
  - Added reconcileResourceWithSSA helper in trustmanager/utils.go
  - Reduced code duplication across RBAC and resource reconciliation
- Removed unused imports across multiple files
- All dupl lint errors resolved (was 10, now 0)

Remaining work:
- 19 cyclomatic complexity (cyclop) errors to fix
- ~80 unused parameter (revive) warnings
- Few staticcheck and unparam issues

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Author: Anand Kumar

.golangci.yaml

@@ -53,21 +53,19 @@ linters:
     - unconvert
     - unparam
     - unused
+    - dupl
     - wastedassign
     - whitespace
     - wrapcheck
+    - err113
+    - gocognit
+    - maintidx
+    - revive
+    - gocyclo
+    - cyclop
   disable:
     # depguard has configuration issues in golangci-lint v2 - see https://github.com/golangci/golangci-lint/issues/3906
     - depguard
-    # Below linters will be enabled once the issues reported by enabled linters are fixed.
-    - dupl
-    - cyclop
-    - gocyclo
-    - gocognit
-    - err113
-    - revive
-    - maintidx
-    - mnd
     # Below linters will be enabled when the golangci-lint is upgraded to the version supporting these.
     #- embeddedstructfieldcheck
     #- godoclint
@@ -101,7 +99,10 @@ linters:
       # Exclude some linters from running on tests files.
       - path: _test\.go
         linters:
+          - gocognit
           - gocyclo
+          - cyclop
+          - maintidx
           - errcheck
           - dupl
           - gosec

pkg/controller/istiocsr/rbacs.go

@@ -250,38 +250,7 @@ func (r *Reconciler) updateClusterRoleBindingNameInStatus(istiocsr *v1alpha1.Ist
 
 func (r *Reconciler) createOrApplyRoles(istiocsr *v1alpha1.IstioCSR, resourceLabels map[string]string, istioCSRCreateRecon bool) error {
 	desired := r.getRoleObject(istiocsr.GetNamespace(), istiocsr.Spec.IstioCSRConfig.Istio.Namespace, resourceLabels)
-
-	roleName := fmt.Sprintf("%s/%s", desired.GetNamespace(), desired.GetName())
-	r.log.V(4).Info("reconciling role resource", "name", roleName)
-	fetched := &rbacv1.Role{}
-	exist, err := r.Exists(r.ctx, client.ObjectKeyFromObject(desired), fetched)
-	if err != nil {
-		return common.FromClientError(err, "failed to check %s role resource already exists", roleName)
-	}
-
-	if exist {
-		if istioCSRCreateRecon {
-			r.eventRecorder.Eventf(istiocsr, corev1.EventTypeWarning, "ResourceAlreadyExists", "%s role resource already exists, maybe from previous installation", roleName)
-		}
-		if hasObjectChanged(desired, fetched) {
-			r.log.V(1).Info("role has been modified, updating to desired state", "name", roleName)
-			if err := r.UpdateWithRetry(r.ctx, desired); err != nil {
-				return common.FromClientError(err, "failed to update %s role resource", roleName)
-			}
-			r.eventRecorder.Eventf(istiocsr, corev1.EventTypeNormal, "Reconciled", "role resource %s reconciled back to desired state", roleName)
-		} else {
-			r.log.V(4).Info("role resource already exists and is in expected state", "name", roleName)
-		}
-	}
-
-	if !exist {
-		if err := r.Create(r.ctx, desired); err != nil {
-			return common.FromClientError(err, "failed to create %s role resource", roleName)
-		}
-		r.eventRecorder.Eventf(istiocsr, corev1.EventTypeNormal, "Reconciled", "role resource %s created", roleName)
-	}
-
-	return nil
+	return r.reconcileNamespacedRBACObject(istiocsr, desired, &rbacv1.Role{}, "reconciling role resource", "role resource", istioCSRCreateRecon)
 }
 
 func (r *Reconciler) getRoleObject(istiocsrNamespace, roleNamespace string, resourceLabels map[string]string) *rbacv1.Role {
@@ -293,38 +262,7 @@ func (r *Reconciler) getRoleObject(istiocsrNamespace, roleNamespace string, reso
 
 func (r *Reconciler) createOrApplyRoleBindings(istiocsr *v1alpha1.IstioCSR, serviceAccount string, resourceLabels map[string]string, istioCSRCreateRecon bool) error {
 	desired := r.getRoleBindingObject(serviceAccount, istiocsr.GetNamespace(), istiocsr.Spec.IstioCSRConfig.Istio.Namespace, resourceLabels)
-
-	roleBindingName := fmt.Sprintf("%s/%s", desired.GetNamespace(), desired.GetName())
-	r.log.V(4).Info("reconciling rolebinding resource", "name", roleBindingName)
-	fetched := &rbacv1.RoleBinding{}
-	exist, err := r.Exists(r.ctx, client.ObjectKeyFromObject(desired), fetched)
-	if err != nil {
-		return common.FromClientError(err, "failed to check %s rolebinding resource already exists", roleBindingName)
-	}
-
-	if exist {
-		if istioCSRCreateRecon {
-			r.eventRecorder.Eventf(istiocsr, corev1.EventTypeWarning, "ResourceAlreadyExists", "%s rolebinding resource already exists, maybe from previous installation", roleBindingName)
-		}
-		if hasObjectChanged(desired, fetched) {
-			r.log.V(1).Info("rolebinding has been modified, updating to desired state", "name", roleBindingName)
-			if err := r.UpdateWithRetry(r.ctx, desired); err != nil {
-				return common.FromClientError(err, "failed to update %s rolebinding resource", roleBindingName)
-			}
-			r.eventRecorder.Eventf(istiocsr, corev1.EventTypeNormal, "Reconciled", "rolebinding resource %s reconciled back to desired state", roleBindingName)
-		} else {
-			r.log.V(4).Info("rolebinding resource already exists and is in expected state", "name", roleBindingName)
-		}
-	}
-
-	if !exist {
-		if err := r.Create(r.ctx, desired); err != nil {
-			return common.FromClientError(err, "failed to create %s rolebinding resource", roleBindingName)
-		}
-		r.eventRecorder.Eventf(istiocsr, corev1.EventTypeNormal, "Reconciled", "rolebinding resource %s created", roleBindingName)
-	}
-
-	return nil
+	return r.reconcileNamespacedRBACObject(istiocsr, desired, &rbacv1.RoleBinding{}, "reconciling rolebinding resource", "rolebinding resource", istioCSRCreateRecon)
 }
 
 func (r *Reconciler) getRoleBindingObject(serviceAccount, istiocsrNamespace, roleNamespace string, resourceLabels map[string]string) *rbacv1.RoleBinding {
@@ -337,38 +275,7 @@ func (r *Reconciler) getRoleBindingObject(serviceAccount, istiocsrNamespace, rol
 
 func (r *Reconciler) createOrApplyRoleForLeases(istiocsr *v1alpha1.IstioCSR, resourceLabels map[string]string, istioCSRCreateRecon bool) error {
 	desired := r.getRoleForLeasesObject(istiocsr.GetNamespace(), istiocsr.Spec.IstioCSRConfig.Istio.Namespace, resourceLabels)
-
-	roleName := fmt.Sprintf("%s/%s", desired.GetNamespace(), desired.GetName())
-	r.log.V(4).Info("reconciling role for lease resource", "name", roleName)
-	fetched := &rbacv1.Role{}
-	exist, err := r.Exists(r.ctx, client.ObjectKeyFromObject(desired), fetched)
-	if err != nil {
-		return common.FromClientError(err, "failed to check %s role resource already exists", roleName)
-	}
-
-	if exist {
-		if istioCSRCreateRecon {
-			r.eventRecorder.Eventf(istiocsr, corev1.EventTypeWarning, "ResourceAlreadyExists", "%s role resource already exists, maybe from previous installation", roleName)
-		}
-		if hasObjectChanged(desired, fetched) {
-			r.log.V(1).Info("role has been modified, updating to desired state", "name", roleName)
-			if err := r.UpdateWithRetry(r.ctx, desired); err != nil {
-				return common.FromClientError(err, "failed to update %s role resource", roleName)
-			}
-			r.eventRecorder.Eventf(istiocsr, corev1.EventTypeNormal, "Reconciled", "role resource %s reconciled back to desired state", roleName)
-		} else {
-			r.log.V(4).Info("role resource already exists and is in expected state", "name", roleName)
-		}
-	}
-
-	if !exist {
-		if err := r.Create(r.ctx, desired); err != nil {
-			return common.FromClientError(err, "failed to create %s role resource", roleName)
-		}
-		r.eventRecorder.Eventf(istiocsr, corev1.EventTypeNormal, "Reconciled", "role resource %s created", roleName)
-	}
-
-	return nil
+	return r.reconcileNamespacedRBACObject(istiocsr, desired, &rbacv1.Role{}, "reconciling role for lease resource", "role for lease resource", istioCSRCreateRecon)
 }
 
 func (r *Reconciler) getRoleForLeasesObject(istiocsrNamespace, roleNamespace string, resourceLabels map[string]string) *rbacv1.Role {
@@ -380,38 +287,7 @@ func (r *Reconciler) getRoleForLeasesObject(istiocsrNamespace, roleNamespace str
 
 func (r *Reconciler) createOrApplyRoleBindingForLeases(istiocsr *v1alpha1.IstioCSR, serviceAccount string, resourceLabels map[string]string, istioCSRCreateRecon bool) error {
 	desired := r.getRoleBindingForLeasesObject(serviceAccount, istiocsr.GetNamespace(), istiocsr.Spec.IstioCSRConfig.Istio.Namespace, resourceLabels)
-
-	roleBindingName := fmt.Sprintf("%s/%s", desired.GetNamespace(), desired.GetName())
-	r.log.V(4).Info("reconciling rolebinding for lease resource", "name", roleBindingName)
-	fetched := &rbacv1.RoleBinding{}
-	exist, err := r.Exists(r.ctx, client.ObjectKeyFromObject(desired), fetched)
-	if err != nil {
-		return common.FromClientError(err, "failed to check %s rolebinding resource already exists", roleBindingName)
-	}
-
-	if exist {
-		if istioCSRCreateRecon {
-			r.eventRecorder.Eventf(istiocsr, corev1.EventTypeWarning, "ResourceAlreadyExists", "%s rolebinding resource already exists, maybe from previous installation", roleBindingName)
-		}
-		if hasObjectChanged(desired, fetched) {
-			r.log.V(1).Info("rolebinding has been modified, updating to desired state", "name", roleBindingName)
-			if err := r.UpdateWithRetry(r.ctx, desired); err != nil {
-				return common.FromClientError(err, "failed to update %s rolebinding resource", roleBindingName)
-			}
-			r.eventRecorder.Eventf(istiocsr, corev1.EventTypeNormal, "Reconciled", "rolebinding resource %s reconciled back to desired state", roleBindingName)
-		} else {
-			r.log.V(4).Info("rolebinding resource already exists and is in expected state", "name", roleBindingName)
-		}
-	}
-
-	if !exist {
-		if err := r.Create(r.ctx, desired); err != nil {
-			return common.FromClientError(err, "failed to create %s rolebinding resource", roleBindingName)
-		}
-		r.eventRecorder.Eventf(istiocsr, corev1.EventTypeNormal, "Reconciled", "rolebinding resource %s created", roleBindingName)
-	}
-
-	return nil
+	return r.reconcileNamespacedRBACObject(istiocsr, desired, &rbacv1.RoleBinding{}, "reconciling rolebinding for lease resource", "rolebinding for lease resource", istioCSRCreateRecon)
 }
 
 func (r *Reconciler) getRoleBindingForLeasesObject(serviceAccount, istiocsrNamespace, roleNamespace string, resourceLabels map[string]string) *rbacv1.RoleBinding {
@@ -437,6 +313,40 @@ func updateServiceAccountNamespaceInRBACBindingObject[Object *rbacv1.RoleBinding
 	}
 }
 
+// reconcileNamespacedRBACObject handles the common create-or-update logic for namespaced RBAC
+// resources (Role and RoleBinding). logMsg is used for the initial reconciliation log; resourceKind
+// is used in error and event messages. fetched must be an empty instance of the same type as desired.
+func (r *Reconciler) reconcileNamespacedRBACObject(istiocsr *v1alpha1.IstioCSR, desired, fetched client.Object, logMsg, resourceKind string, istioCSRCreateRecon bool) error {
+	resourceName := fmt.Sprintf("%s/%s", desired.GetNamespace(), desired.GetName())
+	r.log.V(4).Info(logMsg, "name", resourceName)
+	exist, err := r.Exists(r.ctx, client.ObjectKeyFromObject(desired), fetched)
+	if err != nil {
+		return common.FromClientError(err, "failed to check %s %s already exists", resourceName, resourceKind)
+	}
+
+	if exist && istioCSRCreateRecon {
+		r.eventRecorder.Eventf(istiocsr, corev1.EventTypeWarning, "ResourceAlreadyExists", "%s %s already exists, maybe from previous installation", resourceName, resourceKind)
+	}
+	if exist && hasObjectChanged(desired, fetched) {
+		r.log.V(1).Info(resourceKind+" has been modified, updating to desired state", "name", resourceName)
+		if err := r.UpdateWithRetry(r.ctx, desired); err != nil {
+			return common.FromClientError(err, "failed to update %s %s", resourceName, resourceKind)
+		}
+		r.eventRecorder.Eventf(istiocsr, corev1.EventTypeNormal, "Reconciled", "%s %s reconciled back to desired state", resourceKind, resourceName)
+	} else if exist {
+		r.log.V(4).Info(resourceKind+" already exists and is in expected state", "name", resourceName)
+	}
+
+	if !exist {
+		if err := r.Create(r.ctx, desired); err != nil {
+			return common.FromClientError(err, "failed to create %s %s", resourceName, resourceKind)
+		}
+		r.eventRecorder.Eventf(istiocsr, corev1.EventTypeNormal, "Reconciled", "%s %s created", resourceKind, resourceName)
+	}
+
+	return nil
+}
+
 // handleClusterRoleBindingModification reconciles a drifted ClusterRoleBinding back to its desired state.
 // It copies the live object's name onto the desired spec (which was built with GenerateName for creation)
 // and then attempts an in-place update. Because the Kubernetes API treats RoleRef as immutable, a RoleRef

pkg/controller/trustmanager/certificates.go

@@ -5,9 +5,7 @@ import (
 	"reflect"
 	"slices"
 
-	corev1 "k8s.io/api/core/v1"
 	"k8s.io/utils/ptr"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
 	certmanagermetav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1"
@@ -20,26 +18,10 @@ import (
 // createOrApplyIssuer reconciles the self-signed Issuer used for trust-manager's webhook TLS.
 func (r *Reconciler) createOrApplyIssuer(trustManager *v1alpha1.TrustManager, resourceLabels, resourceAnnotations map[string]string) error {
 	desired := getIssuerObject(resourceLabels, resourceAnnotations)
-	resourceName := fmt.Sprintf("%s/%s", desired.GetNamespace(), desired.GetName())
-	r.log.V(4).Info("reconciling issuer resource", "name", resourceName)
-
 	existing := &certmanagerv1.Issuer{}
-	exists, err := r.Exists(r.ctx, client.ObjectKeyFromObject(desired), existing)
-	if err != nil {
-		return common.FromClientError(err, "failed to check if issuer %q exists", resourceName)
-	}
-	if exists && !issuerModified(desired, existing) {
-		r.log.V(4).Info("issuer resource exists and is in desired state", "name", resourceName)
-		return nil
-	}
-
-	r.log.V(2).Info("issuer resource has been modified, updating to desired state", "name", resourceName)
-	if err := r.Patch(r.ctx, desired, client.Apply, client.FieldOwner(fieldOwner), client.ForceOwnership); err != nil {
-		return common.FromClientError(err, "failed to apply issuer %q", resourceName)
-	}
-
-	r.eventRecorder.Eventf(trustManager, corev1.EventTypeNormal, "Reconciled", "issuer resource %s applied", resourceName)
-	return nil
+	return r.reconcileResourceWithSSA(trustManager, desired, existing, "issuer", func() bool {
+		return issuerModified(desired, existing)
+	})
 }
 
 func getIssuerObject(resourceLabels, resourceAnnotations map[string]string) *certmanagerv1.Issuer {
@@ -54,26 +36,10 @@ func getIssuerObject(resourceLabels, resourceAnnotations map[string]string) *cer
 // createOrApplyCertificate reconciles the Certificate used for trust-manager's webhook TLS.
 func (r *Reconciler) createOrApplyCertificate(trustManager *v1alpha1.TrustManager, resourceLabels, resourceAnnotations map[string]string) error {
 	desired := getCertificateObject(resourceLabels, resourceAnnotations)
-	resourceName := fmt.Sprintf("%s/%s", desired.GetNamespace(), desired.GetName())
-	r.log.V(4).Info("reconciling certificate resource", "name", resourceName)
-
 	existing := &certmanagerv1.Certificate{}
-	exists, err := r.Exists(r.ctx, client.ObjectKeyFromObject(desired), existing)
-	if err != nil {
-		return common.FromClientError(err, "failed to check if certificate %q exists", resourceName)
-	}
-	if exists && !certificateModified(desired, existing) {
-		r.log.V(4).Info("certificate resource exists and is in desired state", "name", resourceName)
-		return nil
-	}
-
-	r.log.V(2).Info("certificate resource has been modified, updating to desired state", "name", resourceName)
-	if err := r.Patch(r.ctx, desired, client.Apply, client.FieldOwner(fieldOwner), client.ForceOwnership); err != nil {
-		return common.FromClientError(err, "failed to apply certificate %q", resourceName)
-	}
-
-	r.eventRecorder.Eventf(trustManager, corev1.EventTypeNormal, "Reconciled", "certificate resource %s applied", resourceName)
-	return nil
+	return r.reconcileResourceWithSSA(trustManager, desired, existing, "certificate", func() bool {
+		return certificateModified(desired, existing)
+	})
 }
 
 func getCertificateObject(resourceLabels, resourceAnnotations map[string]string) *certmanagerv1.Certificate {