From 8bcb9bde6ba59d7f21da3a13d90fc5f45d284f9b Mon Sep 17 00:00:00 2001 From: Anand Kumar Date: Wed, 15 Apr 2026 17:33:32 +0530 Subject: [PATCH] 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) --- .golangci.yaml | 19 +- main.go | 2 +- .../cert_manager_controller_set.go | 12 +- .../certmanager/cert_manager_networkpolicy.go | 26 +-- .../certmanager/certmanager_controller.go | 8 +- .../certmanager/credentials_request.go | 5 +- .../certmanager/credentials_request_test.go | 1 + .../certmanager/deployment_helper.go | 6 + .../certmanager/deployment_overrides.go | 22 +- .../deployment_overrides_validation.go | 20 +- .../generic_deployment_controller.go | 4 +- pkg/controller/common/client.go | 12 +- pkg/controller/common/client_test.go | 1 - pkg/controller/common/constants.go | 1 + pkg/controller/common/errors_test.go | 1 + pkg/controller/common/utils_test.go | 1 + pkg/controller/istiocsr/certificates.go | 1 + pkg/controller/istiocsr/certificates_test.go | 52 ++--- pkg/controller/istiocsr/controller.go | 3 +- pkg/controller/istiocsr/controller_test.go | 96 ++++----- pkg/controller/istiocsr/deployments.go | 12 ++ pkg/controller/istiocsr/deployments_test.go | 190 +++++++++--------- .../istiocsr/install_instiocsr_test.go | 19 +- pkg/controller/istiocsr/networkpolicies.go | 1 + pkg/controller/istiocsr/rbacs.go | 170 ++++------------ pkg/controller/istiocsr/rbacs_test.go | 154 +++++++------- .../istiocsr/serviceaccounts_test.go | 30 +-- pkg/controller/istiocsr/services_test.go | 18 +- pkg/controller/istiocsr/utils.go | 8 + pkg/controller/trustmanager/certificates.go | 46 +---- .../trustmanager/certificates_test.go | 60 +++--- pkg/controller/trustmanager/configmaps.go | 3 +- .../trustmanager/configmaps_test.go | 48 ++--- pkg/controller/trustmanager/controller.go | 4 +- .../trustmanager/controller_test.go | 58 +++--- pkg/controller/trustmanager/deployments.go | 3 + .../trustmanager/deployments_test.go | 30 +-- .../trustmanager/install_trustmanager.go | 1 + .../trustmanager/install_trustmanager_test.go | 4 +- pkg/controller/trustmanager/rbacs.go | 135 ++----------- pkg/controller/trustmanager/rbacs_test.go | 60 +++--- .../trustmanager/serviceaccounts_test.go | 22 +- pkg/controller/trustmanager/services_test.go | 40 ++-- pkg/controller/trustmanager/utils.go | 39 ++++ pkg/controller/trustmanager/utils_test.go | 2 +- pkg/controller/trustmanager/webhooks.go | 24 +-- pkg/controller/trustmanager/webhooks_test.go | 34 ++-- pkg/features/features_test.go | 1 + pkg/operator/operatorclient/operatorclient.go | 5 +- pkg/operator/starter.go | 2 +- pkg/operator/utils/apidiscovery.go | 2 + pkg/operator/utils/apidiscovery_test.go | 3 +- 52 files changed, 694 insertions(+), 827 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index e1999ddc8..1b3f8892c 100644 --- a/.golangci.yaml +++ b/.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 diff --git a/main.go b/main.go index 74b74a740..53cb4b93c 100644 --- a/main.go +++ b/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) }, diff --git a/pkg/controller/certmanager/cert_manager_controller_set.go b/pkg/controller/certmanager/cert_manager_controller_set.go index 08981cb33..f40918106 100644 --- a/pkg/controller/certmanager/cert_manager_controller_set.go +++ b/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,8 +39,8 @@ 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), @@ -48,11 +48,11 @@ func NewCertManagerControllerSet( 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, diff --git a/pkg/controller/certmanager/cert_manager_networkpolicy.go b/pkg/controller/certmanager/cert_manager_networkpolicy.go index 50d8d32b7..52a565789 100644 --- a/pkg/controller/certmanager/cert_manager_networkpolicy.go +++ b/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(), diff --git a/pkg/controller/certmanager/certmanager_controller.go b/pkg/controller/certmanager/certmanager_controller.go index 68b73dd71..688c6dd84 100644 --- a/pkg/controller/certmanager/certmanager_controller.go +++ b/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) diff --git a/pkg/controller/certmanager/credentials_request.go b/pkg/controller/certmanager/credentials_request.go index af5878334..a5ae5b206 100644 --- a/pkg/controller/certmanager/credentials_request.go +++ b/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) } diff --git a/pkg/controller/certmanager/credentials_request_test.go b/pkg/controller/certmanager/credentials_request_test.go index d72192701..47c93568e 100644 --- a/pkg/controller/certmanager/credentials_request_test.go +++ b/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 ( diff --git a/pkg/controller/certmanager/deployment_helper.go b/pkg/controller/certmanager/deployment_helper.go index f26c92ecb..2e3a85de2 100644 --- a/pkg/controller/certmanager/deployment_helper.go +++ b/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 diff --git a/pkg/controller/certmanager/deployment_overrides.go b/pkg/controller/certmanager/deployment_overrides.go index 1e735107d..5506f9117 100644 --- a/pkg/controller/certmanager/deployment_overrides.go +++ b/pkg/controller/certmanager/deployment_overrides.go @@ -74,7 +74,7 @@ type overrideSchedulingFunc func(certmanagerinformer.CertManagerInformer, string // withOperandImageOverrideHook overrides the deployment image with // the operand images provided to the operator. -func withOperandImageOverrideHook(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { +func withOperandImageOverrideHook(_ *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { for index := range deployment.Spec.Template.Spec.Containers { deployment.Spec.Template.Spec.Containers[index].Image = certManagerImage(deployment.Spec.Template.Spec.Containers[index].Image) } @@ -91,7 +91,7 @@ func withOperandImageOverrideHook(operatorSpec *operatorv1.OperatorSpec, deploym // withContainerArgsOverrideHook overrides the container args with those provided by // the overrideArgsFunc function. func withContainerArgsOverrideHook(certmanagerinformer certmanagerinformer.CertManagerInformer, deploymentName string, fn overrideArgsFunc) func(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { - return func(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { + return func(_ *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { overrideArgs, err := fn(certmanagerinformer, deploymentName) if err != nil { return err @@ -109,7 +109,7 @@ func withContainerArgsOverrideHook(certmanagerinformer certmanagerinformer.CertM // withContainerEnvOverrideHook verrides the container env with those provided by // the overrideEnvFunc function. func withContainerEnvOverrideHook(certmanagerinformer certmanagerinformer.CertManagerInformer, deploymentName string, fn overrideEnvFunc) func(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { - return func(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { + return func(_ *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { overrideEnv, err := fn(certmanagerinformer, deploymentName) if err != nil { return err @@ -126,7 +126,7 @@ func withContainerEnvOverrideHook(certmanagerinformer certmanagerinformer.CertMa // withContainerResourcesOverrideHook overrides the container resources with those provided by // the overrideResourcesFunc function. func withContainerResourcesOverrideHook(certmanagerinformer certmanagerinformer.CertManagerInformer, deploymentName string, fn overrideResourcesFunc) func(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { - return func(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { + return func(_ *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { overrideResources, err := fn(certmanagerinformer, deploymentName) if err != nil { return err @@ -144,7 +144,7 @@ func withContainerResourcesOverrideHook(certmanagerinformer certmanagerinformer. // withDeploymentReplicasOverrideHook overrides the deployment replicas with those provided by // the overrideReplicasFunc function. func withDeploymentReplicasOverrideHook(certmanagerinformer certmanagerinformer.CertManagerInformer, deploymentName string, fn overrideReplicasFunc) func(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { - return func(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { + return func(_ *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { overrideReplicas, err := fn(certmanagerinformer, deploymentName) if err != nil { return err @@ -160,7 +160,7 @@ func withDeploymentReplicasOverrideHook(certmanagerinformer certmanagerinformer. // withPodSchedulingOverrideHook overrides the pod scheduling with those provided by // the overrideSchedulingFunc function. func withPodSchedulingOverrideHook(certmanagerinformer certmanagerinformer.CertManagerInformer, deploymentName string, fn overrideSchedulingFunc) func(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { - return func(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { + return func(_ *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { overrideScheduling, err := fn(certmanagerinformer, deploymentName) if err != nil { return err @@ -180,15 +180,15 @@ func withPodSchedulingOverrideHook(certmanagerinformer certmanagerinformer.CertM // withProxyEnv patches the operand deployment if operator // has proxy variables set. Sets HTTPS_PROXY, HTTP_PROXY and NO_PROXY. -func withProxyEnv(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { +func withProxyEnv(_ *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { deployment.Spec.Template.Spec.Containers[0].Env = mergeContainerEnvs(deployment.Spec.Template.Spec.Containers[0].Env, proxy.ReadProxyVarsFromEnv()) return nil } // withCAConfigMap patches the operand deployment to include the custom // ca bundle as a volume. This is set when a trusted ca configmap is provided. -func withCAConfigMap(configmapinformer coreinformersv1.ConfigMapInformer, deployment *appsv1.Deployment, trustedCAConfigmapName string) func(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { - return func(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { +func withCAConfigMap(configmapinformer coreinformersv1.ConfigMapInformer, _ *appsv1.Deployment, trustedCAConfigmapName string) func(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { + return func(_ *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { if len(trustedCAConfigmapName) == 0 { return nil } @@ -225,7 +225,7 @@ func withCAConfigMap(configmapinformer coreinformersv1.ConfigMapInformer, deploy // withPodLabels patches the operand deployment to include custom pod labels. func withPodLabelsOverrideHook(certmanagerinformer certmanagerinformer.CertManagerInformer, deploymentName string, fn overrideLabelsFunc) func(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { - return func(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { + return func(_ *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { overrideLabels, err := fn(certmanagerinformer, deploymentName) if err != nil { return err @@ -239,7 +239,7 @@ func withPodLabelsOverrideHook(certmanagerinformer certmanagerinformer.CertManag } } -func withSABoundToken(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { +func withSABoundToken(_ *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { volume := corev1.Volume{ Name: boundSATokenVolumeName, VolumeSource: corev1.VolumeSource{ diff --git a/pkg/controller/certmanager/deployment_overrides_validation.go b/pkg/controller/certmanager/deployment_overrides_validation.go index 65e323f0d..e8801b13c 100644 --- a/pkg/controller/certmanager/deployment_overrides_validation.go +++ b/pkg/controller/certmanager/deployment_overrides_validation.go @@ -74,13 +74,14 @@ func withContainerArgsValidateHook(certmanagerinformer certmanagerinformer.CertM validateArgs := func(argMap map[string]string, supportedArgs []string) error { for k, v := range argMap { if !slices.Contains(supportedArgs, k) { + //nolint:err113 // validation error with arg key-value for debugging return fmt.Errorf("validation failed due to unsupported arg %q=%q", k, v) } } return nil } - return func(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { + return func(_ *operatorv1.OperatorSpec, _ *appsv1.Deployment) error { certmanager, err := certmanagerinformer.Lister().Get("cluster") if err != nil { return fmt.Errorf("failed to get certmanager %q due to %w", "cluster", err) @@ -104,6 +105,7 @@ func withContainerArgsValidateHook(certmanagerinformer certmanagerinformer.CertM return validateArgs(argMap, supportedCertManageCainjectorArgs) } default: + //nolint:err113 // validation error with deployment name for debugging return fmt.Errorf("unsupported deployment name %q provided", deploymentName) } @@ -123,13 +125,14 @@ func withContainerEnvValidateHook(certmanagerinformer certmanagerinformer.CertMa validateEnv := func(argMap map[string]corev1.EnvVar, supportedEnv []string) error { for k, v := range argMap { if !slices.Contains(supportedEnv, k) { + //nolint:err113 // validation error with env key-value for debugging return fmt.Errorf("validation failed due to unsupported arg %q=%q", k, v) } } return nil } - return func(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { + return func(_ *operatorv1.OperatorSpec, _ *appsv1.Deployment) error { certmanager, err := certmanagerinformer.Lister().Get("cluster") if err != nil { return fmt.Errorf("failed to get certmanager %q due to %w", "cluster", err) @@ -153,6 +156,7 @@ func withContainerEnvValidateHook(certmanagerinformer certmanagerinformer.CertMa return validateEnv(envMap, supportedCertManageCainjectorEnv) } default: + //nolint:err113 // validation error with deployment name for debugging return fmt.Errorf("unsupported deployment name %q provided", deploymentName) } @@ -172,13 +176,14 @@ func withPodLabelsValidateHook(certmanagerinformer certmanagerinformer.CertManag validateLabels := func(labels map[string]string, supportedLabelKeys []string) error { for k, v := range labels { if !slices.Contains(supportedLabelKeys, k) { + //nolint:err113 // validation error with label key-value for debugging return fmt.Errorf("validation failed due to unsupported label %q=%q", k, v) } } return nil } - return func(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { + return func(_ *operatorv1.OperatorSpec, _ *appsv1.Deployment) error { certmanager, err := certmanagerinformer.Lister().Get("cluster") if err != nil { return fmt.Errorf("failed to get certmanager %q due to %w", "cluster", err) @@ -198,6 +203,7 @@ func withPodLabelsValidateHook(certmanagerinformer certmanagerinformer.CertManag return validateLabels(certmanager.Spec.CAInjectorConfig.OverrideLabels, supportedCertManagerCainjectorLabelKeys) } default: + //nolint:err113 // validation error with deployment name for debugging return fmt.Errorf("unsupported deployment name %q provided", deploymentName) } @@ -218,7 +224,7 @@ func withContainerResourcesValidateHook(certmanagerinformer certmanagerinformer. string(corev1.ResourceCPU), string(corev1.ResourceMemory), } - return func(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { + return func(_ *operatorv1.OperatorSpec, _ *appsv1.Deployment) error { certmanager, err := certmanagerinformer.Lister().Get("cluster") if err != nil { return fmt.Errorf("failed to get certmanager %q due to %w", "cluster", err) @@ -238,6 +244,7 @@ func withContainerResourcesValidateHook(certmanagerinformer certmanagerinformer. return validateResources(certmanager.Spec.CAInjectorConfig.OverrideResources, supportedCertManagerCainjectorResourceNames) } default: + //nolint:err113 // validation error with deployment name for debugging return fmt.Errorf("unsupported deployment name %q provided", deploymentName) } @@ -250,11 +257,13 @@ func validateResources(resources v1alpha1.CertManagerResourceRequirements, suppo errs := []error{} for k, v := range resources.Limits { if !slices.Contains(supportedResourceNames, string(k)) { + //nolint:err113 // validation error with resource limit key-value for debugging errs = append(errs, fmt.Errorf("validation failed due to unsupported resource limits %q=%s", k, v.String())) } } for k, v := range resources.Requests { if !slices.Contains(supportedResourceNames, string(k)) { + //nolint:err113 // validation error with resource request key-value for debugging errs = append(errs, fmt.Errorf("validation failed due to unsupported resource requests %q=%s", k, v.String())) } } @@ -263,7 +272,7 @@ func validateResources(resources v1alpha1.CertManagerResourceRequirements, suppo // withPodSchedulingValidateHook validates the overrides scheduling field for each operand. func withPodSchedulingValidateHook(certmanagerinformer certmanagerinformer.CertManagerInformer, deploymentName string) func(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { - return func(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { + return func(_ *operatorv1.OperatorSpec, _ *appsv1.Deployment) error { certmanager, err := certmanagerinformer.Lister().Get("cluster") if err != nil { return fmt.Errorf("failed to get certmanager %q due to %w", "cluster", err) @@ -286,6 +295,7 @@ func withPodSchedulingValidateHook(certmanagerinformer certmanagerinformer.CertM field.NewPath("spec", "cainjectorConfig", "overrideScheduling")) } default: + //nolint:err113 // validation error with deployment name for debugging return fmt.Errorf("unsupported deployment name %q provided", deploymentName) } diff --git a/pkg/controller/certmanager/generic_deployment_controller.go b/pkg/controller/certmanager/generic_deployment_controller.go index c6775cea3..c3bc00d35 100644 --- a/pkg/controller/certmanager/generic_deployment_controller.go +++ b/pkg/controller/certmanager/generic_deployment_controller.go @@ -18,14 +18,14 @@ import ( ) func newGenericDeploymentController( - controllerName, targetVersion, deploymentFile string, + controllerName, _ /* targetVersion */, deploymentFile string, operatorClient v1helpers.OperatorClientWithFinalizers, certManagerOperatorInformers certmanoperatorinformers.SharedInformerFactory, infraInformers utils.OptionalInformer[configinformers.SharedInformerFactory], kubeClient kubernetes.Interface, kubeInformersForTargetNamespace informers.SharedInformerFactory, eventsRecorder events.Recorder, - versionRecorder status.VersionGetter, + _ /* versionRecorder */ status.VersionGetter, trustedCAConfigmapName string, cloudCredentialsSecretName string, ) factory.Controller { diff --git a/pkg/controller/common/client.go b/pkg/controller/common/client.go index f7d025ee2..47fd3ebe0 100644 --- a/pkg/controller/common/client.go +++ b/pkg/controller/common/client.go @@ -2,16 +2,22 @@ package common import ( "context" + "errors" "fmt" "reflect" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/client-go/util/retry" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/manager" ) +var ( + // ErrTypeConversion indicates a type conversion failure. + ErrTypeConversion = errors.New("failed to convert to client.Object") +) + // ctrlClientImpl implements the CtrlClient interface using the manager's client. type ctrlClientImpl struct { client.Client @@ -100,7 +106,7 @@ func (c *ctrlClientImpl) UpdateWithRetry( currentInterface := reflect.New(reflect.TypeOf(obj).Elem()).Interface() current, ok := currentInterface.(client.Object) if !ok { - return fmt.Errorf("failed to convert to client.Object") + return ErrTypeConversion } if err := c.Client.Get(ctx, key, current); err != nil { return fmt.Errorf("failed to fetch latest %q for update: %w", key, err) @@ -139,7 +145,7 @@ func (c *ctrlClientImpl) Patch( func (c *ctrlClientImpl) Exists(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { if err := c.Client.Get(ctx, key, obj); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { return false, nil } return false, err diff --git a/pkg/controller/common/client_test.go b/pkg/controller/common/client_test.go index 585448716..cd8ee4274 100644 --- a/pkg/controller/common/client_test.go +++ b/pkg/controller/common/client_test.go @@ -111,7 +111,6 @@ func TestNewClient(t *testing.T) { got, err := NewClient(mgr) require.NoError(t, err) require.NotNil(t, got) - var _ CtrlClient = got impl, ok := got.(*ctrlClientImpl) require.True(t, ok, "NewClient must return *ctrlClientImpl") assert.True(t, impl.Client == cl, "wrapped client must be the exact manager client instance") diff --git a/pkg/controller/common/constants.go b/pkg/controller/common/constants.go index 45dd74b6d..82b8ee175 100644 --- a/pkg/controller/common/constants.go +++ b/pkg/controller/common/constants.go @@ -1,3 +1,4 @@ +//nolint:revive // common is an established package name in this codebase package common const ( diff --git a/pkg/controller/common/errors_test.go b/pkg/controller/common/errors_test.go index c263d652e..fd8c36c33 100644 --- a/pkg/controller/common/errors_test.go +++ b/pkg/controller/common/errors_test.go @@ -1,3 +1,4 @@ +//nolint:err113 // test file uses dynamic errors for simplicity package common import ( diff --git a/pkg/controller/common/utils_test.go b/pkg/controller/common/utils_test.go index d4117d34d..2e7372caf 100644 --- a/pkg/controller/common/utils_test.go +++ b/pkg/controller/common/utils_test.go @@ -1,3 +1,4 @@ +//nolint:revive // common is an established package name in this codebase package common import ( diff --git a/pkg/controller/istiocsr/certificates.go b/pkg/controller/istiocsr/certificates.go index a77bb3987..4a433db5e 100644 --- a/pkg/controller/istiocsr/certificates.go +++ b/pkg/controller/istiocsr/certificates.go @@ -127,6 +127,7 @@ func updateCertificateParams(istiocsr *v1alpha1.IstioCSR, certificate *certmanag } if (certificate.Spec.PrivateKey.Algorithm == certmanagerv1.RSAKeyAlgorithm && certificate.Spec.PrivateKey.Size < DefaultRSAPrivateKeySize) || (certificate.Spec.PrivateKey.Algorithm == certmanagerv1.ECDSAKeyAlgorithm && certificate.Spec.PrivateKey.Size != DefaultECDSA256PrivateKeySize && certificate.Spec.PrivateKey.Size != DefaultECDSA384PrivateKeySize) { + //nolint:err113 // user-facing validation error message return fmt.Errorf("certificate parameters PrivateKeySize and PrivateKeyAlgorithm do not comply") } diff --git a/pkg/controller/istiocsr/certificates_test.go b/pkg/controller/istiocsr/certificates_test.go index e9bb417eb..ac2be1236 100644 --- a/pkg/controller/istiocsr/certificates_test.go +++ b/pkg/controller/istiocsr/certificates_test.go @@ -22,8 +22,8 @@ func TestCreateOrApplyCertificates(t *testing.T) { }{ { name: "reconciliation of certificate fails while checking if exists", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.IstioCSR: istiocsr := testIstioCSR() @@ -31,7 +31,7 @@ func TestCreateOrApplyCertificates(t *testing.T) { } return nil }) - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch obj.(type) { case *certmanagerv1.Certificate: return false, errTestClient @@ -43,8 +43,8 @@ func TestCreateOrApplyCertificates(t *testing.T) { }, { name: "reconciliation of certificate fails while restoring to expected state", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.IstioCSR: istiocsr := testIstioCSR() @@ -52,7 +52,7 @@ func TestCreateOrApplyCertificates(t *testing.T) { } return nil }) - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *certmanagerv1.Certificate: cert := testCertificate() @@ -61,7 +61,7 @@ func TestCreateOrApplyCertificates(t *testing.T) { } return true, nil }) - m.UpdateWithRetryCalls(func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + m.UpdateWithRetryCalls(func(_ context.Context, obj client.Object, _ ...client.UpdateOption) error { switch obj.(type) { case *certmanagerv1.Certificate: return errTestClient @@ -73,8 +73,8 @@ func TestCreateOrApplyCertificates(t *testing.T) { }, { name: "reconciliation of certificate which already exists in expected state", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.IstioCSR: istiocsr := testIstioCSR() @@ -82,7 +82,7 @@ func TestCreateOrApplyCertificates(t *testing.T) { } return nil }) - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *certmanagerv1.Certificate: cert := testCertificate() @@ -94,8 +94,8 @@ func TestCreateOrApplyCertificates(t *testing.T) { }, { name: "reconciliation of certificate creation fails", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.IstioCSR: istiocsr := testIstioCSR() @@ -103,14 +103,14 @@ func TestCreateOrApplyCertificates(t *testing.T) { } return nil }) - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch obj.(type) { case *certmanagerv1.Certificate: return false, nil } return true, nil }) - m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + m.CreateCalls(func(_ context.Context, obj client.Object, _ ...client.CreateOption) error { switch obj.(type) { case *certmanagerv1.Certificate: return errTestClient @@ -122,8 +122,8 @@ func TestCreateOrApplyCertificates(t *testing.T) { }, { name: "reconciliation of certificate when revisions are configured", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.IstioCSR: istiocsr := testIstioCSR() @@ -136,8 +136,8 @@ func TestCreateOrApplyCertificates(t *testing.T) { }, { name: "reconciliation of certificate when certificate duration not configured", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.IstioCSR: istiocsr := testIstioCSR() @@ -150,8 +150,8 @@ func TestCreateOrApplyCertificates(t *testing.T) { }, { name: "reconciliation of certificate when certificate RenewBefore not configured", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.IstioCSR: istiocsr := testIstioCSR() @@ -164,8 +164,8 @@ func TestCreateOrApplyCertificates(t *testing.T) { }, { name: "reconciliation of certificate when certificate PrivateKeyAlgorithm not configured", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.IstioCSR: istiocsr := testIstioCSR() @@ -178,8 +178,8 @@ func TestCreateOrApplyCertificates(t *testing.T) { }, { name: "reconciliation of certificate when certificate PrivateKeySize not configured", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.IstioCSR: istiocsr := testIstioCSR() @@ -192,8 +192,8 @@ func TestCreateOrApplyCertificates(t *testing.T) { }, { name: "reconciliation of certificate when certificate PrivateKeySize and PrivateKeyAlgorithm is misconfigured", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.IstioCSR: istiocsr := testIstioCSR() diff --git a/pkg/controller/istiocsr/controller.go b/pkg/controller/istiocsr/controller.go index c25a6e985..2ccf669dd 100644 --- a/pkg/controller/istiocsr/controller.go +++ b/pkg/controller/istiocsr/controller.go @@ -67,7 +67,7 @@ func New(mgr ctrl.Manager) (*Reconciler, error) { // SetupWithManager sets up the controller with the Manager. func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { - mapFunc := func(ctx context.Context, obj client.Object) []reconcile.Request { + mapFunc := func(_ context.Context, obj client.Object) []reconcile.Request { r.log.V(4).Info("received reconcile event", "object", fmt.Sprintf("%T", obj), "name", obj.GetName(), "namespace", obj.GetNamespace()) objLabels := obj.GetLabels() @@ -90,6 +90,7 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { } key := strings.Split(value, "_") if len(key) != 2 { + //nolint:err113 // error used for logging with dynamic context r.log.Error(fmt.Errorf("invalid label format"), "%s label value(%s) not in expected format on %s resource", IstiocsrResourceWatchLabelName, value, obj.GetName()) return false } diff --git a/pkg/controller/istiocsr/controller_test.go b/pkg/controller/istiocsr/controller_test.go index 9d29a2d86..edaee1415 100644 --- a/pkg/controller/istiocsr/controller_test.go +++ b/pkg/controller/istiocsr/controller_test.go @@ -35,8 +35,8 @@ func TestReconcile(t *testing.T) { }{ { name: "reconciliation successful", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.IstioCSR: istiocsr := testIstioCSR() @@ -53,7 +53,7 @@ func TestReconcile(t *testing.T) { } return nil }) - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -64,7 +64,7 @@ func TestReconcile(t *testing.T) { } return true, nil }) - m.CreateCalls(func(ctx context.Context, obj client.Object, option ...client.CreateOption) error { + m.CreateCalls(func(_ context.Context, obj client.Object, _ ...client.CreateOption) error { switch o := obj.(type) { case *rbacv1.ClusterRoleBinding: roleBinding := testClusterRoleBinding() @@ -90,8 +90,8 @@ func TestReconcile(t *testing.T) { }, { name: "reconciliation failed", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, ns types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.IstioCSR: istiocsr := testIstioCSR() @@ -139,7 +139,7 @@ func TestReconcile(t *testing.T) { } return nil }) - m.UpdateWithRetryCalls(func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + m.UpdateWithRetryCalls(func(_ context.Context, obj client.Object, _ ...client.UpdateOption) error { switch o := obj.(type) { case *v1alpha1.IstioCSR: // fail the operation where controller is trying to add processed annotation. @@ -149,7 +149,7 @@ func TestReconcile(t *testing.T) { } return nil }) - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -160,7 +160,7 @@ func TestReconcile(t *testing.T) { } return true, nil }) - m.CreateCalls(func(ctx context.Context, obj client.Object, option ...client.CreateOption) error { + m.CreateCalls(func(_ context.Context, obj client.Object, _ ...client.CreateOption) error { switch o := obj.(type) { case *rbacv1.ClusterRoleBinding: roleBinding := testClusterRoleBinding() @@ -186,8 +186,8 @@ func TestReconcile(t *testing.T) { }, { name: "reconciliation failed with irrecoverable error", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, ns types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.IstioCSR: istiocsr := testIstioCSR() @@ -235,7 +235,7 @@ func TestReconcile(t *testing.T) { } return nil }) - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch obj.(type) { case *appsv1.Deployment: return false, nil @@ -245,7 +245,7 @@ func TestReconcile(t *testing.T) { return true, nil }) // fail the operation where controller is trying to create deployment resource. - m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + m.CreateCalls(func(_ context.Context, obj client.Object, _ ...client.CreateOption) error { switch o := obj.(type) { case *appsv1.Deployment: return apierrors.NewUnauthorized("test error") @@ -274,8 +274,8 @@ func TestReconcile(t *testing.T) { }, { name: "reconciliation istiocsr resource does not exist", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch obj.(type) { case *v1alpha1.IstioCSR: return apierrors.NewNotFound(v1alpha1.Resource("istiocsr"), "default") @@ -287,8 +287,8 @@ func TestReconcile(t *testing.T) { }, { name: "reconciliation failed to fetch istiocsr resource", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch obj.(type) { case *v1alpha1.IstioCSR: return apierrors.NewBadRequest("test error") @@ -301,8 +301,8 @@ func TestReconcile(t *testing.T) { }, { name: "reconciliation istiocsr marked for deletion", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.IstioCSR: istiocsr := testIstioCSR() @@ -316,8 +316,8 @@ func TestReconcile(t *testing.T) { }, { name: "reconciliation updating istiocsr status subresource failed", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.IstioCSR: istiocsr := testIstioCSR() @@ -328,7 +328,7 @@ func TestReconcile(t *testing.T) { } return nil }) - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -337,7 +337,7 @@ func TestReconcile(t *testing.T) { return true, nil }) m.CreateReturns(nil) - m.StatusUpdateCalls(func(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error { + m.StatusUpdateCalls(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error { switch obj.(type) { case *v1alpha1.IstioCSR: return apierrors.NewBadRequest("test error") @@ -362,8 +362,8 @@ func TestReconcile(t *testing.T) { }, { name: "reconciliation remove finalizer from istiocsr fails", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.IstioCSR: istiocsr := testIstioCSR() @@ -373,7 +373,7 @@ func TestReconcile(t *testing.T) { } return nil }) - m.UpdateWithRetryCalls(func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + m.UpdateWithRetryCalls(func(_ context.Context, obj client.Object, _ ...client.UpdateOption) error { switch obj.(type) { case *v1alpha1.IstioCSR: return errTestClient @@ -386,8 +386,8 @@ func TestReconcile(t *testing.T) { }, { name: "reconciliation adding finalizer to istiocsr fails", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.IstioCSR: istiocsr := testIstioCSR() @@ -395,7 +395,7 @@ func TestReconcile(t *testing.T) { } return nil }) - m.UpdateWithRetryCalls(func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + m.UpdateWithRetryCalls(func(_ context.Context, obj client.Object, _ ...client.UpdateOption) error { switch obj.(type) { case *v1alpha1.IstioCSR: return errTestClient @@ -458,8 +458,8 @@ func TestProcessReconcileRequest(t *testing.T) { getIstioCSR: func() *v1alpha1.IstioCSR { return testIstioCSR() }, - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -473,7 +473,7 @@ func TestProcessReconcileRequest(t *testing.T) { } return nil }) - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -484,7 +484,7 @@ func TestProcessReconcileRequest(t *testing.T) { } return true, nil }) - m.CreateCalls(func(ctx context.Context, obj client.Object, option ...client.CreateOption) error { + m.CreateCalls(func(_ context.Context, obj client.Object, _ ...client.CreateOption) error { switch o := obj.(type) { case *rbacv1.ClusterRoleBinding: roleBinding := testClusterRoleBinding() @@ -519,8 +519,8 @@ func TestProcessReconcileRequest(t *testing.T) { } return istiocsr }, - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -534,7 +534,7 @@ func TestProcessReconcileRequest(t *testing.T) { } return nil }) - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -545,7 +545,7 @@ func TestProcessReconcileRequest(t *testing.T) { } return true, nil }) - m.CreateCalls(func(ctx context.Context, obj client.Object, option ...client.CreateOption) error { + m.CreateCalls(func(_ context.Context, obj client.Object, _ ...client.CreateOption) error { switch o := obj.(type) { case *rbacv1.ClusterRoleBinding: roleBinding := testClusterRoleBinding() @@ -580,7 +580,7 @@ func TestProcessReconcileRequest(t *testing.T) { } return istiocsr }, - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) {}, + preReq: func(_ *Reconciler, _ *fakes.FakeCtrlClient) {}, expectedStatusCondition: []metav1.Condition{ { Type: v1alpha1.Ready, @@ -598,8 +598,8 @@ func TestProcessReconcileRequest(t *testing.T) { getIstioCSR: func() *v1alpha1.IstioCSR { return testIstioCSR() }, - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ListCalls(func(ctx context.Context, list client.ObjectList, option ...client.ListOption) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ListCalls(func(_ context.Context, list client.ObjectList, _ ...client.ListOption) error { switch list.(type) { case *v1alpha1.IstioCSRList: return errTestClient @@ -617,8 +617,8 @@ func TestProcessReconcileRequest(t *testing.T) { istiocsr.SetCreationTimestamp(metav1.NewTime(time.Now().Add(time.Second * 2))) return istiocsr }, - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ListCalls(func(ctx context.Context, list client.ObjectList, option ...client.ListOption) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ListCalls(func(_ context.Context, list client.ObjectList, _ ...client.ListOption) error { switch l := list.(type) { case *v1alpha1.IstioCSRList: istiocsr1 := testIstioCSR() @@ -658,8 +658,8 @@ func TestProcessReconcileRequest(t *testing.T) { istiocsr.SetCreationTimestamp(metav1.NewTime(time.Now().Add(time.Second * 2))) return istiocsr }, - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ListCalls(func(ctx context.Context, list client.ObjectList, option ...client.ListOption) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ListCalls(func(_ context.Context, list client.ObjectList, _ ...client.ListOption) error { switch l := list.(type) { case *v1alpha1.IstioCSRList: istiocsr1 := testIstioCSR() @@ -676,7 +676,7 @@ func TestProcessReconcileRequest(t *testing.T) { l.Items = []v1alpha1.IstioCSR{*istiocsr1, *istiocsr2, *istiocsr3} } - m.StatusUpdateCalls(func(ctx context.Context, obj client.Object, option ...client.SubResourceUpdateOption) error { + m.StatusUpdateCalls(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error { switch obj.(type) { case *v1alpha1.IstioCSR: return errTestClient @@ -707,8 +707,8 @@ func TestProcessReconcileRequest(t *testing.T) { istiocsr.SetCreationTimestamp(metav1.NewTime(time.Now().Add(time.Second * 2))) return istiocsr }, - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ListCalls(func(ctx context.Context, list client.ObjectList, option ...client.ListOption) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ListCalls(func(_ context.Context, list client.ObjectList, _ ...client.ListOption) error { switch l := list.(type) { case *v1alpha1.IstioCSRList: istiocsr1 := testIstioCSR() @@ -725,7 +725,7 @@ func TestProcessReconcileRequest(t *testing.T) { l.Items = []v1alpha1.IstioCSR{*istiocsr1, *istiocsr2, *istiocsr3} } - m.UpdateWithRetryCalls(func(ctx context.Context, obj client.Object, option ...client.UpdateOption) error { + m.UpdateWithRetryCalls(func(_ context.Context, obj client.Object, _ ...client.UpdateOption) error { switch obj.(type) { case *v1alpha1.IstioCSR: return errTestClient diff --git a/pkg/controller/istiocsr/deployments.go b/pkg/controller/istiocsr/deployments.go index 77b306309..b3d1f835f 100644 --- a/pkg/controller/istiocsr/deployments.go +++ b/pkg/controller/istiocsr/deployments.go @@ -108,6 +108,7 @@ func (r *Reconciler) getDeploymentObject(istiocsr *v1alpha1.IstioCSR, resourceLa func (r *Reconciler) updateImage(deployment *appsv1.Deployment) error { image := os.Getenv(istiocsrImageNameEnvVarName) if image == "" { + //nolint:err113 // environment variable name included for debugging return fmt.Errorf("%s environment variable with istiocsr image not set", istiocsrImageNameEnvVarName) } for i, container := range deployment.Spec.Template.Spec.Containers { @@ -335,6 +336,7 @@ func (r *Reconciler) handleUserProvidedCA(deployment *appsv1.Deployment, istiocs // Validate that the specified key exists in the ConfigMap if _, exists := sourceConfigMap.Data[caCertConfig.Key]; !exists { + //nolint:err113 // validation error with ConfigMap key and namespace details for debugging return common.NewIrrecoverableError(fmt.Errorf("key %q not found in ConfigMap %s/%s", caCertConfig.Key, sourceConfigMapKey.Namespace, sourceConfigMapKey.Name), "invalid CA certificate ConfigMap %s/%s", sourceConfigMapKey.Namespace, sourceConfigMapKey.Name) } @@ -378,12 +380,14 @@ func (r *Reconciler) handleIssuerBasedCA(deployment *appsv1.Deployment, istiocsr case clusterIssuerKind: clusterIssuer, ok := obj.(*certmanagerv1.ClusterIssuer) if !ok { + //nolint:err113 // type conversion error for error wrapping return common.FromClientError(fmt.Errorf("failed to convert to ClusterIssuer"), "failed to fetch issuer") } issuerConfig = clusterIssuer.Spec.IssuerConfig case issuerKind: issuer, ok := obj.(*certmanagerv1.Issuer) if !ok { + //nolint:err113 // type conversion error for error wrapping return common.FromClientError(fmt.Errorf("failed to convert to Issuer"), "failed to fetch issuer") } issuerConfig = issuer.Spec.IssuerConfig @@ -528,6 +532,7 @@ func (r *Reconciler) createCAConfigMapFromIstiodCertificate(istiocsr *v1alpha1.I func (r *Reconciler) createCAConfigMapFromIssuerSecret(istiocsr *v1alpha1.IstioCSR, issuerConfig certmanagerv1.IssuerConfig, resourceLabels map[string]string) error { if issuerConfig.CA.SecretName == "" { + //nolint:err113 // issuer name included for debugging return fmt.Errorf("failed to fetch CA certificate configured for the %s issuer of CA type", istiocsr.Spec.IstioCSRConfig.CertManager.IssuerRef.Name) } @@ -550,6 +555,7 @@ func (r *Reconciler) createCAConfigMapFromIssuerSecret(istiocsr *v1alpha1.IstioC // createOrUpdateCAConfigMap creates or updates the CA ConfigMap with the provided certificate data. func (r *Reconciler) createOrUpdateCAConfigMap(istiocsr *v1alpha1.IstioCSR, certData string, resourceLabels map[string]string) error { if certData == "" { + //nolint:err113 // validation error message return fmt.Errorf("failed to find CA certificate") } @@ -596,16 +602,19 @@ func (r *Reconciler) createOrUpdateCAConfigMap(istiocsr *v1alpha1.IstioCSR, cert func (r *Reconciler) validatePEMData(pemData string) error { if pemData == "" { + //nolint:err113 // validation error message return fmt.Errorf("PEM data is empty") } // Parse the first certificate from PEM data block, _ := pem.Decode([]byte(pemData)) if block == nil { + //nolint:err113 // validation error message return fmt.Errorf("no valid PEM data found") } if block.Type != "CERTIFICATE" { + //nolint:err113 // validation error with PEM block type for debugging return fmt.Errorf("PEM block is not a certificate, found: %s", block.Type) } @@ -615,15 +624,18 @@ func (r *Reconciler) validatePEMData(pemData string) error { } if !cert.BasicConstraintsValid { + //nolint:err113 // certificate validation error message return fmt.Errorf("certificate does not have valid Basic Constraints extension") } if !cert.IsCA { + //nolint:err113 // certificate validation error message return fmt.Errorf("certificate is not a CA certificate") } // Check Key Usage for certificate signing if cert.KeyUsage&x509.KeyUsageCertSign == 0 { + //nolint:err113 // certificate validation error message return fmt.Errorf("certificate does not have Certificate Sign key usage") } diff --git a/pkg/controller/istiocsr/deployments_test.go b/pkg/controller/istiocsr/deployments_test.go index 883a6d6ff..1ec9442be 100644 --- a/pkg/controller/istiocsr/deployments_test.go +++ b/pkg/controller/istiocsr/deployments_test.go @@ -33,8 +33,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }{ { name: "deployment reconciliation successful", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -45,7 +45,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *certmanagerv1.Issuer: issuer := testIssuer() @@ -63,8 +63,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }, { name: "deployment reconciliation fails as issuerRef does not exist", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -72,7 +72,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch obj.(type) { case *certmanagerv1.ClusterIssuer: return apierrors.NewNotFound(certmanagerv1.Resource("issuers"), testResourcesName) @@ -87,8 +87,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }, { name: "deployment reconciliation fails as image env var is empty", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -102,8 +102,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }, { name: "deployment reconciliation fails while creating configmap", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -113,14 +113,14 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.CreateCalls(func(ctx context.Context, obj client.Object, _ ...client.CreateOption) error { + m.CreateCalls(func(_ context.Context, obj client.Object, _ ...client.CreateOption) error { switch obj.(type) { case *corev1.ConfigMap: return errTestClient } return nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *certmanagerv1.Issuer: issuer := testIssuer() @@ -136,8 +136,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }, { name: "deployment reconciliation updating volume successful", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -148,7 +148,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *certmanagerv1.Issuer: issuer := testIssuer() @@ -163,8 +163,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }, { name: "deployment reconciliation fails while checking if exists", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: return false, errTestClient @@ -174,7 +174,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *certmanagerv1.Issuer: issuer := testIssuer() @@ -190,8 +190,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }, { name: "deployment reconciliation failed while restoring to desired state", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -203,7 +203,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *certmanagerv1.Issuer: issuer := testIssuer() @@ -214,7 +214,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return nil }) - m.UpdateWithRetryCalls(func(ctx context.Context, obj client.Object, _ ...client.UpdateOption) error { + m.UpdateWithRetryCalls(func(_ context.Context, obj client.Object, _ ...client.UpdateOption) error { switch obj.(type) { case *appsv1.Deployment: return errTestClient @@ -226,8 +226,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }, { name: "deployment reconciliation with user custom config successful", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -238,7 +238,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *certmanagerv1.ClusterIssuer: issuer := testClusterIssuer() @@ -327,8 +327,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }, { name: "deployment reconciliation fails while updating image in istiocsr status", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: return false, nil @@ -338,7 +338,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *certmanagerv1.Issuer: issuer := testIssuer() @@ -349,7 +349,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return nil }) - m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + m.CreateCalls(func(_ context.Context, obj client.Object, _ ...client.CreateOption) error { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -357,7 +357,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return nil }) - m.StatusUpdateCalls(func(ctx context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error { + m.StatusUpdateCalls(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error { switch obj.(type) { case *v1alpha1.IstioCSR: return errTestClient @@ -369,8 +369,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }, { name: "deployment reconciliation fails as invalid kind in issuerRef", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -389,8 +389,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }, { name: "deployment reconciliation fails as invalid group in issuerRef", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -409,8 +409,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }, { name: "deployment reconciliation fails as unsupported ACME issuer is used", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -424,7 +424,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *certmanagerv1.ClusterIssuer: issuer := testACMEIssuer() @@ -440,8 +440,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }, { name: "deployment reconciliation while fetching issuer", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -449,7 +449,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch obj.(type) { case *certmanagerv1.Issuer: return apierrors.NewUnauthorized("no access") @@ -461,8 +461,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }, { name: "deployment reconciliation fails while fetching secret referenced in issuer", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -470,7 +470,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *corev1.Secret: return apierrors.NewUnauthorized("no access") @@ -488,8 +488,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }, { name: "deployment reconciliation fails while updating labels on secret referenced in issuer", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -497,14 +497,14 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.UpdateWithRetryCalls(func(ctx context.Context, obj client.Object, _ ...client.UpdateOption) error { + m.UpdateWithRetryCalls(func(_ context.Context, obj client.Object, _ ...client.UpdateOption) error { switch obj.(type) { case *corev1.Secret: return apierrors.NewUnauthorized("no access") } return nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *corev1.ConfigMap: configmap := testConfigMap() @@ -520,8 +520,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }, { name: "deployment reconciliation fails while checking configmap exists", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -531,7 +531,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *certmanagerv1.Issuer: issuer := testIssuer() @@ -547,8 +547,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }, { name: "deployment reconciliation fails while updating configmap to desired state", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -560,14 +560,14 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.UpdateWithRetryCalls(func(ctx context.Context, obj client.Object, _ ...client.UpdateOption) error { + m.UpdateWithRetryCalls(func(_ context.Context, obj client.Object, _ ...client.UpdateOption) error { switch obj.(type) { case *corev1.ConfigMap: return errTestClient } return nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *certmanagerv1.Issuer: issuer := testIssuer() @@ -583,8 +583,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }, { name: "deployment reconciliation configmap creation successful", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -594,7 +594,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *certmanagerv1.Issuer: issuer := testIssuer() @@ -609,8 +609,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }, { name: "deployment reconciliation with invalid toleration configuration", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -621,7 +621,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *certmanagerv1.ClusterIssuer: issuer := testClusterIssuer() @@ -644,8 +644,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }, { name: "deployment reconciliation with invalid nodeSelector configuration", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -656,7 +656,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *certmanagerv1.ClusterIssuer: issuer := testClusterIssuer() @@ -673,8 +673,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }, { name: "deployment reconciliation with invalid affinity configuration", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -685,7 +685,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *certmanagerv1.ClusterIssuer: issuer := testClusterIssuer() @@ -750,8 +750,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }, { name: "deployment reconciliation with invalid resource requirement configuration", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -762,7 +762,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *certmanagerv1.ClusterIssuer: issuer := testClusterIssuer() @@ -789,8 +789,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }, { name: "deployment reconciliation successful with CA certificate ConfigMap", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -801,7 +801,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, ns types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *corev1.ConfigMap: if ns.Name == "ca-cert-test" { @@ -824,8 +824,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }, { name: "deployment reconciliation successful with CA certificate ConfigMap in custom namespace", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -833,7 +833,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, ns types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *corev1.ConfigMap: if ns.Name == "ca-cert-test" && ns.Namespace == "custom-namespace" { @@ -857,8 +857,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }, { name: "deployment reconciliation fails with missing CA certificate ConfigMap", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -866,7 +866,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, ns types.NamespacedName, obj client.Object) error { switch obj.(type) { case *corev1.ConfigMap: if ns.Name == "ca-cert-test" { @@ -887,8 +887,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }, { name: "deployment reconciliation fails with missing key in CA certificate ConfigMap", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -896,7 +896,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, ns types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *corev1.ConfigMap: if ns.Name == "ca-cert-test" { @@ -920,8 +920,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { }, { name: "deployment reconciliation fails with invalid PEM data in CA certificate ConfigMap", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -929,7 +929,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, ns types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *corev1.ConfigMap: if ns.Name == "ca-cert-test" { @@ -954,8 +954,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { { name: "deployment reconciliation fails while updating watch label on CA certificate ConfigMap", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -966,7 +966,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, ns types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *corev1.ConfigMap: if ns.Name == "watch-label-fail-test" { @@ -977,7 +977,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return nil }) - m.UpdateWithRetryCalls(func(ctx context.Context, obj client.Object, _ ...client.UpdateOption) error { + m.UpdateWithRetryCalls(func(_ context.Context, obj client.Object, _ ...client.UpdateOption) error { switch obj.(type) { case *corev1.ConfigMap: // Fail when trying to update the source ConfigMap with watch label @@ -1001,8 +1001,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { { name: "deployment reconciliation fails with non-CA certificate in ConfigMap", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -1010,7 +1010,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, ns types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *corev1.ConfigMap: if ns.Name == "non-ca-cert-test" { @@ -1033,8 +1033,8 @@ func TestCreateOrApplyDeployments(t *testing.T) { { name: "deployment reconciliation fails with certificate missing KeyUsageCertSign in ConfigMap", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *appsv1.Deployment: deployment := testDeployment() @@ -1042,7 +1042,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { } return true, nil }) - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + m.GetCalls(func(_ context.Context, ns types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *corev1.ConfigMap: if ns.Name == "ca-without-certsign-test" { @@ -1101,7 +1101,7 @@ func TestUpdateArgList(t *testing.T) { }{ { name: "clusterID not provided should default to Kubernetes", - updateIstioCSR: func(istiocsr *v1alpha1.IstioCSR) { + updateIstioCSR: func(_ *v1alpha1.IstioCSR) { // Server config is nil, so clusterID should default }, expectedArgs: map[string]string{ diff --git a/pkg/controller/istiocsr/install_instiocsr_test.go b/pkg/controller/istiocsr/install_instiocsr_test.go index d128b1f00..a290d825a 100644 --- a/pkg/controller/istiocsr/install_instiocsr_test.go +++ b/pkg/controller/istiocsr/install_instiocsr_test.go @@ -1,3 +1,4 @@ +//nolint:err113 // test file uses dynamic errors for validation messages package istiocsr import ( @@ -36,8 +37,8 @@ func TestReconcileIstioCSRDeployment(t *testing.T) { }{ { name: "istiocsr reconciliation with user labels successful", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { switch o := obj.(type) { case *certmanagerv1.Issuer: issuer := testIssuer() @@ -48,7 +49,7 @@ func TestReconcileIstioCSRDeployment(t *testing.T) { } return nil }) - m.CreateCalls(func(ctx context.Context, obj client.Object, option ...client.CreateOption) error { + m.CreateCalls(func(_ context.Context, obj client.Object, _ ...client.CreateOption) error { switch o := obj.(type) { case *appsv1.Deployment, *corev1.Service, *corev1.ServiceAccount: if !reflect.DeepEqual(o.GetLabels(), labels) { @@ -68,8 +69,8 @@ func TestReconcileIstioCSRDeployment(t *testing.T) { }, { name: "istiocsr reconciliation fails with serviceaccount creation error", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.CreateCalls(func(ctx context.Context, obj client.Object, option ...client.CreateOption) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.CreateCalls(func(_ context.Context, obj client.Object, _ ...client.CreateOption) error { switch obj.(type) { case *corev1.ServiceAccount: return errTestClient @@ -81,8 +82,8 @@ func TestReconcileIstioCSRDeployment(t *testing.T) { }, { name: "istiocsr reconciliation fails with role creation error", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.CreateCalls(func(ctx context.Context, obj client.Object, option ...client.CreateOption) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.CreateCalls(func(_ context.Context, obj client.Object, _ ...client.CreateOption) error { switch o := obj.(type) { case *rbacv1.Role: return errTestClient @@ -97,8 +98,8 @@ func TestReconcileIstioCSRDeployment(t *testing.T) { }, { name: "istiocsr reconciliation fails with certificate creation error", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.CreateCalls(func(ctx context.Context, obj client.Object, option ...client.CreateOption) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.CreateCalls(func(_ context.Context, obj client.Object, _ ...client.CreateOption) error { switch o := obj.(type) { case *certmanagerv1.Certificate: return errTestClient diff --git a/pkg/controller/istiocsr/networkpolicies.go b/pkg/controller/istiocsr/networkpolicies.go index 03bbca008..9371ad049 100644 --- a/pkg/controller/istiocsr/networkpolicies.go +++ b/pkg/controller/istiocsr/networkpolicies.go @@ -47,6 +47,7 @@ func (r *Reconciler) getNetworkPolicyFromAsset(assetPath string, istiocsr *v1alp policy, ok := obj.(*networkingv1.NetworkPolicy) if !ok { + //nolint:err113 // type assertion error with actual type for debugging return nil, fmt.Errorf("decoded object is not a NetworkPolicy, got %T", obj) } diff --git a/pkg/controller/istiocsr/rbacs.go b/pkg/controller/istiocsr/rbacs.go index 1d77c1a63..97a795a46 100644 --- a/pkg/controller/istiocsr/rbacs.go +++ b/pkg/controller/istiocsr/rbacs.go @@ -88,6 +88,7 @@ func (r *Reconciler) createOrApplyClusterRoles(istiocsr *v1alpha1.IstioCSR, reso if len(clusterRoleList.Items) > 0 { if len(clusterRoleList.Items) != 1 { r.eventRecorder.Eventf(istiocsr, corev1.EventTypeWarning, "DuplicateResources", "more than 1 clusterrole resources exist with matching labels") + //nolint:err113 // error used for event recording and error wrapping return "", common.NewIrrecoverableError(fmt.Errorf("more than 1 clusterrole resources exist with matching labels"), "matched clusterrole resources: %+v", clusterRoleList.Items) } clusterRoleList.Items[0].DeepCopyInto(fetched) @@ -146,6 +147,7 @@ func (r *Reconciler) updateClusterRoleNameInStatus(istiocsr *v1alpha1.IstioCSR, if existing != nil && existing.GetName() != "" { name = existing.GetName() } else { + //nolint:err113 // error used for logging r.log.Error(fmt.Errorf("error updating clusterrole name in status"), "istiocsr", istiocsr.GetNamespace()) } } @@ -187,6 +189,7 @@ func (r *Reconciler) createOrApplyClusterRoleBindings(istiocsr *v1alpha1.IstioCS if len(clusterRoleBindingsList.Items) > 0 { if len(clusterRoleBindingsList.Items) != 1 { r.eventRecorder.Eventf(istiocsr, corev1.EventTypeWarning, "DuplicateResources", "more than 1 clusterrolebinding resources exist with matching labels") + //nolint:err113 // error used for event recording and error wrapping return common.NewIrrecoverableError(fmt.Errorf("more than 1 clusterrolebinding resources exist with matching labels"), "matched clusterrolebinding resources: %+v", clusterRoleBindingsList.Items) } clusterRoleBindingsList.Items[0].DeepCopyInto(fetched) @@ -241,6 +244,7 @@ func (r *Reconciler) updateClusterRoleBindingNameInStatus(istiocsr *v1alpha1.Ist if existing != nil && existing.GetName() != "" { name = existing.GetName() } else { + //nolint:err113 // error used for logging r.log.Error(fmt.Errorf("error updating clusterrolebinding name in status"), "istiocsr", istiocsr.GetNamespace()) } } @@ -250,38 +254,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 +266,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 +279,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 +291,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 +317,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 diff --git a/pkg/controller/istiocsr/rbacs_test.go b/pkg/controller/istiocsr/rbacs_test.go index c2706e78a..80be431d6 100644 --- a/pkg/controller/istiocsr/rbacs_test.go +++ b/pkg/controller/istiocsr/rbacs_test.go @@ -26,8 +26,8 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }{ { name: "clusterrole reconciliation fails while checking if exists", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch obj.(type) { case *rbacv1.ClusterRole: return false, errTestClient @@ -42,8 +42,8 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }, { name: "clusterrolebindings reconciliation fails while checking if exists", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch obj.(type) { case *rbacv1.ClusterRoleBinding: return false, errTestClient @@ -58,8 +58,8 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }, { name: "role reconciliation fails while checking if exists", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch obj.(type) { case *rbacv1.Role: return false, errTestClient @@ -71,8 +71,8 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }, { name: "rolebindings reconciliation fails while checking if exists", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch obj.(type) { case *rbacv1.RoleBinding: return false, errTestClient @@ -84,8 +84,8 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }, { name: "role-leases reconciliation fails while checking if exists", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { switch obj.(type) { case *rbacv1.Role: if strings.HasSuffix(ns.Name, "-leases") { @@ -99,8 +99,8 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }, { name: "rolebindings-leases reconciliation fails while checking if exists", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { switch obj.(type) { case *rbacv1.RoleBinding: if strings.HasSuffix(ns.Name, "-leases") { @@ -114,8 +114,8 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }, { name: "clusterrolebindings reconciliation fails while listing existing resources", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ListCalls(func(ctx context.Context, obj client.ObjectList, opts ...client.ListOption) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ListCalls(func(_ context.Context, obj client.ObjectList, _ ...client.ListOption) error { switch obj.(type) { case *rbacv1.ClusterRoleBindingList: return errTestClient @@ -127,8 +127,8 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }, { name: "clusterrolebindings reconciliation fails while listing multiple existing resources", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ListCalls(func(ctx context.Context, obj client.ObjectList, opts ...client.ListOption) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ListCalls(func(_ context.Context, obj client.ObjectList, _ ...client.ListOption) error { switch o := obj.(type) { case *rbacv1.ClusterRoleBindingList: clusterRoleBindingsList := &rbacv1.ClusterRoleBindingList{} @@ -145,8 +145,8 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }, { name: "clusterrolebindings reconciliation successful", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ListCalls(func(ctx context.Context, obj client.ObjectList, opts ...client.ListOption) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ListCalls(func(_ context.Context, obj client.ObjectList, _ ...client.ListOption) error { switch o := obj.(type) { case *rbacv1.ClusterRoleBindingList: clusterRoleBindingsList := &rbacv1.ClusterRoleBindingList{} @@ -162,8 +162,8 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }, { name: "clusterrolebindings reconciliation updating to desired state fails", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, object client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, object client.Object) (bool, error) { switch o := object.(type) { case *rbacv1.ClusterRole: cr := testClusterRole() @@ -177,7 +177,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) { } return true, nil }) - m.UpdateWithRetryCalls(func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + m.UpdateWithRetryCalls(func(_ context.Context, obj client.Object, _ ...client.UpdateOption) error { switch obj.(type) { case *rbacv1.ClusterRoleBinding: return errTestClient @@ -196,8 +196,8 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }, { name: "clusterrolebindings reconciliation updating to desired state successful", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, object client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, object client.Object) (bool, error) { switch o := object.(type) { case *rbacv1.ClusterRole: cr := testClusterRole() @@ -223,8 +223,8 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }, { name: "clusterrolebindings reconciliation roleRef change delete fails", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, object client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, object client.Object) (bool, error) { switch o := object.(type) { case *rbacv1.ClusterRole: cr := testClusterRole() @@ -236,7 +236,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) { } return true, nil }) - m.DeleteCalls(func(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { + m.DeleteCalls(func(_ context.Context, obj client.Object, _ ...client.DeleteOption) error { switch obj.(type) { case *rbacv1.ClusterRoleBinding: return errTestClient @@ -255,8 +255,8 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }, { name: "clusterrolebindings reconciliation roleRef change delete succeeds and recreate succeeds", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, object client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, object client.Object) (bool, error) { switch o := object.(type) { case *rbacv1.ClusterRole: cr := testClusterRole() @@ -280,8 +280,8 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }, { name: "clusterrolebindings reconciliation roleRef change delete succeeds and recreate fails", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, object client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, object client.Object) (bool, error) { switch o := object.(type) { case *rbacv1.ClusterRole: cr := testClusterRole() @@ -293,7 +293,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) { } return true, nil }) - m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + m.CreateCalls(func(_ context.Context, obj client.Object, _ ...client.CreateOption) error { switch obj.(type) { case *rbacv1.ClusterRoleBinding: return errTestClient @@ -312,15 +312,15 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }, { name: "clusterrolebindings reconciliation creation fails", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, object client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, object client.Object) (bool, error) { switch object.(type) { case *rbacv1.ClusterRoleBinding: return false, nil } return true, nil }) - m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + m.CreateCalls(func(_ context.Context, obj client.Object, _ ...client.CreateOption) error { switch obj.(type) { case *rbacv1.ClusterRoleBinding: return errTestClient @@ -332,8 +332,8 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }, { name: "clusterrole reconciliation updating name in status fails", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ListCalls(func(ctx context.Context, obj client.ObjectList, opts ...client.ListOption) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ListCalls(func(_ context.Context, obj client.ObjectList, _ ...client.ListOption) error { switch o := obj.(type) { case *rbacv1.ClusterRoleBindingList: clusterRoleBindingsList := &rbacv1.ClusterRoleBindingList{} @@ -344,7 +344,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) { } return nil }) - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, object client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, object client.Object) (bool, error) { switch o := object.(type) { case *rbacv1.ClusterRole: clusterRole := testClusterRole() @@ -353,7 +353,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) { } return true, nil }) - m.StatusUpdateCalls(func(ctx context.Context, obj client.Object, option ...client.SubResourceUpdateOption) error { + m.StatusUpdateCalls(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error { switch obj.(type) { case *v1alpha1.IstioCSR: return errTestClient @@ -368,8 +368,8 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }, { name: "clusterrolebindings reconciliation updating name in status fails", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ListCalls(func(ctx context.Context, obj client.ObjectList, opts ...client.ListOption) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ListCalls(func(_ context.Context, obj client.ObjectList, _ ...client.ListOption) error { switch o := obj.(type) { case *rbacv1.ClusterRoleBindingList: clusterRoleBindingsList := &rbacv1.ClusterRoleBindingList{} @@ -380,7 +380,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) { } return nil }) - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, object client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, object client.Object) (bool, error) { switch o := object.(type) { case *rbacv1.ClusterRole: clusterRole := testClusterRole() @@ -389,7 +389,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) { } return true, nil }) - m.StatusUpdateCalls(func(ctx context.Context, obj client.Object, option ...client.SubResourceUpdateOption) error { + m.StatusUpdateCalls(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error { switch o := obj.(type) { case *v1alpha1.IstioCSR: if o.Status.ClusterRoleBinding != "" { @@ -406,8 +406,8 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }, { name: "clusterrole reconciliation updating to desired state fails", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, object client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, object client.Object) (bool, error) { switch o := object.(type) { case *rbacv1.ClusterRole: clusterRole := testClusterRole() @@ -416,7 +416,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) { } return true, nil }) - m.UpdateWithRetryCalls(func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + m.UpdateWithRetryCalls(func(_ context.Context, obj client.Object, _ ...client.UpdateOption) error { switch obj.(type) { case *rbacv1.ClusterRole: return errTestClient @@ -434,15 +434,15 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }, { name: "clusterrole reconciliation creation fails", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, object client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, object client.Object) (bool, error) { switch object.(type) { case *rbacv1.ClusterRole: return false, nil } return true, nil }) - m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + m.CreateCalls(func(_ context.Context, obj client.Object, _ ...client.CreateOption) error { switch obj.(type) { case *rbacv1.ClusterRole: return errTestClient @@ -454,8 +454,8 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }, { name: "role reconciliation updating to desired state fails", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, object client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, object client.Object) (bool, error) { switch o := object.(type) { case *rbacv1.Role: role := testRole() @@ -464,7 +464,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) { } return true, nil }) - m.UpdateWithRetryCalls(func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + m.UpdateWithRetryCalls(func(_ context.Context, obj client.Object, _ ...client.UpdateOption) error { switch obj.(type) { case *rbacv1.Role: return errTestClient @@ -476,15 +476,15 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }, { name: "role reconciliation creation fails", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, object client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, object client.Object) (bool, error) { switch object.(type) { case *rbacv1.Role: return false, nil } return true, nil }) - m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + m.CreateCalls(func(_ context.Context, obj client.Object, _ ...client.CreateOption) error { switch obj.(type) { case *rbacv1.Role: return errTestClient @@ -496,8 +496,8 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }, { name: "role-leases reconciliation updating to desired state fails", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, object client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, ns types.NamespacedName, object client.Object) (bool, error) { switch o := object.(type) { case *rbacv1.Role: if strings.HasSuffix(ns.Name, "-leases") { @@ -508,7 +508,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) { } return true, nil }) - m.UpdateWithRetryCalls(func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + m.UpdateWithRetryCalls(func(_ context.Context, obj client.Object, _ ...client.UpdateOption) error { switch obj.(type) { case *rbacv1.Role: if strings.HasSuffix(obj.GetName(), "-leases") { @@ -522,8 +522,8 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }, { name: "role-leases reconciliation creation fails", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, object client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, ns types.NamespacedName, object client.Object) (bool, error) { switch object.(type) { case *rbacv1.Role: if strings.HasSuffix(ns.Name, "-leases") { @@ -532,7 +532,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) { } return true, nil }) - m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + m.CreateCalls(func(_ context.Context, obj client.Object, _ ...client.CreateOption) error { switch obj.(type) { case *rbacv1.Role: if strings.HasSuffix(obj.GetName(), "-leases") { @@ -546,8 +546,8 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }, { name: "rolebindings reconciliation updating to desired state fails", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, object client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, object client.Object) (bool, error) { switch o := object.(type) { case *rbacv1.RoleBinding: role := testRoleBinding() @@ -556,7 +556,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) { } return true, nil }) - m.UpdateWithRetryCalls(func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + m.UpdateWithRetryCalls(func(_ context.Context, obj client.Object, _ ...client.UpdateOption) error { switch obj.(type) { case *rbacv1.RoleBinding: return errTestClient @@ -568,15 +568,15 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }, { name: "rolebindings reconciliation creation fails", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, object client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, object client.Object) (bool, error) { switch object.(type) { case *rbacv1.RoleBinding: return false, nil } return true, nil }) - m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + m.CreateCalls(func(_ context.Context, obj client.Object, _ ...client.CreateOption) error { switch obj.(type) { case *rbacv1.RoleBinding: return errTestClient @@ -588,8 +588,8 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }, { name: "rolebinding-leases reconciliation updating to desired state fails", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, object client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, ns types.NamespacedName, object client.Object) (bool, error) { switch o := object.(type) { case *rbacv1.RoleBinding: if strings.HasSuffix(ns.Name, "-leases") { @@ -600,7 +600,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) { } return true, nil }) - m.UpdateWithRetryCalls(func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + m.UpdateWithRetryCalls(func(_ context.Context, obj client.Object, _ ...client.UpdateOption) error { switch obj.(type) { case *rbacv1.RoleBinding: if strings.HasSuffix(obj.GetName(), "-leases") { @@ -614,8 +614,8 @@ func TestCreateOrApplyRBACResource(t *testing.T) { }, { name: "rolebinding-leases reconciliation creation fails", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, object client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, ns types.NamespacedName, object client.Object) (bool, error) { switch object.(type) { case *rbacv1.RoleBinding: if strings.HasSuffix(ns.Name, "-leases") { @@ -624,7 +624,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) { } return true, nil }) - m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + m.CreateCalls(func(_ context.Context, obj client.Object, _ ...client.CreateOption) error { switch obj.(type) { case *rbacv1.RoleBinding: if strings.HasSuffix(obj.GetName(), "-leases") { @@ -676,7 +676,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) { func assertClusterRoleBindingUpdateUsesLiveName(t *testing.T, m *fakes.FakeCtrlClient, wantName string) { t.Helper() var crbUpdates int - for i := 0; i < m.UpdateWithRetryCallCount(); i++ { + for i := range m.UpdateWithRetryCallCount() { _, obj, _ := m.UpdateWithRetryArgsForCall(i) if crb, ok := obj.(*rbacv1.ClusterRoleBinding); ok { crbUpdates++ @@ -697,7 +697,7 @@ func assertClusterRoleBindingUpdateUsesLiveName(t *testing.T, m *fakes.FakeCtrlC func assertClusterRoleUpdateUsesLiveName(t *testing.T, m *fakes.FakeCtrlClient, wantName string) { t.Helper() var crUpdates int - for i := 0; i < m.UpdateWithRetryCallCount(); i++ { + for i := range m.UpdateWithRetryCallCount() { _, obj, _ := m.UpdateWithRetryArgsForCall(i) if cr, ok := obj.(*rbacv1.ClusterRole); ok { crUpdates++ @@ -716,7 +716,7 @@ func assertClusterRoleUpdateUsesLiveName(t *testing.T, m *fakes.FakeCtrlClient, func assertNoClusterRoleBindingUpdateWithRetry(t *testing.T, m *fakes.FakeCtrlClient) { t.Helper() - for i := 0; i < m.UpdateWithRetryCallCount(); i++ { + for i := range m.UpdateWithRetryCallCount() { _, obj, _ := m.UpdateWithRetryArgsForCall(i) if _, ok := obj.(*rbacv1.ClusterRoleBinding); ok { t.Fatalf("expected no UpdateWithRetry on ClusterRoleBinding for roleRef replace path, got call index %d", i) @@ -726,7 +726,7 @@ func assertNoClusterRoleBindingUpdateWithRetry(t *testing.T, m *fakes.FakeCtrlCl func clusterRoleBindingDeleteCount(m *fakes.FakeCtrlClient) int { n := 0 - for i := 0; i < m.DeleteCallCount(); i++ { + for i := range m.DeleteCallCount() { _, obj, _ := m.DeleteArgsForCall(i) if _, ok := obj.(*rbacv1.ClusterRoleBinding); ok { n++ @@ -754,7 +754,7 @@ func assertClusterRoleBindingRoleRefReplaceUsesDeleteCreate(t *testing.T, m *fak t.Fatalf("expected Delete on ClusterRoleBinding for roleRef replace") } var sawCreate bool - for i := 0; i < m.CreateCallCount(); i++ { + for i := range m.CreateCallCount() { _, obj, _ := m.CreateArgsForCall(i) crb, ok := obj.(*rbacv1.ClusterRoleBinding) if !ok { diff --git a/pkg/controller/istiocsr/serviceaccounts_test.go b/pkg/controller/istiocsr/serviceaccounts_test.go index 82e0f9c4a..24f8b70ba 100644 --- a/pkg/controller/istiocsr/serviceaccounts_test.go +++ b/pkg/controller/istiocsr/serviceaccounts_test.go @@ -31,8 +31,8 @@ func TestCreateOrApplyServiceAccounts(t *testing.T) { }{ { name: "serviceaccount reconciliation successful", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *corev1.ServiceAccount: serviceaccount := testServiceAccount() @@ -44,8 +44,8 @@ func TestCreateOrApplyServiceAccounts(t *testing.T) { }, { name: "serviceaccount reconciliation fails while checking if exists", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch obj.(type) { case *corev1.ServiceAccount: return false, errTestClient @@ -57,8 +57,8 @@ func TestCreateOrApplyServiceAccounts(t *testing.T) { }, { name: "serviceaccount reconciliation fails while updating status", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.StatusUpdateCalls(func(ctx context.Context, obj client.Object, option ...client.SubResourceUpdateOption) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.StatusUpdateCalls(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error { switch obj.(type) { case *v1alpha1.IstioCSR: return errTestClient @@ -71,8 +71,8 @@ func TestCreateOrApplyServiceAccounts(t *testing.T) { { name: "serviceaccount already exists with istioCSRCreateRecon true records ResourceAlreadyExists event", istioCSRCreateRecon: true, - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *corev1.ServiceAccount: serviceaccount := testServiceAccount() @@ -95,8 +95,8 @@ func TestCreateOrApplyServiceAccounts(t *testing.T) { }, { name: "serviceaccount exists but modified is updated and reconciled", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *corev1.ServiceAccount: serviceaccount := testServiceAccount() @@ -126,8 +126,8 @@ func TestCreateOrApplyServiceAccounts(t *testing.T) { }, { name: "serviceaccount reconciliation fails while updating modified serviceaccount", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *corev1.ServiceAccount: serviceaccount := testServiceAccount() @@ -136,7 +136,7 @@ func TestCreateOrApplyServiceAccounts(t *testing.T) { } return true, nil }) - m.UpdateWithRetryCalls(func(ctx context.Context, obj client.Object, _ ...client.UpdateOption) error { + m.UpdateWithRetryCalls(func(_ context.Context, obj client.Object, _ ...client.UpdateOption) error { switch obj.(type) { case *corev1.ServiceAccount: return errTestClient @@ -148,8 +148,8 @@ func TestCreateOrApplyServiceAccounts(t *testing.T) { }, { name: "serviceaccount created when it does not exist", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch obj.(type) { case *corev1.ServiceAccount: return false, nil diff --git a/pkg/controller/istiocsr/services_test.go b/pkg/controller/istiocsr/services_test.go index 708a116a4..586451a35 100644 --- a/pkg/controller/istiocsr/services_test.go +++ b/pkg/controller/istiocsr/services_test.go @@ -27,8 +27,8 @@ func TestCreateOrApplyServices(t *testing.T) { }{ { name: "service reconciliation successful", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *corev1.Service: service := testService() @@ -41,8 +41,8 @@ func TestCreateOrApplyServices(t *testing.T) { }, { name: "service reconciliation fails while checking if exists", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch obj.(type) { case *corev1.Service: return false, errTestClient @@ -54,15 +54,15 @@ func TestCreateOrApplyServices(t *testing.T) { }, { name: "service reconciliation fails while updating to desired state", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.UpdateWithRetryCalls(func(ctx context.Context, obj client.Object, option ...client.UpdateOption) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.UpdateWithRetryCalls(func(_ context.Context, obj client.Object, _ ...client.UpdateOption) error { switch obj.(type) { case *corev1.Service: return errTestClient } return nil }) - m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) (bool, error) { switch o := obj.(type) { case *corev1.Service: service := testService() @@ -77,8 +77,8 @@ func TestCreateOrApplyServices(t *testing.T) { }, { name: "service reconciliation fails while creating", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.CreateCalls(func(_ context.Context, obj client.Object, _ ...client.CreateOption) error { switch obj.(type) { case *corev1.Service: return errTestClient diff --git a/pkg/controller/istiocsr/utils.go b/pkg/controller/istiocsr/utils.go index 34477b561..038b40275 100644 --- a/pkg/controller/istiocsr/utils.go +++ b/pkg/controller/istiocsr/utils.go @@ -75,6 +75,7 @@ func (r *Reconciler) addFinalizer(ctx context.Context, istiocsr *v1alpha1.IstioC namespacedName := client.ObjectKeyFromObject(istiocsr) if !controllerutil.ContainsFinalizer(istiocsr, finalizer) { if !controllerutil.AddFinalizer(istiocsr, finalizer) { + //nolint:err113 // namespacedName included for debugging return fmt.Errorf("failed to create %q istiocsr.openshift.operator.io object with finalizers added", namespacedName) } @@ -98,6 +99,7 @@ func (r *Reconciler) removeFinalizer(ctx context.Context, istiocsr *v1alpha1.Ist namespacedName := client.ObjectKeyFromObject(istiocsr) if controllerutil.ContainsFinalizer(istiocsr, finalizer) { if !controllerutil.RemoveFinalizer(istiocsr, finalizer) { + //nolint:err113 // namespacedName included for debugging return fmt.Errorf("failed to create %q istiocsr.openshift.operator.io object with finalizers removed", namespacedName) } @@ -463,15 +465,19 @@ func networkPolicySpecModified(desired, fetched *networkingv1.NetworkPolicy) boo func validateIstioCSRConfig(istiocsr *v1alpha1.IstioCSR) error { if reflect.ValueOf(istiocsr.Spec.IstioCSRConfig).IsZero() { + //nolint:err113 // user-facing validation error message return fmt.Errorf("spec.istioCSRConfig config cannot be empty") } if reflect.ValueOf(istiocsr.Spec.IstioCSRConfig.IstiodTLSConfig).IsZero() { + //nolint:err113 // user-facing validation error message return fmt.Errorf("spec.istioCSRConfig.istiodTLSConfig config cannot be empty") } if reflect.ValueOf(istiocsr.Spec.IstioCSRConfig.Istio).IsZero() { + //nolint:err113 // user-facing validation error message return fmt.Errorf("spec.istioCSRConfig.istio config cannot be empty") } if reflect.ValueOf(istiocsr.Spec.IstioCSRConfig.CertManager).IsZero() { + //nolint:err113 // user-facing validation error message return fmt.Errorf("spec.istioCSRConfig.certManager config cannot be empty") } return nil @@ -498,6 +504,7 @@ func (r *Reconciler) disallowMultipleIstioCSRInstances(istiocsr *v1alpha1.IstioC if istiocsr.Status.SetCondition(v1alpha1.Ready, metav1.ConditionFalse, v1alpha1.ReasonFailed, statusMessage) { updateErr = r.updateCondition(istiocsr, nil) } + //nolint:err113 // dynamic error message from variable for error wrapping return common.NewMultipleInstanceError(utilerrors.NewAggregate([]error{fmt.Errorf("%s", statusMessage), updateErr})) } @@ -544,5 +551,6 @@ func (r *Reconciler) disallowMultipleIstioCSRInstances(istiocsr *v1alpha1.IstioC return utilerrors.NewAggregate([]error{condUpdateErr, annUpdateErr}) } + //nolint:err113 // dynamic error message from variable for error wrapping return common.NewMultipleInstanceError(fmt.Errorf("%s", statusMessage)) } diff --git a/pkg/controller/trustmanager/certificates.go b/pkg/controller/trustmanager/certificates.go index be60843e7..0c4c141ed 100644 --- a/pkg/controller/trustmanager/certificates.go +++ b/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 { diff --git a/pkg/controller/trustmanager/certificates_test.go b/pkg/controller/trustmanager/certificates_test.go index 122773ed8..b7ef70a33 100644 --- a/pkg/controller/trustmanager/certificates_test.go +++ b/pkg/controller/trustmanager/certificates_test.go @@ -178,8 +178,8 @@ func TestIssuerReconciliation(t *testing.T) { }{ { name: "successful apply when not found", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, nil }) }, @@ -188,8 +188,8 @@ func TestIssuerReconciliation(t *testing.T) { }, { name: "skip apply when existing matches desired", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { issuer := getIssuerObject(testResourceLabels(), testResourceAnnotations()) issuer.DeepCopyInto(obj.(*certmanagerv1.Issuer)) return true, nil @@ -200,8 +200,8 @@ func TestIssuerReconciliation(t *testing.T) { }, { name: "apply when existing has label drift", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { issuer := getIssuerObject(testResourceLabels(), testResourceAnnotations()) issuer.Labels["app.kubernetes.io/instance"] = "modified-value" issuer.DeepCopyInto(obj.(*certmanagerv1.Issuer)) @@ -214,8 +214,8 @@ func TestIssuerReconciliation(t *testing.T) { { name: "apply when existing has annotation drift", tmBuilder: testTrustManager().WithAnnotations(map[string]string{"user-annotation": "original"}), - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { tm := testTrustManager().WithAnnotations(map[string]string{"user-annotation": "original"}).Build() issuer := getIssuerObject(getResourceLabels(tm), getResourceAnnotations(tm)) issuer.Annotations["user-annotation"] = "tampered" @@ -228,8 +228,8 @@ func TestIssuerReconciliation(t *testing.T) { }, { name: "exists error propagates", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, errTestClient }) }, @@ -239,11 +239,11 @@ func TestIssuerReconciliation(t *testing.T) { }, { name: "patch error propagates", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, nil }) - m.PatchCalls(func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + m.PatchCalls(func(_ context.Context, _ client.Object, _ client.Patch, _ ...client.PatchOption) error { return errTestClient }) }, @@ -291,8 +291,8 @@ func TestCertificateReconciliation(t *testing.T) { }{ { name: "successful apply when not found", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, nil }) }, @@ -301,8 +301,8 @@ func TestCertificateReconciliation(t *testing.T) { }, { name: "skip apply when existing matches desired", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { cert := getCertificateObject(testResourceLabels(), testResourceAnnotations()) cert.DeepCopyInto(obj.(*certmanagerv1.Certificate)) return true, nil @@ -313,8 +313,8 @@ func TestCertificateReconciliation(t *testing.T) { }, { name: "apply when existing has label drift", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { cert := getCertificateObject(testResourceLabels(), testResourceAnnotations()) cert.Labels["app.kubernetes.io/instance"] = "modified-value" cert.DeepCopyInto(obj.(*certmanagerv1.Certificate)) @@ -327,8 +327,8 @@ func TestCertificateReconciliation(t *testing.T) { { name: "apply when existing has annotation drift", tmBuilder: testTrustManager().WithAnnotations(map[string]string{"user-annotation": "original"}), - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { tm := testTrustManager().WithAnnotations(map[string]string{"user-annotation": "original"}).Build() cert := getCertificateObject(getResourceLabels(tm), getResourceAnnotations(tm)) cert.Annotations["user-annotation"] = "tampered" @@ -341,8 +341,8 @@ func TestCertificateReconciliation(t *testing.T) { }, { name: "apply when existing has secret name drift", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { cert := getCertificateObject(testResourceLabels(), testResourceAnnotations()) cert.Spec.SecretName = "wrong-secret" cert.DeepCopyInto(obj.(*certmanagerv1.Certificate)) @@ -354,8 +354,8 @@ func TestCertificateReconciliation(t *testing.T) { }, { name: "apply when existing has issuer ref drift", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { cert := getCertificateObject(testResourceLabels(), testResourceAnnotations()) cert.Spec.IssuerRef = certmanagermetav1.ObjectReference{ Name: "wrong-issuer", @@ -371,8 +371,8 @@ func TestCertificateReconciliation(t *testing.T) { }, { name: "exists error propagates", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, errTestClient }) }, @@ -382,11 +382,11 @@ func TestCertificateReconciliation(t *testing.T) { }, { name: "patch error propagates", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, nil }) - m.PatchCalls(func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + m.PatchCalls(func(_ context.Context, _ client.Object, _ client.Patch, _ ...client.PatchOption) error { return errTestClient }) }, diff --git a/pkg/controller/trustmanager/configmaps.go b/pkg/controller/trustmanager/configmaps.go index 9aa0c1c1e..1b19d555f 100644 --- a/pkg/controller/trustmanager/configmaps.go +++ b/pkg/controller/trustmanager/configmaps.go @@ -15,7 +15,7 @@ import ( "github.com/openshift/cert-manager-operator/pkg/controller/common" ) -// caPackage represents the JSON format expected by trust-manager +// caPackage represents the JSON format expected by trust-manager. type caPackage struct { Name string `json:"name"` Bundle string `json:"bundle"` @@ -86,6 +86,7 @@ func (r *Reconciler) readTrustedCABundle() (string, string, error) { caBundle, ok := injectionCM.Data[common.TrustedCABundleKey] if !ok || caBundle == "" { + //nolint:err113 // validation error with ConfigMap name and key for debugging return "", "", common.FromClientError( fmt.Errorf("CA bundle ConfigMap %q does not contain key %q", common.TrustedCABundleConfigMapName, common.TrustedCABundleKey), "missing key %q in ConfigMap %q", common.TrustedCABundleKey, common.TrustedCABundleConfigMapName, diff --git a/pkg/controller/trustmanager/configmaps_test.go b/pkg/controller/trustmanager/configmaps_test.go index 4629949b0..d81cdf933 100644 --- a/pkg/controller/trustmanager/configmaps_test.go +++ b/pkg/controller/trustmanager/configmaps_test.go @@ -160,7 +160,7 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { { name: "skips when policy is Disabled", tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyDisabled), - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + preReq: func(_ *Reconciler, _ *fakes.FakeCtrlClient) { }, wantExistsCount: 0, wantPatchCount: 0, @@ -168,7 +168,7 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { { name: "skips when policy is unset (defaults to Disabled)", tm: testTrustManager(), - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + preReq: func(_ *Reconciler, _ *fakes.FakeCtrlClient) { }, wantExistsCount: 0, wantPatchCount: 0, @@ -176,8 +176,8 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { { name: "returns error when injection ConfigMap is not found", tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled), - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) error { return errTestClient }) }, @@ -186,8 +186,8 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { { name: "returns error when CA bundle key is missing", tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled), - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) error { cm := obj.(*corev1.ConfigMap) cm.Data = map[string]string{} cm.ResourceVersion = "100" @@ -199,8 +199,8 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { { name: "returns error when CA bundle is empty", tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled), - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) error { cm := obj.(*corev1.ConfigMap) cm.Data = map[string]string{common.TrustedCABundleKey: ""} cm.ResourceVersion = "100" @@ -212,14 +212,14 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { { name: "creates ConfigMap and returns hash when bundle is available", tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled), - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) error { cm := obj.(*corev1.ConfigMap) cm.Data = map[string]string{common.TrustedCABundleKey: testCABundle} cm.ResourceVersion = "100" return nil }) - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, nil }) }, @@ -230,14 +230,14 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { { name: "skips patch when existing ConfigMap matches desired", tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled), - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) error { cm := obj.(*corev1.ConfigMap) cm.Data = map[string]string{common.TrustedCABundleKey: testCABundle} cm.ResourceVersion = "100" return nil }) - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { pkgJSON, _ := formatCAPackage(testCABundle, "100") cm := obj.(*corev1.ConfigMap) cm.Labels = testResourceLabels() @@ -252,14 +252,14 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { { name: "patches when existing ConfigMap data differs", tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled), - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) error { cm := obj.(*corev1.ConfigMap) cm.Data = map[string]string{common.TrustedCABundleKey: testCABundle} cm.ResourceVersion = "100" return nil }) - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { cm := obj.(*corev1.ConfigMap) cm.Data = map[string]string{defaultCAPackageFilename: `{"name":"stale"}`} return true, nil @@ -272,14 +272,14 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { { name: "propagates Exists error", tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled), - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) error { cm := obj.(*corev1.ConfigMap) cm.Data = map[string]string{common.TrustedCABundleKey: testCABundle} cm.ResourceVersion = "100" return nil }) - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, errTestClient }) }, @@ -289,17 +289,17 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { { name: "propagates Patch error", tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled), - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) error { cm := obj.(*corev1.ConfigMap) cm.Data = map[string]string{common.TrustedCABundleKey: testCABundle} cm.ResourceVersion = "100" return nil }) - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, nil }) - m.PatchCalls(func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + m.PatchCalls(func(_ context.Context, _ client.Object, _ client.Patch, _ ...client.PatchOption) error { return errTestClient }) }, diff --git a/pkg/controller/trustmanager/controller.go b/pkg/controller/trustmanager/controller.go index 9445542f9..f0294c608 100644 --- a/pkg/controller/trustmanager/controller.go +++ b/pkg/controller/trustmanager/controller.go @@ -79,7 +79,7 @@ func New(mgr ctrl.Manager) (*Reconciler, error) { func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { // mapFunc unconditionally enqueues the singleton TrustManager CR. // Filtering is handled by predicates attached to each Watch. - mapFunc := func(ctx context.Context, obj client.Object) []reconcile.Request { + mapFunc := func(_ context.Context, obj client.Object) []reconcile.Request { r.log.V(4).Info("received reconcile event", "object", fmt.Sprintf("%T", obj), "name", obj.GetName(), "namespace", obj.GetNamespace()) return []reconcile.Request{{NamespacedName: types.NamespacedName{Name: trustManagerObjectName}}} } @@ -210,6 +210,8 @@ func (r *Reconciler) processReconcileRequest(trustManager *v1alpha1.TrustManager } // cleanUp handles deletion of trustmanager.openshift.operator.io gracefully. +// +//nolint:unparam // error will be used when cleanup logic is implemented (see TODO below) func (r *Reconciler) cleanUp(trustManager *v1alpha1.TrustManager) (bool, error) { // TODO: For GA, handle cleaning up of resources created for installing trust-manager operand. // As per Non-Goals in the enhancement, removing the TrustManager CR will not remove the diff --git a/pkg/controller/trustmanager/controller_test.go b/pkg/controller/trustmanager/controller_test.go index 3efd817a0..8fea3ed99 100644 --- a/pkg/controller/trustmanager/controller_test.go +++ b/pkg/controller/trustmanager/controller_test.go @@ -27,16 +27,16 @@ func TestReconcile(t *testing.T) { }{ { name: "resource not found returns no error", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) error { return apierrors.NewNotFound(v1alpha1.Resource("trustmanager"), trustManagerObjectName) }) }, }, { name: "failed to fetch resource propagates error", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) error { switch obj.(type) { case *v1alpha1.TrustManager: return apierrors.NewBadRequest("test error") @@ -48,8 +48,8 @@ func TestReconcile(t *testing.T) { }, { name: "resource marked for deletion without finalizer", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.TrustManager: tm := testTrustManager().Build() @@ -62,8 +62,8 @@ func TestReconcile(t *testing.T) { }, { name: "remove finalizer fails on deletion", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.TrustManager: tm := testTrustManager().Build() @@ -73,7 +73,7 @@ func TestReconcile(t *testing.T) { } return nil }) - m.UpdateWithRetryCalls(func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + m.UpdateWithRetryCalls(func(_ context.Context, _ client.Object, _ ...client.UpdateOption) error { return errTestClient }) }, @@ -81,8 +81,8 @@ func TestReconcile(t *testing.T) { }, { name: "adding finalizer fails", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.TrustManager: tm := testTrustManager().Build() @@ -90,7 +90,7 @@ func TestReconcile(t *testing.T) { } return nil }) - m.UpdateWithRetryCalls(func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + m.UpdateWithRetryCalls(func(_ context.Context, _ client.Object, _ ...client.UpdateOption) error { return errTestClient }) }, @@ -98,8 +98,8 @@ func TestReconcile(t *testing.T) { }, { name: "status update failure", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.TrustManager: tm := testTrustManager().Build() @@ -109,7 +109,7 @@ func TestReconcile(t *testing.T) { } return nil }) - m.StatusUpdateCalls(func(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error { + m.StatusUpdateCalls(func(_ context.Context, _ client.Object, _ ...client.SubResourceUpdateOption) error { return apierrors.NewBadRequest("test error") }) }, @@ -151,8 +151,8 @@ func TestProcessReconcileRequest(t *testing.T) { getTrustManager: func() *v1alpha1.TrustManager { return testTrustManager().Build() }, - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.TrustManager: testTrustManager().Build().DeepCopyInto(o) @@ -161,7 +161,7 @@ func TestProcessReconcileRequest(t *testing.T) { }) // Namespace exists; all other resources return not-found so they // are created via SSA Patch (which succeeds by default). - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { switch obj.(type) { case *corev1.Namespace: return true, nil @@ -192,8 +192,8 @@ func TestProcessReconcileRequest(t *testing.T) { tm.Spec.TrustManagerConfig = v1alpha1.TrustManagerConfig{} return tm }, - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.TrustManager: tm := testTrustManager().Build() @@ -221,8 +221,8 @@ func TestProcessReconcileRequest(t *testing.T) { getTrustManager: func() *v1alpha1.TrustManager { return testTrustManager().Build() }, - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.TrustManager: testTrustManager().Build().DeepCopyInto(o) @@ -231,7 +231,7 @@ func TestProcessReconcileRequest(t *testing.T) { }) // Namespace Exists succeeds (passes validateTrustNamespace), but // ServiceAccount Exists returns a FromClientError - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { switch obj.(type) { case *corev1.Namespace: return true, nil @@ -258,8 +258,8 @@ func TestProcessReconcileRequest(t *testing.T) { getTrustManager: func() *v1alpha1.TrustManager { return testTrustManager().Build() }, - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.TrustManager: testTrustManager().Build().DeepCopyInto(o) @@ -267,7 +267,7 @@ func TestProcessReconcileRequest(t *testing.T) { return nil }) // Namespace does not exist - validateTrustNamespace will fail - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { switch obj.(type) { case *corev1.Namespace: return false, nil @@ -300,8 +300,8 @@ func TestProcessReconcileRequest(t *testing.T) { tm.Spec.TrustManagerConfig.TrustNamespace = "custom-trust-ns" return tm }, - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.TrustManager: tm := testTrustManager().Build() @@ -311,7 +311,7 @@ func TestProcessReconcileRequest(t *testing.T) { return nil }) // Custom namespace exists; so SSA Patch will create or update all resources successfully. - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { switch obj.(type) { case *corev1.Namespace: return true, nil diff --git a/pkg/controller/trustmanager/deployments.go b/pkg/controller/trustmanager/deployments.go index da96689d4..caad26b5d 100644 --- a/pkg/controller/trustmanager/deployments.go +++ b/pkg/controller/trustmanager/deployments.go @@ -99,6 +99,7 @@ func validateDeploymentManifest(deployment *appsv1.Deployment) error { } } if !hasContainer { + //nolint:err113 // validation error with container name for debugging return fmt.Errorf("deployment manifest missing required container %q", trustManagerContainerName) } @@ -110,6 +111,7 @@ func validateDeploymentManifest(deployment *appsv1.Deployment) error { } } if !hasVolume { + //nolint:err113 // validation error with volume name for debugging return fmt.Errorf("deployment manifest missing required secret volume %q", tlsVolumeName) } @@ -212,6 +214,7 @@ func updateDefaultCAPackageVolume(deployment *appsv1.Deployment, config v1alpha1 func updateImage(deployment *appsv1.Deployment) error { image := os.Getenv(trustManagerImageNameEnvVarName) if image == "" { + //nolint:err113 // environment variable name included for debugging return fmt.Errorf("%s environment variable with trust-manager image not set", trustManagerImageNameEnvVarName) } for i, container := range deployment.Spec.Template.Spec.Containers { diff --git a/pkg/controller/trustmanager/deployments_test.go b/pkg/controller/trustmanager/deployments_test.go index 6841031b8..77c0d1680 100644 --- a/pkg/controller/trustmanager/deployments_test.go +++ b/pkg/controller/trustmanager/deployments_test.go @@ -435,8 +435,8 @@ func TestDeploymentReconciliation(t *testing.T) { { name: "successful apply when not found", setImage: true, - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, nil }) }, @@ -447,7 +447,7 @@ func TestDeploymentReconciliation(t *testing.T) { name: "skip apply when existing matches desired", setImage: true, preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { dep, err := r.getDeploymentObject(testTrustManager().Build(), testResourceLabels(), testResourceAnnotations(), "") if err != nil { t.Fatalf("unexpected error building desired deployment: %v", err) @@ -463,7 +463,7 @@ func TestDeploymentReconciliation(t *testing.T) { name: "apply when existing has label drift", setImage: true, preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { dep, err := r.getDeploymentObject(testTrustManager().Build(), testResourceLabels(), testResourceAnnotations(), "") if err != nil { t.Fatalf("unexpected error: %v", err) @@ -481,7 +481,7 @@ func TestDeploymentReconciliation(t *testing.T) { tmBuilder: testTrustManager().WithAnnotations(map[string]string{"user-annotation": "original"}), setImage: true, preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { tm := testTrustManager().WithAnnotations(map[string]string{"user-annotation": "original"}).Build() dep, err := r.getDeploymentObject(tm, getResourceLabels(tm), getResourceAnnotations(tm), "") if err != nil { @@ -499,7 +499,7 @@ func TestDeploymentReconciliation(t *testing.T) { name: "apply when defaultCAPackage changed from Enabled to Disabled", setImage: true, preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { tm := testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled).Build() dep, err := r.getDeploymentObject(tm, testResourceLabels(), testResourceAnnotations(), "abc123hash") if err != nil { @@ -518,7 +518,7 @@ func TestDeploymentReconciliation(t *testing.T) { caBundleHash: "abc123hash", setImage: true, preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { tm := testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled).Build() dep, err := r.getDeploymentObject(tm, testResourceLabels(), testResourceAnnotations(), "abc123hash") if err != nil { @@ -536,7 +536,7 @@ func TestDeploymentReconciliation(t *testing.T) { name: "apply when existing has image drift", setImage: true, preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { dep, err := r.getDeploymentObject(testTrustManager().Build(), testResourceLabels(), testResourceAnnotations(), "") if err != nil { t.Fatalf("unexpected error: %v", err) @@ -553,7 +553,7 @@ func TestDeploymentReconciliation(t *testing.T) { name: "apply when existing has replicas drift", setImage: true, preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { dep, err := r.getDeploymentObject(testTrustManager().Build(), testResourceLabels(), testResourceAnnotations(), "") if err != nil { t.Fatalf("unexpected error: %v", err) @@ -570,7 +570,7 @@ func TestDeploymentReconciliation(t *testing.T) { name: "apply when existing has args drift", setImage: true, preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { dep, err := r.getDeploymentObject(testTrustManager().Build(), testResourceLabels(), testResourceAnnotations(), "") if err != nil { t.Fatalf("unexpected error: %v", err) @@ -588,8 +588,8 @@ func TestDeploymentReconciliation(t *testing.T) { { name: "exists error propagates", setImage: true, - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, errTestClient }) }, @@ -600,11 +600,11 @@ func TestDeploymentReconciliation(t *testing.T) { { name: "patch error propagates", setImage: true, - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, nil }) - m.PatchCalls(func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + m.PatchCalls(func(_ context.Context, _ client.Object, _ client.Patch, _ ...client.PatchOption) error { return errTestClient }) }, diff --git a/pkg/controller/trustmanager/install_trustmanager.go b/pkg/controller/trustmanager/install_trustmanager.go index 932253355..0743d032d 100644 --- a/pkg/controller/trustmanager/install_trustmanager.go +++ b/pkg/controller/trustmanager/install_trustmanager.go @@ -77,6 +77,7 @@ func (r *Reconciler) validateTrustNamespace(namespace string) error { return fmt.Errorf("failed to check if namespace %q exists: %w", namespace, err) } if !exists { + //nolint:err113 // user-facing validation error with namespace name return fmt.Errorf("trust namespace %q does not exist, create the namespace before creating TrustManager CR", namespace) } return nil diff --git a/pkg/controller/trustmanager/install_trustmanager_test.go b/pkg/controller/trustmanager/install_trustmanager_test.go index 437022391..bc19b9b6b 100644 --- a/pkg/controller/trustmanager/install_trustmanager_test.go +++ b/pkg/controller/trustmanager/install_trustmanager_test.go @@ -78,14 +78,14 @@ func TestUpdateStatusObservedState(t *testing.T) { mock := &fakes.FakeCtrlClient{} tm := tt.trustManager() - mock.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + mock.GetCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) error { switch o := obj.(type) { case *v1alpha1.TrustManager: tm.DeepCopyInto(o) } return nil }) - mock.StatusUpdateCalls(func(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error { + mock.StatusUpdateCalls(func(_ context.Context, _ client.Object, _ ...client.SubResourceUpdateOption) error { return nil }) r.CtrlClient = mock diff --git a/pkg/controller/trustmanager/rbacs.go b/pkg/controller/trustmanager/rbacs.go index d8c3bcc04..3d86ad918 100644 --- a/pkg/controller/trustmanager/rbacs.go +++ b/pkg/controller/trustmanager/rbacs.go @@ -1,13 +1,10 @@ package trustmanager import ( - "fmt" "reflect" "slices" - corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" - "sigs.k8s.io/controller-runtime/pkg/client" "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" "github.com/openshift/cert-manager-operator/pkg/controller/common" @@ -52,26 +49,10 @@ func (r *Reconciler) createOrApplyRBACResources(trustManager *v1alpha1.TrustMana func (r *Reconciler) createOrApplyClusterRole(trustManager *v1alpha1.TrustManager, resourceLabels, resourceAnnotations map[string]string) error { desired := getClusterRoleObject(trustManager.Spec.TrustManagerConfig.SecretTargets, resourceLabels, resourceAnnotations) - resourceName := desired.GetName() - r.log.V(4).Info("reconciling clusterrole resource", "name", resourceName) - existing := &rbacv1.ClusterRole{} - exists, err := r.Exists(r.ctx, client.ObjectKeyFromObject(desired), existing) - if err != nil { - return common.FromClientError(err, "failed to check if clusterrole %q exists", resourceName) - } - if exists && !clusterRoleModified(desired, existing) { - r.log.V(4).Info("clusterrole resource exists and is in desired state", "name", resourceName) - return nil - } - - r.log.V(2).Info("clusterrole 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 clusterrole %q", resourceName) - } - - r.eventRecorder.Eventf(trustManager, corev1.EventTypeNormal, "Reconciled", "clusterrole resource %s applied", resourceName) - return nil + return r.reconcileResourceWithSSA(trustManager, desired, existing, "clusterrole", func() bool { + return clusterRoleModified(desired, existing) + }) } func getClusterRoleObject(secretTargets v1alpha1.SecretTargetsConfig, resourceLabels, resourceAnnotations map[string]string) *rbacv1.ClusterRole { @@ -112,26 +93,10 @@ func appendSecretTargetRules(clusterRole *rbacv1.ClusterRole, secretTargets v1al func (r *Reconciler) createOrApplyClusterRoleBinding(trustManager *v1alpha1.TrustManager, resourceLabels, resourceAnnotations map[string]string) error { desired := getClusterRoleBindingObject(resourceLabels, resourceAnnotations) - resourceName := desired.GetName() - r.log.V(4).Info("reconciling clusterrolebinding resource", "name", resourceName) - existing := &rbacv1.ClusterRoleBinding{} - exists, err := r.Exists(r.ctx, client.ObjectKeyFromObject(desired), existing) - if err != nil { - return common.FromClientError(err, "failed to check if clusterrolebinding %q exists", resourceName) - } - if exists && !clusterRoleBindingModified(desired, existing) { - r.log.V(4).Info("clusterrolebinding resource exists and is in desired state", "name", resourceName) - return nil - } - - r.log.V(2).Info("clusterrolebinding 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 clusterrolebinding %q", resourceName) - } - - r.eventRecorder.Eventf(trustManager, corev1.EventTypeNormal, "Reconciled", "clusterrolebinding resource %s applied", resourceName) - return nil + return r.reconcileResourceWithSSA(trustManager, desired, existing, "clusterrolebinding", func() bool { + return clusterRoleBindingModified(desired, existing) + }) } func getClusterRoleBindingObject(resourceLabels, resourceAnnotations map[string]string) *rbacv1.ClusterRoleBinding { @@ -148,26 +113,10 @@ func getClusterRoleBindingObject(resourceLabels, resourceAnnotations map[string] func (r *Reconciler) createOrApplyTrustNamespaceRole(trustManager *v1alpha1.TrustManager, resourceLabels, resourceAnnotations map[string]string, trustNamespace string) error { desired := getTrustNamespaceRoleObject(resourceLabels, resourceAnnotations, trustNamespace) - resourceName := fmt.Sprintf("%s/%s", desired.GetNamespace(), desired.GetName()) - r.log.V(4).Info("reconciling role resource for trust namespace", "name", resourceName) - existing := &rbacv1.Role{} - exists, err := r.Exists(r.ctx, client.ObjectKeyFromObject(desired), existing) - if err != nil { - return common.FromClientError(err, "failed to check if role %q exists", resourceName) - } - if exists && !roleModified(desired, existing) { - r.log.V(4).Info("role resource exists and is in desired state", "name", resourceName) - return nil - } - - r.log.V(2).Info("role 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 role %q", resourceName) - } - - r.eventRecorder.Eventf(trustManager, corev1.EventTypeNormal, "Reconciled", "role resource %s applied", resourceName) - return nil + return r.reconcileResourceWithSSA(trustManager, desired, existing, "role", func() bool { + return roleModified(desired, existing) + }) } func getTrustNamespaceRoleObject(resourceLabels, resourceAnnotations map[string]string, trustNamespace string) *rbacv1.Role { @@ -183,26 +132,10 @@ func getTrustNamespaceRoleObject(resourceLabels, resourceAnnotations map[string] func (r *Reconciler) createOrApplyTrustNamespaceRoleBinding(trustManager *v1alpha1.TrustManager, resourceLabels, resourceAnnotations map[string]string, trustNamespace string) error { desired := getTrustNamespaceRoleBindingObject(resourceLabels, resourceAnnotations, trustNamespace) - resourceName := fmt.Sprintf("%s/%s", desired.GetNamespace(), desired.GetName()) - r.log.V(4).Info("reconciling rolebinding resource for trust namespace", "name", resourceName) - existing := &rbacv1.RoleBinding{} - exists, err := r.Exists(r.ctx, client.ObjectKeyFromObject(desired), existing) - if err != nil { - return common.FromClientError(err, "failed to check if rolebinding %q exists", resourceName) - } - if exists && !roleBindingModified(desired, existing) { - r.log.V(4).Info("rolebinding resource exists and is in desired state", "name", resourceName) - return nil - } - - r.log.V(2).Info("rolebinding 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 rolebinding %q", resourceName) - } - - r.eventRecorder.Eventf(trustManager, corev1.EventTypeNormal, "Reconciled", "rolebinding resource %s applied", resourceName) - return nil + return r.reconcileResourceWithSSA(trustManager, desired, existing, "rolebinding", func() bool { + return roleBindingModified(desired, existing) + }) } func getTrustNamespaceRoleBindingObject(resourceLabels, resourceAnnotations map[string]string, trustNamespace string) *rbacv1.RoleBinding { @@ -220,26 +153,10 @@ func getTrustNamespaceRoleBindingObject(resourceLabels, resourceAnnotations map[ func (r *Reconciler) createOrApplyLeaderElectionRole(trustManager *v1alpha1.TrustManager, resourceLabels, resourceAnnotations map[string]string) error { desired := getLeaderElectionRoleObject(resourceLabels, resourceAnnotations) - resourceName := fmt.Sprintf("%s/%s", desired.GetNamespace(), desired.GetName()) - r.log.V(4).Info("reconciling leader election role resource", "name", resourceName) - existing := &rbacv1.Role{} - exists, err := r.Exists(r.ctx, client.ObjectKeyFromObject(desired), existing) - if err != nil { - return common.FromClientError(err, "failed to check if leader election role %q exists", resourceName) - } - if exists && !roleModified(desired, existing) { - r.log.V(4).Info("leader election role resource exists and is in desired state", "name", resourceName) - return nil - } - - r.log.V(2).Info("leader election role 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 leader election role %q", resourceName) - } - - r.eventRecorder.Eventf(trustManager, corev1.EventTypeNormal, "Reconciled", "leader election role resource %s applied", resourceName) - return nil + return r.reconcileResourceWithSSA(trustManager, desired, existing, "leader election role", func() bool { + return roleModified(desired, existing) + }) } func getLeaderElectionRoleObject(resourceLabels, resourceAnnotations map[string]string) *rbacv1.Role { @@ -255,26 +172,10 @@ func getLeaderElectionRoleObject(resourceLabels, resourceAnnotations map[string] func (r *Reconciler) createOrApplyLeaderElectionRoleBinding(trustManager *v1alpha1.TrustManager, resourceLabels, resourceAnnotations map[string]string) error { desired := getLeaderElectionRoleBindingObject(resourceLabels, resourceAnnotations) - resourceName := fmt.Sprintf("%s/%s", desired.GetNamespace(), desired.GetName()) - r.log.V(4).Info("reconciling leader election rolebinding resource", "name", resourceName) - existing := &rbacv1.RoleBinding{} - exists, err := r.Exists(r.ctx, client.ObjectKeyFromObject(desired), existing) - if err != nil { - return common.FromClientError(err, "failed to check if leader election rolebinding %q exists", resourceName) - } - if exists && !roleBindingModified(desired, existing) { - r.log.V(4).Info("leader election rolebinding resource exists and is in desired state", "name", resourceName) - return nil - } - - r.log.V(2).Info("leader election rolebinding 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 leader election rolebinding %q", resourceName) - } - - r.eventRecorder.Eventf(trustManager, corev1.EventTypeNormal, "Reconciled", "leader election rolebinding resource %s applied", resourceName) - return nil + return r.reconcileResourceWithSSA(trustManager, desired, existing, "leader election rolebinding", func() bool { + return roleBindingModified(desired, existing) + }) } func getLeaderElectionRoleBindingObject(resourceLabels, resourceAnnotations map[string]string) *rbacv1.RoleBinding { diff --git a/pkg/controller/trustmanager/rbacs_test.go b/pkg/controller/trustmanager/rbacs_test.go index 634bffc30..12424b383 100644 --- a/pkg/controller/trustmanager/rbacs_test.go +++ b/pkg/controller/trustmanager/rbacs_test.go @@ -334,8 +334,8 @@ func TestRBACReconciliation(t *testing.T) { }{ { name: "successful apply of all RBAC resources", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, nil }) }, @@ -344,9 +344,9 @@ func TestRBACReconciliation(t *testing.T) { }, { name: "skip apply when all RBAC resources match desired", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { existsCall := 0 - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { existsCall++ switch existsCall { case 1: @@ -376,9 +376,9 @@ func TestRBACReconciliation(t *testing.T) { }, { name: "apply when cluster role has label drift", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { existsCall := 0 - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { existsCall++ switch existsCall { case 1: @@ -410,12 +410,12 @@ func TestRBACReconciliation(t *testing.T) { { name: "apply when cluster role binding has annotation drift", tmBuilder: testTrustManager().WithAnnotations(map[string]string{"user-annotation": "original"}), - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { tm := testTrustManager().WithAnnotations(map[string]string{"user-annotation": "original"}).Build() labels := getResourceLabels(tm) annotations := getResourceAnnotations(tm) existsCall := 0 - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { existsCall++ switch existsCall { case 1: @@ -446,9 +446,9 @@ func TestRBACReconciliation(t *testing.T) { }, { name: "apply when role has rules drift", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { existsCall := 0 - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { existsCall++ switch existsCall { case 1: @@ -481,8 +481,8 @@ func TestRBACReconciliation(t *testing.T) { }, { name: "exists error propagates on first resource", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, errTestClient }) }, @@ -492,11 +492,11 @@ func TestRBACReconciliation(t *testing.T) { }, { name: "clusterrole patch error propagates", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, nil }) - m.PatchCalls(func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + m.PatchCalls(func(_ context.Context, obj client.Object, _ client.Patch, _ ...client.PatchOption) error { if _, ok := obj.(*rbacv1.ClusterRole); ok { return errTestClient } @@ -509,11 +509,11 @@ func TestRBACReconciliation(t *testing.T) { }, { name: "clusterrolebinding patch error propagates", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, nil }) - m.PatchCalls(func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + m.PatchCalls(func(_ context.Context, obj client.Object, _ client.Patch, _ ...client.PatchOption) error { if _, ok := obj.(*rbacv1.ClusterRoleBinding); ok { return errTestClient } @@ -526,11 +526,11 @@ func TestRBACReconciliation(t *testing.T) { }, { name: "role patch error propagates", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, nil }) - m.PatchCalls(func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + m.PatchCalls(func(_ context.Context, obj client.Object, _ client.Patch, _ ...client.PatchOption) error { if _, ok := obj.(*rbacv1.Role); ok { return errTestClient } @@ -543,11 +543,11 @@ func TestRBACReconciliation(t *testing.T) { }, { name: "rolebinding patch error propagates", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, nil }) - m.PatchCalls(func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + m.PatchCalls(func(_ context.Context, obj client.Object, _ client.Patch, _ ...client.PatchOption) error { if _, ok := obj.(*rbacv1.RoleBinding); ok { return errTestClient } @@ -669,9 +669,9 @@ func TestRBACReconciliationWithSecretTargets(t *testing.T) { { name: "apply when secretTargets policy changes from Disabled to Custom", tmBuilder: testTrustManager().WithSecretTargets(v1alpha1.SecretTargetsPolicyCustom, []string{"bundle-secret"}), - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient, tm *v1alpha1.TrustManager) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient, _ *v1alpha1.TrustManager) { existsCall := 0 - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { existsCall++ switch existsCall { case 1: @@ -703,10 +703,10 @@ func TestRBACReconciliationWithSecretTargets(t *testing.T) { { name: "skip apply when secretTargets rules already match", tmBuilder: testTrustManager().WithSecretTargets(v1alpha1.SecretTargetsPolicyCustom, []string{"bundle-secret"}), - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient, tm *v1alpha1.TrustManager) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient, tm *v1alpha1.TrustManager) { secretTargets := tm.Spec.TrustManagerConfig.SecretTargets existsCall := 0 - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { existsCall++ switch existsCall { case 1: @@ -737,13 +737,13 @@ func TestRBACReconciliationWithSecretTargets(t *testing.T) { { name: "apply when authorizedSecrets list changes", tmBuilder: testTrustManager().WithSecretTargets(v1alpha1.SecretTargetsPolicyCustom, []string{"new-secret"}), - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient, tm *v1alpha1.TrustManager) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient, _ *v1alpha1.TrustManager) { oldSecretTargets := v1alpha1.SecretTargetsConfig{ Policy: v1alpha1.SecretTargetsPolicyCustom, AuthorizedSecrets: []string{"old-secret"}, } existsCall := 0 - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { existsCall++ switch existsCall { case 1: diff --git a/pkg/controller/trustmanager/serviceaccounts_test.go b/pkg/controller/trustmanager/serviceaccounts_test.go index 70c119582..c0db66ef4 100644 --- a/pkg/controller/trustmanager/serviceaccounts_test.go +++ b/pkg/controller/trustmanager/serviceaccounts_test.go @@ -81,8 +81,8 @@ func TestServiceAccountReconciliation(t *testing.T) { }{ { name: "successful apply when not found", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, nil }) }, @@ -92,7 +92,7 @@ func TestServiceAccountReconciliation(t *testing.T) { { name: "skip apply when existing matches desired", preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { sa := r.getServiceAccountObject(testResourceLabels(), testResourceAnnotations()) sa.DeepCopyInto(obj.(*corev1.ServiceAccount)) return true, nil @@ -104,7 +104,7 @@ func TestServiceAccountReconciliation(t *testing.T) { { name: "apply when existing has label drift", preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { sa := r.getServiceAccountObject(testResourceLabels(), testResourceAnnotations()) sa.Labels["app.kubernetes.io/instance"] = "modified-value" sa.DeepCopyInto(obj.(*corev1.ServiceAccount)) @@ -118,7 +118,7 @@ func TestServiceAccountReconciliation(t *testing.T) { name: "apply when existing has annotation drift", tmBuilder: testTrustManager().WithAnnotations(map[string]string{"user-annotation": "original"}), preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { tm := testTrustManager().WithAnnotations(map[string]string{"user-annotation": "original"}).Build() sa := r.getServiceAccountObject(getResourceLabels(tm), getResourceAnnotations(tm)) sa.Annotations["user-annotation"] = "tampered" @@ -132,7 +132,7 @@ func TestServiceAccountReconciliation(t *testing.T) { { name: "apply when automountServiceAccountToken is modified", preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { sa := r.getServiceAccountObject(testResourceLabels(), testResourceAnnotations()) sa.AutomountServiceAccountToken = ptr.To(false) sa.DeepCopyInto(obj.(*corev1.ServiceAccount)) @@ -144,8 +144,8 @@ func TestServiceAccountReconciliation(t *testing.T) { }, { name: "exists error propagates", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, errTestClient }) }, @@ -155,11 +155,11 @@ func TestServiceAccountReconciliation(t *testing.T) { }, { name: "patch error propagates", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, nil }) - m.PatchCalls(func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + m.PatchCalls(func(_ context.Context, _ client.Object, _ client.Patch, _ ...client.PatchOption) error { return errTestClient }) }, diff --git a/pkg/controller/trustmanager/services_test.go b/pkg/controller/trustmanager/services_test.go index 4c7432370..48d479b0e 100644 --- a/pkg/controller/trustmanager/services_test.go +++ b/pkg/controller/trustmanager/services_test.go @@ -91,8 +91,8 @@ func TestServiceReconciliation(t *testing.T) { }{ { name: "successful apply of both services", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, nil }) }, @@ -101,9 +101,9 @@ func TestServiceReconciliation(t *testing.T) { }, { name: "skip apply when both services match desired", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { existsCall := 0 - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { existsCall++ var svc *corev1.Service if existsCall == 1 { @@ -120,9 +120,9 @@ func TestServiceReconciliation(t *testing.T) { }, { name: "apply when existing has label drift", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { existsCall := 0 - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { existsCall++ if existsCall == 1 { svc := getWebhookServiceObject(testResourceLabels(), testResourceAnnotations()) @@ -141,12 +141,12 @@ func TestServiceReconciliation(t *testing.T) { { name: "apply when existing has annotation drift", tmBuilder: testTrustManager().WithAnnotations(map[string]string{"user-annotation": "original"}), - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { tm := testTrustManager().WithAnnotations(map[string]string{"user-annotation": "original"}).Build() labels := getResourceLabels(tm) annotations := getResourceAnnotations(tm) existsCall := 0 - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { existsCall++ if existsCall == 1 { svc := getWebhookServiceObject(labels, annotations) @@ -164,9 +164,9 @@ func TestServiceReconciliation(t *testing.T) { }, { name: "apply when existing has port drift", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { existsCall := 0 - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { existsCall++ if existsCall == 1 { svc := getWebhookServiceObject(testResourceLabels(), testResourceAnnotations()) @@ -184,9 +184,9 @@ func TestServiceReconciliation(t *testing.T) { }, { name: "apply when existing has selector drift", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { existsCall := 0 - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { existsCall++ if existsCall == 1 { svc := getWebhookServiceObject(testResourceLabels(), testResourceAnnotations()) @@ -204,8 +204,8 @@ func TestServiceReconciliation(t *testing.T) { }, { name: "exists error propagates on first service", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, errTestClient }) }, @@ -215,11 +215,11 @@ func TestServiceReconciliation(t *testing.T) { }, { name: "webhook service patch error propagates", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, nil }) - m.PatchCalls(func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + m.PatchCalls(func(_ context.Context, _ client.Object, _ client.Patch, _ ...client.PatchOption) error { return errTestClient }) }, @@ -229,12 +229,12 @@ func TestServiceReconciliation(t *testing.T) { }, { name: "metrics service patch error propagates on second call", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, nil }) callCount := 0 - m.PatchCalls(func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + m.PatchCalls(func(_ context.Context, _ client.Object, _ client.Patch, _ ...client.PatchOption) error { callCount++ if callCount == 2 { return errTestClient diff --git a/pkg/controller/trustmanager/utils.go b/pkg/controller/trustmanager/utils.go index 3e4507959..0c793d35d 100644 --- a/pkg/controller/trustmanager/utils.go +++ b/pkg/controller/trustmanager/utils.go @@ -74,6 +74,7 @@ func (r *Reconciler) addFinalizer(ctx context.Context, trustManager *v1alpha1.Tr namespacedName := client.ObjectKeyFromObject(trustManager) if !controllerutil.ContainsFinalizer(trustManager, finalizer) { if !controllerutil.AddFinalizer(trustManager, finalizer) { + //nolint:err113 // finalizer and namespacedName included for debugging return fmt.Errorf("failed to add finalizer %q on trustmanager.openshift.operator.io %q", finalizer, namespacedName) } @@ -97,6 +98,7 @@ func (r *Reconciler) removeFinalizer(ctx context.Context, trustManager *v1alpha1 namespacedName := client.ObjectKeyFromObject(trustManager) if controllerutil.ContainsFinalizer(trustManager, finalizer) { if !controllerutil.RemoveFinalizer(trustManager, finalizer) { + //nolint:err113 // finalizer and namespacedName included for debugging return fmt.Errorf("failed to remove finalizer %q from trustmanager.openshift.operator.io %q", finalizer, namespacedName) } @@ -111,6 +113,7 @@ func (r *Reconciler) removeFinalizer(ctx context.Context, trustManager *v1alpha1 func validateTrustManagerConfig(trustManager *v1alpha1.TrustManager) error { if reflect.ValueOf(trustManager.Spec.TrustManagerConfig).IsZero() { + //nolint:err113 // user-facing validation error message return fmt.Errorf("spec.trustManagerConfig config cannot be empty") } @@ -230,3 +233,39 @@ func (r *Reconciler) namespaceExists(namespace string) (bool, error) { key := client.ObjectKey{Name: namespace} return r.Exists(r.ctx, key, ns) } + +// reconcileResourceWithSSA is a generic helper for reconciling Kubernetes resources using Server-Side Apply. +// It checks if the resource exists and has been modified, then applies changes if needed. +// The modifiedCheck function should compare desired vs existing and return true if different. +func (r *Reconciler) reconcileResourceWithSSA( + trustManager *v1alpha1.TrustManager, + desired, existing client.Object, + resourceKind string, + modifiedCheck func() bool, +) error { + var resourceName string + if desired.GetNamespace() != "" { + resourceName = fmt.Sprintf("%s/%s", desired.GetNamespace(), desired.GetName()) + } else { + resourceName = desired.GetName() + } + + r.log.V(4).Info("reconciling "+resourceKind+" resource", "name", resourceName) + + exists, err := r.Exists(r.ctx, client.ObjectKeyFromObject(desired), existing) + if err != nil { + return common.FromClientError(err, "failed to check if %s %q exists", resourceKind, resourceName) + } + if exists && !modifiedCheck() { + r.log.V(4).Info(resourceKind+" resource exists and is in desired state", "name", resourceName) + return nil + } + + r.log.V(2).Info(resourceKind+" 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 %s %q", resourceKind, resourceName) + } + + r.eventRecorder.Eventf(trustManager, corev1.EventTypeNormal, "Reconciled", "%s resource %s applied", resourceKind, resourceName) + return nil +} diff --git a/pkg/controller/trustmanager/utils_test.go b/pkg/controller/trustmanager/utils_test.go index a113ace02..14f53cec9 100644 --- a/pkg/controller/trustmanager/utils_test.go +++ b/pkg/controller/trustmanager/utils_test.go @@ -322,7 +322,7 @@ func TestDecodeServiceAccountObjBytes(t *testing.T) { }, { name: "invalid bytes panics", - getBytes: func(t *testing.T) []byte { + getBytes: func(_ *testing.T) []byte { return []byte("not valid yaml or json") }, wantPanic: true, diff --git a/pkg/controller/trustmanager/webhooks.go b/pkg/controller/trustmanager/webhooks.go index 51e85cd7f..0dfc4b010 100644 --- a/pkg/controller/trustmanager/webhooks.go +++ b/pkg/controller/trustmanager/webhooks.go @@ -6,9 +6,7 @@ import ( "reflect" "slices" - corev1 "k8s.io/api/core/v1" "k8s.io/utils/ptr" - "sigs.k8s.io/controller-runtime/pkg/client" "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" "github.com/openshift/cert-manager-operator/pkg/controller/common" @@ -19,26 +17,10 @@ import ( func (r *Reconciler) createOrApplyValidatingWebhookConfiguration(trustManager *v1alpha1.TrustManager, resourceLabels, resourceAnnotations map[string]string) error { desired := getValidatingWebhookConfigObject(resourceLabels, resourceAnnotations) - resourceName := desired.GetName() - r.log.V(4).Info("reconciling validatingwebhookconfiguration resource", "name", resourceName) - existing := &admissionregistrationv1.ValidatingWebhookConfiguration{} - exists, err := r.Exists(r.ctx, client.ObjectKeyFromObject(desired), existing) - if err != nil { - return common.FromClientError(err, "failed to check if validatingwebhookconfiguration %q exists", resourceName) - } - if exists && !webhookConfigModified(desired, existing) { - r.log.V(4).Info("validatingwebhookconfiguration resource exists and is in desired state", "name", resourceName) - return nil - } - - r.log.V(2).Info("validatingwebhookconfiguration 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 validatingwebhookconfiguration %q", resourceName) - } - - r.eventRecorder.Eventf(trustManager, corev1.EventTypeNormal, "Reconciled", "validatingwebhookconfiguration resource %s applied", resourceName) - return nil + return r.reconcileResourceWithSSA(trustManager, desired, existing, "validatingwebhookconfiguration", func() bool { + return webhookConfigModified(desired, existing) + }) } func getValidatingWebhookConfigObject(resourceLabels, resourceAnnotations map[string]string) *admissionregistrationv1.ValidatingWebhookConfiguration { diff --git a/pkg/controller/trustmanager/webhooks_test.go b/pkg/controller/trustmanager/webhooks_test.go index 17841c685..161ae1062 100644 --- a/pkg/controller/trustmanager/webhooks_test.go +++ b/pkg/controller/trustmanager/webhooks_test.go @@ -121,8 +121,8 @@ func TestValidatingWebhookConfigReconciliation(t *testing.T) { }{ { name: "successful apply when not found", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, nil }) }, @@ -131,8 +131,8 @@ func TestValidatingWebhookConfigReconciliation(t *testing.T) { }, { name: "skip apply when existing matches desired", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { vwc := getValidatingWebhookConfigObject(testResourceLabels(), testResourceAnnotations()) vwc.DeepCopyInto(obj.(*admissionregistrationv1.ValidatingWebhookConfiguration)) return true, nil @@ -143,8 +143,8 @@ func TestValidatingWebhookConfigReconciliation(t *testing.T) { }, { name: "apply when existing has label drift", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { vwc := getValidatingWebhookConfigObject(testResourceLabels(), testResourceAnnotations()) vwc.Labels["app.kubernetes.io/instance"] = "modified-value" vwc.DeepCopyInto(obj.(*admissionregistrationv1.ValidatingWebhookConfiguration)) @@ -157,11 +157,11 @@ func TestValidatingWebhookConfigReconciliation(t *testing.T) { { name: "apply when existing has annotation drift", tmBuilder: testTrustManager().WithAnnotations(map[string]string{"user-annotation": "original"}), - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { tm := testTrustManager().WithAnnotations(map[string]string{"user-annotation": "original"}).Build() labels := getResourceLabels(tm) annotations := getResourceAnnotations(tm) - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { vwc := getValidatingWebhookConfigObject(labels, annotations) vwc.Annotations["user-annotation"] = "tampered" vwc.DeepCopyInto(obj.(*admissionregistrationv1.ValidatingWebhookConfiguration)) @@ -173,8 +173,8 @@ func TestValidatingWebhookConfigReconciliation(t *testing.T) { }, { name: "apply when existing has service reference drift", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { vwc := getValidatingWebhookConfigObject(testResourceLabels(), testResourceAnnotations()) vwc.Webhooks[0].ClientConfig.Service.Name = "wrong-service" vwc.DeepCopyInto(obj.(*admissionregistrationv1.ValidatingWebhookConfiguration)) @@ -186,8 +186,8 @@ func TestValidatingWebhookConfigReconciliation(t *testing.T) { }, { name: "apply when existing has failure policy drift", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, obj client.Object) (bool, error) { vwc := getValidatingWebhookConfigObject(testResourceLabels(), testResourceAnnotations()) vwc.Webhooks[0].FailurePolicy = ptr.To(admissionregistrationv1.Ignore) vwc.DeepCopyInto(obj.(*admissionregistrationv1.ValidatingWebhookConfiguration)) @@ -199,8 +199,8 @@ func TestValidatingWebhookConfigReconciliation(t *testing.T) { }, { name: "exists error propagates", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, errTestClient }) }, @@ -210,11 +210,11 @@ func TestValidatingWebhookConfigReconciliation(t *testing.T) { }, { name: "patch error propagates", - preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { - m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + preReq: func(_ *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(_ context.Context, _ client.ObjectKey, _ client.Object) (bool, error) { return false, nil }) - m.PatchCalls(func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + m.PatchCalls(func(_ context.Context, _ client.Object, _ client.Patch, _ ...client.PatchOption) error { return errTestClient }) }, diff --git a/pkg/features/features_test.go b/pkg/features/features_test.go index 7530ebd64..8d7f760fb 100644 --- a/pkg/features/features_test.go +++ b/pkg/features/features_test.go @@ -1,3 +1,4 @@ +//nolint:err113 // test file uses dynamic errors for simulating failures package features import ( diff --git a/pkg/operator/operatorclient/operatorclient.go b/pkg/operator/operatorclient/operatorclient.go index 3a72aceb0..bd8406a3c 100644 --- a/pkg/operator/operatorclient/operatorclient.go +++ b/pkg/operator/operatorclient/operatorclient.go @@ -32,12 +32,13 @@ type OperatorClient struct { var _ v1helpers.OperatorClient = &OperatorClient{} -func (c OperatorClient) ApplyOperatorSpec(ctx context.Context, fieldManager string, applyConfiguration *applyoperatorv1.OperatorSpecApplyConfiguration) (err error) { +func (c OperatorClient) ApplyOperatorSpec(_ context.Context, _ string, _ *applyoperatorv1.OperatorSpecApplyConfiguration) (err error) { return nil } func (c OperatorClient) ApplyOperatorStatus(ctx context.Context, fieldManager string, desiredConfiguration *applyoperatorv1.OperatorStatusApplyConfiguration) (err error) { if desiredConfiguration == nil { + //nolint:err113 // validation error message return fmt.Errorf("applyConfiguration must have a value") } @@ -117,7 +118,7 @@ func (c OperatorClient) ApplyOperatorStatus(ctx context.Context, fieldManager st return nil } -func (c OperatorClient) PatchOperatorStatus(ctx context.Context, jsonPatch *jsonpatch.PatchSet) (err error) { +func (c OperatorClient) PatchOperatorStatus(_ context.Context, _ *jsonpatch.PatchSet) (err error) { return nil } diff --git a/pkg/operator/starter.go b/pkg/operator/starter.go index dc1541c9d..c1a67592b 100644 --- a/pkg/operator/starter.go +++ b/pkg/operator/starter.go @@ -98,7 +98,7 @@ func RunOperator(ctx context.Context, cc *controllercmd.ControllerContext) error return fmt.Errorf("failed to discover Infrastructure presence: %w", err) } - certManagerControllerSet := certmanager.NewCertManagerControllerSet( + certManagerControllerSet := certmanager.NewControllerSet( kubeClient, kubeInformersForNamespaces, kubeInformersForNamespaces.InformersFor(operatorclient.TargetNamespace), diff --git a/pkg/operator/utils/apidiscovery.go b/pkg/operator/utils/apidiscovery.go index 9b9b4d891..b48f7caf9 100644 --- a/pkg/operator/utils/apidiscovery.go +++ b/pkg/operator/utils/apidiscovery.go @@ -1,6 +1,8 @@ // Package utils provides small shared helpers for the cert-manager operator, // including optional API discovery (whether a GroupVersionResource is served) // and shared informer wiring that runs only when that API exists. +// +//nolint:revive // utils is an established package name in this codebase package utils import ( diff --git a/pkg/operator/utils/apidiscovery_test.go b/pkg/operator/utils/apidiscovery_test.go index 6f3fcf5c4..7278187e6 100644 --- a/pkg/operator/utils/apidiscovery_test.go +++ b/pkg/operator/utils/apidiscovery_test.go @@ -1,3 +1,4 @@ +//nolint:revive,err113 // utils is an established package name; test file uses dynamic errors package utils import ( @@ -54,7 +55,7 @@ type alwaysErrorFakeDiscovery struct { } // ServerResourcesForGroupVersion is the only func apiResourceDiscoverer's discovery client calls. -func (f *alwaysErrorFakeDiscovery) ServerResourcesForGroupVersion(groupVersion string) (*metav1.APIResourceList, error) { +func (f *alwaysErrorFakeDiscovery) ServerResourcesForGroupVersion(_ string) (*metav1.APIResourceList, error) { return nil, fmt.Errorf("simulated discovery error") }