Merge pull request #14 from sapcc/feat/golangci-lint

add golangci action

.github/workflows/golangci-lint.yml

@@ -0,0 +1,29 @@
+name: golangci-lint
+on:
+  push:
+    branches:
+      - master
+  pull_request:
+
+env:
+  GO_VERSION: 1.21
+  GOLANGCI_LINT_VERSION: v1.60.2
+
+permissions:
+  contents: read
+  pull-requests: read
+  checks: write
+
+jobs:
+  golangci:
+    name: lint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-go@v5
+        with:
+          go-version: ${{ env.GO_VERSION }}
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v6
+        with:
+          version: ${{ env.GOLANGCI_LINT_VERSION }}

.golangci.yaml

@@ -0,0 +1,162 @@
+# SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Greenhouse contributors
+# SPDX-License-Identifier: Apache-2.0
+
+run:
+  timeout: 3m # 1m by default
+  modules-download-mode: readonly
+
+output:
+  # Do not print lines of code with issue.
+  print-issued-lines: false
+
+issues:
+  exclude:
+    # It is idiomatic Go to reuse the name 'err' with ':=' for subsequent errors.
+    # Ref: https://go.dev/doc/effective_go#redeclaration
+    - 'declaration of "err" shadows declaration at'
+  exclude-rules:
+    - path: _test\.go
+      linters:
+        - bodyclose
+        - dupl
+  # '0' disables the following options.
+  max-issues-per-linter: 0
+  max-same-issues: 0
+
+linters-settings:
+  dupl:
+    # Tokens count to trigger issue, 150 by default.
+    threshold: 100
+  errcheck:
+    # Report about assignment of errors to blank identifier.
+    check-blank: true
+    # Report about not checking of errors in type assertions.
+    check-type-assertions: true
+    exclude-functions:
+      - encoding/json.Marshal
+  forbidigo:
+    analyze-types: true # required for pkg:
+    forbid:
+      # ioutil package has been deprecated: https://github.com/golang/go/issues/42026
+      - ^ioutil\..*$
+      # Using http.DefaultServeMux is discouraged because it's a global variable that some packages silently and magically add handlers to (esp. net/http/pprof).
+      # Applications wishing to use http.ServeMux should obtain local instances through http.NewServeMux() instead of using the global default instance.
+      - ^http\.DefaultServeMux$
+      - ^http\.Handle(?:Func)?$
+      # Forbid usage of old and archived square/go-jose
+      - pkg: ^gopkg\.in/square/go-jose\.v2$
+        msg: "gopk.in/square/go-jose is arcived and has CVEs. Replace it with gopkg.in/go-jose/go-jose.v2"
+      - pkg: ^github.com/coreos/go-oidc$
+        msg: "github.com/coreos/go-oidc depends on gopkg.in/square/go-jose which has CVEs. Replace it with github.com/coreos/go-oidc/v3"
+
+      - pkg: ^github.com/howeyc/gopass$
+        msg: "github.com/howeyc/gopass is archived, use golang.org/x/term instead"
+  goconst:
+    ignore-tests: true
+    min-occurrences: 5
+  gocritic:
+    enabled-checks:
+      - boolExprSimplify
+      - builtinShadow
+      - emptyStringTest
+      - evalOrder
+      - httpNoBody
+      - importShadow
+      - initClause
+      - methodExprCall
+      - paramTypeCombine
+      - preferFilepathJoin
+      - ptrToRefParam
+      - redundantSprint
+      - returnAfterHttpError
+      - stringConcatSimplify
+      - timeExprSimplify
+      - truncateCmp
+      - typeAssertChain
+      - typeUnparen
+      - unnamedResult
+      - unnecessaryBlock
+      - unnecessaryDefer
+      - weakCond
+      - yodaStyleExpr
+  goimports:
+    # Put local imports after 3rd-party packages.
+    local-prefixes: github.com/sapcc/git-cert-shim
+  gosec:
+    excludes:
+      # gosec wants us to set a short ReadHeaderTimeout to avoid Slowloris attacks, but doing so would expose us to Keep-Alive race conditions (see https://iximiuz.com/en/posts/reverse-proxy-http-keep-alive-and-502s/)
+      - G112
+      # created file permissions are restricted by umask if necessary
+      - G306
+  govet:
+    enable-all: true
+    disable:
+      - fieldalignment
+  nolintlint:
+    require-specific: true
+  misspell:
+    ignore-words:
+      - metis
+  stylecheck:
+    dot-import-whitelist:
+      - github.com/onsi/ginkgo/v2
+      - github.com/onsi/gomega
+  usestdlibvars:
+    constant-kind: true
+    crypto-hash: true
+    default-rpc-path: true
+    http-method: true
+    http-status-code: true
+    sql-isolation-level: true
+    time-layout: true
+    time-month: true
+    time-weekday: true
+    tls-signature-scheme: true
+  whitespace:
+    # Enforce newlines (or comments) after multi-line function signatures.
+    multi-func: true
+
+linters:
+  # We use 'disable-all' and enable linters explicitly so that a newer version
+  # does not introduce new linters unexpectedly.
+  disable-all: true
+  enable:
+    - bodyclose
+    - containedctx
+    - copyloopvar
+    # - dupl
+    - dupword
+    - durationcheck
+    - errcheck
+    - errname
+    - errorlint
+    - forbidigo
+    - ginkgolinter
+    - gocheckcompilerdirectives
+    - goconst
+    - gocritic
+    - gofmt
+    - goimports
+    - gosec
+    - gosimple
+    - govet
+    - ineffassign
+    - intrange
+    - misspell
+    - nilerr
+    - noctx
+    - nolintlint
+    - nosprintfhostport
+    - perfsprint
+    - predeclared
+    - rowserrcheck
+    - sqlclosecheck
+    - staticcheck
+    - stylecheck
+    - tenv
+    - typecheck
+    - unconvert
+    - unparam
+    - unused
+    - usestdlibvars
+    - whitespace

