From 53d2e57ded2a400bde99e60b1cb33b45e54461c0 Mon Sep 17 00:00:00 2001 From: Nathan Oyler Date: Fri, 4 Oct 2024 12:38:05 -0700 Subject: [PATCH 1/9] upgrade go makefile maker, run --- .dockerignore | 30 +++++++++++++------------ .github/renovate.json | 19 ++++++++-------- .github/workflows/checks.yaml | 13 ++++++++++- .github/workflows/ci.yaml | 19 ++++++++-------- .github/workflows/codeql.yaml | 41 +++++++++++++++++++++++++++++++++++ .golangci.yaml | 31 ++++++++++++++++++++------ .license-scan-overrides.jsonl | 3 +++ Dockerfile | 4 ++-- Makefile | 13 +++++++---- go.mod | 2 +- 10 files changed, 127 insertions(+), 48 deletions(-) create mode 100644 .github/workflows/codeql.yaml diff --git a/.dockerignore b/.dockerignore index 4dbfd2df..0656a225 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,18 +1,20 @@ -.dockerignore +/.dockerignore +.DS_Store # TODO: uncomment when applications no longer use git to get version information #.git/ -.github/ -.gitignore -.goreleaser.yml +/.github/ +/.gitignore +/.goreleaser.yml /*.env* -.golangci.yaml -build/ -CONTRIBUTING.md -Dockerfile -docs/ -LICENSE* -Makefile.maker.yaml -README.md -report.html -shell.nix +/.golangci.yaml +/.vscode/ +/build/ +/CONTRIBUTING.md +/Dockerfile +/docs/ +/LICENSE* +/Makefile.maker.yaml +/README.md +/report.html +/shell.nix /testing/ diff --git a/.github/renovate.json b/.github/renovate.json index 6cd10d2b..4dfc8cfd 100644 --- a/.github/renovate.json +++ b/.github/renovate.json @@ -1,6 +1,7 @@ { + "$schema": "https://docs.renovatebot.com/renovate-schema.json", "extends": [ - "config:base", + "config:recommended", "default:pinDigestsDisabled", "mergeConfidence:all-badges", "docker:disable" @@ -10,7 +11,7 @@ ], "commitMessageAction": "Renovate: Update", "constraints": { - "go": "1.22" + "go": "1.23" }, "dependencyDashboardOSVVulnerabilitySummary": "all", "osvVulnerabilityAlerts": true, @@ -23,21 +24,19 @@ "matchPackageNames": [ "golang" ], - "allowedVersions": "1.22.x" + "allowedVersions": "1.23.x" }, { - "matchPackagePatterns": [ - "^github\\.com\\/sapcc\\/.*" + "matchPackageNames": [ + "/^github\\.com\\/sapcc\\/.*/" ], "automerge": true, "groupName": "github.com/sapcc" }, { - "excludePackagePatterns": [ - "^github\\.com\\/sapcc\\/.*" - ], - "matchPackagePatterns": [ - ".*" + "matchPackageNames": [ + "!/^github\\.com\\/sapcc\\/.*/", + "/.*/" ], "groupName": "External dependencies" } diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index 23a71257..c036768c 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -11,6 +11,7 @@ name: Checks pull_request: branches: - '*' + workflow_dispatch: {} permissions: checks: write contents: read @@ -25,7 +26,15 @@ jobs: uses: actions/setup-go@v5 with: check-latest: true - go-version: "1.22" + go-version: 1.23.2 + - name: Run golangci-lint + uses: golangci/golangci-lint-action@v6 + with: + version: latest + - name: Dependency Licenses Review + run: make check-dependency-licenses + - name: Run govulncheck + uses: golang/govulncheck-action@v1 - name: Check for spelling errors uses: reviewdog/action-misspell@v1 with: @@ -34,3 +43,5 @@ jobs: github_token: ${{ secrets.GITHUB_TOKEN }} ignore: importas reporter: github-check + - name: Check if source code files have license header + run: make check-license-headers diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index bd286a62..88a75763 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -8,14 +8,19 @@ name: CI push: branches: - master + paths-ignore: + - '**.md' pull_request: branches: - '*' + paths-ignore: + - '**.md' + workflow_dispatch: {} permissions: contents: read jobs: - buildAndLint: - name: Build & Lint + build: + name: Build runs-on: ubuntu-latest steps: - name: Check out code @@ -24,17 +29,13 @@ jobs: uses: actions/setup-go@v5 with: check-latest: true - go-version: "1.22" + go-version: 1.23.2 - name: Build all binaries run: make build-all - - name: Run golangci-lint - uses: golangci/golangci-lint-action@v4 - with: - version: latest test: name: Test needs: - - buildAndLint + - build runs-on: ubuntu-latest steps: - name: Check out code @@ -43,7 +44,7 @@ jobs: uses: actions/setup-go@v5 with: check-latest: true - go-version: "1.22" + go-version: 1.23.2 - name: Run tests and generate coverage report run: make build/cover.out - name: Upload coverage report to Coveralls diff --git a/.github/workflows/codeql.yaml b/.github/workflows/codeql.yaml new file mode 100644 index 00000000..ffe7a865 --- /dev/null +++ b/.github/workflows/codeql.yaml @@ -0,0 +1,41 @@ +################################################################################ +# This file is AUTOGENERATED with # +# Edit Makefile.maker.yaml instead. # +################################################################################ + +name: CodeQL +"on": + push: + branches: + - master + pull_request: + branches: + - master + schedule: + - cron: '00 07 * * 1' + workflow_dispatch: {} +permissions: + actions: read + contents: read + security-events: write +jobs: + analyze: + name: CodeQL + runs-on: ubuntu-latest + steps: + - name: Check out code + uses: actions/checkout@v4 + - name: Set up Go + uses: actions/setup-go@v5 + with: + check-latest: true + go-version: 1.23.2 + - name: Initialize CodeQL + uses: github/codeql-action/init@v3 + with: + languages: go + queries: security-extended + - name: Autobuild + uses: github/codeql-action/autobuild@v3 + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v3 diff --git a/.golangci.yaml b/.golangci.yaml index e7a6bf41..d22c2c0b 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -20,6 +20,7 @@ issues: - path: _test\.go linters: - bodyclose + - dupl # '0' disables the following options. max-issues-per-linter: 0 max-same-issues: 0 @@ -34,6 +35,7 @@ linters-settings: # Report about not checking of errors in type assertions. check-type-assertions: true forbidigo: + analyze-types: true # required for pkg: forbid: # ioutil package has been deprecated: https://github.com/golang/go/issues/42026 - ^ioutil\..*$ @@ -41,6 +43,17 @@ linters-settings: # 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 archived 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 @@ -76,8 +89,9 @@ linters-settings: # created file permissions are restricted by umask if necessary - G306 govet: - # Report about shadowed variables. - check-shadowing: true + enable-all: true + disable: + - fieldalignment nolintlint: require-specific: true stylecheck: @@ -90,12 +104,10 @@ linters-settings: default-rpc-path: true http-method: true http-status-code: true - os-dev-null: true sql-isolation-level: true - syslog-priority: true - time-weekday: true - time-month: true time-layout: true + time-month: true + time-weekday: true tls-signature-scheme: true whitespace: # Enforce newlines (or comments) after multi-line function signatures. @@ -108,15 +120,17 @@ linters: enable: - bodyclose - containedctx + - copyloopvar - dupl - dupword - durationcheck - errcheck + - errname - errorlint - - exportloopref - forbidigo - ginkgolinter - gocheckcompilerdirectives + - goconst - gocritic - gofmt - goimports @@ -124,11 +138,14 @@ linters: - gosimple - govet - ineffassign + - intrange - misspell + - nilerr - noctx - nolintlint - nosprintfhostport - perfsprint + - predeclared - rowserrcheck - sqlclosecheck - staticcheck diff --git a/.license-scan-overrides.jsonl b/.license-scan-overrides.jsonl index 708e4f0e..29aac5c8 100644 --- a/.license-scan-overrides.jsonl +++ b/.license-scan-overrides.jsonl @@ -2,3 +2,6 @@ {"name": "github.com/hashicorp/vault/api/auth/approle", "licenceType": "MPL-2.0"} {"name": "github.com/jpillora/longestcommon", "licenceType": "MIT"} {"name": "github.com/spdx/tools-golang", "licenceTextOverrideFile": "vendor/github.com/spdx/tools-golang/LICENSE.code"} +{"name": "github.com/xeipuuv/gojsonpointer", "licenceType": "Apache-2.0"} +{"name": "github.com/xeipuuv/gojsonreference", "licenceType": "Apache-2.0"} +{"name": "github.com/xeipuuv/gojsonschema", "licenceType": "Apache-2.0"} diff --git a/Dockerfile b/Dockerfile index e1a3fb8a..1309f2d7 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM golang:1.22.1-alpine3.19 as builder +FROM golang:1.23.2-alpine3.20 AS builder RUN apk add --no-cache --no-progress ca-certificates gcc git make musl-dev @@ -8,7 +8,7 @@ RUN make -C /src install PREFIX=/pkg GOTOOLCHAIN=local ################################################################################ -FROM alpine:3.19 +FROM alpine:3.20 RUN addgroup -g 4200 appgroup \ && adduser -h /home/appuser -s /sbin/nologin -G appgroup -D -u 4200 appuser diff --git a/Makefile b/Makefile index 93c907af..26a19c7e 100644 --- a/Makefile +++ b/Makefile @@ -46,11 +46,12 @@ prepare-static-check: FORCE GO_BUILDFLAGS = GO_LDFLAGS = GO_TESTENV = +GO_BUILDENV = build-all: build/maia build/maia: FORCE - go build $(GO_BUILDFLAGS) -ldflags '-s -w $(GO_LDFLAGS)' -o build/maia . + @env $(GO_BUILDENV) go build $(GO_BUILDFLAGS) -ldflags '-s -w $(GO_LDFLAGS)' -o build/maia . DESTDIR = ifeq ($(shell uname -s),Darwin) @@ -65,6 +66,9 @@ install: FORCE build/maia # which packages to test with test runner GO_TESTPKGS := $(shell go list -f '{{if or .TestGoFiles .XTestGoFiles}}{{.ImportPath}}{{end}}' ./...) +ifeq ($(GO_TESTPKGS),) +GO_TESTPKGS := ./... +endif # which packages to measure coverage for GO_COVERPKGS := $(shell go list ./...) # to get around weird Makefile syntax restrictions, we need variables containing nothing, a space and comma @@ -98,11 +102,11 @@ tidy-deps: FORCE license-headers: FORCE prepare-static-check @printf "\e[1;36m>> addlicense\e[0m\n" - @addlicense -c "SAP SE" -- $(patsubst $(shell awk '$$1 == "module" {print $$2}' go.mod)%,.%/*.go,$(shell go list ./...)) + @addlicense -c "SAP SE" -- $(patsubst $(shell awk '$$1 == "module" {print $$2}' go.mod)%,.%/*.go,$(shell go list ./...)) check-license-headers: FORCE prepare-static-check @printf "\e[1;36m>> addlicense --check\e[0m\n" - @addlicense --check -- $(patsubst $(shell awk '$$1 == "module" {print $$2}' go.mod)%,.%/*.go,$(shell go list ./...)) + @addlicense --check -- $(patsubst $(shell awk '$$1 == "module" {print $$2}' go.mod)%,.%/*.go,$(shell go list ./...)) check-dependency-licenses: FORCE prepare-static-check @printf "\e[1;36m>> go-licence-detector\e[0m\n" @@ -113,6 +117,7 @@ clean: FORCE vars: FORCE @printf "DESTDIR=$(DESTDIR)\n" + @printf "GO_BUILDENV=$(GO_BUILDENV)\n" @printf "GO_BUILDFLAGS=$(GO_BUILDFLAGS)\n" @printf "GO_COVERPKGS=$(GO_COVERPKGS)\n" @printf "GO_LDFLAGS=$(GO_LDFLAGS)\n" @@ -145,7 +150,7 @@ help: FORCE @printf "\n" @printf "\e[1mDevelopment\e[0m\n" @printf " \e[36mtidy-deps\e[0m Run go mod tidy and go mod verify.\n" - @printf " \e[36mlicense-headers\e[0m Add license headers to all non-vendored .go files.\n" + @printf " \e[36mlicense-headers\e[0m Add license headers to all non-vendored source code files.\n" @printf " \e[36mcheck-license-headers\e[0m Check license headers in all non-vendored .go files.\n" @printf " \e[36mcheck-dependency-licenses\e[0m Check all dependency licenses using go-licence-detector.\n" @printf " \e[36mclean\e[0m Run git clean.\n" diff --git a/go.mod b/go.mod index 13688cfc..4894b0cd 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/sapcc/maia -go 1.22 +go 1.23 require ( github.com/databus23/goslo.policy v0.0.0-20210929125152-81bf2876dbdb From fbf101f64e00ca78098e57bdd57d82aee5c07863 Mon Sep 17 00:00:00 2001 From: Nathan Oyler Date: Fri, 4 Oct 2024 12:45:23 -0700 Subject: [PATCH 2/9] turning off analyze types manually, seeing if that's the typecheck failing --- .golangci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.golangci.yaml b/.golangci.yaml index d22c2c0b..ab76cafb 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -35,7 +35,7 @@ linters-settings: # Report about not checking of errors in type assertions. check-type-assertions: true forbidigo: - analyze-types: true # required for pkg: + # analyze-types: true # required for pkg: forbid: # ioutil package has been deprecated: https://github.com/golang/go/issues/42026 - ^ioutil\..*$ From 6cb4b07f2e6af970fd35a6bdfd5ffa78ae7bf937 Mon Sep 17 00:00:00 2001 From: Nathan Oyler Date: Fri, 4 Oct 2024 12:54:28 -0700 Subject: [PATCH 3/9] didn't work, adding type check back. What about if I add a make generate to the checks? --- .github/workflows/checks.yaml | 2 ++ .golangci.yaml | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index c036768c..ffb253ce 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -27,6 +27,8 @@ jobs: with: check-latest: true go-version: 1.23.2 + - name: Generate code + run: make generate - name: Run golangci-lint uses: golangci/golangci-lint-action@v6 with: diff --git a/.golangci.yaml b/.golangci.yaml index ab76cafb..d22c2c0b 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -35,7 +35,7 @@ linters-settings: # Report about not checking of errors in type assertions. check-type-assertions: true forbidigo: - # analyze-types: true # required for pkg: + analyze-types: true # required for pkg: forbid: # ioutil package has been deprecated: https://github.com/golang/go/issues/42026 - ^ioutil\..*$ From 453b655361c7fcf04eb14a785c6e8272a19fe1fd Mon Sep 17 00:00:00 2001 From: Nathan Oyler Date: Fri, 4 Oct 2024 13:18:31 -0700 Subject: [PATCH 4/9] fix keystone_test lint while i try to sort out how to make this work overall --- pkg/keystone/keystone_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/keystone/keystone_test.go b/pkg/keystone/keystone_test.go index ec6eb2b7..1905e79d 100644 --- a/pkg/keystone/keystone_test.go +++ b/pkg/keystone/keystone_test.go @@ -167,7 +167,7 @@ func TestChildProjects(t *testing.T) { assertDone(t) } -func TestAuthenticateRequest(t *testing.T) { //nolint:dupl // the tests being verbose i think is best. perhaps not +func TestAuthenticateRequest(t *testing.T) { defer gock.Off() ks := setupTest() @@ -187,7 +187,7 @@ func TestAuthenticateRequest(t *testing.T) { //nolint:dupl // the tests being ve assertDone(t) } -func TestAuthenticateRequest_urlScope(t *testing.T) { //nolint:dupl // the tests being verbose i think is best. perhaps not +func TestAuthenticateRequest_urlScope(t *testing.T) { defer gock.Off() ks := setupTest() From 4f850714e9e82ac106f52c7c99808cb7c278e368 Mon Sep 17 00:00:00 2001 From: Nathan Oyler Date: Fri, 4 Oct 2024 14:31:27 -0700 Subject: [PATCH 5/9] doubt this does anything productive, but trying things. --- .github/workflows/checks.yaml | 2 -- Makefile | 1 + Makefile.maker.yaml | 1 + 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index ffb253ce..c036768c 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -27,8 +27,6 @@ jobs: with: check-latest: true go-version: 1.23.2 - - name: Generate code - run: make generate - name: Run golangci-lint uses: golangci/golangci-lint-action@v6 with: diff --git a/Makefile b/Makefile index 26a19c7e..b1480daf 100644 --- a/Makefile +++ b/Makefile @@ -22,6 +22,7 @@ build/maia: generate build-all: generate static-check: generate build/cover.out: generate +prepare-static-check: generate generate: FORCE if ! hash mockgen 2>/dev/null; then go install go.uber.org/mock/mockgen@latest; fi diff --git a/Makefile.maker.yaml b/Makefile.maker.yaml index a9b60229..0f7958ca 100644 --- a/Makefile.maker.yaml +++ b/Makefile.maker.yaml @@ -38,6 +38,7 @@ verbatim: | build-all: generate static-check: generate build/cover.out: generate + prepare-static-check: generate generate: FORCE if ! hash mockgen 2>/dev/null; then go install go.uber.org/mock/mockgen@latest; fi From 9dbdf8909eb8b52f72a9de1b1e890937be4c1e60 Mon Sep 17 00:00:00 2001 From: Nathan Oyler Date: Fri, 4 Oct 2024 14:33:28 -0700 Subject: [PATCH 6/9] add generate to the check github action.. --- .github/workflows/checks.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index c036768c..ffb253ce 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -27,6 +27,8 @@ jobs: with: check-latest: true go-version: 1.23.2 + - name: Generate code + run: make generate - name: Run golangci-lint uses: golangci/golangci-lint-action@v6 with: From f43a75fd190867608783d21e6ad2c34c0481b510 Mon Sep 17 00:00:00 2001 From: Nathan Oyler Date: Fri, 4 Oct 2024 14:40:40 -0700 Subject: [PATCH 7/9] try skip dirs --- .github/workflows/checks.yaml | 2 -- .golangci.yaml | 5 +++++ Makefile.maker.yaml | 5 +++++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index ffb253ce..c036768c 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -27,8 +27,6 @@ jobs: with: check-latest: true go-version: 1.23.2 - - name: Generate code - run: make generate - name: Run golangci-lint uses: golangci/golangci-lint-action@v6 with: diff --git a/.golangci.yaml b/.golangci.yaml index d22c2c0b..a09f5a4e 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -16,6 +16,11 @@ issues: # 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-dirs: + - vendor + - web/static + - web/templates + - pkg/api/ui exclude-rules: - path: _test\.go linters: diff --git a/Makefile.maker.yaml b/Makefile.maker.yaml index 0f7958ca..64d9ed5d 100644 --- a/Makefile.maker.yaml +++ b/Makefile.maker.yaml @@ -16,6 +16,11 @@ dockerfile: golangciLint: createConfig: true + skipDirs: + - vendor + - web/static + - web/templates + - pkg/api/ui githubWorkflow: ci: From 1bd35e9a1827c7913141239f8b2415162bce20eb Mon Sep 17 00:00:00 2001 From: Nathan Oyler Date: Fri, 4 Oct 2024 14:44:36 -0700 Subject: [PATCH 8/9] try skip dirs 2 --- Makefile.maker.yaml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Makefile.maker.yaml b/Makefile.maker.yaml index 64d9ed5d..6905383b 100644 --- a/Makefile.maker.yaml +++ b/Makefile.maker.yaml @@ -17,10 +17,7 @@ dockerfile: golangciLint: createConfig: true skipDirs: - - vendor - - web/static - - web/templates - - pkg/api/ui + - pkg/api/ui/* githubWorkflow: ci: From 93bcca1a43ffd737483c01f0b3d96820e9c29768 Mon Sep 17 00:00:00 2001 From: Nathan Oyler Date: Fri, 4 Oct 2024 14:44:54 -0700 Subject: [PATCH 9/9] actually run the go-makefile-maker after conf change --- .golangci.yaml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index a09f5a4e..e3cbeea3 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -17,10 +17,7 @@ issues: # Ref: https://go.dev/doc/effective_go#redeclaration - 'declaration of "err" shadows declaration at' exclude-dirs: - - vendor - - web/static - - web/templates - - pkg/api/ui + - pkg/api/ui/* exclude-rules: - path: _test\.go linters: