Various bits of cleanup detected when using Go Workspaces

TLDR with many modules the versions included in each diverged quite a bit. Attempting to use Go Workspaces produces a bunch of errors. This commit: 1. Fixes envoy-library-references.sh to work again 2. Ensures we are pulling in [email protected] everywhere (previously it was at that version in some modules and others were much older) 3. Remove one usage of golang/protobuf that caused us to have a direct dependency on it. 4. Remove deprecated usage of the Endpoint field in the grpc resolver.Target struct. The current version of grpc (v1.55.0) has removed that field and recommended replacement with URL.Opaque and calls to the Endpoint() func when needing to consume the previous field. 4. `go work init <all the paths to go.mod files>` && `go work sync`. This syncrhonized versions of dependencies from the main workspace/root module to all submodules 5. Updated .gitignore to ignore the go.work and go.work.sum files. This seems to be standard practice at the moment. 6. Update doc comments in protoc-gen-consul-rate-limit to be go fmt compatible 7. Upgraded makefile infra to perform linting, testing and go mod tidy on all modules in a flexible manner.
hashicorp · Jun 5, 2023 · 8a84638 · 8a84638
1 parent a043981
commit 8a84638
Show file tree

Hide file tree

Showing 75 changed files with 3,755 additions and 687 deletions.
diff --git a/.gitignore b/.gitignore
@@ -66,3 +66,5 @@ override.tf.json
 # Ignore CLI configuration files
 .terraformrc
 terraform.rc
+/go.work
+/go.work.sum
diff --git a/.golangci.yml b/.golangci.yml
@@ -88,6 +88,9 @@ linters-settings:
         - github.com/hashicorp/go-msgpack:
             recommendations:
               - github.com/hashicorp/consul-net-rpc/go-msgpack
+        - github.com/golang/protobuf:
+            recommendations:
+              - google.golang.org/protobuf
 
   depguard:
     list-type: denylist
@@ -101,7 +104,9 @@ linters-settings:
     # Default: []
     packages-with-error-message:
       - net/rpc: "only use forked copy in github.com/hashicorp/consul-net-rpc/net/rpc"
+      - github.com/golang/protobuf: "only use google.golang.org/protobuf"
 
 run:
   timeout: 10m
   concurrency: 4
+  skip-dirs-use-default: false
diff --git a/GNUmakefile b/GNUmakefile
@@ -3,6 +3,8 @@
 
 SHELL = bash
 
+GO_MODULES := $(shell find . -name go.mod -exec dirname {} \; | grep -v "proto-gen-rpc-glue/e2e" | sort)
+
 ###
 # These version variables can either be a valid string for "go install <module>@<version>"
 # or the string @DEV to imply use what is currently installed locally.
@@ -249,19 +251,13 @@ cov: other-consul dev-build
 
 test: other-consul dev-build lint test-internal
 
-go-mod-tidy:
-	@echo "--> Running go mod tidy"
-	@cd sdk && go mod tidy
-	@cd api && go mod tidy
-	@go mod tidy
-	@cd test/integration/consul-container && go mod tidy
-	@cd test/integration/connect/envoy/test-sds-server && go mod tidy
-	@cd proto-public && go mod tidy
-	@cd internal/tools/proto-gen-rpc-glue && go mod tidy
-	@cd internal/tools/proto-gen-rpc-glue/e2e && go mod tidy
-	@cd internal/tools/proto-gen-rpc-glue/e2e/consul && go mod tidy
-	@cd internal/tools/protoc-gen-consul-rate-limit && go mod tidy
+.PHONY: go-mod-tidy
+go-mod-tidy: $(foreach mod,$(GO_MODULES),go-mod-tidy/$(mod))
 
+.PHONY: mod-tidy/%
+go-mod-tidy/%:
+	@echo "--> Running go mod tidy ($*)"
+	@cd $* && go mod tidy
 
 test-internal:
 	@echo "--> Running go test"
@@ -292,6 +288,12 @@ test-internal:
 	@grep '^FAIL' test.log || true
 	@if [ "$$(cat exit-code)" == "0" ] ; then echo "PASS" ; exit 0 ; else exit 1 ; fi
 
+test-all: other-consul dev-build lint $(foreach mod,$(GO_MODULES),test-module/$(mod))
+
+test-module/%:
+	@echo "--> Running go test ($*)"
+	cd $* && go test $(GOTEST_FLAGS) -tags '$(GOTAGS)' ./...
+
 test-race:
 	$(MAKE) GOTEST_FLAGS=-race
 
@@ -328,21 +330,26 @@ other-consul:
 		echo "Found other running consul agents. This may affect your tests." ; \
 		exit 1 ; \
 	fi
-
-lint: -lint-main lint-container-test-deps
-
-.PHONY: -lint-main
--lint-main: lint-tools
-	@echo "--> Running golangci-lint"
-	@golangci-lint run --build-tags '$(GOTAGS)' && \
-		(cd api && golangci-lint run --build-tags '$(GOTAGS)') && \
-		(cd sdk && golangci-lint run --build-tags '$(GOTAGS)')
-	@echo "--> Running golangci-lint (container tests)"
-	@cd test/integration/consul-container && golangci-lint run --build-tags '$(GOTAGS)'
-	@echo "--> Running lint-consul-retry"
-	@lint-consul-retry
-	@echo "--> Running enumcover"
-	@enumcover ./...
+
+.PHONY: fmt
+fmt: $(foreach mod,$(GO_MODULES),fmt/$(mod)) 
+
+.PHONY: fmt/%
+fmt/%:
+	@echo "--> Running go fmt ($*)"
+	@cd $* && go fmt ./...
+
+.PHONY: lint
+lint: $(foreach mod,$(GO_MODULES),lint/$(mod)) lint-container-test-deps
+
+.PHONY: lint/%
+lint/%:
+	@echo "--> Running golangci-lint ($*)"
+	@cd $* && GOWORK=off golangci-lint run --build-tags '$(GOTAGS)'
+	@echo "--> Running lint-consul-retry ($*)"
+	@cd $* && GOWORK=off lint-consul-retry
+	@echo "--> Running enumcover ($*)"
+	@cd $* && GOWORK=off enumcover ./...
 
 .PHONY: lint-container-test-deps
 lint-container-test-deps:

