From df74ebd53a32212369094bf2f56e4940855b0694 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Mon, 18 Aug 2025 08:04:39 +0200 Subject: [PATCH 1/8] chore: enable-all rules from go-critic by default Signed-off-by: Matthieu MOREL --- .golangci.yaml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/.golangci.yaml b/.golangci.yaml index 96a9aeb37..16e8b5dfa 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -18,9 +18,24 @@ linters: gocritic: disabled-checks: - appendAssign + - appendCombine - assignOp + - badRegexp + - builtinShadow + - commentedOutCode + - emptyStringTest - exitAfterDefer + - hugeParam + - importShadow + - paramTypeCombine + - rangeValCopy + - stringsCompare + - stringXbytes + - typeDefFirst - typeSwitchVar + - unnamedResult + - whyNoLint + enable-all: true importas: alias: - pkg: k8s.io/api/apps/v1 From e2442c962eb25922a982b372b7795bd62f8f0397 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Mon, 18 Aug 2025 08:12:33 +0200 Subject: [PATCH 2/8] chore: enable appendCombine rule from go-critic Signed-off-by: Matthieu MOREL --- .golangci.yaml | 1 - pkg/sync/sync_context_test.go | 7 +++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 16e8b5dfa..2e36b9970 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -18,7 +18,6 @@ linters: gocritic: disabled-checks: - appendAssign - - appendCombine - assignOp - badRegexp - builtinShadow diff --git a/pkg/sync/sync_context_test.go b/pkg/sync/sync_context_test.go index 0e8d01ebb..cf61cd14d 100644 --- a/pkg/sync/sync_context_test.go +++ b/pkg/sync/sync_context_test.go @@ -1827,8 +1827,11 @@ func TestSetOperationFailedDuplicatedMessages(t *testing.T) { sc.log = textlogger.NewLogger(textlogger.NewConfig()).WithValues("application", "fake-app") tasks := make([]*syncTask, 0) - tasks = append(tasks, &syncTask{message: "namespace not found"}) - tasks = append(tasks, &syncTask{message: "namespace not found"}) + tasks = append( + tasks, + &syncTask{message: "namespace not found"}, + &syncTask{message: "namespace not found"}, + ) sc.setOperationFailed(nil, tasks, "one or more objects failed to apply") From 9943ddba2bc22d2ede6a7bd07f7e322c5e9ad994 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Mon, 18 Aug 2025 08:16:18 +0200 Subject: [PATCH 3/8] chore: enable emptyStringTest rule from go-critic Signed-off-by: Matthieu MOREL --- .golangci.yaml | 1 - pkg/utils/kube/resource_ops.go | 8 ++++---- pkg/utils/kube/uniqueprotomodels.go | 4 ++-- pkg/utils/text/text.go | 4 ++-- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 2e36b9970..4ca8e8f16 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -22,7 +22,6 @@ linters: - badRegexp - builtinShadow - commentedOutCode - - emptyStringTest - exitAfterDefer - hugeParam - importShadow diff --git a/pkg/utils/kube/resource_ops.go b/pkg/utils/kube/resource_ops.go index f425fb2ef..6adeac8b9 100644 --- a/pkg/utils/kube/resource_ops.go +++ b/pkg/utils/kube/resource_ops.go @@ -140,10 +140,10 @@ func (k *kubectlResourceOperations) runResourceCommand(ctx context.Context, obj if err != nil { return "", errors.New(cleanKubectlOutput(err.Error())) } - if buf := strings.TrimSpace(ioStreams.Out.(*bytes.Buffer).String()); len(buf) > 0 { + if buf := strings.TrimSpace(ioStreams.Out.(*bytes.Buffer).String()); buf != "" { out = append(out, buf) } - if buf := strings.TrimSpace(ioStreams.ErrOut.(*bytes.Buffer).String()); len(buf) > 0 { + if buf := strings.TrimSpace(ioStreams.ErrOut.(*bytes.Buffer).String()); buf != "" { out = append(out, buf) } return strings.Join(out, ". "), nil @@ -608,10 +608,10 @@ func (k *kubectlResourceOperations) authReconcile(ctx context.Context, obj *unst } var out []string - if buf := strings.TrimSpace(ioStreams.Out.(*bytes.Buffer).String()); len(buf) > 0 { + if buf := strings.TrimSpace(ioStreams.Out.(*bytes.Buffer).String()); buf != "" { out = append(out, buf) } - if buf := strings.TrimSpace(ioStreams.ErrOut.(*bytes.Buffer).String()); len(buf) > 0 { + if buf := strings.TrimSpace(ioStreams.ErrOut.(*bytes.Buffer).String()); buf != "" { out = append(out, buf) } return strings.Join(out, ". "), nil diff --git a/pkg/utils/kube/uniqueprotomodels.go b/pkg/utils/kube/uniqueprotomodels.go index ba8b2a3ea..a52768454 100644 --- a/pkg/utils/kube/uniqueprotomodels.go +++ b/pkg/utils/kube/uniqueprotomodels.go @@ -104,7 +104,7 @@ func newUniqueModels(models proto.Models) (proto.Models, []schema.GroupVersionKi // Add GVKs to the map, so we can detect a duplicate GVK later. for _, gvk := range gvkList { - if len(gvk.Kind) > 0 { + if gvk.Kind != "" { gvks[gvk] = modelName } } @@ -124,7 +124,7 @@ func modelGvkWasAlreadyProcessed(model proto.Schema, gvks map[schema.GroupVersio for _, gvk := range gvkList { // The kind length check is copied from managedfields.NewGVKParser. It's unclear what edge case it's handling, // but the behavior of this function should match NewGVKParser. - if len(gvk.Kind) > 0 { + if gvk.Kind != "" { _, ok := gvks[gvk] if ok { // This is the only condition under which NewGVKParser would return a duplicate GVK error. diff --git a/pkg/utils/text/text.go b/pkg/utils/text/text.go index 7c8f865a6..b3a952faa 100644 --- a/pkg/utils/text/text.go +++ b/pkg/utils/text/text.go @@ -2,7 +2,7 @@ package text func FirstNonEmpty(args ...string) string { for _, value := range args { - if len(value) > 0 { + if value != "" { return value } } @@ -11,7 +11,7 @@ func FirstNonEmpty(args ...string) string { // WithDefault return defaultValue when val is blank func WithDefault(val string, defaultValue string) string { - if len(val) == 0 { + if val == "" { return defaultValue } return val From c9ec4e022ed691a00824e92b636f822faf6553a2 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Mon, 18 Aug 2025 08:17:46 +0200 Subject: [PATCH 4/8] chore: enable stringsCompare rule from go-critic Signed-off-by: Matthieu MOREL --- .golangci.yaml | 1 - pkg/cache/cluster.go | 3 +-- pkg/cache/cluster_test.go | 3 +-- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 4ca8e8f16..7619f9a9d 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -27,7 +27,6 @@ linters: - importShadow - paramTypeCombine - rangeValCopy - - stringsCompare - stringXbytes - typeDefFirst - typeSwitchVar diff --git a/pkg/cache/cluster.go b/pkg/cache/cluster.go index 38f1e6016..8059140c5 100644 --- a/pkg/cache/cluster.go +++ b/pkg/cache/cluster.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "runtime/debug" - "strings" "sync" "time" @@ -1140,7 +1139,7 @@ func buildGraph(nsNodes map[kube.ResourceKey]*Resource) map[kube.ResourceKey]map // It is ok to pick any object, but we need to make sure we pick the same child after every refresh. key1 := r.ResourceKey() key2 := childNode.ResourceKey() - if strings.Compare(key1.String(), key2.String()) > 0 { + if key1.String() > key2.String() { graph[uidNode.ResourceKey()][childNode.Ref.UID] = childNode } } diff --git a/pkg/cache/cluster_test.go b/pkg/cache/cluster_test.go index 7bac78043..dc225f24d 100644 --- a/pkg/cache/cluster_test.go +++ b/pkg/cache/cluster_test.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "sort" - "strings" "sync" "testing" "time" @@ -681,7 +680,7 @@ func TestProcessNewChildEvent(t *testing.T) { rsChildren := getChildren(cluster, mustToUnstructured(testRS())) sort.Slice(rsChildren, func(i, j int) bool { - return strings.Compare(rsChildren[i].Ref.Name, rsChildren[j].Ref.Name) < 0 + return rsChildren[i].Ref.Name < rsChildren[j].Ref.Name }) assert.Equal(t, []*Resource{{ Ref: corev1.ObjectReference{ From 4380ab42fe02961e188c5506f1f54575c7c725f1 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Mon, 18 Aug 2025 08:18:38 +0200 Subject: [PATCH 5/8] chore: enable stringXbytes rule from go-critic Signed-off-by: Matthieu MOREL --- .golangci.yaml | 1 - pkg/diff/diff.go | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 7619f9a9d..ca2aa9744 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -27,7 +27,6 @@ linters: - importShadow - paramTypeCombine - rangeValCopy - - stringXbytes - typeDefFirst - typeSwitchVar - unnamedResult diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index 95c19b469..744a9129d 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -473,7 +473,7 @@ func normalizeTypedValue(tv *typed.TypedValue) ([]byte, error) { func buildDiffResult(predictedBytes []byte, liveBytes []byte) *DiffResult { return &DiffResult{ - Modified: string(liveBytes) != string(predictedBytes), + Modified: !bytes.Equal(liveBytes, predictedBytes), NormalizedLive: liveBytes, PredictedLive: predictedBytes, } From cc15eb2f3acddc4e1041ea743ba6b3f61d40ade6 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Mon, 18 Aug 2025 08:18:54 +0200 Subject: [PATCH 6/8] chore: enable typeSwitchVar rule from go-critic Signed-off-by: Matthieu MOREL --- .golangci.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/.golangci.yaml b/.golangci.yaml index ca2aa9744..a45bc5b87 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -28,7 +28,6 @@ linters: - paramTypeCombine - rangeValCopy - typeDefFirst - - typeSwitchVar - unnamedResult - whyNoLint enable-all: true From bd0dc622d71e7c7877e31e07fe6c63b688a1a07d Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Mon, 18 Aug 2025 08:34:42 +0200 Subject: [PATCH 7/8] chore: enable assignOp rule from go-critic Signed-off-by: Matthieu MOREL --- .golangci.yaml | 1 - pkg/diff/diff.go | 2 +- pkg/sync/sync_context.go | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index a45bc5b87..8e62bd637 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -18,7 +18,6 @@ linters: gocritic: disabled-checks: - appendAssign - - assignOp - badRegexp - builtinShadow - commentedOutCode diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index 744a9129d..0904801bb 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -1106,7 +1106,7 @@ func hide(target, live, liveLastAppliedAnnotation *unstructured.Unstructured, ke replacement, ok := valToReplacement[val] if !ok { replacement = nextReplacement - nextReplacement = nextReplacement + "++++" + nextReplacement += "++++" valToReplacement[val] = replacement } data[k] = replacement diff --git a/pkg/sync/sync_context.go b/pkg/sync/sync_context.go index 8f4d51e4f..27a561ec9 100644 --- a/pkg/sync/sync_context.go +++ b/pkg/sync/sync_context.go @@ -939,7 +939,7 @@ func (sc *syncContext) getSyncTasks() (_ syncTasks, successful bool) { } } } - syncPhaseLastWave = syncPhaseLastWave + 1 + syncPhaseLastWave++ for _, task := range tasks { if task.isPrune() && From 4d798bdb507f6321d92492ba11ceb9e51901f944 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Mon, 18 Aug 2025 08:36:01 +0200 Subject: [PATCH 8/8] chore: enable typeDefFirst rule from go-critic Signed-off-by: Matthieu MOREL --- .golangci.yaml | 1 - pkg/sync/sync_context.go | 102 +++++++++++++++++++-------------------- 2 files changed, 51 insertions(+), 52 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 8e62bd637..ee060b938 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -26,7 +26,6 @@ linters: - importShadow - paramTypeCombine - rangeValCopy - - typeDefFirst - unnamedResult - whyNoLint enable-all: true diff --git a/pkg/sync/sync_context.go b/pkg/sync/sync_context.go index 27a561ec9..3d2bc20ae 100644 --- a/pkg/sync/sync_context.go +++ b/pkg/sync/sync_context.go @@ -57,6 +57,57 @@ type SyncContext interface { GetState() (common.OperationPhase, string, []common.ResourceSyncResult) } +type syncContext struct { + healthOverride health.HealthOverride + permissionValidator common.PermissionValidator + resources map[kubeutil.ResourceKey]reconciledResource + hooks []*unstructured.Unstructured + config *rest.Config + rawConfig *rest.Config + dynamicIf dynamic.Interface + disco discovery.DiscoveryInterface + extensionsclientset *clientset.Clientset + kubectl kubeutil.Kubectl + resourceOps kubeutil.ResourceOperations + namespace string + + dryRun bool + skipDryRunOnMissingResource bool + force bool + validate bool + skipHooks bool + resourcesFilter func(key kubeutil.ResourceKey, target *unstructured.Unstructured, live *unstructured.Unstructured) bool + prune bool + replace bool + serverSideApply bool + serverSideApplyManager string + pruneLast bool + prunePropagationPolicy *metav1.DeletionPropagation + pruneConfirmed bool + clientSideApplyMigrationManager string + enableClientSideApplyMigration bool + + syncRes map[string]common.ResourceSyncResult + startedAt time.Time + revision string + phase common.OperationPhase + message string + + log logr.Logger + // lock to protect concurrent updates of the result list + lock sync.Mutex + + // syncNamespace is a function that will determine if the managed + // namespace should be synced + syncNamespace func(*unstructured.Unstructured, *unstructured.Unstructured) (bool, error) + + syncWaveHook common.SyncWaveHook + + applyOutOfSyncOnly bool + // stores whether the resource is modified or not + modificationResult map[kubeutil.ResourceKey]bool +} + // SyncOpt is a callback that update sync operation settings type SyncOpt func(ctx *syncContext) @@ -340,57 +391,6 @@ func (sc *syncContext) getOperationPhase(obj *unstructured.Unstructured) (common return phase, message, nil } -type syncContext struct { - healthOverride health.HealthOverride - permissionValidator common.PermissionValidator - resources map[kubeutil.ResourceKey]reconciledResource - hooks []*unstructured.Unstructured - config *rest.Config - rawConfig *rest.Config - dynamicIf dynamic.Interface - disco discovery.DiscoveryInterface - extensionsclientset *clientset.Clientset - kubectl kubeutil.Kubectl - resourceOps kubeutil.ResourceOperations - namespace string - - dryRun bool - skipDryRunOnMissingResource bool - force bool - validate bool - skipHooks bool - resourcesFilter func(key kubeutil.ResourceKey, target *unstructured.Unstructured, live *unstructured.Unstructured) bool - prune bool - replace bool - serverSideApply bool - serverSideApplyManager string - pruneLast bool - prunePropagationPolicy *metav1.DeletionPropagation - pruneConfirmed bool - clientSideApplyMigrationManager string - enableClientSideApplyMigration bool - - syncRes map[string]common.ResourceSyncResult - startedAt time.Time - revision string - phase common.OperationPhase - message string - - log logr.Logger - // lock to protect concurrent updates of the result list - lock sync.Mutex - - // syncNamespace is a function that will determine if the managed - // namespace should be synced - syncNamespace func(*unstructured.Unstructured, *unstructured.Unstructured) (bool, error) - - syncWaveHook common.SyncWaveHook - - applyOutOfSyncOnly bool - // stores whether the resource is modified or not - modificationResult map[kubeutil.ResourceKey]bool -} - func (sc *syncContext) setRunningPhase(tasks []*syncTask, isPendingDeletion bool) { if len(tasks) > 0 { firstTask := tasks[0]