From 0762522cc18125e4baeeccd0918e5a70be9f2942 Mon Sep 17 00:00:00 2001 From: dprotaso Date: Fri, 21 Jun 2024 16:55:03 -0400 Subject: [PATCH 01/22] fix whitespace linter --- apis/duck/v1/knative_reference.go | 1 - apis/duck/v1/status_types.go | 1 - codegen/cmd/injection-gen/generators/packages.go | 1 - injection/informers.go | 1 - test/logstream/v2/stream.go | 1 - test/logstream/v2/stream_test.go | 1 - 6 files changed, 6 deletions(-) diff --git a/apis/duck/v1/knative_reference.go b/apis/duck/v1/knative_reference.go index 4d03b6b97c..2609fb1c99 100644 --- a/apis/duck/v1/knative_reference.go +++ b/apis/duck/v1/knative_reference.go @@ -102,7 +102,6 @@ func (kr *KReference) Validate(ctx context.Context) *apis.FieldError { Details: fmt.Sprintf("parent namespace: %q does not match ref: %q", parentNS, kr.Namespace), }) } - } } return errs diff --git a/apis/duck/v1/status_types.go b/apis/duck/v1/status_types.go index 15e2645346..2e8d66f93b 100644 --- a/apis/duck/v1/status_types.go +++ b/apis/duck/v1/status_types.go @@ -97,7 +97,6 @@ func (s *Status) ConvertTo(ctx context.Context, sink *Status, predicates ...func conditions := make(apis.Conditions, 0, len(s.Conditions)) for _, c := range s.Conditions { - // Copy over the "happy" condition, which is the only condition that // we can reliably transfer. if c.Type == apis.ConditionReady || c.Type == apis.ConditionSucceeded { diff --git a/codegen/cmd/injection-gen/generators/packages.go b/codegen/cmd/injection-gen/generators/packages.go index f0ca43f2bf..0422c94644 100644 --- a/codegen/cmd/injection-gen/generators/packages.go +++ b/codegen/cmd/injection-gen/generators/packages.go @@ -501,7 +501,6 @@ func versionInformerPackages(basePackage string, groupPkgName string, gv clientg return tags.NeedsInformerInjection() }, }) - } return vers } diff --git a/injection/informers.go b/injection/informers.go index 9356f8d7f9..e29b2da57b 100644 --- a/injection/informers.go +++ b/injection/informers.go @@ -93,7 +93,6 @@ func (i *impl) SetupInformers(ctx context.Context, cfg *rest.Config) (context.Co for _, fii := range i.GetFilteredInformers() { ctx, filteredinfs = fii(ctx) informers = append(informers, filteredinfs...) - } return ctx, informers } diff --git a/test/logstream/v2/stream.go b/test/logstream/v2/stream.go index 1a37a54d2f..9b5c42454d 100644 --- a/test/logstream/v2/stream.go +++ b/test/logstream/v2/stream.go @@ -156,7 +156,6 @@ func (s *logSource) watchPods() error { s.startForPod(p) } } - } } }() diff --git a/test/logstream/v2/stream_test.go b/test/logstream/v2/stream_test.go index d7d63b6274..7de7522ee5 100644 --- a/test/logstream/v2/stream_test.go +++ b/test/logstream/v2/stream_test.go @@ -376,7 +376,6 @@ func logsForContainer(container string) string { result += "\n" } result += s - } return result } From 01cf58aaad7dfa7a10b8eeb0d0e0c8a218a522c6 Mon Sep 17 00:00:00 2001 From: dprotaso Date: Fri, 21 Jun 2024 17:10:52 -0400 Subject: [PATCH 02/22] fix usestdlibvars --- apis/contexts_test.go | 2 +- network/handlers/drain_test.go | 2 +- test/spoof/error_checks_test.go | 6 +++--- test/spoof/spoof_test.go | 2 +- webhook/admission_integration_test.go | 10 +++++----- webhook/conversion_integration_test.go | 4 ++-- webhook/webhook_integration_test.go | 4 ++-- 7 files changed, 15 insertions(+), 15 deletions(-) diff --git a/apis/contexts_test.go b/apis/contexts_test.go index 469ada6149..be32735974 100644 --- a/apis/contexts_test.go +++ b/apis/contexts_test.go @@ -231,7 +231,7 @@ func TestGetHTTPRequest(t *testing.T) { t.Errorf("GetHTTPRequest() = %v, wanted %v", got, nil) } - req, err := http.NewRequest("GET", "https://google.com", nil) + req, err := http.NewRequest(http.MethodGet, "https://google.com", nil) if err != nil { t.Fatalf("NewRequest() = %v", err) } diff --git a/network/handlers/drain_test.go b/network/handlers/drain_test.go index 88e4f07e57..31205ac389 100644 --- a/network/handlers/drain_test.go +++ b/network/handlers/drain_test.go @@ -560,7 +560,7 @@ func TestResetWithActiveRequests(t *testing.T) { defer close(trafficStopped) go func() { - req, _ := http.NewRequest("GET", "knative.dev", nil) + req, _ := http.NewRequest(http.MethodGet, "knative.dev", nil) rec := httptest.NewRecorder() close(trafficStarted) diff --git a/test/spoof/error_checks_test.go b/test/spoof/error_checks_test.go index 6d7ae01588..618e6ee9a9 100644 --- a/test/spoof/error_checks_test.go +++ b/test/spoof/error_checks_test.go @@ -49,7 +49,7 @@ func TestDNSError(t *testing.T) { dnsError: false, }} { t.Run(tt.name, func(t *testing.T) { - req, _ := http.NewRequest("GET", tt.url, nil) + req, _ := http.NewRequest(http.MethodGet, tt.url, nil) _, err := client.Do(req) if dnsError := isDNSError(err); tt.dnsError != dnsError { t.Errorf("Expected dnsError=%v, got %v", tt.dnsError, dnsError) @@ -79,7 +79,7 @@ func TestConnectionRefused(t *testing.T) { connRefused: false, }} { t.Run(tt.name, func(t *testing.T) { - req, _ := http.NewRequest("GET", tt.url, nil) + req, _ := http.NewRequest(http.MethodGet, tt.url, nil) _, err := client.Do(req) if connRefused := isConnectionRefused(err); tt.connRefused != connRefused { t.Errorf("Expected connRefused=%v, got %v", tt.connRefused, connRefused) @@ -136,7 +136,7 @@ func TestTCPTimeout(t *testing.T) { tcpTimeout: false, }} { t.Run(tt.name, func(t *testing.T) { - req, _ := http.NewRequest("GET", tt.url, nil) + req, _ := http.NewRequest(http.MethodGet, tt.url, nil) _, err := client.Do(req) if tcpTimeout := isTCPTimeout(err); tt.tcpTimeout != tcpTimeout { t.Errorf("Expected tcpTimeout=%v, got %v", tt.tcpTimeout, tcpTimeout) diff --git a/test/spoof/spoof_test.go b/test/spoof/spoof_test.go index 10dc64367d..2c00712e60 100644 --- a/test/spoof/spoof_test.go +++ b/test/spoof/spoof_test.go @@ -32,7 +32,7 @@ import ( var ( successResponse = &http.Response{ Status: "200 ok", - StatusCode: 200, + StatusCode: http.StatusOK, Header: http.Header{}, Body: http.NoBody, } diff --git a/webhook/admission_integration_test.go b/webhook/admission_integration_test.go index f66d7b155a..3a3096de7c 100644 --- a/webhook/admission_integration_test.go +++ b/webhook/admission_integration_test.go @@ -156,7 +156,7 @@ func TestAdmissionValidResponseForResourceTLS(t *testing.T) { u.Path = path.Join(u.Path, ac.Path()) - req, err := http.NewRequest("GET", u.String(), reqBuf) + req, err := http.NewRequest(http.MethodGet, u.String(), reqBuf) if err != nil { t.Fatal("http.NewRequest() =", err) } @@ -280,7 +280,7 @@ func TestAdmissionValidResponseForResource(t *testing.T) { u.Path = path.Join(u.Path, ac.Path()) - req, err := http.NewRequest("GET", u.String(), reqBuf) + req, err := http.NewRequest(http.MethodGet, u.String(), reqBuf) if err != nil { t.Fatal("http.NewRequest() =", err) } @@ -415,7 +415,7 @@ func TestAdmissionInvalidResponseForResource(t *testing.T) { u.Path = path.Join(u.Path, ac.Path()) - req, err := http.NewRequest("GET", u.String(), reqBuf) + req, err := http.NewRequest(http.MethodGet, u.String(), reqBuf) if err != nil { t.Fatal("http.NewRequest() =", err) } @@ -531,7 +531,7 @@ func TestAdmissionWarningResponseForResource(t *testing.T) { u.Path = path.Join(u.Path, ac.Path()) - req, err := http.NewRequest("GET", u.String(), reqBuf) + req, err := http.NewRequest(http.MethodGet, u.String(), reqBuf) if err != nil { t.Fatal("http.NewRequest() =", err) } @@ -628,7 +628,7 @@ func TestAdmissionValidResponseForRequestBody(t *testing.T) { u.Path = path.Join(u.Path, ac.Path()) - req, err := http.NewRequest("GET", u.String(), reqBuf) + req, err := http.NewRequest(http.MethodGet, u.String(), reqBuf) if err != nil { t.Fatal("http.NewRequest() =", err) } diff --git a/webhook/conversion_integration_test.go b/webhook/conversion_integration_test.go index 15aecb2a8f..8c1e442838 100644 --- a/webhook/conversion_integration_test.go +++ b/webhook/conversion_integration_test.go @@ -105,7 +105,7 @@ func TestConversionValidResponse(t *testing.T) { t.Fatal("Failed to marshal conversion review:", err) } - req, err := http.NewRequest("GET", fmt.Sprintf("https://%s%s", serverURL, cc.Path()), reqBuf) + req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("https://%s%s", serverURL, cc.Path()), reqBuf) if err != nil { t.Fatal("http.NewRequest() =", err) } @@ -193,7 +193,7 @@ func TestConversionInvalidResponse(t *testing.T) { t.Fatal("Failed to marshal conversion review:", err) } - req, err := http.NewRequest("GET", fmt.Sprintf("https://%s%s", serverURL, cc.Path()), reqBuf) + req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("https://%s%s", serverURL, cc.Path()), reqBuf) if err != nil { t.Fatal("http.NewRequest() =", err) } diff --git a/webhook/webhook_integration_test.go b/webhook/webhook_integration_test.go index 595548d5c3..0daa3fbdc3 100644 --- a/webhook/webhook_integration_test.go +++ b/webhook/webhook_integration_test.go @@ -77,7 +77,7 @@ func TestMissingContentType(t *testing.T) { t.Fatal("createSecureTLSClient() =", err) } - req, err := http.NewRequest("GET", fmt.Sprintf("https://%s", serverURL), nil) + req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("https://%s", serverURL), nil) if err != nil { t.Fatal("http.NewRequest() =", err) } @@ -152,7 +152,7 @@ func testEmptyRequestBody(t *testing.T, controller interface{}) { t.Fatal("createSecureTLSClient() =", err) } - req, err := http.NewRequest("GET", fmt.Sprintf("https://%s/bazinga", serverURL), nil) + req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("https://%s/bazinga", serverURL), nil) if err != nil { t.Fatal("http.NewRequest() =", err) } From 9cb7000225143d084219331416cd3fc7c8d0d588 Mon Sep 17 00:00:00 2001 From: dprotaso Date: Sat, 22 Jun 2024 14:12:27 -0400 Subject: [PATCH 03/22] fix staticheck --- configmap/informer/synced_callback.go | 4 ++-- controller/controller.go | 6 +++--- controller/controller_test.go | 16 ++++++++-------- test/helpers/name.go | 6 ++++-- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/configmap/informer/synced_callback.go b/configmap/informer/synced_callback.go index 0b3e2d3a9a..80e776aa90 100644 --- a/configmap/informer/synced_callback.go +++ b/configmap/informer/synced_callback.go @@ -17,10 +17,10 @@ limitations under the License. package informer import ( + "context" "sync" "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/apimachinery/pkg/util/wait" ) // namedWaitGroup is used to increment and decrement a WaitGroup by name @@ -107,6 +107,6 @@ func (s *syncedCallback) WaitForAllKeys(stopCh <-chan struct{}) error { case <-c: return nil case <-stopCh: - return wait.ErrWaitTimeout + return context.DeadlineExceeded } } diff --git a/controller/controller.go b/controller/controller.go index bc7c4be495..16a92c6bc6 100644 --- a/controller/controller.go +++ b/controller/controller.go @@ -762,8 +762,8 @@ func RunInformers(stopCh <-chan struct{}, informers ...Informer) (func(), error) // WaitForCacheSyncQuick is the same as cache.WaitForCacheSync but with a much reduced // check-rate for the sync period. func WaitForCacheSyncQuick(stopCh <-chan struct{}, cacheSyncs ...cache.InformerSynced) bool { - err := wait.PollImmediateUntil(time.Millisecond, - func() (bool, error) { + err := wait.PollUntilContextCancel(wait.ContextForChannel(stopCh), time.Millisecond, true, + func(context.Context) (bool, error) { for _, syncFunc := range cacheSyncs { if !syncFunc() { return false, nil @@ -771,7 +771,7 @@ func WaitForCacheSyncQuick(stopCh <-chan struct{}, cacheSyncs ...cache.InformerS } return true, nil }, - stopCh) + ) return err == nil } diff --git a/controller/controller_test.go b/controller/controller_test.go index 0c29fafedd..261be297a2 100644 --- a/controller/controller_test.go +++ b/controller/controller_test.go @@ -741,8 +741,8 @@ const ( queueCheckTimeout = shortDelay + 500*time.Millisecond ) -func pollQ(q workqueue.RateLimitingInterface, sig chan int) func() (bool, error) { - return func() (bool, error) { +func pollQ(q workqueue.RateLimitingInterface, sig chan int) func(context.Context) (bool, error) { + return func(context.Context) (bool, error) { if ql := q.Len(); ql > 0 { sig <- ql return true, nil @@ -794,8 +794,8 @@ func TestEnqueueAfter(t *testing.T) { cancel() }) - go wait.PollImmediateUntil(5*time.Millisecond, - pollQ(impl.WorkQueue(), queuePopulated), ctx.Done()) + go wait.PollUntilContextCancel(ctx, 5*time.Millisecond, true, + pollQ(impl.WorkQueue(), queuePopulated)) select { case qlen := <-queuePopulated: @@ -846,8 +846,8 @@ func TestEnqueueKeyAfter(t *testing.T) { cancel() }) - go wait.PollImmediateUntil(5*time.Millisecond, - pollQ(impl.WorkQueue(), queuePopulated), ctx.Done()) + go wait.PollUntilContextCancel(ctx, 5*time.Millisecond, true, + pollQ(impl.WorkQueue(), queuePopulated)) select { case qlen := <-queuePopulated: @@ -1211,7 +1211,7 @@ func TestStartAndShutdownWithErroringWork(t *testing.T) { itemRequeued := make(chan struct{}) defer close(itemRequeued) - var successCheck wait.ConditionFunc = func() (bool, error) { + var successCheck wait.ConditionWithContextFunc = func(context.Context) (bool, error) { // Check that the work was requeued in RateLimiter, as NumRequeues // can't fully reflect the real state of queue length. // Here we need to wait for NumRequeues to be more than 1, to ensure @@ -1222,7 +1222,7 @@ func TestStartAndShutdownWithErroringWork(t *testing.T) { } return false, nil } - go wait.PollImmediateUntil(5*time.Millisecond, successCheck, ctx.Done()) + go wait.PollUntilContextCancel(ctx, 5*time.Millisecond, true, successCheck) select { case <-itemRequeued: diff --git a/test/helpers/name.go b/test/helpers/name.go index 0ceaed5943..4da8e4dc84 100644 --- a/test/helpers/name.go +++ b/test/helpers/name.go @@ -32,12 +32,14 @@ const ( testNamePrefix = "Test" ) +var nameRand *rand.Rand + func init() { // Properly seed the random number generator so RandomString() is actually random. // Otherwise, rerunning tests will generate the same names for the test resources, causing conflicts with // already existing resources. seed := time.Now().UTC().UnixNano() - rand.Seed(seed) + nameRand = rand.New(rand.NewSource(seed)) } type named interface { @@ -74,7 +76,7 @@ func AppendRandomString(prefix string) string { func RandomString() string { suffix := make([]byte, randSuffixLen) for i := range suffix { - suffix[i] = letterBytes[rand.Intn(len(letterBytes))] + suffix[i] = letterBytes[nameRand.Intn(len(letterBytes))] } return string(suffix) } From 697d36b6add8df0cae7b0cbe1c73bd1fc10a8503 Mon Sep 17 00:00:00 2001 From: dprotaso Date: Sat, 22 Jun 2024 14:20:47 -0400 Subject: [PATCH 04/22] ignore spancheck warning --- test/logging/logging.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/logging/logging.go b/test/logging/logging.go index 74df6dfdea..0946077aa6 100644 --- a/test/logging/logging.go +++ b/test/logging/logging.go @@ -67,6 +67,8 @@ func (e *zapMetricExporter) ExportView(vd *view.Data) { // GetEmitableSpan starts and returns a trace.Span with a name that // is used by the ExportSpan method to emit the span. +// +//nolint:spancheck func GetEmitableSpan(ctx context.Context, metricName string) *trace.Span { _, span := trace.StartSpan(ctx, emitableSpanNamePrefix+metricName) return span From 776c1513d2819186118b7f337681956a95674912 Mon Sep 17 00:00:00 2001 From: dprotaso Date: Sat, 22 Jun 2024 16:17:42 -0400 Subject: [PATCH 05/22] fix linter perfsprint --- apiextensions/storageversion/migrator_test.go | 5 ++--- apis/condition_set.go | 3 ++- apis/kind2resource.go | 5 ++--- codegen/cmd/injection-gen/args/args.go | 8 ++++---- configmap/filter.go | 8 ++++---- configmap/store_test.go | 3 +-- kmeta/names.go | 3 ++- metrics/resource_view.go | 2 +- metrics/resource_view_test.go | 3 ++- test/ingress/ingress.go | 5 +++-- test/spoof/spoof_test.go | 5 ++--- test/upgrade/testing_operations_test.go | 2 +- testing/resource.go | 2 +- tracing/config/tracing.go | 2 +- webhook/configmaps/configmaps_test.go | 5 +++-- webhook/resourcesemantics/defaulting/defaulting_test.go | 4 ++-- webhook/webhook_integration_test.go | 2 +- 17 files changed, 34 insertions(+), 33 deletions(-) diff --git a/apiextensions/storageversion/migrator_test.go b/apiextensions/storageversion/migrator_test.go index 95a48930a1..a906b6390d 100644 --- a/apiextensions/storageversion/migrator_test.go +++ b/apiextensions/storageversion/migrator_test.go @@ -19,7 +19,6 @@ package storageversion import ( "context" "errors" - "fmt" "testing" "github.com/google/go-cmp/cmp" @@ -109,10 +108,10 @@ func TestMigrate_SingleStoredVersion(t *testing.T) { dclient := dynamicFake.NewSimpleDynamicClient(runtime.NewScheme()) cclient := apixFake.NewSimpleClientset(singleVersionFakeCRD) - cclient.Fake.PrependReactor("patch", "customresourcedefinitions", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + cclient.PrependReactor("patch", "customresourcedefinitions", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { if pa, ok := action.(k8stesting.PatchAction); ok { if pa.GetName() == singleVersionFakeCRD.Name { - return false, nil, fmt.Errorf("resource shouldn't have been patched") + return false, nil, errors.New("resource shouldn't have been patched") } } diff --git a/apis/condition_set.go b/apis/condition_set.go index 9ad14902e7..940d1b3ab3 100644 --- a/apis/condition_set.go +++ b/apis/condition_set.go @@ -17,6 +17,7 @@ limitations under the License. package apis import ( + "errors" "fmt" "reflect" "sort" @@ -233,7 +234,7 @@ func (r conditionsImpl) ClearCondition(t ConditionType) error { } // Terminal conditions are not handled as they can't be nil if r.isTerminal(t) { - return fmt.Errorf("clearing terminal conditions not implemented") + return errors.New("clearing terminal conditions not implemented") } cond := r.GetCondition(t) if cond == nil { diff --git a/apis/kind2resource.go b/apis/kind2resource.go index 37ffe08034..b1937086e6 100644 --- a/apis/kind2resource.go +++ b/apis/kind2resource.go @@ -17,7 +17,6 @@ limitations under the License. package apis import ( - "fmt" "strings" "k8s.io/apimachinery/pkg/runtime/schema" @@ -41,7 +40,7 @@ func KindToResource(gvk schema.GroupVersionKind) schema.GroupVersionResource { func pluralizeKind(kind string) string { ret := strings.ToLower(kind) if strings.HasSuffix(ret, "s") { - return fmt.Sprintf("%ses", ret) + return ret + "es" } - return fmt.Sprintf("%ss", ret) + return ret + "s" } diff --git a/codegen/cmd/injection-gen/args/args.go b/codegen/cmd/injection-gen/args/args.go index 265b4e40ae..92595e2ffe 100644 --- a/codegen/cmd/injection-gen/args/args.go +++ b/codegen/cmd/injection-gen/args/args.go @@ -17,7 +17,7 @@ limitations under the License. package args import ( - "fmt" + "errors" "github.com/spf13/pflag" "k8s.io/gengo/args" @@ -59,13 +59,13 @@ func Validate(genericArgs *args.GeneratorArgs) error { customArgs := genericArgs.CustomArgs.(*CustomArgs) if len(genericArgs.OutputPackagePath) == 0 { - return fmt.Errorf("output package cannot be empty") + return errors.New("output package cannot be empty") } if len(customArgs.VersionedClientSetPackage) == 0 { - return fmt.Errorf("versioned clientset package cannot be empty") + return errors.New("versioned clientset package cannot be empty") } if len(customArgs.ExternalVersionsInformersPackage) == 0 { - return fmt.Errorf("external versions informers package cannot be empty") + return errors.New("external versions informers package cannot be empty") } return nil diff --git a/configmap/filter.go b/configmap/filter.go index ed1040e27b..6fb332a8cf 100644 --- a/configmap/filter.go +++ b/configmap/filter.go @@ -17,7 +17,7 @@ limitations under the License. package configmap import ( - "fmt" + "errors" "reflect" corev1 "k8s.io/api/core/v1" @@ -58,17 +58,17 @@ func ValidateConstructor(constructor interface{}) error { cType := reflect.TypeOf(constructor) if cType.Kind() != reflect.Func { - return fmt.Errorf("config constructor must be a function") + return errors.New("config constructor must be a function") } if cType.NumIn() != 1 || cType.In(0) != reflect.TypeOf(&corev1.ConfigMap{}) { - return fmt.Errorf("config constructor must be of the type func(*k8s.io/api/core/v1/ConfigMap) (..., error)") + return errors.New("config constructor must be of the type func(*k8s.io/api/core/v1/ConfigMap) (..., error)") } errorType := reflect.TypeOf((*error)(nil)).Elem() if cType.NumOut() != 2 || !cType.Out(1).Implements(errorType) { - return fmt.Errorf("config constructor must be of the type func(*k8s.io/api/core/v1/ConfigMap) (..., error)") + return errors.New("config constructor must be of the type func(*k8s.io/api/core/v1/ConfigMap) (..., error)") } return nil } diff --git a/configmap/store_test.go b/configmap/store_test.go index 75d20fd563..5c64dbd25c 100644 --- a/configmap/store_test.go +++ b/configmap/store_test.go @@ -18,7 +18,6 @@ package configmap import ( "errors" - "fmt" "os" "os/exec" "testing" @@ -196,7 +195,7 @@ func TestStoreFailedFirstConversionCrashes(t *testing.T) { return } - cmd := exec.Command(os.Args[0], fmt.Sprintf("-test.run=%s", t.Name())) + cmd := exec.Command(os.Args[0], "-test.run="+t.Name()) cmd.Env = append(os.Environ(), "CRASH=1") err := cmd.Run() diff --git a/kmeta/names.go b/kmeta/names.go index 963b121d29..0977544c8c 100644 --- a/kmeta/names.go +++ b/kmeta/names.go @@ -18,6 +18,7 @@ package kmeta import ( "crypto/md5" //nolint:gosec // No strong cryptography needed. + "encoding/hex" "fmt" "regexp" ) @@ -53,7 +54,7 @@ func ChildName(parent, suffix string) string { // Format the return string, if it's shorter than longest: pad with // beginning of the suffix. This happens, for example, when parent is // short, but the suffix is very long. - ret := parent + fmt.Sprintf("%x", h) + ret := parent + hex.EncodeToString(h[:]) if d := longest - len(ret); d > 0 { ret += suffix[:d] } diff --git a/metrics/resource_view.go b/metrics/resource_view.go index 930b389319..d4e2c599da 100644 --- a/metrics/resource_view.go +++ b/metrics/resource_view.go @@ -307,7 +307,7 @@ func optionForResource(r *resource.Resource) (stats.Options, error) { // If we can't create exporters but we have a Meter, return that. return mE.o, nil } - return nil, fmt.Errorf("whoops, allMeters.factory is nil") + return nil, errors.New("whoops, allMeters.factory is nil") } exporter, err := allMeters.factory(r) if err != nil { diff --git a/metrics/resource_view_test.go b/metrics/resource_view_test.go index 9529edb0d4..3bb2faea1f 100644 --- a/metrics/resource_view_test.go +++ b/metrics/resource_view_test.go @@ -19,6 +19,7 @@ package metrics import ( "context" "fmt" + "strconv" "testing" "time" @@ -204,7 +205,7 @@ func TestIfAllMeterResourcesAreRemoved(t *testing.T) { // Register many resources at once for i := 1; i <= 100; i++ { res := resource.Resource{Labels: map[string]string{"foo": "bar"}} - res.Labels["id"] = fmt.Sprint(i) + res.Labels["id"] = strconv.Itoa(i) if _, err := optionForResource(&res); err != nil { t.Error("Should succeed getting option, instead got error ", err) } diff --git a/test/ingress/ingress.go b/test/ingress/ingress.go index 8de4052c75..1bf38b8cc5 100644 --- a/test/ingress/ingress.go +++ b/test/ingress/ingress.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "os" + "strconv" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -59,8 +60,8 @@ func GetIngressEndpoint(ctx context.Context, kubeClientset kubernetes.Interface, if endpointOverride != "" { return endpointOverride, func(port string) string { for _, sp := range ingress.Spec.Ports { - if fmt.Sprint(sp.Port) == port { - return fmt.Sprint(sp.NodePort) + if strconv.Itoa(int(sp.Port)) == port { + return strconv.Itoa(int(sp.NodePort)) } } return port diff --git a/test/spoof/spoof_test.go b/test/spoof/spoof_test.go index 2c00712e60..5a43a78149 100644 --- a/test/spoof/spoof_test.go +++ b/test/spoof/spoof_test.go @@ -21,7 +21,6 @@ package spoof import ( "context" "errors" - "fmt" "net/http" "net/url" "sync/atomic" @@ -75,7 +74,7 @@ func TestSpoofingClient_CheckEndpointState(t *testing.T) { name: "Error response doesn't trigger a second check", transport: &fakeTransport{response: successResponse}, inState: func(resp *Response) (done bool, err error) { - return false, fmt.Errorf("response error") + return false, errors.New("response error") }, wantErr: true, wantCalls: 1, @@ -147,7 +146,7 @@ func TestSpoofingClient_WaitForEndpointState(t *testing.T) { name: "Error response doesn't trigger more requests", transport: &fakeTransport{response: successResponse}, inState: func(resp *Response) (done bool, err error) { - return false, fmt.Errorf("response error") + return false, errors.New("response error") }, wantErr: true, wantCalls: 1, diff --git a/test/upgrade/testing_operations_test.go b/test/upgrade/testing_operations_test.go index 0142aefe2a..54646b4f5e 100644 --- a/test/upgrade/testing_operations_test.go +++ b/test/upgrade/testing_operations_test.go @@ -238,7 +238,7 @@ func (o operation) Name() string { } func (o *operation) fail(setupFail bool) { - testName := fmt.Sprintf("FailingOf%s", o.Name()) + testName := "FailingOf" + o.Name() if o.op != nil { prev := o.op o.op = upgrade.NewOperation(testName, func(c upgrade.Context) { diff --git a/testing/resource.go b/testing/resource.go index cb95ac60a9..6f892cc9df 100644 --- a/testing/resource.go +++ b/testing/resource.go @@ -156,7 +156,7 @@ func (r *Resource) CheckAllowedSubresourceUpdate(ctx context.Context, original * if apis.GetUpdatedSubresource(ctx) == disallowedSubresource { return &apis.FieldError{ Message: "Disallowed subresource update", - Details: fmt.Sprintf("Disallowed subresource update: %s", disallowedSubresource), + Details: "Disallowed subresource update: " + disallowedSubresource, } } return nil diff --git a/tracing/config/tracing.go b/tracing/config/tracing.go index 4d1831fc1b..ed615c291e 100644 --- a/tracing/config/tracing.go +++ b/tracing/config/tracing.go @@ -150,7 +150,7 @@ func TracingConfigToJSON(cfg *Config) (string, error) { //nolint // for backcomp if cfg.ZipkinEndpoint != "" { out[zipkinEndpointKey] = cfg.ZipkinEndpoint } - out[debugKey] = fmt.Sprint(cfg.Debug) + out[debugKey] = strconv.FormatBool(cfg.Debug) out[sampleRateKey] = fmt.Sprint(cfg.SampleRate) jsonCfg, err := json.Marshal(out) diff --git a/webhook/configmaps/configmaps_test.go b/webhook/configmaps/configmaps_test.go index ae37d4fa76..bddc0d0397 100644 --- a/webhook/configmaps/configmaps_test.go +++ b/webhook/configmaps/configmaps_test.go @@ -19,6 +19,7 @@ package configmaps import ( "context" "encoding/json" + "errors" "fmt" "strconv" "testing" @@ -274,7 +275,7 @@ func newConfigFromConfigMap(configMap *corev1.ConfigMap) (*config, error) { field: &cfg.value, }} { if raw, ok := data[b.key]; !ok { - return nil, fmt.Errorf("not found") + return nil, errors.New("not found") } else if val, err := strconv.ParseFloat(raw, 64); err != nil { return nil, err } else { @@ -284,7 +285,7 @@ func newConfigFromConfigMap(configMap *corev1.ConfigMap) (*config, error) { // some sample validation on the value if cfg.value > 2.0 || cfg.value < 0.0 { - return nil, fmt.Errorf("out of range") + return nil, errors.New("out of range") } return cfg, nil diff --git a/webhook/resourcesemantics/defaulting/defaulting_test.go b/webhook/resourcesemantics/defaulting/defaulting_test.go index 4f88c6b84a..3e83c648f9 100644 --- a/webhook/resourcesemantics/defaulting/defaulting_test.go +++ b/webhook/resourcesemantics/defaulting/defaulting_test.go @@ -978,13 +978,13 @@ func resourceCallback(ctx context.Context, uns *unstructured.Unstructured) error resource.Spec.FieldForCallbackDefaulting = "I'm a default" if apis.IsInUpdate(ctx) { if apis.GetBaseline(ctx) == nil { - return fmt.Errorf("expected baseline object") + return errors.New("expected baseline object") } if v, ok := apis.GetBaseline(ctx).(*unstructured.Unstructured); !ok { return fmt.Errorf("expected *unstructured.Unstructured, got %v", reflect.TypeOf(v)) } } else if !apis.IsInCreate(ctx) { - return fmt.Errorf("expected to have context within update or create") + return errors.New("expected to have context within update or create") } } diff --git a/webhook/webhook_integration_test.go b/webhook/webhook_integration_test.go index 0daa3fbdc3..4ff476a29f 100644 --- a/webhook/webhook_integration_test.go +++ b/webhook/webhook_integration_test.go @@ -77,7 +77,7 @@ func TestMissingContentType(t *testing.T) { t.Fatal("createSecureTLSClient() =", err) } - req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("https://%s", serverURL), nil) + req, err := http.NewRequest(http.MethodGet, "https://"+serverURL, nil) if err != nil { t.Fatal("http.NewRequest() =", err) } From cb82367d05099d7dd3cc8a9d72bcd4209881cab6 Mon Sep 17 00:00:00 2001 From: dprotaso Date: Sat, 22 Jun 2024 20:52:57 -0400 Subject: [PATCH 06/22] fix nolintlint feedback --- controller/controller.go | 5 +---- metrics/prometheus_exporter.go | 1 - tracing/config/tracing.go | 2 +- webhook/json/decode.go | 2 +- webhook/resourcesemantics/validation/validation_admit.go | 3 ++- 5 files changed, 5 insertions(+), 8 deletions(-) diff --git a/controller/controller.go b/controller/controller.go index 16a92c6bc6..536ae793a0 100644 --- a/controller/controller.go +++ b/controller/controller.go @@ -224,7 +224,7 @@ type Impl struct { // ControllerOptions encapsulates options for creating a new controller, // including throttling and stats behavior. -type ControllerOptions struct { //nolint // for backcompat. +type ControllerOptions struct { WorkQueueName string Logger *zap.SugaredLogger Reporter StatsReporter @@ -623,7 +623,6 @@ func IsSkipKey(err error) bool { // Is implements the Is() interface of error. It returns whether the target // error can be treated as equivalent to a permanentError. func (skipKeyError) Is(target error) bool { - //nolint: errorlint // This check is actually fine. _, ok := target.(skipKeyError) return ok } @@ -650,7 +649,6 @@ func IsPermanentError(err error) bool { // Is implements the Is() interface of error. It returns whether the target // error can be treated as equivalent to a permanentError. func (permanentError) Is(target error) bool { - //nolint: errorlint // This check is actually fine. _, ok := target.(permanentError) return ok } @@ -710,7 +708,6 @@ func IsRequeueKey(err error) (bool, time.Duration) { // Is implements the Is() interface of error. It returns whether the target // error can be treated as equivalent to a requeueKeyError. func (requeueKeyError) Is(target error) bool { - //nolint: errorlint // This check is actually fine. _, ok := target.(requeueKeyError) return ok } diff --git a/metrics/prometheus_exporter.go b/metrics/prometheus_exporter.go index cb5238476f..7dfcc094d8 100644 --- a/metrics/prometheus_exporter.go +++ b/metrics/prometheus_exporter.go @@ -42,7 +42,6 @@ func (emptyPromExporter) ExportView(viewData *view.Data) { // a signal to enrich the internal Meters with Resource information. } -// nolint: unparam // False positive of flagging the second result of this function unused. func newPrometheusExporter(config *metricsConfig, logger *zap.SugaredLogger) (view.Exporter, ResourceExporterFactory, error) { e, err := prom.NewExporter(prom.Options{Namespace: config.component}) if err != nil { diff --git a/tracing/config/tracing.go b/tracing/config/tracing.go index ed615c291e..953b5a388b 100644 --- a/tracing/config/tracing.go +++ b/tracing/config/tracing.go @@ -140,7 +140,7 @@ func JSONToTracingConfig(jsonCfg string) (*Config, error) { return cfg, nil } -func TracingConfigToJSON(cfg *Config) (string, error) { //nolint // for backcompat. +func TracingConfigToJSON(cfg *Config) (string, error) { if cfg == nil { return "", nil } diff --git a/webhook/json/decode.go b/webhook/json/decode.go index c8e5707907..54fb352b30 100644 --- a/webhook/json/decode.go +++ b/webhook/json/decode.go @@ -95,7 +95,7 @@ func findMetadataOffsets(bites []byte) (start, end int64, err error) { for { t, err = dec.Token() - if err == io.EOF { //nolint + if err == io.EOF { break } if err != nil { diff --git a/webhook/resourcesemantics/validation/validation_admit.go b/webhook/resourcesemantics/validation/validation_admit.go index ed5b834920..94093e6482 100644 --- a/webhook/resourcesemantics/validation/validation_admit.go +++ b/webhook/resourcesemantics/validation/validation_admit.go @@ -160,7 +160,8 @@ func (ac *reconciler) decodeRequestAndPrepareContext( return ctx, newObj, nil } -func validate(ctx context.Context, resource resourcesemantics.GenericCRD, req *admissionv1.AdmissionRequest) (err error, warn []error) { //nolint +//nolint:stylecheck +func validate(ctx context.Context, resource resourcesemantics.GenericCRD, req *admissionv1.AdmissionRequest) (err error, warn []error) { logger := logging.FromContext(ctx) // Only run validation for supported create and update validation. From e67e54a9cf3d43404c1c1ddcb6102851fbba991c Mon Sep 17 00:00:00 2001 From: dprotaso Date: Sat, 22 Jun 2024 20:55:41 -0400 Subject: [PATCH 07/22] fix nilerr lint checks --- tracing/config/tracing.go | 2 +- webhook/psbinding/reconciler.go | 2 +- websocket/connection_test.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tracing/config/tracing.go b/tracing/config/tracing.go index 953b5a388b..633ce3d4b2 100644 --- a/tracing/config/tracing.go +++ b/tracing/config/tracing.go @@ -135,7 +135,7 @@ func JSONToTracingConfig(jsonCfg string) (*Config, error) { cfg, err := NewTracingConfigFromMap(configMap) if err != nil { - return NoopConfig(), nil + return NoopConfig(), nil //nolint:nilerr } return cfg, nil } diff --git a/webhook/psbinding/reconciler.go b/webhook/psbinding/reconciler.go index 233af0d389..0dae1d8f15 100644 --- a/webhook/psbinding/reconciler.go +++ b/webhook/psbinding/reconciler.go @@ -113,7 +113,7 @@ func (r *BaseReconciler) Reconcile(ctx context.Context, key string) error { namespace, name, err := cache.SplitMetaNamespaceKey(key) if err != nil { logging.FromContext(ctx).Error("invalid resource key: ", key) - return nil + return nil //nolint:nilerr } // Only the leader should reconcile binding resources. diff --git a/websocket/connection_test.go b/websocket/connection_test.go index 5caab95caa..916c25ee0f 100644 --- a/websocket/connection_test.go +++ b/websocket/connection_test.go @@ -364,7 +364,7 @@ func TestDurableConnectionWhenConnectionBreaksDown(t *testing.T) { for i := 0; i < 10; i++ { err := wait.PollUntilContextTimeout(context.Background(), 50*time.Millisecond, 5*time.Second, true, func(ctx context.Context) (bool, error) { if err := conn.Send(testPayload); err != nil { - return false, nil + return false, nil //nolint:nilerr } return true, nil }) From ae55b0c4d2a133ac8640f3f6c98e1cf88d7bc66f Mon Sep 17 00:00:00 2001 From: dprotaso Date: Sat, 22 Jun 2024 20:58:52 -0400 Subject: [PATCH 08/22] fix misspell --- apiextensions/storageversion/migrator_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apiextensions/storageversion/migrator_test.go b/apiextensions/storageversion/migrator_test.go index a906b6390d..bd17e867a1 100644 --- a/apiextensions/storageversion/migrator_test.go +++ b/apiextensions/storageversion/migrator_test.go @@ -176,7 +176,7 @@ func TestMigrate_Errors(t *testing.T) { return true, nil, apierrs.NewNotFound(fakeGR, "resource-removed") }) }, - // Resouce not found error should not block the storage migration. + // Resource not found error should not block the storage migration. pass: true, }, // todo paging fails From 0025b7c4576c045c1e62e76506c2580919fd3fab Mon Sep 17 00:00:00 2001 From: dprotaso Date: Sat, 22 Jun 2024 21:00:44 -0400 Subject: [PATCH 09/22] fix mirror lint --- tracker/enqueue_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tracker/enqueue_test.go b/tracker/enqueue_test.go index 1703b12e61..2f7e926e1c 100644 --- a/tracker/enqueue_test.go +++ b/tracker/enqueue_test.go @@ -401,7 +401,7 @@ func TestBadObjectReferences(t *testing.T) { if err := trk.Track(test.objRef, thing1); err == nil { t.Fatal("Track() = nil, wanted error") } else { - match, e2 := regexp.Match(test.match, []byte(err.Error())) + match, e2 := regexp.MatchString(test.match, err.Error()) if e2 != nil { t.Fatalf("Failed to compile %q: %v", e2, test.match) } else if !match { @@ -463,7 +463,7 @@ func TestBadReferences(t *testing.T) { if err := trk.TrackReference(test.objRef, thing1); err == nil { t.Fatal("Track() = nil, wanted error") } else { - match, e2 := regexp.Match(test.match, []byte(err.Error())) + match, e2 := regexp.MatchString(test.match, err.Error()) if e2 != nil { t.Fatalf("Failed to compile %q: %v", e2, test.match) } else if !match { From 3ca877f9cbd2549a22ddfcf859f68e808135e8a2 Mon Sep 17 00:00:00 2001 From: dprotaso Date: Sun, 23 Jun 2024 16:25:11 -0400 Subject: [PATCH 10/22] fix intrange linter --- apis/deprecated.go | 2 +- apis/duck/cached_test.go | 4 ++-- apis/field_error_test.go | 4 ++-- apis/testing/roundtrip/roundtrip.go | 2 +- controller/controller.go | 2 +- hash/hash_test.go | 6 +++--- leaderelection/context.go | 4 ++-- leaderelection/context_test.go | 8 ++++---- metrics/resource_view_test.go | 4 ++-- network/handlers/drain_test.go | 2 +- test/vegeta/pacers/steady_up_pacer.go | 2 +- tracing/sampler_test.go | 2 +- websocket/connection_test.go | 4 ++-- 13 files changed, 23 insertions(+), 23 deletions(-) diff --git a/apis/deprecated.go b/apis/deprecated.go index 8f07e71b31..b5dfde2fbb 100644 --- a/apis/deprecated.go +++ b/apis/deprecated.go @@ -94,7 +94,7 @@ func getPrefixedNamedFieldValues(prefix string, obj interface{}) (map[string]ref return fields, inlined } - for i := 0; i < objValue.NumField(); i++ { + for i := range objValue.NumField() { tf := objValue.Type().Field(i) if v := objValue.Field(i); v.IsValid() { jTag := tf.Tag.Get("json") diff --git a/apis/duck/cached_test.go b/apis/duck/cached_test.go index 656425129f..e2c7d192e1 100644 --- a/apis/duck/cached_test.go +++ b/apis/duck/cached_test.go @@ -72,7 +72,7 @@ func TestSameGVR(t *testing.T) { } const iter = 10 - for i := 0; i < iter; i++ { + for range iter { errGrp.Go(func() error { _, _, err := cif.Get(ctx, gvr) retGetCount.Add(1) @@ -124,7 +124,7 @@ func TestDifferentGVRs(t *testing.T) { errGrp, ctx := errgroup.WithContext(context.Background()) const iter = 10 - for i := 0; i < iter; i++ { + for i := range iter { // Use a different GVR each iteration to check that calls // to bif.Get can proceed even if a call is in progress // for another GVR. diff --git a/apis/field_error_test.go b/apis/field_error_test.go index 4a3828dca1..5cf8276d90 100644 --- a/apis/field_error_test.go +++ b/apis/field_error_test.go @@ -227,7 +227,7 @@ Second: X, Y, Z`, } for _, p := range []string{"3", "2", "1"} { - for i := 0; i < 3; i++ { + for range 3 { fe = fe.Also(fe) } fe = fe.ViaField(p) @@ -247,7 +247,7 @@ Second: X, Y, Z`, for _, p := range []string{"3", "2", "1"} { e := fe.ViaField(p) e.Details = "here at " + p - for i := 0; i < 3; i++ { + for range 3 { fe = fe.Also(e) } } diff --git a/apis/testing/roundtrip/roundtrip.go b/apis/testing/roundtrip/roundtrip.go index bdf33349f1..54493b02e2 100644 --- a/apis/testing/roundtrip/roundtrip.go +++ b/apis/testing/roundtrip/roundtrip.go @@ -144,7 +144,7 @@ func ExternalTypesViaHub(t *testing.T, scheme, hubs *runtime.Scheme, fuzzerFuncs } t.Run(gvk.Group+"."+gvk.Version+"."+gvk.Kind, func(t *testing.T) { - for i := 0; i < *roundtrip.FuzzIters; i++ { + for range *roundtrip.FuzzIters { roundTripViaHub(t, gvk, scheme, hubs, f) if t.Failed() { diff --git a/controller/controller.go b/controller/controller.go index 536ae793a0..153142f3bb 100644 --- a/controller/controller.go +++ b/controller/controller.go @@ -482,7 +482,7 @@ func (c *Impl) RunContext(ctx context.Context, threadiness int) error { // Launch workers to process resources that get enqueued to our workqueue. c.logger.Info("Starting controller and workers") - for i := 0; i < threadiness; i++ { + for range threadiness { sg.Add(1) go func() { defer sg.Done() diff --git a/hash/hash_test.go b/hash/hash_test.go index 4e26fb3325..a1706d0c2c 100644 --- a/hash/hash_test.go +++ b/hash/hash_test.go @@ -149,12 +149,12 @@ func TestOverlay(t *testing.T) { threshold = want / 5 // 20% ) from := sets.New[string]() - for i := 0; i < sources; i++ { + for range sources { from.Insert(uuid.NewString()) } freqs := make(map[string]int, sources) - for i := 0; i < samples; i++ { + for range samples { target := uuid.NewString() got := ChooseSubset(from, selection, target) for k := range got { @@ -177,7 +177,7 @@ func TestOverlay(t *testing.T) { func BenchmarkSelection(b *testing.B) { const maxSet = 200 from := make([]string, maxSet) - for i := 0; i < maxSet; i++ { + for i := range maxSet { from[i] = uuid.NewString() } for _, v := range []int{5, 10, 25, 50, 100, 150, maxSet} { diff --git a/leaderelection/context.go b/leaderelection/context.go index 4cd6830639..1d336cfdab 100644 --- a/leaderelection/context.go +++ b/leaderelection/context.go @@ -192,7 +192,7 @@ func newStandardBuckets(queueName string, cc ComponentConfig) []reconciler.Bucke } } names := make(sets.Set[string], cc.Buckets) - for i := uint32(0); i < cc.Buckets; i++ { + for i := range cc.Buckets { names.Insert(ln(i)) } @@ -239,7 +239,7 @@ func NewStatefulSetBucketAndSet(buckets int) (reconciler.Bucket, *hash.BucketSet } names := make(sets.Set[string], buckets) - for i := 0; i < buckets; i++ { + for i := range buckets { names.Insert(statefulSetPodDNS(i, ssc)) } diff --git a/leaderelection/context_test.go b/leaderelection/context_test.go index f9822fd9a6..01ff6466c0 100644 --- a/leaderelection/context_test.go +++ b/leaderelection/context_test.go @@ -118,7 +118,7 @@ func TestWithBuilder(t *testing.T) { go le.Run(ctx) // We expect 3 lease to be created. - for i := 0; i < buckets; i++ { + for range buckets { select { case <-created: case <-time.After(1 * time.Second): @@ -126,7 +126,7 @@ func TestWithBuilder(t *testing.T) { } } // We expect to have been promoted 3 times. - for i := 0; i < buckets; i++ { + for range buckets { select { case s := <-promoted: gotNames.Insert(s) @@ -145,7 +145,7 @@ func TestWithBuilder(t *testing.T) { t.Fatal("Timed out waiting for lease update.") } // We expect to have been demoted 3 times. - for i := 0; i < buckets; i++ { + for range buckets { select { case <-demoted: case <-time.After(time.Second): @@ -198,7 +198,7 @@ func TestBuilderWithCustomizedLeaseName(t *testing.T) { go le.Run(ctx) // We expect to have been promoted 3 times. - for i := 0; i < buckets; i++ { + for range buckets { select { case s := <-promoted: gotNames.Insert(s) diff --git a/metrics/resource_view_test.go b/metrics/resource_view_test.go index 3bb2faea1f..4475d3de1f 100644 --- a/metrics/resource_view_test.go +++ b/metrics/resource_view_test.go @@ -266,7 +266,7 @@ func TestResourceAsString(t *testing.T) { r3 := &resource.Resource{Type: "foobar", Labels: map[string]string{"k1": "v1", "k2": "v2", "k4": "v4"}} // Test 5 time since the iteration could be random. - for i := 0; i < 5; i++ { + for range 5 { if s1, s2 := resourceToKey(r1), resourceToKey(r2); s1 != s2 { t.Errorf("Expect same resources, but got %q and %q", s1, s2) } @@ -280,7 +280,7 @@ func TestResourceAsString(t *testing.T) { func BenchmarkResourceToKey(b *testing.B) { for _, count := range []int{0, 1, 5, 10} { labels := make(map[string]string, count) - for i := 0; i < count; i++ { + for i := range count { labels[fmt.Sprint("key", i)] = fmt.Sprint("value", i) } r := &resource.Resource{Type: "foobar", Labels: labels} diff --git a/network/handlers/drain_test.go b/network/handlers/drain_test.go index 31205ac389..b31218b509 100644 --- a/network/handlers/drain_test.go +++ b/network/handlers/drain_test.go @@ -154,7 +154,7 @@ func TestDrainMechanics(t *testing.T) { } rc++ - for i := 0; i < 3; i++ { + for i := range 3 { mt.advance(timeout - epsilon) select { case <-done: diff --git a/test/vegeta/pacers/steady_up_pacer.go b/test/vegeta/pacers/steady_up_pacer.go index fa4878f188..d396a77fce 100644 --- a/test/vegeta/pacers/steady_up_pacer.go +++ b/test/vegeta/pacers/steady_up_pacer.go @@ -96,7 +96,7 @@ func (sup *steadyUpPacer) Pace(elapsedTime time.Duration, elapsedHits uint64) (t // If we can't converge to an error of <1e-3 within 10 iterations, bail. // This rarely even loops for any large Period if hitsToWait is small. - for i := 0; i < 10; i++ { + for range 10 { hitsAtGuess := sup.hits(elapsedTime + nextHitIn) err := float64(elapsedHits+1) - hitsAtGuess if math.Abs(err) < 1e-3 { diff --git a/tracing/sampler_test.go b/tracing/sampler_test.go index a247ce60f5..7a4ab5ec27 100644 --- a/tracing/sampler_test.go +++ b/tracing/sampler_test.go @@ -78,7 +78,7 @@ func TestCreateOCTConfig(t *testing.T) { octCfg := createOCTConfig(&tc.cfg) // Create 100 traceIDs and make sure our expected sampler samples the same as what we get - for i := 0; i < 100; i++ { + for range 100 { spanID := make([]byte, 8) rand.Read(spanID) param := trace.SamplingParameters{} diff --git a/websocket/connection_test.go b/websocket/connection_test.go index 916c25ee0f..7b35a6278f 100644 --- a/websocket/connection_test.go +++ b/websocket/connection_test.go @@ -361,7 +361,7 @@ func TestDurableConnectionWhenConnectionBreaksDown(t *testing.T) { conn := NewDurableSendingConnection(target, logger) defer conn.Shutdown() - for i := 0; i < 10; i++ { + for range 10 { err := wait.PollUntilContextTimeout(context.Background(), 50*time.Millisecond, 5*time.Second, true, func(ctx context.Context) (bool, error) { if err := conn.Send(testPayload); err != nil { return false, nil //nolint:nilerr @@ -414,7 +414,7 @@ func TestDurableConnectionSendsPingsRegularly(t *testing.T) { defer conn.Shutdown() // Wait for 5 pings to be received by the server. - for i := 0; i < 5; i++ { + for range 5 { <-pingReceived } } From dc443152c24c2003dd7c6965f3a546ec29cb1eda Mon Sep 17 00:00:00 2001 From: dprotaso Date: Sun, 23 Jun 2024 16:26:11 -0400 Subject: [PATCH 11/22] fix gofumpt linter --- apis/duck/verify_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apis/duck/verify_test.go b/apis/duck/verify_test.go index 8dd7e23795..7de7a91501 100644 --- a/apis/duck/verify_test.go +++ b/apis/duck/verify_test.go @@ -302,6 +302,7 @@ type ( Status SliceStatus `json:"status"` } ) + type SliceStatus struct { Sliceable *Sliceable `json:"sliceable,omitempty"` } @@ -338,6 +339,7 @@ type ( Status StringStatus `json:"status"` } ) + type StringStatus struct { Stringable Stringable `json:"stringable,omitempty"` } From 654bcf71df32bb44d449bfc0c2463b9f54b3e016 Mon Sep 17 00:00:00 2001 From: dprotaso Date: Sun, 23 Jun 2024 16:40:43 -0400 Subject: [PATCH 12/22] fix gosec linter - ignore warning since default min tls version is 1.3 --- webhook/webhook.go | 1 + 1 file changed, 1 insertion(+) diff --git a/webhook/webhook.go b/webhook/webhook.go index e91262db2c..e05c6f041e 100644 --- a/webhook/webhook.go +++ b/webhook/webhook.go @@ -176,6 +176,7 @@ func New( // a new secret informer from it. secretInformer := kubeinformerfactory.Get(ctx).Core().V1().Secrets() + //nolint:gosec // operator configures TLS min version (default is 1.3) webhook.tlsConfig = &tls.Config{ MinVersion: opts.TLSMinVersion, From 6c91f1112807ea5d1c4f00f70ca79db982d4b1a0 Mon Sep 17 00:00:00 2001 From: dprotaso Date: Sun, 23 Jun 2024 20:46:42 -0400 Subject: [PATCH 13/22] fix gocritic linter --- apis/duck/v1/destination.go | 4 ++-- apis/duck/v1beta1/destination.go | 4 ++-- codegen/cmd/injection-gen/generators/packages.go | 3 ++- metrics/opencensus_exporter.go | 4 +++- reconciler/testing/table.go | 4 +++- webhook/resourcesemantics/conversion/internal/types.go | 2 +- 6 files changed, 13 insertions(+), 8 deletions(-) diff --git a/apis/duck/v1/destination.go b/apis/duck/v1/destination.go index 720d406776..34c1af94f9 100644 --- a/apis/duck/v1/destination.go +++ b/apis/duck/v1/destination.go @@ -103,11 +103,11 @@ func (d *Destination) SetDefaults(ctx context.Context) { } } -func validateCACerts(CACert *string) *apis.FieldError { +func validateCACerts(caCert *string) *apis.FieldError { // Check the object. var errs *apis.FieldError - block, err := pem.Decode([]byte(*CACert)) + block, err := pem.Decode([]byte(*caCert)) if err != nil && block == nil { errs = errs.Also(apis.ErrInvalidValue("CA Cert provided is invalid", "caCert")) return errs diff --git a/apis/duck/v1beta1/destination.go b/apis/duck/v1beta1/destination.go index 41c7702df1..bc6e56ae41 100644 --- a/apis/duck/v1beta1/destination.go +++ b/apis/duck/v1beta1/destination.go @@ -168,11 +168,11 @@ func validateDestinationRef(ref corev1.ObjectReference) *apis.FieldError { return errs } -func validateCACerts(CACert *string) *apis.FieldError { +func validateCACerts(caCert *string) *apis.FieldError { // Check the object. var errs *apis.FieldError - block, err := pem.Decode([]byte(*CACert)) + block, err := pem.Decode([]byte(*caCert)) if err != nil && block == nil { errs = errs.Also(apis.ErrInvalidValue("CA Cert provided is invalid", "caCert")) return errs diff --git a/codegen/cmd/injection-gen/generators/packages.go b/codegen/cmd/injection-gen/generators/packages.go index 0422c94644..f963094401 100644 --- a/codegen/cmd/injection-gen/generators/packages.go +++ b/codegen/cmd/injection-gen/generators/packages.go @@ -43,7 +43,7 @@ func Packages(context *generator.Context, arguments *args.GeneratorArgs) generat klog.Fatalf("Wrong CustomArgs type: %T", arguments.CustomArgs) } - versionPackagePath := filepath.Join(arguments.OutputPackagePath) + versionPackagePath := filepath.Clean(arguments.OutputPackagePath) var packageList generator.Packages @@ -501,6 +501,7 @@ func versionInformerPackages(basePackage string, groupPkgName string, gv clientg return tags.NeedsInformerInjection() }, }) + } return vers } diff --git a/metrics/opencensus_exporter.go b/metrics/opencensus_exporter.go index 59e33ab099..8ea0f6e419 100644 --- a/metrics/opencensus_exporter.go +++ b/metrics/opencensus_exporter.go @@ -64,7 +64,9 @@ func getFactory(defaultExporter view.Exporter, stored []ocagent.ExporterOption) // Don't create duplicate exporters for the default exporter. return defaultExporter, nil } - opts := append(stored, ocagent.WithResourceDetector( + opts := make([]ocagent.ExporterOption, 0, len(stored)+1) + opts = append(opts, stored...) + opts = append(opts, ocagent.WithResourceDetector( func(context.Context) (*resource.Resource, error) { return r, nil })) diff --git a/reconciler/testing/table.go b/reconciler/testing/table.go index 18a83a2bb5..97cc921572 100644 --- a/reconciler/testing/table.go +++ b/reconciler/testing/table.go @@ -20,6 +20,7 @@ import ( "context" "path" "reflect" + "slices" "strings" "testing" @@ -168,7 +169,8 @@ func (r *TableRow) Test(t *testing.T, factory Factory) { t.Errorf("Error capturing actions by verb: %q", err) } - effectiveOpts := append(r.CmpOpts, defaultCmpOpts...) + effectiveOpts := slices.Concat(r.CmpOpts, defaultCmpOpts) + // Previous state is used to diff resource expected state for update requests that were missed. objPrevState := make(map[string]runtime.Object, len(r.Objects)) for _, o := range r.Objects { diff --git a/webhook/resourcesemantics/conversion/internal/types.go b/webhook/resourcesemantics/conversion/internal/types.go index 7f650c885b..592e2f30b4 100644 --- a/webhook/resourcesemantics/conversion/internal/types.go +++ b/webhook/resourcesemantics/conversion/internal/types.go @@ -88,7 +88,7 @@ type ( // ErrorResource explodes in various settings depending on the property // set. Use the Error* constants // - //This type is used for testing conversion webhooks + // This type is used for testing conversion webhooks // // +k8s:deepcopy-gen=true // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object From da1459a6b70c279a2c6a07ed270f137a9204d01d Mon Sep 17 00:00:00 2001 From: dprotaso Date: Sun, 23 Jun 2024 20:47:43 -0400 Subject: [PATCH 14/22] fix whitespace --- codegen/cmd/injection-gen/generators/packages.go | 1 - 1 file changed, 1 deletion(-) diff --git a/codegen/cmd/injection-gen/generators/packages.go b/codegen/cmd/injection-gen/generators/packages.go index f963094401..7720e63e17 100644 --- a/codegen/cmd/injection-gen/generators/packages.go +++ b/codegen/cmd/injection-gen/generators/packages.go @@ -501,7 +501,6 @@ func versionInformerPackages(basePackage string, groupPkgName string, gv clientg return tags.NeedsInformerInjection() }, }) - } return vers } From 8f7c105cd9be2cc3c3baf45139c2663bb859affd Mon Sep 17 00:00:00 2001 From: dprotaso Date: Sun, 23 Jun 2024 20:52:22 -0400 Subject: [PATCH 15/22] fix fatcontext linter --- injection/informers.go | 1 + reconciler/configstore.go | 2 +- reconciler/testing/context.go | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/injection/informers.go b/injection/informers.go index e29b2da57b..2c7a283c28 100644 --- a/injection/informers.go +++ b/injection/informers.go @@ -14,6 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ +//nolint:fatcontext package injection import ( diff --git a/reconciler/configstore.go b/reconciler/configstore.go index 6b61856d56..3cbfbbffba 100644 --- a/reconciler/configstore.go +++ b/reconciler/configstore.go @@ -33,7 +33,7 @@ var _ ConfigStore = ConfigStores{} func (stores ConfigStores) ToContext(ctx context.Context) context.Context { for _, s := range stores { - ctx = s.ToContext(ctx) + ctx = s.ToContext(ctx) //nolint:fatcontext } return ctx } diff --git a/reconciler/testing/context.go b/reconciler/testing/context.go index 58b3b8d29a..eb4d45b73c 100644 --- a/reconciler/testing/context.go +++ b/reconciler/testing/context.go @@ -51,7 +51,7 @@ func SetupFakeContextWithCancel(t testing.TB, fs ...func(context.Context) contex ctx, c := context.WithCancel(logtesting.TestContextWithLogger(t)) ctx = controller.WithEventRecorder(ctx, record.NewFakeRecorder(1000)) for _, f := range fs { - ctx = f(ctx) + ctx = f(ctx) //nolint:fatcontext } ctx = injection.WithConfig(ctx, &rest.Config{}) From c26b28876359855dc2bb8802363acdad677c7f85 Mon Sep 17 00:00:00 2001 From: dprotaso Date: Sun, 23 Jun 2024 20:58:39 -0400 Subject: [PATCH 16/22] fix errorlint --- network/transports_test.go | 2 +- webhook/json/decode.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/network/transports_test.go b/network/transports_test.go index 112219dd34..d426de8f71 100644 --- a/network/transports_test.go +++ b/network/transports_test.go @@ -236,7 +236,7 @@ func findUnusedPortOrFail(t testingT) int { var errTest = errors.New("testing") func newTestErr(msg string, err error) error { - return fmt.Errorf("%w: %s: %v", errTest, msg, err) + return fmt.Errorf("%w: %s: %w", errTest, msg, err) } // listenOne creates a socket with backlog of one, and use that socket, so diff --git a/webhook/json/decode.go b/webhook/json/decode.go index 54fb352b30..57720e81d2 100644 --- a/webhook/json/decode.go +++ b/webhook/json/decode.go @@ -19,6 +19,7 @@ package json import ( "bytes" "encoding/json" + "errors" "io" ) @@ -95,7 +96,7 @@ func findMetadataOffsets(bites []byte) (start, end int64, err error) { for { t, err = dec.Token() - if err == io.EOF { + if errors.Is(err, io.EOF) { break } if err != nil { From f0b85f9caa9917855cf7dae1c9c68fcab940c17a Mon Sep 17 00:00:00 2001 From: dprotaso Date: Sun, 23 Jun 2024 21:00:37 -0400 Subject: [PATCH 17/22] fix errname linter --- reconciler/events.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/reconciler/events.go b/reconciler/events.go index df4c366cca..9d249bc9db 100644 --- a/reconciler/events.go +++ b/reconciler/events.go @@ -62,7 +62,7 @@ func NewEvent(eventtype, reason, messageFmt string, args ...interface{}) Event { // ReconcilerEvent wraps the fields required for recorders to create a // kubernetes recorder Event. -type ReconcilerEvent struct { //nolint:revive // for backcompat. +type ReconcilerEvent struct { //nolint:errname EventType string Reason string Format string From 108e930ba44bf7ae33b2b3b96fec3a3f189abec1 Mon Sep 17 00:00:00 2001 From: dprotaso Date: Sun, 23 Jun 2024 21:07:04 -0400 Subject: [PATCH 18/22] fix copyloopvar (go1.22) linter --- codegen/cmd/injection-gen/generators/packages.go | 5 ----- controller/controller.go | 2 -- leaderelection/chaosduck/main.go | 1 - leaderelection/context.go | 3 --- webhook/psbinding/reconciler.go | 1 - 5 files changed, 12 deletions(-) diff --git a/codegen/cmd/injection-gen/generators/packages.go b/codegen/cmd/injection-gen/generators/packages.go index 7720e63e17..02f61519f0 100644 --- a/codegen/cmd/injection-gen/generators/packages.go +++ b/codegen/cmd/injection-gen/generators/packages.go @@ -389,8 +389,6 @@ func versionInformerPackages(basePackage string, groupPkgName string, gv clientg vers := make([]generator.Package, 0, 2*len(typesToGenerate)) for _, t := range typesToGenerate { - // Fix for golang iterator bug. - t := t packagePath := packagePath + "/" + strings.ToLower(t.Name.Name) typedInformerPackage := typedInformerPackage(groupPkgName, gv, customArgs.ExternalVersionsInformersPackage) @@ -512,8 +510,6 @@ func reconcilerPackages(basePackage string, groupPkgName string, gv clientgentyp vers := make([]generator.Package, 0, 4*len(typesToGenerate)) for _, t := range typesToGenerate { - // Fix for golang iterator bug. - t := t extracted := extractCommentTags(t) reconcilerClasses, hasReconcilerClass := extractReconcilerClassesTag(extracted) nonNamespaced := isNonNamespaced(extracted) @@ -676,7 +672,6 @@ func versionDuckPackages(basePackage string, groupPkgName string, gv clientgenty for _, t := range typesToGenerate { // Fix for golang iterator bug. - t := t packagePath := filepath.Join(packagePath, strings.ToLower(t.Name.Name)) // Impl diff --git a/controller/controller.go b/controller/controller.go index 153142f3bb..152d837d6d 100644 --- a/controller/controller.go +++ b/controller/controller.go @@ -723,7 +723,6 @@ type Informer interface { // of them to synchronize. func StartInformers(stopCh <-chan struct{}, informers ...Informer) error { for _, informer := range informers { - informer := informer go informer.Run(stopCh) } @@ -741,7 +740,6 @@ func RunInformers(stopCh <-chan struct{}, informers ...Informer) (func(), error) var wg sync.WaitGroup wg.Add(len(informers)) for _, informer := range informers { - informer := informer go func() { defer wg.Done() informer.Run(stopCh) diff --git a/leaderelection/chaosduck/main.go b/leaderelection/chaosduck/main.go index 3125d8e3b9..1e9420f53b 100644 --- a/leaderelection/chaosduck/main.go +++ b/leaderelection/chaosduck/main.go @@ -153,7 +153,6 @@ func main() { continue } - name, leaders := name, leaders eg.Go(func() error { return quack(ctx, kc, name, leaders) }) diff --git a/leaderelection/context.go b/leaderelection/context.go index 1d336cfdab..15f9376ed9 100644 --- a/leaderelection/context.go +++ b/leaderelection/context.go @@ -133,9 +133,6 @@ func (b *standardBuilder) buildElector(ctx context.Context, la reconciler.Leader bkts := newStandardBuckets(queueName, b.lec) electors := make([]Elector, 0, b.lec.Buckets) for _, bkt := range bkts { - // Use a local var which won't change across the for loop since it is - // used in a callback asynchronously. - bkt := bkt rl, err := resourcelock.New(knativeResourceLock, system.Namespace(), // use namespace we are running in bkt.Name(), diff --git a/webhook/psbinding/reconciler.go b/webhook/psbinding/reconciler.go index 0dae1d8f15..b7f1ba0c7e 100644 --- a/webhook/psbinding/reconciler.go +++ b/webhook/psbinding/reconciler.go @@ -391,7 +391,6 @@ func (r *BaseReconciler) ReconcileSubject(ctx context.Context, fb Bindable, muta // For each of the referents, apply the mutation. eg := errgroup.Group{} for _, ps := range referents { - ps := ps eg.Go(func() error { // Do the binding to the pod specable. orig := ps.DeepCopy() From 1fc15a7ad86d157a5828bdfa35d10e54e0696440 Mon Sep 17 00:00:00 2001 From: dprotaso Date: Sun, 23 Jun 2024 21:19:54 -0400 Subject: [PATCH 19/22] fix bodyclose linter --- metrics/e2e_test.go | 3 +++ network/transports_test.go | 5 ++++- test/spoof/error_checks_test.go | 15 ++++++++++++--- websocket/connection.go | 2 +- 4 files changed, 20 insertions(+), 5 deletions(-) diff --git a/metrics/e2e_test.go b/metrics/e2e_test.go index 57ded9f1f8..d4dfde11b4 100644 --- a/metrics/e2e_test.go +++ b/metrics/e2e_test.go @@ -153,6 +153,9 @@ func TestMetricsExport(t *testing.T) { // Wait for the webserver to actually start serving metrics return wait.PollUntilContextTimeout(context.Background(), 10*time.Millisecond, 10*time.Second, true, func(ctx context.Context) (bool, error) { resp, err := http.Get(fmt.Sprintf("http://localhost:%d/metrics", prometheusPort)) + if err != nil { + defer resp.Body.Close() + } return err == nil && resp.StatusCode == http.StatusOK, nil }) }, diff --git a/network/transports_test.go b/network/transports_test.go index d426de8f71..d0b4e9b51f 100644 --- a/network/transports_test.go +++ b/network/transports_test.go @@ -68,7 +68,10 @@ func TestHTTPRoundTripper(t *testing.T) { t.Run(e.label, func(t *testing.T) { wants.Delete(e.want) r := &http.Request{ProtoMajor: e.protoMajor} - rt.RoundTrip(r) + resp, err := rt.RoundTrip(r) + if err != nil { + defer resp.Body.Close() + } if !wants.Has(e.want) { t.Error("Wrong transport selected for request.") diff --git a/test/spoof/error_checks_test.go b/test/spoof/error_checks_test.go index 618e6ee9a9..d516d8cae5 100644 --- a/test/spoof/error_checks_test.go +++ b/test/spoof/error_checks_test.go @@ -50,7 +50,10 @@ func TestDNSError(t *testing.T) { }} { t.Run(tt.name, func(t *testing.T) { req, _ := http.NewRequest(http.MethodGet, tt.url, nil) - _, err := client.Do(req) + resp, err := client.Do(req) + if err != nil { + defer resp.Body.Close() + } if dnsError := isDNSError(err); tt.dnsError != dnsError { t.Errorf("Expected dnsError=%v, got %v", tt.dnsError, dnsError) } @@ -80,7 +83,10 @@ func TestConnectionRefused(t *testing.T) { }} { t.Run(tt.name, func(t *testing.T) { req, _ := http.NewRequest(http.MethodGet, tt.url, nil) - _, err := client.Do(req) + resp, err := client.Do(req) + if err != nil { + defer resp.Body.Close() + } if connRefused := isConnectionRefused(err); tt.connRefused != connRefused { t.Errorf("Expected connRefused=%v, got %v", tt.connRefused, connRefused) } @@ -137,7 +143,10 @@ func TestTCPTimeout(t *testing.T) { }} { t.Run(tt.name, func(t *testing.T) { req, _ := http.NewRequest(http.MethodGet, tt.url, nil) - _, err := client.Do(req) + resp, err := client.Do(req) + if err != nil { + defer resp.Body.Close() + } if tcpTimeout := isTCPTimeout(err); tt.tcpTimeout != tcpTimeout { t.Errorf("Expected tcpTimeout=%v, got %v", tt.tcpTimeout, tcpTimeout) } diff --git a/websocket/connection.go b/websocket/connection.go index 6aa1e61098..b70d8b1ed1 100644 --- a/websocket/connection.go +++ b/websocket/connection.go @@ -134,7 +134,7 @@ func NewDurableConnection(target string, messageChan chan []byte, logger *zap.Su // by restarting the serving side of the connection behind a Kubernetes Service. HandshakeTimeout: 3 * time.Second, } - conn, resp, err := dialer.Dial(target, nil) + conn, resp, err := dialer.Dial(target, nil) //nolint:bodyclose if err != nil { if resp != nil { dresp, _ := httputil.DumpResponse(resp, true /*body*/) // This is for logging so don't care if it fails. From 39b32e8deef5c7d80f46c50cdb5579f3aa93b39f Mon Sep 17 00:00:00 2001 From: dprotaso Date: Sun, 23 Jun 2024 21:20:07 -0400 Subject: [PATCH 20/22] update linter config --- .golangci.yaml | 212 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 200 insertions(+), 12 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 6d714b036f..c0a8028a7f 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,27 +1,215 @@ run: timeout: 10m - - skip-dirs: + allow-parallel-runners: true + exclude-dirs: - client - - injection/client - - third_party + - injection/clients + +output: + uniq-by-line: true + sort-results: true + sort-order: + - linter + - file + show-stats: true + + +issues: + max-issues-per-linter: 0 + max-same-issues: 0 + exclude-rules: + - path: test # Excludes /test, *_test.go etc. + linters: + - gosec + - unparam + - noctx + - protogetter + - linters: ["gocritic"] + # Fixes are non-trivial do in a follow up + text: "ifElseChain" + +linters-settings: + # goimports: + # local-prefixes: knative.dev/pkg + gomodguard: + blocked: + modules: + - github.com/ghodss/yaml: + recommendations: + - sigs.k8s.io/yaml + - go.uber.org/atomic: + recommendations: + - sync/atomic + - io/ioutil: + recommendations: + - os + - io + - github.com/hashicorp/go-multierror: + reason: "use errors.Join" + recommendations: + - errors + - go.uber.org/multierr: + reason: "use errors.Join" + recommendations: + - errors + revive: + rules: + # use unparam linter instead - defaults are better + - name: unused-parameter + disabled: true linters: + disable: + - errcheck enable: + # Check for pass []any as any in variadic func(...any). + - asasalint + + # Only use ASCII chars in indentifiers - asciicheck + + # Dangerous unicode characters + - bidichk + + # Checks whether HTTP response body is closed successfully. + - bodyclose + + # Canonicalheader checks whether net/http.Header uses canonical header. + - canonicalheader + + # TODO - do a follow up PR + # # Containedctx is a linter that detects struct contained context.Context + # # field. + # - containedctx + + # TODO - do a follow up PR + # # Check whether the function uses a non-inherited context. + # - contextcheck + + # Copyloopvar is a linter detects places where loop variables are copied. + - copyloopvar + + # Check declaration order of types, consts, vars and funcs. + - decorder + + # Check for two durations multiplied together. + - durationcheck + + # Checks that sentinel errors are prefixed with the Err- and error types + # are suffixed with the -Error. + - errname + + # Errorlint is a linter for that can be used to find code that will cause + # problems with the error wrapping scheme introduced in Go 1.13. - errorlint + + # Checks for pointers to enclosing loop variables. + - exportloopref + + # Detects nested contexts in loops. + - fatcontext + + # Checks that go compiler directive comments (//go:) are valid. + - gocheckcompilerdirectives + + # Provides diagnostics that check for bugs, performance and style issues. + # Extensible without recompilation through dynamic rules. + # Dynamic rules are written declaratively with AST patterns, filters, + # report message and optional suggestion. + - gocritic + + # Gofmt checks whether code was gofmt-ed. By default this tool runs + # with -s option to check for code simplification. + - gofmt + + # Gofumpt checks whether code was gofumpt-ed. + - gofumpt + + # Check import statements are formatted according to the 'goimport' + # command. Reformat imports in autofix mode. + - goimports + + # See config below + - gomodguard + + # Inspects source code for security problems. - gosec + + # Linter that specializes in simplifying code. + - gosimple + - govet + + # Intrange is a linter to find places where for loops could make use of + # an integer range. + - intrange + + # Checks key value pairs for common logger libraries (kitlog,klog,logr,zap). + - loggercheck + + # Finds slice declarations with non-zero initial length. + - makezero + + # Reports wrong mirror patterns of bytes/strings usage + - mirror + + # Finds commonly misspelled English words. + - misspell + + # Finds the code that returns nil even if it checks that the error is not nil. + - nilerr + + # Finds sending HTTP request without context.Context. + - noctx + + # Reports ill-formed or insufficient nolint directives. + - nolintlint + + # Checks for misuse of Sprintf to construct a host with port in a URL. + - nosprintfhostport + + # Checks that fmt.Sprintf can be replaced with a faster alternative. + - perfsprint + + # Finds slice declarations that could potentially be pre-allocated. - prealloc + + # Reports direct reads from proto message fields when getters should be used. + - protogetter + + # Checks that package variables are not reassigned. + - reassign + + # Fast, configurable, extensible, flexible, and beautiful linter for + # Go. Drop-in replacement of golint. - revive + + # Checks for mistakes with OpenTelemetry/Census spans. + - spancheck + + # Stylecheck is a replacement for golint. - stylecheck + + # Tenv is analyzer that detects using os.Setenv instead of t.Setenv + # since Go1.17. + - tenv + + # Linter checks if examples are testable (have an expected output). + - testableexamples + + # Remove unnecessary type conversions. - unconvert + + # Reports unused function parameters and results in your code. - unparam - disable: - - errcheck -issues: - exclude-rules: - - path: test # Excludes /test, *_test.go etc. - linters: - - gosec - - unparam + # A linter that detect the possibility to use variables/constants from the + # Go standard library. + - usestdlibvars + + # Finds wasted assignment statements. + - wastedassign + + # Whitespace is a linter that checks for unnecessary newlines at the start + # and end of functions, if, for, etc. + - whitespace + From 9b2f5731fba1958e6e813a5bf0305b3abb0c1830 Mon Sep 17 00:00:00 2001 From: dprotaso Date: Sun, 23 Jun 2024 21:22:38 -0400 Subject: [PATCH 21/22] add script to format code --- hack/format-code.sh | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100755 hack/format-code.sh diff --git a/hack/format-code.sh b/hack/format-code.sh new file mode 100755 index 0000000000..b9ccea8fc9 --- /dev/null +++ b/hack/format-code.sh @@ -0,0 +1,23 @@ +#!/usr/bin/env bash + +# Copyright 2024 The Knative Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +set -o errexit +set -o nounset +set -o pipefail + + +go run mvdan.cc/gofumpt@latest -l -w . + From bf64ccd2d5e25aaa9f0f8d6f1359595c1506827a Mon Sep 17 00:00:00 2001 From: dprotaso Date: Mon, 24 Jun 2024 13:21:23 -0400 Subject: [PATCH 22/22] fix unit test --- test/spoof/error_checks_test.go | 6 +++--- test/spoof/spoof_test.go | 12 ++++++++---- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/test/spoof/error_checks_test.go b/test/spoof/error_checks_test.go index d516d8cae5..55bdc5568e 100644 --- a/test/spoof/error_checks_test.go +++ b/test/spoof/error_checks_test.go @@ -51,7 +51,7 @@ func TestDNSError(t *testing.T) { t.Run(tt.name, func(t *testing.T) { req, _ := http.NewRequest(http.MethodGet, tt.url, nil) resp, err := client.Do(req) - if err != nil { + if resp != nil { defer resp.Body.Close() } if dnsError := isDNSError(err); tt.dnsError != dnsError { @@ -84,7 +84,7 @@ func TestConnectionRefused(t *testing.T) { t.Run(tt.name, func(t *testing.T) { req, _ := http.NewRequest(http.MethodGet, tt.url, nil) resp, err := client.Do(req) - if err != nil { + if resp != nil { defer resp.Body.Close() } if connRefused := isConnectionRefused(err); tt.connRefused != connRefused { @@ -144,7 +144,7 @@ func TestTCPTimeout(t *testing.T) { t.Run(tt.name, func(t *testing.T) { req, _ := http.NewRequest(http.MethodGet, tt.url, nil) resp, err := client.Do(req) - if err != nil { + if resp != nil { defer resp.Body.Close() } if tcpTimeout := isTCPTimeout(err); tt.tcpTimeout != tcpTimeout { diff --git a/test/spoof/spoof_test.go b/test/spoof/spoof_test.go index 5a43a78149..355d1c6b9f 100644 --- a/test/spoof/spoof_test.go +++ b/test/spoof/spoof_test.go @@ -47,12 +47,16 @@ type fakeTransport struct { func (ft *fakeTransport) RoundTrip(req *http.Request) (*http.Response, error) { call := ft.calls.Add(1) - if ft.response != nil && call == 2 { - // If both a response and an error is defined, we return just the response on - // the second call to simulate a retry that passes eventually. + if ft.response != nil && ft.err != nil { + if call == 2 { + return ft.response, nil + } + return nil, ft.err + } else if ft.response != nil { return ft.response, nil } - return ft.response, ft.err + + return nil, ft.err } func TestSpoofingClient_CheckEndpointState(t *testing.T) {