diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000000..462c4c68a7 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,154 @@ +# options for analysis running +run: + # default concurrency is a available CPU number + concurrency: 4 + + # timeout for analysis, e.g. 30s, 5m, default is 1m + deadline: 5m + + # exit code when at least one issue was found, default is 1 + issues-exit-code: 1 + + # include test files or not, default is true + tests: false + + # list of build tags, all linters use it. Default is empty list. + #build-tags: + # - mytag + + # which dirs to skip: they won't be analyzed; + # can use regexp here: generated.*, regexp is applied on full path; + # default value is empty list, but next dirs are always skipped independently + # from this option's value: + # vendor$, third_party$, testdata$, examples$, Godeps$, builtin$ + skip-dirs: + - ^vendor\/ + - ^build\/ + - ^pkg\/nodebootstrap\/assets.go + - ^pkg\/testutils\/ + - ^pkg\/eks\/mocks\/ + - ^tests\/ + - \/usr\/local\/go + + # which files to skip: they will be analyzed, but issues from them + # won't be reported. Default value is empty list, but there is + # no need to include all autogenerated files, we confidently recognize + # autogenerated files. If it's not please let us know. + #skip-files: + # - ".*\\.my\\.go$" + # - lib/bad.go + + +# output configuration options +output: + # colored-line-number|line-number|json|tab|checkstyle, default is "colored-line-number" + format: tab + + # print lines of code with issue, default is true + print-issued-lines: true + + # print linter name in the end of issue text, default is true + print-linter-name: true + + +# all available settings of specific linters +linters-settings: + errcheck: + # report about not checking of errors in type assetions: `a := b.(MyStruct)`; + # default is false: such cases aren't reported by default. + check-type-assertions: false + + # report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`; + # default is false: such cases aren't reported by default. + check-blank: false + govet: + # report about shadowed variables + check-shadowing: true + + # Obtain type information from installed (to $GOPATH/pkg) package files: + # golangci-lint will execute `go install -i` and `go test -i` for analyzed packages + # before analyzing them. + # By default this option is disabled and govet gets type information by loader from source code. + # Loading from source code is slow, but it's done only once for all linters. + # Go-installing of packages first time is much slower than loading them from source code, + # therefore this option is disabled by default. + # But repeated installation is fast in go >= 1.10 because of build caching. + # Enable this option only if all conditions are met: + # 1. you use only "fast" linters (--fast e.g.): no program loading occurs + # 2. you use go >= 1.10 + # 3. you do repeated runs (false for CI) or cache $GOPATH/pkg or `go env GOCACHE` dir in CI. + use-installed-packages: false + golint: + # minimal confidence for issues, default is 0.8 + min-confidence: 0.8 + gofmt: + # simplify code: gofmt with `-s` option, true by default + simplify: false + gocyclo: + # minimal code complexity to report, 30 by default (but we recommend 10-20) + min-complexity: 10 + maligned: + # print struct with more effective memory layout or not, false by default + suggest-new: true + dupl: + # tokens count to trigger issue, 150 by default + threshold: 100 + goconst: + # minimal length of string constant, 3 by default + min-len: 3 + # minimal occurrences count to trigger, 3 by default + min-occurrences: 3 + depguard: + list-type: blacklist + include-go-root: false + packages: + - github.com/golang/glog + misspell: + # Correct spellings using locale preferences for US or UK. + # Default is to use a neutral variety of English. + # Setting locale to US will correct the British spelling of 'colour' to 'color'. + locale: UK + lll: + # max line length, lines longer will be reported. Default is 120. + # '\t' is counted as 1 character by default, and can be changed with the tab-width option + line-length: 120 + # tab width in spaces. Default to 1. + tab-width: 1 + unused: + # treat code as a program (not a library) and report unused exported identifiers; default is false. + # XXX: if you enable this setting, unused will report a lot of false-positives in text editors: + # if it's called for subdir of a project it can't find funcs usages. All text editor integrations + # with golangci-lint call it on a directory with the changed file. + check-exported: false + unparam: + # call graph construction algorithm (cha, rta). In general, use cha for libraries, + # and rta for programs with main packages. Default is cha. + algo: cha + + # Inspect exported functions, default is false. Set to true if no external program/library imports your code. + # XXX: if you enable this setting, unparam will report a lot of false-positives in text editors: + # if it's called for subdir of a project it can't find external interfaces. All text editor integrations + # with golangci-lint call it on a directory with the changed file. + check-exported: false + nakedret: + # make an issue if func has more lines of code than this setting and it has naked returns; default is 30 + max-func-lines: 30 + prealloc: + # XXX: we don't recommend using this linter before doing performance profiling. + # For most programs usage of prealloc will be a premature optimization. + + # Report preallocation suggestions only on simple loops that have no returns/breaks/continues/gotos in them. + # True by default. + simple: true + range-loops: true # Report preallocation suggestions on range loops, true by default + for-loops: false # Report preallocation suggestions on for loops, false by default + + +linters: + enable-all: true + disable: + - maligned + - prealloc + - gocyclo # TODO: enable this in the future + - lll #TODO: enable this in the future + - gosec #TODO: enable this in the future diff --git a/.gometalinter.json b/.gometalinter.json deleted file mode 100644 index a8e11d5991..0000000000 --- a/.gometalinter.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "Enable": [ - "vet", - "golint", - "errcheck", - "deadcode" - ], - "Exclude": [ - "^vendor\/", - "^build\/", - "^pkg\/nodebootstrap\/assets.go", - "\/usr\/local\/go" - ], - "Deadline": "5m" -} \ No newline at end of file diff --git a/Makefile b/Makefile index 2231626d9a..fd463580fe 100644 --- a/Makefile +++ b/Makefile @@ -28,8 +28,8 @@ test: generate ## Run unit tests @test -z $(COVERALLS_TOKEN) || $(GOPATH)/bin/goveralls -coverprofile=coverage.out -service=circle-ci .PHONY: lint -lint: ## Run GoMetalinter over the codebase - @$(GOPATH)/bin/gometalinter ./... +lint: ## Run linter over the codebase + @$(GOPATH)/bin/golangci-lint run .PHONY: ci ci: test lint ## Target for CI system to invoke to run tests and linting diff --git a/build/install.sh b/build/install.sh index 0e4f2ec905..02c197f511 100755 --- a/build/install.sh +++ b/build/install.sh @@ -6,23 +6,4 @@ go install ./vendor/golang.org/x/tools/cmd/stringer go install ./vendor/github.com/mattn/goveralls go install ./vendor/github.com/vektra/mockery/cmd/mockery -# managing all linters that gometalinter uses with dep is going to take -# a lot of work, so we install all of those from the release tarball -install_gometalinter() { - version="${1}" - prefix="https://github.com/alecthomas/gometalinter/releases/download" - if [ "$(uname)" = "Darwin" ] ; then - suffix="darwin-amd64" - else - suffix="linux-amd64" - fi - basename="gometalinter-${version}-${suffix}" - url="${prefix}/v${version}/${basename}.tar.gz" - cd "${GOPATH}/bin/" - curl --silent --location "${url}" | tar xz - (cd "./${basename}/" ; mv ./* ../) - rmdir "./${basename}" - unset version prefix suffix basename url -} - -install_gometalinter "2.0.11" \ No newline at end of file +curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $GOPATH/bin v1.10.2 diff --git a/cmd/eksctl/create.go b/cmd/eksctl/create.go index 6804572dd8..b63d2b1bf8 100644 --- a/cmd/eksctl/create.go +++ b/cmd/eksctl/create.go @@ -192,23 +192,23 @@ func doCreateCluster(cfg *api.ClusterConfig, name string) error { return err } - if err := ctl.WaitForControlPlane(clientSet); err != nil { + if err = ctl.WaitForControlPlane(clientSet); err != nil { return err } // authorise nodes to join - if err := ctl.CreateDefaultNodeGroupAuthConfigMap(clientSet); err != nil { + if err = ctl.CreateDefaultNodeGroupAuthConfigMap(clientSet); err != nil { return err } // wait for nodes to join - if err := ctl.WaitForNodes(clientSet); err != nil { + if err = ctl.WaitForNodes(clientSet); err != nil { return err } // add default storage class - if cfg.Addons.Storage == true { - if err := ctl.AddDefaultStorageClass(clientSet); err != nil { + if cfg.Addons.Storage { + if err = ctl.AddDefaultStorageClass(clientSet); err != nil { return err } } diff --git a/cmd/eksctl/utils.go b/cmd/eksctl/utils.go index 57daae90c1..a6261ab2c9 100644 --- a/cmd/eksctl/utils.go +++ b/cmd/eksctl/utils.go @@ -154,7 +154,7 @@ func doWriteKubeconfigCmd(cfg *api.ClusterConfig, name string) error { logger.Debug("cluster = %#v", cluster) - if err := ctl.GetCredentials(*cluster); err != nil { + if err = ctl.GetCredentials(*cluster); err != nil { return err } diff --git a/pkg/ami/api.go b/pkg/ami/api.go index 43f7b9e10b..4edd14201a 100644 --- a/pkg/ami/api.go +++ b/pkg/ami/api.go @@ -92,7 +92,9 @@ func FindImage(api ec2iface.EC2API, namePattern string) (string, error) { // Sort images so newest is first sort.Slice(output.Images, func(i, j int) bool { + //nolint:gosec creationLeft, _ := time.Parse(time.RFC3339, *output.Images[i].CreationDate) + //nolint:gosec creationRight, _ := time.Parse(time.RFC3339, *output.Images[j].CreationDate) return creationLeft.After(creationRight) }) diff --git a/pkg/cfn/builder/api.go b/pkg/cfn/builder/api.go index d05b13b1d2..83578fa75b 100644 --- a/pkg/cfn/builder/api.go +++ b/pkg/cfn/builder/api.go @@ -58,23 +58,6 @@ func makeStringSlice(s ...string) []*gfn.Value { return slice } -func (r *resourceSet) newParameter(name, valueType, defaultValue string) *gfn.Value { - p := map[string]string{"Type": valueType} - if defaultValue != "" { - p["Default"] = defaultValue - } - r.template.Parameters[name] = p - return gfn.MakeRef(name) -} - -func (r *resourceSet) newStringParameter(name, defaultValue string) *gfn.Value { - return r.newParameter(name, "String", defaultValue) -} - -func (r *resourceSet) newNumberParameter(name, defaultValue string) *gfn.Value { - return r.newParameter(name, "Number", defaultValue) -} - // makeAutoNameTag create a new Name tag in the following format: // {Key: "Name", Value: !Sub "${AWS::StackName}/"} func makeAutoNameTag(suffix string) gfn.Tag { diff --git a/pkg/cfn/builder/vpc.go b/pkg/cfn/builder/vpc.go index 188bedfc8f..551f0f9bda 100644 --- a/pkg/cfn/builder/vpc.go +++ b/pkg/cfn/builder/vpc.go @@ -13,6 +13,7 @@ const ( cfnOutputClusterSecurityGroup = "SecurityGroup" ) +//nolint:interfacer func (c *ClusterResourceSet) addResourcesForVPC(globalCIDR *net.IPNet, subnets map[string]*net.IPNet) { refVPC := c.newResource("VPC", &gfn.AWSEC2VPC{ CidrBlock: gfn.NewString(globalCIDR.String()), diff --git a/pkg/cfn/manager/api.go b/pkg/cfn/manager/api.go index 8bce8b1250..27c25822fe 100644 --- a/pkg/cfn/manager/api.go +++ b/pkg/cfn/manager/api.go @@ -208,7 +208,7 @@ func (c *StackCollection) DescribeStacks(name string) ([]*Stack, error) { return stacks, nil } -// DescribeStackEvents describes the occured stack events +// DescribeStackEvents describes the occurred stack events func (c *StackCollection) DescribeStackEvents(i *Stack) ([]*cloudformation.StackEvent, error) { input := &cloudformation.DescribeStackEventsInput{ StackName: i.StackId, diff --git a/pkg/cloudconfig/cloudconfig.go b/pkg/cloudconfig/cloudconfig.go index 1a60f9c361..c31625f23b 100644 --- a/pkg/cloudconfig/cloudconfig.go +++ b/pkg/cloudconfig/cloudconfig.go @@ -108,10 +108,10 @@ func (c *CloudConfig) Encode() (string, error) { gw := gzip.NewWriter(buf) - if _, err := gw.Write(data); err != nil { + if _, err = gw.Write(data); err != nil { return "", err } - if err := gw.Close(); err != nil { + if err = gw.Close(); err != nil { return "", err } data, err = ioutil.ReadAll(buf) @@ -135,6 +135,9 @@ func DecodeCloudConfig(s string) (*CloudConfig, error) { } gr, err := gzip.NewReader(ioutil.NopCloser(bytes.NewBuffer(data))) + if err != nil { + return nil, err + } defer close(gr) data, err = ioutil.ReadAll(gr) if err != nil { diff --git a/pkg/eks/api.go b/pkg/eks/api.go index 2b16403e4b..97adf0f771 100644 --- a/pkg/eks/api.go +++ b/pkg/eks/api.go @@ -29,7 +29,7 @@ import ( "github.com/kubicorn/kubicorn/pkg/logger" ) -// ClusterProvider stores infromation about the cluster +// ClusterProvider stores information about the cluster type ClusterProvider struct { // core fields used for config and AWS APIs Spec *api.ClusterConfig @@ -60,9 +60,8 @@ func (p ProviderServices) STS() stsiface.STSAPI { return p.sts } // ProviderStatus stores information about the used IAM role and the resulting session type ProviderStatus struct { - iamRoleARN string - sessionCreds *credentials.Credentials - availabilityZones []string + iamRoleARN string + sessionCreds *credentials.Credentials } // New creates a new setup of the used AWS APIs diff --git a/pkg/eks/auth.go b/pkg/eks/auth.go index 7a26ec03e7..8f4bde94f9 100644 --- a/pkg/eks/auth.go +++ b/pkg/eks/auth.go @@ -76,7 +76,7 @@ func (c *ClusterProvider) importSSHPublicKeyIfNeeded() error { PublicKeyMaterial: c.Spec.SSHPublicKey, } logger.Info("importing SSH public key %q as %q", c.Spec.SSHPublicKeyPath, c.Spec.SSHPublicKeyName) - if _, err := c.Provider.EC2().ImportKeyPair(input); err != nil { + if _, err = c.Provider.EC2().ImportKeyPair(input); err != nil { return errors.Wrap(err, "importing SSH public key") } return nil diff --git a/pkg/eks/nodegroup.go b/pkg/eks/nodegroup.go index 045e95360d..9f68b829cd 100644 --- a/pkg/eks/nodegroup.go +++ b/pkg/eks/nodegroup.go @@ -65,7 +65,7 @@ func isNodeReady(node *corev1.Node) bool { } func getNodes(clientSet *clientset.Clientset) (int, error) { - nodes, err := clientSet.Core().Nodes().List(metav1.ListOptions{}) + nodes, err := clientSet.CoreV1().Nodes().List(metav1.ListOptions{}) if err != nil { return 0, err } @@ -88,7 +88,7 @@ func (c *ClusterProvider) WaitForNodes(clientSet *clientset.Clientset) error { } timer := time.After(c.Spec.WaitTimeout) timeout := false - watcher, err := clientSet.Core().Nodes().Watch(metav1.ListOptions{}) + watcher, err := clientSet.CoreV1().Nodes().Watch(metav1.ListOptions{}) if err != nil { return errors.Wrap(err, "creating node watcher") } @@ -101,7 +101,7 @@ func (c *ClusterProvider) WaitForNodes(clientSet *clientset.Clientset) error { logger.Info("waiting for at least %d nodes to become ready", c.Spec.MinNodes) for !timeout && counter <= c.Spec.MinNodes { select { - case event, _ := <-watcher.ResultChan(): + case event := <-watcher.ResultChan(): logger.Debug("event = %#v", event) if event.Object != nil && event.Type != watch.Deleted { if node, ok := event.Object.(*corev1.Node); ok { diff --git a/pkg/nodebootstrap/userdata.go b/pkg/nodebootstrap/userdata.go index 66b6a851da..d76cc2fde4 100644 --- a/pkg/nodebootstrap/userdata.go +++ b/pkg/nodebootstrap/userdata.go @@ -71,7 +71,7 @@ func addFilesAndScripts(config *cloudconfig.CloudConfig, files configFiles, scri return nil } -func makeAmazonLinux2Config(config *cloudconfig.CloudConfig, spec *api.ClusterConfig) (configFiles, error) { +func makeAmazonLinux2Config(spec *api.ClusterConfig) (configFiles, error) { if spec.MaxPodsPerNode == 0 { spec.MaxPodsPerNode = maxPodsPerNodeType[spec.NodeType] } @@ -120,12 +120,12 @@ func NewUserDataForAmazonLinux2(spec *api.ClusterConfig) (string, error) { "bootstrap.al2.sh", } - files, err := makeAmazonLinux2Config(config, spec) + files, err := makeAmazonLinux2Config(spec) if err != nil { return "", err } - if err := addFilesAndScripts(config, files, scripts); err != nil { + if err = addFilesAndScripts(config, files, scripts); err != nil { return "", err } @@ -134,6 +134,6 @@ func NewUserDataForAmazonLinux2(spec *api.ClusterConfig) (string, error) { return "", errors.Wrap(err, "encoding user data") } - logger.Debug("user-data = %s", string(body)) - return string(body), nil + logger.Debug("user-data = %s", body) + return body, nil } diff --git a/pkg/utils/kubeconfig/kubeconfig.go b/pkg/utils/kubeconfig/kubeconfig.go index f8c3f74190..a06186a17e 100644 --- a/pkg/utils/kubeconfig/kubeconfig.go +++ b/pkg/utils/kubeconfig/kubeconfig.go @@ -81,13 +81,13 @@ func Write(path string, newConfig *clientcmdapi.Config, setContext bool) (string configAccess := getConfigAccess(path) config, err := configAccess.GetStartingConfig() - - logger.Debug("merging kubeconfig files") - merged, err := merge(config, newConfig) if err != nil { - return "", errors.Wrapf(err, "unable to merge configuration with existing kubeconfig file %q", path) + return "", errors.Wrapf(err, "enable to read existing kubeconfig file %q", path) } + logger.Debug("merging kubeconfig files") + merged := merge(config, newConfig) + if setContext && newConfig.CurrentContext != "" { logger.Debug("setting current-context to %s", newConfig.CurrentContext) merged.CurrentContext = newConfig.CurrentContext @@ -112,7 +112,7 @@ func getConfigAccess(explicitPath string) clientcmd.ConfigAccess { return interface{}(pathOptions).(clientcmd.ConfigAccess) } -func merge(existing *clientcmdapi.Config, tomerge *clientcmdapi.Config) (*clientcmdapi.Config, error) { +func merge(existing *clientcmdapi.Config, tomerge *clientcmdapi.Config) *clientcmdapi.Config { for k, v := range tomerge.Clusters { existing.Clusters[k] = v } @@ -123,7 +123,7 @@ func merge(existing *clientcmdapi.Config, tomerge *clientcmdapi.Config) (*client existing.Contexts[k] = v } - return existing, nil + return existing } // AutoPath returns the path for the auto-generated kubeconfig @@ -166,18 +166,23 @@ func MaybeDeleteConfig(ctl *api.ClusterConfig) { return } if autoConfExists { - if err := isValidConfig(p, ctl.ClusterName); err != nil { + if err = isValidConfig(p, ctl.ClusterName); err != nil { logger.Debug(err.Error()) return } - if err := os.Remove(p); err != nil { + if err = os.Remove(p); err != nil { logger.Debug("ignoring error while removing auto-generated config file %q: %s", p, err.Error()) } return } configAccess := getConfigAccess(DefaultPath) - config, _ := configAccess.GetStartingConfig() + config, err := configAccess.GetStartingConfig() + if err != nil { + logger.Debug("error reading kubeconfig file %q: %s", DefaultPath, err.Error()) + return + } + if !deleteClusterInfo(config, ctl) { return } diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 88b36afb22..72e73e4aee 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -11,7 +11,7 @@ import ( ) // IsGPUInstanceType returns tru of the instance type is GPU -// optimized. +// optimised. func IsGPUInstanceType(instanceType string) bool { return strings.HasPrefix(instanceType, "p2") || strings.HasPrefix(instanceType, "p3") }