chore: update go version

[Okabe-Junya]

Change List

• Update go version and specify go patch version to clarify forwards compatibility

Note

Go 1.21 Release Notes

To improve forwards compatibility, Go 1.21 now reads the go line in a go.work or go.mod file as a strict minimum requirement: go 1.21.0 means that the workspace or module cannot be used with Go 1.20 or with Go 1.21rc1. This allows projects that depend on fixes made in later versions of Go to ensure that they are not used with earlier versions. It also gives better error reporting for projects that make use of new Go features: when the problem is that a newer Go version is needed, that problem is reported clearly, instead of attempting to build the code and printing errors about unresolved imports or syntax errors.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Okabe-Junya
Once this PR has been reviewed and has the lgtm label, please assign upodroid for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• groups/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Okabe-Junya]

/hold

[MadhavJivrajani]

/retest

[Okabe-Junya]

I'm sorry for not being able to share the progress. I thought I could resolve this issue quickly, but it might be a bit hard.

I tried to update the k/test-infra config to pass the CI, but it pointed to the "latest" tag of the container image. (I thought it could be resolved with a simple update like kubernetes/test-infra#32786, but that doesn’t seem to be the case)

https://github.com/kubernetes/test-infra/blob/7ea48879406f8bfbf7b420498f673f1c2007943f/config/jobs/kubernetes/sig-k8s-infra/k8sio-presubmit.yaml#L14-L15

To investigate this image, I checked k/k8s.io/registry.k8s.io/images/k8s-staging-infra-tools/images.yaml, but I’m still not sure how to update this file (I’m currently looking into it).

Additionally, since "k8s.gcr.io" is deprecated, it would be better to use "registry.k8s.io" as well.

[Okabe-Junya]

I think the simplest approach to update the "latest" of the image would be to set the go version of k/registry.k8s.io to 1.22.4. This would allow us to update the Go version that archeio depends on. Releasing a new archeio would then update the original image.

(This is a somewhat hard task.)

What I'm not sure about is which image the golang image in k/test-infra's config/jobs/kubernetes/sig-k8s-infra/registry.k8s.io/sig-k8s-infra-registry-presubmits.yaml is referring to. Is it from Docker Hub? - If so, I want to refer specific tag

In any case, I think updating the go version of k/registry.k8s.io is a good try (and it would be an easy place to start!).

[Okabe-Junya]

FYI: I have written the context

See https://kubernetes.slack.com/archives/CCK68P2Q2/p1718816152584299

[MadhavJivrajani]

@Okabe-Junya do you think we can directly use a golang image itself for the k8sio-groups-test? It would decouple it from bumping registry.k8s.io which would have a wider impact.

[BenTheElder]

bumping this doesn't necessarily change the version of go being used to compile, this changes the version of go required to build the module (and what go features are enabled / compatibility changes)

are we intending to use newer go features?

[Okabe-Junya]

Are we intending to use newer Go features?

We might not need to use the new features introduced in Go 1.18 and later such as generics, slog, and some built-in functions and so on ... immediately (of course, there would be some if we look for them).

However, from the perspective of performance and security, I don’t see any reason to oppose updating the software version (unless there are breaking changes - which is not the case with Go).

Is there a clear reason why we shouldn’t update?

[Okabe-Junya]

(Of course, I understand that the issues will be mitigated by GOTOOLCHAIN…)

[BenTheElder]

However, from the perspective of performance and security, I don’t see any reason to oppose updating the software version (unless there are breaking changes - which is not the case with Go).

This isn't a software version bump, this line means the go language version, not the toolchain version.

[BenTheElder]

(like the difference between upgrading gcc and going from --std=c99 to --std=c14, this is the latter)

[BenTheElder]

The image should be able to have multiple modules at different versions.

I think the simplest approach to update the "latest" of the image would be to set the go version of k/registry.k8s.io to 1.22.4. This would allow us to update the Go version that archeio depends on. Releasing a new archeio would then update the original image.

We shouldn't be coupling these. Updating images in this repo should not depend on the go version in registry.k8s.io, which automatically manages it's own go version with .go-version and associated tooling (and probably just GOTOOLCHAIN in the future)

[Okabe-Junya]

Do you think we can directly use a golang image itself for the k8sio-groups-test?

As BenTheElder mentioned, using GOTOOLCHAIN seems to be the best solution. This would indeed remove dependencies on the Go version of registry.k8s.io. Would specifying it in the following spec solve this issue? (or will that be in the future?)

https://github.com/kubernetes/test-infra/blob/b24a97a3640d4a53c7ff02f9cbe7ccc7036d18ad/config/jobs/kubernetes/sig-k8s-infra/k8sio-presubmit.yaml#L13-L32

And it looks like gcr.io/k8s-staging-infra-tools/k8s-infra is using Go 1.20.5 internally. Do you think it is reasonable to update this 1.20.5 to 1.21 or later? - The reason, of course, is that we want to use GOTOOLCHAIN.

k8s.io/images/k8s-infra/Dockerfile

Line 32 in 9f67194

ARG GO_VERSION=1.20.5

[Okabe-Junya]

friendly cc @BenTheElder @MadhavJivrajani

I would like to move this PR forward (or close it if these changes are not needed). In any case, I want to advance this discussion. WDYT?

[BenTheElder]

I think using gotoolchain is reasonable but it should be set here with the code sources and the version in that spec should just be anything new enough to provide toolchain support

setting it in that spec means we're still unable to test PRs that update the go version

[Okabe-Junya]

Thank you BenTheElder!

Indeed, it seems better to make changes along with the code source, just like the maintenance PR you created.

• https://github.com/kubernetes/registry.k8s.io/pull/284/files

I will reconsider this, thanks

[andyzhangx]

can we use go version 1.22.5 directly since there is a new CVE:

┌─────────┬────────────────┬──────────┬────────┬───────────────────┬─────────────────┬──────────────────────────────────────────────────────────┐
│ Library │ Vulnerability  │ Severity │ Status │ Installed Version │  Fixed Version  │                          Title                           │
├─────────┼────────────────┼──────────┼────────┼───────────────────┼─────────────────┼──────────────────────────────────────────────────────────┤
│ stdlib  │ CVE-2024-24791 │ MEDIUM   │ fixed  │ 1.22.4            │ 1.21.12, 1.22.5 │ net/http: Denial of service due to improper 100-continue │
│         │                │          │        │                   │                 │ handling in net/http                                     │
│         │                │          │        │                   │                 │ https://avd.aquasec.com/nvd/cve-2024-24791               │
└─────────┴────────────────┴──────────┴────────┴───────────────────┴─────────────────┴──────────────────────────────────────────────────────────┘

[k8s-ci-robot]

@Okabe-Junya: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-k8sio-groups-test	ce99707	link	true	/test pull-k8sio-groups-test

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

[Okabe-Junya]

I've reconsidered this but still couldn't come up with a good solution.

Is there a way to solve the problem you mentioned,

setting it in that spec means we're still unable to test PRs that update the go version?

(By "go version" here, I mean GO_VERSION in k8s.io/images/k8s-infra/Dockerfile.)

If we could merge a simple one-line PR that updates this go version, I think this PR could move forward but...

[Okabe-Junya]

I have a question for you, @BenTheElder

In https://github.com/kubernetes/k8s.io/pull/5490/files, it looks like many dependencies are being bumped at once. If we can merge this PR, wouldn't it also be possible to bump the GO_VERSION in the Dockerfile like this?

Of course, I understand that such a PR would (might) affect the entire Kubernetes community. If it's better for a TL or maintainer to handle this, I'd like to ask for your help, thanks
(Or, if there are any SIG or community guidelines for creating such PRs, I will, of course, follow them)

[Okabe-Junya]

/cc BenTheElder
for kindly taking a look

[BenTheElder]

Sorry I was out friday and there's been a lot going on with the CI migration and the k8s release.

I'm still not sure what we're trying to solve, are we trying to use go 1.22 language features?

it looks like many dependencies are being bumped at once. If we can merge this PR, wouldn't it also be possible to bump the GO_VERSION in the Dockerfile like this?

Yes, though I think we should switch to toolchain so that the go version is tested before any container image is built.

can we use go version 1.22.5 directly since there is a new CVE:

That's not necessarily controlled by changing the go statement or any of the other changes currently present.

The go statement specifies the language spec, the compiler version is incidental.

Increasing the language version means changing the behavior of the language, something we may not need or desire at the time and also prevents compiling with older versions. If we just want binaries built from a newer version, these are separate changes and we should improve how we manage that.

[BenTheElder]

cc @upodroid @ameukam

[BenTheElder]

I think this is fine, and we can merge it, but I'm not sure it solves any issues, and I'd prefer to not add additional vectors for issues at this moment, versus solving the toolchain management issue.

[ameukam]

I also to understand why we need to do this. Concerns about a MEDIUM CVE on this tool are not really justified since this only executed on a postsubmit.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

• Mark this PR as fresh with /remove-lifecycle stale
• Close this PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[Okabe-Junya]

Thank you, and I apologize for the delayed response.
(I missed the notification for this PR and noticed it due to the lifecycle message from k8s-triage-robot)

I studied Go version management in more detail and was finally able to understand your intent.

but I’m not sure it solves any issues, and I’d prefer to not add additional vectors for issues at this moment,

Given Go backward compatibility and toolchain, it seems that updating the go directive is indeed less important; rather, it would be preferable for it to specify the oldest version at which the build is still possible.

Thank you for your support. I will close this PR.

/close

[k8s-ci-robot]

@Okabe-Junya: Closed this PR.

In response to this:

Thank you, and I apologize for the delayed response.
(I missed the notification for this PR and noticed it due to the lifecycle message from k8s-triage-robot)

I studied Go version management in more detail and was finally able to understand your intent.

but I’m not sure it solves any issues, and I’d prefer to not add additional vectors for issues at this moment,

Given Go backward compatibility and toolchain, it seems that updating the go directive is indeed less important; rather, it would be preferable for it to specify the oldest version at which the build is still possible.

Thank you for your support. I will close this PR.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.