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-lint version to v1.49.0 #4806

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 5 additions & 3 deletions .golangci.yml
Expand Up @@ -14,7 +14,6 @@ linters:
- bodyclose
- contextcheck
# - cyclop
- deadcode
- depguard
- dogsled
- dupl
Expand Down Expand Up @@ -71,7 +70,6 @@ linters:
- rowserrcheck
- sqlclosecheck
- staticcheck
- structcheck
# - stylecheck
- tagliatelle
- tenv
Expand All @@ -82,12 +80,16 @@ linters:
- unconvert
- unparam
- unused
- varcheck
# - varnamelen
- wastedassign
- whitespace
- wrapcheck
# - wsl
- asasalint
- usestdlibvars
- interfacebloat
- logrlint
- reassign

linters-settings:
dupl:
Expand Down
4 changes: 3 additions & 1 deletion Makefile-tools.mk
@@ -1,6 +1,8 @@
# Copyright 2022 The Kubernetes Authors.
# SPDX-License-Identifier: Apache-2.0

GOLANGCI_LINT_VERSION=v1.49.0

MYGOBIN = $(shell go env GOBIN)
ifeq ($(MYGOBIN),)
MYGOBIN = $(shell go env GOPATH)/bin
Expand Down Expand Up @@ -28,7 +30,7 @@ uninstall-out-of-tree-tools:
rm -f $(MYGOBIN)/stringer

$(MYGOBIN)/golangci-lint:
go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.46.2
go install github.com/golangci/golangci-lint/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION)

$(MYGOBIN)/mdrip:
go install github.com/monopole/mdrip@v1.0.2
Expand Down
2 changes: 1 addition & 1 deletion api/krusty/namespaces_test.go
Expand Up @@ -692,7 +692,7 @@ resources:
th.AssertActualEqualsExpected(m, namespaceNeedInVarExpectedOutput)
}