diff --git a/agent/envoyextensions/builtin/ext-authz/structs.go b/agent/envoyextensions/builtin/ext-authz/structs.go
@@ -167,7 +167,7 @@ func (c extAuthzConfig) isHTTP() bool {
 //
 // If the extension is configured with the ext_authz service as an upstream there is no need to insert
 // a new cluster so this method returns nil.
-func (c *extAuthzConfig) toEnvoyCluster(cfg *cmn.RuntimeConfig) (*envoy_cluster_v3.Cluster, error) {
+func (c *extAuthzConfig) toEnvoyCluster(_ *cmn.RuntimeConfig) (*envoy_cluster_v3.Cluster, error) {
 	var target *Target
 	if c.isHTTP() {
 		target = c.HttpService.Target
@@ -678,8 +678,10 @@ type TransportApiVersion string
 func (t TransportApiVersion) toEnvoy() envoy_core_v3.ApiVersion {
 	switch strings.ToLower(string(t)) {
 	case "v2":
+		//nolint:staticcheck
 		return envoy_core_v3.ApiVersion_V2
 	case "auto":
+		//nolint:staticcheck
 		return envoy_core_v3.ApiVersion_AUTO
 	default:
 		return envoy_core_v3.ApiVersion_V3

diff --git a/agent/envoyextensions/builtin/http/localratelimit/ratelimit.go b/agent/envoyextensions/builtin/http/localratelimit/ratelimit.go
@@ -14,10 +14,10 @@ import (
 	envoy_http_v3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3"
 	envoy_type_v3 "github.com/envoyproxy/go-control-plane/envoy/type/v3"
 	envoy_resource_v3 "github.com/envoyproxy/go-control-plane/pkg/resource/v3"
-	"github.com/golang/protobuf/ptypes/wrappers"
 	"github.com/hashicorp/go-multierror"
 	"github.com/mitchellh/mapstructure"
 	"google.golang.org/protobuf/types/known/durationpb"
+	"google.golang.org/protobuf/types/known/wrapperspb"
 
 	"github.com/hashicorp/consul/api"
 	"github.com/hashicorp/consul/envoyextensions/extensioncommon"
@@ -125,7 +125,7 @@ func (r ratelimit) PatchFilter(p extensioncommon.FilterPayload) (*envoy_listener
 	tokenBucket := envoy_type_v3.TokenBucket{}
 
 	if r.TokensPerFill != nil {
-		tokenBucket.TokensPerFill = &wrappers.UInt32Value{
+		tokenBucket.TokensPerFill = &wrapperspb.UInt32Value{
 			Value: uint32(*r.TokensPerFill),
 		}
 	}

diff --git a/agent/envoyextensions/builtin/property-override/property_override_test.go b/agent/envoyextensions/builtin/property-override/property_override_test.go
@@ -559,6 +559,7 @@ type MockPatcher[K proto.Message] struct {
 	appliedPatches []Patch
 }
 
+//nolint:unparam
 func (m *MockPatcher[K]) applyPatch(k K, p Patch, _ bool) (result K, e error) {
 	m.appliedPatches = append(m.appliedPatches, p)
 	return k, nil

diff --git a/agent/envoyextensions/builtin/property-override/structpatcher_test.go b/agent/envoyextensions/builtin/property-override/structpatcher_test.go
@@ -2,18 +2,19 @@ package propertyoverride
 
 import (
 	"fmt"
+	"testing"
+
 	envoy_cluster_v3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3"
 	corev3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
 	envoy_endpoint_v3 "github.com/envoyproxy/go-control-plane/envoy/config/endpoint/v3"
 	envoy_listener_v3 "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3"
 	envoy_route_v3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3"
-	_struct "github.com/golang/protobuf/ptypes/struct"
 	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/testing/protocmp"
+	_struct "google.golang.org/protobuf/types/known/structpb"
 	"google.golang.org/protobuf/types/known/wrapperspb"
-	"testing"
 )
 
 func TestPatchStruct(t *testing.T) {

diff --git a/agent/grpc-internal/resolver/resolver.go b/agent/grpc-internal/resolver/resolver.go
@@ -96,7 +96,7 @@ func (s *ServerResolverBuilder) Build(target resolver.Target, cc resolver.Client
 	}
 
 	//nolint:staticcheck
-	serverType, datacenter, err := parseEndpoint(target.Endpoint)
+	serverType, datacenter, err := parseEndpoint(target.Endpoint())
 	if err != nil {
 		return nil, err
 	}

diff --git a/agent/grpc-internal/resolver/resolver_test.go b/agent/grpc-internal/resolver/resolver_test.go
@@ -3,6 +3,7 @@ package resolver
 import (
 	"fmt"
 	"net"
+	"net/url"
 	"strings"
 	"testing"
 
@@ -40,7 +41,7 @@ func TestServerResolverBuilder(t *testing.T) {
 		_, err := rs.Build(resolver.Target{
 			Scheme:    "consul",
 			Authority: rs.Authority(),
-			Endpoint:  endpoint,
+			URL:       url.URL{Opaque: endpoint},
 		}, cc, resolver.BuildOptions{})
 		require.NoError(t, err)
 

diff --git a/agent/grpc-middleware/testutil/testservice/simple.pb.go b/agent/grpc-middleware/testutil/testservice/simple.pb.go