Skip to content

chore: update to go1.24 #3467

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

chore: update to go1.24 #3467

wants to merge 1 commit into from

Conversation

rbtr
Copy link
Contributor

@rbtr rbtr commented Mar 3, 2025

notably, this kills hack/tools in favor of the new tools directive in the go.mod.
Go 1.24 now has FIPS certification of the 1P crypto, but we are currently still required to use MSFT Go...

@Copilot Copilot AI review requested due to automatic review settings March 3, 2025 21:46
@rbtr rbtr requested a review from rayaisaiah March 3, 2025 21:46
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Overview

This PR updates the Go version configuration across multiple CI workflows to align with the upgrade to Go 1.24, while also removing an obsolete tools registration file and updating some dependency configurations.

  • Update go-version settings from fixed version ranges (e.g. "^1.23") to non-deterministic strings ("latest" or "stable").
  • Remove the build/tools/tools.go file, which previously imported various tool dependencies.
  • Adjust the invocation of go-junit-report in unit test pipelines.

Reviewed Changes

File Description
.github/workflows/cyclonus-netpol-extended-nightly-test.yaml Updated go-version from "^1.23" to "latest" for Go version configuration.
.github/workflows/cyclonus-netpol-test.yaml Updated go-version from '^1.23' to "latest".
.github/workflows/golangci.yaml Switched go-version to "stable" and updated golangci-lint version to "latest".
.github/workflows/codeql.yaml Updated go-version from "1.23" to "latest".
.github/dependabot.yaml Removed a dependency block for "/build/tools".
.github/workflows/crdgen.yaml Removed matrix configuration and set a fixed runner and go-version to "stable".
.pipelines/templates/unit-tests.stages.yaml Changed go-junit-report invocation to use "go tool go-junit-report".
.pipelines/templates/run-unit-tests.yaml Changed go-junit-report invocation to use "go tool go-junit-report".
build/tools/tools.go Deleted file registering tool dependencies.

Copilot reviewed 48 out of 48 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

build/tools/tools.go:1

  • Removal of the tools.go file may affect dependencies on tool registration; please verify that no parts of the build or tooling process rely on these imports.
package tools

@rbtr rbtr enabled auto-merge March 3, 2025 21:53
@rbtr
Copy link
Contributor Author

rbtr commented Mar 4, 2025

/azp run Azure Container Networking PR

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Comment on lines -15 to +16
strategy:
matrix:
go-version: ['1.22', '1.23']
os: [ubuntu-latest]
name: CRDs are Generated
runs-on: ${{ matrix.os }}
runs-on: ubuntu-latest
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going from matrix to a single job has changed the required status checks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This applies to all workflows

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's what I want. the required status checks will be updated to match the new workflow names

os: [ubuntu-latest, windows-latest]
name: Lint
runs-on: ${{ matrix.os }}
steps:
- uses: actions/setup-go@v5
with:
go-version: ${{ matrix.go-version }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break all PRs every go minor version bump until we bump our go.mod to match. Is that acceptable?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we can do go-version-file: go.mod here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break all PRs every go minor version bump

I'm not sure that's true, but I'm amenable to the go-version-file change anyway

Comment on lines +6 to +7
# skopeo inspect docker://mcr.microsoft.com/oss/go/microsoft/golang:1.24-cbl-mariner2.0 --format "{{.Name}}@{{.Digest}}"
FROM --platform=linux/${ARCH} mcr.microsoft.com/oss/go/microsoft/golang@sha256:15c9b9b8449f55446243ce20c5d3808cc18625d0b358d70aaad402fb73c0766f AS go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This image tag will be constantly overwritten. Using the sha here prevents us from pulling the newer versions which is counter to what I believe this is trying to do. New sha is 605d0a6f05734845927f450a62e081bd19c03dbe1fdbf993cfedc8506c9192b4

Also, we want reproducible and expected builds. Why not specify 1.24.X image tag?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the sha here prevents us from pulling the newer versions

Yes, this is the goal with sha pinning. Pulling images via sha will allow us to control the exact image that we want to use across builds, hence reproducible and expected 🙂

Copy link
Contributor

@jpayne3506 jpayne3506 Mar 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to be able to answer this question. What is the expected patch version?

I want to be able to update this later and know when I pull the sha for golang:1.24-cbl-mariner2.0 it is a certain version. Stdlib CVE updates have taken up residence in my mind.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stdlib CVE updates come in new patch versions. Do you want to have to update the hint with that new patch version, or just rerun rerun it and copy/paste the SHA?
See also #3397

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thought about this more and if I really wanted detailed information on the patch version being used, I would only care after trivy has informed me I need to update.

@jpayne3506 jpayne3506 mentioned this pull request Mar 17, 2025
4 tasks
huntergregory
huntergregory previously approved these changes Mar 20, 2025
Copy link
Contributor

@huntergregory huntergregory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we run npm conformance?

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>
Copy link

github-actions bot commented Apr 4, 2025

This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions github-actions bot added the stale Stale due to inactivity. label Apr 4, 2025
Copy link

Pull request closed due to inactivity.

@github-actions github-actions bot closed this Apr 11, 2025
auto-merge was automatically disabled April 11, 2025 00:01

Pull request was closed

@github-actions github-actions bot deleted the chore/go1.24 branch April 11, 2025 00:01
@rbtr rbtr removed the stale Stale due to inactivity. label Apr 11, 2025
@rbtr rbtr restored the chore/go1.24 branch April 11, 2025 16:05
@rbtr rbtr reopened this Apr 11, 2025
Copy link

This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions github-actions bot added the stale Stale due to inactivity. label Apr 30, 2025
@rbtr rbtr removed the stale Stale due to inactivity. label Apr 30, 2025
Copy link

This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions github-actions bot added the stale Stale due to inactivity. label May 15, 2025
Copy link

Pull request closed due to inactivity.

@github-actions github-actions bot closed this May 23, 2025
@github-actions github-actions bot deleted the chore/go1.24 branch May 23, 2025 00:01
@rbtr rbtr self-assigned this May 29, 2025
@rbtr rbtr added go Pull requests that update Go code exempt-stale Keep this fresh dependencies Dependencies only. and removed stale Stale due to inactivity. labels May 29, 2025
@rbtr rbtr restored the chore/go1.24 branch May 29, 2025 23:00
@rbtr rbtr reopened this May 29, 2025
Copy link

This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions github-actions bot added the stale Stale due to inactivity. label Jun 13, 2025
@rbtr rbtr removed the stale Stale due to inactivity. label Jun 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Dependencies only. exempt-stale Keep this fresh go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants