Merge pull request #218 from mercedes-benz/introduce_golangci_config

Introduce golangci config and `make help`

.github/workflows/go-tests.yml

@@ -30,10 +30,8 @@ jobs:
         with:
           go-version: 'stable'
       - uses: actions/checkout@v3
-      - uses: golangci/golangci-lint-action@v3
-        with:
-          skip-cache: true
-          args: --timeout=8m --build-tags testing
+      - name: make lint
+        run: make golangci-lint && GOLANGCI_LINT_EXTRA_ARGS="--timeout=8m --build-tags testing" make lint
       - name: Verify go vendor, go modules and gofmt
         run: |
           sudo apt-get install -y jq

.golangci.yml

@@ -0,0 +1,40 @@
+# SPDX-License-Identifier: MIT
+linters:
+  disable-all: true
+  fast: false
+  enable:
+    - gci
+    - goconst
+    - gocritic
+    - gocyclo
+    - gofmt
+    - gofumpt
+    - goimports
+    - godox
+    - govet
+    - gosec
+    - gosimple
+    - importas
+    - ineffassign
+    - loggercheck
+    - misspell
+    - nakedret
+    - nilerr
+    - predeclared
+    - promlinter
+    - revive
+    - staticcheck
+    - unconvert
+    - unused
+    - wastedassign
+    - whitespace
+
+linters-settings:
+  gci:
+    sections:
+      - standard
+      - default
+      - prefix(github.com/cloudbase/garm)
+
+  goimports:
+    local-prefixes: github.com/cloudbase/garm

Makefile

@@ -10,47 +10,49 @@ VERSION ?= $(shell git describe --tags --match='v[0-9]*' --dirty --always)
 GARM_REF ?= $(shell git rev-parse --abbrev-ref HEAD)
 GO ?= go
 
