Skip to content

fix(ci): Revert "ROX-27677: Eliminate Python dependency...#19860

Open
porridge wants to merge 1 commit intomasterfrom
konflux-porridge-revert-go-bundle-helpers
Open

fix(ci): Revert "ROX-27677: Eliminate Python dependency...#19860
porridge wants to merge 1 commit intomasterfrom
konflux-porridge-revert-go-bundle-helpers

Conversation

@porridge
Copy link
Copy Markdown
Contributor

@porridge porridge commented Apr 7, 2026

This reverts commit 15ab6e7 from PR #18947

Description

Does not work on konflux:

build log output
[1/2] STEP 3/48: RUN . /cachi2/cachi2.env &&     dnf -y install --allowerasing python3.12-pyyaml &&     alternatives --install /usr/bin/python3 python3 /usr/bin/python3.12 1
  Updating Subscription Management repositories.
  Unable to read consumer identity
  
  This system is not registered with an entitlement server. You can use subscription-manager to register.
  
  Repository 'rhel-9-for-x86_64-appstream-rpms' is missing name in configuration, using id.
  Repository 'rhel-9-for-x86_64-baseos-rpms' is missing name in configuration, using id.
  Repository 'rhel-9-for-x86_64-appstream-source-rpms' is missing name in configuration, using id.
  Repository 'rhel-9-for-x86_64-baseos-source-rpms' is missing name in configuration, using id.
  rhel-9-for-x86_64-appstream-rpms                 38 MB/s |  47 kB     00:00    
  rhel-9-for-x86_64-baseos-rpms                   130 MB/s | 133 kB     00:00    
  rhel-9-for-x86_64-appstream-source-rpms         5.3 MB/s | 5.4 kB     00:00    
  rhel-9-for-x86_64-baseos-source-rpms             40 MB/s |  41 kB     00:00    
  Dependencies resolved.
  ==============================================================================================
   Package                    Arch    Version            Repository                         Size
  ==============================================================================================
  Installing:
   python3.12-pyyaml          x86_64  6.0.1-2.el9        rhel-9-for-x86_64-appstream-rpms  226 k
  Upgrading:
   openssl                    x86_64  1:3.5.1-7.el9_7    rhel-9-for-x86_64-baseos-rpms     1.5 M
   openssl-fips-provider      x86_64  3.0.7-8.el9        rhel-9-for-x86_64-baseos-rpms     9.2 k
   openssl-libs               x86_64  1:3.5.1-7.el9_7    rhel-9-for-x86_64-baseos-rpms     2.3 M
  Installing dependencies:
   libnsl2                    x86_64  2.0.0-1.el9        rhel-9-for-x86_64-appstream-rpms   33 k
   mpdecimal                  x86_64  2.5.1-3.el9        rhel-9-for-x86_64-appstream-rpms   88 k
   openssl-fips-provider-so   x86_64  3.0.7-8.el9        rhel-9-for-x86_64-baseos-rpms     576 k
   python3.12                 x86_64  3.12.12-4.el9_7.1  rhel-9-for-x86_64-appstream-rpms   32 k
   python3.12-libs            x86_64  3.12.12-4.el9_7.1  rhel-9-for-x86_64-appstream-rpms  9.7 M
   python3.12-pip-wheel       noarch  23.2.1-5.el9       rhel-9-for-x86_64-appstream-rpms  1.5 M
  Removing dependent packages:
   golang                     x86_64  1.25.7-1.el9_7     @rhel-97-golang-rpms-x86_64       9.6 M
   golang-bin                 x86_64  1.25.7-1.el9_7     @rhel-97-golang-rpms-x86_64        88 M
   golang-race                x86_64  1.25.7-1.el9_7     @rhel-97-golang-rpms-x86_64        13 M
   openssl-devel              x86_64  1:3.0.7-29.el9_4.2 @rhel-9-appstream-rpms-x86_64     4.7 M
[...]
  Removed:
    golang-1.25.7-1.el9_7.x86_64        golang-bin-1.25.7-1.el9_7.x86_64         
    golang-race-1.25.7-1.el9_7.x86_64   openssl-devel-1:3.0.7-29.el9_4.2.x86_64  
  
  Complete!
  [1/2] STEP 4/48: COPY . /stackrox
  [1/2] STEP 5/48: RUN . /cachi2/cachi2.env &&     which go || true
  which: no go in (/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin)

Slack thread - including results of failed quick fix attempts.

