Update golangci version and configuration, and fix errors

This is a relatively huge commit that updates the golangci-lint
version as well as configuration.

```
make golangci-lint
🐱 running golangci-lint…
WARN [linters_context] copyloopvar: this linter is disabled because the Go version (1.21) of your project is lower than Go 1.22

Compilation finished at Thu Apr  4 13:44:19
```

Signed-off-by: Vincent Demeester <[email protected]>

.golangci.yml

@@ -16,6 +16,20 @@ linters-settings:
         - github.com/ghodss/yaml:
             recommendations:
               - sigs.k8s.io/yaml
+  depguard:
+    rules:
+      prevent_unmaintained_packages:
+        list-mode: lax # allow unless explicitely denied
+        files:
+          - $all
+          - "!$test"
+        allow:
+          - $gostd
+        deny:
+          - pkg: io/ioutil
+            desc: "replaced by io and os packages since Go 1.16: https://tip.golang.org/doc/go1.16#ioutil"
+          - pkg: github.com/ghodss/yaml
+            desc: "use sigs.k8s.io/yaml instead, to be consistent"
 linters:
   enable:
   - bodyclose
@@ -174,16 +188,16 @@ issues:
   # Enable off-by-default rules for revive requiring that all exported elements have a properly formatted comment.
   - EXC0012 # https://golangci-lint.run/usage/false-positives/#exc0012
   - EXC0014 # https://golangci-lint.run/usage/false-positives/#exc0014
-run:
-  issues-exit-code: 1
-  build-tags:
-  - e2e
-  skip-files:
+  exclude-files:
   - .*/zz_generated.deepcopy.go
   - pkg/apis/pipeline/v1beta1/openapi_generated.go
-  skip-dirs:
+  exclude-dirs:
   - vendor
   - pkg/client
   - pkg/spire/test
+run:
+  issues-exit-code: 1
+  build-tags:
+  - e2e
   timeout: 20m
   modules-download-mode: vendor

Makefile

