From 4fc6b2580871bcca3a9c2fedc77d54f17c81038d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20Ma=C5=82ek?= Date: Thu, 22 Aug 2024 10:44:02 +0200 Subject: [PATCH] fix(ci): fix linter issues and use deterministic golangci-lint version used both locally and in CI --- .github/workflows/golangci-lint.yml | 12 +++---- .golangci.yml | 6 ++-- .tools_versions.yaml | 2 ++ Makefile | 31 ++++++++++++----- go.mod | 7 ++-- go.sum | 26 ++++++++------- kong/custom/entity_crud.go | 3 +- kong/custom_entity_service.go | 15 +++------ kong/plugin_service_test.go | 4 +-- kong/utils_test.go | 52 ++++++++++++----------------- 10 files changed, 81 insertions(+), 77 deletions(-) create mode 100644 .tools_versions.yaml diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 96bb08348..ac94ab51e 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -21,16 +21,16 @@ jobs: name: lint runs-on: ubuntu-latest steps: - - name: Checkout repository - uses: actions/checkout@v4 - - name: Setup go - uses: actions/setup-go@v5 + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 with: go-version-file: go.mod - - name: Setup golangci-lint - uses: golangci/golangci-lint-action@v6 + - run: | + echo "YQ_VERSION=$(yq -r '.golangci-lint' < .tools_versions.yaml)" >> $GITHUB_ENV + - uses: golangci/golangci-lint-action@v6 with: # actions/setup-go@v5 introduced automatic cache handling so skip cache here skip-cache: true + version: "v${{ env.YQ_VERSION }}" - name: Verify Codegen run: make verify-codegen diff --git a/.golangci.yml b/.golangci.yml index bedae5ae0..a13910d31 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -14,13 +14,14 @@ linters: - errname - errorlint - exhaustive - - exportloopref + # TODO: enable when we set minimal Go version to 1.23 + # - copyloopvar - gci - gochecknoinits - gofmt - gofumpt - goimports - - gomnd + - mnd - gomodguard - gosec - gosimple @@ -28,7 +29,6 @@ linters: - importas - ineffassign - lll - - megacheck - misspell - nakedret - nilerr diff --git a/.tools_versions.yaml b/.tools_versions.yaml new file mode 100644 index 000000000..64fdcab2c --- /dev/null +++ b/.tools_versions.yaml @@ -0,0 +1,2 @@ +# renovate: datasource=github-releases depName=golangci/golangci-lint +golangci-lint: "1.60.2" diff --git a/Makefile b/Makefile index b016568be..9faa77206 100644 --- a/Makefile +++ b/Makefile @@ -3,19 +3,32 @@ # ------------------------------------------------------------------------------ PROJECT_DIR := $(shell dirname $(abspath $(lastword $(MAKEFILE_LIST)))) +TOOLS_VERSIONS_FILE = $(PROJECT_DIR)/.tools_versions.yaml +export MISE_DATA_DIR = $(PROJECT_DIR)/bin/ -.PHONY: _download_tool -_download_tool: - (cd third_party && go mod tidy && \ - GOBIN=$(PROJECT_DIR)/bin go generate -tags=third_party ./$(TOOL).go ) +MISE := $(shell which mise) +.PHONY: mise +mise: + @mise -V >/dev/null || (echo "mise - https://github.com/jdx/mise - not found. Please install it." && exit 1) .PHONY: tools -tools: golangci-lint - -GOLANGCI_LINT = $(PROJECT_DIR)/bin/golangci-lint +tools: golangci-lint yq + +# Do not store yq's version in .tools_versions.yaml as it is used to get tool versions. +# renovate: datasource=github-releases depName=mikefarah/yq +YQ_VERSION = 4.43.1 +YQ = $(PROJECT_DIR)/bin/installs/yq/$(YQ_VERSION)/bin/yq +.PHONY: yq +yq: mise # Download yq locally if necessary. + @$(MISE) plugin install --yes -q yq + @$(MISE) install -q yq@$(YQ_VERSION) + +GOLANGCI_LINT_VERSION = $(shell $(YQ) -r '.golangci-lint' < $(TOOLS_VERSIONS_FILE)) +GOLANGCI_LINT = $(PROJECT_DIR)/bin/installs/golangci-lint/$(GOLANGCI_LINT_VERSION)/bin/golangci-lint .PHONY: golangci-lint -golangci-lint: ## Download golangci-lint locally if necessary. - @$(MAKE) _download_tool TOOL=golangci-lint +golangci-lint: mise yq ## Download golangci-lint locally if necessary. + @$(MISE) plugin install --yes -q golangci-lint + @$(MISE) install -q golangci-lint@$(GOLANGCI_LINT_VERSION) # ------------------------------------------------------------------------------ # Testing diff --git a/go.mod b/go.mod index ca0d2fd53..e5de7fe0b 100644 --- a/go.mod +++ b/go.mod @@ -18,6 +18,7 @@ require ( github.com/imdario/mergo v0.3.12 github.com/kong/semver/v4 v4.0.1 github.com/mitchellh/mapstructure v1.5.0 + github.com/samber/lo v1.47.0 github.com/stretchr/testify v1.9.0 github.com/tidwall/gjson v1.17.3 k8s.io/code-generator v0.30.3 @@ -38,8 +39,10 @@ require ( github.com/spf13/pflag v1.0.5 // indirect github.com/tidwall/match v1.1.1 // indirect github.com/tidwall/pretty v1.2.0 // indirect - golang.org/x/mod v0.15.0 // indirect - golang.org/x/tools v0.18.0 // indirect + golang.org/x/mod v0.17.0 // indirect + golang.org/x/sync v0.7.0 // indirect + golang.org/x/text v0.16.0 // indirect + golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect google.golang.org/protobuf v1.33.0 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/go.sum b/go.sum index b6ae6ae61..8b770d865 100644 --- a/go.sum +++ b/go.sum @@ -51,6 +51,8 @@ github.com/onsi/gomega v1.19.0 h1:4ieX6qQjPP/BfC3mpsAtIGGlxTWPeA3Inl/7DtXw1tw= github.com/onsi/gomega v1.19.0/go.mod h1:LY+I3pBVzYsTBU1AnDwOSxaYi9WoWiqgwooUqq9yPro= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/samber/lo v1.47.0 h1:z7RynLwP5nbyRscyvcD043DWYoOcYRv3mV8lBeqOCLc= +github.com/samber/lo v1.47.0/go.mod h1:RmDH9Ct32Qy3gduHQuKJ3gW1fMHAnE/fAzQuf6He5cU= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= @@ -68,18 +70,18 @@ github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA= github.com/tidwall/match v1.1.1/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM= github.com/tidwall/pretty v1.2.0 h1:RWIZEg2iJ8/g6fDDYzMpobmaoGh5OLl4AXtGUGPcqCs= github.com/tidwall/pretty v1.2.0/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU= -golang.org/x/mod v0.15.0 h1:SernR4v+D55NyBH2QiEQrlBAnj1ECL6AGrA5+dPaMY8= -golang.org/x/mod v0.15.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= -golang.org/x/net v0.21.0 h1:AQyQV4dYCvJ7vGmJyKki9+PBdyvhkSd8EIx/qb0AYv4= -golang.org/x/net v0.21.0/go.mod h1:bIjVDfnllIU7BJ2DNgfnXvpSvtn8VRwhlsaeUTyUS44= -golang.org/x/sync v0.6.0 h1:5BMeUDZ7vkXGfEr1x9B4bRcTH4lpkTkpdh0T/J+qjbQ= -golang.org/x/sync v0.6.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= -golang.org/x/sys v0.17.0 h1:25cE3gD+tdBA7lp7QfhuV+rJiE9YXTcS3VG1SqssI/Y= -golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ= -golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= -golang.org/x/tools v0.18.0 h1:k8NLag8AGHnn+PHbl7g43CtqZAwG60vZkLqgyZgIHgQ= -golang.org/x/tools v0.18.0/go.mod h1:GL7B4CwcLLeo59yx/9UWWuNOW1n3VZ4f5axWfML7Lcg= +golang.org/x/mod v0.17.0 h1:zY54UmvipHiNd+pm+m0x9KhZ9hl1/7QNMyxXbc6ICqA= +golang.org/x/mod v0.17.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= +golang.org/x/net v0.25.0 h1:d/OCCoBEUq33pjydKrGQhw7IlUPI2Oylr+8qLx49kac= +golang.org/x/net v0.25.0/go.mod h1:JkAGAh7GEvH74S6FOH42FLoXpXbE/aqXSrIQjXgsiwM= +golang.org/x/sync v0.7.0 h1:YsImfSBoP9QPYL0xyKJPq0gcaJdG3rInoqxTWbfQu9M= +golang.org/x/sync v0.7.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= +golang.org/x/sys v0.20.0 h1:Od9JTbYCk261bKm4M/mw7AklTlFYIa0bIp9BgSm1S8Y= +golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/text v0.16.0 h1:a94ExnEXNtEwYLGJSIUxnWoxoRz/ZcCsV63ROupILh4= +golang.org/x/text v0.16.0/go.mod h1:GhwF1Be+LQoKShO3cGOHzqOgRrGaYc9AvblQOmPVHnI= +golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d h1:vU5i/LfpvrRCpgM/VPfJLg5KjxD3E+hfT1SH+d9zLwg= +golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d/go.mod h1:aiJjzUbINMkxbQROHiO6hDPo2LHcIPhhQsa9DLh0yGk= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/protobuf v1.33.0 h1:uNO2rsAINq/JlFpSdYEKIZ0uKD/R9cpdv0T+yoGwGmI= google.golang.org/protobuf v1.33.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= diff --git a/kong/custom/entity_crud.go b/kong/custom/entity_crud.go index f7c7d9ffa..af74cd21e 100644 --- a/kong/custom/entity_crud.go +++ b/kong/custom/entity_crud.go @@ -53,8 +53,7 @@ func render(template string, entity Entity) (string, error) { if v := entity.GetRelation(m[1]); v != "" { result = strings.Replace(result, m[0], v, 1) } else { - return "", fmt.Errorf("cannot substitute '" + m[1] + - "' in URL: " + template) + return "", fmt.Errorf("cannot substitute '%s' in URL: %s", m[1], template) } } return result, nil diff --git a/kong/custom_entity_service.go b/kong/custom_entity_service.go index 104f9aa93..8ce32fa15 100644 --- a/kong/custom_entity_service.go +++ b/kong/custom_entity_service.go @@ -36,8 +36,7 @@ func (s *CustomEntityService) Get(ctx context.Context, ) (custom.Entity, error) { def := s.client.Lookup(entity.Type()) if def == nil { - return nil, fmt.Errorf("entity '" + string(entity.Type()) + - "' not registered") + return nil, fmt.Errorf("entity '%s' not registered", entity.Type()) } queryPath, err := def.GetEndpoint(entity) @@ -66,8 +65,7 @@ func (s *CustomEntityService) Create(ctx context.Context, ) (custom.Entity, error) { def := s.client.Lookup(entity.Type()) if def == nil { - return nil, fmt.Errorf("entity '" + string(entity.Type()) + - "' not registered") + return nil, fmt.Errorf("entity '%s' not registered", entity.Type()) } method := "POST" @@ -111,8 +109,7 @@ func (s *CustomEntityService) Update(ctx context.Context, ) (custom.Entity, error) { def := s.client.Lookup(entity.Type()) if def == nil { - return nil, fmt.Errorf("entity '" + string(entity.Type()) + - "' not registered") + return nil, fmt.Errorf("entity '%s' not registered", entity.Type()) } queryPath, err := def.PatchEndpoint(entity) @@ -146,8 +143,7 @@ func (s *CustomEntityService) Delete(ctx context.Context, ) error { def := s.client.Lookup(entity.Type()) if def == nil { - return fmt.Errorf("entity '" + string(entity.Type()) + - "' not registered") + return fmt.Errorf("entity '%s' not registered", entity.Type()) } queryPath, err := def.PatchEndpoint(entity) @@ -170,8 +166,7 @@ func (s *CustomEntityService) List(ctx context.Context, opt *ListOpt, ) ([]custom.Entity, *ListOpt, error) { def := s.client.Lookup(entity.Type()) if def == nil { - return nil, nil, fmt.Errorf("entity '" + string(entity.Type()) + - "' not registered") + return nil, nil, fmt.Errorf("entity '%s' not registered", entity.Type()) } queryPath, err := def.ListEndpoint(entity) diff --git a/kong/plugin_service_test.go b/kong/plugin_service_test.go index c0f7fe7a4..e91b2eccb 100644 --- a/kong/plugin_service_test.go +++ b/kong/plugin_service_test.go @@ -730,7 +730,7 @@ func TestFillPluginDefaults(T *testing.T) { require.NoError(t, FillPluginsDefaults(p, fullSchema)) if diff := cmp.Diff(p, tc.expected); diff != "" { - t.Errorf(diff) + t.Errorf("unexpected diff:\n%s", diff) } }) } @@ -795,7 +795,7 @@ func TestFillPluginDefaultsArbitraryMap(T *testing.T) { // https://github.com/Kong/kong/commit/9df893f6aff98cd51f27f1c27fa30fdcf13fcf48 changes a number of other // fields for 3.3, so this test only checks the relevant field to avoid needing a version split if diff := cmp.Diff(p.Config["custom_fields_by_lua"], tc.expected.Config["custom_fields_by_lua"]); diff != "" { - t.Errorf(diff) + t.Errorf("unexpected diff:\n%s", diff) } }) } diff --git a/kong/utils_test.go b/kong/utils_test.go index 5dd6204dd..fd7a4fdaf 100644 --- a/kong/utils_test.go +++ b/kong/utils_test.go @@ -1018,16 +1018,14 @@ func TestFillRoutesDefaults(T *testing.T) { fullSchema, err := client.Schemas.Get(defaultCtx, "routes") assert.NoError(err) assert.NotNil(fullSchema) - if err = FillEntityDefaults(r, fullSchema); err != nil { - t.Errorf(err.Error()) - } + require.NoError(t, FillEntityDefaults(r, fullSchema)) // Ignore fields to make tests pass despite small differences across releases. opts := cmpopts.IgnoreFields( Route{}, "RequestBuffering", "ResponseBuffering", "PathHandling", ) if diff := cmp.Diff(r, tc.expected, opts); diff != "" { - t.Errorf(diff) + t.Errorf("unexpected diff:\n%s", diff) } }) } @@ -1108,14 +1106,12 @@ func TestFillServiceDefaults(T *testing.T) { fullSchema, err := client.Schemas.Get(defaultCtx, "services") assert.NoError(err) assert.NotNil(fullSchema) - if err := FillEntityDefaults(s, fullSchema); err != nil { - t.Errorf(err.Error()) - } + require.NoError(t, FillEntityDefaults(s, fullSchema)) opt := []cmp.Option{ cmpopts.IgnoreFields(Service{}, "Enabled"), } if diff := cmp.Diff(s, tc.expected, opt...); diff != "" { - t.Errorf(diff) + t.Errorf("unexpected diff:\n%s", diff) } }) } @@ -1157,11 +1153,9 @@ func TestFillTargetDefaults(T *testing.T) { fullSchema, err := client.Schemas.Get(defaultCtx, "targets") assert.NoError(err) assert.NotNil(fullSchema) - if err := FillEntityDefaults(target, fullSchema); err != nil { - t.Errorf(err.Error()) - } + require.NoError(t, FillEntityDefaults(target, fullSchema)) if diff := cmp.Diff(target, tc.expected); diff != "" { - t.Errorf(diff) + t.Errorf("unexpected diff:\n%s", diff) } }) } @@ -1297,16 +1291,14 @@ func TestFillUpstreamsDefaults(T *testing.T) { fullSchema, err := client.Schemas.Get(defaultCtx, "upstreams") assert.NoError(err) assert.NotNil(fullSchema) - if err = FillEntityDefaults(u, fullSchema); err != nil { - t.Errorf(err.Error()) - } + require.NoError(t, FillEntityDefaults(u, fullSchema)) // Ignore fields to make tests pass despite small differences across releases. opts := []cmp.Option{ cmpopts.IgnoreFields(Healthcheck{}, "Threshold"), cmpopts.IgnoreFields(Upstream{}, "UseSrvName"), } if diff := cmp.Diff(u, tc.expected, opts...); diff != "" { - t.Errorf(diff) + t.Errorf("unexpected diff:\n%s", diff) } }) } @@ -1449,7 +1441,7 @@ func TestFillUpstreamsDefaultsFromJSONSchema(t *testing.T) { // Ignore fields to make tests pass despite small differences across releases. opts := cmpopts.IgnoreFields(Healthcheck{}, "Threshold") if diff := cmp.Diff(u, tc.expected, opts); diff != "" { - t.Errorf(diff) + t.Errorf("unexpected diff:\n%s", diff) } }) } @@ -1529,7 +1521,7 @@ func TestFillServicesDefaultsFromJSONSchema(t *testing.T) { cmpopts.IgnoreFields(Service{}, "Enabled"), } if diff := cmp.Diff(s, tc.expected, opt...); diff != "" { - t.Errorf(diff) + t.Errorf("unexpected diff:\n%s", diff) } }) } @@ -1608,7 +1600,7 @@ func TestFillRoutesDefaultsFromJSONSchema(t *testing.T) { "RequestBuffering", "ResponseBuffering", "PathHandling", ) if diff := cmp.Diff(r, tc.expected, opts); diff != "" { - t.Errorf(diff) + t.Errorf("unexpected diff:\n%s", diff) } }) } @@ -1732,11 +1724,9 @@ func TestFillConsumerGroupPluginDefaults(T *testing.T) { fullSchema, err := client.Schemas.Get(defaultCtx, "consumer_group_plugins") assert.NoError(err) assert.NotNil(fullSchema) - if err := FillEntityDefaults(plugin, fullSchema); err != nil { - t.Errorf(err.Error()) - } + require.NoError(t, FillEntityDefaults(plugin, fullSchema)) if diff := cmp.Diff(plugin, tc.expected); diff != "" { - t.Errorf(diff) + t.Errorf("unexpected diff:\n%s", diff) } }) } @@ -1849,7 +1839,7 @@ func Test_fillConfigRecord(t *testing.T) { config := fillConfigRecord(configSchema, tc.config) require.NotNil(t, config) if diff := cmp.Diff(config, tc.expected); diff != "" { - t.Errorf(diff) + t.Errorf("unexpected diff:\n%s", diff) } }) } @@ -2005,7 +1995,7 @@ func Test_fillConfigRecord_shorthand_fields(t *testing.T) { config := fillConfigRecord(configSchema, tc.config) require.NotNil(t, config) if diff := cmp.Diff(config, tc.expected); diff != "" { - t.Errorf(diff) + t.Errorf("unexpected diff:\n%s", diff) } }) } @@ -2142,7 +2132,7 @@ func Test_FillPluginsDefaults(t *testing.T) { "Protocols", "Enabled", ) if diff := cmp.Diff(plugin, tc.expected, opts); diff != "" { - t.Errorf(diff) + t.Errorf("unexpected diff:\n%s", diff) } }) } @@ -2237,7 +2227,7 @@ func Test_FillPluginsDefaults_RequestTransformer(t *testing.T) { assert.NoError(t, FillPluginsDefaults(plugin, fullSchema)) opts := cmpopts.IgnoreFields(*plugin, "Enabled", "Protocols") if diff := cmp.Diff(plugin, tc.expected, opts); diff != "" { - t.Errorf(diff) + t.Errorf("unexpected diff:\n%s", diff) } }) } @@ -2358,7 +2348,7 @@ func Test_FillPluginsDefaults_SetType(t *testing.T) { "Protocols", "Enabled", ) if diff := cmp.Diff(plugin, tc.expected, opts); diff != "" { - t.Errorf(diff) + t.Errorf("unexpected diff:\n%s", diff) } }) } @@ -2448,7 +2438,7 @@ func Test_FillPluginsDefaults_Acme(t *testing.T) { assert.NoError(t, FillPluginsDefaults(plugin, fullSchema)) opts := cmpopts.IgnoreFields(*plugin, "Enabled", "Protocols") if diff := cmp.Diff(plugin, tc.expected, opts); diff != "" { - t.Errorf(diff) + t.Errorf("unexpected diff:\n%s", diff) } }) } @@ -2523,7 +2513,7 @@ func Test_FillPluginsDefaults_DefaultRecord(t *testing.T) { assert.NoError(t, FillPluginsDefaults(plugin, fullSchema)) opts := cmpopts.IgnoreFields(*plugin, "Enabled", "Protocols") if diff := cmp.Diff(plugin, tc.expected, opts); diff != "" { - t.Errorf(diff) + t.Errorf("unexpected diff:\n%s", diff) } }) } @@ -2656,7 +2646,7 @@ func Test_FillPluginsDefaults_NonEmptyDefaultArrayField(t *testing.T) { assert.NoError(t, FillPluginsDefaults(plugin, fullSchema)) opts := cmpopts.IgnoreFields(*plugin, "Enabled", "Protocols") if diff := cmp.Diff(plugin, tc.expected, opts); diff != "" { - t.Errorf(diff) + t.Errorf("unexpected diff:\n%s", diff) } }) }