diff --git a/.github/scripts/generate-test-package-lists.sh b/.github/scripts/generate-test-package-lists.sh index 3e2fca15be..584ae9237f 100755 --- a/.github/scripts/generate-test-package-lists.sh +++ b/.github/scripts/generate-test-package-lists.sh @@ -45,8 +45,8 @@ fi test_packages[4]+=" $base/http" test_packages[4]+=" $base/sdk/helper/pluginutil" test_packages[4]+=" $base/serviceregistration/kubernetes" -test_packages[4]+=" $base/tools/godoctests/pkg/analyzer" -test_packages[4]+=" $base/tools/gonilnilfunctions/pkg/analyzer" +test_packages[4]+=" $base/tools/codechecker/pkg/godoctests" +test_packages[4]+=" $base/tools/codechecker/pkg/gonilnilfunctions" if [ "${ENTERPRISE:+x}" == "x" ] ; then test_packages[4]+=" $base/vault/external_tests/apilock" test_packages[4]+=" $base/vault/external_tests/filteredpaths" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 593ac80dff..7e957eb427 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,18 +48,6 @@ jobs: echo 'enterprise=' >> "$GITHUB_OUTPUT" echo 'go-build-tags=' >> "$GITHUB_OUTPUT" fi - semgrep: - name: Semgrep - needs: - - setup - runs-on: ${{ fromJSON(needs.setup.outputs.compute-tiny) }} - container: - image: returntocorp/semgrep@sha256:ffc6f3567654f9431456d49fd059dfe548f007c494a7eb6cd5a1a3e50d813fb3 - steps: - - uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3 - - name: Run Semgrep Rules - id: semgrep - run: semgrep ci --include '*.go' --config 'tools/semgrep/ci' setup-go-cache: name: Go Caches needs: @@ -68,25 +56,6 @@ jobs: with: runs-on: ${{ needs.setup.outputs.compute-standard }} secrets: inherit - fmt: - name: Check Format - needs: - - setup - runs-on: ${{ fromJSON(needs.setup.outputs.compute-tiny) }} - steps: - - uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3 - - uses: actions/setup-go@fac708d6674e30b6ba41289acaab6d4b75aa0753 # v4.0.1 - with: - go-version-file: ./.go-version - cache: true - - id: format - run: | - echo "Using gofumpt version $(go run mvdan.cc/gofumpt -version)" - make fmt - if ! git diff --exit-code; then - echo "Code has formatting errors. Run 'make fmt' to fix" - exit 1 - fi diff-oss-ci: name: Diff OSS needs: diff --git a/.github/workflows/code-checker.yml b/.github/workflows/code-checker.yml new file mode 100644 index 0000000000..584f707740 --- /dev/null +++ b/.github/workflows/code-checker.yml @@ -0,0 +1,73 @@ +name: Run linters + +on: + pull_request: + types: [opened, synchronize, reopened, ready_for_review] + push: + branches: + - main + - release/** +concurrency: + group: ${{ github.head_ref || github.run_id }}-lint + cancel-in-progress: true + +jobs: + deprecations: + name: Deprecated functions + runs-on: ubuntu-latest + if: github.base_ref == 'main' + steps: + - uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3 + with: + fetch-depth: 0 + - uses: actions/setup-go@fac708d6674e30b6ba41289acaab6d4b75aa0753 # v4.0.1 + with: + go-version-file: ./.go-version + cache: true + - run: make ci-deprecations + name: Check deprecations + codechecker: + name: Code checks + runs-on: ubuntu-latest + if: github.base_ref == 'main' + steps: + - uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3 + with: + fetch-depth: 0 + - uses: actions/setup-go@fac708d6674e30b6ba41289acaab6d4b75aa0753 # v4.0.1 + with: + go-version-file: ./.go-version + cache: true + # Note: if there is a function we want to ignore the nilnil check for, + # You can add 'ignore-nil-nil-function-check' somewhere in the + # godoc for the function. + - run: make ci-vet-codechecker + name: Check custom linters + format: + name: Format + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3 + - uses: actions/setup-go@fac708d6674e30b6ba41289acaab6d4b75aa0753 # v4.0.1 + with: + go-version-file: ./.go-version + cache: true + - name: Go format + run: | + make ci-bootstrap + echo "Using gofumpt version $(go run mvdan.cc/gofumpt -version)" + make fmt + if ! git diff --exit-code; then + echo "Code has formatting errors. Run 'make fmt' to fix" + exit 1 + fi + semgrep: + name: Semgrep + runs-on: ubuntu-latest + container: + image: returntocorp/semgrep@sha256:ffc6f3567654f9431456d49fd059dfe548f007c494a7eb6cd5a1a3e50d813fb3 + steps: + - uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3 + - name: Run Semgrep Rules + id: semgrep + run: semgrep ci --include '*.go' --config 'tools/semgrep/ci' diff --git a/.github/workflows/drepecated-functions-checker.yml b/.github/workflows/drepecated-functions-checker.yml deleted file mode 100644 index e23219b597..0000000000 --- a/.github/workflows/drepecated-functions-checker.yml +++ /dev/null @@ -1,31 +0,0 @@ -name: "Check Deprecations" - -on: - pull_request: - # Runs on PRs to main - branches: - - main - -jobs: - deprecations-check: - runs-on: ubuntu-latest - timeout-minutes: 30 - steps: - - name: Checkout code - uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3 - with: - fetch-depth: 0 # by default the checkout action doesn't checkout all branches - - name: Setup Go - uses: actions/setup-go@fac708d6674e30b6ba41289acaab6d4b75aa0753 # v4.0.1 - with: - go-version-file: ./.go-version - cache: true - - name: Install required tools - run: | - make bootstrap - - name: Check deprecations for files in diff - run: | - # Need to run this from repository root and not from scripts/ as staticcheck works - # only on packages - ./scripts/deprecations-checker.sh ${{ github.event.pull_request.base.ref }} ${{ github.event.repository.name }} - \ No newline at end of file diff --git a/.github/workflows/godoc-test-checker.yml b/.github/workflows/godoc-test-checker.yml deleted file mode 100644 index c1defdc3d2..0000000000 --- a/.github/workflows/godoc-test-checker.yml +++ /dev/null @@ -1,23 +0,0 @@ -name: Check Go Docs for tests - -on: - pull_request: - types: [opened, synchronize] - # Runs on PRs to main - branches: - - main - -jobs: - godoc-test-check: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3 - with: - fetch-depth: 0 - - name: Set Up Go - uses: actions/setup-go@fac708d6674e30b6ba41289acaab6d4b75aa0753 # v4.0.1 - with: - cache: true - go-version-file: ./.go-version - - name: Verify new tests have go docs - run: make ci-vet-godoctests diff --git a/.github/workflows/nil-nil-function-checker.yml b/.github/workflows/nil-nil-function-checker.yml deleted file mode 100644 index fc78deb3b2..0000000000 --- a/.github/workflows/nil-nil-function-checker.yml +++ /dev/null @@ -1,26 +0,0 @@ -name: Check Functions For nil, nil returns - -on: - pull_request: - types: [opened, synchronize] - # Runs on PRs to main - branches: - - main - -jobs: - # Note: if there is a function we want to ignore this check for, - # You can add 'ignore-nil-nil-function-check' somewhere in the - # godoc for the function. - nil-nil-function-check: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3 - with: - fetch-depth: 0 - - name: Set Up Go - uses: actions/setup-go@fac708d6674e30b6ba41289acaab6d4b75aa0753 # v4.0.1 - with: - cache: true - go-version-file: ./.go-version - - name: Verify functions don't return nil, nil - run: make ci-vet-gonilnilfunctions diff --git a/.gitignore b/.gitignore index c8d0ffbb95..8c7c498183 100644 --- a/.gitignore +++ b/.gitignore @@ -129,3 +129,5 @@ website/components/node_modules tools/godoctests/.bin tools/gonilnilfunctions/.bin +tools/codechecker/.bin +.ci-bootstrap \ No newline at end of file diff --git a/Makefile b/Makefile index 82fed7a838..0c30e3a3cb 100644 --- a/Makefile +++ b/Makefile @@ -9,7 +9,9 @@ INTEG_TEST_TIMEOUT=120m VETARGS?=-asmdecl -atomic -bool -buildtags -copylocks -methods -nilfunc -printf -rangeloops -shift -structtags -unsafeptr EXTERNAL_TOOLS_CI=\ golang.org/x/tools/cmd/goimports \ - github.com/golangci/revgrep/cmd/revgrep + github.com/golangci/revgrep/cmd/revgrep \ + mvdan.cc/gofumpt \ + honnef.co/go/tools/cmd/staticcheck EXTERNAL_TOOLS=\ github.com/client9/misspell/cmd/misspell GOFMT_FILES?=$$(find . -name '*.go' | grep -v pb.go | grep -v vendor) @@ -113,46 +115,28 @@ vet: # deprecations runs staticcheck tool to look for deprecations. Checks entire code to see if it # has deprecated function, variable, constant or field -deprecations: - make bootstrap - repositoryName=$(basename `git rev-parse --show-toplevel`) - ./scripts/deprecations-checker.sh "" repositoryName +deprecations: bootstrap + @BUILD_TAGS='$(BUILD_TAGS)' ./scripts/deprecations-checker.sh "" # ci-deprecations runs staticcheck tool to look for deprecations. All output gets piped to revgrep # which will only return an error if changes that is not on main has deprecated function, variable, constant or field -ci-deprecations: - make bootstrap - repositoryName=$(basename `git rev-parse --show-toplevel`) - ./scripts/deprecations-checker.sh main repositoryName +ci-deprecations: ci-bootstrap + @BUILD_TAGS='$(BUILD_TAGS)' ./scripts/deprecations-checker.sh main -# tools/godoctests/.bin/godoctests builds the custom analyzer to check for godocs for tests -tools/godoctests/.bin/godoctests: - @cd tools/godoctests && $(GO_CMD) build -o .bin/godoctests . +tools/codechecker/.bin/codechecker: + @cd tools/codechecker && $(GO_CMD) build -o .bin/codechecker . -# vet-godoctests runs godoctests on the test functions. All output gets piped to revgrep -# which will only return an error if a new function is missing a godoc -vet-godoctests: bootstrap tools/godoctests/.bin/godoctests - @$(GO_CMD) vet -vettool=./tools/godoctests/.bin/godoctests $(TEST) 2>&1 | revgrep - -# ci-vet-godoctests runs godoctests on the test functions. All output gets piped to revgrep -# which will only return an error if a new function that is not on main is missing a godoc -ci-vet-godoctests: ci-bootstrap tools/godoctests/.bin/godoctests - @$(GO_CMD) vet -vettool=./tools/godoctests/.bin/godoctests $(TEST) 2>&1 | revgrep origin/main - -# tools/gonilnilfunctions/.bin/gonilnilfunctions builds the custom analyzer to check for nil, nil function returns -tools/gonilnilfunctions/.bin/gonilnilfunctions: - @cd tools/gonilnilfunctions && $(GO_CMD) build -o .bin/gonilnilfunctions . - -# vet-gonilnilfunctions runs gonilnilfunctions on functions. All output gets piped to revgrep -# which will only return an error if a new function returns nil, nil (where one of the nils could be an error) -vet-gonilnilfunctions: bootstrap tools/gonilnilfunctions/.bin/gonilnilfunctions - @$(GO_CMD) vet -vettool=./tools/gonilnilfunctions/.bin/gonilnilfunctions ./... 2>&1 | revgrep - -# ci-vet-gonilnilfunctions runs gonilnilfunctions on functions. All output gets piped to revgrep -# which will only return an error if a new function that is not on main has an issue -ci-vet-gonilnilfunctions: ci-bootstrap tools/gonilnilfunctions/.bin/gonilnilfunctions - @$(GO_CMD) vet -vettool=./tools/gonilnilfunctions/.bin/gonilnilfunctions ./... 2>&1 | revgrep origin/main +# vet-codechecker runs our custom linters on the test functions. All output gets +# piped to revgrep which will only return an error if new piece of code violates +# the check +vet-codechecker: bootstrap tools/codechecker/.bin/codechecker + @$(GO_CMD) vet -vettool=./tools/codechecker/.bin/codechecker -tags=$(BUILD_TAGS) ./... 2>&1 | revgrep +# vet-codechecker runs our custom linters on the test functions. All output gets +# piped to revgrep which will only return an error if new piece of code that is +# not on main violates the check +ci-vet-codechecker: ci-bootstrap tools/codechecker/.bin/codechecker + @$(GO_CMD) vet -vettool=./tools/codechecker/.bin/codechecker -tags=$(BUILD_TAGS) ./... 2>&1 | revgrep origin/main # lint runs vet plus a number of other checkers, it is more comprehensive, but louder lint: @@ -174,11 +158,13 @@ prep: fmtcheck @if [ -d .git/hooks ]; then cp .hooks/* .git/hooks/; fi # bootstrap the build by downloading additional tools needed to build -ci-bootstrap: +ci-bootstrap: .ci-bootstrap +.ci-bootstrap: @for tool in $(EXTERNAL_TOOLS_CI) ; do \ echo "Installing/Updating $$tool" ; \ GO111MODULE=off $(GO_CMD) get -u $$tool; \ done + @touch .ci-bootstrap # bootstrap the build by downloading additional tools that may be used by devs bootstrap: ci-bootstrap @@ -255,7 +241,7 @@ fmtcheck: @true #@sh -c "'$(CURDIR)/scripts/gofmtcheck.sh'" -fmt: +fmt: ci-bootstrap find . -name '*.go' | grep -v pb.go | grep -v vendor | xargs go run mvdan.cc/gofumpt -w semgrep: @@ -296,7 +282,7 @@ hana-database-plugin: mongodb-database-plugin: @CGO_ENABLED=0 $(GO_CMD) build -o bin/mongodb-database-plugin ./plugins/database/mongodb/mongodb-database-plugin -.PHONY: bin default prep test vet bootstrap ci-bootstrap fmt fmtcheck mysql-database-plugin mysql-legacy-database-plugin cassandra-database-plugin influxdb-database-plugin postgresql-database-plugin mssql-database-plugin hana-database-plugin mongodb-database-plugin ember-dist ember-dist-dev static-dist static-dist-dev assetcheck check-vault-in-path packages build build-ci semgrep semgrep-ci vet-godoctests ci-vet-godoctests vet-gonilnilfunctions ci-vet-gonilnilfunctions +.PHONY: bin default prep test vet bootstrap ci-bootstrap fmt fmtcheck mysql-database-plugin mysql-legacy-database-plugin cassandra-database-plugin influxdb-database-plugin postgresql-database-plugin mssql-database-plugin hana-database-plugin mongodb-database-plugin ember-dist ember-dist-dev static-dist static-dist-dev assetcheck check-vault-in-path packages build build-ci semgrep semgrep-ci vet-codechecker ci-vet-codechecker .NOTPARALLEL: ember-dist ember-dist-dev diff --git a/scripts/deprecations-checker.sh b/scripts/deprecations-checker.sh index 017149e6d7..508464245f 100755 --- a/scripts/deprecations-checker.sh +++ b/scripts/deprecations-checker.sh @@ -22,31 +22,17 @@ # Here, it is used to check if a deprecated function, variable, constant or field is used. # Run staticcheck +set -e echo "Performing deprecations check: running staticcheck" -# Identify repository name -if [ -z $2 ]; then - # local repository name - repositoryName=$(basename `git rev-parse --show-toplevel`) -else - # github repository name from deprecated-functions-checker.yml - repositoryName=$2 -fi - -# Modify the command with the correct build tag based on repository -if [ $repositoryName == "vault-enterprise" ]; then - staticcheckCommand=$(echo "staticcheck ./... -tags=enterprise") -else - staticcheckCommand=$(echo "staticcheck ./...") -fi # If no compare branch name is specified, output all deprecations # Else only output the deprecations from the changes added if [ -z $1 ] then - $staticcheckCommand | grep deprecated + staticcheck -checks="SA1019" -tags="$BUILD_TAGS" else # GitHub Actions will use this to find only changes wrt PR's base ref branch # revgrep CLI tool will return an exit status of 1 if any issues match, else it will return 0 - $staticcheckCommand | grep deprecated 2>&1 | revgrep "$(git merge-base HEAD "origin/$1")" + staticcheck -checks="SA1019" -tags="$BUILD_TAGS" 2>&1 | revgrep "$(git merge-base HEAD "origin/$1")" fi diff --git a/tools/codechecker/.bin/codechecker b/tools/codechecker/.bin/codechecker new file mode 100755 index 0000000000..7c47d65efa Binary files /dev/null and b/tools/codechecker/.bin/codechecker differ diff --git a/tools/codechecker/main.go b/tools/codechecker/main.go new file mode 100644 index 0000000000..94d80482bb --- /dev/null +++ b/tools/codechecker/main.go @@ -0,0 +1,14 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package main + +import ( + "github.com/hashicorp/vault/tools/codechecker/pkg/godoctests" + "github.com/hashicorp/vault/tools/codechecker/pkg/gonilnilfunctions" + "golang.org/x/tools/go/analysis/multichecker" +) + +func main() { + multichecker.Main(gonilnilfunctions.Analyzer, godoctests.Analyzer) +} diff --git a/tools/godoctests/pkg/analyzer/analyzer.go b/tools/codechecker/pkg/godoctests/analyzer.go similarity index 98% rename from tools/godoctests/pkg/analyzer/analyzer.go rename to tools/codechecker/pkg/godoctests/analyzer.go index 38ed37d933..e771f9c8f9 100644 --- a/tools/godoctests/pkg/analyzer/analyzer.go +++ b/tools/codechecker/pkg/godoctests/analyzer.go @@ -1,7 +1,7 @@ // Copyright (c) HashiCorp, Inc. // SPDX-License-Identifier: MPL-2.0 -package analyzer +package godoctests import ( "go/ast" diff --git a/tools/gonilnilfunctions/pkg/analyzer/analyzer_test.go b/tools/codechecker/pkg/godoctests/analyzer_test.go similarity index 97% rename from tools/gonilnilfunctions/pkg/analyzer/analyzer_test.go rename to tools/codechecker/pkg/godoctests/analyzer_test.go index df1bfafd46..81af24d3cc 100644 --- a/tools/gonilnilfunctions/pkg/analyzer/analyzer_test.go +++ b/tools/codechecker/pkg/godoctests/analyzer_test.go @@ -1,7 +1,7 @@ // Copyright (c) HashiCorp, Inc. // SPDX-License-Identifier: MPL-2.0 -package analyzer +package godoctests import ( "os" diff --git a/tools/godoctests/pkg/analyzer/testdata/funcs.go b/tools/codechecker/pkg/godoctests/testdata/funcs.go similarity index 100% rename from tools/godoctests/pkg/analyzer/testdata/funcs.go rename to tools/codechecker/pkg/godoctests/testdata/funcs.go diff --git a/tools/gonilnilfunctions/pkg/analyzer/analyzer.go b/tools/codechecker/pkg/gonilnilfunctions/analyzer.go similarity index 99% rename from tools/gonilnilfunctions/pkg/analyzer/analyzer.go rename to tools/codechecker/pkg/gonilnilfunctions/analyzer.go index 2b9f17a6e8..5f4dd1dd84 100644 --- a/tools/gonilnilfunctions/pkg/analyzer/analyzer.go +++ b/tools/codechecker/pkg/gonilnilfunctions/analyzer.go @@ -1,7 +1,7 @@ // Copyright (c) HashiCorp, Inc. // SPDX-License-Identifier: MPL-2.0 -package analyzer +package gonilnilfunctions import ( "go/ast" diff --git a/tools/godoctests/pkg/analyzer/analyzer_test.go b/tools/codechecker/pkg/gonilnilfunctions/analyzer_test.go similarity index 96% rename from tools/godoctests/pkg/analyzer/analyzer_test.go rename to tools/codechecker/pkg/gonilnilfunctions/analyzer_test.go index df1bfafd46..b4c8cf2971 100644 --- a/tools/godoctests/pkg/analyzer/analyzer_test.go +++ b/tools/codechecker/pkg/gonilnilfunctions/analyzer_test.go @@ -1,7 +1,7 @@ // Copyright (c) HashiCorp, Inc. // SPDX-License-Identifier: MPL-2.0 -package analyzer +package gonilnilfunctions import ( "os" diff --git a/tools/gonilnilfunctions/pkg/analyzer/testdata/funcs.go b/tools/codechecker/pkg/gonilnilfunctions/testdata/funcs.go similarity index 100% rename from tools/gonilnilfunctions/pkg/analyzer/testdata/funcs.go rename to tools/codechecker/pkg/gonilnilfunctions/testdata/funcs.go diff --git a/tools/godoctests/main.go b/tools/godoctests/main.go deleted file mode 100644 index caa6ca0b93..0000000000 --- a/tools/godoctests/main.go +++ /dev/null @@ -1,13 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: MPL-2.0 - -package main - -import ( - "github.com/hashicorp/vault/tools/godoctests/pkg/analyzer" - "golang.org/x/tools/go/analysis/singlechecker" -) - -func main() { - singlechecker.Main(analyzer.Analyzer) -} diff --git a/tools/gonilnilfunctions/main.go b/tools/gonilnilfunctions/main.go deleted file mode 100644 index 68fd796a9f..0000000000 --- a/tools/gonilnilfunctions/main.go +++ /dev/null @@ -1,13 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: MPL-2.0 - -package main - -import ( - "github.com/hashicorp/vault/tools/gonilnilfunctions/pkg/analyzer" - "golang.org/x/tools/go/analysis/singlechecker" -) - -func main() { - singlechecker.Main(analyzer.Analyzer) -}