Run golangci-lint as part of pull request workflow #604

- Merge build and linter jobs in a single file
- Runs tests for go 1.13 and 1.14
- Introduces code coverage with coveralls
- Fixes some issues reported by golangci-lint and relax/disable some
rules until the codebase is refactored

Resolves #604

Author: Bruno Oliveira da Silva

.github/workflows/build.yml

@@ -0,0 +1,53 @@
+name: Build & Lint
+
+# Only trigger the event on pull-requests
+on: [pull_request]
+
+jobs:
+  build:
+    name: Build
+    runs-on: ubuntu-latest
+    # Test the latest release of Go
+    strategy:
+      matrix:
+        go: [ '1.14', '1.13' ]
+    steps:
+      # Setup the workflow to use the specific version of Go
+      - name: Set up Go 1.x
+        uses: actions/setup-go@v2
+        with:
+          go-version: ${{ matrix.go }}
+        id: go
+      # Checkout the repository
+      - name: Checkout
+        uses: actions/checkout@v2
+      # Verify downloaded dependencies
+      - name: Verify dependencies
+        run: go mod verify
+      # Run tests and generates a coverage profile
+      - name: Test
+        run: |
+          go test -v -coverprofile=profile.cov ./...
+      # Run Go benchmarks
+      - name: Benchmark
+        run: |
+          go test -bench=. -benchmem
+      # Sends code coverage report to Coveralls
+      - name: Coveralls
+        env:
+          COVERALLS_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        run: |
+          GO111MODULE=off go get github.com/mattn/goveralls
+          $(go env GOPATH)/bin/goveralls -coverprofile=profile.cov -service=github
+  # Run the linter as a separate job
+  golangci:
+    name: Lint
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v2
+      - name: golangci-lint
+        uses: golangci/[email protected]
+        with:
+          version: v1.26
+          github-token: "${{ secrets.GITHUB_TOKEN }}"

.golangci.yml

@@ -4,15 +4,19 @@ linters-settings:
   golint:
     min-confidence: 0
   gocyclo:
-    min-complexity: 60
+    min-complexity: 64
   maligned:
     suggest-new: true
   dupl:
     threshold: 100
   goconst:
     min-len: 2
     min-occurrences: 2
-
+  # TODO: Revisit after the refactor
+  funlen:
+    lines: 220
+    statements: 110
+  
 linters:
   enable-all: true
   disable:
@@ -21,3 +25,13 @@ linters:
     - lll
     - gochecknoinits
     - gochecknoglobals
+    - dupl
+    - wsl
+    - godox
+    - gomnd
+    # TODO: Revisit after the refactor
+    - gocognit
+    - testpackage
+    - goerr113
+    - godot
+    - nestif

forwarding.go

@@ -140,7 +140,6 @@ func (r *oauthProxy) forwardProxyHandler() func(*http.Request, *http.Response) {
 					zap.String("subject", state.identity.ID),
 					zap.String("email", state.identity.Email),
 					zap.String("expires", state.expiration.Format(time.RFC3339)))
-
 			} else {
 				r.log.Info("access token is about to expiry",
 					zap.String("subject", state.identity.ID),
@@ -182,7 +181,6 @@ func (r *oauthProxy) forwardProxyHandler() func(*http.Request, *http.Response) {
 						zap.String("subject", state.identity.ID),
 						zap.String("email", state.identity.Email),
 						zap.String("expires", state.expiration.Format(time.RFC3339)))
-
 				} else {
 					r.log.Info("session does not support refresh token, acquiring new token",
 						zap.String("subject", state.identity.ID),

self_signed.go

@@ -21,7 +21,7 @@ import (
 	"crypto/rand"
 	"crypto/rsa"
 	"crypto/tls"
-	"errors"
+	"fmt"
 	"sync"
 	"time"
 
@@ -47,10 +47,10 @@ type selfSignedCertificate struct {
 // newSelfSignedCertificate creates and returns a self signed certificate manager
 func newSelfSignedCertificate(hostnames []string, expiry time.Duration, log *zap.Logger) (*selfSignedCertificate, error) {
 	if len(hostnames) == 0 {
-		return nil, errors.New("no hostnames specified")
+		return nil, fmt.Errorf("no hostnames specified")
 	}
 	if expiry < 5*time.Minute {
-		return nil, errors.New("expiration must be greater then 5 minutes")
+		return nil, fmt.Errorf("expiration must be greater then 5 minutes")
 	}
 
 	// @step: generate a certificate pair

server.go

@@ -512,7 +512,6 @@ func (r *oauthProxy) createHTTPListener(config listenerConfig) (net.Listener, er
 				return nil, err
 			}
 			getCertificate = rotate.GetCertificate
-
 		}
 
 		if config.useFileTLS {

utils.go

@@ -18,7 +18,6 @@ package main
 import (
 	"crypto/aes"
 	"crypto/cipher"
-	"crypto/rand"
 	cryptorand "crypto/rand"
 	"crypto/rsa"
 	sha "crypto/sha256"
@@ -70,7 +69,7 @@ var (
 // createCertificate is responsible for creating a certificate
 func createCertificate(key *rsa.PrivateKey, hostnames []string, expire time.Duration) (tls.Certificate, error) {
 	// @step: create a serial for the certificate
-	serial, err := cryptorand.Int(rand.Reader, new(big.Int).Lsh(big.NewInt(1), 128))
+	serial, err := cryptorand.Int(cryptorand.Reader, new(big.Int).Lsh(big.NewInt(1), 128))
 	if err != nil {
 		return tls.Certificate{}, err
 	}

utils_test.go

@@ -276,7 +276,6 @@ func TestDecryptDataBlock(t *testing.T) {
 			t.Errorf("test case: %d are not the same", i)
 		}
 	}
-
 }
 
 func TestHasAccessOK(t *testing.T) {
@@ -586,7 +585,7 @@ func writeFakeConfigFile(t *testing.T, content string) *os.File {
 	}
 	f.Close()
 
-	if err := ioutil.WriteFile(f.Name(), []byte(content), 0700); err != nil {
+	if err := ioutil.WriteFile(f.Name(), []byte(content), 0600); err != nil {
 		t.Fatalf("unexpected error writing node label file: %v", err)
 	}