User-facing documentation

Testing and quality

  • the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
  • CI results are inspected

Automated testing

  • added unit tests
  • added e2e tests
  • added regression tests
  • added compatibility tests
  • modified existing tests

How I validated my change

CI is enough.

@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Apr 7, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

/konflux-retest central-db-on-push

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

/konflux-retest central-db-on-push

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 2 issues, and left some high level feedback:

  • The Python version references are inconsistent: the Makefile comment mentions Python 3.9 while the Konflux Dockerfile installs Python 3.12 and the .python-version file is referenced as the source of truth; please align the comments and tooling so they clearly reflect and enforce a single required Python version.
  • In ACTIVATE_PYTHON you explicitly pin pip/setuptools to versions noted as incompatible with Python 3.12 in the preceding comment; if Konflux is using Python 3.12, consider either relaxing these pins or updating them to versions known to work with 3.12 to avoid build-time import errors.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The Python version references are inconsistent: the Makefile comment mentions Python 3.9 while the Konflux Dockerfile installs Python 3.12 and the .python-version file is referenced as the source of truth; please align the comments and tooling so they clearly reflect and enforce a single required Python version.
- In ACTIVATE_PYTHON you explicitly pin pip/setuptools to versions noted as incompatible with Python 3.12 in the preceding comment; if Konflux is using Python 3.12, consider either relaxing these pins or updating them to versions known to work with 3.12 to avoid build-time import errors.

## Individual Comments

### Comment 1
<location path="operator/Makefile" line_range="15-19" />
<code_context>

-# Python binary to use for bundle helpers. Can be overridden to use a specific version (e.g., python3.12).
-# The Konflux build uses Python 3.12, so using python3.12 locally ensures compatibility.
+# Python binary to use for bundle helpers. Can be overridden to use a specific version (e.g., python3.9).
+# The Konflux build uses Python 3.9, so using python3.9 locally ensures compatibility.
 PYTHON ?= python3
</code_context>
<issue_to_address>
**issue (bug_risk):** Pinned pip/setuptools versions are incompatible with Python 3.12 but the Konflux stack still uses 3.12, which is likely to break venv setup.

Konflux’s Dockerfile still installs and defaults to python3.12, while `ACTIVATE_PYTHON` now pins `pip==21.3.1` and `setuptools==59.6.0`, which you’ve noted don’t support Python 3.12 (`pkgutil.ImpImporter` removal). In that environment, `pip install` is likely to fail in `bundle-post-process` or `test-bundle-helpers`. Please either: (1) align the base image Python version with `.python-version` (e.g., 3.9), or (2) choose pip/setuptools versions compatible with Python 3.12, so the configuration is consistent and doesn’t break at runtime.
</issue_to_address>

### Comment 2
<location path="operator/konflux.bundle.Dockerfile" line_range="3-5" />
<code_context>
-    alternatives --install /usr/bin/python3 python3 /usr/bin/python3.12 1
+FROM registry.access.redhat.com/ubi9/ubi-minimal:latest@sha256:69f5c9886ecb19b23e88275a5cd904c47dd982dfa370fbbd0c356d7b1047ef68 AS builder
+
+# This installs both PyYAML and Python.
+# The alternatives command ensures that the python3 command points to the installed Python 3.12, which is required by the bundle generation scripts.
+RUN microdnf -y install python3.12-pyyaml && alternatives --install /usr/bin/python3 python3 /usr/bin/python3.12 1

 COPY . /stackrox
</code_context>
<issue_to_address>
**issue (bug_risk):** Konflux image forces Python 3.12 while the Makefile tooling now assumes an older Python for pinned pip/setuptools versions.

This container forces `/usr/bin/python3` to be 3.12, but the Makefile’s `ACTIVATE_PYTHON` target pins `pip==21.3.1` and `setuptools==59.6.0`, which depend on `pkgutil.ImpImporter` removed in Python 3.12. That mismatch can break `bundle-post-process` and `test-bundle-helpers`. Please either align the image with the Python version in `bundle_helpers/.python-version` (and point `python3` there) or update the pinned pip/setuptools versions to ones compatible with 3.12.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 7, 2026

📝 Walkthrough

Walkthrough

This diff removes the Go-based bundle helper implementation entirely, consolidating around Python-only bundle manifest post-processing. Changes include deleting Go source and test files, wrapper/comparison scripts, supporting documentation, and adjusting build infrastructure (Makefile, Dockerfile) to directly invoke Python scripts and use a Python-capable base image instead of a Go builder.

