Add golangci-lint gh action (#5045)

* Add golangci-lint gh action Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Increase timeout Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Remove wrongly commited file Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Add linter to dockerfile Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Add gh linter - fix issues in kubeops, asset-syncer and apprepo-ctrl (#5060) * Fix issues in kubeops Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Fix issues in assety-syncer Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Fix isses in apprepository-controller Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Fix linter issues (#5059) Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Add gh linter - fix issues in kubeappsapis (#5058) * Fix linter issues in test files Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Fix linter issues Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Fix issues Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Fix errata Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Fix lint issues and tests Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Comment out line as it is just being used by an in-progress unit test Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Fix test case Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
vmware-tanzu · Jul 15, 2022 · be7a37f · be7a37f
1 parent bb87b2c
commit be7a37f
Show file tree

Hide file tree

Showing 53 changed files with 482 additions and 289 deletions.
diff --git a/.github/workflows/golangci-lint.yaml b/.github/workflows/golangci-lint.yaml
@@ -0,0 +1,26 @@
+# Copyright 2022 VMware, Inc.
+# SPDX-License-Identifier: Apache-2.0
+
+name: golangci-lint
+
+on:
+  pull_request:
+
+permissions:
+  contents: read
+  pull-requests: read
+
+jobs:
+  golangci:
+    name: lint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/setup-go@v3
+        with:
+          go-version: 1.18
+      - uses: actions/checkout@v3
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v3
+        with:
+          version: latest
+          args: --timeout=10m
diff --git a/cmd/apprepository-controller/Dockerfile b/cmd/apprepository-controller/Dockerfile
@@ -11,7 +11,14 @@ COPY cmd cmd
 ARG VERSION
 
 ARG GOSEC_VERSION="2.12.0"
+ARG GOLANGCILINT_VERSION="1.46.2"
+
 RUN curl -sfL https://raw.githubusercontent.com/securego/gosec/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v$GOSEC_VERSION
+RUN go install github.com/golangci/golangci-lint/cmd/golangci-lint@v$GOLANGCILINT_VERSION
+
+# Run golangci-lint to detect issues
+RUN golangci-lint run --timeout=10m ./cmd/apprepository-controller/...
+RUN golangci-lint run --timeout=10m ./pkg/...
 
 # Run gosec to detect any security-related error at build time
 RUN gosec ./cmd/apprepository-controller/...

diff --git a/cmd/apprepository-controller/cmd/root_test.go b/cmd/apprepository-controller/cmd/root_test.go
@@ -16,9 +16,10 @@ import (
 
 func TestParseFlagsCorrect(t *testing.T) {
 	var tests = []struct {
-		name string
-		args []string
-		conf server.Config
+		name        string
+		args        []string
+		conf        server.Config
+		errExpected bool
 	}{
 		{
 			"no arguments returns default flag values",
@@ -45,6 +46,7 @@ func TestParseFlagsCorrect(t *testing.T) {
 				ParsedCustomAnnotations:  map[string]string{},
 				ParsedCustomLabels:       map[string]string{},
 			},
+			true,
 		},
 		{
 			"pullSecrets with spaces",
@@ -75,6 +77,7 @@ func TestParseFlagsCorrect(t *testing.T) {
 				ParsedCustomAnnotations:  map[string]string{},
 				ParsedCustomLabels:       map[string]string{},
 			},
+			true,
 		},
 		{
 			"all arguments are captured",
@@ -122,6 +125,7 @@ func TestParseFlagsCorrect(t *testing.T) {
 				ParsedCustomAnnotations:  map[string]string{"foo13": "bar13", "foo13x": "bar13x", "extra13": "extra13"},
 				ParsedCustomLabels:       map[string]string{"foo14": "bar14", "foo14x": "bar14x"},
 			},
+			true,
 		},
 	}
 
@@ -133,7 +137,10 @@ func TestParseFlagsCorrect(t *testing.T) {
 			cmd.SetErr(b)
 			setFlags(cmd)
 			cmd.SetArgs(tt.args)
-			cmd.Execute()
+			err := cmd.Execute()
+			if !tt.errExpected && err != nil {
+				t.Errorf("unexpected error: %v", err)
+			}
 			if got, want := serveOpts, tt.conf; !cmp.Equal(want, got) {
 				t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(want, got))
 			}

diff --git a/cmd/asset-syncer/Dockerfile b/cmd/asset-syncer/Dockerfile
@@ -11,7 +11,14 @@ COPY cmd cmd
 ARG VERSION
 
 ARG GOSEC_VERSION="2.12.0"
+ARG GOLANGCILINT_VERSION="1.46.2"
+
 RUN curl -sfL https://raw.githubusercontent.com/securego/gosec/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v$GOSEC_VERSION
+RUN go install github.com/golangci/golangci-lint/cmd/golangci-lint@v$GOLANGCILINT_VERSION
+
+# Run golangci-lint to detect issues
+RUN golangci-lint run --timeout=10m ./cmd/asset-syncer/...
+RUN golangci-lint run --timeout=10m ./pkg/...
 
 # Run gosec to detect any security-related error at build time
 RUN gosec ./cmd/asset-syncer/...

diff --git a/cmd/asset-syncer/cmd/root_test.go b/cmd/asset-syncer/cmd/root_test.go
@@ -13,9 +13,10 @@ import (
 
 func TestParseFlagsCorrect(t *testing.T) {
 	var tests = []struct {
-		name string
-		args []string
-		conf server.Config
+		name        string
+		args        []string
+		conf        server.Config
+		errExpected bool
 	}{
 		{
 			"all arguments are captured (invalidate command)",
@@ -46,6 +47,7 @@ func TestParseFlagsCorrect(t *testing.T) {
 				UserAgent:             "asset-syncer/devel (foo05)",
 				UserAgentComment:      "foo05",
 			},
+			true,
 		},
 		{
 			"all arguments are captured (sync command)",
@@ -77,6 +79,7 @@ func TestParseFlagsCorrect(t *testing.T) {
 				UserAgent:             "asset-syncer/devel (foo05)",
 				UserAgentComment:      "foo05",
 			},
+			true,
 		},
 		{
 			"all arguments are captured (delete command)",
@@ -107,6 +110,7 @@ func TestParseFlagsCorrect(t *testing.T) {
 				UserAgent:             "asset-syncer/devel (foo05)",
 				UserAgentComment:      "foo05",
 			},
+			true,
 		},
 	}
 
@@ -117,7 +121,10 @@ func TestParseFlagsCorrect(t *testing.T) {
 			cmd.SetOut(b)
 			cmd.SetErr(b)
 			cmd.SetArgs(tt.args)
-			cmd.Execute()
+			err := cmd.Execute()
+			if !tt.errExpected && err != nil {
+				t.Errorf("unexpected error: %v", err)
+			}
 			if got, want := serveOpts, tt.conf; !cmp.Equal(want, got) {
 				t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(want, got))
 			}

diff --git a/cmd/asset-syncer/server/postgresql_db_test.go b/cmd/asset-syncer/server/postgresql_db_test.go
@@ -405,15 +405,24 @@ func TestRepoAlreadyProcessed(t *testing.T) {
 	}
 
 	// not processed when repo exists but has not been processed
-	pam.EnsureRepoExists(repoNamespace, repoName)
+	_, err := pam.EnsureRepoExists(repoNamespace, repoName)
+	if err != nil {
+		t.Errorf("%+v", err)
+	}
 	got = pam.LastChecksum(repo)
 	if got != "" {
 		t.Errorf("got: %s, want: %s", got, "")
 	}
 
-	pam.UpdateLastCheck(repoNamespace, repoName, checksum, time.Now())
+	err = pam.UpdateLastCheck(repoNamespace, repoName, checksum, time.Now())
+	if err != nil {
+		t.Errorf("%+v", err)
+	}
 	// processed when checksums match
-	pam.EnsureRepoExists(repoNamespace, repoName)
+	_, err = pam.EnsureRepoExists(repoNamespace, repoName)
+	if err != nil {
+		t.Errorf("%+v", err)
+	}
 	got = pam.LastChecksum(repo)
 	if got != checksum {
 		t.Errorf("got: %s, want: %s", got, checksum)

diff --git a/cmd/asset-syncer/server/sync.go b/cmd/asset-syncer/server/sync.go
@@ -47,6 +47,9 @@ func Sync(serveOpts Config, version string, args []string) error {
 			return fmt.Errorf("Error: %v", err)
 		}
 		authorizationHeader, err = kube.GetAuthHeaderFromDockerConfig(dockerConfig)
+		if err != nil {
+			return fmt.Errorf("Error: %v", err)
+		}
 	}
 
 	filters, err := parseFilters(serveOpts.FilterRules)

diff --git a/cmd/asset-syncer/server/utils_test.go b/cmd/asset-syncer/server/utils_test.go
@@ -6,7 +6,6 @@ package server
 import (
 	"bytes"
 	"compress/gzip"
-	"encoding/base64"
 	"errors"
 	"fmt"
 	"image"
@@ -22,7 +21,6 @@ import (
 	"github.com/DATA-DOG/go-sqlmock"
 	"github.com/disintegration/imaging"
 	"github.com/google/go-cmp/cmp"
-	log "github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	apprepov1alpha1 "github.com/vmware-tanzu/kubeapps/cmd/apprepository-controller/pkg/apis/apprepository/v1alpha1"
 	"github.com/vmware-tanzu/kubeapps/pkg/chart/models"
@@ -33,6 +31,7 @@ import (
 	httpclient "github.com/vmware-tanzu/kubeapps/pkg/http-client"
 	tartest "github.com/vmware-tanzu/kubeapps/pkg/tarutil/test"
 	"helm.sh/helm/v3/pkg/chart"
+	log "k8s.io/klog/v2"
 )
 
 var validRepoIndexYAMLBytes, _ = ioutil.ReadFile("testdata/valid-index.yaml")
@@ -46,7 +45,10 @@ func (h *badHTTPClient) Do(req *http.Request) (*http.Response, error) {
 	w := httptest.NewRecorder()
 	w.WriteHeader(500)
 	if len(h.errMsg) > 0 {
-		w.Write([]byte(h.errMsg))
+		_, err := w.Write([]byte(h.errMsg))
+		if err != nil {
+			return nil, err
+		}
 	}
 	return w.Result(), nil
 }
@@ -72,7 +74,10 @@ func (h *goodHTTPClient) Do(req *http.Request) (*http.Response, error) {
 		w.WriteHeader(500)
 	}
 
-	w.Write([]byte(validRepoIndexYAML))
+	_, err := w.Write([]byte(validRepoIndexYAML))
+	if err != nil {
+		log.Fatalf("%+v", err)
+	}
 	return w.Result(), nil
 }
 
@@ -88,7 +93,10 @@ func (h *goodAuthenticatedHTTPClient) Do(req *http.Request) (*http.Response, err
 		// Ensure we're sending the right Authorization header
 		w.WriteHeader(403)
 	} else {
-		w.Write(iconBytes())
+		_, err := w.Write(iconBytes())
+		if err != nil {
+			log.Fatalf("%+v", err)
+		}
 	}
 	return w.Result(), nil
 }
@@ -102,15 +110,21 @@ func (h *authenticatedHTTPClient) Do(req *http.Request) (*http.Response, error)
 	if !strings.Contains(req.Header.Get("Authorization"), "Bearer ThisSecretAccessTokenAuthenticatesTheClient") {
 		w.WriteHeader(500)
 	}
-	w.Write([]byte(validRepoIndexYAML))
+	_, err := w.Write([]byte(validRepoIndexYAML))
+	if err != nil {
+		log.Fatalf("%+v", err)
+	}
 	return w.Result(), nil
 }
 
 type badIconClient struct{}
 
 func (h *badIconClient) Do(req *http.Request) (*http.Response, error) {
 	w := httptest.NewRecorder()
-	w.Write([]byte("not-an-image"))
+	_, err := w.Write([]byte("not-an-image"))
+	if err != nil {
+		log.Fatalf("%+v", err)
+	}
 	return w.Result(), nil
 }
 
@@ -119,25 +133,30 @@ type goodIconClient struct{}
 func iconBytes() []byte {
 	var b bytes.Buffer
 	img := imaging.New(1, 1, color.White)
-	imaging.Encode(&b, img, imaging.PNG)
+	err := imaging.Encode(&b, img, imaging.PNG)
+	if err != nil {
+		return nil
+	}
 	return b.Bytes()
 }
 
-func iconB64() string {
-	return base64.StdEncoding.EncodeToString(iconBytes())
-}
-
 func (h *goodIconClient) Do(req *http.Request) (*http.Response, error) {
 	w := httptest.NewRecorder()
-	w.Write(iconBytes())
+	_, err := w.Write(iconBytes())
+	if err != nil {
+		log.Fatalf("%+v", err)
+	}
 	return w.Result(), nil
 }
 
 type svgIconClient struct{}
 
 func (h *svgIconClient) Do(req *http.Request) (*http.Response, error) {
 	w := httptest.NewRecorder()
-	w.Write([]byte("<svg width='100' height='100'></svg>"))
+	_, err := w.Write([]byte("<svg width='100' height='100'></svg>"))
+	if err != nil {
+		log.Fatalf("%+v", err)
+	}
 	res := w.Result()
 	res.Header.Set("Content-Type", "image/svg")
 	return res, nil
@@ -282,7 +301,10 @@ func Test_fetchRepoIndexUserAgent(t *testing.T) {
 
 			server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
 				assert.Equal(t, req.Header.Get("User-Agent"), tt.expectedUserAgent, "expected user agent")
-				rw.Write([]byte(validRepoIndexYAML))
+				_, err := rw.Write([]byte(validRepoIndexYAML))
+				if err != nil {
+					log.Fatalf("%+v", err)
+				}
 			}))
 			// Close the server when test finishes
 			defer server.Close()
@@ -657,9 +679,15 @@ func (h *goodOCIAPIHTTPClient) Do(req *http.Request) (*http.Response, error) {
 	}
 
 	if r, ok := h.responseByPath[req.URL.Path]; ok {
-		w.Write([]byte(r))
+		_, err := w.Write([]byte(r))
+		if err != nil {
+			log.Fatalf("%+v", err)
+		}
 	} else {
-		w.Write([]byte(h.response))
+		_, err := w.Write([]byte(h.response))
+		if err != nil {
+			log.Fatalf("%+v", err)
+		}
 	}
 	return w.Result(), nil
 }
@@ -675,7 +703,10 @@ func (h *authenticatedOCIAPIHTTPClient) Do(req *http.Request) (*http.Response, e
 	if req.Header.Get("Authorization") != "Bearer ThisSecretAccessTokenAuthenticatesTheClient" {
 		w.WriteHeader(500)
 	}
-	w.Write([]byte(h.response))
+	_, err := w.Write([]byte(h.response))
+	if err != nil {
+		log.Fatalf("%+v", err)
+	}
 	return w.Result(), nil
 }
 
@@ -1011,7 +1042,6 @@ version: 1.0.0
 	}
 	for _, tt := range tests {
 		t.Run(tt.description, func(t *testing.T) {
-			log.SetLevel(log.DebugLevel)
 			w := map[string]*httptest.ResponseRecorder{}
 			content := map[string]*bytes.Buffer{}
 			for _, tag := range tt.tags {

diff --git a/cmd/kubeapps-apis/Dockerfile b/cmd/kubeapps-apis/Dockerfile
@@ -10,10 +10,12 @@ ARG VERSION="devel"
 
 ARG BUF_VERSION="1.5.0"
 ARG GOSEC_VERSION="2.12.0"
+ARG GOLANGCILINT_VERSION="1.46.2"
 
 # Install lint tools
 RUN curl -sSL "https://github.com/bufbuild/buf/releases/download/v$BUF_VERSION/buf-Linux-x86_64" -o "/tmp/buf" && chmod +x "/tmp/buf"
 RUN curl -sfL https://raw.githubusercontent.com/securego/gosec/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v$GOSEC_VERSION
+RUN go install github.com/golangci/golangci-lint/cmd/golangci-lint@v$GOLANGCILINT_VERSION
 
 
 # With the trick below, Go's build cache is kept between builds.
@@ -27,13 +29,17 @@ RUN --mount=type=cache,target=/go/pkg/mod  \
 COPY pkg pkg
 COPY cmd cmd
 
-# Lint the proto files to detect errors at build time
-RUN /tmp/buf lint ./cmd/kubeapps-apis
+# Run golangci-lint to detect issues
+RUN golangci-lint run --timeout=10m ./cmd/kubeapps-apis/...
+RUN golangci-lint run --timeout=10m ./pkg/...
 
 # Run gosec to detect any security-related error at build time
 RUN gosec ./cmd/kubeapps-apis/...
 RUN gosec ./pkg/...
 
+# Lint the proto files to detect errors at build time
+RUN /tmp/buf lint ./cmd/kubeapps-apis
+
 # Build the main grpc server
 RUN --mount=type=cache,target=/go/pkg/mod \
     --mount=type=cache,target=/root/.cache/go-build \