diff --git a/.github/workflows/linting.yml b/.github/workflows/linting.yml new file mode 100644 index 000000000..e3742a965 --- /dev/null +++ b/.github/workflows/linting.yml @@ -0,0 +1,30 @@ +name: "golangci-lint" +on: ["pull_request"] + +jobs: + lint: + name: "Run Linter" + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + with: + fetch-depth: '0' + - name: Determine Go version + id: get-go-version + # We use .go-version as our source of truth for current Go + # version, because "goenv" can react to it automatically. + run: | + echo "Building with Go $(cat .go-version)" + echo "::set-output name=go-version::$(cat .go-version)" + - name: Set up Go + uses: actions/setup-go@v3 + with: + go-version: "${{ steps.get-go-version.outputs.go-version }}" + - name: Install Dependencies + # if we really need to we can update this to run `make tools` + # later but its just not necessary to only run linters + run: | + make golangci-lint + - name: Running Linters + run: | + LINT_DIFF_BRANCH="origin/${GITHUB_BASE_REF}" make lint-diff diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 000000000..d8f0917fb --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,14 @@ +linters: + disable-all: true + enable: + - staticcheck + - stylecheck + +issues: + exclude-rules: + - linters: + - stylecheck + text: "ST1005:" + - linters: + - stylecheck + text: "ST1003:" diff --git a/Makefile b/Makefile index b7f3782ff..7f155bc17 100644 --- a/Makefile +++ b/Makefile @@ -7,6 +7,7 @@ TMP_DIR := $(shell mktemp -d) REPO_PATH := github.com/hashicorp/boundary CGO_ENABLED?=0 +GO_PATH = $(shell go env GOPATH) export GEN_BASEPATH := $(shell pwd) @@ -19,11 +20,21 @@ cli: $(MAKE) --environment-overrides -C internal/cmd/gencli cli .PHONY: tools -tools: +tools: golangci-lint go generate -tags tools tools/tools.go go install github.com/bufbuild/buf/cmd/buf@v1.3.1 go install github.com/mfridman/tparse@v0.10.3 +# golangci-lint recommends installing the binary directly, instead of using go get +# See the note: https://golangci-lint.run/usage/install/#install-from-source +.PHONY: golangci-lint +golangci-lint: + $(eval GOLINT_INSTALLED := $(shell which golangci-lint)) + + if [ "$(GOLINT_INSTALLED)" = "" ]; then \ + sh scripts/install-golangci-lint.sh -b $(GO_PATH)/bin v1.50.1; \ + fi; + .PHONY: cleangen cleangen: @rm -f $(shell find ${THIS_DIR} -name '*.gen.go' && find ${THIS_DIR} -name '*.pb.go' && find ${THIS_DIR} -name '*.pb.gw.go') @@ -63,6 +74,17 @@ fmt: grep -L -R "^\/\/ Code generated .* DO NOT EDIT\.$$" --exclude-dir=.git --include="*.go" . | xargs gofumpt -w buf format -w +lint: + golangci-lint run --timeout 10m + +ifndef LINT_DIFF_BRANCH +override LINT_DIFF_BRANCH = main +endif + +lint-diff: + @echo "Checking for lint compared to $(LINT_DIFF_BRANCH)" + golangci-lint run --timeout 10m --new-from-rev=$(LINT_DIFF_BRANCH) + # Set env for all UI targets. UI_TARGETS := update-ui-version build-ui build-ui-ifne clean-ui # Note the extra .tmp path segment in UI_CLONE_DIR is significant and required. diff --git a/internal/credential/vault/repository_credential_library.go b/internal/credential/vault/repository_credential_library.go index 14b645734..5f39dbe76 100644 --- a/internal/credential/vault/repository_credential_library.go +++ b/internal/credential/vault/repository_credential_library.go @@ -405,7 +405,7 @@ func (pl *listLookupLibrary) toCredentialLibrary() *CredentialLibrary { } // TableName returns the table name for gorm. -func (_ *listLookupLibrary) TableName() string { return "credential_vault_library_list_lookup" } +func (*listLookupLibrary) TableName() string { return "credential_vault_library_list_lookup" } // GetPublicId returns the public id. func (pl *listLookupLibrary) GetPublicId() string { return pl.PublicId } diff --git a/internal/credential/vault/repository_credential_store.go b/internal/credential/vault/repository_credential_store.go index 1310c1ce1..85978782d 100644 --- a/internal/credential/vault/repository_credential_store.go +++ b/internal/credential/vault/repository_credential_store.go @@ -320,7 +320,7 @@ func (ps *listLookupStore) toCredentialStore() *CredentialStore { } // TableName returns the table name for gorm. -func (_ *listLookupStore) TableName() string { return "credential_vault_store_list_lookup" } +func (*listLookupStore) TableName() string { return "credential_vault_store_list_lookup" } // GetPublicId returns the public id. func (ps *listLookupStore) GetPublicId() string { return ps.PublicId } diff --git a/internal/db/read_writer.go b/internal/db/read_writer.go index de58eb56a..0c3026320 100644 --- a/internal/db/read_writer.go +++ b/internal/db/read_writer.go @@ -397,9 +397,9 @@ func (rw *Db) DeleteItems(ctx context.Context, deleteItems []any, opt ...Option) // you should ensure that any objects written to the db in your TxHandler are retryable, which // means that the object may be sent to the db several times (retried), so things like the primary key must // be reset before retry -func (w *Db) DoTx(ctx context.Context, retries uint, backOff Backoff, handler TxHandler) (RetryInfo, error) { +func (rw *Db) DoTx(ctx context.Context, retries uint, backOff Backoff, handler TxHandler) (RetryInfo, error) { const op = "db.DoTx" - if w.underlying == nil { + if rw.underlying == nil { return RetryInfo{}, errors.New(ctx, errors.InvalidParameter, op, "missing underlying db") } if backOff == nil { @@ -415,7 +415,7 @@ func (w *Db) DoTx(ctx context.Context, retries uint, backOff Backoff, handler Tx } // step one of this, start a transaction... - beginTx, err := dbw.New(w.underlying.wrapped.Load()).Begin(ctx) + beginTx, err := dbw.New(rw.underlying.wrapped.Load()).Begin(ctx) if err != nil { return info, wrapError(ctx, err, op) } diff --git a/internal/iam/principal_role.go b/internal/iam/principal_role.go index ab85f2995..796e93d23 100644 --- a/internal/iam/principal_role.go +++ b/internal/iam/principal_role.go @@ -108,12 +108,12 @@ func (r *UserRole) Clone() any { } // VetForWrite implements db.VetForWrite() interface for user roles. -func (role *UserRole) VetForWrite(ctx context.Context, _ db.Reader, _ db.OpType, _ ...db.Option) error { +func (r *UserRole) VetForWrite(ctx context.Context, _ db.Reader, _ db.OpType, _ ...db.Option) error { const op = "iam.(UserRole).VetForWrite" - if role.RoleId == "" { + if r.RoleId == "" { return errors.New(ctx, errors.InvalidParameter, op, "missing role id") } - if role.PrincipalId == "" { + if r.PrincipalId == "" { return errors.New(ctx, errors.InvalidParameter, op, "missing user id") } return nil @@ -185,12 +185,12 @@ func (r *GroupRole) Clone() any { } // VetForWrite implements db.VetForWrite() interface for group roles. -func (role *GroupRole) VetForWrite(ctx context.Context, r db.Reader, opType db.OpType, opt ...db.Option) error { +func (r *GroupRole) VetForWrite(ctx context.Context, _ db.Reader, opType db.OpType, opt ...db.Option) error { const op = "iam.(GroupRole).VetForWrite" - if role.RoleId == "" { + if r.RoleId == "" { return errors.New(ctx, errors.InvalidParameter, op, "missing role id") } - if role.PrincipalId == "" { + if r.PrincipalId == "" { return errors.New(ctx, errors.InvalidParameter, op, "missing group id") } return nil @@ -264,12 +264,12 @@ func (r *ManagedGroupRole) Clone() any { } // VetForWrite implements db.VetForWrite() interface for managed group roles. -func (role ManagedGroupRole) VetForWrite(ctx context.Context, r db.Reader, opType db.OpType, opt ...db.Option) error { +func (r ManagedGroupRole) VetForWrite(ctx context.Context, _ db.Reader, opType db.OpType, opt ...db.Option) error { const op = "iam.(ManagedGroupRole).VetForWrite" - if role.RoleId == "" { + if r.RoleId == "" { return errors.New(ctx, errors.InvalidParameter, op, "missing role id") } - if role.PrincipalId == "" { + if r.PrincipalId == "" { return errors.New(ctx, errors.InvalidParameter, op, "missing managed group id") } return nil diff --git a/internal/observability/event/event_observation.go b/internal/observability/event/event_observation.go index 68c11824b..311d44a87 100644 --- a/internal/observability/event/event_observation.go +++ b/internal/observability/event/event_observation.go @@ -55,14 +55,14 @@ func newObservation(fromOperation Op, opt ...Option) (*observation, error) { } // EventType is required for all event types by the eventlogger broker -func (i *observation) EventType() string { return string(ObservationType) } +func (o *observation) EventType() string { return string(ObservationType) } -func (i *observation) validate() error { +func (o *observation) validate() error { const op = "event.(Observation).validate" - if i.ID == "" { + if o.ID == "" { return fmt.Errorf("%s: missing id: %w", op, ErrInvalidParameter) } - if i.Op == "" { + if o.Op == "" { return fmt.Errorf("%s: missing operation: %w", op, ErrInvalidParameter) } return nil diff --git a/internal/session/job_session_cleanup_test.go b/internal/session/job_session_cleanup_test.go index 037280fa5..5f64ead79 100644 --- a/internal/session/job_session_cleanup_test.go +++ b/internal/session/job_session_cleanup_test.go @@ -40,6 +40,7 @@ func TestSessionConnectionCleanupJob(t *testing.T) { serversRepo, err := server.NewRepository(rw, rw, kms) require.NoError(err) sessionRepo, err := NewRepository(ctx, rw, rw, kms) + require.NoError(err) connectionRepo, err := NewConnectionRepository(ctx, rw, rw, kms) require.NoError(err) diff --git a/internal/session/repository_session_test.go b/internal/session/repository_session_test.go index 1b2e25523..73db29fa2 100644 --- a/internal/session/repository_session_test.go +++ b/internal/session/repository_session_test.go @@ -1564,6 +1564,7 @@ func TestRepository_deleteTargetFKey(t *testing.T) { kms := kms.TestKms(t, conn, wrapper) ctx := context.Background() repo, err := NewRepository(ctx, rw, rw, kms) + require.NoError(t, err) targetRepo, err := target.NewRepository(ctx, rw, rw, kms) require.NoError(t, err) diff --git a/scripts/install-golangci-lint.sh b/scripts/install-golangci-lint.sh new file mode 100644 index 000000000..edc353c44 --- /dev/null +++ b/scripts/install-golangci-lint.sh @@ -0,0 +1,408 @@ +#!/bin/sh +set -e + +usage() { + this=$1 + cat </dev/null +} +echoerr() { + echo "$@" 1>&2 +} +log_prefix() { + echo "$0" +} +_logp=6 +log_set_priority() { + _logp="$1" +} +log_priority() { + if test -z "$1"; then + echo "$_logp" + return + fi + [ "$1" -le "$_logp" ] +} +log_tag() { + case $1 in + 0) echo "emerg" ;; + 1) echo "alert" ;; + 2) echo "crit" ;; + 3) echo "err" ;; + 4) echo "warning" ;; + 5) echo "notice" ;; + 6) echo "info" ;; + 7) echo "debug" ;; + *) echo "$1" ;; + esac +} +log_debug() { + log_priority 7 || return 0 + echoerr "$(log_prefix)" "$(log_tag 7)" "$@" +} +log_info() { + log_priority 6 || return 0 + echoerr "$(log_prefix)" "$(log_tag 6)" "$@" +} +log_err() { + log_priority 3 || return 0 + echoerr "$(log_prefix)" "$(log_tag 3)" "$@" +} +log_crit() { + log_priority 2 || return 0 + echoerr "$(log_prefix)" "$(log_tag 2)" "$@" +} +uname_os() { + os=$(uname -s | tr '[:upper:]' '[:lower:]') + case "$os" in + msys*) os="windows" ;; + mingw*) os="windows" ;; + cygwin*) os="windows" ;; + win*) os="windows" ;; + esac + echo "$os" +} +uname_arch() { + arch=$(uname -m) + case $arch in + x86_64) arch="amd64" ;; + x86) arch="386" ;; + i686) arch="386" ;; + i386) arch="386" ;; + aarch64) arch="arm64" ;; + armv5*) arch="armv5" ;; + armv6*) arch="armv6" ;; + armv7*) arch="armv7" ;; + esac + echo ${arch} +} +uname_os_check() { + os=$(uname_os) + case "$os" in + darwin) return 0 ;; + dragonfly) return 0 ;; + freebsd) return 0 ;; + linux) return 0 ;; + android) return 0 ;; + nacl) return 0 ;; + netbsd) return 0 ;; + openbsd) return 0 ;; + plan9) return 0 ;; + solaris) return 0 ;; + windows) return 0 ;; + esac + log_crit "uname_os_check '$(uname -s)' got converted to '$os' which is not a GOOS value." + return 1 +} +uname_arch_check() { + arch=$(uname_arch) + case "$arch" in + 386) return 0 ;; + amd64) return 0 ;; + arm64) return 0 ;; + armv5) return 0 ;; + armv6) return 0 ;; + armv7) return 0 ;; + ppc64) return 0 ;; + ppc64le) return 0 ;; + mips) return 0 ;; + mipsle) return 0 ;; + mips64) return 0 ;; + mips64le) return 0 ;; + s390x) return 0 ;; + riscv64) return 0 ;; + amd64p32) return 0 ;; + esac + log_crit "uname_arch_check '$(uname -m)' got converted to '$arch' which is not a GOARCH value." + return 1 +} +untar() { + tarball=$1 + case "${tarball}" in + *.tar.gz | *.tgz) tar --no-same-owner -xzf "${tarball}" ;; + *.tar) tar --no-same-owner -xf "${tarball}" ;; + *.zip) unzip "${tarball}" ;; + *) + log_err "untar unknown archive format for ${tarball}" + return 1 + ;; + esac +} +http_download_curl() { + local_file=$1 + source_url=$2 + header=$3 + if [ -z "$header" ]; then + code=$(curl -w '%{http_code}' -sL -o "$local_file" "$source_url") + else + code=$(curl -w '%{http_code}' -sL -H "$header" -o "$local_file" "$source_url") + fi + if [ "$code" != "200" ]; then + log_debug "http_download_curl received HTTP status $code" + return 1 + fi + return 0 +} +http_download_wget() { + local_file=$1 + source_url=$2 + header=$3 + if [ -z "$header" ]; then + wget -q -O "$local_file" "$source_url" + else + wget -q --header "$header" -O "$local_file" "$source_url" + fi +} +http_download() { + log_debug "http_download $2" + if is_command curl; then + http_download_curl "$@" + return + elif is_command wget; then + http_download_wget "$@" + return + fi + log_crit "http_download unable to find wget or curl" + return 1 +} +http_copy() { + tmp=$(mktemp) + http_download "${tmp}" "$1" "$2" || return 1 + body=$(cat "$tmp") + rm -f "${tmp}" + echo "$body" +} +github_release() { + owner_repo=$1 + version=$2 + test -z "$version" && version="latest" + giturl="https://github.com/${owner_repo}/releases/${version}" + json=$(http_copy "$giturl" "Accept:application/json") + test -z "$json" && return 1 + version=$(echo "$json" | tr -s '\n' ' ' | sed 's/.*"tag_name":"//' | sed 's/".*//') + test -z "$version" && return 1 + echo "$version" +} +hash_sha256() { + TARGET=${1:-/dev/stdin} + if is_command gsha256sum; then + hash=$(gsha256sum "$TARGET") || return 1 + echo "$hash" | cut -d ' ' -f 1 + elif is_command sha256sum; then + hash=$(sha256sum "$TARGET") || return 1 + echo "$hash" | cut -d ' ' -f 1 + elif is_command shasum; then + hash=$(shasum -a 256 "$TARGET" 2>/dev/null) || return 1 + echo "$hash" | cut -d ' ' -f 1 + elif is_command openssl; then + hash=$(openssl -dst openssl dgst -sha256 "$TARGET") || return 1 + echo "$hash" | cut -d ' ' -f a + else + log_crit "hash_sha256 unable to find command to compute sha-256 hash" + return 1 + fi +} +hash_sha256_verify() { + TARGET=$1 + checksums=$2 + if [ -z "$checksums" ]; then + log_err "hash_sha256_verify checksum file not specified in arg2" + return 1 + fi + BASENAME=${TARGET##*/} + want=$(grep "${BASENAME}" "${checksums}" 2>/dev/null | tr '\t' ' ' | cut -d ' ' -f 1) + if [ -z "$want" ]; then + log_err "hash_sha256_verify unable to find checksum for '${TARGET}' in '${checksums}'" + return 1 + fi + got=$(hash_sha256 "$TARGET") + if [ "$want" != "$got" ]; then + log_err "hash_sha256_verify checksum for '$TARGET' did not verify ${want} vs $got" + return 1 + fi +} +cat /dev/null <