From 6fd0330b80b33b19054e0229bce91d57831fe653 Mon Sep 17 00:00:00 2001 From: Oleg Atamanenko Date: Thu, 14 Jun 2018 15:38:32 -0400 Subject: [PATCH] Add gometalinter to pre-commit hook Enable varcheck and fix found issues Add ineffassign to list of checks and fix found issues Added nakedret and fixed found issues Add interfacer check and fix found issue Add lll and fix found issues Add deadcode linter, remove unused code --- .travis.yml | 1 + bin/pre-commit.sh | 24 ++++- pkg/commands/configmap.go | 4 +- pkg/commands/init.go | 96 ------------------- pkg/configmapandsecret/util/env_file.go | 2 +- pkg/configmapandsecret/util/util.go | 2 +- pkg/diff/program.go | 3 +- pkg/exec/exec_test.go | 2 +- pkg/fs/fakefile.go | 5 - pkg/hash/hash_test.go | 18 +++- pkg/transformers/labelsandannotations_test.go | 1 - pkg/transformers/namereference_test.go | 3 + 12 files changed, 43 insertions(+), 118 deletions(-) delete mode 100644 pkg/commands/init.go diff --git a/.travis.yml b/.travis.yml index 0e45d1d0f..0040ed07e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,6 +13,7 @@ before_install: - go get -u github.com/onsi/ginkgo/ginkgo - go get -u github.com/monopole/mdrip - go get -u github.com/fzipp/gocyclo + - go get -u gopkg.in/alecthomas/gometalinter.v2 && gometalinter.v2 --install # Install must be set to prevent default `go get` to run. # The dependencies have already been vendored by `dep` so diff --git a/bin/pre-commit.sh b/bin/pre-commit.sh index 308edf862..95f6e89a8 100755 --- a/bin/pre-commit.sh +++ b/bin/pre-commit.sh @@ -1,4 +1,5 @@ #!/bin/bash +set -e # Make sure, we run in the root of the repo and # therefore run the tests on all packages @@ -36,14 +37,27 @@ function testGoCyclo { diff <(echo -n) <(go_dirs | xargs -0 gocyclo -over 15) } -function testGoImports { - diff -u <(echo -n) <(go_dirs | xargs -0 goimports -l) -} - function testGoLint { diff -u <(echo -n) <(go_dirs | xargs -0 golint --min_confidence 0.85 ) } +function testGoMetalinter { + diff -u <(echo -n) <(go_dirs | xargs -0 gometalinter.v2 --disable-all --deadline 5m \ + --enable=misspell \ + --enable=structcheck \ + --enable=deadcode \ + --enable=goimports \ + --enable=varcheck \ + --enable=goconst \ + --enable=ineffassign \ + --enable=nakedret \ + --enable=interfacer \ + --enable=misspell \ + --line-length=170 --enable=lll \ + --dupl-threshold=400 --enable=dupl) +} + + function testGoVet { go vet -all ./... } @@ -57,7 +71,7 @@ function testExamples { } runTest testGoFmt -runTest testGoImports +runTest testGoMetalinter runTest testGoLint runTest testGoVet runTest testGoCyclo diff --git a/pkg/commands/configmap.go b/pkg/commands/configmap.go index 71c55ca21..9570d7e48 100644 --- a/pkg/commands/configmap.go +++ b/pkg/commands/configmap.go @@ -76,7 +76,9 @@ func newCmdAddConfigMap(errOut io.Writer, fsys fs.FileSystem) *cobra.Command { &config.FileSources, "from-file", []string{}, - "Key file can be specified using its file path, in which case file basename will be used as configmap key, or optionally with a key and file path, in which case the given key will be used. Specifying a directory will iterate each named file in the directory whose basename is a valid configmap key.") + "Key file can be specified using its file path, in which case file basename will be used as configmap "+ + "key, or optionally with a key and file path, in which case the given key will be used. Specifying a "+ + "directory will iterate each named file in the directory whose basename is a valid configmap key.") cmd.Flags().StringArrayVar( &config.LiteralSources, "from-literal", diff --git a/pkg/commands/init.go b/pkg/commands/init.go deleted file mode 100644 index 32ee0ce1d..000000000 --- a/pkg/commands/init.go +++ /dev/null @@ -1,96 +0,0 @@ -/* -Copyright 2017 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package commands - -import ( - "fmt" - "io" - - "errors" - - "github.com/kubernetes-sigs/kustomize/pkg/constants" - "github.com/kubernetes-sigs/kustomize/pkg/fs" - "github.com/spf13/cobra" -) - -const kustomizationTemplate = ` -namePrefix: some-prefix -# Labels to add to all objects and selectors. -# These labels would also be used to form the selector for apply --prune -# Named differently than “labels” to avoid confusion with metadata for this object -commonLabels: - app: helloworld -commonAnnotations: - note: This is an example annotation -resources: [] -#- service.yaml -#- ../some-dir/ -# There could also be configmaps in Base, which would make these overlays -configMapGenerator: [] -# There could be secrets in Base, if just using a fork/rebase workflow -secretGenerator: [] -` - -type initOptions struct { -} - -// NewCmdInit makes the init command. -func newCmdInit(out, errOut io.Writer, fs fs.FileSystem) *cobra.Command { - var o initOptions - - cmd := &cobra.Command{ - Use: "init", - Short: "Creates a file called \"" + constants.KustomizationFileName + "\" in the current directory", - Long: "Creates a file called \"" + - constants.KustomizationFileName + "\" in the current directory with example values.", - Example: `init`, - SilenceUsage: true, - RunE: func(cmd *cobra.Command, args []string) error { - err := o.Validate(cmd, args) - if err != nil { - return err - } - err = o.Complete(cmd, args) - if err != nil { - return err - } - return o.RunInit(out, errOut, fs) - }, - } - return cmd -} - -// Validate validates init command. -func (o *initOptions) Validate(cmd *cobra.Command, args []string) error { - if len(args) > 0 { - return errors.New("The init command takes no arguments.") - } - return nil -} - -// Complete completes init command. -func (o *initOptions) Complete(cmd *cobra.Command, args []string) error { - return nil -} - -// RunInit writes a kustomization file. -func (o *initOptions) RunInit(out, errOut io.Writer, fs fs.FileSystem) error { - if _, err := fs.Stat(constants.KustomizationFileName); err == nil { - return fmt.Errorf("%q already exists", constants.KustomizationFileName) - } - return fs.WriteFile(constants.KustomizationFileName, []byte(kustomizationTemplate)) -} diff --git a/pkg/configmapandsecret/util/env_file.go b/pkg/configmapandsecret/util/env_file.go index 5f687202c..ef4183062 100644 --- a/pkg/configmapandsecret/util/env_file.go +++ b/pkg/configmapandsecret/util/env_file.go @@ -66,7 +66,7 @@ func processEnvFileLine(line []byte, filePath string, // from the environment. value = os.Getenv(key) } - return + return key, value, err } // addFromEnvFile processes an env file allows a generic addTo to handle the diff --git a/pkg/configmapandsecret/util/util.go b/pkg/configmapandsecret/util/util.go index af9c69021..7d0bd7288 100644 --- a/pkg/configmapandsecret/util/util.go +++ b/pkg/configmapandsecret/util/util.go @@ -42,7 +42,7 @@ func ParseRFC3339(s string, nowFn func() metav1.Time) (metav1.Time, error) { } // HashObject encodes object using given codec and returns MD5 sum of the result. -func HashObject(obj runtime.Object, codec runtime.Codec) (string, error) { +func HashObject(obj runtime.Object, codec runtime.Encoder) (string, error) { data, err := runtime.Encode(codec, obj) if err != nil { return "", err diff --git a/pkg/diff/program.go b/pkg/diff/program.go index 8fb35571b..13137dcf0 100644 --- a/pkg/diff/program.go +++ b/pkg/diff/program.go @@ -39,11 +39,10 @@ func newProgram(out, errOut io.Writer) *program { } func (d *program) makeCommand(args ...string) exec.Cmd { - diff := "" + diff := "diff" if envDiff := os.Getenv("KUBERNETES_EXTERNAL_DIFF"); envDiff != "" { diff = envDiff } else { - diff = "diff" args = append([]string{"-u", "-N"}, args...) } cmd := exec.New().Command(diff, args...) diff --git a/pkg/exec/exec_test.go b/pkg/exec/exec_test.go index 99ff44df0..5a2209e75 100644 --- a/pkg/exec/exec_test.go +++ b/pkg/exec/exec_test.go @@ -54,7 +54,7 @@ func TestExecutorNoArgs(t *testing.T) { } cmd = ex.Command("/does/not/exist") - out, err = cmd.CombinedOutput() + _, err = cmd.CombinedOutput() if err == nil { t.Errorf("expected failure, got nil error") } diff --git a/pkg/fs/fakefile.go b/pkg/fs/fakefile.go index a40bf7a55..64bc55685 100644 --- a/pkg/fs/fakefile.go +++ b/pkg/fs/fakefile.go @@ -31,11 +31,6 @@ type FakeFile struct { open bool } -// makeFile makes a fake file. -func makeFile() *FakeFile { - return &FakeFile{} -} - // makeDir makes a fake directory. func makeDir(name string) *FakeFile { return &FakeFile{name: name, dir: true} diff --git a/pkg/hash/hash_test.go b/pkg/hash/hash_test.go index f527a98a2..603956376 100644 --- a/pkg/hash/hash_test.go +++ b/pkg/hash/hash_test.go @@ -96,15 +96,19 @@ func TestEncodeConfigMap(t *testing.T) { // one key {"one key", &v1.ConfigMap{Data: map[string]string{"one": ""}}, `{"data":{"one":""},"kind":"ConfigMap","name":""}`, ""}, // three keys (tests sorting order) - {"three keys", &v1.ConfigMap{Data: map[string]string{"two": "2", "one": "", "three": "3"}}, `{"data":{"one":"","three":"3","two":"2"},"kind":"ConfigMap","name":""}`, ""}, + {"three keys", &v1.ConfigMap{Data: map[string]string{"two": "2", "one": "", "three": "3"}}, + `{"data":{"one":"","three":"3","two":"2"},"kind":"ConfigMap","name":""}`, ""}, // empty binary map {"empty data", &v1.ConfigMap{BinaryData: map[string][]byte{}}, `{"data":null,"kind":"ConfigMap","name":""}`, ""}, // one key with binary data - {"one key", &v1.ConfigMap{BinaryData: map[string][]byte{"one": []byte("")}}, `{"binaryData":{"one":""},"data":null,"kind":"ConfigMap","name":""}`, ""}, + {"one key", &v1.ConfigMap{BinaryData: map[string][]byte{"one": []byte("")}}, + `{"binaryData":{"one":""},"data":null,"kind":"ConfigMap","name":""}`, ""}, // three keys with binary data (tests sorting order) - {"three keys", &v1.ConfigMap{BinaryData: map[string][]byte{"two": []byte("2"), "one": []byte(""), "three": []byte("3")}}, `{"binaryData":{"one":"","three":"Mw==","two":"Mg=="},"data":null,"kind":"ConfigMap","name":""}`, ""}, + {"three keys", &v1.ConfigMap{BinaryData: map[string][]byte{"two": []byte("2"), "one": []byte(""), "three": []byte("3")}}, + `{"binaryData":{"one":"","three":"Mw==","two":"Mg=="},"data":null,"kind":"ConfigMap","name":""}`, ""}, // two keys, one string and one binary values - {"two keys with one each", &v1.ConfigMap{Data: map[string]string{"one": ""}, BinaryData: map[string][]byte{"two": []byte("")}}, `{"binaryData":{"two":""},"data":{"one":""},"kind":"ConfigMap","name":""}`, ""}, + {"two keys with one each", &v1.ConfigMap{Data: map[string]string{"one": ""}, BinaryData: map[string][]byte{"two": []byte("")}}, + `{"binaryData":{"two":""},"data":{"one":""},"kind":"ConfigMap","name":""}`, ""}, } for _, c := range cases { s, err := encodeConfigMap(c.cm) @@ -129,7 +133,11 @@ func TestEncodeSecret(t *testing.T) { // one key {"one key", &v1.Secret{Type: "my-type", Data: map[string][]byte{"one": []byte("")}}, `{"data":{"one":""},"kind":"Secret","name":"","type":"my-type"}`, ""}, // three keys (tests sorting order) - note json.Marshal base64 encodes the values because they come in as []byte - {"three keys", &v1.Secret{Type: "my-type", Data: map[string][]byte{"two": []byte("2"), "one": []byte(""), "three": []byte("3")}}, `{"data":{"one":"","three":"Mw==","two":"Mg=="},"kind":"Secret","name":"","type":"my-type"}`, ""}, + {"three keys", &v1.Secret{ + Type: "my-type", + Data: map[string][]byte{"two": []byte("2"), "one": []byte(""), "three": []byte("3")}, + }, + `{"data":{"one":"","three":"Mw==","two":"Mg=="},"kind":"Secret","name":"","type":"my-type"}`, ""}, } for _, c := range cases { s, err := encodeSecret(c.secret) diff --git a/pkg/transformers/labelsandannotations_test.go b/pkg/transformers/labelsandannotations_test.go index 26d123f57..73b27f100 100644 --- a/pkg/transformers/labelsandannotations_test.go +++ b/pkg/transformers/labelsandannotations_test.go @@ -30,7 +30,6 @@ var secret = schema.GroupVersionKind{Version: "v1", Kind: "Secret"} var cmap = schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"} var ns = schema.GroupVersionKind{Version: "v1", Kind: "Namespace"} var deploy = schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"} -var statefulset = schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "StatefulSet"} var foo = schema.GroupVersionKind{Group: "example.com", Version: "v1", Kind: "Foo"} var crd = schema.GroupVersionKind{Group: "apiwctensions.k8s.io", Version: "v1beta1", Kind: "CustomResourceDefinition"} diff --git a/pkg/transformers/namereference_test.go b/pkg/transformers/namereference_test.go index a66d3780c..009de8ffe 100644 --- a/pkg/transformers/namereference_test.go +++ b/pkg/transformers/namereference_test.go @@ -191,6 +191,9 @@ func TestNameReferenceRun(t *testing.T) { } nrt, err := NewDefaultingNameReferenceTransformer() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } err = nrt.Transform(m) if err != nil { t.Fatalf("unexpected error: %v", err)