Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce golangci config and make help #218

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
9ea2a31
feat: introduce golangci config and help in make
bavarianbidi Feb 22, 2024
023652d
fix: make build as PHONY target
bavarianbidi Feb 22, 2024
b3854ea
fix: whitespace linter warnings
bavarianbidi Feb 22, 2024
8fc001f
fix: misspell linter warnings
bavarianbidi Feb 22, 2024
bd0b27a
fix: gci section warnings
bavarianbidi Feb 22, 2024
3b9f8b5
fix: var-naming linter findings
bavarianbidi Feb 22, 2024
0ab86a7
fix: unused-parameter linter findings
bavarianbidi Feb 22, 2024
e5ed45c
fix: unnecessary conversion linter findings
bavarianbidi Feb 22, 2024
e664639
fix: var-declaration linter findings
bavarianbidi Feb 22, 2024
6065fb2
fix: increment-decrement linter findings
bavarianbidi Feb 22, 2024
f640445
fix: indent-error-flow linter findings
bavarianbidi Feb 22, 2024
acc17ea
fix: receiver-naming linter findings
bavarianbidi Feb 22, 2024
b0e3f78
fix: godoc linter warnings (TODOs)
bavarianbidi Feb 22, 2024
9f405e0
fix: ifElseChain linter findings
bavarianbidi Feb 22, 2024
09e25ca
fix: gosec linter finding - memory aliasing
bavarianbidi Feb 22, 2024
55fe81f
fix: gosec linter findings
bavarianbidi Feb 22, 2024
c89deae
fix: ifElseChain linter finding
bavarianbidi Feb 22, 2024
3fd09f6
fix: assignOp linter finding
bavarianbidi Feb 22, 2024
cbe8f09
fix: run make lint-fix
bavarianbidi Feb 22, 2024
f9e41f1
fix: gosec linter finding
bavarianbidi Feb 22, 2024
f4e5149
fix: var-naming linter findings
bavarianbidi Feb 22, 2024
b27f523
fix: ifElseChain linter finding
bavarianbidi Feb 22, 2024
742ebab
fix: unneded type conversion
bavarianbidi Feb 22, 2024
c137f9b
fix: gocritic linter findings
bavarianbidi Feb 22, 2024
b8a9b6c
fix: ignore testing package typechecks
bavarianbidi Feb 22, 2024
73eb214
fix: run make lint same way as in ci
bavarianbidi Feb 22, 2024
9f5c38e
fix: unused-parameter linter findings
bavarianbidi Feb 22, 2024
60dbf97
fix: ignore testing secret
bavarianbidi Feb 22, 2024
4409beb
fix: G601: Implicit memory aliasing in for loop
bavarianbidi Feb 22, 2024
7221812
fix: remove unused cobra args
bavarianbidi Feb 22, 2024
ee3a670
fix: var-naming linter findings
bavarianbidi Feb 22, 2024
fd0550e
fix: godoc linter findings (TODO comments)
bavarianbidi Feb 22, 2024
9e7ac60
fix: gosec linter finding
bavarianbidi Feb 22, 2024
4c81c16
fix: goconst linter findings
bavarianbidi Feb 22, 2024
d332e7a
fix: gocritic linter finding
bavarianbidi Feb 22, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions .github/workflows/go-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
40 changes: 40 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -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
72 changes: 51 additions & 21 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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)
19 changes: 11 additions & 8 deletions apiserver/controllers/controllers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
7 changes: 2 additions & 5 deletions apiserver/controllers/enterprises.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions apiserver/controllers/instances.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion apiserver/controllers/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 2 additions & 4 deletions apiserver/controllers/organizations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions apiserver/controllers/pools.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
6 changes: 2 additions & 4 deletions apiserver/controllers/repositories.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions apiserver/routers/routers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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),
Expand Down
Loading
Loading