update linter config and fix issues

.golangci.yaml

@@ -1,28 +1,212 @@
 run:
-  timeout: 5m
-
+  timeout: 10m
+  allow-parallel-runners: true
+  exclude-dirs:
+  - pkg/client
   build-tags:
-    - e2e
+  - e2e
+
+output:
+  sort-results: true
+  sort-order:
+  - linter
+  - file
+  show-stats: true
+
+issues:
+  uniq-by-line: true
+  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"
 
-  skip-dirs:
-    - pkg/client
+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
+
+    # 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
+

pkg/defaults/deployment_defaults_test.go

@@ -198,7 +198,6 @@ func TestValidate(t *testing.T) {
 }
 
 func TestDeepCopyObject(t *testing.T) {
-
 	tests := []struct {
 		name string
 		in   *IstioDeployment

pkg/reconciler/accessor/core/secret_test.go

@@ -172,8 +172,8 @@ func TestNotOwnedFailure(t *testing.T) {
 }
 
 func setup(ctx context.Context, secrets []*corev1.Secret,
-	kubeClient kubernetes.Interface, t *testing.T) (*FakeAccessor, func()) {
-
+	kubeClient kubernetes.Interface, t *testing.T,
+) (*FakeAccessor, func()) {
 	secretInformer := fakesecretinformer.Get(ctx)
 
 	fake := fakekubeclient.Get(ctx)

pkg/reconciler/accessor/istio/destinationrule.go

@@ -47,8 +47,8 @@ func destionationRuleIsDifferent(current, desired *v1beta1.DestinationRule) bool
 
 // ReconcileDestinationRule reconciles DestinationRule to the desired status.
 func ReconcileDestinationRule(ctx context.Context, owner kmeta.Accessor, desired *v1beta1.DestinationRule,
-	drAccessor DestinationRuleAccessor) (*v1beta1.DestinationRule, error) {
-
+	drAccessor DestinationRuleAccessor,
+) (*v1beta1.DestinationRule, error) {
 	recorder := controller.GetEventRecorder(ctx)
 	if recorder == nil {
 		return nil, fmt.Errorf("recorder for reconciling DestinationRule %s/%s is not created", desired.Namespace, desired.Name)

pkg/reconciler/accessor/istio/virtualservice.go

@@ -48,8 +48,8 @@ func hasDesiredDiff(current, desired *v1beta1.VirtualService) bool {
 
 // ReconcileVirtualService reconciles VirtualService to the desired status.
 func ReconcileVirtualService(ctx context.Context, owner kmeta.Accessor, desired *v1beta1.VirtualService,
-	vsAccessor VirtualServiceAccessor) (*v1beta1.VirtualService, error) {
-
+	vsAccessor VirtualServiceAccessor,
+) (*v1beta1.VirtualService, error) {
 	recorder := controller.GetEventRecorder(ctx)
 	if recorder == nil {
 		return nil, fmt.Errorf("recoder for reconciling VirtualService %s/%s is not created", desired.Namespace, desired.Name)

pkg/reconciler/accessor/istio/virtualservice_test.go

@@ -175,12 +175,11 @@ func TestReconcileVirtualService_NotOwnedFailure(t *testing.T) {
 	if !kaccessor.IsNotOwned(err) {
 		t.Error("Expected to get NotOwnedError but got", err)
 	}
-
 }
 
 func setup(ctx context.Context, vses []*v1beta1.VirtualService,
-	istioClient istioclientset.Interface, t *testing.T) (*FakeAccessor, func()) {
-
+	istioClient istioclientset.Interface, t *testing.T,
+) (*FakeAccessor, func()) {
 	fake := istiofake.NewSimpleClientset()
 	informer := istioinformers.NewSharedInformerFactory(fake, 0)
 	vsInformer := informer.Networking().V1beta1().VirtualServices()

pkg/reconciler/accessor/networking/certificate.go

@@ -40,8 +40,8 @@ type CertificateAccessor interface {
 
 // ReconcileCertificate reconciles Certificate to the desired status.
 func ReconcileCertificate(ctx context.Context, owner kmeta.Accessor, desired *v1alpha1.Certificate,
-	certAccessor CertificateAccessor) (*v1alpha1.Certificate, error) {
-
+	certAccessor CertificateAccessor,
+) (*v1alpha1.Certificate, error) {
 	recorder := controller.GetEventRecorder(ctx)
 	if recorder == nil {
 		return nil, fmt.Errorf("recoder for reconciling Certificate %s/%s is not created", desired.Namespace, desired.Name)

pkg/reconciler/accessor/networking/certificate_test.go

@@ -177,8 +177,8 @@ func TestNotOwnedFailure(t *testing.T) {
 }
 
 func setup(ctx context.Context, certs []*v1alpha1.Certificate,
-	client clientset.Interface, t *testing.T) (*FakeAccessor, func()) {
-
+	client clientset.Interface, t *testing.T,
+) (*FakeAccessor, func()) {
 	fake := fakenetworkingclient.Get(ctx)
 	certInformer := fakecertinformer.Get(ctx)
 

pkg/reconciler/ingress/config/istio.go

@@ -17,6 +17,7 @@ limitations under the License.
 package config
 
 import (
+	"errors"
 	"fmt"
 	"sort"
 	"strings"
@@ -90,15 +91,15 @@ func (g Gateway) QualifiedName() string {
 
 func (g Gateway) Validate() error {
 	if g.Namespace == "" {
-		return fmt.Errorf("missing namespace")
+		return errors.New("missing namespace")
 	}
 
 	if g.Name == "" {
-		return fmt.Errorf("missing name")
+		return errors.New("missing name")
 	}
 
 	if g.ServiceURL == "" {
-		return fmt.Errorf("missing service")
+		return errors.New("missing service")
 	}
 
 	if errs := validation.IsDNS1123Subdomain(strings.TrimSuffix(g.ServiceURL, ".")); len(errs) > 0 {

pkg/reconciler/ingress/controller.go

@@ -71,14 +71,13 @@ func AnnotateLoggerWithName(ctx context.Context, name string) context.Context {
 		With(zap.String(logkey.ControllerType, name))
 
 	return logging.WithLogger(ctx, logger)
-
 }
+
 func newControllerWithOptions(
 	ctx context.Context,
 	cmw configmap.Watcher,
 	opts ...ingressOption,
 ) *controller.Impl {
-
 	ctx = AnnotateLoggerWithName(ctx, controllerAgentName)
 	logger := logging.FromContext(ctx)
 	virtualServiceInformer := virtualserviceinformer.Get(ctx)

pkg/reconciler/ingress/ingress.go

@@ -337,7 +337,8 @@ func (r *Reconciler) reconcileSystemGeneratedGateway(ctx context.Context, desire
 }
 
 func (r *Reconciler) reconcileVirtualServices(ctx context.Context, ing *v1alpha1.Ingress,
-	desired []*v1beta1.VirtualService) error {
+	desired []*v1beta1.VirtualService,
+) error {
 	// First, create all needed VirtualServices.
 	kept := sets.New[string]()
 	for _, d := range desired {