Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
12 changes: 6 additions & 6 deletions elliott/elliottlib/errata_async.py
Original file line number Diff line number Diff line change
Expand Up @@ -380,16 +380,16 @@ 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
# Golang CVEs have a builder component (delivery repo or legacy name)
# 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:
expected_brew_components.update(components)
else:
if any(constants.is_golang_builder_component(c) for c in components):
golang_cve_names.add(cve_name)
else:
expected_brew_components.update(components)

missing_brew_components = expected_brew_components - attached_brew_components
if missing_brew_components:
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