Skip to content

Commit

Permalink
Various bits of cleanup detected when using Go Workspaces
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mkeeler committed May 25, 2023
1 parent a90c9ce commit 9d66009
Show file tree
Hide file tree
Showing 33 changed files with 3,710 additions and 638 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,5 @@ override.tf.json
# Ignore CLI configuration files
.terraformrc
terraform.rc
/go.work
/go.work.sum
5 changes: 5 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
61 changes: 34 additions & 27 deletions GNUmakefile
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,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"
Expand Down Expand Up @@ -134,7 +134,7 @@ func (p ratelimit) PatchFilter(_ *extensioncommon.RuntimeConfig, filter *envoy_l
tokenBucket := envoy_type_v3.TokenBucket{}

if p.TokensPerFill != nil {
tokenBucket.TokensPerFill = &wrappers.UInt32Value{
tokenBucket.TokensPerFill = &wrapperspb.UInt32Value{
Value: uint32(*p.TokensPerFill),
}
}
Expand Down
2 changes: 1 addition & 1 deletion agent/grpc-internal/resolver/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
3 changes: 2 additions & 1 deletion agent/grpc-internal/resolver/resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package resolver
import (
"fmt"
"net"
"net/url"
"strings"
"testing"

Expand Down Expand Up @@ -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)

Expand Down
Loading

0 comments on commit 9d66009

Please sign in to comment.