WIP: Rebase 1.37 - #31505
Conversation
…vendor Signed-off-by: jubittajohn <jujohn@redhat.com>
Signed-off-by: jubittajohn <jujohn@redhat.com>
Signed-off-by: jubittajohn <jujohn@redhat.com>
Signed-off-by: jubittajohn <jujohn@redhat.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jubittajohn The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
WalkthroughThe PR updates Go dependencies and Kubernetes/OpenShift replacements. It adapts extended tests to newer context-aware APIs, changes CSI skip handling, removes a lifecycle import, and refreshes test container images. ChangesDependency and test alignment
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@go.mod`:
- Line 106: Remove the vulnerable gopkg.in/src-d/go-git.v4 dependency and the
unused GitStorage.repo field; update GitStorage’s PlainInit and PlainOpen usage
to the existing Git CLI path, or migrate to github.com/go-git/go-git/v5 at
version 5.13.0 or newer while preserving local repository behavior. Regenerate
dependencies with go mod tidy and go mod vendor, then run make verify and make
check.
In `@test/extended/cpu_partitioning/crio.go`:
- Line 375: Replace context.Background() with the caller-provided ctx in every
TContext call used by collectContainerInfo and the four GPU validator methods.
Update test/extended/cpu_partitioning/crio.go:375 and
test/extended/node/dra/nvidia/gpu_validator.go:50, 98, 326, and 356; make no
other changes.
In `@test/extended/testdata/bindata.go`:
- Line 51732: Update the reencrypt serving-cert NGINX configuration to replace
the removed ssl on directive with the supported TLS configuration, regenerate
the embedded bindata artifact, then run make verify and make check to validate
the change.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| google.golang.org/grpc v1.81.1 | ||
| google.golang.org/grpc v1.82.1 | ||
| gopkg.in/ini.v1 v1.62.0 | ||
| gopkg.in/src-d/go-git.v4 v4.13.1 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- go.mod target and replacement context ---'
awk 'NR >= 95 && NR <= 115 {printf "%5d %s\n", NR, $0} NR >= 430 && NR <= 450 {printf "%5d %s\n", NR, $0}' go.mod
printf '%s\n' '--- dependency references in repository ---'
rg -n --hidden -S 'gopkg\.in/src-d/go-git\.v4|src-d/go-git|go-git' \
-g '!CLAUDE.md' -g '!*.sum' -g '!*.mod' . || true
printf '%s\n' '--- module files and vendor presence ---'
git ls-files 'go.mod' 'go.sum' 'vendor/**' | sed -n '1,80p'
printf '%s\n' '--- go.mod diff summary and target diff ---'
git diff --stat -- go.mod go.sum vendor
git diff -- go.mod | sed -n '1,220p'
printf '%s\n' '--- go tool availability and module why ---'
if command -v go >/dev/null 2>&1; then
go version
go mod why -m gopkg.in/src-d/go-git.v4 2>&1 || true
else
echo 'go is unavailable'
fi
printf '%s\n' '--- OSV record ---'
if command -v curl >/dev/null 2>&1; then
curl -fsSL --max-time 20 \
'https://api.osv.dev/v1/vulns/GO-2024-2456' \
| python3 -c 'import json,sys; d=json.load(sys.stdin); print(json.dumps({"id":d.get("id"),"summary":d.get("summary"),"details":d.get("details"),"aliases":d.get("aliases"),"affected":d.get("affected")}, indent=2))' \
|| true
else
echo 'curl is unavailable'
fiRepository: openshift/origin
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- module why ---'
if command -v go >/dev/null 2>&1; then
go mod why -m gopkg.in/src-d/go-git.v4 2>&1 || true
else
echo 'go is unavailable'
fi
printf '%s\n' '--- application Git store ---'
sed -n '1,260p' pkg/resourcewatch/git/git_store.go
printf '%s\n' '--- non-vendor call sites ---'
rg -n -S 'git_store|git\.Clone|git\.PlainOpen|\.Fetch\(|\.Pull\(|\.Push\(|Remote\(|gopkg\.in/src-d/go-git\.v4' \
--glob '!vendor/**' --glob '!*.sum' --glob '!*.mod' . | sed -n '1,180p'
printf '%s\n' '--- vendor network and filesystem entry points ---'
rg -n -S 'func (Clone|PlainOpen|Fetch|Pull|Push)|func (Decode|decode)|filepath\.(Join|Clean)|os\.(Create|OpenFile|MkdirAll|WriteFile)|Command|exec\.' \
vendor/gopkg.in/src-d/go-git.v4 | sed -n '1,220p'
printf '%s\n' '--- OSV package query ---'
if command -v curl >/dev/null 2>&1; then
curl -fsSL --max-time 20 -X POST 'https://api.osv.dev/v1/query' \
-H 'content-type: application/json' \
--data '{"package":{"name":"gopkg.in/src-d/go-git.v4","ecosystem":"Go"},"version":"v4.13.1"}' \
| python3 -c 'import json,sys; d=json.load(sys.stdin); print(json.dumps(d, indent=2))' \
|| true
printf '%s\n' '--- cited OSV record ---'
curl -fsSL --max-time 20 'https://api.osv.dev/v1/vulns/GO-2024-2456' \
| python3 -c 'import json,sys; d=json.load(sys.stdin); print(json.dumps({k:d.get(k) for k in ("id","summary","details","aliases","severity","affected","references")}, indent=2))' \
|| true
else
echo 'curl is unavailable'
fiRepository: openshift/origin
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -u
python3 - <<'PY'
from pathlib import Path
p = Path("pkg/resourcewatch/git/git_store.go")
text = p.read_text()
print("--- go-git calls in application file ---")
for i, line in enumerate(text.splitlines(), 1):
if "git." in line or "s.repo" in line or "repo *git." in line:
print(f"{i}: {line}")
print("--- GitStorage.repo references ---")
for i, line in enumerate(text.splitlines(), 1):
if "repo" in line:
print(f"{i}: {line}")
PY
printf '%s\n' '--- all NewGitStorage callers ---'
rg -n -S 'NewGitStorage\(' --glob '!vendor/**' .
printf '%s\n' '--- go-git v5 versions available from proxy ---'
if command -v curl >/dev/null 2>&1; then
curl -fsSL --max-time 20 \
'https://proxy.golang.org/github.com/go-git/go-git/v5/@v/list' \
| awk '$0 ~ /^v5\.(11|13)\./ {print}' | tail -n 20 || true
fi
printf '%s\n' '--- concise OSV package findings ---'
if command -v curl >/dev/null 2>&1; then
curl -fsSL --max-time 20 -X POST 'https://api.osv.dev/v1/query' \
-H 'content-type: application/json' \
--data '{"package":{"name":"gopkg.in/src-d/go-git.v4","ecosystem":"Go"},"version":"v4.13.1"}' \
| python3 -c '
import json,sys
d=json.load(sys.stdin)
for v in d.get("vulns", []):
print(v.get("id"), "|", v.get("summary"))
for a in v.get("affected", []):
if a.get("package",{}).get("name") == "gopkg.in/src-d/go-git.v4":
print(" range:", a.get("ranges"))
' || true
fiRepository: openshift/origin
Length of output: 2573
Remove or replace gopkg.in/src-d/go-git.v4.
OSV lists v4.13.1 as affected by path traversal/RCE, two DoS issues, and argument injection: GO-2024-2456, GO-2024-2466, GO-2025-3367, and GO-2025-3368.
GitStorage only uses PlainInit and PlainOpen for local repositories. Remove the unused GitStorage.repo field and replace these calls with the existing Git CLI path, or migrate to github.com/go-git/go-git/v5 at least v5.13.0. Run go mod tidy && go mod vendor, make verify, and make check after the change.
🧰 Tools
🪛 OSV Scanner (2.4.0)
[CRITICAL] 106-106: gopkg.in/src-d/go-git.v4 4.13.1: Path traversal and RCE in github.com/go-git/go-git/v5 and gopkg.in/src-d/go-git.v4
(GO-2024-2456)
[CRITICAL] 106-106: gopkg.in/src-d/go-git.v4 4.13.1: Denial of service in github.com/go-git/go-git/v5 and gopkg.in/src-d/go-git.v4
(GO-2024-2466)
[CRITICAL] 106-106: gopkg.in/src-d/go-git.v4 4.13.1: Clients vulnerable to DoS via maliciously crafted Git server replies in github.com/go-git/go-git
(GO-2025-3367)
[CRITICAL] 106-106: gopkg.in/src-d/go-git.v4 4.13.1: Argument Injection via the URL field in github.com/go-git/go-git
(GO-2025-3368)
[CRITICAL] 106-106: gopkg.in/src-d/go-git.v4 4.13.1: Maliciously crafted Git server replies can lead to path traversal and RCE on go-git clients
[CRITICAL] 106-106: gopkg.in/src-d/go-git.v4 4.13.1: Maliciously crafted Git server replies can cause DoS on go-git clients
[CRITICAL] 106-106: gopkg.in/src-d/go-git.v4 4.13.1: go-git clients vulnerable to DoS via maliciously crafted Git server replies
[CRITICAL] 106-106: gopkg.in/src-d/go-git.v4 4.13.1: go-git has an Argument Injection via the URL field
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@go.mod` at line 106, Remove the vulnerable gopkg.in/src-d/go-git.v4
dependency and the unused GitStorage.repo field; update GitStorage’s PlainInit
and PlainOpen usage to the existing Git CLI path, or migrate to
github.com/go-git/go-git/v5 at version 5.13.0 or newer while preserving local
repository behavior. Regenerate dependencies with go mod tidy and go mod vendor,
then run make verify and make check.
Sources: Path instructions, MCP tools, Linters/SAST tools
| err = retry.OnError(backoff, shouldRetryExec, | ||
| func() error { | ||
| out, outStdErr, execErr := e2epod.ExecWithOptions(oc.KubeFramework(), execOptions) | ||
| out, outStdErr, execErr := e2epod.Exec(oc.KubeFramework().TContext(context.Background()), execOptions) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- cpu_partitioning/crio.go: surrounding function ---'
sed -n '330,400p' test/extended/cpu_partitioning/crio.go
printf '%s\n' '--- gpu_validator.go: relevant declarations and call sites ---'
sed -n '1,125p' test/extended/node/dra/nvidia/gpu_validator.go
sed -n '285,375p' test/extended/node/dra/nvidia/gpu_validator.go
printf '%s\n' '--- context and execution call structure ---'
rg -n -C 4 'context\.Background\(\)|TContext\(|e2epod\.Exec\(' \
test/extended/cpu_partitioning/crio.go \
test/extended/node/dra/nvidia/gpu_validator.goRepository: openshift/origin
Length of output: 14554
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- exact enclosing function signatures ---'
python3 - <<'PY'
from pathlib import Path
targets = [
Path("test/extended/cpu_partitioning/crio.go"),
Path("test/extended/node/dra/nvidia/gpu_validator.go"),
]
for path in targets:
lines = path.read_text().splitlines()
wanted = {375, 50, 98, 326, 356}
print(f"--- {path} ---")
for n in sorted(wanted):
if n > len(lines):
continue
start = max(0, n - 45)
while start > 0 and not lines[start].lstrip().startswith(("func ", "var ", "const ")):
start -= 1
print(f"site line {n}; context starts at {start + 1}")
for i in range(start, min(len(lines), n + 2)):
print(f"{i+1}: {lines[i]}")
PY
printf '%s\n' '--- all TContext definitions and related context helpers ---'
rg -n -C 5 'func .*TContext|TContext\(' --glob '*.go' .
printf '%s\n' '--- context.Background usage in the affected files ---'
rg -n -C 2 'context\.Background\(\)|\bctx\b' \
test/extended/cpu_partitioning/crio.go \
test/extended/node/dra/nvidia/gpu_validator.goRepository: openshift/origin
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- enclosing signatures from the previous log ---'
sed -n '1,220p' /tmp/coderabbit-shell-logs/shell-output-GDzRTl
printf '%s\n' '--- TContext implementation locations ---'
rg -l 'func .*TContext' vendor/k8s.io/kubernetes/test/e2e vendor/k8s.io/kubernetes/test -g '*.go' | head -20
rg -n -C 12 '^func \(.*\) TContext|^func TContext' vendor/k8s.io/kubernetes/test/e2e/framework vendor/k8s.io/kubernetes/test -g '*.go' | head -120
printf '%s\n' '--- affected call sites and method declarations only ---'
rg -n '^(func |[[:space:]]*stdout, stderr, err := e2epod\.Exec|[[:space:]]*out, outStdErr, execErr := e2epod\.Exec)' \
test/extended/cpu_partitioning/crio.go \
test/extended/node/dra/nvidia/gpu_validator.goRepository: openshift/origin
Length of output: 26230
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- Framework.TContext implementation ---'
sed -n '174,200p' vendor/k8s.io/kubernetes/test/e2e/framework/framework.go
printf '%s\n' '--- e2epod.Exec implementation and context use ---'
rg -n -C 10 '^func Exec|TContext\.Context|ctx\.Context|context\.Done|WithContext' \
vendor/k8s.io/kubernetes/test/e2e/framework/pod/exec_util.go \
vendor/k8s.io/kubernetes/test/utils/ktesting \
vendor/k8s.io/kubernetes/test/utils/client-go/ktesting/client.go | head -240
printf '%s\n' '--- concise static verifier for all listed sites ---'
python3 - <<'PY'
from pathlib import Path
import re
sites = {
"test/extended/cpu_partitioning/crio.go": [375],
"test/extended/node/dra/nvidia/gpu_validator.go": [50, 98, 326, 356],
}
for name, lines in sites.items():
text = Path(name).read_text().splitlines()
print(name)
for line_no in lines:
line = text[line_no - 1]
call = "context.Background()" in line
# Find the nearest preceding function declaration.
fn = None
for candidate in reversed(text[:line_no]):
match = re.search(r'^\s*func\s+(?:\([^)]*\)\s*)?([A-Za-z_]\w*)\s*\(', candidate)
if match:
fn = candidate.strip()
break
print(f" line {line_no}: background={call}; enclosing_function={fn}")
PYRepository: openshift/origin
Length of output: 26143
Pass ctx to every TContext call.
The five listed calls use context.Background(), so e2epod.Exec cannot receive caller cancellation or deadlines. Replace it with ctx in collectContainerInfo and all four GPU validator methods.
📍 Affects 2 files
test/extended/cpu_partitioning/crio.go#L375-L375(this comment)test/extended/node/dra/nvidia/gpu_validator.go#L50-L50test/extended/node/dra/nvidia/gpu_validator.go#L98-L98test/extended/node/dra/nvidia/gpu_validator.go#L326-L326test/extended/node/dra/nvidia/gpu_validator.go#L356-L356
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/extended/cpu_partitioning/crio.go` at line 375, Replace
context.Background() with the caller-provided ctx in every TContext call used by
collectContainerInfo and the four GPU validator methods. Update
test/extended/cpu_partitioning/crio.go:375 and
test/extended/node/dra/nvidia/gpu_validator.go:50, 98, 326, and 356; make no
other changes.
Sources: Path instructions, MCP tools
| spec: | ||
| containers: | ||
| - image: registry.k8s.io/e2e-test-images/nginx:1.15-4 | ||
| - image: registry.k8s.io/e2e-test-images/nginx:1.27.0-2 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- referenced lines in embedded data ---'
sed -n '51720,51785p' test/extended/testdata/bindata.go
printf '%s\n' '--- source manifest candidates ---'
fd -i 'reencrypt-serving-cert.yaml' .
printf '%s\n' '--- ssl directive occurrences ---'
rg -n -C 3 'ssl on;|listen .*8443|nginx:1\.27\.0-2' test/extended/testdata
printf '%s\n' '--- generation and verification references ---'
rg -n -C 2 'bindata|go-bindata|make verify|make check' Makefile* .github test/extended 2>/dev/null | head -200Repository: openshift/origin
Length of output: 18354
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- referenced lines in embedded data ---'
sed -n '51720,51785p' test/extended/testdata/bindata.go
printf '%s\n' '--- source manifest candidates ---'
fd -i 'reencrypt-serving-cert.yaml' .
printf '%s\n' '--- ssl directive occurrences ---'
rg -n -C 3 'ssl on;|listen .*8443|nginx:1\.27\.0-2' test/extended/testdata
printf '%s\n' '--- generation and verification references ---'
rg -n -C 2 'bindata|go-bindata|make verify|make check' Makefile* .github test/extended 2>/dev/null | head -200Repository: openshift/origin
Length of output: 17891
🌐 Web query:
NGINX 1.27.0 ssl on directive removed 1.25.1 listen ssl ipv6only documentation
💡 Result:
The deprecated ssl directive was removed from NGINX in version 1.25.1 [1][2][3]. It had been deprecated since version 1.15.0 [2][4][5]. To enable SSL in modern NGINX versions (including 1.27.0), you must use the ssl parameter of the listen directive [6][2]. Example usage: listen 443 ssl; The parameter ipv6only is also a component of the listen directive and remains independent of the removal of the old ssl directive [6]. You can combine them on the same line if needed: listen [::]:443 ssl ipv6only=on; The removal of the ssl directive was a clean-up of legacy configuration syntax; it does not affect how SSL/TLS is otherwise configured, managed, or enabled for specific listening sockets [6][7].
Citations:
- 1: https://nginx.org/en/CHANGES
- 2: https://nginx.org/en/docs/http/ngx_http_ssl_module.html
- 3: https://mailman.nginx.org/pipermail/nginx-announce/2023/BYSVLPUZESCZHJMTDD25QD7ZKZYADAR2.html
- 4: https://trac.nginx.org/nginx/ticket/1580
- 5: https://mailman.nginx.org/pipermail/nginx-devel/2023-June/VXYR5QMSOBXBVOFTGMKKSY22PMY73MB3.html
- 6: https://nginx.org/en/docs/http/configuring_https_servers.html
- 7: https://mailman.nginx.org/pipermail/nginx-devel/2023-June/NEFKNNN7LXVCPN77LIAHF7N6HEG2VAU4.html
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
source = Path("test/extended/testdata/router/reencrypt-serving-cert.yaml").read_text()
generated = Path("test/extended/testdata/bindata.go").read_text()
checks = {
"source_has_legacy_ssl": " ssl on;" in source,
"generated_has_legacy_ssl": " ssl on;" in generated,
"source_has_nginx_image": "registry.k8s.io/e2e-test-images/nginx:1.27.0-2" in source,
"generated_has_nginx_image": "registry.k8s.io/e2e-test-images/nginx:1.27.0-2" in generated,
"source_has_proposed_ipv4": " listen 8443 ssl;" in source,
"source_has_proposed_ipv6": " listen [::]:8443 ipv6only=on ssl;" in source,
}
for name, value in checks.items():
print(f"{name}={value}")
# Extract the embedded YAML payload for a direct source-vs-generated comparison.
prefix = "var _testExtendedTestdataRouterReencryptServingCertYaml = []byte(`"
start = generated.find(prefix)
if start < 0:
raise SystemExit("embedded asset declaration not found")
start += len(prefix)
end = generated.find("`)", start)
if end < 0:
raise SystemExit("embedded asset terminator not found")
embedded = generated[start:end]
print(f"source_equals_embedded={source == embedded}")
print(f"source_bytes={len(source.encode())}")
print(f"embedded_bytes={len(embedded.encode())}")
PYRepository: openshift/origin
Length of output: 398
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
source = Path("test/extended/testdata/router/reencrypt-serving-cert.yaml").read_text()
generated = Path("test/extended/testdata/bindata.go").read_text()
checks = {
"source_has_legacy_ssl": " ssl on;" in source,
"generated_has_legacy_ssl": " ssl on;" in generated,
"source_has_nginx_image": "registry.k8s.io/e2e-test-images/nginx:1.27.0-2" in source,
"generated_has_nginx_image": "registry.k8s.io/e2e-test-images/nginx:1.27.0-2" in generated,
"source_has_proposed_ipv4": " listen 8443 ssl;" in source,
"source_has_proposed_ipv6": " listen [::]:8443 ipv6only=on ssl;" in source,
}
for name, value in checks.items():
print(f"{name}={value}")
prefix = "var _testExtendedTestdataRouterReencryptServingCertYaml = []byte(`"
start = generated.find(prefix)
if start < 0:
raise SystemExit("embedded asset declaration not found")
start += len(prefix)
end = generated.find("`)", start)
if end < 0:
raise SystemExit("embedded asset terminator not found")
embedded = generated[start:end]
print(f"source_equals_embedded={source == embedded}")
print(f"source_bytes={len(source.encode())}")
print(f"embedded_bytes={len(embedded.encode())}")
PYRepository: openshift/origin
Length of output: 398
Replace the removed ssl on; directive.
NGINX 1.27 rejects ssl on; because NGINX removed the directive in version 1.25.1. Update test/extended/testdata/router/reencrypt-serving-cert.yaml, regenerate test/extended/testdata/bindata.go, then run make verify and make check.
Proposed configuration change
- listen 8443;
- listen [::]:8443 ipv6only=on;
- ssl on;
+ listen 8443 ssl;
+ listen [::]:8443 ipv6only=on ssl;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/extended/testdata/bindata.go` at line 51732, Update the reencrypt
serving-cert NGINX configuration to replace the removed ssl on directive with
the supported TLS configuration, regenerate the embedded bindata artifact, then
run make verify and make check to validate the change.
Source: Coding guidelines
|
@jubittajohn: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
/pipeline required |
|
Scheduling required tests: Scheduling tests matching the |
|
/test verify-image-manifest-lists |
Summary by CodeRabbit
Maintenance
Test Improvements