From 140f17255aa1a7177ac9d39aa70e5eaed7d18804 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Tue, 11 Jun 2024 21:33:22 +0200 Subject: [PATCH] chore: enable errorlint linter on controller folder (#18596) * chore: enable errorlint linter on controller folder Signed-off-by: Matthieu MOREL * Update cache.go Signed-off-by: Matthieu MOREL * Update cache.go Signed-off-by: Matthieu MOREL --------- Signed-off-by: Matthieu MOREL --- .golangci.yaml | 2 +- controller/appcontroller.go | 34 ++++++++++++++++----------------- controller/cache/cache.go | 18 +++++++++++------ controller/sharding/sharding.go | 16 ++++++++-------- controller/state.go | 4 ++-- 5 files changed, 40 insertions(+), 34 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 8af39a4e7b210..5e97da31c8759 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -2,7 +2,7 @@ issues: exclude: - SA5011 exclude-rules: - - path: "(applicationset|cmpserver|controller|reposerver|server)/" + - path: "(applicationset|cmpserver|reposerver|server)/" linters: - errorlint max-issues-per-linter: 0 diff --git a/controller/appcontroller.go b/controller/appcontroller.go index 7562ecb591e1a..9d500a7af0b59 100644 --- a/controller/appcontroller.go +++ b/controller/appcontroller.go @@ -260,7 +260,7 @@ func NewApplicationController( if kubeerrors.IsNotFound(err) { appControllerDeployment = nil } else { - return fmt.Errorf("error retrieving Application Controller Deployment: %s", err) + return fmt.Errorf("error retrieving Application Controller Deployment: %w", err) } } if appControllerDeployment != nil { @@ -269,7 +269,7 @@ func NewApplicationController( } shard := env.ParseNumFromEnv(common.EnvControllerShard, -1, -math.MaxInt32, math.MaxInt32) if _, err := sharding.GetOrUpdateShardFromConfigMap(kubeClientset.(*kubernetes.Clientset), settingsMgr, int(*appControllerDeployment.Spec.Replicas), shard); err != nil { - return fmt.Errorf("error while updating the heartbeat for to the Shard Mapping ConfigMap: %s", err) + return fmt.Errorf("error while updating the heartbeat for to the Shard Mapping ConfigMap: %w", err) } } } @@ -381,7 +381,7 @@ func (ctrl *ApplicationController) getAppProj(app *appv1.Application) (*appv1.Ap if apierr.IsNotFound(err) { return nil, err } else { - return nil, fmt.Errorf("could not retrieve AppProject '%s' from cache: %v", app.Spec.Project, err) + return nil, fmt.Errorf("could not retrieve AppProject '%s' from cache: %w", app.Spec.Project, err) } } if !proj.IsAppNamespacePermitted(app, ctrl.namespace) { @@ -457,19 +457,19 @@ func (ctrl *ApplicationController) handleObjectUpdated(managedByApp map[string]b func (ctrl *ApplicationController) setAppManagedResources(a *appv1.Application, comparisonResult *comparisonResult) (*appv1.ApplicationTree, error) { managedResources, err := ctrl.hideSecretData(a, comparisonResult) if err != nil { - return nil, fmt.Errorf("error getting managed resources: %s", err) + return nil, fmt.Errorf("error getting managed resources: %w", err) } tree, err := ctrl.getResourceTree(a, managedResources) if err != nil { - return nil, fmt.Errorf("error getting resource tree: %s", err) + return nil, fmt.Errorf("error getting resource tree: %w", err) } err = ctrl.cache.SetAppResourcesTree(a.InstanceName(ctrl.namespace), tree) if err != nil { - return nil, fmt.Errorf("error setting app resource tree: %s", err) + return nil, fmt.Errorf("error setting app resource tree: %w", err) } err = ctrl.cache.SetAppManagedResources(a.InstanceName(ctrl.namespace), managedResources) if err != nil { - return nil, fmt.Errorf("error setting app managed resources: %s", err) + return nil, fmt.Errorf("error setting app managed resources: %w", err) } return tree, nil } @@ -716,28 +716,28 @@ func (ctrl *ApplicationController) hideSecretData(app *appv1.Application, compar var err error target, live, err = diff.HideSecretData(res.Target, res.Live) if err != nil { - return nil, fmt.Errorf("error hiding secret data: %s", err) + return nil, fmt.Errorf("error hiding secret data: %w", err) } compareOptions, err := ctrl.settingsMgr.GetResourceCompareOptions() if err != nil { - return nil, fmt.Errorf("error getting resource compare options: %s", err) + return nil, fmt.Errorf("error getting resource compare options: %w", err) } resourceOverrides, err := ctrl.settingsMgr.GetResourceOverrides() if err != nil { - return nil, fmt.Errorf("error getting resource overrides: %s", err) + return nil, fmt.Errorf("error getting resource overrides: %w", err) } appLabelKey, err := ctrl.settingsMgr.GetAppInstanceLabelKey() if err != nil { - return nil, fmt.Errorf("error getting app instance label key: %s", err) + return nil, fmt.Errorf("error getting app instance label key: %w", err) } trackingMethod, err := ctrl.settingsMgr.GetTrackingMethod() if err != nil { - return nil, fmt.Errorf("error getting tracking method: %s", err) + return nil, fmt.Errorf("error getting tracking method: %w", err) } clusterCache, err := ctrl.stateCache.GetClusterCache(app.Spec.Destination.Server) if err != nil { - return nil, fmt.Errorf("error getting cluster cache: %s", err) + return nil, fmt.Errorf("error getting cluster cache: %w", err) } diffConfig, err := argodiff.NewDiffConfigBuilder(). WithDiffSettings(app.Spec.IgnoreDifferences, resourceOverrides, compareOptions.IgnoreAggregatedRoles, ctrl.ignoreNormalizerOpts). @@ -747,12 +747,12 @@ func (ctrl *ApplicationController) hideSecretData(app *appv1.Application, compar WithGVKParser(clusterCache.GetGVKParser()). Build() if err != nil { - return nil, fmt.Errorf("appcontroller error building diff config: %s", err) + return nil, fmt.Errorf("appcontroller error building diff config: %w", err) } diffResult, err := argodiff.StateDiff(live, target, diffConfig) if err != nil { - return nil, fmt.Errorf("error applying diff: %s", err) + return nil, fmt.Errorf("error applying diff: %w", err) } resDiff = diffResult } @@ -760,7 +760,7 @@ func (ctrl *ApplicationController) hideSecretData(app *appv1.Application, compar if live != nil { data, err := json.Marshal(live) if err != nil { - return nil, fmt.Errorf("error marshaling live json: %s", err) + return nil, fmt.Errorf("error marshaling live json: %w", err) } item.LiveState = string(data) } else { @@ -770,7 +770,7 @@ func (ctrl *ApplicationController) hideSecretData(app *appv1.Application, compar if target != nil { data, err := json.Marshal(target) if err != nil { - return nil, fmt.Errorf("error marshaling target json: %s", err) + return nil, fmt.Errorf("error marshaling target json: %w", err) } item.TargetState = string(data) } else { diff --git a/controller/cache/cache.go b/controller/cache/cache.go index e8eb2afeb9dfe..bc4c9725f9608 100644 --- a/controller/cache/cache.go +++ b/controller/cache/cache.go @@ -396,12 +396,17 @@ func isResourceQuotaConflictErr(err error) bool { } func isTransientNetworkErr(err error) bool { - switch err.(type) { - case net.Error: - switch err.(type) { - case *net.DNSError, *net.OpError, net.UnknownNetworkError: + var netErr net.Error + switch { + case errors.As(err, &netErr): + var dnsErr *net.DNSError + var opErr *net.OpError + var unknownNetworkErr net.UnknownNetworkError + var urlErr *url.Error + switch { + case errors.As(err, &dnsErr), errors.As(err, &opErr), errors.As(err, &unknownNetworkErr): return true - case *url.Error: + case errors.As(err, &urlErr): // For a URL error, where it replies "connection closed" // retry again. return strings.Contains(err.Error(), "Connection closed by foreign host") @@ -409,7 +414,8 @@ func isTransientNetworkErr(err error) bool { } errorString := err.Error() - if exitErr, ok := err.(*exec.ExitError); ok { + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { errorString = fmt.Sprintf("%s %s", errorString, exitErr.Stderr) } if strings.Contains(errorString, "net/http: TLS handshake timeout") || diff --git a/controller/sharding/sharding.go b/controller/sharding/sharding.go index c50b68365e62d..e593547b00f8f 100644 --- a/controller/sharding/sharding.go +++ b/controller/sharding/sharding.go @@ -313,7 +313,7 @@ func GetOrUpdateShardFromConfigMap(kubeClient kubernetes.Interface, settingsMgr if err != nil { if !kubeerrors.IsNotFound(err) { - return -1, fmt.Errorf("error getting sharding config map: %s", err) + return -1, fmt.Errorf("error getting sharding config map: %w", err) } log.Infof("shard mapping configmap %s not found. Creating default shard mapping configmap.", common.ArgoCDAppControllerShardConfigMapName) @@ -323,10 +323,10 @@ func GetOrUpdateShardFromConfigMap(kubeClient kubernetes.Interface, settingsMgr } shardMappingCM, err = generateDefaultShardMappingCM(settingsMgr.GetNamespace(), hostname, replicas, shard) if err != nil { - return -1, fmt.Errorf("error generating default shard mapping configmap %s", err) + return -1, fmt.Errorf("error generating default shard mapping configmap %w", err) } if _, err = kubeClient.CoreV1().ConfigMaps(settingsMgr.GetNamespace()).Create(context.Background(), shardMappingCM, metav1.CreateOptions{}); err != nil { - return -1, fmt.Errorf("error creating shard mapping configmap %s", err) + return -1, fmt.Errorf("error creating shard mapping configmap %w", err) } // return 0 as the controller is assigned to shard 0 while generating default shard mapping ConfigMap return shard, nil @@ -336,13 +336,13 @@ func GetOrUpdateShardFromConfigMap(kubeClient kubernetes.Interface, settingsMgr var shardMappingData []shardApplicationControllerMapping err := json.Unmarshal([]byte(data), &shardMappingData) if err != nil { - return -1, fmt.Errorf("error unmarshalling shard config map data: %s", err) + return -1, fmt.Errorf("error unmarshalling shard config map data: %w", err) } shard, shardMappingData := getOrUpdateShardNumberForController(shardMappingData, hostname, replicas, shard) updatedShardMappingData, err := json.Marshal(shardMappingData) if err != nil { - return -1, fmt.Errorf("error marshalling data of shard mapping ConfigMap: %s", err) + return -1, fmt.Errorf("error marshalling data of shard mapping ConfigMap: %w", err) } shardMappingCM.Data[ShardControllerMappingKey] = string(updatedShardMappingData) @@ -439,7 +439,7 @@ func generateDefaultShardMappingCM(namespace, hostname string, replicas, shard i data, err := json.Marshal(shardMappingData) if err != nil { - return nil, fmt.Errorf("error generating default ConfigMap: %s", err) + return nil, fmt.Errorf("error generating default ConfigMap: %w", err) } shardingCM.Data[ShardControllerMappingKey] = string(data) @@ -465,7 +465,7 @@ func GetClusterSharding(kubeClient kubernetes.Interface, settingsMgr *settings.S appControllerDeployment, err := kubeClient.AppsV1().Deployments(settingsMgr.GetNamespace()).Get(context.Background(), applicationControllerName, metav1.GetOptions{}) // if app controller deployment is not found when dynamic cluster distribution is enabled error out if err != nil { - return nil, fmt.Errorf("(dynamic cluster distribution) failed to get app controller deployment: %v", err) + return nil, fmt.Errorf("(dynamic cluster distribution) failed to get app controller deployment: %w", err) } if appControllerDeployment != nil && appControllerDeployment.Spec.Replicas != nil { @@ -487,7 +487,7 @@ func GetClusterSharding(kubeClient kubernetes.Interface, settingsMgr *settings.S for i := 0; i <= common.AppControllerHeartbeatUpdateRetryCount; i++ { shardNumber, err = GetOrUpdateShardFromConfigMap(kubeClient, settingsMgr, replicasCount, shardNumber) if err != nil && !kubeerrors.IsConflict(err) { - err = fmt.Errorf("unable to get shard due to error updating the sharding config map: %s", err) + err = fmt.Errorf("unable to get shard due to error updating the sharding config map: %w", err) break } log.Warnf("conflict when getting shard from shard mapping configMap. Retrying (%d/3)", i) diff --git a/controller/state.go b/controller/state.go index f5cacfb09914a..a9a3be4bdd6b8 100644 --- a/controller/state.go +++ b/controller/state.go @@ -179,7 +179,7 @@ func (m *appStateManager) GetRepoObjs(app *v1alpha1.Application, sources []v1alp // revisions for the rollback refSources, err := argo.GetRefSources(context.Background(), sources, app.Spec.Project, m.db.GetRepository, revisions, rollback) if err != nil { - return nil, nil, fmt.Errorf("failed to get ref sources: %v", err) + return nil, nil, fmt.Errorf("failed to get ref sources: %w", err) } for i, source := range sources { @@ -539,7 +539,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1 permitted, err := project.IsLiveResourcePermitted(v, app.Spec.Destination.Server, app.Spec.Destination.Name, func(project string) ([]*v1alpha1.Cluster, error) { clusters, err := m.db.GetProjectClusters(context.TODO(), project) if err != nil { - return nil, fmt.Errorf("failed to get clusters for project %q: %v", project, err) + return nil, fmt.Errorf("failed to get clusters for project %q: %w", project, err) } return clusters, nil })