Merge pull request #1438 from ankitm123/lint-repo

refactor: fix lint issues and enable lint step

Makefile

@@ -141,11 +141,10 @@ $(GOLINT):
 	$(GO_NOMOD) get -u golang.org/x/lint/golint
 
 .PHONY: lint
-lint: $(GOLINT) ## Runs 'go vet' anf 'go lint'
-	@echo "VETTING"
-	$(GO) vet ./...
-	@echo "LINTING"
-	$(GOLINT) -set_exit_status ./...
+lint: ## Lint the code
+	./hack/gofmt.sh
+	./hack/linter.sh
+
 
 GOSEC := $(GOPATH)/bin/gosec
 $(GOSEC):

hack/gofmt.sh

@@ -0,0 +1,10 @@
+#!/bin/bash
+
+files=$(find . -name "*.go" | grep -v vendor/ | grep -v ./pkg/client/openapi/all | grep -v Dockerfile | xargs gofmt -l -s)
+if [[ $files ]]; then
+    echo "Gofmt errors in files:"
+    echo "$files"
+    diff=$(find . -name "*.go" | grep -v vendor/ | grep -v ./pkg/client/openapi/all | grep -v Dockerfile | xargs gofmt -d -s)
+    echo "$diff"
+    exit 1
+fi

hack/linter.sh

@@ -0,0 +1,20 @@
+#!/bin/bash
+
+set -e -o pipefail
+
+if [ "$DISABLE_LINTER" == "true" ]
+then
+  exit 0
+fi
+
+DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+
+if ! [ -x "$(command -v golangci-lint)" ]; then
+	echo "Installing GolangCI-Lint"
+	${DIR}/install_golint.sh -b $GOPATH/bin v1.42.1
+fi
+
+export GO111MODULE=on
+golangci-lint run \
+  --verbose \
+  --build-tags build

pkg/apis/lighthouse/v1alpha1/types.go

@@ -384,7 +384,7 @@ type ActivityRecord struct {
 	StartTime       *metav1.Time           `json:"startTime,omitempty"`
 	CompletionTime  *metav1.Time           `json:"completionTime,omitempty"`
 	Stages          []*ActivityStageOrStep `json:"stages,omitempty"`
-	Steps           []*ActivityStageOrStep `json:"steps,omitEmpty"`
+	Steps           []*ActivityStageOrStep `json:"steps,omitempty"`
 }
 
 // ActivityStageOrStep represents a stage of an activity

pkg/engines/jenkins/client_test.go

@@ -112,7 +112,7 @@ func testWrapper(t *testing.T, jobs []string, builds map[string][]Build, status
 			t.Errorf("unexpected error while marshaling builds: %v", err)
 			return
 		}
-		_, _ = fmt.Fprint(w, fmt.Sprintf(`{"builds": %s}`, string(data)))
+		_, _ = fmt.Fprint(w, fmt.Sprintf(`{"builds": %s}`, string(data))) //nolint: gosimple
 	}
 }
 