Changes

Cohort / File(s) Summary
Go Implementation Removal
operator/bundle_helpers/cmd/*, operator/bundle_helpers/pkg/csv/*, operator/bundle_helpers/pkg/descriptor/*, operator/bundle_helpers/pkg/rewrite/*, operator/bundle_helpers/pkg/values/*, operator/bundle_helpers/main.go
Entire Go command implementations, CSV patching/versioning logic, descriptor sorting, string rewriting utilities, and Helm values helpers removed. Includes all supporting unit tests.
Wrapper & Comparison Scripts
operator/bundle_helpers/dispatch.sh, operator/bundle_helpers/compare-implementations.sh
Removed dispatch wrapper that routed between Go/Python implementations and script comparing outputs between the two implementations.
Documentation & Dependencies
operator/bundle_helpers/README.md, operator/bundle_helpers/requirements.txt, operator/bundle_helpers/yaml-normalizer.py
Deleted architecture/usage documentation for Go helpers; updated PyYAML from 6.0.1 to 6.0; removed YAML normalization script.
Build Configuration
operator/Makefile, .tekton/operator-bundle-build.yaml, operator/konflux.bundle.Dockerfile
Makefile now hardcodes Python-only execution (removed RELATED_IMAGES_MODE variable, removed conditional skip on test-bundle-helpers, replaced dispatch.sh invocations with direct Python scripts); removed gomod prefetch from Tekton pipeline; switched Docker builder image from Go builder to ubi9-minimal with Python 3.12 support.
Script Updates
operator/bundle_helpers/prepare-bundle-manifests.sh
Now invokes patch-csv.py directly instead of through the dispatch wrapper.
CI/CD
.github/workflows/test-bundle-helpers.yaml
Removed entire GitHub Actions workflow that tested and compared Python vs Go bundle helper implementations.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The PR description explains the revert reason and includes build logs, but the validation section contains an incomplete placeholder. Complete the 'How I validated my change' section with specific validation steps and results, and mark CI inspection checklist item as verified.
✅ Passed checks (2 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly indicates this PR reverts a previous change (ROX-27677) that eliminated Python dependency, accurately reflecting the main objective of the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch konflux-porridge-revert-go-bundle-helpers

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@operator/konflux.bundle.Dockerfile`:
- Around line 1-5: The Dockerfile installs python3.12-pyyaml via microdnf (RUN
microdnf -y install python3.12-pyyaml && alternatives --install ...) which can
produce a different PyYAML version than the pinned PyYAML==6.0 used in local
builds; update the builder stage to ensure PyYAML is version-pinned (either
install system package with the exact 6.0 RPM if available, or remove the system
PyYAML and instead install Python and pip then pip install -r requirements.txt /
pip install PyYAML==6.0) and make the Python major.minor consistent with
.python-version (either use Python 3.9 in the Dockerfile or document/standardize
on 3.12) so builds use the same YAML library and Python runtime across
environments.

In `@operator/Makefile`:
- Around line 15-17: Update the Makefile comment and optionally the default to
match the Dockerfile: change the comment that currently says "The Konflux build
uses Python 3.9" to state the project uses Python 3.12 (to match
konflux.bundle.Dockerfile), and consider setting the PYTHON variable default to
python3.12 (replace PYTHON ?= python3) so local builds mirror the container
setup; update the comment near the PYTHON variable and the PYTHON assignment
accordingly.
🪄 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: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 726ac1c1-16b8-4490-ba94-d0088fb28ced

📥 Commits

Reviewing files that changed from the base of the PR and between b2a85fc and 156f64e.

📒 Files selected for processing (24)
  • .github/workflows/test-bundle-helpers.yaml
  • .tekton/operator-bundle-build.yaml
  • operator/Makefile
  • operator/bundle_helpers/README.md
  • operator/bundle_helpers/cmd/fix_descriptors.go
  • operator/bundle_helpers/cmd/output.go
  • operator/bundle_helpers/cmd/patch_csv.go
  • operator/bundle_helpers/compare-implementations.sh
  • operator/bundle_helpers/dispatch.sh
  • operator/bundle_helpers/main.go
  • operator/bundle_helpers/pkg/csv/patcher.go
  • operator/bundle_helpers/pkg/csv/patcher_test.go
  • operator/bundle_helpers/pkg/csv/version.go
  • operator/bundle_helpers/pkg/csv/version_test.go
  • operator/bundle_helpers/pkg/descriptor/sorter.go
  • operator/bundle_helpers/pkg/descriptor/sorter_test.go
  • operator/bundle_helpers/pkg/rewrite/rewriter.go
  • operator/bundle_helpers/pkg/rewrite/rewriter_test.go
  • operator/bundle_helpers/pkg/values/values.go
  • operator/bundle_helpers/pkg/values/values_test.go
  • operator/bundle_helpers/prepare-bundle-manifests.sh
  • operator/bundle_helpers/requirements.txt
  • operator/bundle_helpers/yaml-normalizer.py
  • operator/konflux.bundle.Dockerfile
💤 Files with no reviewable changes (19)
  • operator/bundle_helpers/yaml-normalizer.py
  • operator/bundle_helpers/pkg/rewrite/rewriter.go
  • operator/bundle_helpers/pkg/rewrite/rewriter_test.go
  • operator/bundle_helpers/pkg/values/values_test.go
  • operator/bundle_helpers/pkg/csv/patcher_test.go
  • operator/bundle_helpers/README.md
  • operator/bundle_helpers/main.go
  • operator/bundle_helpers/dispatch.sh
  • operator/bundle_helpers/compare-implementations.sh
  • operator/bundle_helpers/cmd/fix_descriptors.go
  • operator/bundle_helpers/cmd/output.go
  • operator/bundle_helpers/pkg/descriptor/sorter_test.go
  • operator/bundle_helpers/pkg/csv/patcher.go
  • operator/bundle_helpers/pkg/csv/version_test.go
  • operator/bundle_helpers/pkg/csv/version.go
  • operator/bundle_helpers/cmd/patch_csv.go
  • operator/bundle_helpers/pkg/descriptor/sorter.go
  • .github/workflows/test-bundle-helpers.yaml
  • operator/bundle_helpers/pkg/values/values.go

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.55%. Comparing base (7678bbc) to head (156f64e).
⚠️ Report is 23 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #19860      +/-   ##
==========================================
- Coverage   49.60%   49.55%   -0.05%     
==========================================
  Files        2763     2758       -5     
  Lines      208339   207855     -484     
==========================================
- Hits       103342   103006     -336     
+ Misses      97332    97222     -110     
+ Partials     7665     7627      -38     
Flag Coverage Δ
go-unit-tests 49.55% <ø> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

🚀 Build Images Ready

Images are ready for commit 156f64e. To use with deploy scripts:

export MAIN_IMAGE_TAG=4.11.x-570-g156f64e5e4

@porridge porridge changed the title Revert "ROX-27677: Eliminate Python dependency from operator build" fix(ci): Revert "ROX-27677: Eliminate Python dependency from operator build" Apr 7, 2026
@porridge porridge changed the title fix(ci): Revert "ROX-27677: Eliminate Python dependency from operator build" fix(ci): Revert "ROX-27677: Eliminate Python dependency... Apr 7, 2026
@porridge
Copy link
Copy Markdown
Contributor Author

porridge commented Apr 8, 2026

/retest operator-bundle-on-push

@openshift-ci

This comment was marked as outdated.

@porridge
Copy link
Copy Markdown
Contributor Author

porridge commented Apr 8, 2026

/konflux-retest operator-bundle-on-push

2 similar comments
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 8, 2026

/konflux-retest operator-bundle-on-push

@porridge
Copy link
Copy Markdown
Contributor Author

porridge commented Apr 8, 2026

/konflux-retest operator-bundle-on-push

@porridge porridge marked this pull request as ready for review April 8, 2026 06:25
@porridge porridge requested review from a team and rhacs-bot as code owners April 8, 2026 06:25
@porridge porridge removed the request for review from a team April 8, 2026 06:25
@porridge porridge requested a review from vladbologa April 8, 2026 06:25
@rhacs-bot rhacs-bot requested a review from a team April 8, 2026 06:26
@mclasmeier
Copy link
Copy Markdown
Contributor

😢

@porridge porridge added auto-merge Auto-merge minor and patch version bumps auto-retest PRs with this label will be automatically retested if prow checks fails labels Apr 8, 2026
Copy link
Copy Markdown
Contributor

@msugakov msugakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review area/ci area/operator auto-merge Auto-merge minor and patch version bumps auto-retest PRs with this label will be automatically retested if prow checks fails coderabbit-review konflux-build Run Konflux in PR. Push commit to trigger it.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants