SREP-3893 CVE fix and go mod bump. Update the deployment to clusterpa…#433
SREP-3893 CVE fix and go mod bump. Update the deployment to clusterpa…#433tkong-redhat wants to merge 1 commit intoopenshift:masterfrom
Conversation
WalkthroughThis pull request updates multiple infrastructure and dependency layers: image tags are bumped to v8.3.4, team memberships are adjusted in OWNERS_ALIASES files, E2E test templates are refactored to use Slack-based notifications instead of cloud credential parameters, and the PKO migration script is enhanced to support custom cluster deployment selectors and improved YAML manifest handling. Test infrastructure is modernized with updated Kubernetes asset versions, and Go module dependencies are upgraded across the board. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tkong-redhat The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
@tkong-redhat: all tests passed! 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. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #433 +/- ##
=======================================
Coverage 57.15% 57.15%
=======================================
Files 29 29
Lines 2138 2138
=======================================
Hits 1222 1222
Misses 802 802
Partials 114 114 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
boilerplate/openshift/golang-osd-operator/Dockerfile.olm-registry (1)
1-7:⚠️ Potential issue | 🟠 MajorPin the runtime base image to a specific version instead of mutable
latest.Line 7 uses
ubi-minimal:latest, which introduces non-deterministic builds and silent CVE/regression drift. Even though the builder image was pinned to v4.21 on Line 1, the runtime base must also be pinned to a specific version tag.Proposed fix
-FROM registry.access.redhat.com/ubi9/ubi-minimal:latest +FROM registry.access.redhat.com/ubi9/ubi-minimal:9.7-1773939694🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@boilerplate/openshift/golang-osd-operator/Dockerfile.olm-registry` around lines 1 - 7, The runtime base image is pinned to a mutable tag `registry.access.redhat.com/ubi9/ubi-minimal:latest`; update the Dockerfile's second FROM to a specific, immutable UBI9 image tag (for example a vendor-provided patch-level tag) to ensure reproducible builds; modify the line that currently reads `FROM registry.access.redhat.com/ubi9/ubi-minimal:latest` to reference a concrete version (matching your security/support policy) so the runtime stage is deterministic and aligns with the pinned builder stage.go.mod (1)
12-114:⚠️ Potential issue | 🔴 Criticalgo.sum must be included in this PR.
When
go.modis updated,go.summust be regenerated with the correct checksums—this is a Go tooling requirement. The repository already has ago.sumfile (584 lines), so it must be committed alongside these changes.Additionally, the PR title references "SREP-3893" but provides no details on which specific CVEs these dependency updates address. Clarify the security fixes in the PR description: for example, the
golang.org/x/crypto v0.46.0andgolang.org/x/net v0.48.0updates address multiple 2025 CVEs (CVE-2025-58181, CVE-2025-47914, CVE-2025-22870, etc.). Specify which vulnerabilities this PR resolves.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` around lines 12 - 114, The PR updated go.mod but omitted the regenerated go.sum—run `go mod tidy` (or `go mod download`) to regenerate go.sum and commit that file alongside the go.mod changes so checksums match; also update the PR description to list the specific security fixes addressed (mention the updated modules like golang.org/x/crypto v0.46.0 and golang.org/x/net v0.48.0 and the CVE identifiers such as CVE-2025-58181, CVE-2025-47914, CVE-2025-22870 or whichever are fixed) so reviewers can verify which vulnerabilities SREP-3893 is resolving.boilerplate/openshift/golang-osd-operator/olm_pko_migration.py (1)
199-244:⚠️ Potential issue | 🟠 MajorAlign
CLUSTERPACKAGE_TEMPLATEwith the new directClusterPackagedeployment.
write_clusterpackage_template()still serializes aSelectorSyncSetwrapper here, but the checked-in target inhack/pko/clusterpackage.yamlat Lines 20-32 is now a top-levelpackage-operator.run/v1alpha1ClusterPackage. The next regeneration will undo this PR and reintroduce the old SSS-based layout.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@boilerplate/openshift/golang-osd-operator/olm_pko_migration.py` around lines 199 - 244, The CLUSTERPACKAGE_TEMPLATE constant still emits a SelectorSyncSet wrapper (via CLUSTERPACKAGE_TEMPLATE) while the desired artifact is now a top-level package-operator.run/v1alpha1 ClusterPackage; update write_clusterpackage_template() to serialize and write a top-level ClusterPackage YAML matching hack/pko/clusterpackage.yaml (remove the SelectorSyncSet envelope and ensure metadata/name, spec.image and spec.config.image use the same parameter substitutions like {operator_name}, ${PKO_IMAGE}:${IMAGE_TAG} and ${OPERATOR_IMAGE}:${IMAGE_TAG}), and update CLUSTERPACKAGE_TEMPLATE accordingly so future regenerations produce the direct ClusterPackage resource instead of the SSS wrapper.boilerplate/openshift/golang-osd-operator/standard.mk (1)
248-281:⚠️ Potential issue | 🟠 MajorUse host OS and architecture for envtest binaries.
Line 275 hardcodes
--arch amd64 --os linux, but the binaries downloaded and executed bysetup-envtest(kube-apiserver, etcd, kubectl) must run on the host machine. This breaks on non-linux/amd64 systems such as macOS/arm64 and non-amd64 builders.Replace the hardcoded values with
GOHOSTOSandGOHOSTARCHto detect the actual host platform:Suggested fix
ENVTEST_K8S_VERSION = 1.28.0 SETUP_ENVTEST_VERSION = release-0.23 GOPATH ?= $(shell go env GOPATH) SETUP_ENVTEST = $(GOPATH)/bin/setup-envtest +ENVTEST_HOST_OS ?= $(shell go env GOHOSTOS) +ENVTEST_HOST_ARCH ?= $(shell go env GOHOSTARCH) @@ - if ! ASSETS_PATH=$$($(SETUP_ENVTEST) use $(ENVTEST_K8S_VERSION) --arch amd64 --os linux --bin-dir /tmp/envtest-binaries -p path 2>&1); then \ + if ! ASSETS_PATH=$$($(SETUP_ENVTEST) use $(ENVTEST_K8S_VERSION) --arch $(ENVTEST_HOST_ARCH) --os $(ENVTEST_HOST_OS) --bin-dir /tmp/envtest-binaries -p path 2>&1); then \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@boilerplate/openshift/golang-osd-operator/standard.mk` around lines 248 - 281, The go-test recipe hardcodes --arch amd64 --os linux when invoking $(SETUP_ENVTEST) which breaks on non-linux/amd64 hosts; update the invocation in the go-test target to pass the host platform by using GOHOSTOS and GOHOSTARCH (e.g., replace the literal flags after the use $(ENVTEST_K8S_VERSION) call with --arch $(GOHOSTARCH) --os $(GOHOSTOS) or equivalent environment expansion) so the SETUP_ENVTEST download produces binaries that match the host; keep references to ENVTEST_K8S_VERSION and SETUP_ENVTEST unchanged and ensure the ASSETS_PATH capture and subsequent KUBEBUILDER_ASSETS export remain the same.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@boilerplate/openshift/golang-osd-e2e/e2e-template.yml`:
- Around line 98-101: The TEST_SUITES_YAML block emits an unquoted slackChannel
scalar which lets values like `#channel` be treated as a YAML comment and lost;
update the TEST_SUITES_YAML value so slackChannel uses a quoted interpolation
(i.e. wrap ${SLACK_CHANNEL} in quotes) to preserve literal channels, and apply
the identical change to the e2e-template.yml copy; ensure you only modify the
slackChannel line (leave TEST_IMAGE and IMAGE_TAG interpolation untouched).
In `@boilerplate/openshift/golang-osd-operator/olm_pko_migration.py`:
- Around line 662-668: The selector YAML is being indented by 6 spaces
(indented_lines = [" " + line ...]) which mis-nests the block and produces
`clusterDeploymentSelector: null`; update the indentation so the dumped selector
lines align as a proper YAML block under the clusterDeploymentSelector key
(e.g., indent by 8 spaces or compute indentation to match the template) by
changing the prefix used when building indented_lines (reference selector_yaml
and indented_lines) so the selector's top-level keys
(matchLabels/matchExpressions) appear nested under clusterDeploymentSelector.
- Around line 698-701: Remove the unnecessary f-string prefixes on the two print
calls in the conditional block (the lines printing "Please review this file and
ensure the deployment targets match your expectation" and the preceding warning
line) by replacing f"...” with regular string literals "..."; locate the print
statements in olm_pko_migration.py (the block that prints the default
clusterDeploymentSelector warning) and remove the leading "f" characters so Ruff
F541 is resolved.
---
Outside diff comments:
In `@boilerplate/openshift/golang-osd-operator/Dockerfile.olm-registry`:
- Around line 1-7: The runtime base image is pinned to a mutable tag
`registry.access.redhat.com/ubi9/ubi-minimal:latest`; update the Dockerfile's
second FROM to a specific, immutable UBI9 image tag (for example a
vendor-provided patch-level tag) to ensure reproducible builds; modify the line
that currently reads `FROM registry.access.redhat.com/ubi9/ubi-minimal:latest`
to reference a concrete version (matching your security/support policy) so the
runtime stage is deterministic and aligns with the pinned builder stage.
In `@boilerplate/openshift/golang-osd-operator/olm_pko_migration.py`:
- Around line 199-244: The CLUSTERPACKAGE_TEMPLATE constant still emits a
SelectorSyncSet wrapper (via CLUSTERPACKAGE_TEMPLATE) while the desired artifact
is now a top-level package-operator.run/v1alpha1 ClusterPackage; update
write_clusterpackage_template() to serialize and write a top-level
ClusterPackage YAML matching hack/pko/clusterpackage.yaml (remove the
SelectorSyncSet envelope and ensure metadata/name, spec.image and
spec.config.image use the same parameter substitutions like {operator_name},
${PKO_IMAGE}:${IMAGE_TAG} and ${OPERATOR_IMAGE}:${IMAGE_TAG}), and update
CLUSTERPACKAGE_TEMPLATE accordingly so future regenerations produce the direct
ClusterPackage resource instead of the SSS wrapper.
In `@boilerplate/openshift/golang-osd-operator/standard.mk`:
- Around line 248-281: The go-test recipe hardcodes --arch amd64 --os linux when
invoking $(SETUP_ENVTEST) which breaks on non-linux/amd64 hosts; update the
invocation in the go-test target to pass the host platform by using GOHOSTOS and
GOHOSTARCH (e.g., replace the literal flags after the use $(ENVTEST_K8S_VERSION)
call with --arch $(GOHOSTARCH) --os $(GOHOSTOS) or equivalent environment
expansion) so the SETUP_ENVTEST download produces binaries that match the host;
keep references to ENVTEST_K8S_VERSION and SETUP_ENVTEST unchanged and ensure
the ASSETS_PATH capture and subsequent KUBEBUILDER_ASSETS export remain the
same.
In `@go.mod`:
- Around line 12-114: The PR updated go.mod but omitted the regenerated
go.sum—run `go mod tidy` (or `go mod download`) to regenerate go.sum and commit
that file alongside the go.mod changes so checksums match; also update the PR
description to list the specific security fixes addressed (mention the updated
modules like golang.org/x/crypto v0.46.0 and golang.org/x/net v0.48.0 and the
CVE identifiers such as CVE-2025-58181, CVE-2025-47914, CVE-2025-22870 or
whichever are fixed) so reviewers can verify which vulnerabilities SREP-3893 is
resolving.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4777d075-aaf1-4f1d-9b3b-3b85e9dce0a7
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (16)
.ci-operator.yamlOWNERS_ALIASESboilerplate/_data/backing-image-tagboilerplate/_data/last-boilerplate-commitboilerplate/openshift/golang-osd-e2e/e2e-template.ymlboilerplate/openshift/golang-osd-operator/Dockerfile.olm-registryboilerplate/openshift/golang-osd-operator/OWNERS_ALIASESboilerplate/openshift/golang-osd-operator/olm_pko_migration.pyboilerplate/openshift/golang-osd-operator/standard.mkboilerplate/openshift/golang-osd-operator/test_olm_pko_migration.pybuild/Dockerfilebuild/Dockerfile.olm-registrygo.modhack/pko/clusterpackage.yamlhack/test/test-clusterpackage.yamltest/e2e/e2e-template.yml
| - name: TEST_SUITES_YAML | ||
| value: |- | ||
| - image: ${TEST_IMAGE}:${IMAGE_TAG} | ||
| slackChannel: ${SLACK_CHANNEL} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'slackChannel:\s*\$\{SLACK_CHANNEL\}'
python - <<'PY'
import sys, subprocess
try:
import yaml
except ImportError:
subprocess.check_call([sys.executable, "-m", "pip", "install", "PyYAML", "-q"])
import yaml
bad = "- image: quay.io/example/osde2e:v1\n slackChannel: `#alerts`\n"
good = '- image: quay.io/example/osde2e:v1\n slackChannel: "#alerts"\n'
print("unquoted:", yaml.safe_load(bad))
print("quoted :", yaml.safe_load(good))
PYRepository: openshift/certman-operator
Length of output: 414
Quote SLACK_CHANNEL inside TEST_SUITES_YAML to prevent silent failures.
Lines 98–101 emit an unquoted YAML scalar for slackChannel. When a caller passes #channel, YAML treats # as a comment delimiter, causing slackChannel to parse as null and the notification target to be silently lost. Add quotes to preserve the value.
Proposed fix
- name: TEST_SUITES_YAML
value: |-
- image: ${TEST_IMAGE}:${IMAGE_TAG}
- slackChannel: ${SLACK_CHANNEL}
+ slackChannel: "${SLACK_CHANNEL}"Apply the same fix to test/e2e/e2e-template.yml at the same lines.
📝 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.
| - name: TEST_SUITES_YAML | |
| value: |- | |
| - image: ${TEST_IMAGE}:${IMAGE_TAG} | |
| slackChannel: ${SLACK_CHANNEL} | |
| - name: TEST_SUITES_YAML | |
| value: |- | |
| - image: ${TEST_IMAGE}:${IMAGE_TAG} | |
| slackChannel: "${SLACK_CHANNEL}" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@boilerplate/openshift/golang-osd-e2e/e2e-template.yml` around lines 98 - 101,
The TEST_SUITES_YAML block emits an unquoted slackChannel scalar which lets
values like `#channel` be treated as a YAML comment and lost; update the
TEST_SUITES_YAML value so slackChannel uses a quoted interpolation (i.e. wrap
${SLACK_CHANNEL} in quotes) to preserve literal channels, and apply the
identical change to the e2e-template.yml copy; ensure you only modify the
slackChannel line (leave TEST_IMAGE and IMAGE_TAG interpolation untouched).
| # Convert the selector back to YAML format with proper indentation | ||
| selector = spec["clusterDeploymentSelector"] | ||
| # Dump with proper indentation for inserting into template | ||
| selector_yaml = yaml.dump(selector, default_flow_style=False, sort_keys=False) | ||
| # Indent each line by 6 spaces (to match template indentation) | ||
| indented_lines = [" " + line for line in selector_yaml.splitlines()] | ||
| return "\n".join(indented_lines) |
There was a problem hiding this comment.
Fix the selector indentation before inserting it.
With six leading spaces here, the rendered YAML becomes clusterDeploymentSelector: null and puts matchLabels / matchExpressions beside it. Custom selectors are therefore not actually applied.
Suggested fix
- # Indent each line by 6 spaces (to match template indentation)
- indented_lines = [" " + line for line in selector_yaml.splitlines()]
+ # Indent each line so it stays nested under `clusterDeploymentSelector`
+ indented_lines = [" " + line for line in selector_yaml.splitlines()]📝 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.
| # Convert the selector back to YAML format with proper indentation | |
| selector = spec["clusterDeploymentSelector"] | |
| # Dump with proper indentation for inserting into template | |
| selector_yaml = yaml.dump(selector, default_flow_style=False, sort_keys=False) | |
| # Indent each line by 6 spaces (to match template indentation) | |
| indented_lines = [" " + line for line in selector_yaml.splitlines()] | |
| return "\n".join(indented_lines) | |
| # Convert the selector back to YAML format with proper indentation | |
| selector = spec["clusterDeploymentSelector"] | |
| # Dump with proper indentation for inserting into template | |
| selector_yaml = yaml.dump(selector, default_flow_style=False, sort_keys=False) | |
| # Indent each line so it stays nested under `clusterDeploymentSelector` | |
| indented_lines = [" " + line for line in selector_yaml.splitlines()] | |
| return "\n".join(indented_lines) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@boilerplate/openshift/golang-osd-operator/olm_pko_migration.py` around lines
662 - 668, The selector YAML is being indented by 6 spaces (indented_lines = ["
" + line ...]) which mis-nests the block and produces
`clusterDeploymentSelector: null`; update the indentation so the dumped selector
lines align as a proper YAML block under the clusterDeploymentSelector key
(e.g., indent by 8 spaces or compute indentation to match the template) by
changing the prefix used when building indented_lines (reference selector_yaml
and indented_lines) so the selector's top-level keys
(matchLabels/matchExpressions) appear nested under clusterDeploymentSelector.
| print(f"Please review this file and ensure the deployment targets match your expectation") | ||
| else: | ||
| print("⚠ Using default clusterDeploymentSelector (hack/olm-registry/olm-artifacts-template.yaml not found)") | ||
| print(f"Please review this file and ensure the deployment targets match your expectation") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and read lines 698-701
cat -n boilerplate/openshift/golang-osd-operator/olm_pko_migration.py | sed -n '695,705p'Repository: openshift/certman-operator
Length of output: 744
🌐 Web query:
Ruff F541 rule unused f-string prefix
💡 Result:
The Ruff rule F541, named "f-string-missing-placeholders" (commonly reported as "f-string without any placeholders" or "unused f-string prefix"), detects f-strings that do not contain any placeholder expressions like {var}. It flags cases where the 'f' prefix is unnecessary, recommending a regular string literal instead to avoid confusion. Example violation: f"Hello, world!" Suggested fix: "Hello, world!" This rule is derived from Pyflakes, added in Ruff v0.0.18, and is automatically fixable. It considers implicit string concatenations: flags f-strings without placeholders even if concatenated with others that have them (matching Pyflakes behavior, fixed in 2024 via PR #10886). Format specifiers (e.g., f"{v:0.2f}") are not flagged as false positives. Official documentation: https://docs.astral.sh/ruff/rules/f-string-missing-placeholders/ Enabled by default under Pyflakes (F) rules.
Citations:
- 1: https://docs.astral.sh/ruff/rules/f-string-missing-placeholders/
- 2: [pyflakes] Identify f-string without a placeholder in concatenated strings (F541) astral-sh/ruff#10886
- 3: https://docs.astral.sh/ruff/rules/
- 4: F541 does not find violations in concatenations astral-sh/ruff#10885
Remove the unnecessary f prefixes from print statements.
Lines 698 and 701 both have f-string prefixes but don't contain any variable interpolation. Ruff F541 flags f-strings without placeholders as these can be regular string literals instead.
Suggested fix
- print(f"Please review this file and ensure the deployment targets match your expectation")
+ print("Please review this file and ensure the deployment targets match your expectation")
else:
print("⚠ Using default clusterDeploymentSelector (hack/olm-registry/olm-artifacts-template.yaml not found)")
- print(f"Please review this file and ensure the deployment targets match your expectation")
+ print("Please review this file and ensure the deployment targets match your expectation")📝 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.
| print(f"Please review this file and ensure the deployment targets match your expectation") | |
| else: | |
| print("⚠ Using default clusterDeploymentSelector (hack/olm-registry/olm-artifacts-template.yaml not found)") | |
| print(f"Please review this file and ensure the deployment targets match your expectation") | |
| print("Please review this file and ensure the deployment targets match your expectation") | |
| else: | |
| print("⚠ Using default clusterDeploymentSelector (hack/olm-registry/olm-artifacts-template.yaml not found)") | |
| print("Please review this file and ensure the deployment targets match your expectation") |
🧰 Tools
🪛 Ruff (0.15.6)
[error] 698-698: f-string without any placeholders
Remove extraneous f prefix
(F541)
[error] 701-701: f-string without any placeholders
Remove extraneous f prefix
(F541)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@boilerplate/openshift/golang-osd-operator/olm_pko_migration.py` around lines
698 - 701, Remove the unnecessary f-string prefixes on the two print calls in
the conditional block (the lines printing "Please review this file and ensure
the deployment targets match your expectation" and the preceding warning line)
by replacing f"...” with regular string literals "..."; locate the print
statements in olm_pko_migration.py (the block that prints the default
clusterDeploymentSelector warning) and remove the leading "f" characters so Ruff
F541 is resolved.
1 participant
CVE fix and go mod bump.
Update the deployment to clusterpackage instead of SSS.
Tested locally. Certman operator can start without issue