Makefile

@@ -10,6 +10,16 @@ else
 GOBIN=$(shell go env GOBIN)
 endif
 
+## Location to install dependencies an GO binaries
+LOCALBIN ?= $(shell pwd)/bin
+$(LOCALBIN):
+	mkdir -p $(LOCALBIN)
+## Tool Binaries
+GOLINT ?= $(LOCALBIN)/golangci-lint
+## Tool Versions
+GOLINT_VERSION ?= v1.60.2
+GINKGOLINTER_VERSION ?= v0.16.2
+
 all: build
 
 # Run tests
@@ -97,3 +107,13 @@ CONTROLLER_GEN=$(GOBIN)/controller-gen
 else
 CONTROLLER_GEN=$(shell which controller-gen)
 endif
+
+.PHONY: lint
+lint: golint
+	$(GOLINT) run -v --timeout 5m
+
+.PHONY: golint
+golint: $(GOLINT)
+$(GOLINT): $(LOCALBIN)
+	GOBIN=$(LOCALBIN) go install github.com/golangci/golangci-lint/cmd/golangci-lint@$(GOLINT_VERSION)
+	GOBIN=$(LOCALBIN) go install github.com/nunnatsa/ginkgolinter/cmd/ginkgolinter@$(GINKGOLINTER_VERSION)

cmd/git-cert-shim/main.go

