Changed linter and lint fixes

Changed the linter from gometalinter to [GolangCI-Lint](
https://github.com/golangci/golangci-lint) as it is faster
and the configuration and output are better.

Fixed issue highlighted by the new linter. The following
linters have been disabled and should be enabled in the future:

- gocyclo
- lll
- gosec

This is further to work already carried out in #206

Signed-off-by: Richard Case <richard.case@outlook.com>

.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

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

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"
+curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $GOPATH/bin v1.10.2

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
 			}
 		}

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
 	}
 

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)
 	})

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}/<logicalResourceName>"}
 func makeAutoNameTag(suffix string) gfn.Tag {

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()),

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,

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 {

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

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

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 {