From 4bc9434f99d9a87dd5b63e738b6b1b16693f10e4 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Wed, 2 Aug 2023 10:57:51 +0200 Subject: [PATCH] SSA: prevent usage of Extract calls via forbidigo Client-side extract calls depend on `managedFields`, which might not be available. Therefore they should not be used in production code. They are okay in test files (because the API has to be tested), in the generated code (because the various type specific APIs still need to be provided) and in unstructured.go (same reason). --- hack/golangci-strict.yaml | 15 +++++++++++++++ hack/golangci.yaml | 15 +++++++++++++++ .../applyconfigurations/meta/v1/unstructured.go | 2 +- 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/hack/golangci-strict.yaml b/hack/golangci-strict.yaml index c5aa8399c04..2515eb81ec8 100644 --- a/hack/golangci-strict.yaml +++ b/hack/golangci-strict.yaml @@ -25,10 +25,16 @@ issues: - linters: - ginkgolinter text: use a function call in (Eventually|Consistently) + # SSA Extract calls are allowed in tests. + - linters: + - forbidigo + text: should not be used because managedFields was removed + path: _test.go$ linters: disable-all: false # in contrast to golangci.yaml, the default set of linters remains enabled enable: # please keep this alphabetized and in sync with golangci.yaml + - forbidigo - ginkgolinter - gocritic - govet @@ -45,6 +51,15 @@ linters-settings: # please keep this alphabetized path: ../_output/local/bin/logcheck.so description: structured logging checker original-url: k8s.io/logtools/logcheck + forbidigo: + analyze-types: true + forbid: + - p: ^managedfields\.ExtractInto$ + pkg: ^k8s\.io/apimachinery/pkg/util/managedfields$ + msg: should not be used because managedFields was removed + - p: \.Extract + pkg: ^k8s\.io/client-go/applyconfigurations/ + msg: should not be used because managedFields was removed gocritic: staticcheck: checks: diff --git a/hack/golangci.yaml b/hack/golangci.yaml index f29986b714a..448374acf09 100644 --- a/hack/golangci.yaml +++ b/hack/golangci.yaml @@ -26,10 +26,16 @@ issues: - linters: - ginkgolinter text: use a function call in (Eventually|Consistently) + # SSA Extract calls are allowed in tests. + - linters: + - forbidigo + text: should not be used because managedFields was removed + path: _test.go$ linters: disable-all: true # not disabled in golangci-strict.yaml enable: # please keep this alphabetized and in sync with golangci-strict.yaml + - forbidigo - ginkgolinter - gocritic - govet @@ -46,6 +52,15 @@ linters-settings: # please keep this alphabetized path: ../_output/local/bin/logcheck.so description: structured logging checker original-url: k8s.io/logtools/logcheck + forbidigo: + analyze-types: true + forbid: + - p: ^managedfields\.ExtractInto$ + pkg: ^k8s\.io/apimachinery/pkg/util/managedfields$ + msg: should not be used because managedFields was removed + - p: \.Extract + pkg: ^k8s\.io/client-go/applyconfigurations/ + msg: should not be used because managedFields was removed gocritic: enabled-checks: # not limited in golangci-strict.yaml - equalFold # not limited in golangci-strict.yaml diff --git a/staging/src/k8s.io/client-go/applyconfigurations/meta/v1/unstructured.go b/staging/src/k8s.io/client-go/applyconfigurations/meta/v1/unstructured.go index 8a58d9e8705..a206bd326ae 100644 --- a/staging/src/k8s.io/client-go/applyconfigurations/meta/v1/unstructured.go +++ b/staging/src/k8s.io/client-go/applyconfigurations/meta/v1/unstructured.go @@ -125,7 +125,7 @@ func (e *extractor) extractUnstructured(object *unstructured.Unstructured, field return nil, fmt.Errorf("failed to fetch the objectType: %v", err) } result := &unstructured.Unstructured{} - err = managedfields.ExtractInto(object, *objectType, fieldManager, result, subresource) + err = managedfields.ExtractInto(object, *objectType, fieldManager, result, subresource) //nolint:forbidigo if err != nil { return nil, fmt.Errorf("failed calling ExtractInto for unstructured: %v", err) }