Skip to content

Commit

Permalink
Enforce more lint checks, fix findings (ko-build#492)
Browse files Browse the repository at this point in the history
  • Loading branch information
imjasonh authored Nov 5, 2021
1 parent 9821190 commit 0015a81
Show file tree
Hide file tree
Showing 20 changed files with 96 additions and 87 deletions.
33 changes: 22 additions & 11 deletions .golangci.yaml
Original file line number Diff line number Diff line change
@@ -1,19 +1,30 @@
run:
timeout: 5m

linters:
enable:
- asciicheck
- gosec
- prealloc
- stylecheck
- unconvert
- unparam
disable:
- errcheck

issues:
exclude-rules:
- path: test # Excludes /test, *_test.go etc.
linters:
- gosec

linters:
enable:
- asciicheck
- deadcode
- depguard
- errorlint
- gofmt
- gosec
- goimports
- importas
- prealloc
- revive
- misspell
- stylecheck
- tparallel
- unconvert
- unparam
- whitespace

disable:
- errcheck
12 changes: 6 additions & 6 deletions pkg/build/gobuild.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func (g *gobuild) QualifyImport(importpath string) (string, error) {
var err error
importpath, err = g.qualifyLocalImport(importpath)
if err != nil {
return "", fmt.Errorf("qualifying local import %s: %v", importpath, err)
return "", fmt.Errorf("qualifying local import %s: %w", importpath, err)
}
}
if !strings.HasPrefix(importpath, StrictScheme) {
Expand Down Expand Up @@ -187,7 +187,7 @@ func getGoarm(platform v1.Platform) (string, error) {
vs := strings.TrimPrefix(platform.Variant, "v")
variant, err := strconv.Atoi(vs)
if err != nil {
return "", fmt.Errorf("cannot parse arm variant %q: %v", platform.Variant, err)
return "", fmt.Errorf("cannot parse arm variant %q: %w", platform.Variant, err)
}
if variant >= 5 {
// TODO(golang/go#29373): Allow for 8 in later go versions if this is fixed.
Expand Down Expand Up @@ -229,7 +229,7 @@ func build(ctx context.Context, ip string, dir string, platform v1.Platform, con

env, err := buildEnv(platform, os.Environ(), config.Env)
if err != nil {
return "", fmt.Errorf("could not create env for %s: %v", ip, err)
return "", fmt.Errorf("could not create env for %s: %w", ip, err)
}
cmd.Env = env

Expand Down Expand Up @@ -260,7 +260,7 @@ func buildEnv(platform v1.Platform, userEnv, configEnv []string) ([]string, erro
if strings.HasPrefix(platform.Architecture, "arm") && platform.Variant != "" {
goarm, err := getGoarm(platform)
if err != nil {
return nil, fmt.Errorf("goarm failure: %v", err)
return nil, fmt.Errorf("goarm failure: %w", err)
}
if goarm != "" {
env = append(env, "GOARM="+goarm)
Expand Down Expand Up @@ -318,7 +318,7 @@ func tarBinary(name, binary string, creationTime v1.Time, platform *v1.Platform)
Mode: 0555,
ModTime: creationTime.Time,
}); err != nil {
return nil, fmt.Errorf("writing dir %q: %v", dir, err)
return nil, fmt.Errorf("writing dir %q: %w", dir, err)
}
}

Expand Down Expand Up @@ -494,7 +494,7 @@ func (g *gobuild) tarKoData(ref reference, platform *v1.Platform) (*bytes.Buffer
Mode: 0555,
ModTime: creationTime.Time,
}); err != nil {
return nil, fmt.Errorf("writing dir %q: %v", dir, err)
return nil, fmt.Errorf("writing dir %q: %w", dir, err)
}
}

Expand Down
5 changes: 3 additions & 2 deletions pkg/build/gobuild_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package build
import (
"archive/tar"
"context"
"errors"
"fmt"
"io"
"io/ioutil"
Expand Down Expand Up @@ -431,7 +432,7 @@ func validateImage(t *testing.T, img v1.Image, baseLayers int64, creationTime v1
}
defer r.Close()
tr := tar.NewReader(r)
if _, err := tr.Next(); err == io.EOF {
if _, err := tr.Next(); errors.Is(err, io.EOF) {
t.Errorf("Layer contained no files")
}
})
Expand All @@ -449,7 +450,7 @@ func validateImage(t *testing.T, img v1.Image, baseLayers int64, creationTime v1
tr := tar.NewReader(r)
for {
header, err := tr.Next()
if err == io.EOF {
if errors.Is(err, io.EOF) {
break
} else if err != nil {
t.Errorf("Next() = %v", err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/build/gobuilds.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func relativePath(baseDir string, importpath string) (string, error) {
}
relPath, err := filepath.Rel(baseDir, importpath)
if err != nil {
return "", fmt.Errorf("cannot determine relative path of baseDir (%q) and local path (%q): %v", baseDir, importpath, err)
return "", fmt.Errorf("cannot determine relative path of baseDir (%q) and local path (%q): %w", baseDir, importpath, err)
}
if strings.HasPrefix(relPath, "..") {
// TODO Is this assumption correct?
Expand Down
3 changes: 1 addition & 2 deletions pkg/build/strict.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ func (r reference) Path() string {
func (r reference) String() string {
if r.IsStrict() {
return StrictScheme + r.Path()
} else {
return r.Path()
}
return r.Path()
}
8 changes: 4 additions & 4 deletions pkg/commands/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,11 @@ func addApply(topLevel *cobra.Command) {
bo.InsecureRegistry = po.InsecureRegistry
builder, err := makeBuilder(ctx, bo)
if err != nil {
return fmt.Errorf("error creating builder: %v", err)
return fmt.Errorf("error creating builder: %w", err)
}
publisher, err := makePublisher(po)
if err != nil {
return fmt.Errorf("error creating publisher: %v", err)
return fmt.Errorf("error creating publisher: %w", err)
}
defer publisher.Close()

Expand All @@ -121,7 +121,7 @@ func addApply(topLevel *cobra.Command) {
// Wire up kubectl stdin to resolveFilesToWriter.
stdin, err := kubectlCmd.StdinPipe()
if err != nil {
return fmt.Errorf("error piping to 'kubectl apply': %v", err)
return fmt.Errorf("error piping to 'kubectl apply': %w", err)
}

// Make sure builds are cancelled if kubectl apply fails.
Expand All @@ -144,7 +144,7 @@ func addApply(topLevel *cobra.Command) {
g.Go(func() error {
// Run it.
if err := kubectlCmd.Run(); err != nil {
return fmt.Errorf("error executing 'kubectl apply': %v", err)
return fmt.Errorf("error executing 'kubectl apply': %w", err)
}
return nil
})
Expand Down
6 changes: 3 additions & 3 deletions pkg/commands/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,16 @@ func addBuild(topLevel *cobra.Command) {
bo.InsecureRegistry = po.InsecureRegistry
builder, err := makeBuilder(ctx, bo)
if err != nil {
return fmt.Errorf("error creating builder: %v", err)
return fmt.Errorf("error creating builder: %w", err)
}
publisher, err := makePublisher(po)
if err != nil {
return fmt.Errorf("error creating publisher: %v", err)
return fmt.Errorf("error creating publisher: %w", err)
}
defer publisher.Close()
images, err := publishImages(ctx, args, publisher, builder)
if err != nil {
return fmt.Errorf("failed to publish images: %v", err)
return fmt.Errorf("failed to publish images: %w", err)
}
for _, img := range images {
fmt.Println(img)
Expand Down
4 changes: 2 additions & 2 deletions pkg/commands/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func getBaseImage(platform string, bo *options.BuildOptions) build.GetBase {
}
ref, err := name.ParseReference(baseImage, nameOpts...)
if err != nil {
return nil, nil, fmt.Errorf("parsing base image (%q): %v", baseImage, err)
return nil, nil, fmt.Errorf("parsing base image (%q): %w", baseImage, err)
}

// For ko.local, look in the daemon.
Expand Down Expand Up @@ -128,7 +128,7 @@ func getTimeFromEnv(env string) (*v1.Time, error) {

seconds, err := strconv.ParseInt(epoch, 10, 64)
if err != nil {
return nil, fmt.Errorf("the environment variable %s should be the number of seconds since January 1st 1970, 00:00 UTC, got: %v", env, err)
return nil, fmt.Errorf("the environment variable %s should be the number of seconds since January 1st 1970, 00:00 UTC, got: %w", env, err)
}
return &v1.Time{Time: time.Unix(seconds, 0)}, nil
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/commands/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,11 @@ func addCreate(topLevel *cobra.Command) {
bo.InsecureRegistry = po.InsecureRegistry
builder, err := makeBuilder(ctx, bo)
if err != nil {
return fmt.Errorf("error creating builder: %v", err)
return fmt.Errorf("error creating builder: %w", err)
}
publisher, err := makePublisher(po)
if err != nil {
return fmt.Errorf("error creating publisher: %v", err)
return fmt.Errorf("error creating publisher: %w", err)
}
defer publisher.Close()

Expand All @@ -106,7 +106,7 @@ func addCreate(topLevel *cobra.Command) {
// Wire up kubectl stdin to resolveFilesToWriter.
stdin, err := kubectlCmd.StdinPipe()
if err != nil {
return fmt.Errorf("error piping to 'kubectl create': %v", err)
return fmt.Errorf("error piping to 'kubectl create': %w", err)
}

// Make sure builds are cancelled if kubectl create fails.
Expand All @@ -129,7 +129,7 @@ func addCreate(topLevel *cobra.Command) {
g.Go(func() error {
// Run it.
if err := kubectlCmd.Run(); err != nil {
return fmt.Errorf("error executing 'kubectl create': %v", err)
return fmt.Errorf("error executing 'kubectl create': %w", err)
}
return nil
})
Expand Down
3 changes: 2 additions & 1 deletion pkg/commands/deps.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package commands

import (
"archive/tar"
"errors"
"fmt"
"io"
"io/ioutil"
Expand Down Expand Up @@ -80,7 +81,7 @@ If the image was not built using ko, or if it was built without embedding depend
// keep reading.
}
h, err := tr.Next()
if err == io.EOF {
if errors.Is(err, io.EOF) {
return fmt.Errorf("no ko-built executable named %q found", bin)
}
if err != nil {
Expand Down
11 changes: 6 additions & 5 deletions pkg/commands/options/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package options

import (
"errors"
"fmt"
"os"
"path/filepath"
Expand Down Expand Up @@ -91,15 +92,15 @@ func (bo *BuildOptions) LoadConfig() error {
v.AddConfigPath(bo.WorkingDirectory)

if err := v.ReadInConfig(); err != nil {
if _, ok := err.(viper.ConfigFileNotFoundError); !ok {
return fmt.Errorf("error reading config file: %v", err)
if !errors.As(err, &viper.ConfigFileNotFoundError{}) {
return fmt.Errorf("error reading config file: %w", err)
}
}

if bo.BaseImage == "" {
ref := v.GetString("defaultBaseImage")
if _, err := name.ParseReference(ref); err != nil {
return fmt.Errorf("'defaultBaseImage': error parsing %q as image reference: %v", ref, err)
return fmt.Errorf("'defaultBaseImage': error parsing %q as image reference: %w", ref, err)
}
bo.BaseImage = ref
}
Expand All @@ -109,7 +110,7 @@ func (bo *BuildOptions) LoadConfig() error {
overrides := v.GetStringMapString("baseImageOverrides")
for key, value := range overrides {
if _, err := name.ParseReference(value); err != nil {
return fmt.Errorf("'baseImageOverrides': error parsing %q as image reference: %v", value, err)
return fmt.Errorf("'baseImageOverrides': error parsing %q as image reference: %w", value, err)
}
baseImageOverrides[key] = value
}
Expand Down Expand Up @@ -172,7 +173,7 @@ func createBuildConfigMap(workingDirectory string, configs []build.Config) (map[

pkgs, err := packages.Load(&packages.Config{Mode: packages.NeedName, Dir: baseDir}, localImportPath)
if err != nil {
return nil, fmt.Errorf("'builds': entry #%d does not contain a valid local import path (%s) for directory (%s): %v", i, localImportPath, baseDir, err)
return nil, fmt.Errorf("'builds': entry #%d does not contain a valid local import path (%s) for directory (%s): %w", i, localImportPath, baseDir, err)
}

if len(pkgs) != 1 {
Expand Down
6 changes: 3 additions & 3 deletions pkg/commands/publisher.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,16 @@ func publishImages(ctx context.Context, importpaths []string, pub publish.Interf
return nil, err
}
if err := b.IsSupportedReference(importpath); err != nil {
return nil, fmt.Errorf("importpath %q is not supported: %v", importpath, err)
return nil, fmt.Errorf("importpath %q is not supported: %w", importpath, err)
}

img, err := b.Build(ctx, importpath)
if err != nil {
return nil, fmt.Errorf("error building %q: %v", importpath, err)
return nil, fmt.Errorf("error building %q: %w", importpath, err)
}
ref, err := pub.Publish(ctx, img, importpath)
if err != nil {
return nil, fmt.Errorf("error publishing %s: %v", importpath, err)
return nil, fmt.Errorf("error publishing %s: %w", importpath, err)
}
imgs[importpath] = ref
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/commands/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,11 @@ func addResolve(topLevel *cobra.Command) {
bo.InsecureRegistry = po.InsecureRegistry
builder, err := makeBuilder(ctx, bo)
if err != nil {
return fmt.Errorf("error creating builder: %v", err)
return fmt.Errorf("error creating builder: %w", err)
}
publisher, err := makePublisher(po)
if err != nil {
return fmt.Errorf("error creating publisher: %v", err)
return fmt.Errorf("error creating publisher: %w", err)
}
defer publisher.Close()
return resolveFilesToWriter(ctx, builder, publisher, fo, so, os.Stdout)
Expand Down
Loading

0 comments on commit 0015a81

Please sign in to comment.