Skip to content

Commit

Permalink
More golanci-lint checks (#762)
Browse files Browse the repository at this point in the history
Active a bunch more linters following #761
This is mostly style checks like remove snake case variables and not starting error messages with an uppercase. This leads to a more consistent codebase
  • Loading branch information
julienduchesne authored Sep 21, 2022
1 parent ba3386a commit 8cc1e0a
Show file tree
Hide file tree
Showing 34 changed files with 112 additions and 99 deletions.
22 changes: 22 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
linters:
enable:
- depguard
- dogsled
- errcheck
- exportloopref
- goconst
- gocritic
- gofmt
- goimports
- gosimple
- govet
- ineffassign
- megacheck
- misspell
- revive
- staticcheck
- stylecheck
- typecheck
- unconvert
- unused
- whitespace
4 changes: 1 addition & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ $(GOLINTER):
go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.49.0

lint: $(GOLINTER)
test -z $$(gofmt -s -l cmd/ pkg/ | tee /dev/stderr)
go vet ./...
$(GOLINTER) run

test:
Expand All @@ -20,7 +18,7 @@ test:
dev:
go build -ldflags "-X main.Version=dev-${VERSION}" ./cmd/tk

LDFLAGS := '-s -w -extldflags "-static" -X github.com/grafana/tanka/pkg/tanka.CURRENT_VERSION=${VERSION}'
LDFLAGS := '-s -w -extldflags "-static" -X github.com/grafana/tanka/pkg/tanka.CurrentVersion=${VERSION}'
static:
CGO_ENABLED=0 go build -ldflags=${LDFLAGS} ./cmd/tk

Expand Down
13 changes: 7 additions & 6 deletions cmd/tk/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@ package main
import (
"encoding/json"
"fmt"
"k8s.io/utils/strings/slices"
"os"
"path/filepath"
"sort"
"text/tabwriter"

"k8s.io/utils/strings/slices"

"github.com/go-clix/cli"
"github.com/pkg/errors"
"github.com/posener/complete"
Expand Down Expand Up @@ -63,7 +64,7 @@ func envSetCmd() *cli.Command {

cmd.Run = func(cmd *cli.Command, args []string) error {
if *name != "" {
return fmt.Errorf("It looks like you attempted to rename the environment using `--name`. However, this is not possible with Tanka, because the environments name is inferred from the directories name. To rename the environment, rename its directory instead.")
return fmt.Errorf("it looks like you attempted to rename the environment using `--name`. However, this is not possible with Tanka, because the environments name is inferred from the directories name. To rename the environment, rename its directory instead")
}

path, err := filepath.Abs(args[0])
Expand All @@ -74,7 +75,7 @@ func envSetCmd() *cli.Command {
if cmd.Flags().Changed("server-from-context") {
server, err := client.IPFromContext(tmp.Spec.APIServer)
if err != nil {
return fmt.Errorf("Resolving IP from context: %s", err)
return fmt.Errorf("resolving IP from context: %s", err)
}
tmp.Spec.APIServer = server
}
Expand Down Expand Up @@ -134,7 +135,7 @@ func envAddCmd() *cli.Command {
if cmd.Flags().Changed("server-from-context") {
server, err := client.IPFromContext(cfg.Spec.APIServer)
if err != nil {
return fmt.Errorf("Resolving IP from context: %s", err)
return fmt.Errorf("resolving IP from context: %s", err)
}
cfg.Spec.APIServer = server
}
Expand Down Expand Up @@ -221,7 +222,7 @@ func envRemoveCmd() *cli.Command {
return err
}
if err := os.RemoveAll(path); err != nil {
return fmt.Errorf("Removing '%s': %s", path, err)
return fmt.Errorf("removing '%s': %s", path, err)
}
fmt.Println("Removed", path)
}
Expand Down Expand Up @@ -272,7 +273,7 @@ func envListCmd() *cli.Command {
if *useJSON {
j, err := json.Marshal(envs)
if err != nil {
return fmt.Errorf("Formatting as json: %s", err)
return fmt.Errorf("formatting as json: %s", err)
}
fmt.Println(string(j))
return nil
Expand Down
4 changes: 2 additions & 2 deletions cmd/tk/fmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func fmtCmd() *cli.Command {
Args: cli.Args{
Validator: cli.ValidateFunc(func(args []string) error {
if len(args) == 0 {
return errors.New("At least one file or directory is required")
return errors.New("at least one file or directory is required")
}
return nil
}),
Expand All @@ -51,7 +51,7 @@ func fmtCmd() *cli.Command {
globs[i] = g
}

var outFn tanka.OutFn = nil
var outFn tanka.OutFn
switch {
case *test:
outFn = func(name, content string) error { return nil }
Expand Down
12 changes: 6 additions & 6 deletions cmd/tk/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,22 +32,22 @@ func initCmd() *cli.Command {

files, err := os.ReadDir(".")
if err != nil {
return fmt.Errorf("Error listing files: %s", err)
return fmt.Errorf("error listing files: %s", err)
}
if len(files) > 0 && !*force {
return fmt.Errorf("Error: directory not empty. Use `-f` to force")
return fmt.Errorf("error: directory not empty. Use `-f` to force")
}

if err := writeNewFile("jsonnetfile.json", "{}"); err != nil {
return fmt.Errorf("Error creating `jsonnetfile.json`: %s", err)
return fmt.Errorf("error creating `jsonnetfile.json`: %s", err)
}

if err := os.Mkdir("vendor", os.ModePerm); err != nil {
return fmt.Errorf("Error creating `vendor/` folder: %s", err)
return fmt.Errorf("error creating `vendor/` folder: %s", err)
}

if err := os.Mkdir("lib", os.ModePerm); err != nil {
return fmt.Errorf("Error creating `lib/` folder: %s", err)
return fmt.Errorf("error creating `lib/` folder: %s", err)
}

cfg := v1alpha1.New()
Expand Down Expand Up @@ -79,7 +79,7 @@ func initCmd() *cli.Command {
fmt.Println("Directory structure set up! Remember to configure the API endpoint:\n`tk env set environments/default --server=https://127.0.0.1:6443`")
}
if failed {
log.Println("Errors occured while initializing the project. Check the above logs for details.")
log.Println("Errors occurred while initializing the project. Check the above logs for details.")
}

return nil
Expand Down
2 changes: 1 addition & 1 deletion cmd/tk/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func main() {
rootCmd := &cli.Command{
Use: "tk",
Short: "tanka <3 jsonnet",
Version: tanka.CURRENT_VERSION,
Version: tanka.CurrentVersion,
}

// workflow commands
Expand Down
12 changes: 6 additions & 6 deletions cmd/tk/prefix.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,16 @@ func prefixCommands(prefix string) (cmds []*cli.Command) {
Args: cli.ArgsAny(),
}

ext_command := exec.Command(path)
extCommand := exec.Command(path)
if ex, err := os.Executable(); err == nil {
ext_command.Env = append(os.Environ(), fmt.Sprintf("EXECUTABLE=%s", ex))
extCommand.Env = append(os.Environ(), fmt.Sprintf("EXECUTABLE=%s", ex))
}
ext_command.Stdout = os.Stdout
ext_command.Stderr = os.Stderr
extCommand.Stdout = os.Stdout
extCommand.Stderr = os.Stderr

cmd.Run = func(cmd *cli.Command, args []string) error {
ext_command.Args = append(ext_command.Args, args...)
return ext_command.Run()
extCommand.Args = append(extCommand.Args, args...)
return extCommand.Run()
}
cmds = append(cmds, cmd)
}
Expand Down
12 changes: 6 additions & 6 deletions cmd/tk/tool.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,12 @@ func jpathCmd() *cli.Command {

entrypoint, err := jpath.Entrypoint(path)
if err != nil {
return fmt.Errorf("Resolving JPATH: %s", err)
return fmt.Errorf("resolving JPATH: %s", err)
}

jsonnetpath, base, root, err := jpath.Resolve(entrypoint, false)
if err != nil {
return fmt.Errorf("Resolving JPATH: %s", err)
return fmt.Errorf("resolving JPATH: %s", err)
}

if *debug {
Expand Down Expand Up @@ -89,17 +89,17 @@ func importsCmd() *cli.Command {

path, err := filepath.Abs(args[0])
if err != nil {
return fmt.Errorf("Loading environment: %s", err)
return fmt.Errorf("loading environment: %s", err)
}

deps, err := jsonnet.TransitiveImports(path)
if err != nil {
return fmt.Errorf("Resolving imports: %s", err)
return fmt.Errorf("resolving imports: %s", err)
}

root, err := gitRoot()
if err != nil {
return fmt.Errorf("Invoking git: %s", err)
return fmt.Errorf("invoking git: %s", err)
}
if modFiles != nil {
for _, m := range modFiles {
Expand All @@ -121,7 +121,7 @@ func importsCmd() *cli.Command {

s, err := json.Marshal(deps)
if err != nil {
return fmt.Errorf("Formatting: %s", err)
return fmt.Errorf("formatting: %s", err)
}
fmt.Println(string(s))

Expand Down
2 changes: 1 addition & 1 deletion cmd/tk/toolCharts.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func chartsInitCmd() *cli.Command {

path := filepath.Join(wd, helm.Filename)
if _, err := os.Stat(path); err == nil {
return fmt.Errorf("Chartfile at '%s' already exists. Aborting", path)
return fmt.Errorf("chartfile at '%s' already exists. Aborting", path)
}

if _, err := helm.InitChartfile(path); err != nil {
Expand Down
6 changes: 3 additions & 3 deletions pkg/helm/jsonnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@ func NativeFunc(h Helm) *jsonnet.NativeFunction {
Func: func(data []interface{}) (interface{}, error) {
name, ok := data[0].(string)
if !ok {
return nil, fmt.Errorf("First argument 'name' must be of 'string' type, got '%T' instead", data[0])
return nil, fmt.Errorf("first argument 'name' must be of 'string' type, got '%T' instead", data[0])
}

chartpath, ok := data[1].(string)
if !ok {
return nil, fmt.Errorf("Second argument 'chart' must be of 'string' type, got '%T' instead", data[1])
return nil, fmt.Errorf("second argument 'chart' must be of 'string' type, got '%T' instead", data[1])
}

// TODO: validate data[2] actually follows the struct scheme
Expand Down Expand Up @@ -90,7 +90,7 @@ func parseOpts(data interface{}) (*JsonnetOpts, error) {

// Charts are only allowed at relative paths. Use conf.CalledFrom to find the callers directory
if opts.CalledFrom == "" {
return nil, fmt.Errorf("helmTemplate: 'opts.calledFrom' is unset or empty.\nTanka needs this to find your charts. See https://tanka.dev/helm#optscalledfrom-unset\n")
return nil, fmt.Errorf("helmTemplate: 'opts.calledFrom' is unset or empty.\nTanka needs this to find your charts. See https://tanka.dev/helm#optscalledfrom-unset")
}

return &opts, nil
Expand Down
4 changes: 2 additions & 2 deletions pkg/helm/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ import (

// Template expands a Helm Chart into a regular manifest.List using the `helm
// template` command
func (h ExecHelm) Template(name, chart string, opts TemplateOpts) (manifest.List, error) {
func (e ExecHelm) Template(name, chart string, opts TemplateOpts) (manifest.List, error) {
args := []string{name, chart,
"--values", "-", // values from stdin
}
args = append(args, opts.Flags()...)

cmd := h.cmd("template", args...)
cmd := e.cmd("template", args...)
var buf bytes.Buffer
cmd.Stdout = &buf
cmd.Stderr = os.Stderr
Expand Down
6 changes: 3 additions & 3 deletions pkg/jsonnet/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,13 @@ type Opts struct {

// PathIsCached determines if a given path is matched by any of the configured cached path regexes
// If no path regexes are defined, all paths are matched
func (opts Opts) PathIsCached(path string) bool {
for _, regex := range opts.CachePathRegexes {
func (o Opts) PathIsCached(path string) bool {
for _, regex := range o.CachePathRegexes {
if regex.MatchString(path) {
return true
}
}
return len(opts.CachePathRegexes) == 0
return len(o.CachePathRegexes) == 0
}

// Clone returns a deep copy of Opts
Expand Down
2 changes: 0 additions & 2 deletions pkg/jsonnet/imports.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,12 @@ func TransitiveImports(dir string) ([]string, error) {

paths := make([]string, 0, len(imports)+1)
for k := range imports {

// Try to resolve any symlinks; use the original path as a last resort
p, err := filepath.EvalSymlinks(k)
if err != nil {
return nil, errors.Wrap(err, "resolving symlinks")
}
paths = append(paths, p)

}
paths = append(paths, entrypoint)

Expand Down
2 changes: 1 addition & 1 deletion pkg/jsonnet/jpath/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
)

// ErrorNoRoot means no rootDir was found in the parent directories
var ErrorNoRoot = errors.New(`Unable to identify the project root.
var ErrorNoRoot = errors.New(`unable to identify the project root.
Tried to find 'tkrc.yaml' or 'jsonnetfile.json' in the parent directories.
Please refer to https://tanka.dev/directory-structure for more information`)

Expand Down
5 changes: 2 additions & 3 deletions pkg/jsonnet/jpath/jpath.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"path/filepath"
)

const DEFAULT_ENTRYPOINT = "main.jsonnet"
const defaultEntrypoint = "main.jsonnet"

// Resolve the given path and resolves the jPath around it. This means it:
// - figures out the project root (the one with .jsonnetfile, vendor/ and lib/)
Expand Down Expand Up @@ -49,11 +49,10 @@ func Filename(path string) (string, error) {
}

if fi.IsDir() {
return DEFAULT_ENTRYPOINT, nil
return defaultEntrypoint, nil
}

return filepath.Base(fi.Name()), nil

}

// Entrypoint returns the absolute path of the environments entrypoint file (the
Expand Down
3 changes: 1 addition & 2 deletions pkg/jsonnet/jpath/jpath_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package jpath_test

import (
"encoding/json"
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -12,7 +11,7 @@ import (
)

func TestResolvePrecedence(t *testing.T) {
s, err := jsonnet.EvaluateFile(filepath.Join("./testdata/precedence/environments/default/main.jsonnet"), jsonnet.Opts{})
s, err := jsonnet.EvaluateFile("./testdata/precedence/environments/default/main.jsonnet", jsonnet.Opts{})
require.NoError(t, err)

want := map[string]string{
Expand Down
1 change: 0 additions & 1 deletion pkg/jsonnet/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ type LintOpts struct {
// Lint takes a list of files and directories, processes them and prints
// out to stderr if there are linting warnings
func Lint(fds []string, opts *LintOpts) error {

var paths []string
for _, f := range fds {
fs, err := FindFiles(f, opts.Excludes)
Expand Down
6 changes: 3 additions & 3 deletions pkg/kubernetes/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func (k *Kubernetes) Orphaned(state manifest.List) (manifest.List, error) {
if !k.Env.Spec.InjectLabels {
return nil, fmt.Errorf(`spec.injectLabels is set to false in your spec.json. Tanka needs to add
a label to your resources to reliably detect which were removed from Jsonnet.
See https://tanka.dev/garbage-collection for more details.`)
See https://tanka.dev/garbage-collection for more details`)
}

apiResources, err := k.ctl.Resources()
Expand Down Expand Up @@ -95,8 +95,8 @@ func (k *Kubernetes) isKubectlCreated(manifest manifest.Manifest) bool {
}
// Check if created by server-side apply
for _, manager := range manifest.Metadata().ManagedFields() {
manager_name := manager.(map[string]interface{})["manager"]
if manager_name == "tanka" || manager_name == "kubectl-client-side-apply" {
managerName := manager.(map[string]interface{})["manager"]
if managerName == "tanka" || managerName == "kubectl-client-side-apply" {
return true
}
}
Expand Down
1 change: 0 additions & 1 deletion pkg/kubernetes/client/apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ func TestKubectl_applyCtl(t *testing.T) {
if gotSet.HasAny(tt.unExpectedArgs...) {
t.Errorf("Kubectl.applyCtl() = %v has (any) unExpectedArgs='%v'", got.Args, tt.unExpectedArgs)
}

})
}
}
Loading

0 comments on commit 8cc1e0a

Please sign in to comment.