Skip to content

chore(go): build with Go 1.27.1 and clear the lint backlog it surfaces - #422

Open
markussiebert wants to merge 1 commit into
renovate/major-all-majorfrom
chore/go-1.27
Open

markussiebert wants to merge 1 commit into
renovate/major-all-majorfrom
chore/go-1.27

Conversation

@markussiebert

Copy link
Copy Markdown
Collaborator

Description of your changes

Stacked on #410 — review that one first.

Builds with Go 1.27.1 and clears the lint findings that come with it.

The toolchain directive moves to go1.27.1 in both modules while the go directive
stays on the language version 1.27.0, so every build uses the patch release without
raising the minimum for consumers. CI and the promote workflow pin the same 1.27.1.

Raising the go directive forces a golangci-lint built with Go >= the target, because
golangci-lint refuses to run otherwise. The previous pins were built with go1.26.x, so
both move to 2.14.0, built with go1.27.0. The Makefile and the workflow pinned
different versions before; they now agree.

That newer staticcheck changes three things.

Three existing SA1019 exclusions stopped matching, because deprecations are now reported
fully qualified: SA1019: mgr.GetEventRecorderFor became
SA1019: (sigs.k8s.io/controller-runtime.Manager).GetEventRecorderFor. The patterns are
relaxed to match either form, the intent is unchanged.

Deprecated struct fields are detected where they were not before. The Project API still
exposes those fields and the client has to keep reading and sending them, so the affected
lines get the same //nolint:staticcheck the surrounding code already uses. Dropping the
fields is an API change, tracked in #242.

client-go v3 deprecates gitlab.Ptr in favour of new(), which Go 1.26 opened up to
value expressions, and annotates it with //go:fix inline. govet reported all 55 call
sites because it cannot apply the fix itself; they are rewritten with gofmt -r, which
also let one pointless *gitlab.Ptr(x) deref collapse to x.

Ten directives are dropped rather than added. client-go v3.16.2, which #410 picks up,
removed the incorrect "No longer supported in recent versions." annotations from
CIForwardDeploymentEnabled, IssuesTemplate, MergeRequestsTemplate and
ServiceDeskEnabled, so the suppressions on those call sites no longer suppress anything.
nolintlint runs without allow-unused and reports them, which would fail the lint gate.

A note on the suppressions

This adds a lot of //nolint:staticcheck, and they are not all equal. Behind the 73
hand-maintained directives in pkg and apis are 73 real SA1019 findings — nolintlint
confirms none is unused — but they fall into three groups: fields this API deprecated
itself and still has to serve, client-go fields with a genuine replacement that we have
not migrated to yet, and client-go annotations that were simply wrong.

The third group is shrinking on its own: the ten removed here, plus the container
expiration policy ones resolved in #410, all came from
client-go#2303.

Worth saying plainly: one of the directives this change originally added was a true
positive. It sat on a line writing ContainerExpirationPolicyAttributes.NameRegex, which
client-go overwrote before encoding, and silencing it is how the regression in #418 got
in. Bulk-suppressing a backlog can bury real findings, so the remaining ones deserve to be
read individually rather than waved through.

Follow-up, not in this PR

The migrations that would actually reduce the second group: tagList → topics,
packagesEnabled → packageRegistryAccessLevel (replacement added in #419),
emailsDisabled → emailsEnabled, markedForDeletionAt → markedForDeletionOn.

How has this code been tested

golangci-lint 2.14.0 reports 0 issues, go build ./... and go test ./... pass, and
make generate leaves the tree clean, so check-diff is reproducible. The lint gate runs
in CI for this branch, which is the point of the version bump.

I have:

  • Read and followed Crossplane's contribution process.
  • Run make generate and golangci-lint to ensure this PR is ready for review.

Set the toolchain directive to go1.27.1 in both modules while leaving the go
directive on the language version 1.27.0, so every build uses the patch release
without raising the minimum for consumers. CI and the promote workflow pin the
same 1.27.1.

Raising the go directive forces a golangci-lint built with Go >= the target,
because golangci-lint refuses to run otherwise. The previous pins were built
with go1.26.x, so both move to 2.14.0 (built with go1.27.0). The Makefile and
the workflow pinned different versions before; they now agree.

That newer staticcheck surfaces three things:

  - Three existing SA1019 exclusions stopped matching, because staticcheck now
    reports deprecations fully qualified: "SA1019: mgr.GetEventRecorderFor"
    became "SA1019: (sigs.k8s.io/controller-runtime.Manager).GetEventRecorderFor".
    The patterns are relaxed to match either form; the intent is unchanged.

  - Deprecated struct fields are now detected where they were not before. The
    Project API still exposes those fields, so the client has to keep reading
    and sending them; the affected lines get the same //nolint:staticcheck the
    surrounding code already uses. Dropping the fields is an API change,
    tracked in #242.

  - client-go v3 deprecates gitlab.Ptr in favour of new(), which Go 1.26 opened
    up to value expressions, and annotates it with //go:fix inline. govet
    reported all 55 call sites because it cannot apply the fix itself. They are
    rewritten to new() via gofmt -r, which also let one pointless
    *gitlab.Ptr(x) deref collapse to x.

Ten directives are dropped rather than added. client-go v3.16.2 removed the
incorrect "No longer supported in recent versions." annotations from
CIForwardDeploymentEnabled, IssuesTemplate, MergeRequestsTemplate and
ServiceDeskEnabled, so the suppressions on those call sites no longer suppress
anything. nolintlint is enabled without allow-unused and reports them, which
would fail the lint gate.

Go 1.27 also changed gofmt's struct literal alignment, so the two files that
differ are reformatted and the generated cluster-scope mirrors follow. The
generated settings converters pick up the rate limit fields v3.17.0 added to
UpdateSettingsOptions.

golangci-lint 2.14.0 reports no issues, go build and go test pass, and make
generate leaves the tree clean.

Assisted-by: Kiro:claude-opus-5
Signed-off-by: Markus Siebert <markus.siebert@deutschebahn.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant