From 43b85963efce73f1708d712873e779fdc8cb3ae4 Mon Sep 17 00:00:00 2001 From: Fabian Holler Date: Fri, 18 Oct 2024 14:32:05 +0200 Subject: [PATCH 1/5] editorconfig: fix: invalid value for indent_style It must be "tab" not "tabs". --- .editorconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.editorconfig b/.editorconfig index 4138aeb..c4b1877 100644 --- a/.editorconfig +++ b/.editorconfig @@ -8,7 +8,7 @@ insert_final_newline = true trim_trailing_whitespace = true indent_size = 8 -indent_style = tabs +indent_style = tab max_line_length = 80 [*.go] From 760fb99b44e869ebeb8ccbef778f71c49c58db41 Mon Sep 17 00:00:00 2001 From: Fabian Holler Date: Fri, 18 Oct 2024 14:35:10 +0200 Subject: [PATCH 2/5] go: increase minimum version to 1.22.8 --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 793b1d8..b9b6b6d 100644 --- a/go.mod +++ b/go.mod @@ -21,4 +21,4 @@ require ( google.golang.org/protobuf v1.28.1 // indirect ) -go 1.18 +go 1.22.8 From bf1201ea87e68de64a95597a97b37cc8ca401cc4 Mon Sep 17 00:00:00 2001 From: Fabian Holler Date: Fri, 18 Oct 2024 14:38:01 +0200 Subject: [PATCH 3/5] golangci-lint: update to version 1.61 --- .github/workflows/ci.yml | 8 ++++---- .golangci.yml | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d1d5dd3..1b88881 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,7 +12,7 @@ jobs: test: strategy: matrix: - go: ["1.18", "1.20"] + go: ["1.22", "1.23"] runs-on: ubuntu-latest steps: - uses: actions/setup-go@v3 @@ -28,7 +28,7 @@ jobs: build: strategy: matrix: - go: ["1.18", "1.20"] + go: ["1.22", "1.23"] runs-on: ubuntu-latest steps: - uses: actions/setup-go@v3 @@ -47,9 +47,9 @@ jobs: steps: - uses: actions/setup-go@v3 with: - go-version: "1.20" + go-version: "1.23" - uses: actions/checkout@v3 - name: golangci-lint uses: golangci/golangci-lint-action@v3 with: - version: v1.51 + version: v1.61 diff --git a/.golangci.yml b/.golangci.yml index 974f50b..8cbb137 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,12 +1,13 @@ linters: disable-all: true enable: + - copyloopvar - errcheck - - exportloopref - gocritic - goimports - goprintffuncname - gosimple + - govet - ineffassign - misspell - nolintlint @@ -18,7 +19,6 @@ linters: - unconvert - unused - usestdlibvars - - vet linters-settings: goimports: From fe43954de17a68a01c83cc48c0140dd96c53b006 Mon Sep 17 00:00:00 2001 From: Fabian Holler Date: Fri, 18 Oct 2024 14:40:38 +0200 Subject: [PATCH 4/5] fix: unused parameter warnings from revive linter --- consul/resolver.go | 2 +- consul/resolver_test.go | 13 ++++++------- internal/mocks/consulhealth.go | 2 +- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/consul/resolver.go b/consul/resolver.go index 9981476..89a9f53 100644 --- a/consul/resolver.go +++ b/consul/resolver.go @@ -254,7 +254,7 @@ func (c *consulResolver) watcher() { } } -func (c *consulResolver) ResolveNow(o resolver.ResolveNowOptions) { +func (c *consulResolver) ResolveNow(resolver.ResolveNowOptions) { select { case c.resolveNow <- struct{}{}: diff --git a/consul/resolver_test.go b/consul/resolver_test.go index 39928d2..ce1ff0f 100644 --- a/consul/resolver_test.go +++ b/consul/resolver_test.go @@ -245,7 +245,7 @@ func TestResolve(t *testing.T) { health := mocks.NewConsulHealthClient() cleanup := replaceCreateHealthClientFn( - func(cfg *consul.Config) (consulHealthEndpoint, error) { + func(*consul.Config) (consulHealthEndpoint, error) { return health, nil }, ) @@ -284,7 +284,7 @@ func TestResolve(t *testing.T) { func TestResolveNewAddressOnlyCalledOnChange(t *testing.T) { health := mocks.NewConsulHealthClient() cleanup := replaceCreateHealthClientFn( - func(cfg *consul.Config) (consulHealthEndpoint, error) { + func(*consul.Config) (consulHealthEndpoint, error) { return health, nil }, ) @@ -331,7 +331,7 @@ func TestResolveNewAddressOnlyCalledOnChange(t *testing.T) { func TestResolveAddrChange(t *testing.T) { health := mocks.NewConsulHealthClient() cleanup := replaceCreateHealthClientFn( - func(cfg *consul.Config) (consulHealthEndpoint, error) { + func(*consul.Config) (consulHealthEndpoint, error) { return health, nil }, ) @@ -420,7 +420,7 @@ func TestResolveAddrChange(t *testing.T) { func TestResolveAddrChangesToUnresolvable(t *testing.T) { health := mocks.NewConsulHealthClient() cleanup := replaceCreateHealthClientFn( - func(cfg *consul.Config) (consulHealthEndpoint, error) { + func(*consul.Config) (consulHealthEndpoint, error) { return health, nil }, ) @@ -490,7 +490,7 @@ func TestErrorIsReportedOnQueryErrors(t *testing.T) { health.SetRespError(queryErr) cleanup := replaceCreateHealthClientFn( - func(cfg *consul.Config) (consulHealthEndpoint, error) { + func(*consul.Config) (consulHealthEndpoint, error) { return health, nil }, ) @@ -509,7 +509,6 @@ func TestErrorIsReportedOnQueryErrors(t *testing.T) { for ; err == nil; err = cc.LastReportedError() { time.Sleep(time.Millisecond) - } if err != queryErr { @@ -522,7 +521,7 @@ func TestQueryResultsAreSorted(t *testing.T) { newAddressCallCnt := cc.UpdateStateCallCnt() health := mocks.NewConsulHealthClient() cleanup := replaceCreateHealthClientFn( - func(cfg *consul.Config) (consulHealthEndpoint, error) { + func(*consul.Config) (consulHealthEndpoint, error) { return health, nil }, ) diff --git a/internal/mocks/consulhealth.go b/internal/mocks/consulhealth.go index 670725e..541643a 100644 --- a/internal/mocks/consulhealth.go +++ b/internal/mocks/consulhealth.go @@ -43,7 +43,7 @@ func (c *ConsulHealthClient) SetRespError(err error) { c.err = err } -func (c *ConsulHealthClient) ServiceMultipleTags(service string, tags []string, passingOnly bool, q *consul.QueryOptions) ([]*consul.ServiceEntry, *consul.QueryMeta, error) { +func (c *ConsulHealthClient) ServiceMultipleTags(_ string, _ []string, _ bool, q *consul.QueryOptions) ([]*consul.ServiceEntry, *consul.QueryMeta, error) { c.mutex.Lock() defer c.mutex.Unlock() From f5e1c1a79d10d32ff52f121045adfa5b09945897 Mon Sep 17 00:00:00 2001 From: Fabian Holler Date: Fri, 18 Oct 2024 14:49:43 +0200 Subject: [PATCH 5/5] golangci-lint: enable additional linters and fix found issues Enable errorlint, godox, nosprintfhostport and gofumpt. gofumpt replaces the goimports linter. --- .golangci.yml | 13 +++++++++--- consul/resolver.go | 5 ++--- consul/resolver_test.go | 45 ++++++++++++++++++++--------------------- 3 files changed, 34 insertions(+), 29 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 8cbb137..11e78a9 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -3,14 +3,17 @@ linters: enable: - copyloopvar - errcheck + - errorlint - gocritic - - goimports + - godox + - gofumpt - goprintffuncname - gosimple - govet - ineffassign - misspell - nolintlint + - nosprintfhostport - prealloc - revive - staticcheck @@ -21,5 +24,9 @@ linters: - usestdlibvars linters-settings: - goimports: - local-prefixes: github.com/simplesurance/grpcconsulresolver + gofumpt: + module-path: github.com/simplesurance/grpcconsulresolver + extra-rules: true + godox: + keywords: + - FIXME diff --git a/consul/resolver.go b/consul/resolver.go index 89a9f53..5ffbce3 100644 --- a/consul/resolver.go +++ b/consul/resolver.go @@ -9,7 +9,6 @@ import ( "sync" "time" - "github.com/hashicorp/consul/api" consul "github.com/hashicorp/consul/api" "google.golang.org/grpc/grpclog" "google.golang.org/grpc/resolver" @@ -66,7 +65,7 @@ func newConsulResolver( health, err := consulCreateHealthClientFn(&cfg) if err != nil { - return nil, fmt.Errorf("creating consul client failed. %v", err) + return nil, fmt.Errorf("creating consul client failed: %w", err) } ctx, cancel := context.WithCancel(context.Background()) @@ -133,7 +132,7 @@ func filterPreferOnlyHealthy(entries []*consul.ServiceEntry) []*consul.ServiceEn healthy := make([]*consul.ServiceEntry, 0, len(entries)) for _, e := range entries { - if e.Checks.AggregatedStatus() == api.HealthPassing { + if e.Checks.AggregatedStatus() == consul.HealthPassing { healthy = append(healthy, e) } } diff --git a/consul/resolver_test.go b/consul/resolver_test.go index ce1ff0f..6b20953 100644 --- a/consul/resolver_test.go +++ b/consul/resolver_test.go @@ -7,7 +7,6 @@ import ( "testing" "time" - "github.com/hashicorp/consul/api" consul "github.com/hashicorp/consul/api" "google.golang.org/grpc/resolver" @@ -97,13 +96,13 @@ func TestResolve(t *testing.T) { target: resolver.Target{URL: url.URL{Path: "user-service"}}, consulResponse: []*consul.ServiceEntry{ { - Service: &api.AgentService{ + Service: &consul.AgentService{ Address: "localhost", Port: 5678, }, }, { - Service: &api.AgentService{ + Service: &consul.AgentService{ Address: "remotehost", Port: 1234, }, @@ -151,46 +150,46 @@ func TestResolve(t *testing.T) { target: resolver.Target{URL: url.URL{Path: "credit-service", RawQuery: "health=fallbackToUnhealthy"}}, consulResponse: []*consul.ServiceEntry{ { - Service: &api.AgentService{ + Service: &consul.AgentService{ Address: "localhost", Port: 5678, }, - Checks: api.HealthChecks{ + Checks: consul.HealthChecks{ { - Status: api.HealthPassing, + Status: consul.HealthPassing, }, }, }, { - Service: &api.AgentService{ + Service: &consul.AgentService{ Address: "remotehost", Port: 9, }, - Checks: api.HealthChecks{ + Checks: consul.HealthChecks{ { - Status: api.HealthPassing, + Status: consul.HealthPassing, }, }, }, { - Service: &api.AgentService{ + Service: &consul.AgentService{ Address: "unhealthyHost", Port: 1234, }, - Checks: api.HealthChecks{ + Checks: consul.HealthChecks{ { - Status: api.HealthCritical, + Status: consul.HealthCritical, }, }, }, { - Service: &api.AgentService{ + Service: &consul.AgentService{ Address: "warnedHost", Port: 1, }, - Checks: api.HealthChecks{ + Checks: consul.HealthChecks{ { - Status: api.HealthWarning, + Status: consul.HealthWarning, }, }, }, @@ -210,24 +209,24 @@ func TestResolve(t *testing.T) { target: resolver.Target{URL: url.URL{Path: "web-service", RawQuery: "health=fallbackToUnhealthy"}}, consulResponse: []*consul.ServiceEntry{ { - Service: &api.AgentService{ + Service: &consul.AgentService{ Address: "localhost", Port: 5678, }, - Checks: api.HealthChecks{ + Checks: consul.HealthChecks{ { - Status: api.HealthCritical, + Status: consul.HealthCritical, }, }, }, { - Service: &api.AgentService{ + Service: &consul.AgentService{ Address: "remotehost", Port: 1234, }, - Checks: api.HealthChecks{ + Checks: consul.HealthChecks{ { - Status: api.HealthCritical, + Status: consul.HealthCritical, }, }, }, @@ -511,8 +510,8 @@ func TestErrorIsReportedOnQueryErrors(t *testing.T) { time.Sleep(time.Millisecond) } - if err != queryErr { - t.Fatalf("resolver error is %+v, expected %+v", err, queryErr) + if !errors.Is(err, queryErr) { + t.Fatalf("resolver error is: '%+v', expected: '%+v'", err, queryErr) } }