@@ -125,7 +125,7 @@ func intP(i int) *int {
 }
 
 func TestListBuilds(t *testing.T) {
-	type Task struct {
+	type Task struct { //nolint:unused
 		// Used for tracking unscheduled builds for jobs.
 		Name string `json:"name"`
 	}

pkg/engines/jenkins/controller.go

@@ -34,20 +34,12 @@ import (
 	"github.com/jenkins-x/lighthouse/pkg/config"
 	"github.com/jenkins-x/lighthouse/pkg/config/lighthouse"
 	"github.com/jenkins-x/lighthouse/pkg/jobutil"
-	"k8s.io/apimachinery/pkg/types"
 
 	"github.com/bwmarrin/snowflake"
 	"github.com/sirupsen/logrus"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-type lighthouseJobClient interface {
-	Create(*v1alpha1.LighthouseJob) (*v1alpha1.LighthouseJob, error)
-	List(metav1.ListOptions) (*v1alpha1.LighthouseJobList, error)
-	UpdateStatus(*v1alpha1.LighthouseJob) (*v1alpha1.LighthouseJob, error)
-	Patch(name string, pt types.PatchType, data []byte, subresources ...string) (result *v1alpha1.LighthouseJob, err error)
-}
-
 type jenkinsClient interface {
 	Build(*v1alpha1.LighthouseJob, string) error
 	ListBuilds(jobs []BuildQueryParams) (map[string]Build, error)
@@ -63,8 +55,6 @@ type Controller struct {
 	log              *logrus.Entry
 	cfg              config.Getter
 	node             *snowflake.Node
-	// if skip report job results to github
-	skipReport bool
 	// selector that will be applied on Lighthouse jobs.
 	selector string
 

pkg/engines/tekton/controller_test.go

@@ -107,7 +107,7 @@ func TestReconcile(t *testing.T) {
 			// assert observed state matches expected state
 			if expectedPR != nil || generateTestOutput {
 				var pipelineRunList tektonv1beta1.PipelineRunList
-				err := c.List(nil, &pipelineRunList, client.InNamespace(ns))
+				err := c.List(context.TODO(), &pipelineRunList, client.InNamespace(ns))
 				assert.NoError(t, err)
 				assert.Len(t, pipelineRunList.Items, 1)
 				updatedPR := pipelineRunList.Items[0].DeepCopy()
@@ -127,7 +127,7 @@ func TestReconcile(t *testing.T) {
 			}
 			if expectedJob != nil {
 				var jobList lighthousev1alpha1.LighthouseJobList
-				err := c.List(nil, &jobList, client.InNamespace(ns))
+				err := c.List(context.TODO(), &jobList, client.InNamespace(ns))
 				assert.NoError(t, err)
 				assert.Len(t, jobList.Items, 1)
 				// Ignore status.starttime since that's always going to be different

pkg/filebrowser/git_file_browser.go

@@ -27,7 +27,7 @@ type gitFileBrowser struct {
 const headBranchPrefix = "HEAD branch:"
 
 var (
-	shaRegex = regexp.MustCompile("\\b[0-9a-f]{7,40}\\b")
+	shaRegex = regexp.MustCompile("\\b[0-9a-f]{7,40}\\b") //nolint:gosimple
 )
 
 // NewFileBrowserFromGitClient creates a new file browser from an Scm client
@@ -241,7 +241,7 @@ func (c *repoClientFacade) UseRef(ref string, fc FetchCache) error {
 		return errors.Wrapf(err, "failed to checkout repository %s ref %s", c.fullName, ref)
 	}
 
-	duration := time.Now().Sub(start)
+	duration := time.Since(start)
 	logrus.StandardLogger().WithFields(map[string]interface{}{
 		"Name":     c.fullName,
 		"Ref":      ref,

pkg/foghorn/controller_test.go

@@ -122,7 +122,7 @@ func TestReconcile(t *testing.T) {
 			assert.NoError(t, err)
 
 			var jobList lighthousev1alpha1.LighthouseJobList
-			err = c.List(nil, &jobList, client.InNamespace(ns))
+			err = c.List(context.TODO(), &jobList, client.InNamespace(ns))
 			assert.NoError(t, err)
 			assert.Len(t, jobList.Items, 1)
 			// Ignore status.starttime since that's always going to be different

pkg/git/v2/client_factory.go

@@ -299,7 +299,7 @@ func (c *clientFactory) ClientFor(org, repo string, sparseCheckoutPatterns []str
 		}
 	}
 
-	duration := time.Now().Sub(start)
+	duration := time.Since(start)
 	l.WithField("Duration", duration.String()).Debug("cloned repository")
 	return repoClient, nil
 }

pkg/git/v2/no_mirror_client_factory.go

@@ -133,7 +133,7 @@ func (c *noMirrorClientFactory) ClientFor(org, repo string, sparseCheckoutPatter
 			return nil, err
 		}
 	}
-	duration := time.Now().Sub(start)
+	duration := time.Since(start)
 	l.WithField("Duration", duration.String()).Debug("cloned repository")
 	return repoClient, nil
 }

pkg/interrupts/interrupts_test.go

@@ -194,7 +194,7 @@ func TestInterrupts(t *testing.T) {
 	lock.Unlock()
 }
 
-func generateCerts(url string) (string, string, error) {
+func generateCerts(url string) (string, string, error) { //nolint:unused
 	priv, err := rsa.GenerateKey(rand.Reader, 2048)
 	if err != nil {
 		return "", "", fmt.Errorf("failed to generate private key: %v", err)
@@ -251,7 +251,7 @@ func generateCerts(url string) (string, string, error) {
 	if err := keyOut.Close(); err != nil {
 		return "", "", fmt.Errorf("error closing key.pem: %s", err)
 	}
-	if err := os.Chmod(keyOut.Name(), 0600); err != nil {
+	if err := os.Chmod(keyOut.Name(), 0o600); err != nil {
 		return "", "", fmt.Errorf("could not change permissions on key.pem: %v", err)
 	}
 	return certOut.Name(), keyOut.Name(), nil

pkg/jobutil/jobutil.go

@@ -24,8 +24,6 @@ import (
 	"strconv"
 	"strings"
 
-	"k8s.io/apimachinery/pkg/types"
-
 	"github.com/jenkins-x/go-scm/scm"
 	"github.com/jenkins-x/lighthouse/pkg/apis/lighthouse/v1alpha1"
 	"github.com/jenkins-x/lighthouse/pkg/config/job"
@@ -40,11 +38,6 @@ const (
 	maxGenerateNamePrefix = 32
 )
 
-// lighthouseClient a minimalistic lighthouse client required by the aborter
-type lighthouseClient interface {
-	Patch(name string, pt types.PatchType, data []byte, subresources ...string) (result *v1alpha1.LighthouseJob, err error)
-}
-
 // NewLighthouseJob initializes a LighthouseJob out of a LighthouseJobSpec.
 func NewLighthouseJob(spec v1alpha1.LighthouseJobSpec, extraLabels, extraAnnotations map[string]string) v1alpha1.LighthouseJob {
 	labels, annotations := LabelsAndAnnotationsForSpec(spec, extraLabels, extraAnnotations)

pkg/jobutil/jobutil_test.go

@@ -36,9 +36,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/equality"
 )
 
-var (
-	logger = logrus.WithField("client", "git")
-)
+var logger = logrus.WithField("client", "git")
 
 func TestPostsubmitSpec(t *testing.T) {
 	tests := []struct {
@@ -284,7 +282,7 @@ func TestBatchSpec(t *testing.T) {
 }
 
 func TestNewLighthouseJob(t *testing.T) {
-	var testCases = []struct {
+	testCases := []struct {
 		name                string
 		gitKind             string
 		spec                v1alpha1.LighthouseJobSpec
@@ -437,7 +435,8 @@ func TestNewLighthouseJob(t *testing.T) {
 			expectedAnnotations: map[string]string{
 				util.LighthouseJobAnnotation: "job",
 			},
-		}, {
+		},
+		{
 			name:    "job with name too long to fit in a label",
 			gitKind: "github",
 			spec: v1alpha1.LighthouseJobSpec{
@@ -516,7 +515,7 @@ func TestNewLighthouseJob(t *testing.T) {
 }
 
 func TestNewLighthouseJobWithAnnotations(t *testing.T) {
-	var testCases = []struct {
+	testCases := []struct {
 		name                string
 		spec                v1alpha1.LighthouseJobSpec
 		annotations         map[string]string

pkg/keeper/githubapp/github_app_ctrl.go

@@ -23,25 +23,23 @@ import (
 )
 
 type gitHubAppKeeperController struct {
-	controllers        []keeper.Controller
-	ownerTokenFinder   *util.OwnerTokensDir
-	gitServer          string
-	githubAppSecretDir string
-	configAgent        *config.Agent
-	botName            string
-	gitKind            string
-	maxRecordsPerPool  int
-	historyURI         string
-	statusURI          string
-	ns                 string
-	logger             *logrus.Entry
-	m                  sync.Mutex
+	controllers       []keeper.Controller
+	ownerTokenFinder  *util.OwnerTokensDir
+	gitServer         string
+	configAgent       *config.Agent
+	botName           string
+	gitKind           string
+	maxRecordsPerPool int
+	historyURI        string
+	statusURI         string
+	ns                string
+	logger            *logrus.Entry
+	m                 sync.Mutex
 }
 
 // NewGitHubAppKeeperController creates a GitHub App style controller which needs to process each github owner
 // using a separate git provider client due to the way GitHub App tokens work
 func NewGitHubAppKeeperController(githubAppSecretDir string, configAgent *config.Agent, botName string, gitKind string, maxRecordsPerPool int, historyURI string, statusURI string, ns string) (keeper.Controller, error) {
-
 	gitServer := util.GithubServer
 	return &gitHubAppKeeperController{
 		ownerTokenFinder:  util.NewOwnerTokensDir(gitServer, githubAppSecretDir),
@@ -55,7 +53,6 @@ func NewGitHubAppKeeperController(githubAppSecretDir string, configAgent *config
 		ns:                ns,
 		logger:            logrus.NewEntry(logrus.StandardLogger()),
 	}, nil
-
 }
 
 func (g *gitHubAppKeeperController) Sync() error {
@@ -188,7 +185,8 @@ func createKeeperGitHubAppScmClient(gitServer string, token string) (*scm.Client
 		Scheme:      "token",
 		Credentials: token,
 	}
-	tr := &transport.Custom{Base: auth,
+	tr := &transport.Custom{
+		Base: auth,
 		Before: func(r *http.Request) {
 			r.Header.Set("Accept", "application/vnd.github.machine-man-preview+json")
 		},

pkg/keeper/keeper.go

@@ -526,9 +526,7 @@ func (c *DefaultController) GetPools() []Pool {
 	c.m.Lock()
 	defer c.m.Unlock()
 	answer := []Pool{}
-	for _, p := range c.pools {
-		answer = append(answer, p)
-	}
+	answer = append(answer, c.pools...)
 	return answer
 }