Skip to content

Add missing deptrack upload options#230

Open
Erik-Hoffmann wants to merge 18 commits into
sbomify:masterfrom
Erik-Hoffmann:master
Open

Add missing deptrack upload options#230
Erik-Hoffmann wants to merge 18 commits into
sbomify:masterfrom
Erik-Hoffmann:master

Conversation

@Erik-Hoffmann
Copy link
Copy Markdown

@Erik-Hoffmann Erik-Hoffmann commented May 22, 2026

Summary

  1. Added new configuration options for uploads to dependency-track:
  • DTRACK_PROJECT_TAGS - Tags to add to the project (comma seperated)
  • DTRACK_PARENT_ID - Id of the parent project
  • DTRACK_PARENT_NAME - Name of the parent project
  • DTRACK_PARENT_VERSION - Version of the parent project
  • DTRACK_IS_LATEST - Mark the uploaded BOM as the latest version (default: false)
  1. New classmethod in procol.py: _get_env_list, which returns a comma seperated env option as a list.
  2. Added the new options to README and formatted

closes: #212

@vpetersson
Copy link
Copy Markdown
Contributor

vpetersson commented May 26, 2026

Thanks @Erik-Hoffmann. Issues to address before merge:

Blockers

  • CI Sanity Checks failing. Ruff I001 on the new List import in dependency_track.py, and ruff format --check wants to reformat both dependency_track.py and protocol.py. Run:
    uv run ruff check --fix sbomify_action tests && uv run ruff format sbomify_action tests
  • No tests for the new options. Extend TestDependencyTrackConfig / TestDependencyTrackDestination in tests/test_upload_module.py to cover tag parsing, parent fields, isLatest, and the resulting payload. 80% coverage is required.
  • closes: link won't auto-close Enhancements for DependencyTrack Compatibility #212. Needs closes #212.

Correctness

  • upload() always sends parentUUID / parentName / parentVersion (as null when unset) and "projectTags": [] when no tags are set. The empty-list case risks clearing existing tags on repeat uploads. Only add each key to payload when the user configured it, matching how project / projectName + projectVersion work today.
  • _get_env_list doesn't trim or drop empties — "nightly, alpine" becomes ["nightly", " alpine"]. Use:
    [t.strip() for t in value.split(",") if t.strip()]
  • No validation that parent options are coherent. DT requires parentUUID, or both parentName + parentVersion; setting only one silently sends a half-populated payload.

Style

  • Per CLAUDE.md, use 3.10+ generics: list[dict], dict[str, Any], X | None — not List / Dict / Optional.
  • project_tags: List[dict] = None annotates non-optional but defaults to None. Should be:
    project_tags: list[dict] | None = None
    Same fix on _get_env_list(default: list = None)default: list | None = None.
  • Replace the lambda+map with a comprehension:
    [{"name": t} for t in cls._get_env_list("PROJECT_TAGS")]
  • Typo "seperated" → "separated" (PR body, README, docstring).
  • README diff includes unrelated Prettier reformatting (table padding, JSON snippet spacing in Yocto/enrichment sections). Please revert or split into its own commit.

Minor

  • PR is from your fork's master. A topic branch avoids future commits folding into this PR.

@Erik-Hoffmann
Copy link
Copy Markdown
Author

Hi @vpetersson thank you for your review!

I addressed all issues you mentioned. Sorry for not adding tests, that was my stupidity, also I am not working a lot with Github so sorry for not using Branching best-practices here^^

I hope that you (and Claude) are happy with the changes now, else please share again such a detailed review, that is much appreciated.

@vpetersson
Copy link
Copy Markdown
Contributor

Thanks @Erik-Hoffmann — the substantive feedback is all addressed. Tests pass locally (73/73), ruff lint + format both clean. A few items still outstanding:

Still outstanding

  • Typo not fully fixed. sbomify_action/_upload/destinations/dependency_track.py:14 and :123 still say "comma seperated". README was fixed but the Python docstrings were missed.

  • README still contains unrelated Prettier reformatting. The revert README.md commit was followed by format README (8386483), which re-introduced the same noise I flagged last round:

    • line 139: AUGMENT: true #AUGMENT: true # (whitespace)
    • lines 354–362: Yocto CLI options table re-padded
    • lines 411–414: sbomify.json JSON snippet reformatted ({ "name": ... } spacing, multi-line → single-line)
    • lines 615–620: lifecycle properties JSON reformatted
    • line 964: trailing whitespace tweak on Trivy row
    • lines 991–996: Tools install table re-padded
    • lines 1031–1036: Django enrichment JSON reformatted

    Only the DT table addition (lines 264–277) and the workflow example additions (lines 297–300) belong in this PR. Please drop the rest — easiest path is git checkout origin/master -- README.md, then re-apply only the DT-related additions by hand.

  • Type-annotation style isn't applied consistently. The new fields mix old and new style:

    project_tags: list[dict] | None = None     # new style ✓
    parent_id: Optional[str] = None            # old style
    parent_name: Optional[str] = None          # old style
    parent_version: Optional[str] = None       # old style

    Per CLAUDE.md these should all be str | None. (Leaving the pre-existing project_id: Optional[str] alone is fine — out of scope.)

  • tests/test_upload_module.pytest_upload_partial_parent_config has a leftover print(result.error_message) debug line. Please remove.

Nit (non-blocking)

  • isLatest is always sent in the payload (defaults to false). On re-uploads without DTRACK_IS_LATEST=true, this will reset DT's "latest" flag every time. Mirrors how autoCreate is sent today, so it's consistent — flagging in case it's not the intent. If you want it to be opt-in only, change the default to None and gate the payload assignment.

Once those are cleaned up I think this is good to go.

@vpetersson
Copy link
Copy Markdown
Contributor

Correction to my previous comment — Sanity Checks is actually failing on CI. I only ran ruff check and ruff format locally and reported those as clean; I missed that the Sanity Checks job also runs mypy, which is failing:

sbomify_action/_upload/protocol.py:88: error: Missing type parameters for generic type "list"  [type-arg]
sbomify_action/_upload/destinations/dependency_track.py:65: error: Missing type parameters for generic type "dict"  [type-arg]

Reproduced locally with uv run mypy sbomify_action. The bare list and dict need parameters:

  • protocol.py:87def _get_env_list(cls, key: str, default: list | None = None) -> list: → use list[str] | None and -> list[str]
  • dependency_track.py:65project_tags: list[dict] | None = Nonelist[dict[str, str]] | None (since you build [{"name": t}])

Apologies for the misleading "lint + format clean" line — it was true but incomplete.

@Erik-Hoffmann
Copy link
Copy Markdown
Author

@vpetersson thank you for the quick review. Since you suggested to split the README formatting in a separate commit, I did that - but ok, I dropped the formatting now.

I changed the project_tags behaviour last commits to be alway list[str] since I would expect to have this in the configuration, as that is returned by _get_env_list. The dict {"name": "tag"} is now built right before using it in the upload function, making everything more convenient and easier to test.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enhancements for DependencyTrack Compatibility

2 participants