fix: resolve code-generator build failure with Go 1.25#112
fix: resolve code-generator build failure with Go 1.25#112ChughShilpa wants to merge 3 commits intodevfrom
Conversation
📝 WalkthroughWalkthroughThe PR updates go.mod to bump several indirect golang.org/x/* module versions (mod, net, sync, sys, term, text, tools) and adds golang.org/x/tools/go/packages/packagestest v0.1.1-deprecated as a new indirect dependency. hack/update-codegen.sh is rewritten to copy the code-generator from the module cache into a writable temporary workspace, create a go.work that includes the project and the copied generator, export GOWORK to that workspace, source kube_codegen.sh from the workspace copy, and run code generation against the workspace copy. hack/generate-apidoc.sh updates the CRD reference doc generator version from v0.1.0 to v0.2.0. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Security considerations
Actionable checklist:
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| go.uber.org/atomic v1.11.0 // indirect | ||
| go.uber.org/multierr v1.11.0 // indirect | ||
| golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect | ||
| golang.org/x/mod v0.20.0 // indirect |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@go.mod`:
- Around line 69-78: Update the golang.org/x/oauth2 dependency from v0.21.0 to
at least v0.27.0 in the go.mod entry for golang.org/x/oauth2 (replace the
current v0.21.0 token), then run a module tidy/resolve (go get/go mod tidy) so
transitive sums and go.sum are updated; verify no other references pin an older
oauth2 version and that builds/tests pass after the bump.
In `@hack/update-codegen.sh`:
- Line 21: The trap invocation currently expands WORK_DIR unquoted which can
cause word-splitting and injection; change the trap command so the handler is
single-quoted and the variable is quoted with a -- sentinel, e.g. replace trap
"rm -rf ${WORK_DIR}" EXIT with trap 'rm -rf -- "$WORK_DIR"' EXIT to defer
expansion and safely handle spaces/special chars in WORK_DIR.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: f8ce5a2d-43fb-4841-8afb-36e9a7f33d42
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (2)
go.modhack/update-codegen.sh
| golang.org/x/mod v0.33.0 // indirect | ||
| golang.org/x/net v0.50.0 // indirect | ||
| golang.org/x/oauth2 v0.21.0 // indirect | ||
| golang.org/x/sync v0.10.0 // indirect | ||
| golang.org/x/sys v0.28.0 // indirect | ||
| golang.org/x/term v0.27.0 // indirect | ||
| golang.org/x/text v0.21.0 // indirect | ||
| golang.org/x/sync v0.19.0 // indirect | ||
| golang.org/x/sys v0.41.0 // indirect | ||
| golang.org/x/term v0.40.0 // indirect | ||
| golang.org/x/text v0.34.0 // indirect | ||
| golang.org/x/time v0.6.0 // indirect | ||
| golang.org/x/tools v0.24.0 // indirect | ||
| golang.org/x/tools v0.42.0 // indirect | ||
| golang.org/x/tools/go/packages/packagestest v0.1.1-deprecated // indirect |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the first patched version of golang.org/x/oauth2 for GO-2025-3488 / GHSA-6v2p-p543-phr9, and which advisory or release notes confirm it?
💡 Result:
The first patched version of golang.org/x/oauth2 for GO-2025-3488 / GHSA-6v2p-p543-phr9 (CVE-2025-22868) is v0.27.0.
Confirmed by:
- GitHub Security Advisory (GHSA-6v2p-p543-phr9): “Patched versions: 0.27.0” and affected
< 0.27.0. [1] - Go Vulnerability Database entry (GO-2025-3488): shows affected
golang.org/x/oauth2/jwsbefore v0.27.0. [2] - Upstream module tag notes for
golang/oauth2v0.27.0: tag message explicitly says “Fixes CVE-2025-22868”. [3]
Sources: [1] (github.com) [2] (pkg.go.dev) [3] (github.com)
Citations:
- 1: GHSA-6v2p-p543-phr9
- 2: https://pkg.go.dev/vuln/GO-2025-3488
- 3: https://github.com/golang/oauth2/tags
High: do not ship this dependency refresh with golang.org/x/oauth2 still on v0.21.0.
Line 71 remains on a release flagged by GO-2025-3488 / GHSA-6v2p-p543-phr9 (CWE-400, CWE-20). Any auth path that parses attacker-controlled or IdP-supplied tokens can be pushed into excessive memory consumption or malformed-token acceptance. Bump golang.org/x/oauth2 to v0.27.0 and re-tidy the module.
Suggested remediation
- golang.org/x/oauth2 v0.21.0 // indirect
+ golang.org/x/oauth2 v0.27.0 // indirect📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| golang.org/x/mod v0.33.0 // indirect | |
| golang.org/x/net v0.50.0 // indirect | |
| golang.org/x/oauth2 v0.21.0 // indirect | |
| golang.org/x/sync v0.10.0 // indirect | |
| golang.org/x/sys v0.28.0 // indirect | |
| golang.org/x/term v0.27.0 // indirect | |
| golang.org/x/text v0.21.0 // indirect | |
| golang.org/x/sync v0.19.0 // indirect | |
| golang.org/x/sys v0.41.0 // indirect | |
| golang.org/x/term v0.40.0 // indirect | |
| golang.org/x/text v0.34.0 // indirect | |
| golang.org/x/time v0.6.0 // indirect | |
| golang.org/x/tools v0.24.0 // indirect | |
| golang.org/x/tools v0.42.0 // indirect | |
| golang.org/x/tools/go/packages/packagestest v0.1.1-deprecated // indirect | |
| golang.org/x/mod v0.33.0 // indirect | |
| golang.org/x/net v0.50.0 // indirect | |
| golang.org/x/oauth2 v0.27.0 // indirect | |
| golang.org/x/sync v0.19.0 // indirect | |
| golang.org/x/sys v0.41.0 // indirect | |
| golang.org/x/term v0.40.0 // indirect | |
| golang.org/x/text v0.34.0 // indirect | |
| golang.org/x/time v0.6.0 // indirect | |
| golang.org/x/tools v0.42.0 // indirect | |
| golang.org/x/tools/go/packages/packagestest v0.1.1-deprecated // indirect |
🧰 Tools
🪛 OSV Scanner (2.3.3)
[HIGH] 71-71: golang.org/x/oauth2 0.21.0: Unexpected memory consumption during token parsing in golang.org/x/oauth2
(GO-2025-3488)
[HIGH] 71-71: golang.org/x/oauth2 0.21.0: golang.org/x/oauth2 Improper Validation of Syntactic Correctness of Input vulnerability
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@go.mod` around lines 69 - 78, Update the golang.org/x/oauth2 dependency from
v0.21.0 to at least v0.27.0 in the go.mod entry for golang.org/x/oauth2 (replace
the current v0.21.0 token), then run a module tidy/resolve (go get/go mod tidy)
so transitive sums and go.sum are updated; verify no other references pin an
older oauth2 version and that builds/tests pass after the bump.
| # Go's MVS resolves golang.org/x/tools from this module's dependency graph | ||
| # instead of using the code-generator's own (older, Go 1.25-incompatible) pin. | ||
| WORK_DIR=$(mktemp -d) | ||
| trap "rm -rf ${WORK_DIR}" EXIT |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd / && find . -name "update-codegen.sh" -type f 2>/dev/null | head -5Repository: opendatahub-io/training-operator
Length of output: 118
🏁 Script executed:
git ls-files | grep -i "update-codegen.sh"Repository: opendatahub-io/training-operator
Length of output: 98
🏁 Script executed:
head -40 hack/update-codegen.sh | cat -nRepository: opendatahub-io/training-operator
Length of output: 1757
🏁 Script executed:
# Verify the trap behavior and check for other potential issues in the changed section
cat -n hack/update-codegen.sh | sed -n '16,27p'Repository: opendatahub-io/training-operator
Length of output: 624
🏁 Script executed:
# Check if mktemp -d output could have problematic characters
# and verify the proposed fix syntax is valid bash
bash -n <(cat << 'BASHTEST'
#!/bin/bash
set -o errexit
set -o nounset
set -o pipefail
WORK_DIR=$(mktemp -d)
# Original form (with double quotes, early expansion)
trap "rm -rf ${WORK_DIR}" EXIT
# Verify trap exits cleanly with original form
exit 0
BASHTEST
)
echo "Original syntax: valid bash"
# Test the proposed fix syntax
bash -n <(cat << 'BASHTEST'
#!/bin/bash
set -o errexit
set -o nounset
set -o pipefail
WORK_DIR=$(mktemp -d)
# Proposed form (with single quotes, late expansion)
trap 'rm -rf -- "$WORK_DIR"' EXIT
# Verify trap exits cleanly with proposed form
exit 0
BASHTEST
)
echo "Proposed syntax: valid bash"Repository: opendatahub-io/training-operator
Length of output: 236
🏁 Script executed:
# Verify ShellCheck confirms the SC2064 issue
shellcheck -f gcc hack/update-codegen.sh 2>&1 | grep -A2 "line 21" || echo "ShellCheck not available or no warning"Repository: opendatahub-io/training-operator
Length of output: 114
Quote the trap variable to prevent word splitting and injection.
Line 21 uses double quotes in the trap handler without quoting the variable in the command body. If WORK_DIR contains spaces or special characters, the unquoted ${WORK_DIR} expansion would be split incorrectly when the trap executes. Per CWE-78 and the coding guideline to quote all variables, use single quotes to defer expansion and quote the variable: trap 'rm -rf -- "$WORK_DIR"' EXIT. The -- prevents the path from being interpreted as a flag.
Suggested fix
-trap "rm -rf ${WORK_DIR}" EXIT
+trap 'rm -rf -- "$WORK_DIR"' EXIT📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| trap "rm -rf ${WORK_DIR}" EXIT | |
| trap 'rm -rf -- "$WORK_DIR"' EXIT |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 21-21: Use single quotes, otherwise this expands now rather than when signalled.
(SC2064)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hack/update-codegen.sh` at line 21, The trap invocation currently expands
WORK_DIR unquoted which can cause word-splitting and injection; change the trap
command so the handler is single-quoted and the variable is quoted with a --
sentinel, e.g. replace trap "rm -rf ${WORK_DIR}" EXIT with trap 'rm -rf --
"$WORK_DIR"' EXIT to defer expansion and safely handle spaces/special chars in
WORK_DIR.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
hack/update-codegen.sh (1)
30-30:⚠️ Potential issue | 🟡 MinorHarden the EXIT trap against CWE-78.
Line 30 expands
WORK_DIRwhen the trap is registered and leaves the path unquoted in the handler. IfTMPDIRis attacker-controlled and contains spaces or shell metacharacters, cleanup can misparse the path or execute unintended shell syntax on exit.Suggested fix
-trap "chmod -R u+w ${WORK_DIR} && rm -rf ${WORK_DIR}" EXIT +trap 'chmod -R u+w -- "$WORK_DIR" && rm -rf -- "$WORK_DIR"' EXITAs per coding guidelines, "Quote all variables to prevent injection."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hack/update-codegen.sh` at line 30, The trap currently expands WORK_DIR during registration and leaves it unquoted; change the trap registration so the handler is single-quoted and uses quoted variable expansion at runtime (e.g. register the trap with a single-quoted command string and quote "$WORK_DIR" in both chmod and rm invocations) so WORK_DIR is not expanded/interpolated at registration time and is protected from shell word-splitting or metacharacter injection; update the existing trap line that references trap and WORK_DIR accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@hack/update-codegen.sh`:
- Line 30: The trap currently expands WORK_DIR during registration and leaves it
unquoted; change the trap registration so the handler is single-quoted and uses
quoted variable expansion at runtime (e.g. register the trap with a
single-quoted command string and quote "$WORK_DIR" in both chmod and rm
invocations) so WORK_DIR is not expanded/interpolated at registration time and
is protected from shell word-splitting or metacharacter injection; update the
existing trap line that references trap and WORK_DIR accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: ce804b67-1a7a-405e-a9cf-816e31b4c43a
📒 Files selected for processing (1)
hack/update-codegen.sh
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
hack/update-codegen.sh (1)
36-41: Hardcoded Go version in workspace may drift from go.mod.
go 1.25is hardcoded in the generatedgo.work. If the project'sgo.modis updated to a newer Go version, this script will require a corresponding manual update.Optional: Extract version dynamically
+GO_VERSION=$(grep -m1 '^go ' go.mod | awk '{print $2}') + cat > "${WORK_DIR}/go.work" << EOF -go 1.25 +go ${GO_VERSION} use ${TRAINING_OPERATOR_ROOT} use ${CODEGEN_COPY} EOF🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hack/update-codegen.sh` around lines 36 - 41, The generated go.work hardcodes "go 1.25" which can drift from the project's go.mod; update hack/update-codegen.sh to derive the Go version dynamically (e.g., parse the "go X.Y" line from the repository's go.mod) into a variable (e.g., GO_VERSION) and use that variable in the here-doc that writes "${WORK_DIR}/go.work" instead of the literal "go 1.25"; keep existing variables WORK_DIR, TRAINING_OPERATOR_ROOT and CODEGEN_COPY and ensure the parser falls back to a sane default if go.mod is missing or the version line isn't found.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hack/generate-apidoc.sh`:
- Around line 29-30: Update the pinned CRD_REF_GEN_VERSION variable from v0.2.0
to v0.3.0 and adjust the install invocation accordingly: change
CRD_REF_GEN_VERSION to "v0.3.0" where it's defined and let the existing go
install github.com/elastic/crd-ref-docs@${CRD_REF_GEN_VERSION} use that new
value; after changing, run the script locally to verify the new binary installs
and behaves as expected.
---
Nitpick comments:
In `@hack/update-codegen.sh`:
- Around line 36-41: The generated go.work hardcodes "go 1.25" which can drift
from the project's go.mod; update hack/update-codegen.sh to derive the Go
version dynamically (e.g., parse the "go X.Y" line from the repository's go.mod)
into a variable (e.g., GO_VERSION) and use that variable in the here-doc that
writes "${WORK_DIR}/go.work" instead of the literal "go 1.25"; keep existing
variables WORK_DIR, TRAINING_OPERATOR_ROOT and CODEGEN_COPY and ensure the
parser falls back to a sane default if go.mod is missing or the version line
isn't found.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 30da4bec-090a-45b9-af8c-ad1cb3303fca
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (3)
go.modhack/generate-apidoc.shhack/update-codegen.sh
| CRD_REF_GEN_VERSION=v0.2.0 | ||
| go install github.com/elastic/crd-ref-docs@${CRD_REF_GEN_VERSION} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the crd-ref-docs version exists on GitHub
curl -s "https://api.github.com/repos/elastic/crd-ref-docs/releases" | jq -r '.[].tag_name' | grep -q "v0.2.0" && echo "v0.2.0 exists" || echo "v0.2.0 NOT FOUND"
# Check for any security advisories
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: GO, package: "github.com/elastic/crd-ref-docs") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
}
}
}'Repository: opendatahub-io/training-operator
Length of output: 322
🏁 Script executed:
#!/bin/bash
# List all available versions for elastic/crd-ref-docs
curl -s "https://api.github.com/repos/elastic/crd-ref-docs/releases" | jq -r '.[].tag_name' | head -20
echo "---"
echo "Checking for v0.1.0 specifically:"
curl -s "https://api.github.com/repos/elastic/crd-ref-docs/releases" | jq -r '.[].tag_name' | grep "v0.1"Repository: opendatahub-io/training-operator
Length of output: 268
🌐 Web query:
elastic/crd-ref-docs latest release version
💡 Result:
The latest release of elastic/crd-ref-docs is v0.3.0 (released February 2, 2026). (github.com)
Citations:
Consider upgrading to v0.3.0 instead of v0.2.0 — elastic/crd-ref-docs v0.3.0 (released February 2, 2026) is now available and should be evaluated before merging. Pinning to v0.2.0 introduces technical debt when a newer stable release exists.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hack/generate-apidoc.sh` around lines 29 - 30, Update the pinned
CRD_REF_GEN_VERSION variable from v0.2.0 to v0.3.0 and adjust the install
invocation accordingly: change CRD_REF_GEN_VERSION to "v0.3.0" where it's
defined and let the existing go install
github.com/elastic/crd-ref-docs@${CRD_REF_GEN_VERSION} use that new value; after
changing, run the script locally to verify the new binary installs and behaves
as expected.
2 participants
What this PR does / why we need it:
When
kube_codegen.shbuilds code-generator tools, itcds into the code-generator module directory and runsgo install, which resolves dependencies from the code-generator's own go.mod — not the training-operator's. The code-generator's pinned golang.org/x/tools version is incompatible with Go 1.25, causing a compilation error intokeninternal.go.
This PR brings two changes:
Create a temporary go.work file in update-codegen.sh that includes
the training-operator module, so Go's MVS considers dependencies
from both modules when building code-generator tools.
Bump golang.org/x/tools from v0.24.0 to v0.42.0 (and transitive
golang.org/x/* dependencies) since v0.24.0 itself is also
incompatible with Go 1.25.
Which issue(s) this PR fixes (optional, in
Fixes #<issue number>, #<issue number>, ...format, will close the issue(s) when PR gets merged):Fixes #
Checklist:
Summary by CodeRabbit