Add Staticcheck to Build Pipeline (#2421)

Run staticcheck linters on pull requests

.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

.golangci.yml

@@ -0,0 +1,14 @@
+linters:
+  disable-all: true
+  enable:
+    - staticcheck
+    - stylecheck
+
+issues:
+  exclude-rules:
+    - linters:
+        - stylecheck
+      text: "ST1005:"
+    - linters:
+        - stylecheck
+      text: "ST1003:"

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.

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 }

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 }

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)
 		}

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

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

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)
 

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)
 

scripts/install-golangci-lint.sh

@@ -0,0 +1,408 @@
+#!/bin/sh
+set -e
+
+usage() {
+  this=$1
+  cat <<EOF
+$this: download go binaries for golangci/golangci-lint
+
+Usage: $this [-b] bindir [-d] [tag]
+  -b sets bindir or installation directory, Defaults to ./bin
+  -d turns on debug logging
+   [tag] is a tag from
+   https://github.com/golangci/golangci-lint/releases
+   If tag is missing, then the latest will be used.
+
+ Generated by godownloader
+  https://github.com/goreleaser/godownloader
+
+EOF
+  exit 2
+}
+
+parse_args() {
+  #BINDIR is ./bin unless set be ENV
+  # over-ridden by flag below
+
+  BINDIR=${BINDIR:-./bin}
+  while getopts "b:dh?x" arg; do
+    case "$arg" in
+      b) BINDIR="$OPTARG" ;;
+      d) log_set_priority 10 ;;
+      h | \?) usage "$0" ;;
+      x) set -x ;;
+    esac
+  done
+  shift $((OPTIND - 1))
+  TAG=$1
+}
+# this function wraps all the destructive operations
+# if a curl|bash cuts off the end of the script due to
+# network, either nothing will happen or will syntax error
+# out preventing half-done work
+execute() {
+  tmpdir=$(mktemp -d)
+  log_debug "downloading files into ${tmpdir}"
+  http_download "${tmpdir}/${TARBALL}" "${TARBALL_URL}"
+  http_download "${tmpdir}/${CHECKSUM}" "${CHECKSUM_URL}"
+  hash_sha256_verify "${tmpdir}/${TARBALL}" "${tmpdir}/${CHECKSUM}"
+  srcdir="${tmpdir}/${NAME}"
+  rm -rf "${srcdir}"
+  (cd "${tmpdir}" && untar "${TARBALL}")
+  test ! -d "${BINDIR}" && install -d "${BINDIR}"
+  for binexe in $BINARIES; do
+    if [ "$OS" = "windows" ]; then
+      binexe="${binexe}.exe"
+    fi
+    install "${srcdir}/${binexe}" "${BINDIR}/"
+    log_info "installed ${BINDIR}/${binexe}"
+  done
+  rm -rf "${tmpdir}"
+}
+get_binaries() {
+  case "$PLATFORM" in
+    darwin/amd64) BINARIES="golangci-lint" ;;
+    darwin/arm64) BINARIES="golangci-lint" ;;
+    darwin/armv6) BINARIES="golangci-lint" ;;
+    darwin/armv7) BINARIES="golangci-lint" ;;
+    darwin/mips64) BINARIES="golangci-lint" ;;
+    darwin/mips64le) BINARIES="golangci-lint" ;;
+    darwin/ppc64le) BINARIES="golangci-lint" ;;
+    darwin/s390x) BINARIES="golangci-lint" ;;
+    freebsd/386) BINARIES="golangci-lint" ;;
+    freebsd/amd64) BINARIES="golangci-lint" ;;
+    freebsd/armv6) BINARIES="golangci-lint" ;;
+    freebsd/armv7) BINARIES="golangci-lint" ;;
+    freebsd/mips64) BINARIES="golangci-lint" ;;
+    freebsd/mips64le) BINARIES="golangci-lint" ;;
+    freebsd/ppc64le) BINARIES="golangci-lint" ;;
+    freebsd/s390x) BINARIES="golangci-lint" ;;
+    linux/386) BINARIES="golangci-lint" ;;
+    linux/amd64) BINARIES="golangci-lint" ;;
+    linux/arm64) BINARIES="golangci-lint" ;;
+    linux/armv6) BINARIES="golangci-lint" ;;
+    linux/armv7) BINARIES="golangci-lint" ;;
+    linux/mips64) BINARIES="golangci-lint" ;;
+    linux/mips64le) BINARIES="golangci-lint" ;;
+    linux/ppc64le) BINARIES="golangci-lint" ;;
+    linux/s390x) BINARIES="golangci-lint" ;;
+    linux/riscv64) BINARIES="golangci-lint" ;;
+    windows/386) BINARIES="golangci-lint" ;;
+    windows/amd64) BINARIES="golangci-lint" ;;
+    windows/arm64) BINARIES="golangci-lint" ;;
+    windows/armv6) BINARIES="golangci-lint" ;;
+    windows/armv7) BINARIES="golangci-lint" ;;
+    windows/mips64) BINARIES="golangci-lint" ;;
+    windows/mips64le) BINARIES="golangci-lint" ;;
+    windows/ppc64le) BINARIES="golangci-lint" ;;
+    windows/s390x) BINARIES="golangci-lint" ;;
+    *)
+      log_crit "platform $PLATFORM is not supported.  Make sure this script is up-to-date and file request at https://github.com/${PREFIX}/issues/new"
+      exit 1
+      ;;
+  esac
+}
+tag_to_version() {
+  if [ -z "${TAG}" ]; then
+    log_info "checking GitHub for latest tag"
+  else
+    log_info "checking GitHub for tag '${TAG}'"
+  fi
+  REALTAG=$(github_release "$OWNER/$REPO" "${TAG}") && true
+  if test -z "$REALTAG"; then
+    log_crit "unable to find '${TAG}' - use 'latest' or see https://github.com/${PREFIX}/releases for details"
+    exit 1
+  fi
+  # if version starts with 'v', remove it
+  TAG="$REALTAG"
+  VERSION=${TAG#v}
+}
+adjust_format() {
+  # change format (tar.gz or zip) based on OS
+  case ${OS} in
+    windows) FORMAT=zip ;;
+  esac
+  true
+}
+adjust_os() {
+  # adjust archive name based on OS
+  true
+}
+adjust_arch() {
+  # adjust archive name based on ARCH
+  true
+}
+
+cat /dev/null <<EOF
+------------------------------------------------------------------------
+https://github.com/client9/shlib - portable posix shell functions
+Public domain - http://unlicense.org
+https://github.com/client9/shlib/blob/master/LICENSE.md
+but credit (and pull requests) appreciated.
+------------------------------------------------------------------------
+EOF
+is_command() {
+  command -v "$1" >/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 <<EOF
+------------------------------------------------------------------------
+End of functions from https://github.com/client9/shlib
+------------------------------------------------------------------------
+EOF
+
+PROJECT_NAME="golangci-lint"
+OWNER=golangci
+REPO="golangci-lint"
+BINARY=golangci-lint
+FORMAT=tar.gz
+OS=$(uname_os)
+ARCH=$(uname_arch)
+PREFIX="$OWNER/$REPO"
+
+# use in logging routines
+log_prefix() {
+	echo "$PREFIX"
+}
+PLATFORM="${OS}/${ARCH}"
+GITHUB_DOWNLOAD=https://github.com/${OWNER}/${REPO}/releases/download
+
+uname_os_check "$OS"
+uname_arch_check "$ARCH"
+
+parse_args "$@"
+
+get_binaries
+
+tag_to_version
+
+adjust_format
+
+adjust_os
+
+adjust_arch
+
+log_info "found version: ${VERSION} for ${TAG}/${OS}/${ARCH}"
+
+NAME=${BINARY}-${VERSION}-${OS}-${ARCH}
+TARBALL=${NAME}.${FORMAT}
+TARBALL_URL=${GITHUB_DOWNLOAD}/${TAG}/${TARBALL}
+CHECKSUM=${PROJECT_NAME}-${VERSION}-checksums.txt
+CHECKSUM_URL=${GITHUB_DOWNLOAD}/${TAG}/${CHECKSUM}
+
+
+execute