From 9750b4a011ce586a08d83dcd1694de7f1cf701ed Mon Sep 17 00:00:00 2001 From: Chris Bandy Date: Thu, 10 Jul 2025 11:41:48 -0500 Subject: [PATCH] Enable more Go linters and document exclusions --- .github/workflows/lint.yaml | 14 +- .golangci.bck.yaml | 122 ------------------ .golangci.next.yaml | 118 ++++++++++++----- .golangci.yaml | 120 ++++++++++------- internal/bridge/client.go | 34 ++--- internal/controller/pgupgrade/jobs.go | 1 + .../postgrescluster/controller_ref_manager.go | 2 +- .../controller/postgrescluster/instance.go | 5 +- .../postgrescluster/instance_test.go | 1 - .../postgrescluster/patroni_test.go | 2 +- .../controller/postgrescluster/pgbackrest.go | 4 +- .../standalone_pgadmin/statefulset.go | 2 +- internal/patroni/config.go | 2 +- internal/patroni/config_test.go | 1 - internal/pgbackrest/config.go | 2 +- internal/pgbackrest/util_test.go | 6 +- internal/pgbouncer/postgres.go | 2 +- internal/postgres/password/scram.go | 2 +- internal/upgradecheck/http.go | 2 +- internal/upgradecheck/http_test.go | 3 +- internal/util/secrets_test.go | 4 +- 21 files changed, 199 insertions(+), 250 deletions(-) delete mode 100644 .golangci.bck.yaml diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index f164e72a43..66479589ac 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -28,12 +28,8 @@ jobs: # exits zero to ensure it does not fail the pull request check. - name: Count non-blocking issues run: | - golangci-lint run --config .golangci.next.yaml \ - --issues-exit-code 0 \ - --max-issues-per-linter 0 \ - --max-same-issues 0 \ - --out-format json | - jq --sort-keys 'reduce .Issues[] as $i ({}; .[$i.FromLinter] += 1)' | - awk >> "${GITHUB_STEP_SUMMARY}" ' - NR == 1 { print "```json" } { print } END { if (NR > 0) print "```" } - ' || true + golangci-lint run --config .golangci.next.yaml --show-stats >> "${GITHUB_STEP_SUMMARY}" \ + --max-issues-per-linter=0 \ + --max-same-issues=0 \ + --uniq-by-line=0 \ + --output.text.path=/dev/null ||: diff --git a/.golangci.bck.yaml b/.golangci.bck.yaml deleted file mode 100644 index fb18c52e1e..0000000000 --- a/.golangci.bck.yaml +++ /dev/null @@ -1,122 +0,0 @@ -# https://golangci-lint.run/usage/configuration/ - -linters: - disable: - - contextcheck - - gofumpt - enable: - - goheader - - gosimple - - importas - - misspell - - unconvert - presets: - - bugs - - format - - import - - unused - -linters-settings: - depguard: - rules: - everything: - list-mode: lax - allow: - - go.opentelemetry.io/otel/semconv/v1.27.0 - deny: - - pkg: go.opentelemetry.io/otel/semconv - desc: Use "go.opentelemetry.io/otel/semconv/v1.27.0" instead. - - - pkg: io/ioutil - desc: > - Use the "io" and "os" packages instead. - See https://go.dev/doc/go1.16#ioutil - - not-tests: - files: ['!$test'] - deny: - - pkg: net/http/httptest - desc: Should be used only in tests. - - - pkg: testing/* - desc: The "testing" packages should be used only in tests. - - - pkg: github.com/crunchydata/postgres-operator/internal/testing/* - desc: The "internal/testing" packages should be used only in tests. - - - pkg: k8s.io/client-go/discovery - desc: Use the "internal/kubernetes" package instead. - - tests: - files: ['$test'] - deny: - - pkg: github.com/pkg/errors - desc: Use the "errors" package unless you are interacting with stack traces. - - errchkjson: - check-error-free-encoding: true - - exhaustive: - default-signifies-exhaustive: true - - gci: - sections: - - standard - - default - - localmodule - - goheader: - template: |- - Copyright {{ DATES }} Crunchy Data Solutions, Inc. - - SPDX-License-Identifier: Apache-2.0 - values: - regexp: - DATES: '((201[7-9]|202[0-4]) - 2025|2025)' - - gomodguard: - blocked: - modules: - - gopkg.in/yaml.v2: { recommendations: [sigs.k8s.io/yaml] } - - gopkg.in/yaml.v3: { recommendations: [sigs.k8s.io/yaml] } - - gotest.tools: { recommendations: [gotest.tools/v3] } - - k8s.io/kubernetes: - reason: > - k8s.io/kubernetes is for managing dependencies of the Kubernetes - project, i.e. building kubelet and kubeadm. - - gosec: - excludes: - # Flags for potentially-unsafe casting of ints, similar problem to globally-disabled G103 - - G115 - - importas: - alias: - - pkg: k8s.io/api/(\w+)/(v[\w\w]+) - alias: $1$2 - - pkg: k8s.io/apimachinery/pkg/apis/(\w+)/(v[\w\d]+) - alias: $1$2 - - pkg: k8s.io/apimachinery/pkg/api/errors - alias: apierrors - no-unaliased: true - - spancheck: - checks: [end, record-error] - extra-start-span-signatures: - - 'github.com/crunchydata/postgres-operator/internal/tracing.Start:opentelemetry' - ignore-check-signatures: - - 'tracing.Escape' - -issues: - exclude-generated: strict - exclude-rules: - # This internal package is the one place we want to do API discovery. - - linters: [depguard] - path: internal/kubernetes/discovery.go - text: k8s.io/client-go/discovery - - # These value types have unmarshal methods. - # https://github.com/raeperd/recvcheck/issues/7 - - linters: [recvcheck] - path: internal/pki/pki.go - text: 'methods of "(Certificate|PrivateKey)"' diff --git a/.golangci.next.yaml b/.golangci.next.yaml index 9d35cd941a..f35c9265d8 100644 --- a/.golangci.next.yaml +++ b/.golangci.next.yaml @@ -1,44 +1,98 @@ -# https://golangci-lint.run/usage/configuration/ +# https://golangci-lint.run/usage/configuration # # This file is for linters that might be interesting to enforce in the future. # Rules that should be enforced immediately belong in [.golangci.yaml]. # # Both files are used by [.github/workflows/lint.yaml]. +version: "2" +# https://golangci-lint.run/usage/linters linters: - disable-all: true - enable: - - contextcheck - - err113 - - gocritic - - godot - - godox - - gofumpt - - gosec # exclude-use-default - - nilnil + default: all + disable: + - asasalint + - asciicheck + - bidichk + - bodyclose + - copyloopvar + - depguard + - dupword + - durationcheck + - errchkjson + - errname + - errorlint + - exhaustive + - exptostd + - fatcontext + - forbidigo + - ginkgolinter + - gocheckcompilerdirectives + - gochecksumtype + - goheader + - gomoddirectives + - gomodguard + - goprintffuncname + - gosmopolitan + - grouper + - iface + - importas + - interfacebloat + - intrange + - loggercheck + - makezero + - mirror + - misspell + - musttag + - nilerr + - nilnesserr + - noctx - nolintlint - - predeclared - - revive - - staticcheck # exclude-use-default - - tenv - - thelper - - tparallel + - nosprintfhostport + - prealloc + - promlinter + - protogetter + - reassign + - recvcheck + - rowserrcheck + - sloglint + - spancheck + - sqlclosecheck + - tagalign + - testifylint + - unconvert + - unparam + - usestdlibvars + - usetesting - wastedassign - presets: - - performance + - wsl + - zerologlint -issues: - exclude-rules: - # We call external linters when they are installed: Flake8, ShellCheck, etc. - - linters: [gosec] - path: '_test[.]go$' - text: 'G204: Subprocess launched with variable' + settings: + thelper: + # https://github.com/kulti/thelper/issues/27 + tb: { begin: true, first: true } + test: { begin: true, first: true, name: true } + + exclusions: + warn-unused: true + # Ignore built-in exclusions + presets: [] + rules: + # We call external linters when they are installed: Flake8, ShellCheck, etc. + - linters: [gosec] + path: '_test[.]go$' + text: 'G204: Subprocess launched with variable' - # https://github.com/golangci/golangci-lint/issues/2239 - exclude-use-default: false +# https://golangci-lint.run/usage/formatters +formatters: + enable: + - gofumpt + +issues: + # Fix only when requested + fix: false -linters-settings: - thelper: - # https://github.com/kulti/thelper/issues/27 - tb: { begin: true, first: true } - test: { begin: true, first: true, name: true } + # Show all issues at once + max-issues-per-linter: 0 + max-same-issues: 0 + uniq-by-line: false diff --git a/.golangci.yaml b/.golangci.yaml index 36f057e1ff..b1e6c7167b 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,45 +1,72 @@ +# https://golangci-lint.run/usage/configuration version: "2" + +# https://golangci-lint.run/usage/linters linters: + default: standard enable: - asasalint - asciicheck - bidichk - bodyclose + - copyloopvar - depguard + - dupword - durationcheck - errchkjson + - errname - errorlint - exhaustive + - exptostd + - fatcontext + - forbidigo + - ginkgolinter - gocheckcompilerdirectives - gochecksumtype - goheader + - gomoddirectives - gomodguard + - goprintffuncname - gosec - gosmopolitan + - grouper + - iface - importas + - interfacebloat + - intrange - loggercheck - makezero + - mirror - misspell - musttag - nilerr - nilnesserr - noctx + - nolintlint + - nosprintfhostport + - prealloc + - promlinter - protogetter - reassign - recvcheck - rowserrcheck + - sloglint - spancheck - sqlclosecheck + - tagalign - testifylint - unconvert - unparam + - usestdlibvars + - usetesting + - wastedassign - zerologlint - disable: - - contextcheck + settings: depguard: rules: everything: + files: ['$all'] list-mode: lax allow: - go.opentelemetry.io/otel/semconv/v1.27.0 @@ -47,11 +74,10 @@ linters: - pkg: go.opentelemetry.io/otel/semconv desc: Use "go.opentelemetry.io/otel/semconv/v1.27.0" instead. - pkg: io/ioutil - desc: | - Use the "io" and "os" packages instead. See https://go.dev/doc/go1.16#ioutil + desc: Use the "io" and "os" packages instead. See https://go.dev/doc/go1.16#ioutil not-tests: - files: - - '!$test' + files: ['!$test','!**/internal/testing/**'] + list-mode: lax deny: - pkg: net/http/httptest desc: Should be used only in tests. @@ -62,42 +88,35 @@ linters: - pkg: k8s.io/client-go/discovery desc: Use the "internal/kubernetes" package instead. tests: - files: - - $test + files: ['$test'] + list-mode: lax deny: - pkg: github.com/pkg/errors desc: Use the "errors" package unless you are interacting with stack traces. + errchkjson: check-error-free-encoding: true - exhaustive: - default-signifies-exhaustive: true + goheader: - values: - regexp: - DATES: ((201[7-9]|202[0-4]) - 2025|2025) template: |- Copyright {{ DATES }} Crunchy Data Solutions, Inc. SPDX-License-Identifier: Apache-2.0 + values: + regexp: + DATES: ((201[7-9]|202[0-4]) - 2025|2025) + gomodguard: blocked: modules: - - gopkg.in/yaml.v2: - recommendations: - - sigs.k8s.io/yaml - - gopkg.in/yaml.v3: - recommendations: - - sigs.k8s.io/yaml - - gotest.tools: - recommendations: - - gotest.tools/v3 + - gopkg.in/yaml.v2: { recommendations: [sigs.k8s.io/yaml] } + - gopkg.in/yaml.v3: { recommendations: [sigs.k8s.io/yaml] } + - gotest.tools: { recommendations: [gotest.tools/v3] } - k8s.io/kubernetes: - reason: | - k8s.io/kubernetes is for managing dependencies of the Kubernetes project, i.e. building kubelet and kubeadm. - gosec: - excludes: - - G115 + reason: k8s.io/kubernetes is for building kubelet, kubeadm, etc. + importas: + no-unaliased: true alias: - pkg: k8s.io/api/(\w+)/(v[\w\w]+) alias: $1$2 @@ -105,51 +124,54 @@ linters: alias: $1$2 - pkg: k8s.io/apimachinery/pkg/api/errors alias: apierrors - no-unaliased: true + spancheck: - checks: - - end - - record-error - ignore-check-signatures: - - tracing.Escape + checks: [end, record-error] extra-start-span-signatures: - github.com/crunchydata/postgres-operator/internal/tracing.Start:opentelemetry + ignore-check-signatures: + - tracing.Escape + exclusions: + warn-unused: true presets: - - comments - common-false-positives - legacy - std-error-handling rules: - - linters: - - depguard + # This internal package is the one place we want to do API discovery. + - linters: [depguard] path: internal/kubernetes/discovery.go text: k8s.io/client-go/discovery - - linters: - - recvcheck + + # Postgres HBA rules often include "all all all" + - linters: [dupword] + path: /(hba|postgres)[^/]+$ + text: words \(all\) found + + # These value types have unmarshal methods. + # https://github.com/raeperd/recvcheck/issues/7 + - linters: [recvcheck] path: internal/pki/pki.go text: methods of "(Certificate|PrivateKey)" - paths: - - third_party$ - - builtin$ - - examples$ + +# https://golangci-lint.run/usage/formatters formatters: enable: - gci - gofmt - - goimports settings: gci: sections: - standard - default - localmodule - exclusions: - paths: - - third_party$ - - builtin$ - - examples$ + issues: - # Disable max issues limit (default is 50) + # Fix only when requested + fix: false + + # Show all issues at once max-issues-per-linter: 0 max-same-issues: 0 + uniq-by-line: false diff --git a/internal/bridge/client.go b/internal/bridge/client.go index c24ea2b2bb..3e3c4c3b4c 100644 --- a/internal/bridge/client.go +++ b/internal/bridge/client.go @@ -327,7 +327,7 @@ func (c *Client) doWithRetry( // Retry the request when the server responds with "Too many requests". // - https://docs.crunchybridge.com/api-concepts/getting-started/#status-codes // - https://docs.crunchybridge.com/api-concepts/getting-started/#rate-limiting - for err == nil && response.StatusCode == 429 { + for err == nil && response.StatusCode == http.StatusTooManyRequests { seconds, _ := strconv.Atoi(response.Header.Get("Retry-After")) // Only retry when the response indicates how long to wait. @@ -378,11 +378,11 @@ func (c *Client) CreateAuthObject(ctx context.Context, authn AuthObject) (AuthOb } // 401, Unauthorized - case response.StatusCode == 401: + case response.StatusCode == http.StatusUnauthorized: err = fmt.Errorf("%w: %s", errAuthentication, body) default: - //nolint:goerr113 // This is intentionally dynamic. + //nolint:err113 // This is intentionally dynamic. err = fmt.Errorf("%v: %s", response.Status, body) } } @@ -409,7 +409,7 @@ func (c *Client) CreateInstallation(ctx context.Context) (Installation, error) { } default: - //nolint:goerr113 // This is intentionally dynamic. + //nolint:err113 // This is intentionally dynamic. err = fmt.Errorf("%v: %s", response.Status, body) } } @@ -445,7 +445,7 @@ func (c *Client) ListClusters(ctx context.Context, apiKey, teamId string) ([]*Cl } default: - //nolint:goerr113 // This is intentionally dynamic. + //nolint:err113 // This is intentionally dynamic. err = fmt.Errorf("%v: %s", response.Status, body) } } @@ -486,7 +486,7 @@ func (c *Client) CreateCluster( } default: - //nolint:goerr113 // This is intentionally dynamic. + //nolint:err113 // This is intentionally dynamic. err = fmt.Errorf("%v: %s", response.Status, body) } } @@ -524,14 +524,14 @@ func (c *Client) DeleteCluster(ctx context.Context, apiKey, id string) (*Cluster // --https://docs.crunchybridge.com/api-concepts/idempotency#delete-semantics // But also, if we can't find it... // Maybe if no ID we return already deleted? - case response.StatusCode == 410: + case response.StatusCode == http.StatusGone: fallthrough - case response.StatusCode == 404: + case response.StatusCode == http.StatusNotFound: deletedAlready = true err = nil default: - //nolint:goerr113 // This is intentionally dynamic. + //nolint:err113 // This is intentionally dynamic. err = fmt.Errorf("%v: %s", response.Status, body) } } @@ -565,7 +565,7 @@ func (c *Client) GetCluster(ctx context.Context, apiKey, id string) (*ClusterApi } default: - //nolint:goerr113 // This is intentionally dynamic. + //nolint:err113 // This is intentionally dynamic. err = fmt.Errorf("%v: %s", response.Status, body) } } @@ -599,7 +599,7 @@ func (c *Client) GetClusterStatus(ctx context.Context, apiKey, id string) (*Clus } default: - //nolint:goerr113 // This is intentionally dynamic. + //nolint:err113 // This is intentionally dynamic. err = fmt.Errorf("%v: %s", response.Status, body) } } @@ -633,7 +633,7 @@ func (c *Client) GetClusterUpgrade(ctx context.Context, apiKey, id string) (*Clu } default: - //nolint:goerr113 // This is intentionally dynamic. + //nolint:err113 // This is intentionally dynamic. err = fmt.Errorf("%v: %s", response.Status, body) } } @@ -674,7 +674,7 @@ func (c *Client) UpgradeCluster( } default: - //nolint:goerr113 // This is intentionally dynamic. + //nolint:err113 // This is intentionally dynamic. err = fmt.Errorf("%v: %s", response.Status, body) } } @@ -709,7 +709,7 @@ func (c *Client) UpgradeClusterHA(ctx context.Context, apiKey, id, action string } default: - //nolint:goerr113 // This is intentionally dynamic. + //nolint:err113 // This is intentionally dynamic. err = fmt.Errorf("%v: %s", response.Status, body) } } @@ -747,7 +747,7 @@ func (c *Client) UpdateCluster( } default: - //nolint:goerr113 // This is intentionally dynamic. + //nolint:err113 // This is intentionally dynamic. err = fmt.Errorf("%v: %s", response.Status, body) } } @@ -777,7 +777,7 @@ func (c *Client) GetClusterRole(ctx context.Context, apiKey, clusterId, roleName } default: - //nolint:goerr113 // This is intentionally dynamic. + //nolint:err113 // This is intentionally dynamic. err = fmt.Errorf("%v: %s", response.Status, body) } } @@ -807,7 +807,7 @@ func (c *Client) ListClusterRoles(ctx context.Context, apiKey, id string) ([]*Cl } default: - //nolint:goerr113 // This is intentionally dynamic. + //nolint:err113 // This is intentionally dynamic. err = fmt.Errorf("%v: %s", response.Status, body) } } diff --git a/internal/controller/pgupgrade/jobs.go b/internal/controller/pgupgrade/jobs.go index 53420cb8fe..c7b6e4e010 100644 --- a/internal/controller/pgupgrade/jobs.go +++ b/internal/controller/pgupgrade/jobs.go @@ -192,6 +192,7 @@ func (r *PGUpgradeReconciler) generateUpgradeJob( settings := upgrade.Spec.PGUpgradeSettings.DeepCopy() // When jobs is undefined, use one less than the number of CPUs. + //nolint:gosec // The CPU count is clamped to MaxInt32. if settings.Jobs == 0 && feature.Enabled(ctx, feature.PGUpgradeCPUConcurrency) { wholeCPUs := int32(min(math.MaxInt32, largestWholeCPU(upgrade.Spec.Resources))) settings.Jobs = wholeCPUs - 1 diff --git a/internal/controller/postgrescluster/controller_ref_manager.go b/internal/controller/postgrescluster/controller_ref_manager.go index 36f3b67d6d..d229728b12 100644 --- a/internal/controller/postgrescluster/controller_ref_manager.go +++ b/internal/controller/postgrescluster/controller_ref_manager.go @@ -88,7 +88,7 @@ func (r *Reconciler) claimObject(ctx context.Context, postgresCluster *v1beta1.P // At this point the resource has no controller ref and is therefore an orphan. Ignore if // either the PostgresCluster resource or the orphaned resource is being deleted, or if the selector - // for the orphaned resource doesn't doesn't include the proper PostgresCluster label + // for the orphaned resource doesn't include the proper PostgresCluster label _, hasPGClusterLabel := obj.GetLabels()[naming.LabelCluster] if postgresCluster.GetDeletionTimestamp() != nil || !hasPGClusterLabel { return nil diff --git a/internal/controller/postgrescluster/instance.go b/internal/controller/postgrescluster/instance.go index fbb693fd11..473b43ec3e 100644 --- a/internal/controller/postgrescluster/instance.go +++ b/internal/controller/postgrescluster/instance.go @@ -335,7 +335,8 @@ func (r *Reconciler) observeInstances( status.DesiredPGDataVolume = make(map[string]string) for _, instance := range observed.bySet[name] { - status.Replicas += int32(len(instance.Pods)) //nolint:gosec + //nolint:gosec // This slice is always small. + status.Replicas += int32(len(instance.Pods)) if ready, known := instance.IsReady(); known && ready { status.ReadyReplicas++ @@ -752,7 +753,7 @@ func findAvailableInstanceNames(set v1beta1.PostgresInstanceSetSpec, } // Determine whether or not the PVC is associated with an existing instance within the same - // instance set. If not, then the instance name associated with that PVC can be be reused. + // instance set. If not, then the instance name associated with that PVC can be reused. for _, pvc := range setVolumes { pvcInstanceName := pvc.GetLabels()[naming.LabelInstance] instance := observedInstances.byName[pvcInstanceName] diff --git a/internal/controller/postgrescluster/instance_test.go b/internal/controller/postgrescluster/instance_test.go index f31b38624c..3316cbbe2b 100644 --- a/internal/controller/postgrescluster/instance_test.go +++ b/internal/controller/postgrescluster/instance_test.go @@ -1606,7 +1606,6 @@ func TestGenerateInstanceStatefulSetIntent(t *testing.T) { `)) }, }} { - test := test t.Run(test.name, func(t *testing.T) { cluster := test.ip.cluster diff --git a/internal/controller/postgrescluster/patroni_test.go b/internal/controller/postgrescluster/patroni_test.go index 6ba6a30c39..728b75aee3 100644 --- a/internal/controller/postgrescluster/patroni_test.go +++ b/internal/controller/postgrescluster/patroni_test.go @@ -480,7 +480,7 @@ func TestReconcilePatroniStatus(t *testing.T) { instance := &Instance{ Name: instanceName, Runner: runner, } - for i := 0; i < readyReplicas; i++ { + for range readyReplicas { instance.Pods = append(instance.Pods, &corev1.Pod{ Status: corev1.PodStatus{ Conditions: []corev1.PodCondition{{ diff --git a/internal/controller/postgrescluster/pgbackrest.go b/internal/controller/postgrescluster/pgbackrest.go index 2d8d355724..2c0d3d2960 100644 --- a/internal/controller/postgrescluster/pgbackrest.go +++ b/internal/controller/postgrescluster/pgbackrest.go @@ -1180,7 +1180,7 @@ func (r *Reconciler) reconcileRestoreJob(ctx context.Context, var deltaOptFound, foundTarget bool for _, opt := range opts { switch { - case targetRegex.Match([]byte(opt)): + case targetRegex.MatchString(opt): foundTarget = true case strings.Contains(opt, "--delta"): deltaOptFound = true @@ -2799,7 +2799,7 @@ func (r *Reconciler) reconcileStanzaCreate(ctx context.Context, } // Don't record event or return an error if configHashMismatch is true, since this just means // configuration changes in ConfigMaps/Secrets have not yet propagated to the container. - // Therefore, just log an an info message and return an error to requeue and try again. + // Therefore, just log an info message and return an error to requeue and try again. if configHashMismatch { return true, nil diff --git a/internal/controller/standalone_pgadmin/statefulset.go b/internal/controller/standalone_pgadmin/statefulset.go index ed3e3a75f5..b8730b7112 100644 --- a/internal/controller/standalone_pgadmin/statefulset.go +++ b/internal/controller/standalone_pgadmin/statefulset.go @@ -122,7 +122,7 @@ func statefulset( if collector.OpenTelemetryLogsEnabled(ctx, pgadmin) { // Logs for gunicorn and pgadmin write to /var/lib/pgadmin/logs - // so the collector needs access to that that path. + // so the collector needs access to that path. dataVolumeMount := corev1.VolumeMount{ Name: "pgadmin-data", MountPath: "/var/lib/pgadmin", diff --git a/internal/patroni/config.go b/internal/patroni/config.go index 72202fbd78..3e6f7b6c83 100644 --- a/internal/patroni/config.go +++ b/internal/patroni/config.go @@ -465,7 +465,7 @@ func instanceYAML( // created. That value should be injected using the downward API and the // PATRONI_KUBERNETES_POD_IP environment variable. - // Missing here is "ports" which is is connascent with "postgresql.connect_address". + // Missing here is "ports" which is connascent with "postgresql.connect_address". // See the PATRONI_KUBERNETES_PORTS env variable. }, diff --git a/internal/patroni/config_test.go b/internal/patroni/config_test.go index 222c174f40..6f9fd3db2a 100644 --- a/internal/patroni/config_test.go +++ b/internal/patroni/config_test.go @@ -895,7 +895,6 @@ func TestProbeTiming(t *testing.T) { FailureThreshold: 1, }}, } { - tt := tt actual := probeTiming(&v1beta1.PatroniSpec{ LeaderLeaseDurationSeconds: &tt.lease, SyncPeriodSeconds: &tt.sync, diff --git a/internal/pgbackrest/config.go b/internal/pgbackrest/config.go index f4b66fad70..0dd69bbf42 100644 --- a/internal/pgbackrest/config.go +++ b/internal/pgbackrest/config.go @@ -238,7 +238,7 @@ func RestoreCommand(pgdata, hugePagesSetting, fetchKeyCommand string, _ []*corev `read -r max_ptxn <<< "${control##*max_prepared_xacts setting:}"`, `read -r max_work <<< "${control##*max_worker_processes setting:}"`, - // During recovery, only allow connections over the the domain socket. + // During recovery, only allow connections over the domain socket. `echo > /tmp/pg_hba.restore.conf 'local all "postgres" peer'`, // Combine parameters from Go with those detected in Bash. diff --git a/internal/pgbackrest/util_test.go b/internal/pgbackrest/util_test.go index e3c98e0dd7..30ab33fd0d 100644 --- a/internal/pgbackrest/util_test.go +++ b/internal/pgbackrest/util_test.go @@ -80,7 +80,7 @@ func TestCalculateConfigHashes(t *testing.T) { assert.Equal(t, preCalculatedRepo3S3Hash, configHashMap["repo3"]) // call CalculateConfigHashes multiple times to ensure consistent results - for i := 0; i < 10; i++ { + for range 10 { hashMap, hash, err := CalculateConfigHashes(postgresCluster) assert.NilError(t, err) assert.Equal(t, configHash, hash) @@ -92,7 +92,7 @@ func TestCalculateConfigHashes(t *testing.T) { // shuffle the repo slice in order to ensure the same result is returned regardless of the // order of the repos slice shuffleCluster := postgresCluster.DeepCopy() - for i := 0; i < 10; i++ { + for range 10 { repos := shuffleCluster.Spec.Backups.PGBackRest.Repos rand.Shuffle(len(repos), func(i, j int) { repos[i], repos[j] = repos[j], repos[i] @@ -103,7 +103,7 @@ func TestCalculateConfigHashes(t *testing.T) { } // now modify some values in each repo and confirm we see a different result - for i := 0; i < 3; i++ { + for i := range 3 { modCluster := postgresCluster.DeepCopy() switch i { case 0: diff --git a/internal/pgbouncer/postgres.go b/internal/pgbouncer/postgres.go index 202c6bd9be..2d0b675067 100644 --- a/internal/pgbouncer/postgres.go +++ b/internal/pgbouncer/postgres.go @@ -181,7 +181,7 @@ REVOKE ALL PRIVILEGES // - https://www.postgresql.org/docs/current/perm-functions.html `ALTER ROLE :"username" SET search_path TO :'namespace';`, - // Allow the PgBouncer user to to login. + // Allow the PgBouncer user to login. `ALTER ROLE :"username" LOGIN PASSWORD :'verifier';`, // Commit (finish) the transaction. diff --git a/internal/postgres/password/scram.go b/internal/postgres/password/scram.go index bbf8dbcbe6..90eb2a54ad 100644 --- a/internal/postgres/password/scram.go +++ b/internal/postgres/password/scram.go @@ -138,7 +138,7 @@ func (s *SCRAMPassword) isASCII() bool { // iterate through each character of the plaintext password and determine if // it is ASCII. if it is not ASCII, exit early // per research, this loop is optimized to be fast for searching - for i := 0; i < len(s.password); i++ { + for i := range len(s.password) { if s.password[i] > unicode.MaxASCII { return false } diff --git a/internal/upgradecheck/http.go b/internal/upgradecheck/http.go index c2796ffe54..acb4da386b 100644 --- a/internal/upgradecheck/http.go +++ b/internal/upgradecheck/http.go @@ -70,7 +70,7 @@ func checkForUpgrades(ctx context.Context, url, versionString string, backoff wa var headerPayloadStruct *clientUpgradeData // Prep request - req, err := http.NewRequestWithContext(ctx, "GET", url, nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err == nil { // generateHeader always returns some sort of struct, using defaults/nil values // in case some of the checks return errors diff --git a/internal/upgradecheck/http_test.go b/internal/upgradecheck/http_test.go index 6393c305c8..ae84ba16c0 100644 --- a/internal/upgradecheck/http_test.go +++ b/internal/upgradecheck/http_test.go @@ -8,7 +8,6 @@ import ( "context" "encoding/json" "errors" - "fmt" "io" "net/http" "strings" @@ -171,7 +170,7 @@ func TestCheckForUpgradesScheduler(t *testing.T) { // A panicking call funcFoo = func() (*http.Response, error) { - panic(fmt.Errorf("oh no!")) + panic("oh no!") } s := CheckForUpgradesScheduler{ diff --git a/internal/util/secrets_test.go b/internal/util/secrets_test.go index e07a430718..ae5f7f5b05 100644 --- a/internal/util/secrets_test.go +++ b/internal/util/secrets_test.go @@ -55,7 +55,7 @@ func TestGenerateAlphaNumericPassword(t *testing.T) { } previous := sets.Set[string]{} - for i := 0; i < 10; i++ { + for range 10 { password, err := GenerateAlphaNumericPassword(5) assert.NilError(t, err) @@ -80,7 +80,7 @@ func TestGenerateASCIIPassword(t *testing.T) { } previous := sets.Set[string]{} - for i := 0; i < 10; i++ { + for range 10 { password, err := GenerateASCIIPassword(5) assert.NilError(t, err)