@@ -21,7 +21,7 @@ import (
 	"flag"
 	"fmt"
 	"net/http"
-	_ "net/http/pprof"
+	_ "net/http/pprof" //nolint:gosec
 	"os"
 	"time"
 

controllers/git_controller.go

@@ -18,6 +18,7 @@ package controllers
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"path/filepath"
 	"strings"
@@ -150,7 +151,7 @@ func (g *GitController) checkCertificate(cert *certificate.Certificate) error {
 	// If the certmanager.certificate is not ready, we abort here and check again later.
 	// Once it is ready, the secret contains the tls certificate and private key.
 	if !isCertificateReady(c) {
-		return fmt.Errorf("certificate not (yet) ready. re-adding to queue")
+		return errors.New("certificate not (yet) ready. re-adding to queue")
 	}
 
 	tlsSecret, err := k8sutils.GetSecret(ctx, g.client, g.ControllerOptions.Namespace, cert.GetSecretName())
@@ -182,20 +183,20 @@ func (g *GitController) checkCertificate(cert *certificate.Certificate) error {
 		g.mtx.Lock()
 		defer g.mtx.Unlock()
 
-		certFileName := filepath.Join(cert.OutFolder, fmt.Sprintf("%s.pem", cert.CommonName))
+		certFileName := filepath.Join(cert.OutFolder, cert.CommonName+".pem")
 		certFileName = strings.ReplaceAll(certFileName, "*", "wildcard")
 		if err := util.WriteToFileIfNotEmpty(certFileName, certByte); err != nil {
 			return err
 		}
 
-		keyFileName := filepath.Join(cert.OutFolder, fmt.Sprintf("%s-key.pem", cert.CommonName))
+		keyFileName := filepath.Join(cert.OutFolder, cert.CommonName+"-key.pem")
 		keyFileName = strings.ReplaceAll(keyFileName, "*", "wildcard")
 		if err := util.WriteToFileIfNotEmpty(keyFileName, keyByte); err != nil {
 			return err
 		}
 
 		err = g.repositorySyncer.AddFilesAndCommit(
-			fmt.Sprintf("added certificate for %s", cert.CommonName), certFileName, keyFileName,
+			"added certificate for "+cert.CommonName, certFileName, keyFileName,
 		)
 		if err != nil {
 			return err

pkg/certificate/certificate.go

@@ -25,7 +25,7 @@ func (c *Certificate) GetName() string {
 }
 
 func (c *Certificate) GetSecretName() string {
-	return fmt.Sprintf("tls-%s", c.GetName())
+	return "tls-" + c.GetName()
 }
 
 func ReadCertificateConfig(filePath string) ([]*Certificate, error) {

pkg/certificate/secret.go

@@ -13,20 +13,21 @@ const (
 	tlsKey = "tls.key"
 )
 
-func ExtractCAAndCertificateAndPrivateKeyFromSecret(tlsSecret *corev1.Secret) ([]byte, []byte, []byte, error) {
+func ExtractCAAndCertificateAndPrivateKeyFromSecret(tlsSecret *corev1.Secret) (ca, cert, key []byte, err error) {
+	//nolint:gosimple
 	if tlsSecret.Data == nil || len(tlsSecret.Data) == 0 {
 		return nil, nil, nil, errors.New("secret is empty")
 	}
 
 	// Optional.
-	ca, _ := tlsSecret.Data[caCrt]
+	ca = tlsSecret.Data[caCrt]
 
 	cert, ok := tlsSecret.Data[tlsCrt]
 	if !ok {
 		return nil, nil, nil, fmt.Errorf("%s missing in secret data", tlsCrt)
 	}
 
-	key, ok := tlsSecret.Data[tlsKey]
+	key, ok = tlsSecret.Data[tlsKey]
 	if !ok {
 		return nil, nil, nil, fmt.Errorf("%s missing in secret data", tlsKey)
 	}

pkg/git/exec.go

@@ -45,12 +45,12 @@ func newCommand(cmd string, defaultArgs ...string) (*command, error) {
 		timeout:     10 * time.Minute,
 		defaultArgs: defaultArgs,
 	}
-	return c, c.verify()
+	return c, c.verify() //nolint:gocritic
 }
 
 // Run starts the command, waits until it finished and returns stdOut or an error containing the stdError message.
 func (c *command) run(args ...string) (string, error) {
-	cmd := exec.Command(c.cmd, append(c.defaultArgs, args...)...)
+	cmd := exec.Command(c.cmd, append(c.defaultArgs, args...)...) //nolint:gosec
 
 	if v, ok := os.LookupEnv("DEBUG"); ok && v == "true" {
 		fmt.Println("running: ", cmd.String())
@@ -78,7 +78,7 @@ func (c *command) run(args ...string) (string, error) {
 			fmt.Println("failed to kill command: ", err.Error())
 			return strings.TrimSpace(stdOut.String()), err
 		}
-		return "", fmt.Errorf("command timed out after %s: %s\n", time.Since(start).String(), cmd.String())
+		return "", fmt.Errorf("command timed out after %s: %s", time.Since(start).String(), cmd.String())
 	case err := <-done:
 		if stdErr.Len() > 0 {
 			fmt.Println("Output:", strings.TrimSpace(stdErr.String()))

pkg/git/git.go

@@ -64,8 +64,11 @@ func (g *Git) GetRemoteHEADCommitHash() (string, error) {
 
 // PullRebase pulls and rebases.
 func (g *Git) PullRebase() error {
-	g.run("rebase", "--abort")
-	_, err := g.run(
+	_, err := g.run("rebase", "--abort")
+	if err != nil {
+		return errors.Wrap(err, "git rebase --abort failed")
+	}
+	_, err = g.run(
 		"-c", fmt.Sprintf(`user.name="%s"`, g.AuthorName),
 		"-c", fmt.Sprintf(`user.email="%s"`, g.AuthorEmail),
 		"pull",

pkg/git/options.go

@@ -12,7 +12,7 @@ import (
 
 const (
 	gitRemoteURLEnvVarkey      = "GIT_REMOTE_URL"
-	gitTokenEnvVarKey          = "GIT_API_TOKEN"
+	gitTokenEnvVarKey          = "GIT_API_TOKEN" //nolint:gosec
 	gitSSHPrivkeyFileEnvVarKey = "GIT_SSH_PRIVKEY_FILE"
 )
 

pkg/git/syncer.go

@@ -123,6 +123,7 @@ func (r *RepositorySyncer) syncWithRetry() error {
 	defer r.mtx.Unlock()
 
 	err := retry.OnError(retry.DefaultBackoff,
+		//nolint:gocritic
 		func(err error) bool {
 			// Retry the sync, if a git pull --rebase can help.
 			return isErrFailedToPushSomeRefs(err)

pkg/util/util.go

@@ -30,7 +30,7 @@ func EnsureDir(path string, isEnsureEmptyDir bool) error {
 	if isEnsureEmptyDir {
 		p := path
 		if !strings.HasSuffix(p, "/") {
-			p = p + "/"
+			p += "/"
 		}
 		if err := os.RemoveAll(p); os.IsNotExist(err) {
 			return err