NO-ISSUE: Synchronize From Upstream Repositories - #788
Open
openshift-bot wants to merge 124 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. |
WalkthroughThe PR adds Fluent Bit log persistence for E2E support bundles, improves catalog port-forward recovery and query retries, and updates Go and Python dependency pins. ChangesE2E observability and catalog reliability
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, 2 warnings)
✅ Passed checks (11 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>
…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 6, 2026 00:12
a17ced3 to
f217c65
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. |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: openshift-bot 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
testdata/fluentbit/values.yaml (1)
31-36: 🩺 Stability & Availability | 🔵 TrivialBound persisted log growth.
The file output writes records to a hostPath directory. This configuration has no rotation, retention, or size limit. The directory survives Fluent Bit pod restarts, and the support-bundle collector later copies the directory. Repeated retries or rolling updates can consume node disk or exceed the collector timeout. (docs.fluentbit.io)
Clean the directory at run start or end, or add a bounded retention mechanism. Verify whether E2E retries reuse the same nodes.
Also applies to: 38-45
🤖 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 `@testdata/fluentbit/values.yaml` around lines 31 - 36, Update the Fluent Bit file-output configuration in outputs to bound persisted log growth across restarts and E2E retries, using a supported cleanup, rotation, retention, or size-limiting mechanism. Ensure the hostPath directory cannot grow without limit before the support-bundle collector processes it, and apply the same change to the additionally referenced output configuration.
🤖 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 `@testdata/fluentbit/values.yaml`:
- Around line 18-29: Update the chart’s host `/var/log` volume mount to set
`readOnly: true`, preserving all existing mounts and keeping the separate
`/var/log/fluentbit` mount writable.
- Around line 12-17: Update the Fluent Bit [SERVICE] configuration in
values.yaml to include Health_Check On, preserving the existing readiness probe
endpoint and service settings.
- Around line 12-17: Update the Fluent Bit values configuration by enabling
networkPolicy and restricting its ingress rules to only the required monitoring
and probe sources. Preserve the HTTP server on port 2020 while removing
unrestricted pod access, using the chart’s existing networkPolicy configuration
keys and source selectors.
- Around line 4-10: Update the Fluent Bit chart values by setting
serviceAccount.automountServiceAccountToken to false, while preserving the
existing resource limits and requests configuration.
- Around line 4-10: Update the Fluent Bit container configuration in values.yaml
to define a securityContext with runAsNonRoot, readOnlyRootFilesystem,
allowPrivilegeEscalation set to false, and all capabilities dropped. Ensure the
/var/log/fluentbit mount remains writable despite the read-only root filesystem
setting.
---
Nitpick comments:
In `@testdata/fluentbit/values.yaml`:
- Around line 31-36: Update the Fluent Bit file-output configuration in outputs
to bound persisted log growth across restarts and E2E retries, using a supported
cleanup, rotation, retention, or size-limiting mechanism. Ensure the hostPath
directory cannot grow without limit before the support-bundle collector
processes it, and apply the same change to the additionally referenced output
configuration.
🪄 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: 22dfe784-ab4e-441d-b070-5ae1d1fc0252
⛔ 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 (7)
Makefilego.modopenshift/tests-extension/go.modrequirements.txttest/e2e/steps/demo_steps.gotest/e2e/support-bundle.yamltestdata/fluentbit/values.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- go.mod
- requirements.txt
- openshift/tests-extension/go.mod
Comment on lines
+4
to
+10
| resources: | ||
| limits: | ||
| cpu: 100m | ||
| memory: 128Mi | ||
| requests: | ||
| cpu: 50m | ||
| memory: 64Mi |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- candidate values file ---'
cat -n testdata/fluentbit/values.yaml
printf '%s\n' '--- Fluent Bit-related files and references ---'
git ls-files | rg -i 'fluent.?bit|values\.yaml|helm|serviceaccount' | head -200
rg -n -i --glob '!vendor/**' --glob '!node_modules/**' 'fluent.?bit|automountServiceAccountToken|serviceAccount:' . | head -300Repository: openshift/operator-framework-operator-controller
Length of output: 32615
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- deployment command and chart version ---'
sed -n '380,400p' Makefile
sed -n '640,650p' Makefile
printf '%s\n' '--- Fluent Bit chart values and service-account template ---'
base='https://raw.githubusercontent.com/fluent/helm-charts/fluent-bit-0.57.9/charts/fluent-bit'
curl -fsSL "$base/values.yaml" | rg -n -A12 -B3 'serviceAccount|automountServiceAccountToken'
curl -fsSL "$base/templates/serviceaccount.yaml" | cat -n
curl -fsSL "$base/templates/daemonset.yaml" | rg -n -A8 -B8 'serviceAccountName|automountServiceAccountToken'Repository: openshift/operator-framework-operator-controller
Length of output: 2889
🏁 Script executed:
#!/bin/bash
set -euo pipefail
api='https://api.github.com/repos/fluent/helm-charts/git/trees/fluent-bit-0.57.9?recursive=1'
curl -fsSL "$api" |
jq -r '.tree[].path' |
rg '^charts/fluent-bit/templates/.*(daemon|deployment|rbac|role|serviceaccount)' || true
printf '%s\n' '--- chart RBAC templates ---'
for p in templates/clusterrole.yaml templates/role.yaml templates/clusterrolebinding.yaml templates/rolebinding.yaml templates/daemonset.yaml templates/deployment.yaml; do
url="https://raw.githubusercontent.com/fluent/helm-charts/fluent-bit-0.57.9/charts/fluent-bit/$p"
body="$(curl -fsSL "$url" 2>/dev/null || true)"
if [ -n "$body" ]; then
printf '\n--- %s ---\n' "$p"
printf '%s\n' "$body" | cat -n | sed -n '1,180p'
fi
doneRepository: openshift/operator-framework-operator-controller
Length of output: 6132
Security Misconfiguration (CWE-250)
Reachability: Internal
Disable the unused service-account token.
The Fluent Bit chart creates a service account and grants it read access to pods and namespaces. This configuration leaves serviceAccount.automountServiceAccountToken unset, although the pipeline only reads host log files. Set it to false unless Kubernetes API access is required.
🤖 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 `@testdata/fluentbit/values.yaml` around lines 4 - 10, Update the Fluent Bit
chart values by setting serviceAccount.automountServiceAccountToken to false,
while preserving the existing resource limits and requests configuration.
Source: Path instructions
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- matching Fluent Bit testdata files ---'
git ls-files | rg '(^|/)fluentbit(/|$)|fluent-bit' | head -80
printf '%s\n' '--- values.yaml context ---'
fd -i '^values\.yaml$' . | while read -r f; do
case "$f" in
*fluentbit*) echo "FILE: $f"; cat -n "$f" ;;
esac
done
printf '%s\n' '--- chart references and rendered-manifest evidence ---'
rg -n -i --glob '!vendor/**' 'fluentbit|fluent-bit|0\.57\.9|securityContext|podSecurityContext|automountServiceAccountToken|hostPath|/var/log/fluentbit' .Repository: openshift/operator-framework-operator-controller
Length of output: 50405
🏁 Script executed:
#!/bin/bash
set -eu
log=/tmp/coderabbit-shell-logs/shell-output-bRBay6
printf '%s\n' '--- beginning of prior output ---'
sed -n '1,180p' "$log"
printf '%s\n' '--- exact tracked Fluent Bit paths ---'
git ls-files | rg '(^|/)fluentbit(/|$)|fluent-bit' | head -40
printf '%s\n' '--- exact references outside vendor ---'
rg -n -i --glob '!**/vendor/**' --glob '!**/testdata/**' 'fluentbit|fluent-bit' . | head -100Repository: openshift/operator-framework-operator-controller
Length of output: 19207
🏁 Script executed:
#!/bin/bash
set -eu
base=https://raw.githubusercontent.com/fluent/helm-charts/fluent-bit-0.57.9/charts/fluent-bit
tmp=$(mktemp -d)
trap 'rm -rf "$tmp"' EXIT
curl -fsSL "$base/values.yaml" -o "$tmp/values.yaml"
curl -fsSL "$base/templates/daemonset.yaml" -o "$tmp/daemonset.yaml"
printf '%s\n' '--- chart defaults ---'
rg -n -C 4 '^(podSecurityContext|securityContext|automountServiceAccountToken|serviceAccount:|volumeMounts:|extraVolumeMounts:)' "$tmp/values.yaml"
printf '%s\n' '--- daemonset security and service-account rendering ---'
rg -n -C 6 'podSecurityContext|securityContext|automountServiceAccountToken|serviceAccountName|volumeMounts|extraVolumeMounts' "$tmp/daemonset.yaml"
printf '%s\n' '--- repository deployment invocation ---'
sed -n '380,398p' Makefile
sed -n '640,650p' MakefileRepository: openshift/operator-framework-operator-controller
Length of output: 1304
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- chart template files at fluent-bit-0.57.9 ---'
curl -fsSL 'https://api.github.com/repos/fluent/helm-charts/contents/charts/fluent-bit/templates?ref=fluent-bit-0.57.9' |
jq -r '.[] | .name'
printf '%s\n' '--- values relevant to service account and security ---'
curl -fsSL 'https://raw.githubusercontent.com/fluent/helm-charts/fluent-bit-0.57.9/charts/fluent-bit/values.yaml' |
sed -n '24,42p;66,105p;460,490p'Repository: openshift/operator-framework-operator-controller
Length of output: 2094
Security Misconfiguration (CWE-16)
Reachability: Internal · Exploitability: Moderate
Set the Fluent Bit container security context.
The Fluent Bit chart defaults securityContext to {}. Add runAsNonRoot, readOnlyRootFilesystem, allowPrivilegeEscalation: false, and dropped capabilities. Keep /var/log/fluentbit writable.
Proposed container hardening
+securityContext:
+ allowPrivilegeEscalation: false
+ capabilities:
+ drop:
+ - ALL
+ readOnlyRootFilesystem: true
+ runAsNonRoot: true📝 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.
Suggested change
| resources: | |
| limits: | |
| cpu: 100m | |
| memory: 128Mi | |
| requests: | |
| cpu: 50m | |
| memory: 64Mi | |
| resources: | |
| limits: | |
| cpu: 100m | |
| memory: 128Mi | |
| requests: | |
| cpu: 50m | |
| memory: 64Mi | |
| securityContext: | |
| allowPrivilegeEscalation: false | |
| capabilities: | |
| drop: | |
| - ALL | |
| readOnlyRootFilesystem: true | |
| runAsNonRoot: true |
🤖 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 `@testdata/fluentbit/values.yaml` around lines 4 - 10, Update the Fluent Bit
container configuration in values.yaml to define a securityContext with
runAsNonRoot, readOnlyRootFilesystem, allowPrivilegeEscalation set to false, and
all capabilities dropped. Ensure the /var/log/fluentbit mount remains writable
despite the read-only root filesystem setting.
Source: Path instructions
Comment on lines
+12
to
+17
| service: | | ||
| [SERVICE] | ||
| Flush 5 | ||
| Log_Level warn | ||
| Daemon off | ||
| HTTP_Server On |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rendered="$(mktemp)"
trap 'rm -f "$rendered"' EXIT
helm template fluent-bit oci://ghcr.io/fluent/helm-charts/fluent-bit \
--version 0.57.9 \
--namespace fluent-bit \
-f testdata/fluentbit/values.yaml >"$rendered"
rg -n -C 4 'readinessProbe|/api/v2/health|HTTP_Server|Health_Check' "$rendered"Repository: openshift/operator-framework-operator-controller
Length of output: 228
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- repository files ---'
git ls-files | rg '(^|/)(values\.yaml|Makefile)$|fluentbit'
printf '%s\n' '--- values.yaml ---'
cat -n testdata/fluentbit/values.yaml
printf '%s\n' '--- relevant Makefile references ---'
rg -n -C 3 -- '--wait|timeout|fluent-bit|fluentbit' Makefile .github testdata 2>/dev/null || trueRepository: openshift/operator-framework-operator-controller
Length of output: 13206
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
base='https://raw.githubusercontent.com/fluent/helm-charts/fluent-bit-0.57.9/charts/fluent-bit'
for file in values.yaml templates/configmap.yaml templates/daemonset.yaml; do
url="$base/$file"
printf '\n--- %s ---\n' "$url"
curl -fsSL "$url" | nl -ba | rg -n -C 5 \
'Health_Check|HTTP_Server|readinessProbe|/api/v2/health|config\.service|service:'
done
printf '\n--- Fluent Bit health-check documentation ---\n'
curl -fsSL \
'https://docs.fluentbit.io/manual/4.2/administration/configuring-fluent-bit/yaml/service-section' |
rg -n -C 5 'Health_Check|health endpoint|/api/v2/health'Repository: openshift/operator-framework-operator-controller
Length of output: 406
🌐 Web query:
Fluent Bit 4.2 Health_Check On /api/v2/health HTTP server service section
💡 Result:
In Fluent Bit 4.2, the health check feature is configured within the [SERVICE] section of the configuration file. To use the /api/v2/health endpoint, you must enable the built-in HTTP server and the health check feature [1][2][3]. Configuration requirements: 1. Enable the HTTP server: Set http_server On (or http_server: on in YAML) [2][3]. 2. Enable the health check: Set health_check On (or health_check: on in YAML) [2][3]. The health status is determined based on the number of errors and retry failures occurring across all output plugins within a defined time period [1][4]. The following parameters control this behavior: - hc_errors_count: The threshold for output errors (default: 5) [2][3]. - hc_retry_failure_count: The threshold for retry failures (default: 5) [2][3]. - hc_period: The time window in seconds for the evaluation (default: 60) [2][3]. If the number of errors exceeds hc_errors_count OR the number of retry failures exceeds hc_retry_failure_count within the hc_period, Fluent Bit is considered unhealthy [1][4]. In this state, the /api/v2/health endpoint returns an HTTP 500 status code with an "error" message; otherwise, it returns HTTP 200 with an "ok" message [1][4]. Example configuration ([SERVICE] section): [SERVICE] HTTP_Server On HTTP_Listen 0.0.0.0 HTTP_Port 2020 Health_Check On HC_Errors_Count 5 HC_Retry_Failure_Count 5 HC_Period 5 You can query the status using: curl -s http://127.0.0.1:2020/api/v2/health [1]
Citations:
- 1: https://docs.fluentbit.io/manual/administration/monitoring
- 2: https://docs.fluentbit.io/manual/administration/configuring-fluent-bit/yaml/service-section
- 3: https://docs.fluentbit.io/manual/4.2/administration/configuring-fluent-bit/yaml/service-section
- 4: https://docs.fluentbit.io/manual/4.2/administration/monitoring
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
base='https://raw.githubusercontent.com/fluent/helm-charts/fluent-bit-0.57.9/charts/fluent-bit'
for file in values.yaml templates/configmap.yaml templates/daemonset.yaml; do
url="$base/$file"
printf '\n--- %s ---\n' "$url"
curl -fsSL "$url" |
rg -n -C 6 \
'Health_Check|HTTP_Server|readinessProbe|/api/v2/health|config\.service|service:'
doneRepository: openshift/operator-framework-operator-controller
Length of output: 2374
Enable Health_Check On in the [SERVICE] block.
The chart readiness probe requests /api/v2/health, but this override replaces the chart default and removes Health_Check On. The DaemonSet can remain unready, causing helm --wait --timeout 2m to fail. Add Health_Check On or configure a readiness endpoint served by this configuration.
🤖 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 `@testdata/fluentbit/values.yaml` around lines 12 - 17, Update the Fluent Bit
[SERVICE] configuration in values.yaml to include Health_Check On, preserving
the existing readiness probe endpoint and service settings.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate file ---'
nl -ba testdata/fluentbit/values.yaml | sed -n '1,90p'
printf '%s\n' '--- related Fluent Bit testdata files ---'
git ls-files 'testdata/fluentbit/*' | sort
printf '%s\n' '--- Fluent Bit references ---'
rg -n --glob '*.{yaml,yml,md,go,sh}' 'fluentbit|fluent-bit|NetworkPolicy|HTTP_Server|2020|monitoring' testdata deploy config charts 2>/dev/null | head -240Repository: openshift/operator-framework-operator-controller
Length of output: 949
🏁 Script executed:
#!/bin/bash
set -eu
nl -ba testdata/fluentbit/values.yaml | sed -n '1,90p'
git ls-files 'testdata/fluentbit/*' | sort
rg -n --glob '*.{yaml,yml,md,go,sh}' 'fluentbit|fluent-bit|NetworkPolicy|HTTP_Server|2020|monitoring' testdata deploy config charts 2>/dev/null | head -240Repository: openshift/operator-framework-operator-controller
Length of output: 854
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate file ---'
cat -n testdata/fluentbit/values.yaml
printf '%s\n' '--- repository metadata and chart context ---'
find . -maxdepth 3 -type f \( -iname '*fluent*' -o -iname '*helm*' -o -iname '*chart*' \) -print | sort | head -200
printf '%s\n' '--- exact values keys ---'
grep -nE 'HTTP_Server|NetworkPolicy|service:|port:|metrics|health|enabled|namespace|selector' testdata/fluentbit/values.yaml || trueRepository: openshift/operator-framework-operator-controller
Length of output: 2817
🏁 Script executed:
#!/bin/bash
set -eu
cat -n testdata/fluentbit/values.yaml
find . -maxdepth 3 -type f \( -iname '*fluent*' -o -iname '*helm*' -o -iname '*chart*' \) -print | sort | head -200
grep -nE 'HTTP_Server|NetworkPolicy|service:|port:|metrics|health|enabled|namespace|selector' testdata/fluentbit/values.yaml || trueRepository: openshift/operator-framework-operator-controller
Length of output: 2722
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- references to the values file ---'
rg -n --glob '!vendor/**' --glob '!go.sum' 'testdata/fluentbit/values\.yaml|fluentbit/values|fluent-bit' .
printf '%s\n' '--- chart dependency declarations ---'
rg -n --glob '*.{yaml,yml,mod,sum,go,sh}' 'fluent-bit|fluentbit|helm repo|helm install|helm upgrade' .
printf '%s\n' '--- upstream chart metadata and relevant templates ---'
curl -fsSL https://raw.githubusercontent.com/fluent/helm-charts/fluent-bit-0.57.9/charts/fluent-bit/values.yaml \
| grep -nE '^(service|networkPolicy| enabled:| type:| port:| http| annotations:)|httpPort|servicePort|NetworkPolicy' | head -120
curl -fsSL https://raw.githubusercontent.com/fluent/helm-charts/fluent-bit/fluent-bit-0.57.9/charts/fluent-bit/templates/service.yaml \
| sed -n '1,180p'
curl -fsSL https://raw.githubusercontent.com/fluent/helm-charts/fluent-bit/fluent-bit-0.57.9/charts/fluent-bit/templates/networkpolicy.yaml \
| sed -n '1,220p'Repository: openshift/operator-framework-operator-controller
Length of output: 1720
🏁 Script executed:
#!/bin/bash
set -eu
rg -n --glob '!vendor/**' --glob '!go.sum' 'testdata/fluentbit/values\.yaml|fluentbit/values|fluent-bit' .
rg -n --glob '*.{yaml,yml,mod,sum,go,sh}' 'fluent-bit|fluentbit|helm repo|helm install|helm upgrade' .
curl -fsSL https://raw.githubusercontent.com/fluent/helm-charts/fluent-bit-0.57.9/charts/fluent-bit/values.yaml | grep -nE '^(service|networkPolicy| enabled:| type:| port:| http| annotations:)|httpPort|servicePort|NetworkPolicy' | head -120
curl -fsSL https://raw.githubusercontent.com/fluent/helm-charts/fluent-bit/fluent-bit-0.57.9/charts/fluent-bit/templates/service.yaml | sed -n '1,180p'
curl -fsSL https://raw.githubusercontent.com/fluent/helm-charts/fluent-bit/fluent-bit-0.57.9/charts/fluent-bit/templates/networkpolicy.yaml | sed -n '1,220p'Repository: openshift/operator-framework-operator-controller
Length of output: 1589
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- deployment command ---'
sed -n '375,405p' Makefile
printf '%s\n' '--- namespace usage ---'
sed -n '635,655p' Makefile
printf '%s\n' '--- upstream chart templates ---'
for f in templates/service.yaml templates/networkpolicy.yaml templates/deployment.yaml; do
echo "--- $f ---"
curl -fsSL "https://raw.githubusercontent.com/fluent/helm-charts/fluent-bit-0.57.9/charts/fluent-bit/$f" | sed -n '1,240p'
doneRepository: openshift/operator-framework-operator-controller
Length of output: 6887
🏁 Script executed:
#!/bin/bash
set -eu
sed -n '375,405p' Makefile
sed -n '635,655p' Makefile
for f in templates/service.yaml templates/networkpolicy.yaml templates/deployment.yaml; do
echo "--- $f ---"
curl -fsSL "https://raw.githubusercontent.com/fluent/helm-charts/fluent-bit-0.57.9/charts/fluent-bit/$f" | sed -n '1,240p'
doneRepository: openshift/operator-framework-operator-controller
Length of output: 6803
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
import urllib.request
values = Path("testdata/fluentbit/values.yaml").read_text()
assert re.search(r"(?m)^\s*HTTP_Server\s+On\s*$", values)
assert re.search(r"(?m)^networkPolicy:\s*\n\s+enabled:\s+false\s*$", values)
print("local values: HTTP_Server=On; networkPolicy.enabled=false")
base = "https://raw.githubusercontent.com/fluent/helm-charts/fluent-bit-0.57.9/charts/fluent-bit/"
chart_values = urllib.request.urlopen(base + "values.yaml").read().decode()
service = urllib.request.urlopen(base + "templates/service.yaml").read().decode()
network_policy = urllib.request.urlopen(base + "templates/networkpolicy.yaml").read().decode()
configmap = urllib.request.urlopen(base + "templates/configmap.yaml").read().decode()
assert re.search(r"(?m)^service:\s*$", chart_values)
assert re.search(r"(?m)^\s+type:\s+ClusterIP\s*$", chart_values)
assert re.search(r"(?m)^\s+port:\s+2020\s*$", chart_values)
assert "if .Values.networkPolicy.enabled" in network_policy
assert 'port: {{ $.Values.service.port }}' in network_policy
assert 'targetPort: http' in service
print("chart 0.57.9: ClusterIP service port 2020; NetworkPolicy is conditional; policy targets HTTP port")
print("configmap HTTP-related lines:")
for line in configmap.splitlines():
if re.search(r"HTTP|http|service", line):
print(line)
PYRepository: openshift/operator-framework-operator-controller
Length of output: 2531
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
import urllib.request
values = Path("testdata/fluentbit/values.yaml").read_text()
assert re.search(r"(?m)^\s*HTTP_Server\s+On\s*$", values)
assert re.search(r"(?m)^networkPolicy:\s*\n\s+enabled:\s+false\s*$", values)
print("local values: HTTP_Server=On; networkPolicy.enabled=false")
base = "https://raw.githubusercontent.com/fluent/helm-charts/fluent-bit-0.57.9/charts/fluent-bit/"
chart_values = urllib.request.urlopen(base + "values.yaml").read().decode()
service = urllib.request.urlopen(base + "templates/service.yaml").read().decode()
network_policy = urllib.request.urlopen(base + "templates/networkpolicy.yaml").read().decode()
configmap = urllib.request.urlopen(base + "templates/configmap.yaml").read().decode()
assert re.search(r"(?m)^service:\s*$", chart_values)
assert re.search(r"(?m)^\s+type:\s+ClusterIP\s*$", chart_values)
assert re.search(r"(?m)^\s+port:\s+2020\s*$", chart_values)
assert "if .Values.networkPolicy.enabled" in network_policy
assert 'port: {{ $.Values.service.port }}' in network_policy
assert 'targetPort: http' in service
print("chart 0.57.9: ClusterIP service port 2020; NetworkPolicy is conditional; policy targets HTTP port")
for line in configmap.splitlines():
if re.search(r"HTTP|http|service", line):
print(line)
PYRepository: openshift/operator-framework-operator-controller
Length of output: 2531
🏁 Script executed:
#!/bin/bash
set -eu
curl -ksSfL https://raw.githubusercontent.com/fluent/helm-charts/fluent-bit-0.57.9/charts/fluent-bit/values.yaml \
| sed -n '365,395p'Repository: openshift/operator-framework-operator-controller
Length of output: 963
Security Misconfiguration (CWE-693)
Reachability: External
Restrict the Fluent Bit HTTP endpoint with a NetworkPolicy.
HTTP_Server On exposes port 2020, while networkPolicy.enabled: false permits access from any pod with network connectivity. Enable the NetworkPolicy and allow only required monitoring and probe sources.
🤖 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 `@testdata/fluentbit/values.yaml` around lines 12 - 17, Update the Fluent Bit
values configuration by enabling networkPolicy and restricting its ingress rules
to only the required monitoring and probe sources. Preserve the HTTP server on
port 2020 while removing unrestricted pod access, using the chart’s existing
networkPolicy configuration keys and source selectors.
Source: Path instructions
Comment on lines
+18
to
+29
| inputs: | | ||
| [INPUT] | ||
| Name tail | ||
| Path /var/log/containers/operator-controller-*_olmv1-system_*.log, /var/log/containers/catalogd-*_olmv1-system_*.log | ||
| multiline.parser cri | ||
| Tag <namespace_name>/<pod_name>.<container_name> | ||
| Tag_Regex (?<pod_name>[a-z0-9](?:[-a-z0-9]*[a-z0-9])?(?:\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*)_(?<namespace_name>[^_]+)_(?<container_name>.+)-[a-z0-9]{64}\.log$ | ||
| Read_from_Head true | ||
| Skip_Long_Lines On | ||
| Skip_Empty_Lines On | ||
| Mem_Buf_Limit 10MB | ||
| DB /var/log/fluentbit/tail.db |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- relevant files ---'
git ls-files | rg '(^|/)(values\.yaml|Chart\.yaml|daemonset|fluentbit)' | head -80
printf '%s\n' '--- references to daemonSetVolumeMounts and host paths ---'
rg -n -C 4 'daemonSetVolumeMounts|/var/log/fluentbit|/var/lib/docker/containers|/etc/machine-id|/var/log' testdata/fluentbit .github 2>/dev/null || trueRepository: openshift/operator-framework-operator-controller
Length of output: 8985
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- testdata/fluentbit/values.yaml ---'
cat -n testdata/fluentbit/values.yaml
printf '%s\n' '--- chart version reference ---'
rg -n -C 3 'fluent-bit|0\.57\.9' . --glob '!vendor/**' --glob '!hack/tools/test-profiling/vendor/**' --glob '!openshift/tests-extension/vendor/**' || true
printf '%s\n' '--- upstream chart defaults ---'
curl -fsSL https://raw.githubusercontent.com/fluent/helm-charts/fluent-bit-0.57.9/charts/fluent-bit/values.yaml |
rg -n -C 12 'daemonSetVolumeMounts|/var/log|/var/lib/docker/containers|/etc/machine-id' || trueRepository: openshift/operator-framework-operator-controller
Length of output: 5184
Security Misconfiguration (CWE-732): Incorrect Permission Assignment for Critical Resource
Reachability: Internal · Exploitability: Moderate
Mount /var/log read-only.
The chart default mounts the host /var/log path as writable. Set readOnly: true on that mount while preserving the existing mounts and the separate writable /var/log/fluentbit mount. A compromised Fluent Bit process could otherwise modify or delete other host logs.
🤖 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 `@testdata/fluentbit/values.yaml` around lines 18 - 29, Update the chart’s host
`/var/log` volume mount to set `readOnly: true`, preserving all existing mounts
and keeping the separate `/var/log/fluentbit` mount writable.
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
Bug Fixes
New Features
Chores