From 593372f77774dd753293ed068eb065e08f9cbf4f Mon Sep 17 00:00:00 2001 From: Sreeram Narayanan Date: Thu, 28 Aug 2025 16:42:21 +0530 Subject: [PATCH 1/5] fix: golangci-lint issues --- acl/acl_ce.go | 4 +--- acl/enterprisemeta_ce.go | 2 +- agent/acl_endpoint.go | 5 +---- agent/acl_endpoint_test.go | 5 +---- agent/agent.go | 6 ++---- agent/checks/check_test.go | 13 ++++++------- agent/config/config_ce.go | 2 +- agent/connect/ca/provider_aws_test.go | 8 ++++---- .../services/resource/server_ce_test.go | 8 ++------ agent/structs/acl_ce.go | 8 ++------ agent/structs/aclfilter/filter.go | 6 ++---- agent/structs/config_entry_jwt_provider_test.go | 16 ++++++++-------- agent/structs/intention_ce.go | 6 +++--- agent/structs/structs_ce.go | 14 +++++++------- command/snapshot/inspect/formatter.go | 4 ++-- internal/resource/http/http_test.go | 2 +- 16 files changed, 44 insertions(+), 65 deletions(-) diff --git a/acl/acl_ce.go b/acl/acl_ce.go index 0d207ad4211a..d20195d85d3f 100644 --- a/acl/acl_ce.go +++ b/acl/acl_ce.go @@ -32,6 +32,4 @@ type EnterpriseConfig struct { // no fields in CE } -func (_ *EnterpriseConfig) Close() { - // do nothing -} +func (*EnterpriseConfig) Close() {} diff --git a/acl/enterprisemeta_ce.go b/acl/enterprisemeta_ce.go index 4636b36c8eac..0b43477e3689 100644 --- a/acl/enterprisemeta_ce.go +++ b/acl/enterprisemeta_ce.go @@ -117,7 +117,7 @@ func NewEnterpriseMetaWithPartition(_, _ string) EnterpriseMeta { } // FillAuthzContext stub -func (_ *EnterpriseMeta) FillAuthzContext(_ *AuthorizerContext) {} +func (*EnterpriseMeta) FillAuthzContext(_ *AuthorizerContext) {} func NormalizeNamespace(_ string) string { return "" diff --git a/agent/acl_endpoint.go b/agent/acl_endpoint.go index ac773c59b443..b546a70f058e 100644 --- a/agent/acl_endpoint.go +++ b/agent/acl_endpoint.go @@ -26,10 +26,7 @@ var aclDisabled = HTTPError{StatusCode: http.StatusUnauthorized, Reason: "ACL su // checkACLDisabled will return a standard response if ACLs are disabled. This // returns true if they are disabled and we should not continue. func (s *HTTPHandlers) checkACLDisabled() bool { - if s.agent.config.ACLsEnabled { - return false - } - return true + return !(s.agent.config.ACLsEnabled) } // ACLBootstrap is used to perform a one-time ACL bootstrap operation on diff --git a/agent/acl_endpoint_test.go b/agent/acl_endpoint_test.go index 7b484e092b5b..8cc08b39f818 100644 --- a/agent/acl_endpoint_test.go +++ b/agent/acl_endpoint_test.go @@ -34,10 +34,7 @@ import ( func isHTTPBadRequest(err error) bool { if err, ok := err.(HTTPError); ok { - if err.StatusCode != 400 { - return false - } - return true + return err.StatusCode == http.StatusBadRequest } return false } diff --git a/agent/agent.go b/agent/agent.go index de7e5ac58389..e6549e10d563 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -4216,10 +4216,8 @@ func revertStaticConfig(oldCfg tlsutil.ProtocolConfig, newCfg tlsutil.ProtocolCo newOldcfg := newCfg newOldcfg.CertFile = oldCfg.CertFile newOldcfg.KeyFile = oldCfg.KeyFile - if !reflect.DeepEqual(newOldcfg, oldCfg) { - return true - } - return false + + return !reflect.DeepEqual(newOldcfg, oldCfg) } // reloadConfigInternal is mainly needed for some unit tests. Instead of parsing diff --git a/agent/checks/check_test.go b/agent/checks/check_test.go index 4bbac1edda3a..9e61c5f010ad 100644 --- a/agent/checks/check_test.go +++ b/agent/checks/check_test.go @@ -1198,11 +1198,10 @@ func mockUDPServer(ctx context.Context, network string, port int) { } }() - select { - case <-ctx.Done(): - fmt.Println("cancelled") - close(chClose) - } + <-ctx.Done() + fmt.Println("cancelled") + close(chClose) + wg.Wait() } @@ -1366,7 +1365,7 @@ func TestCheckH2PING(t *testing.T) { } func TestCheckH2PING_TLS_BadVerify(t *testing.T) { - handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { return }) + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}) server := httptest.NewUnstartedServer(handler) server.EnableHTTP2 = true server.StartTLS() @@ -1468,7 +1467,7 @@ func TestCheckH2CPING(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { return }) + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}) h2chandler := h2c.NewHandler(handler, &http2.Server{}) server := httptest.NewUnstartedServer(h2chandler) server.Config.ReadTimeout = tt.connTimeout diff --git a/agent/config/config_ce.go b/agent/config/config_ce.go index a60a49be25db..dfbdf5c09cd1 100644 --- a/agent/config/config_ce.go +++ b/agent/config/config_ce.go @@ -13,6 +13,6 @@ import ( // EnterpriseMeta stub type EnterpriseMeta struct{} -func (_ *EnterpriseMeta) ToStructs() acl.EnterpriseMeta { +func (*EnterpriseMeta) ToStructs() acl.EnterpriseMeta { return *structs.DefaultEnterpriseMetaInDefaultPartition() } diff --git a/agent/connect/ca/provider_aws_test.go b/agent/connect/ca/provider_aws_test.go index db96d3f71949..eb7e113b852d 100644 --- a/agent/connect/ca/provider_aws_test.go +++ b/agent/connect/ca/provider_aws_test.go @@ -112,7 +112,7 @@ func TestAWSBootstrapAndSignSecondary(t *testing.T) { rootPEM, err := p1.GenerateCAChain() require.NoError(t, err) - p2 := testAWSProvider(t, testProviderConfigSecondary(t, nil)) + p2 := testAWSProvider(t, testProviderConfigSecondary(nil)) defer p2.Cleanup(true, nil) testSignIntermediateCrossDC(t, p1, p2) @@ -202,7 +202,7 @@ func TestAWSBootstrapAndSignSecondary(t *testing.T) { newIntPEM := strings.TrimSuffix(intPEM, "\n") newRootPEM := strings.TrimSuffix(rootPEM, "\n") - cfg2 := testProviderConfigSecondary(t, map[string]interface{}{ + cfg2 := testProviderConfigSecondary(map[string]interface{}{ "ExistingARN": p2State[AWSStateCAARNKey], }) p2 = testAWSProvider(t, cfg2) @@ -232,7 +232,7 @@ func TestAWSBootstrapAndSignSecondaryConsul(t *testing.T) { _, err := p1.GenerateCAChain() require.NoError(t, err) - p2 := testAWSProvider(t, testProviderConfigSecondary(t, nil)) + p2 := testAWSProvider(t, testProviderConfigSecondary(nil)) defer p2.Cleanup(true, nil) testSignIntermediateCrossDC(t, p1, p2) @@ -437,7 +437,7 @@ func testProviderConfigPrimary(cfg map[string]interface{}) ProviderConfig { } } -func testProviderConfigSecondary(t *testing.T, cfg map[string]interface{}) ProviderConfig { +func testProviderConfigSecondary(cfg map[string]interface{}) ProviderConfig { c := testProviderConfigPrimary(cfg) c.IsPrimary = false c.Datacenter = "dc2" diff --git a/agent/grpc-external/services/resource/server_ce_test.go b/agent/grpc-external/services/resource/server_ce_test.go index f48ff3b52b99..538693d5ae19 100644 --- a/agent/grpc-external/services/resource/server_ce_test.go +++ b/agent/grpc-external/services/resource/server_ce_test.go @@ -7,10 +7,6 @@ package resource_test import "github.com/hashicorp/consul/acl" -func fillEntMeta(entMeta *acl.EnterpriseMeta) { - return -} +func fillEntMeta(_ *acl.EnterpriseMeta) {} -func fillAuthorizerContext(authzContext *acl.AuthorizerContext) { - return -} +func fillAuthorizerContext(_ *acl.AuthorizerContext) {} diff --git a/agent/structs/acl_ce.go b/agent/structs/acl_ce.go index 166558ae65af..f57bb27b0ce3 100644 --- a/agent/structs/acl_ce.go +++ b/agent/structs/acl_ce.go @@ -54,11 +54,11 @@ type ACLAuthMethodEnterpriseFields struct{} type ACLAuthMethodEnterpriseMeta struct{} -func (_ *ACLAuthMethodEnterpriseMeta) FillWithEnterpriseMeta(_ *acl.EnterpriseMeta) { +func (*ACLAuthMethodEnterpriseMeta) FillWithEnterpriseMeta(_ *acl.EnterpriseMeta) { // do nothing } -func (_ *ACLAuthMethodEnterpriseMeta) ToEnterpriseMeta() *acl.EnterpriseMeta { +func (*ACLAuthMethodEnterpriseMeta) ToEnterpriseMeta() *acl.EnterpriseMeta { return DefaultEnterpriseMetaInDefaultPartition() } @@ -66,10 +66,6 @@ func aclServiceIdentityRules(svc string, _ *acl.EnterpriseMeta) string { return fmt.Sprintf(aclPolicyTemplateServiceIdentity, svc) } -func aclNodeIdentityRules(node string, _ *acl.EnterpriseMeta) string { - return fmt.Sprintf(aclPolicyTemplateNodeIdentity, node) -} - func (p *ACLPolicy) EnterprisePolicyMeta() *acl.EnterprisePolicyMeta { return nil } diff --git a/agent/structs/aclfilter/filter.go b/agent/structs/aclfilter/filter.go index 13728b7f6ded..c86656a6afcb 100644 --- a/agent/structs/aclfilter/filter.go +++ b/agent/structs/aclfilter/filter.go @@ -178,10 +178,8 @@ func (f *Filter) allowGateway(gs *structs.GatewayService) bool { } gs.Service.FillAuthzContext(&authzContext) - if !f.allowService(gs.Service.Name, &authzContext) { - return false - } - return true + + return f.allowService(gs.Service.Name, &authzContext) } // allowService is used to determine if a service is accessible for an ACL. diff --git a/agent/structs/config_entry_jwt_provider_test.go b/agent/structs/config_entry_jwt_provider_test.go index 9076960c668f..b82c98020793 100644 --- a/agent/structs/config_entry_jwt_provider_test.go +++ b/agent/structs/config_entry_jwt_provider_test.go @@ -20,8 +20,8 @@ func newTestAuthz(t *testing.T, src string) acl.Authorizer { return authorizer } -var tenSeconds = 10 * time.Second -var hundredSeconds time.Duration = 100 * time.Second +var shortInterval = 10 * time.Second +var longInterval = 100 * time.Second var connectTimeout = time.Duration(5) * time.Second func TestJWTProviderConfigEntry_ValidateAndNormalize(t *testing.T) { @@ -215,8 +215,8 @@ func TestJWTProviderConfigEntry_ValidateAndNormalize(t *testing.T) { URI: "https://example.com/.well-known/jwks.json", RetryPolicy: &JWKSRetryPolicy{ RetryPolicyBackOff: &RetryPolicyBackOff{ - BaseInterval: hundredSeconds, - MaxInterval: tenSeconds, + BaseInterval: longInterval, + MaxInterval: shortInterval, }, }, }, @@ -318,8 +318,8 @@ func TestJWTProviderConfigEntry_ValidateAndNormalize(t *testing.T) { URI: "https://example.com/.well-known/jwks.json", RetryPolicy: &JWKSRetryPolicy{ RetryPolicyBackOff: &RetryPolicyBackOff{ - BaseInterval: tenSeconds, - MaxInterval: hundredSeconds, + BaseInterval: shortInterval, + MaxInterval: longInterval, }, }, JWKSCluster: &JWKSCluster{ @@ -359,8 +359,8 @@ func TestJWTProviderConfigEntry_ValidateAndNormalize(t *testing.T) { URI: "https://example.com/.well-known/jwks.json", RetryPolicy: &JWKSRetryPolicy{ RetryPolicyBackOff: &RetryPolicyBackOff{ - BaseInterval: tenSeconds, - MaxInterval: hundredSeconds, + BaseInterval: shortInterval, + MaxInterval: longInterval, }, }, JWKSCluster: &JWKSCluster{ diff --git a/agent/structs/intention_ce.go b/agent/structs/intention_ce.go index d417a63f5964..e668f6b2b6ce 100644 --- a/agent/structs/intention_ce.go +++ b/agent/structs/intention_ce.go @@ -32,21 +32,21 @@ func (e *IntentionQueryExact) DestinationEnterpriseMeta() *acl.EnterpriseMeta { // FillAuthzContext can fill in an acl.AuthorizerContext object to setup // extra parameters for ACL enforcement. In CE there is currently nothing // extra to be done. -func (_ *Intention) FillAuthzContext(_ *acl.AuthorizerContext, _ bool) { +func (*Intention) FillAuthzContext(_ *acl.AuthorizerContext, _ bool) { // do nothing } // FillAuthzContext can fill in an acl.AuthorizerContext object to setup // extra parameters for ACL enforcement. In CE there is currently nothing // extra to be done. -func (_ *IntentionMatchEntry) FillAuthzContext(_ *acl.AuthorizerContext) { +func (*IntentionMatchEntry) FillAuthzContext(_ *acl.AuthorizerContext) { // do nothing } // FillAuthzContext can fill in an acl.AuthorizerContext object to setup // extra parameters for ACL enforcement. In CE there is currently nothing // extra to be done. -func (_ *IntentionQueryCheck) FillAuthzContext(_ *acl.AuthorizerContext) { +func (*IntentionQueryCheck) FillAuthzContext(_ *acl.AuthorizerContext) { // do nothing } diff --git a/agent/structs/structs_ce.go b/agent/structs/structs_ce.go index 2fa039032c23..3d3060f33427 100644 --- a/agent/structs/structs_ce.go +++ b/agent/structs/structs_ce.go @@ -67,19 +67,19 @@ func (n *Node) OverridePartition(_ string) { n.Partition = "" } -func (_ *Coordinate) FillAuthzContext(_ *acl.AuthorizerContext) {} +func (*Coordinate) FillAuthzContext(_ *acl.AuthorizerContext) {} func (n *NodeInfo) FillAuthzContext(ctx *acl.AuthorizerContext) { ctx.Peer = n.PeerName } // FillAuthzContext stub -func (_ *DirEntry) FillAuthzContext(_ *acl.AuthorizerContext) {} +func (*DirEntry) FillAuthzContext(_ *acl.AuthorizerContext) {} // FillAuthzContext stub -func (_ *RegisterRequest) FillAuthzContext(_ *acl.AuthorizerContext) {} +func (*RegisterRequest) FillAuthzContext(_ *acl.AuthorizerContext) {} -func (_ *RegisterRequest) GetEnterpriseMeta() *acl.EnterpriseMeta { +func (*RegisterRequest) GetEnterpriseMeta() *acl.EnterpriseMeta { return nil } @@ -87,10 +87,10 @@ func (_ *RegisterRequest) GetEnterpriseMeta() *acl.EnterpriseMeta { func (op *TxnNodeOp) FillAuthzContext(ctx *acl.AuthorizerContext) {} // CE Stub -func (_ *TxnServiceOp) FillAuthzContext(_ *acl.AuthorizerContext) {} +func (*TxnServiceOp) FillAuthzContext(_ *acl.AuthorizerContext) {} // CE Stub -func (_ *TxnCheckOp) FillAuthzContext(_ *acl.AuthorizerContext) {} +func (*TxnCheckOp) FillAuthzContext(_ *acl.AuthorizerContext) {} func NodeNameString(node string, _ *acl.EnterpriseMeta) string { return node @@ -136,7 +136,7 @@ func (cid CheckID) String() string { return string(cid.ID) } -func (_ *HealthCheck) Validate() error { +func (*HealthCheck) Validate() error { return nil } diff --git a/command/snapshot/inspect/formatter.go b/command/snapshot/inspect/formatter.go index 53da998b25c8..cd55eac0e765 100644 --- a/command/snapshot/inspect/formatter.go +++ b/command/snapshot/inspect/formatter.go @@ -41,7 +41,7 @@ func NewFormatter(format string) (Formatter, error) { } } -func (_ *prettyFormatter) Format(info *OutputFormat) (string, error) { +func (*prettyFormatter) Format(info *OutputFormat) (string, error) { var b bytes.Buffer tw := tabwriter.NewWriter(&b, 8, 8, 6, ' ', 0) @@ -85,7 +85,7 @@ func newJSONFormatter() Formatter { return &jsonFormatter{} } -func (_ *jsonFormatter) Format(info *OutputFormat) (string, error) { +func (*jsonFormatter) Format(info *OutputFormat) (string, error) { b, err := json.MarshalIndent(info, "", " ") if err != nil { return "", fmt.Errorf("Failed to marshal original snapshot stats: %v", err) diff --git a/internal/resource/http/http_test.go b/internal/resource/http/http_test.go index aae02c694ac8..4eb1b39f757a 100644 --- a/internal/resource/http/http_test.go +++ b/internal/resource/http/http_test.go @@ -56,7 +56,7 @@ func TestResourceHandler_InputValidation(t *testing.T) { Scope: resource.ScopeNamespace, }, client, - func(req *http.Request, token *string) { return }, + func(req *http.Request, token *string) {}, hclog.NewNullLogger(), } From e7dda4a1e7c3cefcdb7831e71fe95cb7cf6f5ee6 Mon Sep 17 00:00:00 2001 From: Sreeram Narayanan Date: Thu, 28 Aug 2025 16:42:52 +0530 Subject: [PATCH 2/5] migrate golangci-lint config to v2 --- .golangci.yml | 224 ++++++++++++++++++++++++-------------------------- 1 file changed, 109 insertions(+), 115 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index d858cabb6169..5c7619fd0af1 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,124 +1,118 @@ # Copyright (c) HashiCorp, Inc. # SPDX-License-Identifier: BUSL-1.1 +version: "2" +run: + concurrency: 4 linters: - disable-all: true + default: none enable: - - gofmt + - depguard + - forbidigo + - gomodguard - govet - - unconvert - - staticcheck - ineffassign + - staticcheck + - unconvert - unparam - - forbidigo - - gomodguard - - depguard - -issues: - # Disable the default exclude list so that all excludes are explicitly - # defined in this file. - exclude-use-default: false - - exclude-rules: - # Temp Ignore SA9004: only the first constant in this group has an explicit type - # https://staticcheck.io/docs/checks#SA9004 - - linters: [staticcheck] - text: "SA9004:" - - - linters: [staticcheck] - text: "SA1006:" - - - linters: [staticcheck] - text: 'SA1019: "io/ioutil" has been deprecated since Go 1.16' - - # Allow usage of deprecated values. - - linters: [ staticcheck ] - text: 'SA1019:' - path: "(agent/grpc-external|agent/grpc-internal)" - - # An argument that always receives the same value is often not a problem. - - linters: [unparam] - text: "always receives" - - - linters: [ unparam ] - text: 'result \d+ \(bool\) is always false' - - # Often functions will implement an interface that returns an error without - # needing to return an error. Sometimes the error return value is unnecessary - # but a linter can not tell the difference. - - linters: [unparam] - text: 'result \d+ \(error\) is always nil' - - # Allow unused parameters to start with an underscore. Arguments with a name - # of '_' are already ignored. - # Ignoring longer names that start with underscore allow for better - # self-documentation than a single underscore by itself. Underscore arguments - # should generally only be used when a function is implementing an interface. - - linters: [unparam] - text: "`_[^`]*` is unused" - - # Temp ignore some common unused parameters so that unparam can be added - # incrementally. - - linters: [unparam] - text: "`(t|resp|req|entMeta)` is unused" - - # Temp ignore everything in _ce(_test).go and _ent(_test).go. Many of these - # could use underscore to ignore the unused arguments, but the "always returns" - # issue will likely remain in CE, and will need to be excluded. - - linters: [unparam] - path: "(_ce.go|_ce_test.go|_ent.go|_ent_test.go)" - - - linters: [ staticcheck ] - text: 'SA1019:' - -linters-settings: - govet: - enable-all: true - disable: - - fieldalignment - - nilness - - shadow - - unusedwrite - - printf - gofmt: - simplify: true - forbidigo: - # Forbid the following identifiers (list of regexp). - # Format includes custom message based on https://github.com/ashanbrown/forbidigo/pull/11 - forbid: - - '\bhtml\/template\b(# Use text/template instead)?' - - '\bioutil\b(# Use io and os packages instead of ioutil)?' - - '\brequire\.New\b(# Use package-level functions with explicit TestingT)?' - - '\bassert\.New\b(# Use package-level functions with explicit TestingT)?' - # Exclude godoc examples from forbidigo checks. - # Default: true - exclude_godoc_examples: false - gomodguard: - blocked: - # List of blocked modules. - modules: - # Blocked module. - - github.com/hashicorp/net-rpc-msgpackrpc: - recommendations: - - github.com/hashicorp/consul-net-rpc/net-rpc-msgpackrpc - - github.com/hashicorp/go-msgpack: - recommendations: - - github.com/hashicorp/consul-net-rpc/go-msgpack - - github.com/golang/protobuf: - recommendations: - - google.golang.org/protobuf - - depguard: + settings: + depguard: + rules: + main: + deny: + - pkg: net/rpc + desc: only use forked copy in github.com/hashicorp/consul-net-rpc/net/rpc + - pkg: github.com/golang/protobuf + desc: only use google.golang.org/protobuf + forbidigo: + forbid: + - pattern: \bhtml\/template\b(# Use text/template instead)? + - pattern: \bioutil\b(# Use io and os packages instead of ioutil)? + - pattern: \brequire\.New\b(# Use package-level functions with explicit TestingT)? + - pattern: \bassert\.New\b(# Use package-level functions with explicit TestingT)? + gomodguard: + blocked: + modules: + - github.com/hashicorp/net-rpc-msgpackrpc: + recommendations: + - github.com/hashicorp/consul-net-rpc/net-rpc-msgpackrpc + - github.com/hashicorp/go-msgpack: + recommendations: + - github.com/hashicorp/consul-net-rpc/go-msgpack + - github.com/golang/protobuf: + recommendations: + - google.golang.org/protobuf + govet: + disable: + - fieldalignment + - nilness + - shadow + - unusedwrite + - printf + enable-all: true + exclusions: + generated: lax rules: - main: - listMode: lax - deny: - - pkg: net/rpc - desc: "only use forked copy in github.com/hashicorp/consul-net-rpc/net/rpc" - - pkg: github.com/golang/protobuf - desc: "only use google.golang.org/protobuf" - -run: - timeout: 10m - concurrency: 4 - skip-dirs-use-default: false + - linters: + - staticcheck + text: 'SA9004:' + - linters: + - staticcheck + text: 'SA1006:' + - linters: + - staticcheck + text: 'SA1019: "io/ioutil" has been deprecated since Go 1.16' + - linters: + - staticcheck + path: (agent/grpc-external|agent/grpc-internal) + text: 'SA1019:' + - linters: + - unparam + text: always receives + - linters: + - unparam + text: result \d+ \(bool\) is always false + - linters: + - unparam + text: result \d+ \(error\) is always nil + - linters: + - unparam + text: '`_[^`]*` is unused' + - linters: + - unparam + text: '`(t|resp|req|entMeta)` is unused' + - linters: + - unparam + path: (_ce.go|_ce_test.go|_ent.go|_ent_test.go) + - linters: + - staticcheck + text: 'SA1019:' + - linters: + - staticcheck + text: 'ST1005: error strings should not end with punctuation or newlines' + - linters: + - staticcheck + text: 'ST1012: error var injectedError should have name of the form errFo' + - linters: + - staticcheck + text: 'ST1005: error strings should not be capitalized' + - linters: + - staticcheck + text: 'ST1012: error var' + paths: + - third_party$ + - builtin$ + - examples$ + - agent/consul +formatters: + enable: + - gofmt + settings: + gofmt: + simplify: true + exclusions: + generated: lax + paths: + - third_party$ + - builtin$ + - examples$ \ No newline at end of file From b1e76ef28573ef9217f4abf349dc8b720764d072 Mon Sep 17 00:00:00 2001 From: Sreeram Narayanan Date: Thu, 28 Aug 2025 17:01:30 +0530 Subject: [PATCH 3/5] upgrade: golangci-lint to v2.4.0 --- .github/workflows/reusable-lint.yml | 2 +- Makefile | 2 +- build-support/scripts/devtools.sh | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/reusable-lint.yml b/.github/workflows/reusable-lint.yml index 836a0bd3f60d..595cf1b4d083 100644 --- a/.github/workflows/reusable-lint.yml +++ b/.github/workflows/reusable-lint.yml @@ -54,7 +54,7 @@ jobs: - name: Set golangci-lint version run: echo "GOLANGCI_LINT_VERSION=$(make --no-print-directory print-GOLANGCI_LINT_VERSION)" >> $GITHUB_ENV - name: lint-${{ matrix.directory }} - uses: golangci/golangci-lint-action@82d40c283aeb1f2b6595839195e95c2d6a49081b # v5.0.0 + uses: golangci/golangci-lint-action@4afd733a84b1f43292c63897423277bb7f4313a9 # v8.0.0 with: working-directory: ${{ matrix.directory }} version: ${{ env.GOLANGCI_LINT_VERSION }} diff --git a/Makefile b/Makefile index bb8245d74196..8aae39254250 100644 --- a/Makefile +++ b/Makefile @@ -10,7 +10,7 @@ GO_MODULES := $(shell find . -name go.mod -exec dirname {} \; | grep -v "proto-g # These version variables can either be a valid string for "go install @" # or the string @DEV to imply use what is currently installed locally. ### -GOLANGCI_LINT_VERSION='v1.64.8' +GOLANGCI_LINT_VERSION='v2.4.0' MOCKERY_VERSION='v3.5.2' BUF_VERSION='v1.26.0' diff --git a/build-support/scripts/devtools.sh b/build-support/scripts/devtools.sh index b12a813b7e44..b5391771732d 100755 --- a/build-support/scripts/devtools.sh +++ b/build-support/scripts/devtools.sh @@ -164,9 +164,9 @@ function lint_install { install_versioned_tool \ 'golangci-lint' \ - 'github.com/golangci/golangci-lint' \ + 'github.com/golangci/golangci-lint/v2' \ "${golangci_lint_version}" \ - 'github.com/golangci/golangci-lint/cmd/golangci-lint' + 'github.com/golangci/golangci-lint/v2/cmd/golangci-lint' install_versioned_tool \ 'gci' \ From 762f735f125fc70908c1d4b2e410aedf1f586e44 Mon Sep 17 00:00:00 2001 From: Sreeram Narayanan Date: Thu, 28 Aug 2025 17:06:45 +0530 Subject: [PATCH 4/5] fix: lint issue in iptables_executor_linux.go --- sdk/iptables/iptables_executor_linux.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/iptables/iptables_executor_linux.go b/sdk/iptables/iptables_executor_linux.go index ad954e82552c..8963cafe4f80 100644 --- a/sdk/iptables/iptables_executor_linux.go +++ b/sdk/iptables/iptables_executor_linux.go @@ -41,7 +41,7 @@ func (i *iptablesExecutor) ApplyRules() error { cmd.Stderr = &cmdOutput err := cmd.Run() if err != nil { - return fmt.Errorf("failed to run command: %s, err: %v, output: %s", cmd.String(), err, string(cmdOutput.Bytes())) + return fmt.Errorf("failed to run command: %s, err: %v, output: %s", cmd.String(), err, cmdOutput.String()) } } From 6fe730b23233bc6120677a6e84218a0ee5005e48 Mon Sep 17 00:00:00 2001 From: Sreeram Narayanan Date: Thu, 28 Aug 2025 17:27:06 +0530 Subject: [PATCH 5/5] fix: linter issues --- api/agent_test.go | 2 +- api/api.go | 6 +- api/api_test.go | 3 +- api/catalog_test.go | 30 +++------- api/mock_api_test.go | 2 +- api/namespace.go | 8 +-- api/peering.go | 4 +- api/raw_test.go | 3 +- api/watch/watch.go | 18 +++--- .../extensioncommon/resources_test.go | 56 +++++++++---------- sdk/testutil/io.go | 4 +- sdk/testutil/server.go | 2 +- .../peering_commontopo/ac1_basic_test.go | 12 ++-- .../ac3_service_defaults_upstream_test.go | 4 +- .../ac4_proxy_defaults_test.go | 4 +- .../ac5_2_pq_failover_test.go | 4 +- test-integ/peering_commontopo/commontopo.go | 18 +++--- test-integ/topoutil/asserter.go | 9 ++- .../test/envoy_extensions/wasm_test.go | 9 ++- .../test/gateways/http_route_test.go | 2 +- testing/deployer/topology/topology.go | 6 +- troubleshoot/ports/troubleshoot_ports_test.go | 4 +- troubleshoot/validate/validate.go | 8 +-- 23 files changed, 97 insertions(+), 121 deletions(-) diff --git a/api/agent_test.go b/api/agent_test.go index 067564f04e38..9a3e6a67084f 100644 --- a/api/agent_test.go +++ b/api/agent_test.go @@ -1462,7 +1462,7 @@ func TestAPI_AgentMonitor(t *testing.T) { // Wait for the first log message and validate it select { case log := <-logCh: - if !(strings.Contains(log, "[INFO]") || strings.Contains(log, "[DEBUG]")) { + if !strings.Contains(log, "[INFO]") && !strings.Contains(log, "[DEBUG]") { r.Fatalf("bad: %q", log) } case <-time.After(10 * time.Second): diff --git a/api/api.go b/api/api.go index cd63605a2789..31319390a484 100644 --- a/api/api.go +++ b/api/api.go @@ -646,9 +646,7 @@ func (c *Client) Headers() http.Header { ret := make(http.Header) for k, v := range c.headers { - for _, val := range v { - ret[k] = append(ret[k], val) - } + ret[k] = append(ret[k], v...) } return ret @@ -1312,7 +1310,7 @@ func generateUnexpectedResponseCodeError(resp *http.Response) error { io.Copy(&buf, resp.Body) closeResponseBody(resp) - trimmed := strings.TrimSpace(string(buf.Bytes())) + trimmed := strings.TrimSpace(buf.String()) return StatusError{Code: resp.StatusCode, Body: trimmed} } diff --git a/api/api_test.go b/api/api_test.go index e1d361f32824..8176fddb7125 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -719,8 +719,7 @@ func TestAPI_ClientTLSOptions(t *testing.T) { _, err = client.Agent().Self() // Check for one of the possible cert error messages // See https://cs.opensource.google/go/go/+/62a994837a57a7d0c58bb364b580a389488446c9 - if err == nil || !(strings.Contains(err.Error(), "tls: bad certificate") || - strings.Contains(err.Error(), "tls: certificate required")) { + if err == nil || (!strings.Contains(err.Error(), "tls: bad certificate") && !strings.Contains(err.Error(), "tls: certificate required")) { t.Fatalf("expected tls certificate error, but got '%v'", err) } }) diff --git a/api/catalog_test.go b/api/catalog_test.go index 708392c42b57..30f1169d15c1 100644 --- a/api/catalog_test.go +++ b/api/catalog_test.go @@ -321,7 +321,7 @@ func TestAPI_CatalogServiceUnmanagedProxy(t *testing.T) { catalog := c.Catalog() - proxyReg := testUnmanagedProxyRegistration(t) + proxyReg := testUnmanagedProxyRegistration() retry.Run(t, func(r *retry.R) { _, err := catalog.Register(proxyReg, nil) @@ -530,7 +530,7 @@ func TestAPI_CatalogService_Filter(t *testing.T) { } -func testUpstreams(t *testing.T) []Upstream { +func testUpstreams() []Upstream { return []Upstream{ { DestinationName: "db", @@ -547,21 +547,9 @@ func testUpstreams(t *testing.T) []Upstream { } } -func testExpectUpstreamsWithDefaults(t *testing.T, upstreams []Upstream) []Upstream { - ups := make([]Upstream, len(upstreams)) - for i := range upstreams { - ups[i] = upstreams[i] - // Fill in default fields we expect to have back explicitly in a response - if ups[i].DestinationType == "" { - ups[i].DestinationType = UpstreamDestTypeService - } - } - return ups -} - // testUnmanagedProxy returns a fully configured external proxy service suitable // for checking that all the config fields make it back in a response intact. -func testUnmanagedProxy(t *testing.T) *AgentService { +func testUnmanagedProxy() *AgentService { return &AgentService{ Kind: ServiceKindConnectProxy, Proxy: &AgentServiceConnectProxyConfig{ @@ -569,7 +557,7 @@ func testUnmanagedProxy(t *testing.T) *AgentService { DestinationServiceID: "web1", LocalServiceAddress: "127.0.0.2", LocalServicePort: 8080, - Upstreams: testUpstreams(t), + Upstreams: testUpstreams(), Mode: ProxyModeTransparent, TransparentProxy: &TransparentProxyConfig{ OutboundListenerPort: 808, @@ -583,12 +571,12 @@ func testUnmanagedProxy(t *testing.T) *AgentService { // testUnmanagedProxyRegistration returns a *CatalogRegistration for a fully // configured external proxy. -func testUnmanagedProxyRegistration(t *testing.T) *CatalogRegistration { +func testUnmanagedProxyRegistration() *CatalogRegistration { return &CatalogRegistration{ Datacenter: "dc1", Node: "foobar", Address: "192.168.10.10", - Service: testUnmanagedProxy(t), + Service: testUnmanagedProxy(), } } @@ -600,7 +588,7 @@ func TestAPI_CatalogConnect(t *testing.T) { catalog := c.Catalog() // Register service and proxy instances to test against. - proxyReg := testUnmanagedProxyRegistration(t) + proxyReg := testUnmanagedProxyRegistration() proxy := proxyReg.Service @@ -755,7 +743,7 @@ func TestAPI_CatalogNode(t *testing.T) { name, err := c.Agent().NodeName() require.NoError(t, err) - proxyReg := testUnmanagedProxyRegistration(t) + proxyReg := testUnmanagedProxyRegistration() proxyReg.Node = name proxyReg.SkipNodeUpdate = true @@ -806,7 +794,7 @@ func TestAPI_CatalogNodeServiceList(t *testing.T) { name, err := c.Agent().NodeName() require.NoError(t, err) - proxyReg := testUnmanagedProxyRegistration(t) + proxyReg := testUnmanagedProxyRegistration() proxyReg.Node = name proxyReg.SkipNodeUpdate = true diff --git a/api/mock_api_test.go b/api/mock_api_test.go index 127011f4e933..24b0e312ec8b 100644 --- a/api/mock_api_test.go +++ b/api/mock_api_test.go @@ -26,7 +26,7 @@ func setupMockAPI(t *testing.T) (*mockAPI, *Client) { mapi.ts = httptest.NewServer(&mapi) t.Cleanup(func() { mapi.ts.Close() - mapi.Mock.AssertExpectations(t) + mapi.AssertExpectations(t) }) cfg := DefaultConfig() diff --git a/api/namespace.go b/api/namespace.go index 98afd229989d..ac189a8fa84a 100644 --- a/api/namespace.go +++ b/api/namespace.go @@ -85,14 +85,10 @@ func (n *NamespaceACLConfig) UnmarshalJSON(data []byte) error { } if n.PolicyDefaults == nil { - for _, pd := range aux.PolicyDefaultsSnake { - n.PolicyDefaults = append(n.PolicyDefaults, pd) - } + n.PolicyDefaults = append(n.PolicyDefaults, aux.PolicyDefaultsSnake...) } if n.RoleDefaults == nil { - for _, pd := range aux.RoleDefaultsSnake { - n.RoleDefaults = append(n.RoleDefaults, pd) - } + n.RoleDefaults = append(n.RoleDefaults, aux.RoleDefaultsSnake...) } return nil } diff --git a/api/peering.go b/api/peering.go index dd7780f630a6..32ed28edbcd0 100644 --- a/api/peering.go +++ b/api/peering.go @@ -218,7 +218,7 @@ func (p *Peerings) GenerateToken(ctx context.Context, g PeeringGenerateTokenRequ return nil, nil, fmt.Errorf("peer name cannot be empty") } - req := p.c.newRequest("POST", fmt.Sprint("/v1/peering/token")) + req := p.c.newRequest("POST", "/v1/peering/token") req.setWriteOptions(wq) req.ctx = ctx req.obj = g @@ -244,7 +244,7 @@ func (p *Peerings) GenerateToken(ctx context.Context, g PeeringGenerateTokenRequ // TODO(peering): verify this is the ultimate signature we want func (p *Peerings) Establish(ctx context.Context, i PeeringEstablishRequest, wq *WriteOptions) (*PeeringEstablishResponse, *WriteMeta, error) { - req := p.c.newRequest("POST", fmt.Sprint("/v1/peering/establish")) + req := p.c.newRequest("POST", "/v1/peering/establish") req.setWriteOptions(wq) req.ctx = ctx req.obj = i diff --git a/api/raw_test.go b/api/raw_test.go index 3f48dfb736a5..65a4b9cf158a 100644 --- a/api/raw_test.go +++ b/api/raw_test.go @@ -4,7 +4,6 @@ package api import ( - "fmt" "strings" "testing" @@ -34,7 +33,7 @@ func TestAPI_RawV2ExportedServices(t *testing.T) { defer s.Stop() - endpoint := strings.ToLower(fmt.Sprintf("/api/multicluster/v2/exportedservices/e1")) + endpoint := strings.ToLower("/api/multicluster/v2/exportedservices/e1") wResp := &V2WriteResponse{} wReq := &V2WriteRequest{ diff --git a/api/watch/watch.go b/api/watch/watch.go index ea00f8ef0cba..53a631508c82 100644 --- a/api/watch/watch.go +++ b/api/watch/watch.go @@ -244,18 +244,18 @@ func assignValueBool(params map[string]interface{}, name string, out *bool) erro func assignValueStringSlice(params map[string]interface{}, name string, out *[]string) error { if raw, ok := params[name]; ok { var tmp []string - switch raw.(type) { + switch raw := raw.(type) { case string: - tmp = make([]string, 1, 1) - tmp[0] = raw.(string) + tmp = make([]string, 1) + tmp[0] = raw case []string: - l := len(raw.([]string)) - tmp = make([]string, l, l) - copy(tmp, raw.([]string)) + l := len(raw) + tmp = make([]string, l) + copy(tmp, raw) case []interface{}: - l := len(raw.([]interface{})) - tmp = make([]string, l, l) - for i, v := range raw.([]interface{}) { + l := len(raw) + tmp = make([]string, l) + for i, v := range raw { if s, ok := v.(string); ok { tmp[i] = s } else { diff --git a/envoyextensions/extensioncommon/resources_test.go b/envoyextensions/extensioncommon/resources_test.go index 477e66494ce5..b28afbe255a3 100644 --- a/envoyextensions/extensioncommon/resources_test.go +++ b/envoyextensions/extensioncommon/resources_test.go @@ -20,46 +20,46 @@ func TestInsertHTTPFilter(t *testing.T) { errStr string }{ "insert first": { - inputFilters: makeHttpFilters(t, "a", "b", "b", "b", "c"), + inputFilters: makeHttpFilters("a", "b", "b", "b", "c"), insertOptions: InsertOptions{Location: InsertFirst}, filterName: "test.filter", - expectedFilters: makeHttpFilters(t, "test.filter", "a", "b", "b", "b", "c"), + expectedFilters: makeHttpFilters("test.filter", "a", "b", "b", "b", "c"), }, "insert last": { - inputFilters: makeHttpFilters(t, "a", "b", "b", "b", "c"), + inputFilters: makeHttpFilters("a", "b", "b", "b", "c"), insertOptions: InsertOptions{Location: InsertLast}, filterName: "test.filter", - expectedFilters: makeHttpFilters(t, "a", "b", "b", "b", "c", "test.filter"), + expectedFilters: makeHttpFilters("a", "b", "b", "b", "c", "test.filter"), }, "insert before first match": { - inputFilters: makeHttpFilters(t, "a", "b", "b", "b", "c"), + inputFilters: makeHttpFilters("a", "b", "b", "b", "c"), insertOptions: InsertOptions{Location: InsertBeforeFirstMatch, FilterName: "b"}, filterName: "test.filter", - expectedFilters: makeHttpFilters(t, "a", "test.filter", "b", "b", "b", "c"), + expectedFilters: makeHttpFilters("a", "test.filter", "b", "b", "b", "c"), }, "insert after first match": { - inputFilters: makeHttpFilters(t, "a", "b", "b", "b", "c"), + inputFilters: makeHttpFilters("a", "b", "b", "b", "c"), insertOptions: InsertOptions{Location: InsertAfterFirstMatch, FilterName: "b"}, filterName: "test.filter", - expectedFilters: makeHttpFilters(t, "a", "b", "test.filter", "b", "b", "c"), + expectedFilters: makeHttpFilters("a", "b", "test.filter", "b", "b", "c"), }, "insert before last match": { - inputFilters: makeHttpFilters(t, "a", "b", "b", "b", "c"), + inputFilters: makeHttpFilters("a", "b", "b", "b", "c"), insertOptions: InsertOptions{Location: InsertBeforeLastMatch, FilterName: "b"}, filterName: "test.filter", - expectedFilters: makeHttpFilters(t, "a", "b", "b", "test.filter", "b", "c"), + expectedFilters: makeHttpFilters("a", "b", "b", "test.filter", "b", "c"), }, "insert after last match": { - inputFilters: makeHttpFilters(t, "a", "b", "b", "b", "c"), + inputFilters: makeHttpFilters("a", "b", "b", "b", "c"), insertOptions: InsertOptions{Location: InsertAfterLastMatch, FilterName: "b"}, filterName: "test.filter", - expectedFilters: makeHttpFilters(t, "a", "b", "b", "b", "test.filter", "c"), + expectedFilters: makeHttpFilters("a", "b", "b", "b", "test.filter", "c"), }, "insert last after last match": { - inputFilters: makeHttpFilters(t, "a", "b", "b", "b", "c"), + inputFilters: makeHttpFilters("a", "b", "b", "b", "c"), insertOptions: InsertOptions{Location: InsertAfterLastMatch, FilterName: "c"}, filterName: "test.filter", - expectedFilters: makeHttpFilters(t, "a", "b", "b", "b", "c", "test.filter"), + expectedFilters: makeHttpFilters("a", "b", "b", "b", "c", "test.filter"), }, } @@ -95,40 +95,40 @@ func TestInsertFilter(t *testing.T) { errStr string }{ "insert first": { - inputFilters: makeFilters(t, "a", "b", "b", "b", "c"), + inputFilters: makeFilters("a", "b", "b", "b", "c"), insertOptions: InsertOptions{Location: InsertFirst}, filterName: "test.filter", - expectedFilters: makeFilters(t, "test.filter", "a", "b", "b", "b", "c"), + expectedFilters: makeFilters("test.filter", "a", "b", "b", "b", "c"), }, "insert last": { - inputFilters: makeFilters(t, "a", "b", "b", "b", "c"), + inputFilters: makeFilters("a", "b", "b", "b", "c"), insertOptions: InsertOptions{Location: InsertLast}, filterName: "test.filter", - expectedFilters: makeFilters(t, "a", "b", "b", "b", "c", "test.filter"), + expectedFilters: makeFilters("a", "b", "b", "b", "c", "test.filter"), }, "insert before first match": { - inputFilters: makeFilters(t, "a", "b", "b", "b", "c"), + inputFilters: makeFilters("a", "b", "b", "b", "c"), insertOptions: InsertOptions{Location: InsertBeforeFirstMatch, FilterName: "b"}, filterName: "test.filter", - expectedFilters: makeFilters(t, "a", "test.filter", "b", "b", "b", "c"), + expectedFilters: makeFilters("a", "test.filter", "b", "b", "b", "c"), }, "insert after first match": { - inputFilters: makeFilters(t, "a", "b", "b", "b", "c"), + inputFilters: makeFilters("a", "b", "b", "b", "c"), insertOptions: InsertOptions{Location: InsertAfterFirstMatch, FilterName: "b"}, filterName: "test.filter", - expectedFilters: makeFilters(t, "a", "b", "test.filter", "b", "b", "c"), + expectedFilters: makeFilters("a", "b", "test.filter", "b", "b", "c"), }, "insert before last match": { - inputFilters: makeFilters(t, "a", "b", "b", "b", "c"), + inputFilters: makeFilters("a", "b", "b", "b", "c"), insertOptions: InsertOptions{Location: InsertBeforeLastMatch, FilterName: "b"}, filterName: "test.filter", - expectedFilters: makeFilters(t, "a", "b", "b", "test.filter", "b", "c"), + expectedFilters: makeFilters("a", "b", "b", "test.filter", "b", "c"), }, "insert after last match": { - inputFilters: makeFilters(t, "a", "b", "b", "b", "c"), + inputFilters: makeFilters("a", "b", "b", "b", "c"), insertOptions: InsertOptions{Location: InsertAfterLastMatch, FilterName: "b"}, filterName: "test.filter", - expectedFilters: makeFilters(t, "a", "b", "b", "b", "test.filter", "c"), + expectedFilters: makeFilters("a", "b", "b", "b", "test.filter", "c"), }, } @@ -158,7 +158,7 @@ func makeHttpConMgr(t *testing.T, filters []*envoy_http_v3.HttpFilter) *envoy_li return filter } -func makeHttpFilters(t *testing.T, names ...string) []*envoy_http_v3.HttpFilter { +func makeHttpFilters(names ...string) []*envoy_http_v3.HttpFilter { var filters []*envoy_http_v3.HttpFilter for _, name := range names { filters = append(filters, &envoy_http_v3.HttpFilter{Name: name}) @@ -166,7 +166,7 @@ func makeHttpFilters(t *testing.T, names ...string) []*envoy_http_v3.HttpFilter return filters } -func makeFilters(t *testing.T, names ...string) []*envoy_listener_v3.Filter { +func makeFilters(names ...string) []*envoy_listener_v3.Filter { var filters []*envoy_listener_v3.Filter for _, name := range names { filters = append(filters, &envoy_listener_v3.Filter{Name: name}) diff --git a/sdk/testutil/io.go b/sdk/testutil/io.go index 38722cc98159..d6df4c4fd085 100644 --- a/sdk/testutil/io.go +++ b/sdk/testutil/io.go @@ -21,7 +21,7 @@ func TempDir(t TestingTB, name string) string { panic("argument t must be non-nil") } name = t.Name() + "-" + name - name = strings.Replace(name, "/", "_", -1) + name = strings.ReplaceAll(name, "/", "_") d, err := os.MkdirTemp("", name) if err != nil { t.Fatalf("err: %s", err) @@ -47,7 +47,7 @@ func TempFile(t testing.TB, name string) *os.File { panic("argument t must be non-nil") } name = t.Name() + "-" + name - name = strings.Replace(name, "/", "_", -1) + name = strings.ReplaceAll(name, "/", "_") f, err := os.CreateTemp("", name) if err != nil { t.Fatalf("err: %s", err) diff --git a/sdk/testutil/server.go b/sdk/testutil/server.go index a9da8f372725..4a3fee705b86 100644 --- a/sdk/testutil/server.go +++ b/sdk/testutil/server.go @@ -310,7 +310,7 @@ func NewTestServerConfigT(t TestingTB, cb ServerConfigCallback) (*TestServer, er prefix := "consul" if t != nil { // Use test name for tmpdir if available - prefix = strings.Replace(t.Name(), "/", "_", -1) + prefix = strings.ReplaceAll(t.Name(), "/", "_") } tmpdir, err = os.MkdirTemp("", prefix) if err != nil { diff --git a/test-integ/peering_commontopo/ac1_basic_test.go b/test-integ/peering_commontopo/ac1_basic_test.go index 0af6b9b387bd..c09bd3d53772 100644 --- a/test-integ/peering_commontopo/ac1_basic_test.go +++ b/test-integ/peering_commontopo/ac1_basic_test.go @@ -140,12 +140,12 @@ func (s *ac1BasicSuite) setup(t *testing.T, ct *commonTopo) { Partition: ConfigEntryPartition(httpServerSID.Partition), Sources: []*api.SourceIntention{ { - Name: tcpClient.Workload.ID.Name, + Name: tcpClient.ID.Name, Peer: cluPeerName, Action: api.IntentionActionAllow, }, { - Name: httpClient.Workload.ID.Name, + Name: httpClient.ID.Name, Peer: cluPeerName, Action: api.IntentionActionAllow, }, @@ -171,12 +171,12 @@ func (s *ac1BasicSuite) setup(t *testing.T, ct *commonTopo) { Partition: ConfigEntryPartition(tcpServerSID.Partition), Sources: []*api.SourceIntention{ { - Name: tcpClient.Workload.ID.Name, + Name: tcpClient.ID.Name, Peer: cluPeerName, Action: api.IntentionActionAllow, }, { - Name: httpClient.Workload.ID.Name, + Name: httpClient.ID.Name, Peer: cluPeerName, Action: api.IntentionActionAllow, }, @@ -187,9 +187,9 @@ func (s *ac1BasicSuite) setup(t *testing.T, ct *commonTopo) { httpServerNode := ct.AddServiceNode(peerClu, httpServer) tcpServerNode := ct.AddServiceNode(peerClu, tcpServer) - s.sidClientHTTP = httpClient.Workload.ID + s.sidClientHTTP = httpClient.ID s.nodeClientHTTP = httpClientNode.ID() - s.sidClientTCP = tcpClient.Workload.ID + s.sidClientTCP = tcpClient.ID s.nodeClientTCP = tcpClientNode.ID() s.upstreamHTTP = upstreamHTTP s.upstreamTCP = upstreamTCP diff --git a/test-integ/peering_commontopo/ac3_service_defaults_upstream_test.go b/test-integ/peering_commontopo/ac3_service_defaults_upstream_test.go index 5c2767c6ebf5..be1f055603f3 100644 --- a/test-integ/peering_commontopo/ac3_service_defaults_upstream_test.go +++ b/test-integ/peering_commontopo/ac3_service_defaults_upstream_test.go @@ -132,7 +132,7 @@ func (s *ac3SvcDefaultsSuite) setup(t *testing.T, ct *commonTopo) { Partition: ConfigEntryPartition(serverSID.Partition), Sources: []*api.SourceIntention{ { - Name: client.Workload.ID.Name, + Name: client.ID.Name, Peer: cluPeerName, Action: api.IntentionActionAllow, }, @@ -142,7 +142,7 @@ func (s *ac3SvcDefaultsSuite) setup(t *testing.T, ct *commonTopo) { serverNode := ct.AddServiceNode(peerClu, server) - s.sidClient = client.Workload.ID + s.sidClient = client.ID s.nodeClient = clientNode.ID() s.upstream = upstream diff --git a/test-integ/peering_commontopo/ac4_proxy_defaults_test.go b/test-integ/peering_commontopo/ac4_proxy_defaults_test.go index 4eb4c920fb91..e898a69f5e45 100644 --- a/test-integ/peering_commontopo/ac4_proxy_defaults_test.go +++ b/test-integ/peering_commontopo/ac4_proxy_defaults_test.go @@ -112,7 +112,7 @@ func (s *ac4ProxyDefaultsSuite) setup(t *testing.T, ct *commonTopo) { Partition: ConfigEntryPartition(serverSID.Partition), Sources: []*api.SourceIntention{ { - Name: client.Workload.ID.Name, + Name: client.ID.Name, Peer: cluPeerName, Action: api.IntentionActionAllow, }, @@ -124,7 +124,7 @@ func (s *ac4ProxyDefaultsSuite) setup(t *testing.T, ct *commonTopo) { &api.ProxyConfigEntry{ Kind: api.ProxyDefaults, Name: api.ProxyConfigGlobal, - Partition: ConfigEntryPartition(server.Workload.ID.Partition), + Partition: ConfigEntryPartition(server.ID.Partition), Config: map[string]interface{}{ "protocol": "http", "local_request_timeout_ms": 500, diff --git a/test-integ/peering_commontopo/ac5_2_pq_failover_test.go b/test-integ/peering_commontopo/ac5_2_pq_failover_test.go index 8a65fcef8851..dad59242439e 100644 --- a/test-integ/peering_commontopo/ac5_2_pq_failover_test.go +++ b/test-integ/peering_commontopo/ac5_2_pq_failover_test.go @@ -40,12 +40,12 @@ var ac5_2Context = make(map[nodeKey]ac5_2PQFailoverSuite) func TestAC5PreparedQueryFailover(t *testing.T) { ct := newCommonTopo(t, "dc2", true, true) s := &ac5_2PQFailoverSuite{} - s.setup(t, ct) + s.setup(ct) ct.Launch(t) s.test(t, ct) } -func (s *ac5_2PQFailoverSuite) setup(t *testing.T, ct *commonTopo) { +func (s *ac5_2PQFailoverSuite) setup(ct *commonTopo) { s.setupDC(ct, ct.DC1, ct.DC2) s.setupDC(ct, ct.DC2, ct.DC1) s.setupDC3(ct, ct.DC3, ct.DC1, ct.DC2) diff --git a/test-integ/peering_commontopo/commontopo.go b/test-integ/peering_commontopo/commontopo.go index 8554776680f1..c4360932557a 100644 --- a/test-integ/peering_commontopo/commontopo.go +++ b/test-integ/peering_commontopo/commontopo.go @@ -239,10 +239,10 @@ type serviceExt struct { func (ct *commonTopo) AddServiceNode(clu *topology.Cluster, svc serviceExt) *topology.Node { clusterName := clu.Name - if _, ok := ct.services[clusterName][svc.Workload.ID]; ok { - panic(fmt.Sprintf("duplicate service %q in cluster %q", svc.Workload.ID, clusterName)) + if _, ok := ct.services[clusterName][svc.ID]; ok { + panic(fmt.Sprintf("duplicate service %q in cluster %q", svc.ID, clusterName)) } - ct.services[clusterName][svc.Workload.ID] = struct{}{} + ct.services[clusterName][svc.ID] = struct{}{} // TODO: inline serviceHostnameString := func(dc string, id topology.ID) string { @@ -268,14 +268,14 @@ func (ct *commonTopo) AddServiceNode(clu *topology.Cluster, svc serviceExt) *top nodeKind := topology.NodeKindClient // TODO: bug in deployer somewhere; it should guard against a KindDataplane node with // DisableServiceMesh services on it; dataplane is only for service-mesh - if !svc.Workload.DisableServiceMesh && clu.Datacenter == ct.agentlessDC { + if !svc.DisableServiceMesh && clu.Datacenter == ct.agentlessDC { nodeKind = topology.NodeKindDataplane } node := &topology.Node{ Kind: nodeKind, - Name: serviceHostnameString(clu.Datacenter, svc.Workload.ID), - Partition: svc.Workload.ID.Partition, + Name: serviceHostnameString(clu.Datacenter, svc.ID), + Partition: svc.ID.Partition, Addresses: []*topology.Address{ {Network: clu.Datacenter}, }, @@ -288,9 +288,9 @@ func (ct *commonTopo) AddServiceNode(clu *topology.Cluster, svc serviceExt) *top // Export if necessary if len(svc.Exports) > 0 { - ct.ExportService(clu, svc.Workload.ID.Partition, api.ExportedService{ - Name: svc.Workload.ID.Name, - Namespace: svc.Workload.ID.Namespace, + ct.ExportService(clu, svc.ID.Partition, api.ExportedService{ + Name: svc.ID.Name, + Namespace: svc.ID.Namespace, Consumers: svc.Exports, }) } diff --git a/test-integ/topoutil/asserter.go b/test-integ/topoutil/asserter.go index 0ba842ad07a6..eca8f11235b6 100644 --- a/test-integ/topoutil/asserter.go +++ b/test-integ/topoutil/asserter.go @@ -281,7 +281,7 @@ func (a *Asserter) fortioFetch2Upstream( ) (body []byte, res *http.Response) { t.Helper() - err, res := getFortioFetch2UpstreamResponse(t, client, addr, us, path, nil) + res, err := getFortioFetch2UpstreamResponse(t, client, addr, us, path, nil) require.NoError(t, err) defer res.Body.Close() @@ -296,7 +296,7 @@ func (a *Asserter) fortioFetch2Upstream( return body, res } -func getFortioFetch2UpstreamResponse(t testutil.TestingTB, client *http.Client, addr string, us *topology.Upstream, path string, headers map[string]string) (error, *http.Response) { +func getFortioFetch2UpstreamResponse(t testutil.TestingTB, client *http.Client, addr string, us *topology.Upstream, path string, headers map[string]string) (*http.Response, error) { actualURL := fmt.Sprintf("http://localhost:%d/%s", us.LocalPort, path) url := fmt.Sprintf("http://%s/fortio/fetch2?url=%s", addr, @@ -311,7 +311,7 @@ func getFortioFetch2UpstreamResponse(t testutil.TestingTB, client *http.Client, } res, err := client.Do(req) require.NoError(t, err) - return err, res + return res, err } // uses the /fortio/fetch2 endpoint to do a header echo check against an @@ -386,7 +386,7 @@ func (a *Asserter) FortioFetch2ServiceStatusCodes(t *testing.T, fortioWrk *topol ) retry.RunWith(&retry.Timer{Timeout: 60 * time.Second, Wait: time.Millisecond * 500}, t, func(r *retry.R) { - _, res := getFortioFetch2UpstreamResponse(r, client, addr, us, path, headers) + res, _ := getFortioFetch2UpstreamResponse(r, client, addr, us, path, headers) defer res.Body.Close() require.Contains(r, statuses, res.StatusCode) }) @@ -459,7 +459,6 @@ func (a *Asserter) AutopilotHealth(t *testing.T, cluster *topology.Cluster, lead r.Log("out", out, "health", out.Healthy) require.Equal(r, expectedHealthy, out.Healthy) }) - return } type AuditEntry struct { diff --git a/test/integration/consul-container/test/envoy_extensions/wasm_test.go b/test/integration/consul-container/test/envoy_extensions/wasm_test.go index ae5cf3b3d5b7..c4f97828b870 100644 --- a/test/integration/consul-container/test/envoy_extensions/wasm_test.go +++ b/test/integration/consul-container/test/envoy_extensions/wasm_test.go @@ -8,7 +8,6 @@ import ( "crypto/sha256" "fmt" "io" - "net/http" "os" "testing" "time" @@ -69,7 +68,7 @@ func TestWASMRemote(t *testing.T) { require.NoError(t, err) // check that header DOES NOT exist before wasm applied - if value := res.Header.Get(http.CanonicalHeaderKey("x-test")); value != "" { + if value := res.Header.Get("x-test"); value != "" { t.Fatal("unexpected test header present before WASM applied") } @@ -190,7 +189,7 @@ func TestWASMRemote(t *testing.T) { res2, err := c2.Get(fmt.Sprintf("http://localhost:%d", port)) require.NoError(r, err) - if value := res2.Header.Get(http.CanonicalHeaderKey("x-test")); value == "" { + if value := res2.Header.Get("x-test"); value == "" { r.Fatal("test header missing after WASM applied") } }) @@ -229,7 +228,7 @@ func TestWASMLocal(t *testing.T) { require.NoError(t, err) // check that header DOES NOT exist before wasm applied - if value := res.Header.Get(http.CanonicalHeaderKey("x-test")); value != "" { + if value := res.Header.Get("x-test"); value != "" { t.Fatal("unexpected test header present before WASM applied") } @@ -271,7 +270,7 @@ func TestWASMLocal(t *testing.T) { res2, err := c2.Get(fmt.Sprintf("http://localhost:%d", port)) require.NoError(r, err) - if value := res2.Header.Get(http.CanonicalHeaderKey("x-test")); value == "" { + if value := res2.Header.Get("x-test"); value == "" { r.Fatal("test header missing after WASM applied") } }) diff --git a/test/integration/consul-container/test/gateways/http_route_test.go b/test/integration/consul-container/test/gateways/http_route_test.go index bf2c7d69567e..fea59a7b22d1 100644 --- a/test/integration/consul-container/test/gateways/http_route_test.go +++ b/test/integration/consul-container/test/gateways/http_route_test.go @@ -720,7 +720,7 @@ func TestHTTPRouteParentRefChange(t *testing.T) { apiEntry := entry.(*api.HTTPRouteConfigEntry) t.Log(apiEntry) - t.Log(fmt.Sprintf("%#v", apiEntry)) + t.Logf("%#v", apiEntry) // check if bound only to correct gateway return len(apiEntry.Parents) == 1 && diff --git a/testing/deployer/topology/topology.go b/testing/deployer/topology/topology.go index a2feefca4b63..b3641b3a25ef 100644 --- a/testing/deployer/topology/topology.go +++ b/testing/deployer/topology/topology.go @@ -215,10 +215,8 @@ func (n *Network) SetSubnet(subnet string) (bool, error) { var ipPool []string addr := p.Addr() - for { - if !p.Contains(addr) { - break - } + for p.Contains(addr) { + ipPool = append(ipPool, addr.String()) addr = addr.Next() } diff --git a/troubleshoot/ports/troubleshoot_ports_test.go b/troubleshoot/ports/troubleshoot_ports_test.go index 2358467e6853..77648d3513d5 100644 --- a/troubleshoot/ports/troubleshoot_ports_test.go +++ b/troubleshoot/ports/troubleshoot_ports_test.go @@ -46,8 +46,8 @@ func TestTroubleShootCustom_Ports_Not_Reachable(t *testing.T) { results := TroubleShootCustomPorts("127.0.0.1", strings.Join([]string{"8777", "8888"}, ",")) expectedResults := []string{ - fmt.Sprintf("TCP: Port 8777 on 127.0.0.1 is closed, unreachable, or the connection timed out.\n"), - fmt.Sprintf("TCP: Port 8888 on 127.0.0.1 is closed, unreachable, or the connection timed out.\n"), + "TCP: Port 8777 on 127.0.0.1 is closed, unreachable, or the connection timed out.\n", + "TCP: Port 8888 on 127.0.0.1 is closed, unreachable, or the connection timed out.\n", } for _, res := range expectedResults { require.Contains(t, results, res) diff --git a/troubleshoot/validate/validate.go b/troubleshoot/validate/validate.go index 93d26bb42ed1..656ae4fc09e0 100644 --- a/troubleshoot/validate/validate.go +++ b/troubleshoot/validate/validate.go @@ -85,14 +85,14 @@ func MakeValidate(ext extensioncommon.RuntimeConfig) (extensioncommon.BasicExten return nil, fmt.Errorf("expected extension name '%s' but got %q", api.BuiltinValidateExtension, name) } - envoyID, _ := ext.EnvoyExtension.Arguments["envoyID"] + envoyID := ext.EnvoyExtension.Arguments["envoyID"] mainEnvoyID, _ := envoyID.(string) vip := "" snis := map[string]struct{}{} upstream, ok := ext.Upstreams[ext.ServiceName] if ok { vip = upstream.VIP - if upstream.SNIs == nil || len(upstream.SNIs) == 0 { + if len(upstream.SNIs) == 0 { return nil, fmt.Errorf("no SNIs were set, unable to validate Envoy clusters") } snis = upstream.SNIs @@ -263,7 +263,7 @@ func (v *Validate) GetMessages(validateEndpoints bool, endpointValidator Endpoin } if numRequiredResources == 0 { - messages = append(messages, Message{Message: fmt.Sprintf("No clusters found on route or listener")}) + messages = append(messages, Message{Message: "No clusters found on route or listener"}) } return messages @@ -292,7 +292,7 @@ func DoEndpointValidation(r *resource, sni string, clusters *envoy_admin_v3.Clus for _, h := range hostStatuses { health := h.GetHealthStatus() if health != nil { - if health.EdsHealthStatus == envoy_core_v3.HealthStatus_HEALTHY && health.FailedOutlierCheck == false { + if health.EdsHealthStatus == envoy_core_v3.HealthStatus_HEALTHY && !health.FailedOutlierCheck { healthyEndpoints += 1 } }