+.PHONY: help
+help: ## Display this help.
+	@awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n  make \033[36m<target>\033[0m\n"} /^[a-zA-Z_0-9-]+:.*?##/ { printf "  \033[36m%-20s\033[0m %s\n", $$1, $$2 } /^##@/ { printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST)
+
 
 default: build
 
+##@ Build
+
 .PHONY : build-static test install-lint-deps lint go-test fmt fmtcheck verify-vendor verify create-release-files release
-build-static:
+build-static: ## Build garm statically
 	@echo Building garm
 	docker build --tag $(IMAGE_TAG) -f Dockerfile.build-static .
 	docker run --rm -e USER_ID=$(USER_ID) -e GARM_REF=$(GARM_REF) -e USER_GROUP=$(USER_GROUP) -v $(PWD)/build:/build/output:z $(IMAGE_TAG) /build-static.sh
 	@echo Binaries are available in $(PWD)/build
 
-create-release-files:
-	./scripts/make-release.sh
-
-release: build-static create-release-files
-
-clean:
+clean: ## Clean up build artifacts
 	@rm -rf ./bin ./build ./release
 
-build:
+.PHONY: build
+build: ## Build garm
 	@echo Building garm ${VERSION}
 	$(shell mkdir -p ./bin)
 	@$(GO) build -ldflags "-s -w -X main.Version=${VERSION}" -tags osusergo,netgo,sqlite_omit_load_extension -o bin/garm ./cmd/garm
 	@$(GO) build -ldflags "-s -w -X github.com/cloudbase/garm/cmd/garm-cli/cmd.Version=${VERSION}" -tags osusergo,netgo,sqlite_omit_load_extension -o bin/garm-cli ./cmd/garm-cli
 	@echo Binaries are available in $(PWD)/bin
 
-test: verify go-test
+test: verify go-test ## Run tests
 
-install-lint-deps:
-	@$(GO) install github.com/golangci/golangci-lint/cmd/golangci-lint@latest
+##@ Release
+create-release-files:
+	./scripts/make-release.sh
 
-lint:
-	@golangci-lint run --timeout=8m --build-tags testing
+release: build-static create-release-files ## Create a release
 
-go-test:
-	@$(GO) test -race -mod=vendor -tags testing -v $(TEST_ARGS) -timeout=15m -parallel=4 -count=1 ./...
-
-fmt:
-	@$(GO) fmt $$(go list ./...)
+##@ Lint / Verify
+.PHONY: lint
+lint: golangci-lint $(GOLANGCI_LINT) ## Run linting.
+	$(GOLANGCI_LINT) run -v --build-tags testing $(GOLANGCI_LINT_EXTRA_ARGS)
 
-fmtcheck:
-	@gofmt -l -s $$(go list ./... | sed 's|github.com/cloudbase/garm/||g') | grep ".*\.go"; if [ "$$?" -eq 0 ]; then echo "gofmt check failed; please run gofmt -w -s"; exit 1;fi
+.PHONY: lint-fix
+lint-fix: golangci-lint $(GOLANGCI_LINT) ## Lint the codebase and run auto-fixers if supported by the linte
+	GOLANGCI_LINT_EXTRA_ARGS=--fix $(MAKE) lint
 
 verify-vendor: ## verify if all the go.mod/go.sum files are up-to-date
 	$(eval TMPDIR := $(shell mktemp -d))
@@ -59,4 +61,32 @@ verify-vendor: ## verify if all the go.mod/go.sum files are up-to-date
 	@diff -r -u -q ${ROOTDIR} ${TMPDIR}/garm >/dev/null 2>&1; if [ "$$?" -ne 0 ];then echo "please run: go mod tidy && go mod vendor"; exit 1; fi
 	@rm -rf ${TMPDIR}
 
-verify: verify-vendor lint fmtcheck
+verify: verify-vendor lint fmtcheck ## Run all verify-* targets
+
+##@ Development
+
+go-test: ## Run tests
+	@$(GO) test -race -mod=vendor -tags testing -v $(TEST_ARGS) -timeout=15m -parallel=4 -count=1 ./...
+
+fmt: ## Run go fmt against code.
+	@$(GO) fmt $$(go list ./...)
+
+
+##@ Build Dependencies
+
+## Location to install dependencies to
+LOCALBIN ?= $(shell pwd)/bin
+$(LOCALBIN):
+	mkdir -p $(LOCALBIN)
+
+## Tool Binaries
+GOLANGCI_LINT ?= $(LOCALBIN)/golangci-lint
+
+## Tool Versions
+GOLANGCI_LINT_VERSION ?= v1.56.2
+
+.PHONY: golangci-lint
+golangci-lint: $(GOLANGCI_LINT) ## Download golangci-lint locally if necessary. If wrong version is installed, it will be overwritten.
+$(GOLANGCI_LINT): $(LOCALBIN)
+	test -s $(LOCALBIN)/golangci-lint && $(LOCALBIN)/golangci-lint --version | grep -q $(GOLANGCI_LINT_VERSION) || \
+	GOBIN=$(LOCALBIN) go install github.com/golangci/golangci-lint/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION)

apiserver/controllers/controllers.go

@@ -22,18 +22,18 @@ import (
 	"net/http"
 	"strings"
 
+	"github.com/gorilla/mux"
+	"github.com/gorilla/websocket"
+	"github.com/pkg/errors"
+
 	gErrors "github.com/cloudbase/garm-provider-common/errors"
 	"github.com/cloudbase/garm-provider-common/util"
 	"github.com/cloudbase/garm/apiserver/params"
 	"github.com/cloudbase/garm/auth"
 	"github.com/cloudbase/garm/metrics"
 	runnerParams "github.com/cloudbase/garm/params"
-	"github.com/cloudbase/garm/runner"
+	"github.com/cloudbase/garm/runner" //nolint:typecheck
 	wsWriter "github.com/cloudbase/garm/websocket"
-
-	"github.com/gorilla/mux"
-	"github.com/gorilla/websocket"
-	"github.com/pkg/errors"
 )
 
 func NewAPIController(r *runner.Runner, authenticator *auth.Authenticator, hub *wsWriter.Hub) (*APIController, error) {
@@ -107,19 +107,21 @@ func (a *APIController) handleWorkflowJobEvent(ctx context.Context, w http.Respo
 	hookType := r.Header.Get("X-Github-Hook-Installation-Target-Type")
 
 	if err := a.r.DispatchWorkflowJob(hookType, signature, body); err != nil {
-		if errors.Is(err, gErrors.ErrNotFound) {
+		switch {
+		case errors.Is(err, gErrors.ErrNotFound):
 			metrics.WebhooksReceived.WithLabelValues(
 				"false",         // label: valid
 				"owner_unknown", // label: reason
 			).Inc()
 			slog.With(slog.Any("error", err)).ErrorContext(ctx, "got not found error from DispatchWorkflowJob. webhook not meant for us?")
 			return
-		} else if strings.Contains(err.Error(), "signature") { // TODO: check error type
+		case strings.Contains(err.Error(), "signature"):
+			// nolint:golangci-lint,godox TODO: check error type
 			metrics.WebhooksReceived.WithLabelValues(
 				"false",             // label: valid
 				"signature_invalid", // label: reason
 			).Inc()
-		} else {
+		default:
 			metrics.WebhooksReceived.WithLabelValues(
 				"false",   // label: valid
 				"unknown", // label: reason
@@ -182,6 +184,7 @@ func (a *APIController) WSHandler(writer http.ResponseWriter, req *http.Request)
 		return
 	}
 
+	// nolint:golangci-lint,godox
 	// TODO (gsamfira): Handle ExpiresAt. Right now, if a client uses
 	// a valid token to authenticate, and keeps the websocket connection
 	// open, it will allow that client to stream logs via websockets

apiserver/controllers/enterprises.go

@@ -19,11 +19,11 @@ import (
 	"log/slog"
 	"net/http"
 
+	"github.com/gorilla/mux"
+
 	gErrors "github.com/cloudbase/garm-provider-common/errors"
 	"github.com/cloudbase/garm/apiserver/params"
 	runnerParams "github.com/cloudbase/garm/params"
-
-	"github.com/gorilla/mux"
 )
 
 // swagger:route POST /enterprises enterprises CreateEnterprise
@@ -165,7 +165,6 @@ func (a *APIController) DeleteEnterpriseHandler(w http.ResponseWriter, r *http.R
 
 	w.Header().Set("Content-Type", "application/json")
 	w.WriteHeader(http.StatusOK)
-
 }
 
 // swagger:route PUT /enterprises/{enterpriseID} enterprises UpdateEnterprise
@@ -318,7 +317,6 @@ func (a *APIController) ListEnterprisePoolsHandler(w http.ResponseWriter, r *htt
 	if err := json.NewEncoder(w).Encode(pools); err != nil {
 		slog.With(slog.Any("error", err)).ErrorContext(ctx, "failed to encode response")
 	}
-
 }
 
 // swagger:route GET /enterprises/{enterpriseID}/pools/{poolID} enterprises pools GetEnterprisePool
@@ -414,7 +412,6 @@ func (a *APIController) DeleteEnterprisePoolHandler(w http.ResponseWriter, r *ht
 
 	w.Header().Set("Content-Type", "application/json")
 	w.WriteHeader(http.StatusOK)
-
 }
 
 // swagger:route PUT /enterprises/{enterpriseID}/pools/{poolID} enterprises pools UpdateEnterprisePool

apiserver/controllers/instances.go

@@ -20,11 +20,11 @@ import (
 	"net/http"
 	"strconv"
 
+	"github.com/gorilla/mux"
+
 	gErrors "github.com/cloudbase/garm-provider-common/errors"
 	"github.com/cloudbase/garm/apiserver/params"
 	runnerParams "github.com/cloudbase/garm/params"
-
-	"github.com/gorilla/mux"
 )
 
 // swagger:route GET /pools/{poolID}/instances instances ListPoolInstances

apiserver/controllers/metadata.go

@@ -20,8 +20,9 @@ import (
 	"log/slog"
 	"net/http"
 
-	"github.com/cloudbase/garm/apiserver/params"
 	"github.com/gorilla/mux"
+
+	"github.com/cloudbase/garm/apiserver/params"
 )
 
 func (a *APIController) InstanceGithubRegistrationTokenHandler(w http.ResponseWriter, r *http.Request) {

apiserver/controllers/organizations.go

@@ -20,11 +20,11 @@ import (
 	"net/http"
 	"strconv"
 
+	"github.com/gorilla/mux"
+
 	gErrors "github.com/cloudbase/garm-provider-common/errors"
 	"github.com/cloudbase/garm/apiserver/params"
 	runnerParams "github.com/cloudbase/garm/params"
-
-	"github.com/gorilla/mux"
 )
 
 // swagger:route POST /organizations organizations CreateOrg
@@ -174,7 +174,6 @@ func (a *APIController) DeleteOrgHandler(w http.ResponseWriter, r *http.Request)
 
 	w.Header().Set("Content-Type", "application/json")
 	w.WriteHeader(http.StatusOK)
-
 }
 
 // swagger:route PUT /organizations/{orgID} organizations UpdateOrg
@@ -423,7 +422,6 @@ func (a *APIController) DeleteOrgPoolHandler(w http.ResponseWriter, r *http.Requ
 
 	w.Header().Set("Content-Type", "application/json")
 	w.WriteHeader(http.StatusOK)
-
 }
 
 // swagger:route PUT /organizations/{orgID}/pools/{poolID} organizations pools UpdateOrgPool

apiserver/controllers/pools.go

@@ -19,11 +19,11 @@ import (
 	"log/slog"
 	"net/http"
 
+	"github.com/gorilla/mux"
+
 	gErrors "github.com/cloudbase/garm-provider-common/errors"
 	"github.com/cloudbase/garm/apiserver/params"
 	runnerParams "github.com/cloudbase/garm/params"
-
-	"github.com/gorilla/mux"
 )
 
 // swagger:route GET /pools pools ListPools
@@ -37,7 +37,6 @@ func (a *APIController) ListAllPoolsHandler(w http.ResponseWriter, r *http.Reque
 	ctx := r.Context()
 
 	pools, err := a.r.ListAllPools(ctx)
-
 	if err != nil {
 		slog.With(slog.Any("error", err)).ErrorContext(ctx, "listing pools")
 		handleError(ctx, w, err)

apiserver/controllers/repositories.go

@@ -20,11 +20,11 @@ import (
 	"net/http"
 	"strconv"
 
+	"github.com/gorilla/mux"
+
 	gErrors "github.com/cloudbase/garm-provider-common/errors"
 	"github.com/cloudbase/garm/apiserver/params"
 	runnerParams "github.com/cloudbase/garm/params"
-
-	"github.com/gorilla/mux"
 )
 
 // swagger:route POST /repositories repositories CreateRepo
@@ -173,7 +173,6 @@ func (a *APIController) DeleteRepoHandler(w http.ResponseWriter, r *http.Request
 
 	w.Header().Set("Content-Type", "application/json")
 	w.WriteHeader(http.StatusOK)
-
 }
 
 // swagger:route PUT /repositories/{repoID} repositories UpdateRepo
@@ -422,7 +421,6 @@ func (a *APIController) DeleteRepoPoolHandler(w http.ResponseWriter, r *http.Req
 
 	w.Header().Set("Content-Type", "application/json")
 	w.WriteHeader(http.StatusOK)
-
 }
 
 // swagger:route PUT /repositories/{repoID}/pools/{poolID} repositories pools UpdateRepoPool

apiserver/routers/routers.go

@@ -49,7 +49,7 @@ import (
 	_ "expvar" // Register the expvar handlers
 	"log/slog"
 	"net/http"
-	_ "net/http/pprof" // Register the pprof handlers
+	_ "net/http/pprof" //nolint:golangci-lint,gosec // Register the pprof handlers
 
 	"github.com/felixge/httpsnoop"
 	"github.com/gorilla/mux"
@@ -87,7 +87,6 @@ func requestLogger(h http.Handler) http.Handler {
 		// gathers metrics from the upstream handlers
 		metrics := httpsnoop.CaptureMetrics(h, w, r)
 
-		//prints log and metrics
 		slog.Info(
 			"access_log",
 			slog.String("method", r.Method),