From 939f5a47d26e228e7ebe14eeeb864cb177a18ba3 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Mon, 27 Jul 2026 21:12:52 +0200 Subject: [PATCH] chore: enable and fix some gocritic issues Signed-off-by: Matthieu MOREL --- .golangci.yml | 30 +++++++++++++++++++ api/defaults/service.go | 24 ++++++--------- ca/keyreadwriter.go | 2 +- ca/renewer.go | 7 +++-- cli/external_ca.go | 2 +- manager/controlapi/cluster.go | 2 +- manager/logbroker/broker.go | 6 ++-- .../jobs/replicated/reconciler.go | 6 ++-- manager/orchestrator/restart/restart.go | 7 +++-- manager/orchestrator/update/updater.go | 7 +++-- protobuf/plugin/deepcopy/deepcopy.go | 21 +++++++------ swarmd/dockerexec/container.go | 2 +- 12 files changed, 71 insertions(+), 45 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 9add6fc6f9..31bc57e78f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -8,6 +8,7 @@ linters: default: none enable: - ginkgolinter + - gocritic - govet - ineffassign - misspell @@ -16,6 +17,35 @@ linters: - unconvert - unused settings: + gocritic: + disabled-checks: + - appendAssign + - appendCombine + - assignOp + - badLock + - builtinShadow + - commentedOutCode + - deferInLoop + - emptyStringTest + - evalOrder + - exposedSyncMutex + - hugeParam + - importShadow + - nestingReduce + - nilValReturn + - octalLiteral + - paramTypeCombine + - rangeValCopy + - regexpSimplify + - singleCaseSwitch + - sloppyReassign + - typeAssertChain + - unlabelStmt + - unlambda + - unnamedResult + - whyNoLint + - yodaStyleExpr + enable-all: true govet: enable: - nilness diff --git a/api/defaults/service.go b/api/defaults/service.go index 3e40c25e99..37e1c6faed 100644 --- a/api/defaults/service.go +++ b/api/defaults/service.go @@ -66,11 +66,9 @@ func InterpolateService(origSpec *api.ServiceSpec) *api.ServiceSpec { if spec.Task.Restart == nil { spec.Task.Restart = Service.Task.Restart.Copy() - } else { - if spec.Task.Restart.Delay == nil { - spec.Task.Restart.Delay = &gogotypes.Duration{} - deepcopy.Copy(spec.Task.Restart.Delay, Service.Task.Restart.Delay) - } + } else if spec.Task.Restart.Delay == nil { + spec.Task.Restart.Delay = &gogotypes.Duration{} + deepcopy.Copy(spec.Task.Restart.Delay, Service.Task.Restart.Delay) } if spec.Task.Placement == nil { @@ -79,20 +77,16 @@ func InterpolateService(origSpec *api.ServiceSpec) *api.ServiceSpec { if spec.Update == nil { spec.Update = Service.Update.Copy() - } else { - if spec.Update.Monitor == nil { - spec.Update.Monitor = &gogotypes.Duration{} - deepcopy.Copy(spec.Update.Monitor, Service.Update.Monitor) - } + } else if spec.Update.Monitor == nil { + spec.Update.Monitor = &gogotypes.Duration{} + deepcopy.Copy(spec.Update.Monitor, Service.Update.Monitor) } if spec.Rollback == nil { spec.Rollback = Service.Rollback.Copy() - } else { - if spec.Rollback.Monitor == nil { - spec.Rollback.Monitor = &gogotypes.Duration{} - deepcopy.Copy(spec.Rollback.Monitor, Service.Rollback.Monitor) - } + } else if spec.Rollback.Monitor == nil { + spec.Rollback.Monitor = &gogotypes.Duration{} + deepcopy.Copy(spec.Rollback.Monitor, Service.Rollback.Monitor) } return spec diff --git a/ca/keyreadwriter.go b/ca/keyreadwriter.go index 55f7d6ba4a..b0b75b6455 100644 --- a/ca/keyreadwriter.go +++ b/ca/keyreadwriter.go @@ -208,7 +208,7 @@ func (k *KeyReadWriter) Read() ([]byte, []byte, error) { switch { case err == nil: _, err = tls.X509KeyPair(cert, keyBytes) - case os.IsNotExist(err): //continue to try temp location + case os.IsNotExist(err): // continue to try temp location break default: return nil, nil, err diff --git a/ca/renewer.go b/ca/renewer.go index 1eacab16df..b1f1ebc90e 100644 --- a/ca/renewer.go +++ b/ca/renewer.go @@ -95,15 +95,16 @@ func (t *TLSRenewer) Start(ctx context.Context) <-chan CertificateUpdate { } else { // If we have an expired certificate, try to renew immediately: the hope that this is a temporary clock skew, or // we can issue our own TLS certs. - if validUntil.Before(time.Now()) { + switch { + case validUntil.Before(time.Now()): logger.Warn("the current TLS certificate is expired, so an attempt to renew it will be made immediately") // retry immediately(ish) with exponential backoff retry = expBackoff.Proceed(nil) - } else if forceRetry { + case forceRetry: // A forced renewal was requested, but did not succeed yet. // retry immediately(ish) with exponential backoff retry = expBackoff.Proceed(nil) - } else { + default: // Random retry time between 50% and 80% of the total time to expiration retry = calculateRandomExpiry(validFrom, validUntil) } diff --git a/cli/external_ca.go b/cli/external_ca.go index d4c0eb29a9..c4c0938125 100644 --- a/cli/external_ca.go +++ b/cli/external_ca.go @@ -75,7 +75,7 @@ func parseExternalCA(caSpec string) (*api.ExternalCA, error) { switch strings.ToLower(key) { case "protocol": hasProtocol = true - if strings.ToLower(value) == "cfssl" { + if strings.EqualFold(value, "cfssl") { externalCA.Protocol = api.ExternalCA_CAProtocolCFSSL } else { return nil, fmt.Errorf("unrecognized external CA protocol %s", value) diff --git a/manager/controlapi/cluster.go b/manager/controlapi/cluster.go index f2f32ee107..dc54e3629d 100644 --- a/manager/controlapi/cluster.go +++ b/manager/controlapi/cluster.go @@ -50,7 +50,7 @@ func validateClusterSpec(spec *api.ClusterSpec) error { // TODO(diogo): Add a global list of acceptance algorithms. We only support bcrypt for now. if len(spec.AcceptancePolicy.Policies) > 0 { for _, policy := range spec.AcceptancePolicy.Policies { - if policy.Secret != nil && strings.ToLower(policy.Secret.Alg) != "bcrypt" { + if policy.Secret != nil && !strings.EqualFold(policy.Secret.Alg, "bcrypt") { return status.Errorf(codes.InvalidArgument, "hashing algorithm is not supported: %s", policy.Secret.Alg) } } diff --git a/manager/logbroker/broker.go b/manager/logbroker/broker.go index 9546e02720..06bc361a07 100644 --- a/manager/logbroker/broker.go +++ b/manager/logbroker/broker.go @@ -405,10 +405,8 @@ func (lb *LogBroker) PublishLogs(stream api.LogBroker_PublishLogsServer) (err er if currentSubscription == nil { return status.Errorf(codes.NotFound, "unknown subscription ID") } - } else { - if logMsg.SubscriptionID != currentSubscription.ID() { - return status.Errorf(codes.InvalidArgument, "different subscription IDs in the same session") - } + } else if logMsg.SubscriptionID != currentSubscription.ID() { + return status.Errorf(codes.InvalidArgument, "different subscription IDs in the same session") } // if we have a close message, close out the subscription diff --git a/manager/orchestrator/jobs/replicated/reconciler.go b/manager/orchestrator/jobs/replicated/reconciler.go index e3b0d5dc69..89eaf3cdb6 100644 --- a/manager/orchestrator/jobs/replicated/reconciler.go +++ b/manager/orchestrator/jobs/replicated/reconciler.go @@ -139,13 +139,11 @@ func (r *Reconciler) ReconcileService(id string) error { restartTasks = append(restartTasks, task.ID) } } - } else { + } else if task.Status.State <= api.TaskStateRunning && task.DesiredState != api.TaskStateRemove { // tasks belonging to a previous iteration of the job may // exist. if any such tasks exist, they should have their task // state set to Remove - if task.Status.State <= api.TaskStateRunning && task.DesiredState != api.TaskStateRemove { - removeTasks = append(removeTasks, task.ID) - } + removeTasks = append(removeTasks, task.ID) } } } diff --git a/manager/orchestrator/restart/restart.go b/manager/orchestrator/restart/restart.go index a13340112b..51001d3739 100644 --- a/manager/orchestrator/restart/restart.go +++ b/manager/orchestrator/restart/restart.go @@ -152,11 +152,12 @@ func (r *Supervisor) Restart(ctx context.Context, tx store.Tx, cluster *api.Clus var restartTask *api.Task - if orchestrator.IsReplicatedService(service) || orchestrator.IsReplicatedJob(service) { + switch { + case orchestrator.IsReplicatedService(service), orchestrator.IsReplicatedJob(service): restartTask = orchestrator.NewTask(cluster, service, t.Slot, "") - } else if orchestrator.IsGlobalService(service) || orchestrator.IsGlobalJob(service) { + case orchestrator.IsGlobalService(service), orchestrator.IsGlobalJob(service): restartTask = orchestrator.NewTask(cluster, service, 0, t.NodeID) - } else { + default: log.G(ctx).Error("service not supported by restart supervisor") return nil } diff --git a/manager/orchestrator/update/updater.go b/manager/orchestrator/update/updater.go index fbad48e0d5..786a6c9ff5 100644 --- a/manager/orchestrator/update/updater.go +++ b/manager/orchestrator/update/updater.go @@ -332,15 +332,16 @@ func (u *Updater) worker(ctx context.Context, queue <-chan orchestrator.Slot, up } } } - if runningTask != nil { + switch { + case runningTask != nil: if err := u.useExistingTask(ctx, slot, runningTask); err != nil { log.G(ctx).WithError(err).Error("update failed") } - } else if cleanTask != nil { + case cleanTask != nil: if err := u.useExistingTask(ctx, slot, cleanTask); err != nil { log.G(ctx).WithError(err).Error("update failed") } - } else { + default: updated := orchestrator.NewTask(u.cluster, u.newService, slot[0].Slot, "") if orchestrator.IsGlobalService(u.newService) { updated = orchestrator.NewTask(u.cluster, u.newService, slot[0].Slot, slot[0].NodeID) diff --git a/protobuf/plugin/deepcopy/deepcopy.go b/protobuf/plugin/deepcopy/deepcopy.go index d65ba5caf6..c1b52763d9 100644 --- a/protobuf/plugin/deepcopy/deepcopy.go +++ b/protobuf/plugin/deepcopy/deepcopy.go @@ -157,7 +157,8 @@ func (d *deepCopyGen) genMap(_ *generator.Descriptor, f *descriptor.FieldDescrip d.P("m.", fName, " = make(", typename, ", ", "len(o.", fName, "))") d.P("for k, v := range o.", fName, " {") d.In() - if mt.ValueField.IsMessage() { + switch { + case mt.ValueField.IsMessage(): if !gogoproto.IsNullable(f) { d.P("n := ", d.TypeName(d.ObjectNamed(mt.ValueField.GetTypeName())), "{}") d.genCopyFunc("&n", "&v") @@ -166,10 +167,10 @@ func (d *deepCopyGen) genMap(_ *generator.Descriptor, f *descriptor.FieldDescrip d.P("m.", fName, "[k] = &", d.TypeName(d.ObjectNamed(mt.ValueField.GetTypeName())), "{}") d.genCopyFunc("m."+fName+"[k]", "v") } - } else if mt.ValueField.IsBytes() { + case mt.ValueField.IsBytes(): d.P("m.", fName, "[k] = o.", fName, "[k]") d.genCopyBytes("m."+fName+"[k]", "o."+fName+"[k]") - } else { + default: d.P("m.", fName, "[k] = v") } d.Out() @@ -192,7 +193,8 @@ func (d *deepCopyGen) genRepeated(m *generator.Descriptor, f *descriptor.FieldDe d.P("if o.", fName, " != nil {") d.In() d.P("m.", fName, " = make(", typename, ", len(o.", fName, "))") - if f.IsMessage() { + switch { + case f.IsMessage(): // TODO(stevvooe): Handle custom type here? goType := d.TypeName(d.ObjectNamed(f.GetTypeName())) // elides [] or * @@ -206,13 +208,13 @@ func (d *deepCopyGen) genRepeated(m *generator.Descriptor, f *descriptor.FieldDe } d.Out() d.P("}") - } else if f.IsBytes() { + case f.IsBytes(): d.P("for i := range m.", fName, " {") d.In() d.genCopyBytes("m."+fName+"[i]", "o."+fName+"[i]") d.Out() d.P("}") - } else { + default: d.P("copy(m.", fName, ", ", "o.", fName, ")") } d.Out() @@ -241,12 +243,13 @@ func (d *deepCopyGen) genOneOf(m *generator.Descriptor, oneof *descriptor.OneofD d.In() var rhs string - if f.IsMessage() { + switch { + case f.IsMessage(): goType := d.TypeName(d.ObjectNamed(f.GetTypeName())) // elides [] or * rhs = "&" + goType + "{}" - } else if f.IsBytes() { + case f.IsBytes(): rhs = "make([]byte, len(o.Get" + fName + "()))" - } else { + default: rhs = "o.Get" + fName + "()" } d.P(fName, ": ", rhs, ",") diff --git a/swarmd/dockerexec/container.go b/swarmd/dockerexec/container.go index f29207cba7..303d1cb611 100644 --- a/swarmd/dockerexec/container.go +++ b/swarmd/dockerexec/container.go @@ -263,7 +263,7 @@ func (c *containerConfig) labels() map[string]string { // finally, we apply the system labels, which override all labels. for k, v := range system { - labels[strings.Join([]string{systemLabelPrefix, k}, ".")] = v + labels[systemLabelPrefix+"."+k] = v } return labels