Update golang builder CVE component name to openshift4/openshift-golang-builder#3043
Update golang builder CVE component name to openshift4/openshift-golang-builder#3043ashwindasr wants to merge 6 commits into
Conversation
…ng-builder The JIRA pscomponent for golang builder security trackers is changing from openshift-golang-builder-container to openshift4/openshift-golang-builder. Introduce GOLANG_BUILDER_BREW_COMPONENT to preserve the Brew/Konflux package name (openshift-golang-builder-container) which remains unchanged, and update GOLANG_BUILDER_CVE_COMPONENT to the new JIRA pscomponent value. All JIRA bug matching uses the CVE constant; all Brew/Konflux NVR and package lookups use the Brew constant. Filtering logic in attach_cve_flaws_cli and find_bugs_sweep_cli is adjusted to handle the new openshift4/ prefix correctly. Co-authored-by: Cursor <cursoragent@cursor.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughSplit golang-builder identifiers into distinct CVE vs Brew names, add is_golang_builder helper, and update detection, mapping, parent-image parsing, errata NVR validation, pyartcd builder discovery, CLI filtering/help, bzutil normalization, and tests to handle both identifiers and delivery-repo normalization/fallbacks. ChangesGolang builder component naming refactor
🎯 3 (Moderate) | ⏱️ ~25 minutes ✨ 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 `@elliott/elliottlib/cli/attach_cve_flaws_cli.py`:
- Around line 540-542: Normalize legacy golang-builder tracker names before
branch-specific logic by treating both constants.GOLANG_BUILDER_CVE_COMPONENT
and constants.GOLANG_BUILDER_BREW_COMPONENT as the same golang-builder tracker:
change the is_golang_builder check to something like `is_golang_builder =
whiteboard_component in {constants.GOLANG_BUILDER_CVE_COMPONENT,
constants.GOLANG_BUILDER_BREW_COMPONENT}` and use that variable in the
subsequent `if is_ocp_delivery_repo(whiteboard_component) and not
is_golang_builder` path (and the analogous block around the other occurrence),
so legacy Brew-named trackers are normalized up front and follow the same
mapping/exclusion logic.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: dc34c9a4-6355-405f-95e3-db99421769c6
📒 Files selected for processing (10)
elliott/elliottlib/cli/attach_cve_flaws_cli.pyelliott/elliottlib/cli/find_bugs_golang_cli.pyelliott/elliottlib/cli/find_bugs_sweep_cli.pyelliott/elliottlib/constants.pyelliott/elliottlib/errata_async.pyelliott/elliottlib/util.pyelliott/tests/test_bzutil.pyelliott/tests/test_find_bugs_golang_cli.pypyartcd/pyartcd/pipelines/update_golang.pypyartcd/tests/pipelines/test_update_golang.py
Add is_golang_builder_component() helper to accept both old (openshift-golang-builder-container) and new (openshift4/openshift-golang-builder) pscomponent names during migration. Replace 8 equality checks across find_bugs_golang, attach_cve_flaws, and find_bugs_sweep CLIs with the helper. Fix is_first_fix_for_tracker/is_first_fix_any to normalize the tracker whiteboard_component via normalize_component_by_ocp_delivery_repo before comparing against unfixed_components from Hydra. Add fallback in get_cve_unfixed_components to preserve the raw delivery-repo name when translation fails, so trackers using that pscomponent still match. Co-authored-by: Cursor <cursoragent@cursor.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
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 `@elliott/elliottlib/cli/find_bugs_golang_cli.py`:
- Around line 489-491: The current check only skips non-builder components that
end with "-container" (the comp.endswith("-container") branch using
constants.is_golang_builder_component), but it must also skip delivery-repo
image trackers like "openshift4/<image>"; update the condition that logs and
returns False (the block using logger.info and return False) to also treat
components that start with "openshift4/" as non-builder image trackers (i.e.,
short-circuit and skip them), preserving the existing
is_golang_builder_component(comp) logic for "-container" names so golang
builders still pass validation.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 29d89ce5-3fcb-4502-a434-02e3ff070595
📒 Files selected for processing (6)
elliott/elliottlib/bzutil.pyelliott/elliottlib/cli/attach_cve_flaws_cli.pyelliott/elliottlib/cli/find_bugs_golang_cli.pyelliott/elliottlib/cli/find_bugs_sweep_cli.pyelliott/elliottlib/constants.pyelliott/tests/test_bzutil.py
🚧 Files skipped from review as they are similar to previous changes (2)
- elliott/elliottlib/constants.py
- elliott/elliottlib/cli/attach_cve_flaws_cli.py
Co-authored-by: Cursor <cursoragent@cursor.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
The is_valid filter only skipped image trackers ending with -container, but with the transition to delivery-repo pscomponents, non-builder image trackers like openshift4/<image> would slip through. Add is_ocp_delivery_repo check to also skip those, preserving the golang builder exemption. Co-authored-by: Cursor <cursoragent@cursor.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
| # Golang builder needs special treatment when associating security tracking bugs with builds: | ||
| GOLANG_BUILDER_CVE_COMPONENT = 'openshift-golang-builder-container' | ||
| GOLANG_BUILDER_CVE_COMPONENT = 'openshift4/openshift-golang-builder' | ||
| GOLANG_BUILDER_BREW_COMPONENT = 'openshift-golang-builder-container' |
There was a problem hiding this comment.
i would prefer if we don't use the word brew here
since we compute component names for konflux the same way
Maybe just golang_builder_component ?
And openshift4/openshift-golang-builder can be the golang_builder_ocp4_delivery_repo?
there will be an ocp5 one also, have a func wrap this is_golang_builder_delivery_repo ?
There was a problem hiding this comment.
thanks - that makes sense. added in 80b30a9
Rename GOLANG_BUILDER_BREW_COMPONENT to GOLANG_BUILDER_COMPONENT since Konflux computes component names the same way as Brew. Rename GOLANG_BUILDER_CVE_COMPONENT to GOLANG_BUILDER_OCP4_DELIVERY_REPO to clarify it is the OCP4 delivery repo name. Add is_golang_builder_delivery_repo() for OCP4/OCP5 forward-compatibility. Co-authored-by: Cursor <cursoragent@cursor.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
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 `@elliott/elliottlib/errata_async.py`:
- Around line 383-392: In compute_cve_exclusions, the golang CVE detection
currently checks only for constants.GOLANG_BUILDER_OCP4_DELIVERY_REPO and misses
legacy builder names; change the loop over expected_cve_components (variables
expected_brew_components, golang_cve_names) to treat a CVE as golang if any
component in components satisfies
constants.is_golang_builder_component(component) (i.e., use that helper instead
of the single-constant membership test), so golang CVEs get routed through
populate_golang_cve_components and the builder component is not incorrectly
required as a brew component.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: f049f706-4e09-4120-969a-854484fb65a5
📒 Files selected for processing (10)
elliott/elliottlib/cli/find_bugs_golang_cli.pyelliott/elliottlib/cli/find_bugs_sweep_cli.pyelliott/elliottlib/constants.pyelliott/elliottlib/errata_async.pyelliott/elliottlib/util.pyelliott/tests/test_bzutil.pyelliott/tests/test_errata_async.pyelliott/tests/test_find_bugs_golang_cli.pyelliott/tests/test_find_bugs_sweep_cli.pypyartcd/pyartcd/pipelines/update_golang.py
✅ Files skipped from review due to trivial changes (1)
- elliott/tests/test_find_bugs_sweep_cli.py
🚧 Files skipped from review as they are similar to previous changes (4)
- elliott/elliottlib/cli/find_bugs_sweep_cli.py
- elliott/elliottlib/util.py
- elliott/tests/test_bzutil.py
- elliott/elliottlib/cli/find_bugs_golang_cli.py
The golang CVE detection was checking for a single constant, missing legacy builder names during migration. Use the helper to match any form of the golang builder component. Co-authored-by: Cursor <cursoragent@cursor.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
|
@ashwindasr: The following test 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. |
|
/hold Might need to confirm with with prodsec |
|
/unhold |
| # Golang builder needs special treatment when associating security tracking bugs with builds: | ||
| GOLANG_BUILDER_CVE_COMPONENT = 'openshift-golang-builder-container' | ||
| GOLANG_BUILDER_COMPONENT = 'openshift-golang-builder-container' | ||
| GOLANG_BUILDER_OCP4_DELIVERY_REPO = 'openshift4/openshift-golang-builder' |
There was a problem hiding this comment.
I don't know if we had discussion around handling openshift5/ here ? If not, then I'd recommend that
|
PR needs rebase. 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. |
Summary
GOLANG_BUILDER_CVE_COMPONENTfromopenshift-golang-builder-containertoopenshift4/openshift-golang-builderto match the new JIRA pscomponent name for golang builder security trackers.GOLANG_BUILDER_BREW_COMPONENT(openshift-golang-builder-container) to preserve the Brew/Konflux package name which remains unchanged.attach_cve_flaws_cliandfind_bugs_sweep_clito correctly handle the newopenshift4/prefix format.is_golang_builder_component()helper that accepts both old and new pscomponent names for backward compatibility during migration.is_first_fix_for_tracker/is_first_fix_anyto normalize trackerwhiteboard_componentbefore comparing against Hydra unfixed components, preventing mismatches when the tracker uses a delivery-repo-format pscomponent.get_cve_unfixed_componentsto preserve raw delivery-repo names when translation fails.is_valid()filter infind_bugs_golang_clito also skip delivery-repo-format image trackers (openshift4/...), not just-containersuffixed ones, while preserving the golang builder exemption.Test plan
is_golang_builder_component()helper (4 cases)is_first_fixnormalization and fallback (2 cases)ruff checkclean)update-golangpipeline correctly sweeps builder bugs with the new component nameMade with Cursor