Chores: set go.mod in main to 1.24 and one in azclient to 1.23 #8376

MartinForReal · 2025-02-19T08:07:11Z

What type of PR is this?

/kind testing

What this PR does / why we need it:

Chores: set go to 1.23.6

Which issue(s) this PR fixes:

Fixes #8535

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

Copilot

Copilot reviewed 5 out of 8 changed files in this pull request and generated no comments.

Files not reviewed (3)

.github/workflows/dependency-review.yml: Language not supported
.github/workflows/lint.yaml: Language not supported
go.mod: Language not supported

github-advanced-security · 2025-02-19T08:53:20Z

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

MartinForReal · 2025-02-19T12:26:22Z

/retest

MartinForReal · 2025-02-19T12:40:37Z

/retest

feiskyer · 2025-02-19T13:06:58Z

@MartinForReal could you check why all tests are failing?

MartinForReal · 2025-02-19T13:20:46Z

Feb 19 12:19:49.800021 capz-xl2x8u-control-plane-bvk7r kubelet[1664]: E0219 12:19:49.799898 1664 pod_workers.go:1301] "Error syncing pod, skipping" err="failed to "StartContainer" for "kube-apiserver" with CrashLoopBackOff: "back-off 1m20s restarting failed container=kube-apiserver pod=kube-apiserver-capz-xl2x8u-control-plane-bvk7r_kube-system(3331e7094b62eb3737eb8f15c80098c3)"" pod="kube-system/kube-apiserver-capz-xl2x8u-control-plane-bvk7r" podUID="3331e7094b62eb3737eb8f15c80098c3"

MartinForReal · 2025-02-19T13:20:54Z

apiserver crashed.

MartinForReal · 2025-02-20T04:52:39Z

/retest

MartinForReal · 2025-02-20T04:52:41Z

/retest

MartinForReal · 2025-02-20T04:54:11Z

/retest

MartinForReal · 2025-02-21T09:09:24Z

kubernetes/kubernetes#129688

MartinForReal · 2025-02-21T09:09:34Z

/test all

MartinForReal · 2025-02-24T01:27:40Z

/retest

feiskyer · 2025-02-24T01:29:21Z

Chores: set go to 1.23.6

The PR is actually upgrading Go to 1.24. Could you we use 1.23.6 until k8s repos officially support it?

feiskyer · 2025-02-24T01:30:36Z

.github/workflows/staticcheck.yaml

@@ -103,14 +197,3 @@ jobs:
        with:
          sarif_file: 'trivy-health-probe-proxy-linux-results.sarif'
          category: health-probe-proxy-linux-image
-
-      - name: Run Trivy vulnerability scanner in repo mode


And why making such huge change to github pipelines? I think we should scan the whole repo

please see line 65

All these actions share two steps: checkout and setup go.
It will save time.

MartinForReal · 2025-02-24T01:36:49Z

Chores: set go to 1.23.6

The PR is actually upgrading Go to 1.24. Could you we use 1.23.6 until k8s repos officially support it?

kubernetes/kubernetes#129688 this pr has been approved.

feiskyer · 2025-02-24T01:54:33Z

that's not merged yet, and full e2e tests are not run yet. Let's wait a while for community validations in case there are breaking changes or behavior changes broken k8s.

/hold

MartinForReal · 2025-03-11T05:07:19Z

/unhold

MartinForReal · 2025-03-11T05:07:35Z

k/k has switched to go1.24

coveralls · 2025-03-11T05:49:22Z

coverage: 74.55%. remained the same
when pulling cc4d10a on MartinForReal:shafan/go
into 027724e on kubernetes-sigs:master.

Signed-off-by: Fan Shang Xiang <[email protected]>

Copilot

PR Overview

This PR updates several CI/CD workflows to use the go.mod file for determining the Go version instead of hardcoding it, ensuring that the correct Go version (1.24 in main and 1.23.6 in azclient) is used consistently throughout the project. It also enables options like check-latest and caching of go.sum, and makes a change in the Cloud Build configuration regarding image pinning.

Reviewed Changes

File	Description
.github/workflows/lint-azclient.yaml	Updates Go version configuration for azclient linting
.github/workflows/update-vendor-license.yml	Switches from go-version to go-version-file with caching settings
.github/workflows/codeql-analysis.yml	Uses go-version-file and caching settings in CodeQL analysis
.github/workflows/release.yaml	Updates Go version configuration and cache dependency settings
.github/workflows/trivy.yaml	Uses go-version-file and caching settings in Trivy scan
.github/workflows/lint.yaml	Updates Go version setup and includes additional path filters
cloudbuild.yaml	Removes digest from the gcb-docker-gcloud image reference
.github/workflows/codeql-azclient.yml	Removes additional cache dependency paths for client-gen and configloader

Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

.github/workflows/codeql-azclient.yml:59

Removing the cache dependency path for 'pkg/azclient/client-gen/go.sum' might affect dependency caching; please confirm if this removal is intentional.

-          pkg/azclient/client-gen/go.sum

.github/workflows/codeql-azclient.yml:60

Removing the cache dependency path for 'pkg/azclient/configloader/go.sum' might affect dependency caching; please confirm if this removal is intentional.

-          pkg/azclient/configloader/go.sum

cloudbuild.yaml

andyzhangx · 2025-03-11T07:51:29Z

@kaovilai can you take a look? thanks.

feiskyer · 2025-03-11T08:33:21Z

.github/workflows/lint.yaml

+      - 'go.*'
+      - 'hack/verify-updates.sh'
+      - '!vendor/**'
+      - '!pkg/azclient/**'


why excluding azclient?

a new aclient job is added. lint-azclient.yaml

these two modules are using different go versions. two actions are better.

feiskyer · 2025-03-11T08:34:28Z

pkg/azclient/cache/go.mod

-go 1.23.2
+go 1.23.0
+
+toolchain go1.24.0


why GO 1.23 using 1.24 toolchain here?

to mitigate go cve. please see @kaovilai 's comment in issue #8535

will take a look thx!

Just for context.. 1.24 or 1.24.0 is also fine. What I want to avoid is use of go <go-directive> bump for CVEs.

CVE should use separate toolchain line to prevent dependents compatibility breakages., keeping go directive with patch .0 or not at all.. ie. 1.24 or 1.24.0.

Toolchain only affect the curent repo and leave it up to depedents if they want to match it.

kaovilai

works for me.

MartinForReal · 2025-03-12T02:44:55Z

/assign @andyzhangx

andyzhangx

/lgtm

k8s-ci-robot · 2025-03-12T05:24:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andyzhangx, kaovilai, MartinForReal

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [andyzhangx]

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

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/testing cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 19, 2025

k8s-ci-robot requested review from jwtty and lzhecheng February 19, 2025 08:07

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 19, 2025

MartinForReal requested a review from Copilot February 19, 2025 08:07

Copilot AI reviewed Feb 19, 2025

View reviewed changes

MartinForReal changed the title ~~Chores: set go to 1.23.6~~ Chores: set go.mod in main to 1.24 and one in azclient to 1.23.6 Feb 21, 2025

feiskyer reviewed Feb 24, 2025

View reviewed changes

k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Feb 24, 2025

k8s-ci-robot removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Mar 11, 2025

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 11, 2025

set go to 1.23 in azclient and 1.24 in cloud provider

cc4d10a

Signed-off-by: Fan Shang Xiang <[email protected]>

MartinForReal requested a review from Copilot March 11, 2025 06:37

Copilot AI reviewed Mar 11, 2025

View reviewed changes

cloudbuild.yaml Show resolved Hide resolved

feiskyer reviewed Mar 11, 2025

View reviewed changes

MartinForReal changed the title ~~Chores: set go.mod in main to 1.24 and one in azclient to 1.23.6~~ Chores: set go.mod in main to 1.24 and one in azclient to 1.23 Mar 11, 2025

kaovilai approved these changes Mar 11, 2025

View reviewed changes

k8s-ci-robot assigned andyzhangx Mar 12, 2025

andyzhangx approved these changes Mar 12, 2025

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 12, 2025

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 12, 2025

k8s-ci-robot merged commit fa24def into kubernetes-sigs:master Mar 12, 2025
20 checks passed

MartinForReal deleted the shafan/go branch March 13, 2025 04:36

gnufied mentioned this pull request Apr 22, 2025

Use lowest go directive possible #8535

Closed

Chores: set go.mod in main to 1.24 and one in azclient to 1.23 #8376

Chores: set go.mod in main to 1.24 and one in azclient to 1.23 #8376

Uh oh!

Conversation

MartinForReal commented Feb 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

github-advanced-security bot commented Feb 19, 2025

Uh oh!

MartinForReal commented Feb 19, 2025

Uh oh!

MartinForReal commented Feb 19, 2025

Uh oh!

feiskyer commented Feb 19, 2025

Uh oh!

MartinForReal commented Feb 19, 2025

Uh oh!

MartinForReal commented Feb 19, 2025

Uh oh!

MartinForReal commented Feb 20, 2025

Uh oh!

MartinForReal commented Feb 20, 2025

Uh oh!

MartinForReal commented Feb 20, 2025

Uh oh!

MartinForReal commented Feb 21, 2025

Uh oh!

MartinForReal commented Feb 21, 2025

Uh oh!

MartinForReal commented Feb 24, 2025

Uh oh!

feiskyer commented Feb 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MartinForReal commented Feb 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

feiskyer commented Feb 24, 2025

Uh oh!

MartinForReal commented Mar 11, 2025

Uh oh!

MartinForReal commented Mar 11, 2025

Uh oh!

coveralls commented Mar 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

PR Overview

Reviewed Changes

Uh oh!

Uh oh!

andyzhangx commented Mar 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MartinForReal commented Feb 19, 2025 •

edited

Loading

MartinForReal commented Feb 24, 2025 •

edited

Loading

coveralls commented Mar 11, 2025 •

edited

Loading