fix: set license and manufacturer for the main component #654

[dimitarp]

Put license and manufacturer data under $.metadata.component.licenses and $.metadata.component.manufacturer instead of $.metadata.licenses and $.metadata.manufacturer. The latter refers to the license and manufacturer of the SBOM itself and not of the main component. The plugin README states that licenseChoice is the "License information for the main component".

fixes #654

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity · 0 duplication

Metric	Results
Complexity	0
Duplication	0

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[skhokhlov]

Thanks for this! It is a correct per the CycloneDX spec.

However, this is a breaking change to the generated SBOM, which we treat as public API. It needs a major version bump (4.x.x). So the merge might be delayed until we understand full scope go the next major release.

[skhokhlov]

Note that toComponent (line ~264) may already set this component's licenses from resolved project metadata, and this line overwrites them. That's the behavior we want - a configured licenseChoice is the single source of truth for the main component's license - so no change needed here. Please add a sentence to the README's licenseChoice docs stating that a configured value replaces any auto-detected license for the main component.

[skhokhlov]

This presence + not-empty check duplicates lines 118-120; only the version comparison differs. Optional cleanup - extract a small helper and reuse it in both branches:

private Optional<OrganizationalEntity> configuredManufacturer() {
    if (!task.getOrganizationalEntity().isPresent()) {
        return Optional.empty();
    }
    OrganizationalEntity oe = task.getOrganizationalEntity().get();
    return new OrganizationalEntity().equals(oe) ? Optional.empty() : Optional.of(oe);
 }

Happy to merge without this.

[skhokhlov]

Please also assert the data left the old (document-level) location, otherwise a regression that writes to both places would pass:

assert bom.getMetadata().getLicenses() == null
assert bom.getMetadata().getManufacturer() == null

[skhokhlov]

Same here. Add the negative assertions so the old paths are verified empty:

assert bom.getMetadata().getLicenses() == null
assert bom.getMetadata().getManufacturer() == null

[skhokhlov]

This branch asserts metadata.manufacture == null but not the BOM-level metadata.manufacturer. Please add the following so a manufacturer accidentally written to both the component and the document level is caught:

assert jsonBom.getMetadata().getManufacturer() == null
assert xmlBom.getMetadata().getManufacturer() == null

[dimitarp]

Thanks for the review! I'll look into it the next days.

[dimitarp]

Thanks for this! It is a correct per the CycloneDX spec.

However, this is a breaking change to the generated SBOM, which we treat as public API. It needs a major version bump (4.x.x). So the merge might be delayed until we understand full scope go the next major release.

I understand your concern. However, maybe the README should describe the actual behavior for the licenseChoice and organizationalEntity parameters.

Optionally, implementing new parameters componentLicenseChoice and componentOrganizationalEntity (similar to componentName, componentVersion and componentGroup) would avoid the breaking change. What are your thoughts on this approach?

[Zuplyx]

Personally I think it would be best to add new parameters for the component manufacturer. This way the manufacturer can be set both for the SBOM itself and the main component. Additionally some Guidelines (e.g. the German BSI TR-03183) actually require the manufacturer on both levels.