NO-ISSUE: Synchronize From Upstream Repositories - #788
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: 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 updates Go and Python dependency pins. It improves catalog port-forward setup, leader resolution, connection recovery, HTTPS query handling, subprocess error reporting, and retries for package, channel, and bundle checks. ChangesCatalog test and dependency updates
Estimated code review effort: 3 (Moderate) | ~20 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: Start or validate port forward
PortForward->>Catalogd: Forward HTTPS request
Catalogd-->>PortForward: Return health response
PortForward-->>CatalogChecks: Return connection status
CatalogChecks->>Catalogd: Query catalog data
Catalogd-->>CatalogChecks: Return catalog data or HTTP failure
CatalogChecks->>PortForward: Reset forward after query failure
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning, 1 inconclusive)
✅ Passed checks (12 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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>
Signed-off-by: dtfranz <dfranz@redhat.com> UPSTREAM: <carry>: Update generate-manifests to handle new directory The `default` directory was renamed `base`. Signed-off-by: Todd Short <todd.short@me.com> The `base` directory was moved to `base\operator-controller`. Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: Drop commitchecker Signed-off-by: Alexander Greene <greene.al1991@gmail.com> UPSTREAM: <carry>: Updating ose-olm-operator-controller-container image to be consistent with ART Reconciling with https://github.com/openshift/ocp-build-data/tree/4022cd290f00a44d667dda03f2d78d84a488c7ed/images/ose-olm-operator-controller.yml UPSTREAM: <carry>: update owners * Remove alumni from owners * Add m1kola to approvers Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com> UPSTREAM: <carry>: Add pointer to tooling README UPSTREAM: <carry>: Disable Validating Admission Policy APIs downstream Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com> UPSTREAM: <carry>: Updating ose-olm-operator-controller-container image to be consistent with ART for 4.16 Reconciling with https://github.com/openshift/ocp-build-data/tree/6250d54c4686a708ca5985afb73080e8ca9a1f7f/images/ose-olm-operator-controller.yml UPSTREAM: <carry>: Enable Validating Admission Policy APIs downstream * This reverts commit 3f079c4. * Includes Validating Admission Policy manifests Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com> UPSTREAM: <carry>: manifests: set required-scc for openshift workloads UPSTREAM: <carry>: Updating ose-olm-operator-controller-container image to be consistent with ART for 4.17 Reconciling with https://github.com/openshift/ocp-build-data/tree/4c1326094222f9209876f06833179a1b9178faf7/images/ose-olm-operator-controller.yml UPSTREAM: <carry>: add everettraven to approvers+reviewers Signed-off-by: everettraven <everettraven@gmail.com> UPSTREAM: <carry>: add openshift kustomize overlay to enable TLS communication with catalogd. Configure the CA certs using the configmap injection method via service-ca-operator Signed-off-by: everettraven <everettraven@gmail.com> UPSTREAM: <carry>: Add tmshort to approvers Also `s/runtime/framework/g` in the DOWNSTREAM_OWNERS Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: Updating ose-olm-operator-controller-container image to be consistent with ART for 4.18 Reconciling with https://github.com/openshift/ocp-build-data/tree/dd68246f3237db5db458127566fc7b05b55e1660/images/ose-olm-operator-controller.yml UPSTREAM: <carry>: Properly copy and call kustomize Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: manifests: add hostPath mount for /etc/containers Signed-off-by: Joe Lanford <joe.lanford@gmail.com> UPSTREAM: <carry>: Add test-e2e target for downstream Makefile to be run by openshift/release. Signed-off-by: dtfranz <dfranz@redhat.com> UPSTREAM: <carry>: Add downstream verify makefile target Signed-off-by: dtfranz <dfranz@redhat.com> UPSTREAM: <carry>: openshift: template log verbosity to be managed by cluster-olm-operator Signed-off-by: Joe Lanford <joe.lanford@gmail.com> UPSTREAM: <carry>: Add global-pull-secret flag Pass global-pull-secret to the manager container. Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com> UPSTREAM: <carry>: Update openshift CAs to operator-controller The /run/secrets/kubernetes.io/serviceaccount/ directory is projected into the pod and contains the following CA certificates: * configmap/kube-root-ca.crt as ca.crt * configmap/openshift-service-ca.crt as service-ca.crt Update the --ca-certs-dir argument to reference the directory. Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: Add HowTo for origin tests Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: Add e2e registry Dockerfile Signed-off-by: dtfranz <dfranz@redhat.com> UPSTREAM: <carry>: add nodeSelector and tolerations to operator-controller deployment via kustomize patch Signed-off-by: everettraven <everettraven@gmail.com> UPSTREAM: <carry>: namespace: use privileged PSA for audit and warn levels Signed-off-by: Joe Lanford <joe.lanford@gmail.com> UPSTREAM: <carry>: Enable downstream e2e Signed-off-by: dtfranz <dfranz@redhat.com> UPSTREAM: <carry>: Remove m1kola from owners Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com> UPSTREAM: <carry>: Updating ose-olm-operator-controller-container image to be consistent with ART for 4.19 Reconciling with https://github.com/openshift/ocp-build-data/tree/a39508c86497b4e5e463d7b2c78e51e577be9e7d/images/ose-olm-operator-controller.yml UPSTREAM: <carry>: generate and mount service-ca server cert Signed-off-by: Joe Lanford <joe.lanford@gmail.com> UPSTREAM: <carry>: Add support for proxy trustedCAs Just map the list of trusted ca certs into the deployment Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: Fix error to build the image Copy correct (new) executable name for operator-controller Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: Fix make verify for mac os envs Joe Lanford <joe.lanford@gmail.com> UPSTREAM: <carry>: Move operator-controller openshift files to its own dir UPSTREAM: <carry>: Upgrade OCP images from 4.18 to 4.19 UPSTREAM: <carry>: Add Openshift's catalogd manifests - Move to openshift/catalogd the specific manifest under: https://github.com/openshift/operator-framework-catalogd/tree/main/openshift - Add call to generate catalogd manifest to 'make manifest'. Make verify test is now done for catalogd and operator-controller Openshift's manifests UPSTREAM: <carry>: resolve issue with pre-mature mounting of trusted CA configmap Signed-off-by: Joe Lanford <joe.lanford@gmail.com> UPSTREAM: <carry>: Add /etc/docker to the operator-controller and catalogd deployments This allows for use of the any image.config.openshift.io trusted CAs Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: fixup catalogd.Dockerfile paths Signed-off-by: Joe Lanford <joe.lanford@gmail.com> UPSTREAM: <carry>: Resolve issue with pre-mature mounting of service CA configmap Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: use projected volume for CAs to avoid subPath limitations Signed-off-by: Joe Lanford <joe.lanford@gmail.com> UPSTREAM: <carry>: Revert "UPSTREAM: <carry>: use projected volume for CAs to avoid subPath limitations" This reverts commit 548caa4. UPSTREAM: <carry>: use projected volume for CAs to avoid subPath limitations Signed-off-by: Joe Lanford <joe.lanford@gmail.com> UPSTREAM: <carry>: Remove vet from openshift verify The `vet` target was removed upstream. Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: Skip another upstream test Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: Cleanup openshift/Makefile by removing no longer required comments regards catalogd e2e tests UPSTREAM: <carry>: Enable OCP metrics collection by default Enables OCP to collect Prometheus metrics for both catalogd and operator-controller by default. This is accomplished via ServiceMonitor CRs which are now created for both projects. UPSTREAM: <carry>: Fix catalogd.Dockerfile to use new paths The root catalogd directory has been removed Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: Update DOWNSTREAM_OWNERS_ALIASES Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: Add openshift node selector annotation Signed-off-by: Catherine Chan-Tse <cchantse@redhat.com> (cherry picked from commit 9b4a113) UPSTREAM: <carry>: Add caalogd-cas-dir option to op-con Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: set the SElinux type Signed-off-by: Jian Zhang <jiazha@redhat.com> UPSTREAM: <carry>: Add initial stack to run tests to validate the catalogs UPSTREAM: <carry>: Add vendor files for the catalog-sync tests UPSTREAM: <carry>: Bump catalog versions to 4.19 Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: revert "Bump catalog versions to 4.19" This reverts commit a98980b. UPSTREAM: <carry>: Update HOWTO-origin-tests techpreview is no longer a required option. Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: [DefaultCatalogTests]: Allow to pass auth path for docker credentials" UPSTREAM: <carry>: fix: set NoLchown=true to allow image unpack on OCPci UPSTREAM: <carry>: [DefaultCatalogTests]: Moving parse of ENVVAR to the caller (follow-up 345) UPSTREAM: <carry>: [Default Catalog]: Create tmp dir to extract layers with right permissions to avoid issues scenarios UPSTREAM: <carry>: [Default Catalog](cleanp) Remove hack directory which is not used UPSTREAM: <carry>: Change code implementation to extract layers in OCP env UPSTREAM: <carry>: Add vendor files for change in the extract code implementation UPSTREAM: <carry>: [Default Catalog Tests]: Final cleanups and enhancements of initial implementation UPSTREAM: <carry>: SELinux type for operator-controller Signed-off-by: Jian Zhang <jiazha@redhat.com> UPSTREAM: <carry>: Bump catalog versions to 4.19 Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: [Default Catalog Consistency Test] (feat) add check for executable files in filesystem Checks if given paths exist and point to executable files or valid symlinks. UPSTREAM: <carry>: [Default Catalog Consistency Test]: fix junit output format to allow generate xml UPSTREAM: <carry>: [Default Catalog Consistency Test] (feat) add check to validate multi-arch support UPSTREAM: <carry>: [Default Catalog Consistency Test]: Enable CatalogChecks UPSTREAM: <carry>: [Default Catalog Consistency Test]: Rename Tests suite and small cleanups UPSTREAM: <carry>: Updating ose-olm-operator-controller-container image to be consistent with ART for 4.20 Reconciling with https://github.com/openshift/ocp-build-data/tree/dfb5c7d531490cfdc61a3b88bc533702b9624997/images/ose-olm-operator-controller.yml UPSTREAM: <carry>: Updating ose-olm-catalogd-container image to be consistent with ART for 4.20 Reconciling with https://github.com/openshift/ocp-build-data/tree/dfb5c7d531490cfdc61a3b88bc533702b9624997/images/ose-olm-catalogd.yml UPSTREAM: <carry>: Update e2e registry to use 1.24/4.20 Update the e2e registry Dockerfile to use golang 1.24/OCP 4.20 Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: [Catalog Default Tests]: Upgrade go version to 1.24.3, dependencies and fix new lint issue UPSTREAM: <carry>: Add structure to allow move the orgin tests using OTE This commit introduces a binary and supporting structure to enable the execution of OpenShift origin (olmv1) tests using the Open Test Environment (OTE). It lays the groundwork for moving origin test in openshift/origin to be executed from this repository using OTE. UPSTREAM: <carry>: Add support for experimental manifests Update the openshift kustomize configuration for both operator-controller and catalogd. Update the manifest generation scripts to put the core generation code into a function (ignore-whitespace will help with the review), so that it can be called twice; once for standard, and once for experimental. Move around some of the kustomization directives to * Create a patch kustomization (Component) file and move the patch directives from olmv1-ns there. This allows it to be referenced from a different directory. * Add a kustomization file for tusted-ca. This allows it to be referenced from a different directory. * Move the setting of the namePrefix for operator-controller; this makes the generation compatible with upstream feature components. * Define experimental kustomization files that reference existing components. * Reference the correct CRDs (standard or experimental). * Add references to upstream feature components into the experimental manifests. This *will* add `--feature-gates` options from the upstream feature components to the experimental manifests. The cluster-olm-operator will strip those arguments from the deployments before adding the enabled feature gates. Update the Dockerfiles to include the experimental manifests and a copy script (`cp-manifests`) into the image containers. The complexity of having multiple sets of manifests mean that the simple initContainer copy mechanism found in cluster-olm-operator is no longer sufficient. This attempts to keep backwards compatibility with older versions of cluster-olm-operator, specifically by keeping the original (standard) manifests in the original location, and adding the experimental manifests in a new directory. The new `cp-manifests` script is used by newer versions of cluster-olm-operator. Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: [OTE] - chore: follow up openshift#383 – remove unreachable target call UPSTREAM: <carry>: Remove build of test image registry Upstream now uses a different image Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: Add test-experimental-e2e target to openshift Makefile This adds a test-experimental-e2e target to allow the CI to run the experimental e2e test. Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: [OTE]: Add binary in the operator controller image to allow proper integration with OCP tests UPSTREAM: <carry>: Fix experimental manifest copying The standard manifest was being copied rather than the experimental manifest. This meant that the expected feature-flags are not present. This is failing now that we are doing a check for those feature-flags. Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: Update manifest generation for upstream rbac/webhooks Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: [OTE] - Add tracking mechanism UPSTREAM: <carry>: Update OTE dep to get fix UPSTREAM: <carry>: [OTE] Add Readme UPSTREAM: <carry>: set GIT_COMMIT env from SOURCE_GIT_COMMIT in Dockerfiles for operator-controller and catalogd Signed-off-by: Rashmi Gottipati <chowdary.grashmi@gmail.com> UPSTREAM: <carry>: add openshift specific build target to pass commit info downstream Signed-off-by: Ankita Thomas <ankithom@redhat.com> UPSTREAM: <carry>: add source commit into binaries when linking - Removes extra GIT_COMMIT set - fixup Dockerfiles after rebase - consider "" unset so build-info can fill commit/date - double quote go flags & honor GIT_COMMIT if set - improve robustness of build-info parsing - Trim whitespace on all version fields - isUnset and valueOrUnknown now call strings.TrimSpace - Avoid clobbering values injected via ldflags - set repoState from build-info only when repoState is still unset - set version from build-info only when unset and build-info value is non-empty UPSTREAM: <carry>: OTE add first test from openshift/origin olmv1.go UPSTREAM: <carry>: Migrate tasks from openshift/origin olm v1.go file which are remaining This commit moves the final OLMv1 tests from openshift/origin/test/extended/olm/olmv1.go to their proper location in this repository. This migration is part of a larger effort to streamline development by co-locating tests with the component they validate. This will reduce CI overhead and allow for faster, more atomic changes. Assisted-by: Gemini UPSTREAM: <carry>: OTE - How to test locally with OCP instances UPSTREAM: <carry>: [OTE] Refac: refac helper and olmv1 test to create namespace instead to use pre-existent UPSTREAM: <carry>: [OTE] add webhook tests Migrates OLMv1 webhook operator tests from using external YAML files to defining resources in Go structs. This change removes file dependencies, improving test reliability and simplifying test setup. The migration is a refactoring of code from openshift/origin#30059. The new code uses better naming conventions and adapts the tests to work with a controller-runtime client, enhancing test consistency and maintainability. The migration covers all core test scenarios: - Validating, mutating, and conversion webhooks. - Certificate and secret rotation tolerance. Assisted-by: Gemini UPSTREAM: <carry>: OTE: rewrite the upgrade incompatible operator test This test replaces the existing upgrade incompatible test. The main change is that operator and catalog bundles are created on-the-fly to support OCP 4.20. This means we are no longer dependent on public operators for this test. This creates new bundles in the OCP ImageRegistry, this requires using a number of OCP APIs, including using a raw API URL to invoke the build. This is done by invoking an external k8s client (either `oc` or `kubectl`), and passing it a tarball of the bundle to be created. So, it can't be done by the golang k8sClient normally available (i.e. the create input is a tarball not a YAML file). This introduces the use of go-bindata to store the bundle contents. It also pulls in openshift mage, buld and operator APIs. Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: Handle service-ca cert availability/rotation There is problem when the service-ca certificate is not available at pod start. This is an issue because the SystemCertPool is created from SSL_CERT_DIR, which may include the empty service-ca. The SystemCertPool is never regenerated during the lifetime of the program execution, so it will never get updated when the service-ca is filled. Thus, we need to use --pull-cas-dir to reference the CAs that we want to use. This will also allow OLMv1 to reload the service-ca when it is reloaded (after 2 years, mind you). Removing the SSL_CERT_DIR setting, and adding the --pull-cas-dir flag ought to be equivalent to what we have now (i.e. SSL_CERT_DIR and no --pull-cas-dir), except that rotation will be handled better. Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: [OTE] add webhook tests Revert "UPSTREAM: <carry>: [OTE] add webhook tests" This reverts commit 9963614. UPSTREAM: <carry>: Upgrade OCP Catalog images from 4.19 to 4.20 UPSTREAM: <carry>: Remove bindata generation from build Using go-bindata is causing problems with ART builds. This removes the use of go-bindata from the builds. This will subsequently require that users MANUALLY run the `bindata` target to refresh the bindata, or use the `build-update` target. This is a quickfix to put out the fire. Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: [OTE] Add webhook tests - Add dumping of container logs and `kubectl describe pods` output for better diagnostics. - Include targeted certificate details dump (`tls.crt` parse) when failures occur. - Add additional check to verify webhook responsiveness after certificate rotation. This change is a refactor of code from openshift/origin#30059. Assisted-by: Gemini UPSTREAM: <carry>: OTE add logs and dumps for olmv1 test and fix helper for clusterextensions UPSTREAM: <carry>: [OTE] Migrate preflight checks from openshift/origin Migrated OLMv1 operator preflight checks from using external YAML files to defining ClusterRole permissions directly in Go structs. This improves test reliability and simplifies test setup by removing file dependencies. The changes ensure precise replication of original test scenarios, including specific permission omissions for services, create verbs, ClusterRoleBindings, ConfigMap resourceNames, and escalate/bind verbs. Assisted-by: Gemini UPSTREAM: <carry>: [OTE] Add webhook to validate openshift-service-ca certificate rotation This change is a refactor of code from openshift/origin#30059. Assisted-by: Gemini UPSTREAM: <carry>: Adds ResourceVersion checks to the tls secret deletion test, mirroring the logic used in the certificate rotation test. This makes the test more robust by ensuring a new secret is created, not just that an existing one is still present. UPSTREAM: <carry>: [OTE] - Readme:Add info to help use payload-aggregate with new tests UPSTREAM: <carry>: remove obsolete owners Signed-off-by: grokspawn <jordan@nimblewidget.com> UPSTREAM: <carry>: [OTE] add catalog tests from openshift/origin This commit migrates the olmv1_catalog set of tests from openshift/origin to OTE as part the broad effort to migrate all tests. Assisted-by: Gemini UPSTREAM: <carry>: Migrate single/own namespace tests This commit migrates the OLMv1 single and own namespace watch mode tests from openshift/origin/test/extended/olm/olmv1-singleownnamespace.go to this repository. This is part of the effort to move component-specific tests into their respective downstream locations. Assisted-by: Gemini UPSTREAM: <carry>: Adds ResourceVersion checks to the tls secret deletion test, mirroring the logic used in the certificate rotation test. This makes the test more robust by ensuring a new secret is created, not just that an existing one is still present. This reverts commit 0bb1953. UPSTREAM: <carry>: [OTE] Add webhook to validate openshift-service-ca certificate rotation This reverts commit e9e3220. UPSTREAM: <carry>: Ensure unique name for bad-catalog tests UPSTREAM: <carry>: Revert "Handle service-ca cert availability/rotation" This reverts commit 9cc13d8. UPSTREAM: <carry>: grant QE approver permission for OTE UPSTREAM: <carry>: Update webhook ote tests to use latest webhook-operator Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com> UPSTREAM: <carry>: update operator-controller to v1.5.1 UPSTREAM: <carry>: configure watchnamespace using spec.config for OTE tests UPSTREAM: <carry>: add jiazha to approvers UPSTREAM: <carry>: Create combined manifests for comparison Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: Use Helm charts for openshift manifests Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: add support for tests-private cases and add the case UPSTREAM: <carry>: Fix cp-manifests copying of helm charts The method used to copy the helm charts is including an extra `helm` directory in the destination path, that is making the cluster-olm-operator code just a bit more complicated than it needs to be. This fixes the copy location. Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: Remove kustomize manifests from images and repo Now that helm manifests are being used to dynamically generate the manifests, the pre-generated manifests are no longer needed. So, we can remove them from the repo and the images. However, because we still want to verify the manifests are "good", we are still creating a "single-file" version of the manifests for verification purposes, and to allow us to see what changes are happening to the manifests (from upstream and/or downstream sources). Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: Add pedjak and trgeiger as reviewers UPSTREAM: <carry>: migrate more cases from tests-private and enhance suites with filters UPSTREAM: <carry>: Updating ose-olm-operator-controller-container image to be consistent with ART for 4.21 Reconciling with https://github.com/openshift/ocp-build-data/tree/4fbe3fab45239dc4be6f5d9d98a0bf36e0274ec9/images/ose-olm-operator-controller.yml UPSTREAM: <carry>: Updating ose-olm-catalogd-container image to be consistent with ART for 4.21 Reconciling with https://github.com/openshift/ocp-build-data/tree/4fbe3fab45239dc4be6f5d9d98a0bf36e0274ec9/images/ose-olm-catalogd.yml UPSTREAM: <carry>: OTE: Enable disconnected environment and build test operator controller image Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com> UPSTREAM: <carry>: for incompatible test add func to wait builder and deployer SA creation by OCP controller UPSTREAM: <carry>: Fix VERSION replacement in catalog bindata Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: check kubeconfig only run-test and run-suite UPSTREAM: <carry>: Clean up cp-manifests There is no longer a need to copy conditionally Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: Update does-not-exist and simple install to work in a disconnected environment Signed-off-by: Todd Short <todd.short@me.com> UPSTREAM: <carry>: support webhook case in disconnected UPSTREAM: <carry>: Consolidate build API This consolidates the in-cluster building of a bundle and catalog. The catalog and bundle bindata are inputs, along with a set of replacements so that catalog and bundle templates can be used to create the images. This can be done in the BeforeEach() for a set of tests that use the same data. Signed-off-by: Todd Short <todd.short@me.com>
…images from openshift/catalogd/manifests.yaml
Signed-off-by: Todd Short <todd.short@me.com>
…oss to avoid flakes
…to run outside of OCP
…ffinity for HA topology Rolling updates in HighlyAvailable clusters leave catalogd and operator-controller unavailable when the only running pod is evicted before its replacement is ready. Fix by defaulting replicas=1 and PDB disabled in the static Helm values (safe for SNO/External topologies, passes the SNO conformance test that asserts exactly one replica in SingleReplica topology mode). Add pod anti-affinity to prefer scheduling replicas on different nodes. cluster-olm-operator detects the cluster's ControlPlaneTopology at startup and overrides these values to replicas=2 and PDB enabled when a HighlyAvailable topology is detected, then re-renders the manifests before starting controllers. When a topology change is observed at runtime (exceedingly rare), the operator exits so its deployment controller restarts it, triggering a fresh Helm render with the correct values for the new topology. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Todd Short <tshort@redhat.com>
…etween both-watch-modes scenarios The both-watch-modes test loops over two scenarios (singlens, ownns) inside a single It block and was blocking on full namespace deletion between them. This caused flaky 300s timeouts on GCP techpreview clusters where master nodes run at 94-99% CPU, which starves the namespace controller and makes namespace termination arbitrarily slow. The wait was not guarding anything real: - EnsureCleanupClusterExtension already ensures the CE and CRD are gone; since CE deletion uses ForegroundPropagation, the ClusterObjectSet teardown must complete before the CE disappears, meaning all managed resources (Deployments, Services, etc.) are already deleted at that point. - The singleown bundle installs no ValidatingWebhookConfiguration or MutatingWebhookConfiguration, so there is no webhook admission risk. - Each scenario generates unique namespace names and CRD group suffixes via rand.String(4), so a terminating namespace from scenario 1 cannot collide with or interfere with scenario 2's resources. Trigger both namespace deletions and proceed without waiting. The DeferCleanup registrations that already exist will handle any residual cleanup after the spec exits. Fixes: OCPBUGS-84943 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Todd Short <tshort@redhat.com>
- Replace broken test-experimental-e2e target (test/experimental-e2e no longer exists) with /bin/true so triggered jobs always succeed - Pass -timeout=60m to go test; the previous invocation relied on Go's 10m default which is too short for BoxcutterRuntime clusters - Set E2E_STEP_TIMEOUT=15m; BoxcutterRuntime applies resources through sequential phases (CRD must reach Established before the deploy phase starts), making installations slower than the upstream 5m default - Skip ~@CatalogdHA scenarios (require multiple catalogd replicas not present in standard topology) - Skip ~@ProgressDeadline scenarios (require progressDeadlineMinutes < 10 but the OpenShift CRD enforces a minimum of 10) - Skip ~@httpproxy scenarios (too disruptive to cluster networking) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Todd Short <tshort@redhat.com>
The e2e-test-registry image is no longer built by CI after openshift/release#78581 removed it from the CI config. The dynamic per-scenario catalog system replaced the pre-built registry image, making this Dockerfile dead code.
It's no longer bring used. Signed-off-by: Todd Short <tshort@redhat.com>
Adds a new test that verifies cluster-olm-operator correctly configures operator-controller and catalogd deployments based on the cluster's control plane topology: - HA topologies (HighlyAvailable, HighlyAvailableArbiter, DualReplica): replicas=2 with a PodDisruptionBudget present - Non-HA topologies (SingleReplica/SNO, External): replicas=1, no PDB Also registers policyv1 in the test scheme to support PDB list queries. Assisted-by: claude Signed-off-by: Todd Short <tshort@redhat.com>
… builders Signed-off-by: Todd Short <tshort@redhat.com>
…ge to be consistent with ART for 5.0 Reconciling with https://github.com/openshift-eng/ocp-build-data/tree/7691ed4dc0b6585b358f9e73fb736ace9a48a286/images/ose-olm-operator-controller.yml
Set catalog image tags to v5.0 for the 4.23/5.0 release. Dynamically discover an installable package from the serving catalogs instead of hardcoding quay-operator v3.13.10, preferring quay-operator, cluster-logging, serverless-operator, logic-operator in that order then alling back to the first available package. Signed-off-by: Todd Short <tshort@redhat.com>
…ntal manifests HelmChartSupport was removed upstream in dbc9b4a but the downstream experimental.yaml values file and its generated manifest still referenced it, causing operator-controller to crash on startup with: invalid argument "HelmChartSupport=false" for "--feature-gates" flag: unrecognized feature gate: HelmChartSupport This made the OLM cluster operator Degraded/Unavailable and caused cluster installation to time out (exit code 6). Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…cluster version Add a second ReleaseGate-eligible OTE test verifying that an operator whose olm.maxOpenShiftVersion exceeds the current cluster version does not block cluster upgrade (InstalledOLMOperatorsUpgradeable stays True). The existing test only covered the blocking path (maxOCPVersion == current version → False). This covers the complementary allow path (maxOCPVersion == next minor → True), directly exercising the normalization logic introduced for the 4.23/5.0 co-release boundary. A nextMinorVersion() helper mirrors the 4.23→5.1 special case so the bundle annotation is always set to the correct next upgrade target. Run 'make build-update' to register the new allow-case test in the extension metadata after adding it to olmv1-incompatible.go. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Todd Short <tshort@redhat.com>
Automate the ClusterExtension rollout failure coverage for OCP-88331 and OCP-88332 by building in-cluster bundle and catalog images for successful and failing bundle versions. The new QE specs verify ProgressDeadlineExceeded on an initial failed rollout and ProbeFailure while upgrading to a bad revision under the BoxCutter runtime. Signed-off-by: Daniel Franz <dfranz@redhat.com> Co-authored-by: Bruno Andrade <bruno.balint@gmail.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Todd Short <tshort@redhat.com>
…eAccount usage in ClusterExtension API, Synthetic Permissions, and PreFlight Admissions Signed-off-by: Daniel Franz <dfranz@redhat.com>
…grade boundary Fix GetNextMinorVersion to return "5.1" for 4.23 clusters instead of "4.24": OCP 4.23 and 5.0 are co-released equivalents whose only upgrade target is 5.1. Remove the redundant `&& strings.Contains(message, "5")` guard from the Upgradeable message poll — the expectedPattern built from GetNextMinorVersion now encodes the full version string and is sufficient on its own. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
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. |
|
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
| 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
| 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
| 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
|
[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 |
|
@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. |
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
Chores