Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions elliott/elliottlib/bzutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
chunk,
get_component_by_delivery_repo,
isolate_timestamp_in_release,
normalize_component_by_ocp_delivery_repo,
)

logger = logutil.get_logger(__name__)
Expand Down Expand Up @@ -1730,6 +1731,7 @@ def get_cve_unfixed_components(runtime, cve_alias: str) -> Dict:
logger.warning(
f"Could not find component name for {pkg_name}! is it an art component? is the delivery repo defined?"
)
unfixed_components.append(pkg_name)
else:
unfixed_components.append(comp_name)
else:
Expand All @@ -1751,7 +1753,8 @@ def is_first_fix_for_tracker(runtime, flaw_bug: BugzillaBug, tracker_bug: JIRABu
)
alias = flaw_bug.alias[0]
unfixed_components = get_cve_unfixed_components(runtime, alias)
first_fix = tracker_bug.whiteboard_component in unfixed_components
tracker_component = normalize_component_by_ocp_delivery_repo(runtime, tracker_bug.whiteboard_component)
Comment thread
thegreyd marked this conversation as resolved.
first_fix = tracker_component in unfixed_components
logger.info(
f"Flaw bug {flaw_bug.id} is {'first-fix' if first_fix else 'second-fix'} for tracker bug {tracker_bug.id}"
)
Expand Down Expand Up @@ -1781,7 +1784,8 @@ def is_first_fix_any(runtime, flaw_bug: BugzillaBug, tracker_bugs: Iterable[JIRA
# and if not then second-fix close operation should be performed on them
# but for now we will just return True if any of the trackers is a first fix
for tracker_bug in tracker_bugs:
if tracker_bug.whiteboard_component in unfixed_components:
tracker_component = normalize_component_by_ocp_delivery_repo(runtime, tracker_bug.whiteboard_component)
if tracker_component in unfixed_components:
logger.info(
f"Flaw bug {flaw_bug.id} is a first-fix for tracker bug {tracker_bug.id}. Other associated trackers of the flaw bug will not be checked."
)
Expand Down
9 changes: 6 additions & 3 deletions elliott/elliottlib/cli/attach_cve_flaws_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,8 @@ def get_cve_component_mapping(
raise ValueError(f"Bug {tracker.id} doesn't have a valid whiteboard component.")

whiteboard_component = tracker.whiteboard_component
if is_ocp_delivery_repo(whiteboard_component):
is_golang_builder = constants.is_golang_builder_component(whiteboard_component)
if is_ocp_delivery_repo(whiteboard_component) and not is_golang_builder:
# this means the component here is the delivery repo name
# we need to translate it to build component name
new_component = get_component_by_delivery_repo(runtime, whiteboard_component)
Expand All @@ -548,10 +549,12 @@ def get_cve_component_mapping(
component_names = None # Initialize to ensure it's always defined

if konflux:
new_component = get_konflux_component_by_component(runtime, whiteboard_component)
new_component = (
get_konflux_component_by_component(runtime, whiteboard_component) if not is_golang_builder else None
)
if not new_component:
# Special case for builder containers: they should map to all components that use this builder
if whiteboard_component == "openshift-golang-builder-container":
if is_golang_builder:
# Check which components actually use the golang builder
logger.info(f"Processing builder container CVE for '{whiteboard_component}' (golang builder)")

Expand Down
23 changes: 13 additions & 10 deletions elliott/elliottlib/cli/find_bugs_golang_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from artcommonlib.release_util import get_patch_from_release, isolate_el_version_in_release
from artcommonlib.rhcos import get_container_configs
from artcommonlib.rpm_utils import parse_nvr
from artcommonlib.util import is_ocp_delivery_repo
from doozerlib.cli.get_nightlies import find_rc_nightlies
from prettytable import PrettyTable
from pyartcd.util import get_release_name_for_assembly
Expand Down Expand Up @@ -162,7 +163,7 @@ def _is_fixed(self, bug: JIRABug, tracker_fixed_in: Set[Version], go_nvr_map) ->
for go_build in go_nvr_map.keys():
# if this is a builder image then fetch the golang rpm
parent_go_build = go_build
if constants.GOLANG_BUILDER_CVE_COMPONENT in go_build:
if constants.GOLANG_BUILDER_COMPONENT in go_build:
parsed_nvr = parse_nvr(go_build)
go_builder_nvr_map = get_golang_container_nvrs(
[(parsed_nvr['name'], parsed_nvr['version'], parsed_nvr['release'])], self._logger, exact=True
Expand Down Expand Up @@ -210,13 +211,13 @@ def _is_fixed(self, bug: JIRABug, tracker_fixed_in: Set[Version], go_nvr_map) ->
# In case this is for builder image
# and if vulnerable builds make up for less than 10% of total builds, consider it fixed
# this is due to etcd and a few payload images lagging behind due to special reasons
if bug.whiteboard_component == constants.GOLANG_BUILDER_CVE_COMPONENT and vuln_builds / total_builds < 0.1:
if constants.is_golang_builder_component(bug.whiteboard_component) and vuln_builds / total_builds < 0.1:
self._logger.info("Vulnerable builds make up for less than 10% of total builds, considering it fixed")
fixed = True
else:
fixed = True

if bug.whiteboard_component == constants.GOLANG_BUILDER_CVE_COMPONENT:
if constants.is_golang_builder_component(bug.whiteboard_component):
build_artifacts = f"Images in {self.pullspec}"
else:
nvrs = []
Expand Down Expand Up @@ -486,7 +487,9 @@ def is_valid(b: JIRABug):
if comp in not_art:
logger.info(f"{b.id} is for a component that is not built by ART: {comp}. Skipping")
return False
if comp.endswith("-container") and comp != constants.GOLANG_BUILDER_CVE_COMPONENT:
if (
comp.endswith("-container") or is_ocp_delivery_repo(comp)
) and not constants.is_golang_builder_component(comp):
logger.info(f"{b.id} is for a non-builder image: {comp}. Skipping")
return False
return True
Expand All @@ -510,7 +513,7 @@ def is_valid(b: JIRABug):
filtered_bugs_rpms_only = []
for b in bugs:
# Skip builder container bugs
if b.whiteboard_component == constants.GOLANG_BUILDER_CVE_COMPONENT:
if constants.is_golang_builder_component(b.whiteboard_component):
continue
# Skip image components (those that start with "openshift<digit>/")
if re.match(r'^openshift\d+/', b.whiteboard_component):
Expand Down Expand Up @@ -648,7 +651,7 @@ def compare(b1, b2):
fixed_bugs, unfixed_bugs, updated_bugs = [], [], []
for bug in bugs:
component = bug.whiteboard_component
art_managed = (component == constants.GOLANG_BUILDER_CVE_COMPONENT) or (component in self._runtime.rpm_map)
art_managed = constants.is_golang_builder_component(component) or (component in self._runtime.rpm_map)
logger.info(f"{bug.id} has security component: {component}")
fixed, comment, component_builds, parent_golang_builds = False, '', [], []

Expand All @@ -658,7 +661,7 @@ def compare(b1, b2):
continue
logger.info(f"{bug.id} is fixed in: {[str(v) for v in tracker_fixed_in]}")

if component == constants.GOLANG_BUILDER_CVE_COMPONENT:
if constants.is_golang_builder_component(component):
fixed, comment, component_builds, parent_golang_builds = await self.is_fixed_golang_builder(
bug, tracker_fixed_in=tracker_fixed_in
)
Expand Down Expand Up @@ -748,7 +751,7 @@ def compare(b1, b2):
"--component",
"components",
multiple=True,
help="Only operate on trackers for these JIRA Bug components e.g. openshift-golang-builder-container",
help="Only operate on trackers for these JIRA Bug components e.g. openshift4/openshift-golang-builder",
)
@click.option('--art-jira', help='Related ART Jira ticket for reference e.g. ART-1234')
@click.option(
Expand Down Expand Up @@ -815,7 +818,7 @@ async def find_bugs_golang_cli(

Bugs are compared with latest builds in `stream` assembly by default. Pass --assembly to specify.

For openshift-golang-builder-container build, use --pullspec <payload_pullspec> to determine if fixed for builds in
For openshift-golang-builder build, use --pullspec <payload_pullspec> to determine if fixed for builds in
given pullspec

Note: rpm trackers cannot be processed if --pullspec is used, for that rely on --assembly.
Expand All @@ -828,7 +831,7 @@ async def find_bugs_golang_cli(
bugs like openshift-golang-builder where we want to move the bug to VERIFIED after or close to when mass rebuild is
triggered.

--component: Only operate on trackers for these JIRA Bug components e.g. openshift-golang-builder-container.
--component: Only operate on trackers for these JIRA Bug components e.g. openshift4/openshift-golang-builder.

--rpms-only: Ignore builder container bugs and only analyze RPM trackers.

Expand Down
5 changes: 3 additions & 2 deletions elliott/elliottlib/cli/find_bugs_sweep_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ def filter_art_managed_jira_trackers(
continue

if (
normalized_component == constants.GOLANG_BUILDER_CVE_COMPONENT
constants.is_golang_builder_component(normalized_component)
or normalized_component in art_managed_image_components
):
art_trackers.append(bug)
Expand Down Expand Up @@ -614,7 +614,8 @@ def categorize_bugs_by_type(
if kind == 'image':
# golang builder is a special tracker component
# which applies to all our golang images
exception_packages.append(constants.GOLANG_BUILDER_CVE_COMPONENT)
exception_packages.append(constants.GOLANG_BUILDER_OCP4_DELIVERY_REPO)
exception_packages.append(constants.GOLANG_BUILDER_COMPONENT)

for bug in tracker_bugs:
package_name = bug.whiteboard_component
Expand Down
13 changes: 12 additions & 1 deletion elliott/elliottlib/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,18 @@
}

# 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'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't know if we had discussion around handling openshift5/ here ? If not, then I'd recommend that



def is_golang_builder_delivery_repo(name: str) -> bool:
return name in (GOLANG_BUILDER_OCP4_DELIVERY_REPO,)


def is_golang_builder_component(component: str) -> bool:
"""Accept both component name and delivery repo names during migration."""
return component == GOLANG_BUILDER_COMPONENT or is_golang_builder_delivery_repo(component)


BUG_LOOKUP_CHUNK_SIZE = 100
BUG_ATTACH_CHUNK_SIZE = 100
Expand Down
8 changes: 4 additions & 4 deletions elliott/elliottlib/errata_async.py
Original file line number Diff line number Diff line change
Expand Up @@ -380,13 +380,13 @@ def compute_cve_exclusions(cls, attached_builds: Iterable[str], expected_cve_com

# separate out golang CVEs and non-golang CVEs
# expected_brew_components contain non-golang CVEs components for regular analysis
# All golang CVEs will have component as constants.GOLANG_BUILDER_CVE_COMPONENT
# All golang CVEs will have component as constants.GOLANG_BUILDER_OCP4_DELIVERY_REPO
# which we do not attach to our advisories since it's a builder image
# It requires special treatment
expected_brew_components = set()
golang_cve_names = set()
for cve_name, components in expected_cve_components.items():
if constants.GOLANG_BUILDER_CVE_COMPONENT not in components:
if constants.GOLANG_BUILDER_OCP4_DELIVERY_REPO not in components:
expected_brew_components.update(components)
else:
golang_cve_names.add(cve_name)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
Expand Down Expand Up @@ -417,10 +417,10 @@ def populate_golang_cve_components(cls, golang_cve_names, expected_cve_component
builder_nvr = parse_nvr(builder_nvr_string)

# Make sure they are go builder nvrs (this should never happen)
if builder_nvr['name'] != constants.GOLANG_BUILDER_CVE_COMPONENT:
if builder_nvr['name'] != constants.GOLANG_BUILDER_COMPONENT:
raise ValueError(
f"Unexpected `name` value for nvr {builder_nvr}. Expected "
f"{constants.GOLANG_BUILDER_CVE_COMPONENT}. Please investigate."
f"{constants.GOLANG_BUILDER_COMPONENT}. Please investigate."
)

if 'etcd' in list(go_nvr_map[builder_nvr_string])[0]:
Expand Down
8 changes: 3 additions & 5 deletions elliott/elliottlib/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ def get_golang_container_nvrs_brew(nvrs: List[Tuple[str, str, str]], logger, exa
logger.error(f'Error parsing {build}')
raise
name = nvr[0]
if name == 'openshift-golang-builder-container' or 'go-toolset' in name:
if name == constants.GOLANG_BUILDER_COMPONENT or 'go-toolset' in name:
go_version = get_parent_golang_from_brew(nvr)
if not go_version:
raise ValueError(f'Could not find go version for golang builder {nvr}')
Expand Down Expand Up @@ -591,7 +591,7 @@ def get_golang_container_nvrs_for_konflux_record(
# NVRs never contain '/', so we can distinguish them from pullspecs.
if '/' not in p:
# This is an NVR - look for golang builder (e.g. openshift-golang-builder-container-v1.24.4-...)
if p.startswith(f'{constants.GOLANG_BUILDER_CVE_COMPONENT}-'):
if p.startswith(f'{constants.GOLANG_BUILDER_COMPONENT}-'):
go_version = p
break
else:
Expand All @@ -601,9 +601,7 @@ def get_golang_container_nvrs_for_konflux_record(
go_version = spec.replace(':', '-container-')
break
elif spec.startswith('art-images:golang-builder-'):
go_version = spec.replace(
'art-images:golang-builder-', f'{constants.GOLANG_BUILDER_CVE_COMPONENT}-'
)
go_version = spec.replace('art-images:golang-builder-', f'{constants.GOLANG_BUILDER_COMPONENT}-')
break

if not go_version:
Expand Down
97 changes: 91 additions & 6 deletions elliott/tests/test_bzutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -740,36 +740,36 @@ def test_whiteboard_component_normalizes_duplicated_component(self):
flexmock(
key=1,
fields=flexmock(
labels=["pscomponent:openshift-golang-builder-container/openshift-golang-builder-container"],
labels=["pscomponent:some-component/some-component"],
issuetype=flexmock(name="Bug"),
),
)
)
self.assertEqual(bug.whiteboard_component, "openshift-golang-builder-container")
self.assertEqual(bug.whiteboard_component, "some-component")

# Non-duplicated component with slash should be left alone
bug = JIRABug(
flexmock(
key=2,
fields=flexmock(
labels=["pscomponent:openshift4/ose-cli"],
labels=["pscomponent:openshift4/openshift-golang-builder"],
issuetype=flexmock(name="Bug"),
),
)
)
self.assertEqual(bug.whiteboard_component, "openshift4/ose-cli")
self.assertEqual(bug.whiteboard_component, "openshift4/openshift-golang-builder")

# Component without slash should be unaffected
bug = JIRABug(
flexmock(
key=3,
fields=flexmock(
labels=["pscomponent:openshift-golang-builder-container"],
labels=["pscomponent:openshift-clients"],
issuetype=flexmock(name="Bug"),
),
)
)
self.assertEqual(bug.whiteboard_component, "openshift-golang-builder-container")
self.assertEqual(bug.whiteboard_component, "openshift-clients")


class TestBugzillaBug(unittest.TestCase):
Expand Down Expand Up @@ -1344,6 +1344,91 @@ def test_is_first_fix_any_false(self):
actual = bzutil.is_first_fix_any(mock_runtime, flaw_bug, tracker_bugs)
self.assertEqual(expected, actual)

def test_is_first_fix_normalizes_delivery_repo_tracker(self):
"""When tracker has delivery-repo pscomponent (openshift4/foo) and Hydra
returns the same name resolved to its Brew component, normalization
should make them match."""
mock_runtime = flexmock()
mock_runtime.should_receive("get_major_minor").and_return((4, 17))

mock_meta = flexmock()
mock_config = flexmock()
mock_delivery = flexmock()
mock_delivery.delivery_repo_names = ["openshift4/ose-cli"]
mock_config.delivery = mock_delivery
mock_meta.config = mock_config
mock_meta.should_receive("get_component_name").and_return("ose-cli-container")

mock_runtime.should_receive("image_metas").and_return([mock_meta])

hydra_data = {
"package_state": [
{
"product_name": "Red Hat OpenShift Container Platform 4",
"fix_state": "Affected",
"package_name": "openshift4/ose-cli",
},
],
}
flexmock(requests).should_receive("get").and_return(
flexmock(json=lambda: hydra_data, raise_for_status=lambda: None)
)

flaw_bug = BugzillaBug(flexmock(id=1, alias=["CVE-2024-999"]))
tracker_bugs = [flexmock(id=2, whiteboard_component="openshift4/ose-cli")]
actual = bzutil.is_first_fix_any(mock_runtime, flaw_bug, tracker_bugs)
self.assertTrue(actual)

def test_is_first_fix_fallback_when_delivery_repo_unresolved(self):
"""When Hydra returns a delivery-repo name that can't be translated,
the raw name should be kept in unfixed_components so trackers using
that same delivery-repo pscomponent still match."""
mock_runtime = flexmock()
mock_runtime.should_receive("get_major_minor").and_return((4, 17))

# Provide a dummy image meta that does NOT match the golang builder,
# so get_component_by_delivery_repo returns None instead of raising.
dummy_meta = flexmock()
dummy_config = flexmock()
dummy_delivery = flexmock()
dummy_delivery.delivery_repo_names = ["openshift4/unrelated-image"]
dummy_config.delivery = dummy_delivery
dummy_meta.config = dummy_config
dummy_meta.should_receive("get_component_name").and_return("unrelated-container")
mock_runtime.should_receive("image_metas").and_return([dummy_meta])

hydra_data = {
"package_state": [
{
"product_name": "Red Hat OpenShift Container Platform 4",
"fix_state": "Affected",
"package_name": "openshift4/openshift-golang-builder",
},
],
}
flexmock(requests).should_receive("get").and_return(
flexmock(json=lambda: hydra_data, raise_for_status=lambda: None)
)

flaw_bug = BugzillaBug(flexmock(id=1, alias=["CVE-2024-888"]))
tracker_bugs = [flexmock(id=2, whiteboard_component="openshift4/openshift-golang-builder")]
actual = bzutil.is_first_fix_any(mock_runtime, flaw_bug, tracker_bugs)
self.assertTrue(actual)


class TestIsGolangBuilderComponent(unittest.TestCase):
def test_matches_cve_component(self):
self.assertTrue(constants.is_golang_builder_component(constants.GOLANG_BUILDER_OCP4_DELIVERY_REPO))

def test_matches_brew_component(self):
self.assertTrue(constants.is_golang_builder_component(constants.GOLANG_BUILDER_COMPONENT))

def test_rejects_unrelated(self):
self.assertFalse(constants.is_golang_builder_component("some-other-container"))

def test_rejects_empty(self):
self.assertFalse(constants.is_golang_builder_component(""))


class TestSearchFilter(unittest.TestCase):
def test_search_filter(self):
Expand Down
Loading
Loading