Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update golangci version and configuration, and fix errors #7832

Merged
merged 1 commit into from
Apr 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 20 additions & 6 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion cmd/entrypoint/subcommands/subcommands.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions cmd/entrypoint/subcommands/subcommands_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
2 changes: 1 addition & 1 deletion hack/spec-gen/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions internal/sidecarlogresults/sidecarlogresults.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
}
}
Expand Down
20 changes: 10 additions & 10 deletions internal/sidecarlogresults/sidecarlogresults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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)
Expand Down Expand Up @@ -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))
}
Expand Down Expand Up @@ -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))
}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/apis/config/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ limitations under the License.
package config

import (
"fmt"
"errors"
"os"
"sort"
"strings"
Expand Down Expand Up @@ -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{}{}
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/apis/config/feature_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -164,8 +166,6 @@ var (

// FeatureFlags holds the features configurations
// +k8s:deepcopy-gen=true
//
//nolint:musttag
type FeatureFlags struct {
DisableAffinityAssistant bool
DisableCredsInit bool
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions pkg/apis/config/resolver/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package resolver

import (
"context"
"fmt"

"knative.dev/pkg/configmap"
)
Expand All @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/pipeline/pod/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
20 changes: 10 additions & 10 deletions pkg/apis/pipeline/v1/container_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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/
Expand All @@ -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.
Expand Down Expand Up @@ -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/
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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/
Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/pipeline/v1/container_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading