NO-ISSUE: Synchronize From Upstream Repositories - #788
Open
openshift-bot wants to merge 128 commits into
Open
Conversation
Bumps [actions/setup-go](https://github.com/actions/setup-go) from 6.5.0 to 7.0.0. - [Release notes](https://github.com/actions/setup-go/releases) - [Commits](actions/setup-go@v6.5.0...v7.0.0) --- updated-dependencies: - dependency-name: actions/setup-go dependency-version: 7.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/checkout](https://github.com/actions/checkout) from 7.0.0 to 7.0.1. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v7.0.0...v7.0.1) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: 7.0.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/setup-python](https://github.com/actions/setup-python) from 6.3.0 to 7.0.0. - [Release notes](https://github.com/actions/setup-python/releases) - [Commits](actions/setup-python@v6.3.0...v7.0.0) --- updated-dependencies: - dependency-name: actions/setup-python dependency-version: 7.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [mkdocs-material](https://github.com/squidfunk/mkdocs-material) from 9.7.6 to 9.7.7. - [Release notes](https://github.com/squidfunk/mkdocs-material/releases) - [Changelog](https://github.com/squidfunk/mkdocs-material/blob/master/CHANGELOG) - [Commits](squidfunk/mkdocs-material@9.7.6...9.7.7) --- updated-dependencies: - dependency-name: mkdocs-material dependency-version: 9.7.7 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [platformdirs](https://github.com/tox-dev/platformdirs) from 4.10.0 to 4.10.1. - [Release notes](https://github.com/tox-dev/platformdirs/releases) - [Changelog](https://github.com/tox-dev/platformdirs/blob/main/docs/changelog.rst) - [Commits](tox-dev/platformdirs@4.10.0...4.10.1) --- updated-dependencies: - dependency-name: platformdirs dependency-version: 4.10.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [regex](https://github.com/mrabarnett/mrab-regex) from 2026.7.10 to 2026.7.19. - [Changelog](https://github.com/mrabarnett/mrab-regex/blob/hg/changelog.txt) - [Commits](mrabarnett/mrab-regex@2026.7.10...2026.7.19) --- updated-dependencies: - dependency-name: regex dependency-version: 2026.7.19 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.23.2 to 1.24.0. - [Release notes](https://github.com/prometheus/client_golang/releases) - [Changelog](https://github.com/prometheus/client_golang/blob/v1.24.0/CHANGELOG.md) - [Commits](prometheus/client_golang@v1.23.2...v1.24.0) --- updated-dependencies: - dependency-name: github.com/prometheus/client_golang dependency-version: 1.24.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [github.com/klauspost/compress](https://github.com/klauspost/compress) from 1.19.0 to 1.19.1. - [Release notes](https://github.com/klauspost/compress/releases) - [Commits](klauspost/compress@v1.19.0...v1.19.1) --- updated-dependencies: - dependency-name: github.com/klauspost/compress dependency-version: 1.19.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [soupsieve](https://github.com/facelessuser/soupsieve) from 2.8.4 to 2.9. - [Release notes](https://github.com/facelessuser/soupsieve/releases) - [Commits](facelessuser/soupsieve@2.8.4...2.9) --- updated-dependencies: - dependency-name: soupsieve dependency-version: '2.9' dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…2838) The generate-demos CI job fails ~45% of the time with jq exit status 5 on the ClusterCatalog Quickstart scenario. Several issues contribute: - jq -s (slurp mode) buffers the entire operatorhubio FBC response in memory before processing, risking system errors on large catalogs - catalog content queries run exactly once with no retry, so any transient port-forward or network hiccup fails the step immediately - bash() does not attach stderr to ExitError, making failures opaque - with CatalogdHA, kubectl port-forward to the service deterministically picks the same pod via GetFirstPod sorting; if that pod is not the leader, it returns 404 (empty local cache) for every retry Remove jq slurp mode so each JSON object is processed in constant memory, prefixing filters with 'objects' to skip non-object values in the FBC stream. Wrap CatalogContainsSomePackages, PackageHasSomeChannels, and PackageHasSomeBundles in waitFor for retry on transient errors. Add curl --compressed to handle gzip-encoded responses and --fail with pipefail to detect HTTP errors. Resolve the catalogd leader pod via its Lease and port-forward directly to it on the container port (8443), falling back to the service when the lease cannot be read. Reset port-forwards on query failure and re-establish dead ones via liveness checks. Inject stderr into ExitError in bash() to match k8sClient diagnostics. Log catalog query errors at V(0) so CI timeout failures are diagnosable. Co-authored-by: Claude <noreply@anthropic.com>
openshift-bot
added
tide/merge-method-merge
openshift-ci-robot
added
the
jira/valid-reference
openshift-bot
added
the
approved
|
@openshift-bot: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR adds Fluent Bit log persistence for E2E support bundles, improves catalog port-forward recovery and query retries, and updates Go, Python, and repository ownership metadata. ChangesE2E observability and catalog reliability
Repository ownership metadata
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CatalogChecks
participant KubernetesAPI
participant PortForward
participant Catalogd
CatalogChecks->>KubernetesAPI: Resolve catalogd leader lease
KubernetesAPI-->>CatalogChecks: Return leader pod or service fallback
CatalogChecks->>PortForward: Validate or start port forward
PortForward->>Catalogd: Check HTTPS health
Catalogd-->>PortForward: Return health response
CatalogChecks->>Catalogd: Query catalog data
Catalogd-->>CatalogChecks: Return data or HTTP failure
CatalogChecks->>PortForward: Reset failed forward
CatalogChecks->>Catalogd: Retry catalog query
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 3 warnings)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@requirements.txt`:
- Line 17: Update requirements.txt so every resolved distribution, including
mkdocs-material, has a pinned artifact hash, and update the installation command
in Makefile.venv to use pip’s --require-hashes option when installing
requirements.txt.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 71f666f1-6bb6-4732-8a2f-5ff4651ad7c1
⛔ Files ignored due to path filters (63)
go.sumis excluded by!**/*.sumopenshift/tests-extension/go.sumis excluded by!**/*.sumopenshift/tests-extension/vendor/github.com/prometheus/client_golang/internal/github.com/golang/gddo/httputil/header/header.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/collectors/go_collector_go116.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/collectors/go_collector_latest.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/counter.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/desc.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/expvar_collector.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/gauge.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/go_collector_go116.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/go_collector_latest.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/histogram.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/internal/difflib.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/labels.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/metric.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/process_collector_darwin.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/process_collector_windows.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/promhttp/http.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/promhttp/instrument_client.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/promhttp/instrument_server.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/promhttp/option.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/registry.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/summary.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/timer.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/vec.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/wrap.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/procfs/net_wireless.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/procfs/proc_cgroup.gois excluded by!**/vendor/**openshift/tests-extension/vendor/modules.txtis excluded by!**/vendor/**vendor/github.com/klauspost/compress/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/flate/inflate_gen.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_decoder_arm64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_arm64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/snappy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/api/client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/api/prometheus/v1/api.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/internal/github.com/golang/gddo/httputil/header/header.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/collectors/go_collector_go116.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/collectors/go_collector_latest.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/counter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/desc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/expvar_collector.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/gauge.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/go_collector_go116.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/go_collector_latest.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/histogram.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/internal/difflib.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/labels.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/metric.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/process_collector_darwin.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/process_collector_windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/promhttp/http.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/promhttp/instrument_client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/promhttp/instrument_server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/promhttp/option.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/registry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/summary.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/timer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/vec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/wrap.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/net_wireless.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/proc_cgroup.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (4)
go.modopenshift/tests-extension/go.modrequirements.txttest/e2e/steps/demo_steps.go
| mergedeep==1.3.4 | ||
| mkdocs==1.6.1 | ||
| mkdocs-material==9.7.6 | ||
| mkdocs-material==9.7.7 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Hash entries in requirements files:"
rg -n --glob 'requirements*.txt' -- '--hash=' . || true
echo "Hash enforcement in repository configuration:"
rg -n --hidden --glob '!**/.git/**' -- 'pip .*--require-hashes|--require-hashes' . || trueRepository: openshift/operator-framework-operator-controller
Length of output: 268
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Tracked requirement files:"
git ls-files '*requirements*.txt' '*Pipfile*' 'pyproject.toml' 'package*.json' 'go.mod' 'go.sum' 'Cargo.toml' 'Gemfile*' 'pom.xml' 'build.gradle*'
echo
echo "requirements.txt context:"
if [ -f requirements.txt ]; then
cat -n requirements.txt
fi
echo
echo "Installation and CI references:"
rg -n --hidden --glob '!**/.git/**' \
'(^|[[:space:]])(pip|python[0-9.]* -m pip|uv|poetry)[[:space:]]+[^#\n]*(install|sync)|requirements\.txt|--require-hashes|pip-tools|pip-compile' \
. || true
echo
echo "Repository metadata and workflows:"
git ls-files '.github/workflows/*' 'Makefile' 'tox.ini' 'setup.cfg' 'pyproject.toml' 'Dockerfile*' | sortRepository: openshift/operator-framework-operator-controller
Length of output: 3343
🌐 Web query:
mkdocs-material 9.7.7 PyPI release hashes known vulnerabilities OSV
💡 Result:
The mkdocs-material package version 9.7.7 was released on PyPI on July 17, 2026 [1][2][3]. Regarding vulnerabilities, version 9.7.7 was specifically released to address a DOM-based cross-site scripting (XSS) vulnerability in the optional search.suggest feature [4][3]. This vulnerability, tracked as GHSA-xvg9-69gf-fjrf, affected versions 7.2.0 through 9.7.6 [4]. Users of these older versions are advised to upgrade to 9.7.7 or later, or disable the search.suggest feature if upgrading is not possible [4]. There is no CVE assigned to this issue [4]. Regarding hashes for version 9.7.7, PyPI provides these directly on the package files page. You can verify the integrity of the downloaded file by comparing its SHA-256 hash against the values provided in the "Download files" section for the 9.7.7 release on the official PyPI project page [5][2]. Top results: [5] https://pypi.org/project/mkdocs-material/9.7.7/ [1] https://github.com/squidfunk/mkdocs-material/releases/tag/9.7.7 [2] https://pypi.org/project/mkdocs-material/ [4] GHSA-xvg9-69gf-fjrf [3] https://squidfunk.github.io/mkdocs-material/changelog/
Citations:
- 1: https://github.com/squidfunk/mkdocs-material/releases/tag/9.7.7
- 2: https://pypi.org/project/mkdocs-material/
- 3: https://squidfunk.github.io/mkdocs-material/changelog/
- 4: GHSA-xvg9-69gf-fjrf
- 5: https://pypi.org/project/mkdocs-material/9.7.7/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Makefile.venv installation logic:"
sed -n '70,105p;210,245p' Makefile.venv
echo
echo "References to the virtual-environment targets:"
rg -n --hidden --glob '!**/.git/**' \
'Makefile\.venv|REQUIREMENTS_TXT|venv|make .*venv|\.venv|pip install' \
Makefile* .github 2>/dev/null || true
echo
echo "Tracked workflow and build files:"
git ls-files | rg '(^|/)(\.github/workflows/|Makefile[^/]*$|Dockerfile[^/]*$|requirements[^/]*\.txt$)' | sort
echo
echo "Hash support for the reviewed package:"
python3 - <<'PY'
import json
import urllib.request
url = "https://pypi.org/pypi/mkdocs-material/9.7.7/json"
with urllib.request.urlopen(url, timeout=10) as response:
data = json.load(response)
for item in data["urls"]:
print(item["filename"], item["packagetype"], item["digests"].get("sha256"))
PYRepository: openshift/operator-framework-operator-controller
Length of output: 11883
Add hashes to every resolved distribution and enforce them in Makefile.venv. Use pip --require-hashes when installing requirements.txt. Version pins alone do not verify downloaded artifacts.
🤖 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 `@requirements.txt` at line 17, Update requirements.txt so every resolved
distribution, including mkdocs-material, has a pinned artifact hash, and update
the installation command in Makefile.venv to use pip’s --require-hashes option
when installing requirements.txt.
Source: Path instructions
Bumps [platformdirs](https://github.com/tox-dev/platformdirs) from 4.10.1 to 4.11.0. - [Release notes](https://github.com/tox-dev/platformdirs/releases) - [Changelog](https://github.com/tox-dev/platformdirs/blob/main/docs/changelog.rst) - [Commits](tox-dev/platformdirs@4.10.1...4.11.0) --- updated-dependencies: - dependency-name: platformdirs dependency-version: 4.11.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
openshift-bot
force-pushed
the
synchronize-upstream
branch
from
August 5, 2026 00:14
6ed83d9 to
a17ced3
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
|
New changes are detected. LGTM label has been removed. |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@test/e2e/steps/demo_steps.go`:
- Around line 175-177: Replace the shell-interpolated script in the catalog
query step with separate exec.CommandContext invocations for curl and jq,
preserving pipefail-equivalent error handling. Path-escape catalogName when
constructing the curl URL, and pass packageName to jq via --arg rather than
interpolating it into jqFilter; update the surrounding query logic in the
relevant step function accordingly.
- Around line 137-150: Update catalogPortForwardAlive to return false when the
probe receives a non-2xx HTTP status, and propagate a false result if
resp.Body.Close() returns an error; only return true after both the request
succeeds with a successful status and the response body closes successfully.
- Line 133: Update the Info log in the leader-resolution flow to report only
that resolution succeeded; remove the holder and podName identity fields from
the logger.Info call while preserving the existing success log context.
- Around line 137-145: Update catalogPortForwardAlive to accept a scenario
context, pass ctx from both callers, and create the health request with
http.NewRequestWithContext before executing it via client.Do. Preserve the
existing URL, timeout, and response-check behavior while ensuring cancellation
interrupts the request.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0e0c75f8-a7fc-4134-8e5e-67b844848517
⛔ Files ignored due to path filters (63)
go.sumis excluded by!**/*.sumopenshift/tests-extension/go.sumis excluded by!**/*.sumopenshift/tests-extension/vendor/github.com/prometheus/client_golang/internal/github.com/golang/gddo/httputil/header/header.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/collectors/go_collector_go116.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/collectors/go_collector_latest.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/counter.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/desc.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/expvar_collector.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/gauge.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/go_collector_go116.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/go_collector_latest.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/histogram.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/internal/difflib.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/labels.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/metric.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/process_collector_darwin.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/process_collector_windows.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/promhttp/http.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/promhttp/instrument_client.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/promhttp/instrument_server.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/promhttp/option.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/registry.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/summary.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/timer.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/vec.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/client_golang/prometheus/wrap.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/procfs/net_wireless.gois excluded by!**/vendor/**openshift/tests-extension/vendor/github.com/prometheus/procfs/proc_cgroup.gois excluded by!**/vendor/**openshift/tests-extension/vendor/modules.txtis excluded by!**/vendor/**vendor/github.com/klauspost/compress/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/flate/inflate_gen.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_decoder_arm64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_arm64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/snappy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/api/client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/api/prometheus/v1/api.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/internal/github.com/golang/gddo/httputil/header/header.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/collectors/go_collector_go116.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/collectors/go_collector_latest.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/counter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/desc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/expvar_collector.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/gauge.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/go_collector_go116.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/go_collector_latest.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/histogram.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/internal/difflib.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/labels.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/metric.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/process_collector_darwin.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/process_collector_windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/promhttp/http.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/promhttp/instrument_client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/promhttp/instrument_server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/promhttp/option.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/registry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/summary.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/timer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/vec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/wrap.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/net_wireless.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/proc_cgroup.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (4)
go.modopenshift/tests-extension/go.modrequirements.txttest/e2e/steps/demo_steps.go
🚧 Files skipped from review as they are similar to previous changes (3)
- openshift/tests-extension/go.mod
- go.mod
- requirements.txt
| if podName == "" { | ||
| return "", fmt.Errorf("catalogd leader lease has empty holderIdentity") | ||
| } | ||
| logger.Info("Resolved catalogd leader pod", "holder", holder, "pod", podName) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file='test/e2e/steps/demo_steps.go'
printf '%s\n' '--- outline ---'
ast-grep outline "$file" 2>/dev/null || true
printf '%s\n' '--- target context ---'
sed -n '105,145p' "$file"
printf '%s\n' '--- holder and podName references ---'
rg -n -C 3 '\b(holder|podName)\b|Resolved catalogd leader pod|logger\.' "$file"
printf '%s\n' '--- logger setup in the file ---'
rg -n -C 3 'logr|klog|NewLogger|SetOutput|logger\s*=' "$file" || trueRepository: openshift/operator-framework-operator-controller
Length of output: 7148
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- package and logger declarations ---'
rg -n -C 4 '(^package |logger\s*=|var\s+logger|func\s+logger|NewLogger|SetLogger|logr\.|klog\.)' test/e2e/steps --glob '*.go'
printf '%s\n' '--- logger imports and package context ---'
sed -n '1,45p' test/e2e/steps/demo_steps.go
printf '%s\n' '--- bounded behavioral verifier ---'
python3 - <<'PY'
from pathlib import Path
import re
p = Path("test/e2e/steps/demo_steps.go")
s = p.read_text()
m = re.search(
r'holder,\s*err\s*:=\s*k8sClient\(ctx,\s*"get",\s*"lease".*?'
r'holder\s*=\s*strings\.TrimSpace\(holder\).*?'
r'podName\s*:=\s*holder.*?'
r'if idx := strings\.LastIndex\(holder, "_"\); idx >= 0 \{\s*'
r'podName = holder\[:idx\]\s*\}.*?'
r'logger\.Info\("Resolved catalogd leader pod",\s*"holder",\s*holder,\s*"pod",\s*podName\)',
s,
re.S,
)
print("leader_lease_to_log_path:", bool(m))
print("returns_logged_pod_name:", bool(re.search(r'return fmt\.Sprintf\("pod/%s", podName\)', s)))
print("info_log_contains_holder_and_pod:", '"holder", holder, "pod", podName' in s)
PYRepository: openshift/operator-framework-operator-controller
Length of output: 4814
Information Disclosure (CWE-200): Exposure of Sensitive Information to an Unauthorized Actor
Reachability: Internal · Exploitability: Difficult
Remove leader identities from the Info log.
holder comes from the catalogd leader Lease, and podName is derived from it. Log only that leader resolution succeeded because CI logs can expose internal pod hostnames.
🤖 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/e2e/steps/demo_steps.go` at line 133, Update the Info log in the
leader-resolution flow to report only that resolution succeeded; remove the
holder and podName identity fields from the logger.Info call while preserving
the existing success log context.
Source: Coding guidelines
Comment on lines
+137
to
+145
| func catalogPortForwardAlive(addr string) bool { | ||
| client := &http.Client{ | ||
| Timeout: 3 * time.Second, | ||
| Transport: &http.Transport{ | ||
| TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec | ||
| DialContext: (&net.Dialer{Timeout: 2 * time.Second}).DialContext, | ||
| }, | ||
| } | ||
| resp, err := client.Get(fmt.Sprintf("https://%s/", addr)) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Find every caller that must pass the scenario context after the signature change.
rg -n -C 4 --type go '\bcatalogPortForwardAlive\s*\(' test/e2eRepository: openshift/operator-framework-operator-controller
Length of output: 1876
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- demo_steps.go ---'
sed -n '1,180p' test/e2e/steps/demo_steps.go
printf '%s\n' '--- waitFor definition and callers ---'
rg -n -C 8 --type go '\bfunc waitFor\b|\bwaitFor\s*\(' test/e2eRepository: openshift/operator-framework-operator-controller
Length of output: 50404
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
tmp="$(mktemp --suffix=.go)"
trap 'rm -f "$tmp"' EXIT
cat >"$tmp" <<'GO'
package main
import (
"context"
"fmt"
"net/http"
"net/http/httptest"
"time"
)
func main() {
seenCanceled := make(chan bool, 2)
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
select {
case <-r.Context().Done():
seenCanceled <- true
case <-time.After(500 * time.Millisecond):
seenCanceled <- false
}
}))
defer server.Close()
outer, cancel := context.WithCancel(context.Background())
done := make(chan error, 1)
go func() {
_, err := (&http.Client{Timeout: time.Second}).Get(server.URL)
done <- err
}()
cancel()
select {
case <-seenCanceled:
fmt.Println("client.Get propagated outer cancellation: true")
case <-time.After(100 * time.Millisecond):
fmt.Println("client.Get propagated outer cancellation: false")
}
<-done
requestCtx, requestCancel := context.WithCancel(context.Background())
req, err := http.NewRequestWithContext(requestCtx, http.MethodGet, server.URL, nil)
if err != nil {
panic(err)
}
done = make(chan error, 1)
go func() {
_, err := (&http.Client{Timeout: time.Second}).Do(req)
done <- err
}()
requestCancel()
select {
case propagated := <-seenCanceled:
fmt.Printf("NewRequestWithContext propagated cancellation: %v\n", propagated)
case <-time.After(100 * time.Millisecond):
fmt.Println("NewRequestWithContext propagated cancellation: false")
}
<-done
}
GO
go run "$tmp"Repository: openshift/operator-framework-operator-controller
Length of output: 298
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
tmp="$(mktemp --suffix=.go)"
trap 'rm -f "$tmp"' EXIT
cat >"$tmp" <<'GO'
package main
import (
"context"
"fmt"
"net/http"
"net/http/httptest"
"time"
)
func main() {
seenCanceled := make(chan bool, 2)
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
select {
case <-r.Context().Done():
seenCanceled <- true
case <-time.After(500 * time.Millisecond):
seenCanceled <- false
}
}))
defer server.Close()
outer, cancel := context.WithCancel(context.Background())
done := make(chan error, 1)
go func() {
_, err := (&http.Client{Timeout: time.Second}).Get(server.URL)
done <- err
}()
cancel()
select {
case <-seenCanceled:
fmt.Println("client.Get propagated outer cancellation: true")
case <-time.After(100 * time.Millisecond):
fmt.Println("client.Get propagated outer cancellation: false")
}
<-done
_ = outer
requestCtx, requestCancel := context.WithCancel(context.Background())
req, err := http.NewRequestWithContext(requestCtx, http.MethodGet, server.URL, nil)
if err != nil {
panic(err)
}
done = make(chan error, 1)
go func() {
_, err := (&http.Client{Timeout: time.Second}).Do(req)
done <- err
}()
requestCancel()
select {
case propagated := <-seenCanceled:
fmt.Printf("NewRequestWithContext propagated cancellation: %v\n", propagated)
case <-time.After(100 * time.Millisecond):
fmt.Println("NewRequestWithContext propagated cancellation: false")
}
<-done
}
GO
CGO_ENABLED=0 go run "$tmp"Repository: openshift/operator-framework-operator-controller
Length of output: 287
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
tmp="$(mktemp --suffix=.go)"
trap 'rm -f "$tmp"' EXIT
cat >"$tmp" <<'GO'
package main
import (
"context"
"fmt"
"net/http"
"net/http/httptest"
"time"
)
func main() {
ready := make(chan struct{})
canceled := make(chan struct{})
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
close(ready)
<-r.Context().Done()
close(canceled)
}))
defer server.Close()
ctx1, cancel1 := context.WithCancel(context.Background())
go func() {
_, _ = (&http.Client{Timeout: time.Second}).Get(server.URL)
}()
select {
case <-ready:
case <-time.After(time.Second):
panic("client.Get did not reach the handler")
}
cancel1()
select {
case <-canceled:
fmt.Println("client.Get propagated an unrelated context cancellation: true")
case <-time.After(100 * time.Millisecond):
fmt.Println("client.Get propagated an unrelated context cancellation: false")
}
_ = ctx1
ready = make(chan struct{})
canceled = make(chan struct{})
ctx2, cancel2 := context.WithCancel(context.Background())
req, err := http.NewRequestWithContext(ctx2, http.MethodGet, server.URL, nil)
if err != nil {
panic(err)
}
go func() {
_, _ = (&http.Client{Timeout: time.Second}).Do(req)
}()
select {
case <-ready:
case <-time.After(time.Second):
panic("context-bound request did not reach the handler")
}
cancel2()
select {
case <-canceled:
fmt.Println("NewRequestWithContext propagated cancellation: true")
case <-time.After(time.Second):
fmt.Println("NewRequestWithContext propagated cancellation: false")
}
}
GO
CGO_ENABLED=0 go run "$tmp"Repository: openshift/operator-framework-operator-controller
Length of output: 314
Bind the health request to the scenario context.
catalogPortForwardAlive does not use the scenario context, so cancellation cannot stop its HTTP request before the client timeout. Pass ctx from both callers, create the request with http.NewRequestWithContext, and call client.Do.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 139-142: This http.Transport is configured with a tls.Config that sets InsecureSkipVerify: true, which disables TLS certificate verification for every request made through the resulting http.Client. The server's certificate chain and host name are not validated, exposing the connection to man-in-the-middle attacks. Remove InsecureSkipVerify (or set it to false) and supply a proper RootCAs pool if you need to trust custom certificates.
Context: http.Transport{
TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec
DialContext: (&net.Dialer{Timeout: 2 * time.Second}).DialContext,
}
Note: [CWE-295] Improper Certificate Validation.
(http-transport-tls-skip-verify-go)
[warning] 140-140: MinVersionis missing from this TLS configuration. By default, TLS 1.2 is currently used as the minimum when acting as a client, and TLS 1.0 when acting as a server. General purpose web applications should default to TLS 1.3 with all other protocols disabled. Only where it is known that a web server must support legacy clients with unsupported an insecure browsers (such as Internet Explorer 10), it may be necessary to enable TLS 1.0 to provide support. AddMinVersion: tls.VersionTLS13' to the TLS configuration to bump the minimum version to TLS 1.3.
Context: tls.Config{InsecureSkipVerify: true}
Note: [CWE-327]: Use of a Broken or Risky Cryptographic Algorithm [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures
(missing-ssl-minversion-go)
🪛 OpenGrep (1.26.0)
[ERROR] 141-141: TLS certificate verification is disabled via InsecureSkipVerify. This allows man-in-the-middle attacks. Remove InsecureSkipVerify or set it to false.
(coderabbit.tls.go-insecure-skip-verify)
[ERROR] 141-141: TLS certificate verification is disabled via InsecureSkipVerify. This allows man-in-the-middle attacks. Remove InsecureSkipVerify or set it to false.
(coderabbit.tls.go-insecure-skip-verify)
🤖 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/e2e/steps/demo_steps.go` around lines 137 - 145, Update
catalogPortForwardAlive to accept a scenario context, pass ctx from both
callers, and create the health request with http.NewRequestWithContext before
executing it via client.Do. Preserve the existing URL, timeout, and
response-check behavior while ensuring cancellation interrupts the request.
Source: Path instructions
Comment on lines
+137
to
+150
| func catalogPortForwardAlive(addr string) bool { | ||
| client := &http.Client{ | ||
| Timeout: 3 * time.Second, | ||
| Transport: &http.Transport{ | ||
| TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec | ||
| DialContext: (&net.Dialer{Timeout: 2 * time.Second}).DialContext, | ||
| }, | ||
| } | ||
| resp, err := client.Get(fmt.Sprintf("https://%s/", addr)) | ||
| if err != nil { | ||
| return false | ||
| } | ||
| resp.Body.Close() | ||
| return true |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file="test/e2e/steps/demo_steps.go"
sed -n '100,175p' "$file"
printf '\n-- related symbols --\n'
rg -n -C 4 'catalogPortForwardAlive|waitFor|non-leader|404|StatusCode' test/e2eRepository: openshift/operator-framework-operator-controller
Length of output: 43094
🏁 Script executed:
#!/bin/bash
set -eu
cat <<'GO' >/tmp/http_probe_check.go
package main
import (
"fmt"
"net/http"
"net/http/httptest"
)
func main() {
for _, code := range []int{http.StatusOK, http.StatusNotFound, http.StatusInternalServerError} {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(code)
}))
resp, err := http.Get(server.URL)
fmt.Printf("status=%d err_nil=%t response_non_nil=%t\n", code, err == nil, resp != nil)
if resp != nil {
_ = resp.Body.Close()
}
server.Close()
}
}
GO
if command -v go >/dev/null 2>&1; then
go run /tmp/http_probe_check.go
else
echo "go is unavailable; source-level result: net/http returns a response with nil error for HTTP status responses, including 404"
fiRepository: openshift/operator-framework-operator-controller
Length of output: 298
🌐 Web query:
Go net/http Client.Get returns error only on client policy or protocol failure HTTP 404 response documentation
💡 Result:
The Go net/http Client.Get method (and the underlying Client.Do) does not return an error for HTTP status codes, including HTTP 404 (Not Found) [1][2]. An HTTP 404 response is considered a successful HTTP transaction from the perspective of the client, meaning the request was successfully sent and a valid HTTP response was received [1][3]. According to the official Go documentation, an error is only returned by Client.Get if [1][4]: - There is a failure in the client's policy, such as when the CheckRedirect function returns an error [5][4]. - There is a failure to complete the HTTP request due to low-level issues, such as network connectivity problems, protocol errors, or request timeouts [1][5]. When the returned error is nil, the response object (resp) is non-nil, and the caller is responsible for checking the resp.StatusCode to handle specific HTTP status codes like 404 [1][2]. Example pattern for checking 404: resp, err:= http.Get("http://example.com/not-found") if err!= nil { // Handle network, protocol, or policy errors return err } defer resp.Body.Close if resp.StatusCode == http.StatusNotFound { // Handle 404 specifically return fmt.Errorf("resource not found") }
Citations:
- 1: https://pkg.go.dev/net/http@go1.26.5
- 2: https://groups.google.com/g/golang-nuts/c/2d5zyBoEuvM
- 3: https://github.com/golang/go/blob/master/src/net/http/client.go
- 4: https://pkg.go.dev/net/http
- 5: https://github.com/golang/go/blob/go1.20/src/net/http/client.go
Reject failed HTTP responses in the port-forward probe.
Return false for non-2xx responses. Return false when resp.Body.Close() fails.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 139-142: This http.Transport is configured with a tls.Config that sets InsecureSkipVerify: true, which disables TLS certificate verification for every request made through the resulting http.Client. The server's certificate chain and host name are not validated, exposing the connection to man-in-the-middle attacks. Remove InsecureSkipVerify (or set it to false) and supply a proper RootCAs pool if you need to trust custom certificates.
Context: http.Transport{
TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec
DialContext: (&net.Dialer{Timeout: 2 * time.Second}).DialContext,
}
Note: [CWE-295] Improper Certificate Validation.
(http-transport-tls-skip-verify-go)
[warning] 140-140: MinVersionis missing from this TLS configuration. By default, TLS 1.2 is currently used as the minimum when acting as a client, and TLS 1.0 when acting as a server. General purpose web applications should default to TLS 1.3 with all other protocols disabled. Only where it is known that a web server must support legacy clients with unsupported an insecure browsers (such as Internet Explorer 10), it may be necessary to enable TLS 1.0 to provide support. AddMinVersion: tls.VersionTLS13' to the TLS configuration to bump the minimum version to TLS 1.3.
Context: tls.Config{InsecureSkipVerify: true}
Note: [CWE-327]: Use of a Broken or Risky Cryptographic Algorithm [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures
(missing-ssl-minversion-go)
🪛 OpenGrep (1.26.0)
[ERROR] 141-141: TLS certificate verification is disabled via InsecureSkipVerify. This allows man-in-the-middle attacks. Remove InsecureSkipVerify or set it to false.
(coderabbit.tls.go-insecure-skip-verify)
[ERROR] 141-141: TLS certificate verification is disabled via InsecureSkipVerify. This allows man-in-the-middle attacks. Remove InsecureSkipVerify or set it to false.
(coderabbit.tls.go-insecure-skip-verify)
🤖 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/e2e/steps/demo_steps.go` around lines 137 - 150, Update
catalogPortForwardAlive to return false when the probe receives a non-2xx HTTP
status, and propagate a false result if resp.Body.Close() returns an error; only
return true after both the request succeeds with a successful status and the
response body closes successfully.
Source: Path instructions
Comment on lines
175
to
177
| script := fmt.Sprintf( | ||
| `curl -s -k https://%s/catalogs/%s/api/v1/all | jq -s '%s'`, | ||
| `set -o pipefail; curl -sS -k --compressed --fail https://%s/catalogs/%s/api/v1/all | jq '%s'`, | ||
| addr, catalogName, jqFilter, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Trace Gherkin captures into the shell command and inspect the CI trust boundary.
ast-grep outline test/e2e/steps/demo_steps.go --items all --type function \
--match 'RegisterDemoSteps|catalogCurlJq|PackageHasSomeChannels|PackageHasSomeBundles'
rg -n -C 4 --glob '*.feature' \
'catalog "[^"]*"|package "[^"]*" in catalog' test
rg -n -C 5 --type go \
'\bcatalogCurlJq\s*\(|\bbash\s*\(' test/e2e
fd -t f -e yml -e yaml .github 2>/dev/null \
| xargs -r rg -n -C 3 'pull_request_target|workflow_run|artifact|credentials|kubeconfig'Repository: openshift/operator-framework-operator-controller
Length of output: 31891
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
sed -n '15,35p;166,230p' test/e2e/steps/demo_steps.go
python3 - <<'PY'
from pathlib import Path
import re
s = Path("test/e2e/steps/demo_steps.go").read_text()
patterns = re.findall(r'sc\.Step\(`([^`]+)`', s)
for p in patterns:
if "package" in p or "catalog" in p:
print(p)
# Model the two interpolations without executing the resulting command.
catalog = 'safe; echo CATALOG_PWNED; #'
package = 'safe"; echo PACKAGE_PWNED; #'
jq = f'objects | select(.schema == "olm.channel") | select(.package == "{package}") | .name'
script = (
"set -o pipefail; curl -sS -k --compressed --fail "
f"https://127.0.0.1/catalogs/{catalog}/api/v1/all | jq '{jq}'"
)
Path("/tmp/generated-catalog-query.sh").write_text(script)
print("\nGenerated command:")
print(script)
PY
bash -n /tmp/generated-catalog-query.sh; printf 'bash -n status: %s\n' "$?"Repository: openshift/operator-framework-operator-controller
Length of output: 4317
Injection (CWE-78): Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection')
Reachability: Internal
Remove shell interpolation from catalog queries.
catalogName and packageName come from Gherkin captures and can inject commands into bash -c. Use separate exec.CommandContext calls for curl and jq; path-escape catalogName and pass packageName through jq --arg.
🤖 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/e2e/steps/demo_steps.go` around lines 175 - 177, Replace the
shell-interpolated script in the catalog query step with separate
exec.CommandContext invocations for curl and jq, preserving pipefail-equivalent
error handling. Path-escape catalogName when constructing the curl URL, and pass
packageName to jq via --arg rather than interpolating it into jqFilter; update
the surrounding query logic in the relevant step function accordingly.
Source: Path instructions
Contributor
|
/retest |
…2848) Serial tests (HA, TLS, proxy scenarios) patch operator-controller and catalogd Deployments, triggering rolling updates that delete the original pods. The support bundle collected at the end only captures replacement pod logs, so parallel test failure logs are lost. Deploy Fluent Bit (gated on ARTIFACT_PATH) to continuously persist olmv1-system container logs to the node filesystem. A copyFromHost collector in the support bundle extracts these persisted logs into the operator-logs/ directory, preserving the complete log history across all pod generations including deleted and restarted instances. Co-authored-by: Claude <noreply@anthropic.com>
Bumps [github.com/prometheus/common](https://github.com/prometheus/common) from 0.70.0 to 0.70.1. - [Release notes](https://github.com/prometheus/common/releases) - [Changelog](https://github.com/prometheus/common/blob/main/CHANGELOG.md) - [Commits](prometheus/common@v0.70.0...v0.70.1) --- updated-dependencies: - dependency-name: github.com/prometheus/common dependency-version: 0.70.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [certifi](https://github.com/certifi/python-certifi) from 2026.6.17 to 2026.7.22. - [Commits](certifi/python-certifi@2026.06.17...2026.07.22) --- updated-dependencies: - dependency-name: certifi dependency-version: 2026.7.22 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Todd Short <todd.short@me.com>
…to run outside of OCP
…ffinity for HA topology Rolling updates in HighlyAvailable clusters leave catalogd and operator-controller unavailable when the only running pod is evicted before its replacement is ready. Fix by defaulting replicas=1 and PDB disabled in the static Helm values (safe for SNO/External topologies, passes the SNO conformance test that asserts exactly one replica in SingleReplica topology mode). Add pod anti-affinity to prefer scheduling replicas on different nodes. cluster-olm-operator detects the cluster's ControlPlaneTopology at startup and overrides these values to replicas=2 and PDB enabled when a HighlyAvailable topology is detected, then re-renders the manifests before starting controllers. When a topology change is observed at runtime (exceedingly rare), the operator exits so its deployment controller restarts it, triggering a fresh Helm render with the correct values for the new topology. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Todd Short <tshort@redhat.com>
…etween both-watch-modes scenarios The both-watch-modes test loops over two scenarios (singlens, ownns) inside a single It block and was blocking on full namespace deletion between them. This caused flaky 300s timeouts on GCP techpreview clusters where master nodes run at 94-99% CPU, which starves the namespace controller and makes namespace termination arbitrarily slow. The wait was not guarding anything real: - EnsureCleanupClusterExtension already ensures the CE and CRD are gone; since CE deletion uses ForegroundPropagation, the ClusterObjectSet teardown must complete before the CE disappears, meaning all managed resources (Deployments, Services, etc.) are already deleted at that point. - The singleown bundle installs no ValidatingWebhookConfiguration or MutatingWebhookConfiguration, so there is no webhook admission risk. - Each scenario generates unique namespace names and CRD group suffixes via rand.String(4), so a terminating namespace from scenario 1 cannot collide with or interfere with scenario 2's resources. Trigger both namespace deletions and proceed without waiting. The DeferCleanup registrations that already exist will handle any residual cleanup after the spec exits. Fixes: OCPBUGS-84943 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Todd Short <tshort@redhat.com>
- Replace broken test-experimental-e2e target (test/experimental-e2e no longer exists) with /bin/true so triggered jobs always succeed - Pass -timeout=60m to go test; the previous invocation relied on Go's 10m default which is too short for BoxcutterRuntime clusters - Set E2E_STEP_TIMEOUT=15m; BoxcutterRuntime applies resources through sequential phases (CRD must reach Established before the deploy phase starts), making installations slower than the upstream 5m default - Skip ~@CatalogdHA scenarios (require multiple catalogd replicas not present in standard topology) - Skip ~@ProgressDeadline scenarios (require progressDeadlineMinutes < 10 but the OpenShift CRD enforces a minimum of 10) - Skip ~@httpproxy scenarios (too disruptive to cluster networking) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Todd Short <tshort@redhat.com>
The e2e-test-registry image is no longer built by CI after openshift/release#78581 removed it from the CI config. The dynamic per-scenario catalog system replaced the pre-built registry image, making this Dockerfile dead code.
It's no longer bring used. Signed-off-by: Todd Short <tshort@redhat.com>
Adds a new test that verifies cluster-olm-operator correctly configures operator-controller and catalogd deployments based on the cluster's control plane topology: - HA topologies (HighlyAvailable, HighlyAvailableArbiter, DualReplica): replicas=2 with a PodDisruptionBudget present - Non-HA topologies (SingleReplica/SNO, External): replicas=1, no PDB Also registers policyv1 in the test scheme to support PDB list queries. Assisted-by: claude Signed-off-by: Todd Short <tshort@redhat.com>
… builders Signed-off-by: Todd Short <tshort@redhat.com>
…ge to be consistent with ART for 5.0 Reconciling with https://github.com/openshift-eng/ocp-build-data/tree/7691ed4dc0b6585b358f9e73fb736ace9a48a286/images/ose-olm-operator-controller.yml
Set catalog image tags to v5.0 for the 4.23/5.0 release. Dynamically discover an installable package from the serving catalogs instead of hardcoding quay-operator v3.13.10, preferring quay-operator, cluster-logging, serverless-operator, logic-operator in that order then alling back to the first available package. Signed-off-by: Todd Short <tshort@redhat.com>
…ntal manifests HelmChartSupport was removed upstream in dbc9b4a but the downstream experimental.yaml values file and its generated manifest still referenced it, causing operator-controller to crash on startup with: invalid argument "HelmChartSupport=false" for "--feature-gates" flag: unrecognized feature gate: HelmChartSupport This made the OLM cluster operator Degraded/Unavailable and caused cluster installation to time out (exit code 6). Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…cluster version Add a second ReleaseGate-eligible OTE test verifying that an operator whose olm.maxOpenShiftVersion exceeds the current cluster version does not block cluster upgrade (InstalledOLMOperatorsUpgradeable stays True). The existing test only covered the blocking path (maxOCPVersion == current version → False). This covers the complementary allow path (maxOCPVersion == next minor → True), directly exercising the normalization logic introduced for the 4.23/5.0 co-release boundary. A nextMinorVersion() helper mirrors the 4.23→5.1 special case so the bundle annotation is always set to the correct next upgrade target. Run 'make build-update' to register the new allow-case test in the extension metadata after adding it to olmv1-incompatible.go. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Todd Short <tshort@redhat.com>
Automate the ClusterExtension rollout failure coverage for OCP-88331 and OCP-88332 by building in-cluster bundle and catalog images for successful and failing bundle versions. The new QE specs verify ProgressDeadlineExceeded on an initial failed rollout and ProbeFailure while upgrading to a bad revision under the BoxCutter runtime. Signed-off-by: Daniel Franz <dfranz@redhat.com> Co-authored-by: Bruno Andrade <bruno.balint@gmail.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Todd Short <tshort@redhat.com>
…eAccount usage in ClusterExtension API, Synthetic Permissions, and PreFlight Admissions Signed-off-by: Daniel Franz <dfranz@redhat.com>
…grade boundary Fix GetNextMinorVersion to return "5.1" for 4.23 clusters instead of "4.24": OCP 4.23 and 5.0 are co-released equivalents whose only upgrade target is 5.1. Remove the redundant `&& strings.Contains(message, "5")` guard from the Upgradeable message poll — the expectedPattern built from GetNextMinorVersion now encodes the full version string and is sufficient on its own. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
openshift-bot
force-pushed
the
synchronize-upstream
branch
from
August 8, 2026 00:17
7a2aa3d to
501b500
Compare
Contributor
|
New changes are detected. LGTM label has been removed. |
Contributor
|
@openshift-bot: The following tests 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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
17 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The downstream repository has been updated with the following following upstream commits:
The
vendor/directory has been updated and the following commits were carried:@catalogd-updateThis pull request is expected to merge without any human intervention. If tests are failing here, changes must land upstream to fix any issues so that future downstreaming efforts succeed.
/assign @openshift/openshift-team-operator-runtime
Summary by CodeRabbit
New Features
Bug Fixes
Chores