Merge pull request #1641 from bshaaban/bump_go

chore: bump Go to 1.17.13

Dockerfile

@@ -1,4 +1,4 @@
-FROM golang:1.16.9
+FROM golang:1.17.13
 
 RUN apt-get update && apt-get install -y \
 	curl \

Makefile

@@ -84,33 +84,43 @@ ${PREFIX}/bin/static/notary:
 	@go build -tags "${NOTARY_BUILDTAGS} netgo" -o $@ ${GO_LDFLAGS_STATIC} ./cmd/notary
 endif
 
+ifeq (, $(shell which staticcheck))
+STATICCHECK_BIN := $(GOBIN)/staticcheck
+$(STATICCHECK_BIN):
+	@echo "+ $@"
+	GOFLAGS="-mod=mod" go install honnef.co/go/tools/cmd/staticcheck@latest
+endif
 
-# run all lint functionality - excludes Godep directory, vendoring, binaries, python tests, and git files
-lint:
-	@echo "+ $@: golint, go vet, go fmt, gocycle, misspell, ineffassign"
-	# golint
-	@test -z "$(shell find . -type f -name "*.go" -not -path "./vendor/*" -not -name "*.pb.*" -exec golint {} \; | tee /dev/stderr)"
-	# gofmt
-	@test -z "$$(gofmt -s -l .| grep -v .pb. | grep -v vendor/ | tee /dev/stderr)"
-	# govet
-ifeq ($(shell uname -s), Darwin)
-	@test -z "$(shell find . -iname *test*.go | grep -v _test.go | grep -v vendor | xargs echo "This file should end with '_test':"  | tee /dev/stderr)"
-else
-	@test -z "$(shell find . -iname *test*.go | grep -v _test.go | grep -v vendor | xargs -r echo "This file should end with '_test':"  | tee /dev/stderr)"
+# spin up a docker instance and run staticcheck inside it
+.PHONY: staticcheck-docker
+staticcheck-docker: $(STATICCHECK_BIN)
+	@$(dockerbuild)
+ifeq ($(RUN_LOCAL),1)
+	staticcheck -checks=all,-ST1000 ./...
 endif
-	@test -z "$$(go vet -printf=false . 2>&1 | grep -v vendor/ | tee /dev/stderr)"
-	# gocyclo - we require cyclomatic complexity to be < 16
-	@test -z "$(shell find . -type f -name "*.go" -not -path "./vendor/*" -not -name "*.pb.*" -exec gocyclo -over 15 {} \; | tee /dev/stderr)"
-	# misspell - requires that the following be run first:
-	#    go get -u github.com/client9/misspell/cmd/misspell
-	@test -z "$$(find . -type f | grep -v vendor/ | grep -v bin/ | grep -v misc/ | grep -v .git/ | grep -v \.pdf | xargs misspell | tee /dev/stderr)"
-	# ineffassign - requires that the following be run first:
-	#    go get -u github.com/gordonklaus/ineffassign
-	@test -z "$(shell find . -type f -name "*.go" -not -path "./vendor/*" -not -name "*.pb.*" -exec ineffassign {} \; | tee /dev/stderr)"
-	# gosec - requires that the following be run first:
-	#    go get -u github.com/securego/gosec/cmd/gosec/...
-	@rm -f gosec_output.csv
-	@gosec -fmt=csv -out=gosec_output.csv -exclude=G104,G304 ./... || (cat gosec_output.csv >&2; exit 1)
+
+.PHONY: staticcheck
+staticcheck: $(STATICCHECK_BIN)
+	staticcheck -checks=all,-ST1000 ./...
+
+ifneq ($(RUN_LOCAL),1)
+dockerbuild = @DOCKER_BUILDKIT=1 docker build \
+			-f build.Dockerfile \
+			--build-arg target=$@ \
+			--target=builder \
+			.
+dockertestbuild = @DOCKER_BUILDKIT=1 docker build \
+			-f build.Dockerfile \
+			--build-arg target=$@ \
+			--target=test-builder \
+			.
+endif
+
+# run lint locally
+lint: staticcheck
+
+# run lint target in docker
+lint-docker: staticcheck-docker
 
 build:
 	@echo "+ $@"
@@ -125,6 +135,17 @@ test:
 	@echo
 	go test -tags "${NOTARY_BUILDTAGS}" $(TESTOPTS) $(PKGS)
 
+# run test target in docker
+test-docker: TESTOPTS =
+test-docker:
+	@$(dockertestbuild)
+ifeq ($(RUN_LOCAL),1)
+	@echo Note: when testing with a yubikey plugged in, make sure to include 'TESTOPTS="-p 1"'
+	@echo "+ $@ $(TESTOPTS)"
+	@echo
+	go test -tags "${NOTARY_BUILDTAGS}" $(TESTOPTS) $(PKGS)
+endif
+
 integration: TESTDB = mysql
 integration: clean
 	buildscripts/integrationtest.sh $(TESTDB)
@@ -134,7 +155,7 @@ testdb:
 	buildscripts/dbtests.sh $(TESTDB)
 
 protos:
-	@protoc --go_out=plugins=grpc:. proto/*.proto
+	@protoc --go_out=. --go-grpc_out=. proto/*.proto
 
 # This allows coverage for a package to come from tests in different package.
 # Requires that the following:

build.Dockerfile

@@ -0,0 +1,38 @@
+FROM golang:1.17.13-alpine3.16 as builder-base
+RUN apk add make bash git openssh build-base curl
+
+#
+# STAGE - Build stage, calls make with given target argument (defaults to all make target)
+#
+FROM builder-base as builder
+ARG target=all
+ENV RUN_LOCAL=1
+RUN mkdir -p /go/src
+ADD . /go/src/
+WORKDIR /go/src
+RUN make $target
+
+#
+# STAGE - Test Build stage, calls make with given target argument (defaults to all make target). Valid for testing purposes only as tests require a specific (non-root) user access for directories read/write access.
+#
+FROM builder-base as test-builder
+ARG target=all
+ENV GROUP=test-group
+ENV USER=test-user
+ENV UID=12345
+ENV GID=23456
+ENV RUN_LOCAL=1
+RUN addgroup -S $GROUP
+RUN adduser \
+    --disabled-password \
+    --gecos "" \
+    --home "$(pwd)" \
+    --ingroup "$GROUP" \
+    --no-create-home \
+    --uid "$UID" \
+    "$USER"
+USER $USER
+RUN mkdir -p /go/src
+ADD . /go/src/
+WORKDIR /go/src
+RUN make $target

buildscripts/circle-validate-vendor.sh

@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
 
-go_version=1.16.9
+go_version=1.17.13
 
 docker run --rm --env GO111MODULE=on -w /notary --volume ${PWD}:/notary \
     golang:${go_version}-alpine \

client/changelist/file_changelist_test.go

@@ -50,6 +50,7 @@ func TestErrorConditions(t *testing.T) {
 	defer os.RemoveAll(tmpDir)
 
 	cl, err := NewFileChangelist(tmpDir)
+	require.NoError(t, err)
 	// Attempt to unmarshall a bad JSON file. Note: causes a WARN on the console.
 	ioutil.WriteFile(filepath.Join(tmpDir, "broken_file.change"), []byte{5}, 0644)
 	noItems := cl.List()
@@ -163,7 +164,9 @@ func TestFileChangeIterator(t *testing.T) {
 	}
 
 	// negative test case: changelist directory does not exist
-	os.RemoveAll(tmpDir)
-	it, err = cl.NewIterator()
+	err = os.RemoveAll(tmpDir)
+	require.NoError(t, err)
+
+	_, err = cl.NewIterator()
 	require.Error(t, err, "Initializing iterator without underlying file store")
 }

client/client.go

@@ -45,7 +45,6 @@ type repository struct {
 	cryptoService  signed.CryptoService
 	tufRepo        *tuf.Repo
 	invalid        *tuf.Repo // known data that was parsable but deemed invalid
-	roundTrip      http.RoundTripper
 	trustPinning   trustpinning.TrustPinConfig
 	LegacyVersions int // number of versions back to fetch roots to sign with
 }
@@ -363,7 +362,7 @@ func (r *repository) Initialize(rootKeyIDs []string, serverManagedRoles ...data.
 type errKeyNotFound struct{}
 
 func (errKeyNotFound) Error() string {
-	return fmt.Sprintf("cannot find matching private key id")
+	return "cannot find matching private key id"
 }
 
 // keyExistsInList returns the id of the private key in ids that matches the public key

client/client_test.go

@@ -1191,6 +1191,7 @@ func fakeServerData(t *testing.T, repo *repository, mux *http.ServeMux,
 	rootJSONFile := filepath.Join(baseDir, "tuf",
 		filepath.FromSlash(repo.gun.String()), "metadata", "root.json")
 	rootFileBytes, err := ioutil.ReadFile(rootJSONFile)
+	require.NoError(t, err)
 
 	signedTargets, err := savedTUFRepo.SignTargets(
 		"targets", data.DefaultExpires("targets"))
@@ -1760,9 +1761,9 @@ func testPublishNoData(t *testing.T, rootType string, clearCache, serverManagesS
 		serverManagesSnapshot)
 	defer os.RemoveAll(baseDir1)
 
-	var rec *passRoleRecorder
+	rec := newRoleRecorder()
+
 	if clearCache {
-		rec = newRoleRecorder()
 		repo1, rec, _ = newRepoToTestRepo(t, repo1, baseDir1)
 	}
 
@@ -1851,9 +1852,9 @@ func testPublishWithData(t *testing.T, rootType string, clearCache, serverManage
 		serverManagesSnapshot)
 	defer os.RemoveAll(baseDir)
 
-	var rec *passRoleRecorder
+	rec := newRoleRecorder()
+
 	if clearCache {
-		rec = newRoleRecorder()
 		repo, rec, _ = newRepoToTestRepo(t, repo, baseDir)
 	}
 

client/client_update_test.go

@@ -993,7 +993,7 @@ func waysToMessUpServerNonRootPerRole(t *testing.T) map[string][]swizzleExpectat
 		perRoleSwizzling[data.CanonicalSnapshotRole.String()] = append(
 			perRoleSwizzling[data.CanonicalSnapshotRole.String()],
 			swizzleExpectations{
-				desc:       fmt.Sprintf("snapshot missing root meta checksum"),
+				desc:       "snapshot missing root meta checksum",
 				expectErrs: []interface{}{data.ErrInvalidMetadata{}},
 				swizzle: func(s *testutils.MetadataSwizzler, role data.RoleName) error {
 					return s.MutateSnapshot(func(sn *data.Snapshot) {
@@ -1003,7 +1003,7 @@ func waysToMessUpServerNonRootPerRole(t *testing.T) map[string][]swizzleExpectat
 			})
 	}
 	perRoleSwizzling[data.CanonicalTargetsRole.String()] = []swizzleExpectations{{
-		desc:       fmt.Sprintf("target missing delegations data"),
+		desc:       "target missing delegations data",
 		expectErrs: []interface{}{data.ErrMismatchedChecksum{}},
 		swizzle: func(s *testutils.MetadataSwizzler, role data.RoleName) error {
 			return s.MutateTargets(func(tg *data.Targets) {
@@ -1012,7 +1012,7 @@ func waysToMessUpServerNonRootPerRole(t *testing.T) map[string][]swizzleExpectat
 		},
 	}}
 	perRoleSwizzling[data.CanonicalTimestampRole.String()] = []swizzleExpectations{{
-		desc:       fmt.Sprintf("timestamp missing snapshot meta checksum"),
+		desc:       "timestamp missing snapshot meta checksum",
 		expectErrs: []interface{}{data.ErrInvalidMetadata{}},
 		swizzle: func(s *testutils.MetadataSwizzler, role data.RoleName) error {
 			return s.MutateTimestamp(func(ts *data.Timestamp) {
@@ -1021,7 +1021,7 @@ func waysToMessUpServerNonRootPerRole(t *testing.T) map[string][]swizzleExpectat
 		},
 	}}
 	perRoleSwizzling["targets/a"] = []swizzleExpectations{{
-		desc:       fmt.Sprintf("delegation has invalid role"),
+		desc:       "delegation has invalid role",
 		expectErrs: []interface{}{data.ErrInvalidMetadata{}},
 		swizzle: func(s *testutils.MetadataSwizzler, role data.RoleName) error {
 			return s.MutateTargets(func(tg *data.Targets) {
@@ -1057,10 +1057,9 @@ func TestUpdateNonRootRemoteCorruptedNoLocalCache(t *testing.T) {
 		t.Skip("skipping test in short mode")
 	}
 
-	for _, role := range append(data.BaseRoles) {
+	for _, role := range data.BaseRoles {
 		switch role {
 		case data.CanonicalRootRole:
-			break
 		default:
 			for _, testData := range waysToMessUpServer {
 				testUpdateRemoteCorruptValidChecksum(t, updateOpts{
@@ -1201,7 +1200,6 @@ func TestUpdateNonRootRemoteCorruptedCannotUseLocalCache(t *testing.T) {
 	for _, role := range data.BaseRoles {
 		switch role {
 		case data.CanonicalRootRole:
-			break
 		default:
 			for _, testData := range waysToMessUpServer {
 				testUpdateRemoteCorruptValidChecksum(t, updateOpts{

client/delegations.go

@@ -212,11 +212,11 @@ func translateDelegationsToCanonicalIDs(delegationInfo data.Delegations) ([]data
 		for _, keyID := range delegation.KeyIDs {
 			pubKey, ok := delegationKeys[keyID]
 			if !ok {
-				return []data.Role{}, fmt.Errorf("Could not translate canonical key IDs for %s", delegation.Name)
+				return []data.Role{}, fmt.Errorf("could not translate canonical key IDs for %s", delegation.Name)
 			}
 			canonicalKeyID, err := utils.CanonicalKeyID(pubKey)
 			if err != nil {
-				return []data.Role{}, fmt.Errorf("Could not translate canonical key IDs for %s: %v", delegation.Name, err)
+				return []data.Role{}, fmt.Errorf("could not translate canonical key IDs for %s: %v", delegation.Name, err)
 			}
 			canonicalKeyIDs = append(canonicalKeyIDs, canonicalKeyID)
 		}

client/tufclient.go

@@ -233,7 +233,6 @@ func (c *tufClient) downloadTargets() error {
 				return err
 			}
 			logrus.Warnf("Error getting %s: %s", role.Name, err)
-			break
 		case nil:
 			toDownload = append(children, toDownload...)
 		default:

cmd/notary-server/config.go

@@ -74,7 +74,7 @@ func grpcTLS(configuration *viper.Viper) (*tls.Config, error) {
 	})
 	if err != nil {
 		return nil, fmt.Errorf(
-			"Unable to configure TLS to the trust service: %s", err.Error())
+			"unable to configure TLS to the trust service: %s", err.Error())
 	}
 	return tlsConfig, nil
 }
@@ -96,7 +96,7 @@ func getStore(configuration *viper.Viper, hRegister healthRegister, doBootstrap
 		}
 		s, err := storage.NewSQLStorage(storeConfig.Backend, storeConfig.Source)
 		if err != nil {
-			return nil, fmt.Errorf("Error starting %s driver: %s", backend, err.Error())
+			return nil, fmt.Errorf("error starting %s driver: %s", backend, err.Error())
 		}
 		store = *storage.NewTUFMetaStorage(s)
 		hRegister("DB operational", 10*time.Second, s.CheckHealth)
@@ -118,7 +118,7 @@ func getStore(configuration *viper.Viper, hRegister healthRegister, doBootstrap
 			sess, err = rethinkdb.UserConnection(tlsOpts, storeConfig.Source, storeConfig.Username, storeConfig.Password)
 		}
 		if err != nil {
-			return nil, fmt.Errorf("Error starting %s driver: %s", backend, err.Error())
+			return nil, fmt.Errorf("error starting %s driver: %s", backend, err.Error())
 		}
 		s := storage.NewRethinkDBStorage(storeConfig.DBName, storeConfig.Username, storeConfig.Password, sess)
 		store = *storage.NewTUFMetaStorage(s)

cmd/notary-server/main.go

@@ -86,7 +86,6 @@ func main() {
 	if err != nil {
 		logrus.Fatal(err.Error())
 	}
-	return
 }
 
 func usage() {

cmd/notary-server/main_test.go

@@ -279,7 +279,7 @@ func TestGetTrustServiceTLSFailure(t *testing.T) {
 
 	require.Error(t, err)
 	require.True(t, strings.Contains(err.Error(),
-		"Unable to configure TLS to the trust service"))
+		"unable to configure TLS to the trust service"))
 
 	// no health function ever registered
 	require.Equal(t, 0, registerCalled)

cmd/notary-signer/config.go

@@ -135,7 +135,7 @@ func setUpCryptoservices(configuration *viper.Viper, allowedBackends []string, d
 			sess, err = rethinkdb.UserConnection(tlsOpts, storeConfig.Source, storeConfig.Username, storeConfig.Password)
 		}
 		if err != nil {
-			return nil, fmt.Errorf("Error starting %s driver: %s", backend, err.Error())
+			return nil, fmt.Errorf("error starting %s driver: %w", backend, err)
 		}
 		s := keydbstore.NewRethinkDBKeyStore(storeConfig.DBName, storeConfig.Username, storeConfig.Password, passphraseRetriever, defaultAlias, sess)
 		health.RegisterPeriodicFunc("DB operational", time.Minute, s.CheckHealth)

cmd/notary/delegations.go

@@ -83,12 +83,12 @@ func (d *delegationCommander) GetCommand() *cobra.Command {
 func (d *delegationCommander) delegationPurgeKeys(cmd *cobra.Command, args []string) error {
 	if len(args) != 1 {
 		cmd.Usage()
-		return fmt.Errorf("Please provide a single Global Unique Name as an argument to remove")
+		return fmt.Errorf("please provide a single Global Unique Name as an argument to remove")
 	}
 
 	if len(d.keyIDs) == 0 {
 		cmd.Usage()
-		return fmt.Errorf("Please provide at least one key ID to be removed using the --key flag")
+		return fmt.Errorf("please provide at least one key ID to be removed using the --key flag")
 	}
 
 	gun := data.GUN(args[0])
@@ -132,7 +132,7 @@ func (d *delegationCommander) delegationsList(cmd *cobra.Command, args []string)
 	if len(args) != 1 {
 		cmd.Usage()
 		return fmt.Errorf(
-			"Please provide a Global Unique Name as an argument to list")
+			"please provide a Global Unique Name as an argument to list")
 	}
 
 	config, err := d.configGetter()
@@ -161,7 +161,7 @@ func (d *delegationCommander) delegationsList(cmd *cobra.Command, args []string)
 
 	delegationRoles, err := nRepo.GetDelegationRoles()
 	if err != nil {
-		return fmt.Errorf("Error retrieving delegation roles for repository %s: %v", gun, err)
+		return fmt.Errorf("error retrieving delegation roles for repository %s: %w", gun, err)
 	}
 
 	cmd.Println("")