sbp commented on code in PR #1411:
URL:
https://github.com/apache/tooling-trusted-releases/pull/1411#discussion_r3587388072
##########
atr/tasks/__init__.py:
##########
@@ -714,45 +771,42 @@ async def _draft_file_checks(
if task:
task.revision_number = str(revision_number)
await _add_task(data, task)
- if util.archive_format_stem(path.as_path().name) is not None:
- archive_comparison_task = await queued(
- asf_uid,
- sql.TaskType.ARCHIVE_COMPARISON,
- release,
- revision_number,
- path_str,
- check_cache_key=await checks.resolve_cache_key(
- resolve(sql.TaskType.ARCHIVE_COMPARISON),
- parity.CHECK_VERSION,
- parity.INPUT_POLICY_KEYS,
- release,
- revision_number,
- await checks.resolve_extra_args(parity.INPUT_EXTRA_ARGS,
release, path_str),
- file=path_str,
- ),
- )
- if archive_comparison_task:
- await _add_task(data, archive_comparison_task)
- # TODO: Should we check .json files for their content?
- # Ideally we would not have to do that
- if analysis.is_cyclonedx_json(path.as_path().name):
- cdx_task = await queued(
- asf_uid,
- sql.TaskType.SBOM_TOOL_SCORE,
+ # Each path may qualify for a handful of standalone checks - archive
parity, an
+ # SBOM presence check, and CycloneDX scoring. The helpers guard their own
+ # applicability and hand back a task only when one is warranted.
+ for maybe_task in (
+ await _archive_comparison_task(asf_uid, release, revision_number,
path),
+ await _has_sbom_task(asf_uid, release, revision_number, path),
+ await _cyclonedx_score_task(
+ asf_uid, release, revision_number, path, project_key,
release_version, previous_version
+ ),
+ ):
+ if maybe_task:
+ await _add_task(data, maybe_task)
+
+
+async def _has_sbom_task(
+ asf_uid: str, release: sql.Release, revision_number: safe.RevisionNumber,
path: safe.RelPath
+) -> sql.Task | None:
+ if not analysis.is_artifact(path.as_path()):
+ return None
+ path_str = str(path)
+ return await queued(
+ asf_uid,
+ sql.TaskType.HAS_SBOM,
+ release,
+ revision_number,
+ path_str,
+ check_cache_key=await checks.resolve_cache_key(
+ resolve(sql.TaskType.HAS_SBOM),
+ sbom_check.CHECK_VERSION,
+ parity.INPUT_POLICY_KEYS,
Review Comment:
Presumably should be `sbom_check.INPUT_POLICY_KEYS` (though it makes no
difference currently since they're both `[]`).
##########
atr/tasks/__init__.py:
##########
@@ -714,45 +771,42 @@ async def _draft_file_checks(
if task:
task.revision_number = str(revision_number)
await _add_task(data, task)
- if util.archive_format_stem(path.as_path().name) is not None:
- archive_comparison_task = await queued(
- asf_uid,
- sql.TaskType.ARCHIVE_COMPARISON,
- release,
- revision_number,
- path_str,
- check_cache_key=await checks.resolve_cache_key(
- resolve(sql.TaskType.ARCHIVE_COMPARISON),
- parity.CHECK_VERSION,
- parity.INPUT_POLICY_KEYS,
- release,
- revision_number,
- await checks.resolve_extra_args(parity.INPUT_EXTRA_ARGS,
release, path_str),
- file=path_str,
- ),
- )
- if archive_comparison_task:
- await _add_task(data, archive_comparison_task)
- # TODO: Should we check .json files for their content?
- # Ideally we would not have to do that
- if analysis.is_cyclonedx_json(path.as_path().name):
- cdx_task = await queued(
- asf_uid,
- sql.TaskType.SBOM_TOOL_SCORE,
+ # Each path may qualify for a handful of standalone checks - archive
parity, an
+ # SBOM presence check, and CycloneDX scoring. The helpers guard their own
+ # applicability and hand back a task only when one is warranted.
+ for maybe_task in (
+ await _archive_comparison_task(asf_uid, release, revision_number,
path),
+ await _has_sbom_task(asf_uid, release, revision_number, path),
+ await _cyclonedx_score_task(
+ asf_uid, release, revision_number, path, project_key,
release_version, previous_version
+ ),
+ ):
+ if maybe_task:
+ await _add_task(data, maybe_task)
+
+
+async def _has_sbom_task(
+ asf_uid: str, release: sql.Release, revision_number: safe.RevisionNumber,
path: safe.RelPath
+) -> sql.Task | None:
+ if not analysis.is_artifact(path.as_path()):
+ return None
+ path_str = str(path)
+ return await queued(
+ asf_uid,
+ sql.TaskType.HAS_SBOM,
+ release,
+ revision_number,
+ path_str,
+ check_cache_key=await checks.resolve_cache_key(
Review Comment:
This ends up only putting the artifact hash in the cache key, which means
that if somebody is missing an SBOM, is notified by this check, and then
uploads an SBOM, the new SBOM won't appear in the cache key so it won't be
rechecked.
We could use the existing `all_files` resolver here, but I guess the proper
thing to do would be to create a new resolver just for the candidate SBOM
files? That's not particularly elegant either way, but I think the latter would
be best. What do you think?
##########
atr/util.py:
##########
@@ -444,7 +444,7 @@ async def check_propagation(
def checker_display_name(checker: str) -> str:
- return checker.removeprefix("atr.tasks.checks.").replace("_", "
").replace(".", " ").title()
+ return checker.removeprefix("atr.tasks.checks.").replace("_", "
").replace(".", " ").title().replace("Sbom", "SBOM")
Review Comment:
This only happens on compose pages, not on report pages, so it'll be Sbom on
there. Not particularly high priority, but thought you might appreciate the
note since I guess the intention is for this to be consistent.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