// nolint:gosec
//nolint:gosec
KnVerey marked this conversation as resolved.
Show resolved Hide resolved
const namespaceNeedInVarMyAppWithNamespace string = `
resources:
- elasticsearch-dev-service.yaml
Expand Down
2 changes: 1 addition & 1 deletion api/resmap/reswrangler_test.go
Expand Up @@ -301,7 +301,7 @@ func TestGetMatchingResourcesByCurrentId(t *testing.T) {
t.Fatalf("Expected single map entry but got %v", result)
}

// nolint:goconst
//nolint:goconst
tests := []struct {
name string
matcher IdMatcher
Expand Down
2 changes: 1 addition & 1 deletion api/types/helmchartargs.go
Expand Up @@ -71,7 +71,7 @@ type HelmChart struct {

// IncludeCRDs specifies if Helm should also generate CustomResourceDefinitions.
// Defaults to 'false'.
IncludeCRDs bool `json:"includeCRDs,omitempty" yaml:"includeCRDs,omitempty"` // nolint: tagliatelle
IncludeCRDs bool `json:"includeCRDs,omitempty" yaml:"includeCRDs,omitempty"` //nolint: tagliatelle
}

// HelmChartArgs contains arguments to helm.
Expand Down
2 changes: 1 addition & 1 deletion cmd/config/internal/commands/cmdxargs.go
Expand Up @@ -106,7 +106,7 @@ func (r *XArgsRunner) runE(c *cobra.Command, _ []string) error {
return fmt.Errorf("must specify -- before command")
}
r.Args = r.Args[cmdIndex:]
run := exec.Command(r.Args[0]) // nolint: gosec
run := exec.Command(r.Args[0]) //nolint: gosec

if len(r.Args) > 1 {
r.Args = r.Args[cmdIndex+1:]
Expand Down
6 changes: 3 additions & 3 deletions cmd/config/internal/commands/e2e/e2e_test.go
Expand Up @@ -719,7 +719,7 @@ metadata:
}

args := append([]string{"fn", "run", "."}, tt.args(binDir)...)
cmd := exec.Command(filepath.Join(binDir, kyamlBin), args...) // nolint: gosec
cmd := exec.Command(filepath.Join(binDir, kyamlBin), args...) //nolint: gosec
cmd.Dir = dir
var stdErr, stdOut bytes.Buffer
cmd.Stdout = &stdOut
Expand Down Expand Up @@ -761,7 +761,7 @@ func build() string {
panic(err)
}

build := exec.Command("go", "build", "-o", // nolint: gosec
build := exec.Command("go", "build", "-o", //nolint: gosec
filepath.Join(binDir, e2econtainerconfigBin))
build.Dir = "e2econtainerconfig"
build.Stdout = os.Stdout
Expand All @@ -773,7 +773,7 @@ func build() string {
panic(err)
}

build = exec.Command("go", "build", "-o", filepath.Join(binDir, kyamlBin)) // nolint: gosec
build = exec.Command("go", "build", "-o", filepath.Join(binDir, kyamlBin)) //nolint: gosec
build.Dir = filepath.Join("..", "..", "..", "kubectl-krm")
build.Stdout = os.Stdout
build.Stderr = os.Stderr
Expand Down
3 changes: 1 addition & 2 deletions cmd/config/internal/commands/grep.go
@@ -1,6 +1,5 @@
// Copyright 2019 The Kubernetes Authors.
// SPDX-License-Identifier: Apache-2.0
//
package commands

import (
Expand Down Expand Up @@ -100,7 +99,7 @@ func (r *GrepRunner) preRunE(c *cobra.Command, args []string) error {
r.Value = last[1]
}

r.Path = append(parts[:len(parts)-1], last[0]) // nolint:gocritic
r.Path = append(parts[:len(parts)-1], last[0]) //nolint:gocritic
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/pluginator/internal/krmfunction/funcwrapper/statik.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion kyaml/filesys/fsnode_test.go
Expand Up @@ -903,7 +903,7 @@ func TestFileOps(t *testing.T) {
defer f.Close()

for {
buf := make([]byte, rand.Intn(10)) // nolint:gosec
buf := make([]byte, rand.Intn(10)) //nolint:gosec
n, err := f.Read(buf)
if err != nil && err != io.EOF {
t.Fatalf("unexpected error: %v", err)
Expand Down
42 changes: 21 additions & 21 deletions kyaml/filesys/util.go
Expand Up @@ -74,30 +74,30 @@ func PathJoin(incoming []string) string {
//
// E.g. if part == 'PEACH'
//
// OLD : NEW : POS
// --------------------------------------------------------
// {empty} : PEACH : irrelevant
// / : /PEACH : irrelevant
// pie : PEACH/pie : 0 (or negative)
// /pie : /PEACH/pie : 0 (or negative)
// raw : raw/PEACH : 1 (or larger)
// /raw : /raw/PEACH : 1 (or larger)
// a/nice/warm/pie : a/nice/warm/PEACH/pie : 3
// /a/nice/warm/pie : /a/nice/warm/PEACH/pie : 3
// OLD : NEW : POS
// --------------------------------------------------------
// {empty} : PEACH : irrelevant
// / : /PEACH : irrelevant
// pie : PEACH/pie : 0 (or negative)
// /pie : /PEACH/pie : 0 (or negative)
// raw : raw/PEACH : 1 (or larger)
// /raw : /raw/PEACH : 1 (or larger)
// a/nice/warm/pie : a/nice/warm/PEACH/pie : 3
// /a/nice/warm/pie : /a/nice/warm/PEACH/pie : 3
//
// * An empty part results in no change.
//
// * Absolute paths get their leading '/' stripped, treated like
// relative paths, and the leading '/' is re-added on output.
// The meaning of pos is intentionally the same in either absolute or
// relative paths; if it weren't, this function could convert absolute
// paths to relative paths, which is not desirable.
// - Absolute paths get their leading '/' stripped, treated like
// relative paths, and the leading '/' is re-added on output.
// The meaning of pos is intentionally the same in either absolute or
// relative paths; if it weren't, this function could convert absolute
// paths to relative paths, which is not desirable.
//
// * For robustness (liberal input, conservative output) Pos values that
// that are too small (large) to index the split filepath result in a
// prefix (postfix) rather than an error. Use extreme position values
// to assure a prefix or postfix (e.g. 0 will always prefix, and
// 9999 will presumably always postfix).
// - For robustness (liberal input, conservative output) Pos values that
// that are too small (large) to index the split filepath result in a
// prefix (postfix) rather than an error. Use extreme position values
// to assure a prefix or postfix (e.g. 0 will always prefix, and
// 9999 will presumably always postfix).
func InsertPathPart(path string, pos int, part string) string {
if part == "" {
return path
Expand All @@ -121,7 +121,7 @@ func InsertPathPart(path string, pos int, part string) string {
result := make([]string, len(parts)+1)
copy(result, parts[0:pos])
result[pos] = part
return PathJoin(append(result, parts[pos:]...)) // nolint: makezero
return PathJoin(append(result, parts[pos:]...)) //nolint: makezero
}

func IsHiddenFilePath(pattern string) bool {
Expand Down
2 changes: 1 addition & 1 deletion kyaml/fn/framework/function_definition.go
Expand Up @@ -82,7 +82,7 @@ type KRMFunctionVersion struct {

type KRMFunctionValidation struct {
// OpenAPIV3Schema is the OpenAPI v3 schema for an instance of the KRM function.
OpenAPIV3Schema *spec.Schema `yaml:"openAPIV3Schema,omitempty" json:"openAPIV3Schema,omitempty"` // nolint: tagliatelle
OpenAPIV3Schema *spec.Schema `yaml:"openAPIV3Schema,omitempty" json:"openAPIV3Schema,omitempty"` //nolint: tagliatelle
}

type KRMFunctionNames struct {
Expand Down
2 changes: 1 addition & 1 deletion kyaml/fn/runtime/exec/exec.go
Expand Up @@ -34,7 +34,7 @@ func (c *Filter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) {
}

func (c *Filter) Run(reader io.Reader, writer io.Writer) error {
cmd := exec.Command(c.Path, c.Args...) // nolint:gosec
cmd := exec.Command(c.Path, c.Args...) //nolint:gosec
cmd.Stdin = reader
cmd.Stdout = writer
cmd.Stderr = os.Stderr
Expand Down
7 changes: 5 additions & 2 deletions kyaml/fn/runtime/runtimeutil/functiontypes.go
Expand Up @@ -204,7 +204,7 @@ func (s *StorageMount) String() string {
func GetFunctionSpec(n *yaml.RNode) (*FunctionSpec, error) {
meta, err := n.GetMeta()
if err != nil {
return nil, nil
return nil, fmt.Errorf("failed to get ResourceMeta: %w", err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although these may be good changes, they look unrelated to this PR. Is a new linter somehow triggering the need for them?

Copy link
Member Author

@koba1t koba1t Sep 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was caused by the nilerr with the new version.
That is not a new linter. That linter was enabled before creating this PR.

That lint would show the below errors if this line weren't fixed.

error is not nil (line 205) but it returns nil (nilerr)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. This was probably done to make the function alpha as unlikely as possible to generate errors, but now that we're trying to move functions forward, I think you're right to return the error here. It would be very surprising to get an error here, and if somehow that happened, we don't know whether or not the metadata we failed to retrieve contained a function.

}

fn, err := getFunctionSpecFromAnnotation(n, meta)
Expand Down Expand Up @@ -237,7 +237,10 @@ func getFunctionSpecFromAnnotation(n *yaml.RNode, meta yaml.ResourceMeta) (*Func
}
}
n, err := n.Pipe(yaml.Lookup("metadata", "configFn"))
if err != nil || yaml.IsMissingOrNull(n) {
if err != nil {
return nil, fmt.Errorf("failed to LookUp configFn: %w", err)
koba1t marked this conversation as resolved.
Show resolved Hide resolved
}
if yaml.IsMissingOrNull(n) {
return nil, nil
}
s, err := n.String()
Expand Down
4 changes: 3 additions & 1 deletion kyaml/runfn/runfn_test.go
Expand Up @@ -281,7 +281,8 @@ metadata:
out: []string{"gcr.io/example.com/image:v1.0.0"},
},

{name: "no function spec",
{
name: "no function spec",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on this title, I think this case is meant to ensure that we exit successfully when given a resource that simply does not contain a function. Unfortunately it didn't use a valid resource, so it is surfacing the error now. I think it is good to have the error test case too though! Would you mind changing the title here to something like "invalid input object" and restoring the original case but with a well-formed object?

Copy link
Member Author

@koba1t koba1t Sep 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately it didn't use a valid resource, so it is surfacing the error now.

Oh, I didn't find that.
I restored the original test case at 6ce230f.

in: []f{
{
explicitFunction: true,
Expand All @@ -290,6 +291,7 @@ foo: bar
`,
},
},
error: "failed to get FunctionSpec: failed to get ResourceMeta: missing Resource metadata",
},

// Test
Expand Down