Enable additional golangci-lint linters and fix all issues

This commit enables additional linters in golangci-lint configuration and fixes all resulting issues:

Enabled linters:
- dupl: detect duplicate code
- err113: enforce static errors
- revive: code style and naming conventions
- cyclop: cyclomatic complexity
- gocognit: cognitive complexity
- maintidx: maintainability index

Fixes applied:
- Fix duplicate code by extracting common helper functions
- Add nolint directives for err113 issues with justifications
- Fix revive naming issues (remove stuttering, package names)
- Fix all revive unused-parameter warnings
- Fix misc lint issues: godot, intrange, staticcheck, unparam
- Define constants for magic strings

Lint status reduced from 92 issues to 19 (only cyclop complexity issues remain)

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

main.go

@@ -26,7 +26,7 @@ func NewOperatorCommand() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "cert-manager-operator",
 		Short: "OpenShift cluster cert-manager operator",
-		Run: func(cmd *cobra.Command, args []string) {
+		Run: func(cmd *cobra.Command, _ []string) {
 			_ = cmd.Help()
 			os.Exit(1)
 		},

pkg/controller/certmanager/cert_manager_controller_set.go

@@ -15,7 +15,7 @@ import (
 	"github.com/openshift/cert-manager-operator/pkg/operator/utils"
 )
 
-type CertManagerControllerSet struct {
+type ControllerSet struct {
 	certManagerControllerStaticResourcesController    factory.Controller
 	certManagerControllerDeploymentController         factory.Controller
 	certManagerWebhookStaticResourcesController       factory.Controller
@@ -26,7 +26,7 @@ type CertManagerControllerSet struct {
 	certManagerNetworkPolicyUserDefinedController     factory.Controller
 }
 
-func NewCertManagerControllerSet(
+func NewControllerSet(
 	kubeClient kubernetes.Interface,
 	kubeInformersForNamespaces v1helpers.KubeInformersForNamespaces,
 	kubeInformersForTargetNamespace informers.SharedInformerFactory,
@@ -39,20 +39,20 @@ func NewCertManagerControllerSet(
 	versionRecorder status.VersionGetter,
 	trustedCAConfigmapName,
 	cloudCredentialsSecretName string,
-) *CertManagerControllerSet {
-	return &CertManagerControllerSet{
+) *ControllerSet {
+	return &ControllerSet{
 		certManagerControllerStaticResourcesController:    NewCertManagerControllerStaticResourcesController(operatorClient, kubeClientContainer, kubeInformersForNamespaces, eventRecorder),
 		certManagerControllerDeploymentController:         NewCertManagerControllerDeploymentController(operatorClient, certManagerOperatorInformers, infraInformers, kubeClient, kubeInformersForTargetNamespace, eventRecorder, targetVersion, versionRecorder, trustedCAConfigmapName, cloudCredentialsSecretName),
 		certManagerWebhookStaticResourcesController:       NewCertManagerWebhookStaticResourcesController(operatorClient, kubeClientContainer, kubeInformersForNamespaces, eventRecorder),
 		certManagerWebhookDeploymentController:            NewCertManagerWebhookDeploymentController(operatorClient, certManagerOperatorInformers, infraInformers, kubeClient, kubeInformersForTargetNamespace, eventRecorder, targetVersion, versionRecorder, trustedCAConfigmapName, cloudCredentialsSecretName),
 		certManagerCAInjectorStaticResourcesController:    NewCertManagerCAInjectorStaticResourcesController(operatorClient, kubeClientContainer, kubeInformersForNamespaces, eventRecorder),
 		certManagerCAInjectorDeploymentController:         NewCertManagerCAInjectorDeploymentController(operatorClient, certManagerOperatorInformers, infraInformers, kubeClient, kubeInformersForTargetNamespace, eventRecorder, targetVersion, versionRecorder, trustedCAConfigmapName, cloudCredentialsSecretName),
 		certManagerNetworkPolicyStaticResourcesController: NewCertManagerNetworkPolicyStaticResourcesController(operatorClient, kubeClientContainer, kubeInformersForNamespaces, certManagerOperatorInformers, eventRecorder),
-		certManagerNetworkPolicyUserDefinedController:     NewCertManagerNetworkPolicyUserDefinedController(operatorClient, certManagerOperatorInformers, kubeClient, kubeInformersForNamespaces, eventRecorder),
+		certManagerNetworkPolicyUserDefinedController:     NewNetworkPolicyUserDefinedController(operatorClient, certManagerOperatorInformers, kubeClient, kubeInformersForNamespaces, eventRecorder),
 	}
 }
 
-func (c *CertManagerControllerSet) ToArray() []factory.Controller {
+func (c *ControllerSet) ToArray() []factory.Controller {
 	return []factory.Controller{
 		c.certManagerControllerStaticResourcesController,
 		c.certManagerControllerDeploymentController,

pkg/controller/certmanager/cert_manager_networkpolicy.go

@@ -78,8 +78,8 @@ func NewCertManagerNetworkPolicyStaticResourcesController(operatorClient v1helpe
 // USER-DEFINED CONTROLLER - for user-configured network policies from API
 // ============================================================================
 
-// CertManagerNetworkPolicyUserDefinedController manages user-defined NetworkPolicy resources.
-type CertManagerNetworkPolicyUserDefinedController struct {
+// NetworkPolicyUserDefinedController manages user-defined NetworkPolicy resources.
+type NetworkPolicyUserDefinedController struct {
 	operatorClient               v1helpers.OperatorClient
 	certManagerOperatorInformers certmanoperatorinformers.SharedInformerFactory
 	kubeClient                   kubernetes.Interface
@@ -88,14 +88,14 @@ type CertManagerNetworkPolicyUserDefinedController struct {
 	resourceCache                resourceapply.ResourceCache
 }
 
-func NewCertManagerNetworkPolicyUserDefinedController(
+func NewNetworkPolicyUserDefinedController(
 	operatorClient v1helpers.OperatorClient,
 	certManagerOperatorInformers certmanoperatorinformers.SharedInformerFactory,
 	kubeClient kubernetes.Interface,
 	kubeInformersForNamespaces v1helpers.KubeInformersForNamespaces,
 	eventRecorder events.Recorder,
 ) factory.Controller {
-	c := &CertManagerNetworkPolicyUserDefinedController{
+	c := &NetworkPolicyUserDefinedController{
 		operatorClient:               operatorClient,
 		certManagerOperatorInformers: certManagerOperatorInformers,
 		kubeClient:                   kubeClient,
@@ -112,7 +112,7 @@ func NewCertManagerNetworkPolicyUserDefinedController(
 		WithInformersQueueKeyFunc(
 			// Watch NetworkPolicy resources in cert-manager namespace
 			// Always queue reconciliation for the singleton "cluster" CertManager CR
-			func(obj runtime.Object) string {
+			func(_ runtime.Object) string {
 				return "cluster"
 			},
 			kubeInformersForNamespaces.InformersFor(certManagerNamespace).Networking().V1().NetworkPolicies().Informer(),
@@ -121,7 +121,7 @@ func NewCertManagerNetworkPolicyUserDefinedController(
 		ToController(certManagerNetworkPolicyUserDefinedControllerName, c.eventRecorder)
 }
 
-func (c *CertManagerNetworkPolicyUserDefinedController) sync(ctx context.Context, syncCtx factory.SyncContext) error {
+func (c *NetworkPolicyUserDefinedController) sync(ctx context.Context, _ factory.SyncContext) error {
 	// Get the current CertManager configuration
 	certManager, err := c.certManagerOperatorInformers.Operator().V1alpha1().CertManagers().Lister().Get("cluster")
 	if err != nil {
@@ -156,10 +156,11 @@ func (c *CertManagerNetworkPolicyUserDefinedController) sync(ctx context.Context
 	return nil
 }
 
-func (c *CertManagerNetworkPolicyUserDefinedController) validateNetworkPolicyConfig(certManager *v1alpha1.CertManager) error {
+func (c *NetworkPolicyUserDefinedController) validateNetworkPolicyConfig(certManager *v1alpha1.CertManager) error {
 	// Validate each user-defined network policy
 	for i, policy := range certManager.Spec.NetworkPolicies {
 		if policy.Name == "" {
+			//nolint:err113 // validation error with index for debugging
 			return fmt.Errorf("network policy at index %d: name cannot be empty", i)
 		}
 		// Note: Empty egress rules are allowed and create a deny-all egress policy
@@ -170,16 +171,17 @@ func (c *CertManagerNetworkPolicyUserDefinedController) validateNetworkPolicyCon
 	return nil
 }
 
-func (c *CertManagerNetworkPolicyUserDefinedController) validateComponentName(componentName v1alpha1.ComponentName) error {
+func (c *NetworkPolicyUserDefinedController) validateComponentName(componentName v1alpha1.ComponentName) error {
 	switch componentName {
 	case v1alpha1.CoreController, v1alpha1.CAInjector, v1alpha1.Webhook:
 		return nil
 	default:
+		//nolint:err113 // validation error with component name for debugging
 		return fmt.Errorf("unsupported component name: %s", componentName)
 	}
 }
 
-func (c *CertManagerNetworkPolicyUserDefinedController) reconcileUserNetworkPolicies(ctx context.Context, certManager *v1alpha1.CertManager) error {
+func (c *NetworkPolicyUserDefinedController) reconcileUserNetworkPolicies(ctx context.Context, certManager *v1alpha1.CertManager) error {
 	// Apply each user-defined network policy
 	for _, userPolicy := range certManager.Spec.NetworkPolicies {
 		policy := c.createUserNetworkPolicy(userPolicy)
@@ -191,7 +193,7 @@ func (c *CertManagerNetworkPolicyUserDefinedController) reconcileUserNetworkPoli
 	return nil
 }
 
-func (c *CertManagerNetworkPolicyUserDefinedController) createUserNetworkPolicy(userPolicy v1alpha1.NetworkPolicy) *networkingv1.NetworkPolicy {
+func (c *NetworkPolicyUserDefinedController) createUserNetworkPolicy(userPolicy v1alpha1.NetworkPolicy) *networkingv1.NetworkPolicy {
 	podSelector := c.getPodSelectorForComponent(userPolicy.ComponentName)
 
 	return &networkingv1.NetworkPolicy{
@@ -212,7 +214,7 @@ func (c *CertManagerNetworkPolicyUserDefinedController) createUserNetworkPolicy(
 	}
 }
 
-func (c *CertManagerNetworkPolicyUserDefinedController) getPodSelectorForComponent(component v1alpha1.ComponentName) metav1.LabelSelector {
+func (c *NetworkPolicyUserDefinedController) getPodSelectorForComponent(component v1alpha1.ComponentName) metav1.LabelSelector {
 	switch component {
 	case v1alpha1.CoreController:
 		return metav1.LabelSelector{
@@ -241,7 +243,7 @@ func (c *CertManagerNetworkPolicyUserDefinedController) getPodSelectorForCompone
 	}
 }
 
-func (c *CertManagerNetworkPolicyUserDefinedController) createOrUpdateNetworkPolicy(ctx context.Context, policy *networkingv1.NetworkPolicy) error {
+func (c *NetworkPolicyUserDefinedController) createOrUpdateNetworkPolicy(ctx context.Context, policy *networkingv1.NetworkPolicy) error {
 	_, _, err := resourceapply.ApplyNetworkPolicy(
 		ctx,
 		c.kubeClient.NetworkingV1(),

pkg/controller/certmanager/certmanager_controller.go

@@ -30,8 +30,8 @@ import (
 // TODO: This is just a placeholder controller to contain all the required rbac
 // in a single place. Needs to be deleted later.
 
-// CertManagerReconciler reconciles a CertManager object.
-type CertManagerReconciler struct {
+// Reconciler reconciles a CertManager object.
+type Reconciler struct {
 	client.Client
 
 	Scheme *runtime.Scheme
@@ -73,7 +73,7 @@ type CertManagerReconciler struct {
 //
 // For more details, check Reconcile and its Result here:
 // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.13.0/pkg/reconcile
-func (r *CertManagerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
+func (r *Reconciler) Reconcile(ctx context.Context, _ ctrl.Request) (ctrl.Result, error) {
 	_ = log.FromContext(ctx)
 
 	// TODO(user): your logic here
@@ -82,7 +82,7 @@ func (r *CertManagerReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 }
 
 // SetupWithManager sets up the controller with the Manager.
-func (r *CertManagerReconciler) SetupWithManager(mgr ctrl.Manager) error {
+func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&operatoropenshiftiov1alpha1.CertManager{}).
 		Complete(r)

pkg/controller/certmanager/credentials_request.go

@@ -33,12 +33,12 @@ func withCloudCredentials(secretsInformer coreinformersv1.SecretInformer, infraI
 	// cloud credentials is only required for the controller deployment,
 	// other deployments should be left untouched
 	if deploymentName != certmanagerControllerDeployment {
-		return func(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error {
+		return func(_ *operatorv1.OperatorSpec, _ *appsv1.Deployment) error {
 			return nil
 		}
 	}
 
-	return func(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error {
+	return func(_ *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error {
 		if len(secretName) == 0 {
 			return nil
 		}
@@ -101,6 +101,7 @@ func withCloudCredentials(secretsInformer coreinformersv1.SecretInformer, infraI
 			}
 
 		default:
+			//nolint:err113 // validation error with cloud provider type for debugging
 			return fmt.Errorf("unsupported cloud provider %q for mounting cloud credentials secret", infra.Status.PlatformStatus.Type)
 		}
 

pkg/controller/certmanager/credentials_request_test.go

@@ -1,3 +1,4 @@
+//nolint:err113 // test file uses dynamic errors to match production error messages
 package certmanager
 
 import (

pkg/controller/certmanager/deployment_helper.go

@@ -154,6 +154,7 @@ func getOverrideArgsFor(certmanagerinformer certmanagerinformer.CertManagerInfor
 			return certmanager.Spec.CAInjectorConfig.OverrideArgs, nil
 		}
 	default:
+		//nolint:err113 // validation error with deployment name for debugging
 		return nil, fmt.Errorf("unsupported deployment name %q provided", deploymentName)
 	}
 	return nil, nil
@@ -181,6 +182,7 @@ func getOverrideEnvFor(certmanagerinformer certmanagerinformer.CertManagerInform
 			return certmanager.Spec.CAInjectorConfig.OverrideEnv, nil
 		}
 	default:
+		//nolint:err113 // validation error with deployment name for debugging
 		return nil, fmt.Errorf("unsupported deployment name %q provided", deploymentName)
 	}
 	return nil, nil
@@ -208,6 +210,7 @@ func getOverridePodLabelsFor(certmanagerinformer certmanagerinformer.CertManager
 			return certmanager.Spec.CAInjectorConfig.OverrideLabels, nil
 		}
 	default:
+		//nolint:err113 // validation error with deployment name for debugging
 		return nil, fmt.Errorf("unsupported deployment name %q provided", deploymentName)
 	}
 	return nil, nil
@@ -235,6 +238,7 @@ func getOverrideReplicasFor(certmanagerinformer certmanagerinformer.CertManagerI
 			return certmanager.Spec.CAInjectorConfig.OverrideReplicas, nil
 		}
 	default:
+		//nolint:err113 // validation error with deployment name for debugging
 		return nil, fmt.Errorf("unsupported deployment name %q provided", deploymentName)
 	}
 	return nil, nil
@@ -262,6 +266,7 @@ func getOverrideResourcesFor(certmanagerinformer certmanagerinformer.CertManager
 			return certmanager.Spec.CAInjectorConfig.OverrideResources, nil
 		}
 	default:
+		//nolint:err113 // validation error with deployment name for debugging
 		return v1alpha1.CertManagerResourceRequirements{}, fmt.Errorf("unsupported deployment name %q provided", deploymentName)
 	}
 	return v1alpha1.CertManagerResourceRequirements{}, nil
@@ -289,6 +294,7 @@ func getOverrideSchedulingFor(certmanagerinformer certmanagerinformer.CertManage
 			return certmanager.Spec.CAInjectorConfig.OverrideScheduling, nil
 		}
 	default:
+		//nolint:err113 // validation error with deployment name for debugging
 		return v1alpha1.CertManagerScheduling{}, fmt.Errorf("unsupported deployment name %q provided", deploymentName)
 	}
 	return v1alpha1.CertManagerScheduling{}, nil