@@ -9,7 +9,7 @@ TESTPKGS = $(shell env GO111MODULE=on $(GO) list -f \
 BIN      = $(CURDIR)/.bin
 WOKE 	?= go run -modfile go.mod github.com/get-woke/woke
 
-GOLANGCI_VERSION = v1.52.2
+GOLANGCI_VERSION = v1.57.2
 WOKE_VERSION     = v0.19.0
 
 GO           = go
@@ -170,7 +170,7 @@ $(BIN)/golangci-lint: ; $(info $(M) getting golangci-lint $(GOLANGCI_VERSION))
 
 .PHONY: golangci-lint
 golangci-lint: | $(GOLANGCILINT) ; $(info $(M) running golangci-lint…) @ ## Run golangci-lint
-	$Q $(GOLANGCILINT) run --modules-download-mode=vendor --max-issues-per-linter=0 --max-same-issues=0 --deadline 5m
+	$Q $(GOLANGCILINT) run --modules-download-mode=vendor --max-issues-per-linter=0 --max-same-issues=0 --timeout 5m
 
 .PHONY: golangci-lint-check
 golangci-lint-check: | $(GOLANGCILINT) ; $(info $(M) Testing if golint has been done…) @ ## Run golangci-lint for build tests CI job

cmd/entrypoint/subcommands/subcommands.go

@@ -95,7 +95,7 @@ func Process(args []string) error {
 			if err := decodeScript(src); err != nil {
 				return SubcommandError{subcommand: DecodeScriptCommand, message: err.Error()}
 			}
-			return OK{message: fmt.Sprintf("Decoded script %s", src)}
+			return OK{message: "Decoded script " + src}
 		}
 	case StepInitCommand:
 		if err := stepInit(args[1:]); err != nil {

cmd/entrypoint/subcommands/subcommands_test.go

@@ -32,12 +32,12 @@ func TestProcessSuccessfulSubcommands(t *testing.T) {
 	src := filepath.Join(tmp, "foo.txt")
 	dst := filepath.Join(tmp, "bar.txt")
 
-	srcFile, err := os.OpenFile(src, os.O_WRONLY|os.O_CREATE, 0666)
+	srcFile, err := os.OpenFile(src, os.O_WRONLY|os.O_CREATE, 0o666)
 	if err != nil {
 		t.Fatalf("error opening temp file for writing: %v", err)
 	}
 	defer srcFile.Close()
-	if _, err := srcFile.Write([]byte(helloWorldBase64)); err != nil {
+	if _, err := srcFile.WriteString(helloWorldBase64); err != nil {
 		t.Fatalf("error writing source file: %v", err)
 	}
 

hack/spec-gen/main.go

@@ -53,7 +53,7 @@ func main() {
 			return spec.MustCreateRef("#/definitions/" + common.EscapeJsonPointer(swaggify(name)))
 		})
 	default:
-		panic(fmt.Sprintf("Unsupported API version: %s", *apiVersion))
+		panic("Unsupported API version: " + *apiVersion)
 	}
 	defs := spec.Definitions{}
 	for defName, val := range oAPIDefs {

internal/sidecarlogresults/sidecarlogresults.go

@@ -47,9 +47,9 @@ const (
 
 // SidecarLogResult holds fields for storing extracted results
 type SidecarLogResult struct {
-	Name  string
-	Value string
-	Type  SidecarLogResultType
+	Name  string               `json:"name"`
+	Value string               `json:"value"`
+	Type  SidecarLogResultType `json:"type"`
 }
 
 func fileExists(filename string) (bool, error) {
@@ -91,7 +91,7 @@ func waitForStepsToFinish(runDir string) error {
 			// in either case, existence of out.err marks that the step errored and the following steps will
 			// not run. We want the function to break out with nil error in that case so that
 			// the existing results can be logged.
-			if exists, err = fileExists(fmt.Sprintf("%s.err", stepFile)); exists || err != nil {
+			if exists, err = fileExists(stepFile + ".err"); exists || err != nil {
 				return err
 			}
 		}

internal/sidecarlogresults/sidecarlogresults_test.go

@@ -40,10 +40,10 @@ import (
 func TestLookForResults_FanOutAndWait(t *testing.T) {
 	for _, c := range []struct {
 		desc    string
-		results []SidecarLogResult
+		Results []SidecarLogResult `json:"result"`
 	}{{
 		desc: "multiple results",
-		results: []SidecarLogResult{{
+		Results: []SidecarLogResult{{
 			Name:  "foo",
 			Value: "bar",
 			Type:  "task",
@@ -57,7 +57,7 @@ func TestLookForResults_FanOutAndWait(t *testing.T) {
 			dir := t.TempDir()
 			resultNames := []string{}
 			wantResults := []byte{}
-			for _, result := range c.results {
+			for _, result := range c.Results {
 				createResult(t, dir, result.Name, result.Value)
 				resultNames = append(resultNames, result.Name)
 				encodedResult, err := json.Marshal(result)
@@ -363,7 +363,7 @@ func TestParseResults_InvalidType(t *testing.T) {
 	}
 	for _, plog := range podLogs {
 		_, err := parseResults([]byte(plog), 4096)
-		wantErr := fmt.Errorf("invalid sidecar result type not task or step. Must be task or step")
+		wantErr := errors.New("invalid sidecar result type not task or step. Must be task or step")
 		if d := cmp.Diff(wantErr.Error(), err.Error()); d != "" {
 			t.Fatal(diff.PrintWantGot(d))
 		}
@@ -468,7 +468,7 @@ func TestExtractStepAndResultFromSidecarResultName(t *testing.T) {
 func TestExtractStepAndResultFromSidecarResultName_Error(t *testing.T) {
 	sidecarResultName := "step-foo-resultName"
 	_, _, err := ExtractStepAndResultFromSidecarResultName(sidecarResultName)
-	wantErr := fmt.Errorf("invalid string step-foo-resultName : expected somtthing that looks like <stepName>.<resultName>")
+	wantErr := errors.New("invalid string step-foo-resultName : expected somtthing that looks like <stepName>.<resultName>")
 	if d := cmp.Diff(wantErr.Error(), err.Error()); d != "" {
 		t.Fatal(diff.PrintWantGot(d))
 	}
@@ -477,9 +477,9 @@ func TestExtractStepAndResultFromSidecarResultName_Error(t *testing.T) {
 func createStepResult(t *testing.T, dir, stepName, resultName, resultValue string) {
 	t.Helper()
 	resultDir := filepath.Join(dir, stepName, "results")
-	_ = os.MkdirAll(resultDir, 0755)
+	_ = os.MkdirAll(resultDir, 0o755)
 	resultFile := filepath.Join(resultDir, resultName)
-	err := os.WriteFile(resultFile, []byte(resultValue), 0644)
+	err := os.WriteFile(resultFile, []byte(resultValue), 0o644)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -488,7 +488,7 @@ func createStepResult(t *testing.T, dir, stepName, resultName, resultValue strin
 func createResult(t *testing.T, dir string, resultName string, resultValue string) {
 	t.Helper()
 	resultFile := filepath.Join(dir, resultName)
-	err := os.WriteFile(resultFile, []byte(resultValue), 0644)
+	err := os.WriteFile(resultFile, []byte(resultValue), 0o644)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -497,12 +497,12 @@ func createResult(t *testing.T, dir string, resultName string, resultValue strin
 func createRun(t *testing.T, dir string, causeErr bool) {
 	t.Helper()
 	stepFile := filepath.Join(dir, "1")
-	_ = os.Mkdir(stepFile, 0755)
+	_ = os.Mkdir(stepFile, 0o755)
 	stepFile = filepath.Join(stepFile, "out")
 	if causeErr {
 		stepFile += ".err"
 	}
-	err := os.WriteFile(stepFile, []byte(""), 0644)
+	err := os.WriteFile(stepFile, []byte(""), 0o644)
 	if err != nil {
 		t.Fatal(err)
 	}

pkg/apis/config/events.go

@@ -17,7 +17,7 @@ limitations under the License.
 package config
 
 import (
-	"fmt"
+	"errors"
 	"os"
 	"sort"
 	"strings"
@@ -106,17 +106,17 @@ func (efs EventFormats) Equals(other EventFormats) bool {
 func ParseEventFormats(formats string) (EventFormats, error) {
 	// An empty string is not a valid configuration
 	if formats == "" {
-		return EventFormats{}, fmt.Errorf("formats cannot be empty")
+		return EventFormats{}, errors.New("formats cannot be empty")
 	}
 	stringFormats := strings.Split(formats, ",")
 	var eventFormats EventFormats = make(map[EventFormat]struct{}, len(stringFormats))
 	for _, format := range stringFormats {
 		if !EventFormat(format).IsValid() {
-			return EventFormats{}, fmt.Errorf("invalid format: %s", format)
+			return EventFormats{}, errors.New("invalid format: " + format)
 		}
 		// If already in the map (duplicate), fail
 		if _, ok := eventFormats[EventFormat(format)]; ok {
-			return EventFormats{}, fmt.Errorf("duplicate format: %s", format)
+			return EventFormats{}, errors.New("duplicate format: " + format)
 		}
 		eventFormats[EventFormat(format)] = struct{}{}
 	}

pkg/apis/config/feature_flags.go

@@ -146,13 +146,15 @@ var (
 	DefaultEnableStepActions = PerFeatureFlag{
 		Name:      EnableStepActions,
 		Stability: AlphaAPIFields,
-		Enabled:   DefaultAlphaFeatureEnabled}
+		Enabled:   DefaultAlphaFeatureEnabled,
+	}
 
 	// DefaultEnableArtifacts is the default PerFeatureFlag value for EnableStepActions
 	DefaultEnableArtifacts = PerFeatureFlag{
 		Name:      EnableStepActions,
 		Stability: AlphaAPIFields,
-		Enabled:   DefaultAlphaFeatureEnabled}
+		Enabled:   DefaultAlphaFeatureEnabled,
+	}
 
 	// DefaultEnableParamEnum is the default PerFeatureFlag value for EnableParamEnum
 	DefaultEnableParamEnum = PerFeatureFlag{
@@ -164,8 +166,6 @@ var (
 
 // FeatureFlags holds the features configurations
 // +k8s:deepcopy-gen=true
-//
-//nolint:musttag
 type FeatureFlags struct {
 	DisableAffinityAssistant         bool
 	DisableCredsInit                 bool
@@ -349,7 +349,7 @@ func setCoschedule(cfgMap map[string]string, defaultValue string, disabledAffini
 // setEnforceNonFalsifiability sets the "enforce-nonfalsifiability" flag based on the content of a given map.
 // If the feature gate is invalid, then an error is returned.
 func setEnforceNonFalsifiability(cfgMap map[string]string, feature *string) error {
-	var value = DefaultEnforceNonfalsifiability
+	value := DefaultEnforceNonfalsifiability
 	if cfg, ok := cfgMap[enforceNonfalsifiability]; ok {
 		value = strings.ToLower(cfg)
 	}
@@ -393,7 +393,7 @@ func setMaxResultSize(cfgMap map[string]string, defaultValue int, feature *int)
 	}
 	// if max limit is > 1.5 MB (CRD limit).
 	if value >= 1572864 {
-		return fmt.Errorf("invalid value for feature flag %q: %q. This is exceeding the CRD limit", resultExtractionMethod, fmt.Sprint(value))
+		return fmt.Errorf("invalid value for feature flag %q: %q. This is exceeding the CRD limit", resultExtractionMethod, strconv.Itoa(value))
 	}
 	*feature = value
 	return nil

pkg/apis/config/resolver/store.go

@@ -18,7 +18,6 @@ package resolver
 
 import (
 	"context"
-	"fmt"
 
 	"knative.dev/pkg/configmap"
 )
@@ -33,7 +32,7 @@ type Config struct {
 
 // ResolversNamespace takes the pipelines namespace and appends "-resolvers" to it.
 func ResolversNamespace(baseNS string) string {
-	return fmt.Sprintf("%s-resolvers", baseNS)
+	return baseNS + "-resolvers"
 }
 
 // FromContext extracts a Config from the provided context.

pkg/apis/pipeline/pod/template.go

@@ -37,7 +37,7 @@ type Template struct {
 	// +patchMergeKey=name
 	// +patchStrategy=merge
 	// +listType=atomic
-	Env []corev1.EnvVar `json:"env,omitempty" patchStrategy:"merge" patchMergeKey:"name" protobuf:"bytes,7,rep,name=env"`
+	Env []corev1.EnvVar `json:"env,omitempty" patchMergeKey:"name" patchStrategy:"merge" protobuf:"bytes,7,rep,name=env"`
 
 	// If specified, the pod's tolerations.
 	// +optional
@@ -59,7 +59,7 @@ type Template struct {
 	// +patchMergeKey=name
 	// +patchStrategy=merge,retainKeys
 	// +listType=atomic
-	Volumes []corev1.Volume `json:"volumes,omitempty" patchStrategy:"merge,retainKeys" patchMergeKey:"name" protobuf:"bytes,1,rep,name=volumes"`
+	Volumes []corev1.Volume `json:"volumes,omitempty" patchMergeKey:"name" patchStrategy:"merge,retainKeys" protobuf:"bytes,1,rep,name=volumes"`
 
 	// RuntimeClassName refers to a RuntimeClass object in the node.k8s.io
 	// group, which should be used to run this pod. If no RuntimeClass resource

pkg/apis/pipeline/v1/container_types.go

@@ -72,7 +72,7 @@ type Step struct {
 	// +patchMergeKey=name
 	// +patchStrategy=merge
 	// +listType=atomic
-	Env []corev1.EnvVar `json:"env,omitempty" patchStrategy:"merge" patchMergeKey:"name" protobuf:"bytes,7,rep,name=env"`
+	Env []corev1.EnvVar `json:"env,omitempty" patchMergeKey:"name" patchStrategy:"merge" protobuf:"bytes,7,rep,name=env"`
 	// ComputeResources required by this Step.
 	// Cannot be updated.
 	// More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
@@ -84,13 +84,13 @@ type Step struct {
 	// +patchMergeKey=mountPath
 	// +patchStrategy=merge
 	// +listType=atomic
-	VolumeMounts []corev1.VolumeMount `json:"volumeMounts,omitempty" patchStrategy:"merge" patchMergeKey:"mountPath" protobuf:"bytes,9,rep,name=volumeMounts"`
+	VolumeMounts []corev1.VolumeMount `json:"volumeMounts,omitempty" patchMergeKey:"mountPath" patchStrategy:"merge" protobuf:"bytes,9,rep,name=volumeMounts"`
 	// volumeDevices is the list of block devices to be used by the Step.
 	// +patchMergeKey=devicePath
 	// +patchStrategy=merge
 	// +optional
 	// +listType=atomic
-	VolumeDevices []corev1.VolumeDevice `json:"volumeDevices,omitempty" patchStrategy:"merge" patchMergeKey:"devicePath" protobuf:"bytes,21,rep,name=volumeDevices"`
+	VolumeDevices []corev1.VolumeDevice `json:"volumeDevices,omitempty" patchMergeKey:"devicePath" patchStrategy:"merge" protobuf:"bytes,21,rep,name=volumeDevices"`
 	// Image pull policy.
 	// One of Always, Never, IfNotPresent.
 	// Defaults to Always if :latest tag is specified, or IfNotPresent otherwise.
@@ -294,7 +294,7 @@ type StepTemplate struct {
 	// +patchMergeKey=name
 	// +patchStrategy=merge
 	// +listType=atomic
-	Env []corev1.EnvVar `json:"env,omitempty" patchStrategy:"merge" patchMergeKey:"name" protobuf:"bytes,7,rep,name=env"`
+	Env []corev1.EnvVar `json:"env,omitempty" patchMergeKey:"name" patchStrategy:"merge" protobuf:"bytes,7,rep,name=env"`
 	// ComputeResources required by this Step.
 	// Cannot be updated.
 	// More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
@@ -306,13 +306,13 @@ type StepTemplate struct {
 	// +patchMergeKey=mountPath
 	// +patchStrategy=merge
 	// +listType=atomic
-	VolumeMounts []corev1.VolumeMount `json:"volumeMounts,omitempty" patchStrategy:"merge" patchMergeKey:"mountPath" protobuf:"bytes,9,rep,name=volumeMounts"`
+	VolumeMounts []corev1.VolumeMount `json:"volumeMounts,omitempty" patchMergeKey:"mountPath" patchStrategy:"merge" protobuf:"bytes,9,rep,name=volumeMounts"`
 	// volumeDevices is the list of block devices to be used by the Step.
 	// +patchMergeKey=devicePath
 	// +patchStrategy=merge
 	// +optional
 	// +listType=atomic
-	VolumeDevices []corev1.VolumeDevice `json:"volumeDevices,omitempty" patchStrategy:"merge" patchMergeKey:"devicePath" protobuf:"bytes,21,rep,name=volumeDevices"`
+	VolumeDevices []corev1.VolumeDevice `json:"volumeDevices,omitempty" patchMergeKey:"devicePath" patchStrategy:"merge" protobuf:"bytes,21,rep,name=volumeDevices"`
 	// Image pull policy.
 	// One of Always, Never, IfNotPresent.
 	// Defaults to Always if :latest tag is specified, or IfNotPresent otherwise.
@@ -410,7 +410,7 @@ type Sidecar struct {
 	// +listType=map
 	// +listMapKey=containerPort
 	// +listMapKey=protocol
-	Ports []corev1.ContainerPort `json:"ports,omitempty" patchStrategy:"merge" patchMergeKey:"containerPort" protobuf:"bytes,6,rep,name=ports"`
+	Ports []corev1.ContainerPort `json:"ports,omitempty" patchMergeKey:"containerPort" patchStrategy:"merge" protobuf:"bytes,6,rep,name=ports"`
 	// List of sources to populate environment variables in the Sidecar.
 	// The keys defined within a source must be a C_IDENTIFIER. All invalid keys
 	// will be reported as an event when the container is starting. When a key exists in multiple
@@ -426,7 +426,7 @@ type Sidecar struct {
 	// +patchMergeKey=name
 	// +patchStrategy=merge
 	// +listType=atomic
-	Env []corev1.EnvVar `json:"env,omitempty" patchStrategy:"merge" patchMergeKey:"name" protobuf:"bytes,7,rep,name=env"`
+	Env []corev1.EnvVar `json:"env,omitempty" patchMergeKey:"name" patchStrategy:"merge" protobuf:"bytes,7,rep,name=env"`
 	// ComputeResources required by this Sidecar.
 	// Cannot be updated.
 	// More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
@@ -438,13 +438,13 @@ type Sidecar struct {
 	// +patchMergeKey=mountPath
 	// +patchStrategy=merge
 	// +listType=atomic
-	VolumeMounts []corev1.VolumeMount `json:"volumeMounts,omitempty" patchStrategy:"merge" patchMergeKey:"mountPath" protobuf:"bytes,9,rep,name=volumeMounts"`
+	VolumeMounts []corev1.VolumeMount `json:"volumeMounts,omitempty" patchMergeKey:"mountPath" patchStrategy:"merge" protobuf:"bytes,9,rep,name=volumeMounts"`
 	// volumeDevices is the list of block devices to be used by the Sidecar.
 	// +patchMergeKey=devicePath
 	// +patchStrategy=merge
 	// +optional
 	// +listType=atomic
-	VolumeDevices []corev1.VolumeDevice `json:"volumeDevices,omitempty" patchStrategy:"merge" patchMergeKey:"devicePath" protobuf:"bytes,21,rep,name=volumeDevices"`
+	VolumeDevices []corev1.VolumeDevice `json:"volumeDevices,omitempty" patchMergeKey:"devicePath" patchStrategy:"merge" protobuf:"bytes,21,rep,name=volumeDevices"`
 	// Periodic probe of Sidecar liveness.
 	// Container will be restarted if the probe fails.
 	// Cannot be updated.

pkg/apis/pipeline/v1/container_validation.go

@@ -29,7 +29,7 @@ import (
 // correctly. No errors are returned for a nil Ref.
 func (ref *Ref) Validate(ctx context.Context) (errs *apis.FieldError) {
 	if ref == nil {
-		return
+		return errs
 	}
 
 	switch {