Repository: CycloneDX/cyclonedx-maven-plugin
Pull request #428 (2023-11-10 15:16:13)

Add support for custom external references
This PR addresses #421 and adds the configured external references into two different places of the generated SBOM:

1. `$.metadata.component.externalReferences`
1. `$.components[].externalReferences` (only for `$.components` contained by the reactor)

While (1) only requires the basic information about the current Maven module that is being processed, (2) was pretty tricky to get right. For (2), the parent module needs to obtain the external references configured for child modules. Hence, using the value injected into the

```
@Parameter
private ExternalReference[] externalReferences;
```

field of the mojo is of no use there. In `DefaultModelConverter#convert(MojoExecution, Artifact, CycloneDxSchema.Version, boolean)`, I locate the matching plugin and its matching execution, and merge the configuration of the two. Notice the `.distinct()` during this merge – without that, merged external references contain duplicates. My suspicion is `getEffectiveMavenProject()` doesn't always return the _effective_ POM.

garydgregory: FYI, this issues Apache Log4j.
hboutemy: @vy do you really need to be able to customize external references in each child module? defining a value in the parent is not sufficient?

I understand the theory of this, but implementing it means that this plugins' code is doing Maven job in parallel to Maven (= mapping plugin execution configuration to objects): this is complex and fragile

> I locate the matching plugin and its matching execution, and merge the configuration of the two. Notice the .distinct() during this merge – without that, merged external references contain duplicates. My suspicion is getEffectiveMavenProject() doesn't always return the effective POM.

your suspicion is not exactly right: the effective POM is the effective POM: it's your merge that does not mimic real Maven way of merging configuration from each level of effective Maven POM = exactly what I told before, you are trying to reimplement a Maven mechanism, that is risky
garydgregory: CC: @ppkarwasz 
ppkarwasz: I believe the `makeAggregateBom` goal logic would become much simpler if the execution order was:

1. the `makeBom` goal is executed for all the modules and the SBOMs artifacts are attached to them,
2. the root project just merges the SBOMs of all its sub-modules.

Unfortunately I have no idea on how to implement this in Maven. @hboutemy any suggestions?

The current order of execution is problematic, since the root project must do the same work as its submodules, including the resolution of the CycloneDX configuration of submodules.
vy: > @vy do you really need to be able to customize external references in each child module? defining a value in the parent is not sufficient?

I think it is indeed a good idea to re-evaluate our assumptions. @stevespringett, you were the major lead in hinting us to place the VDR/VEX references in the following two places:

1. `$.metadata.component.externalReferences`
1. `$.components[].externalReferences` (only for `$.components` contained by the reactor)

@stevespringett, can we live with only (1)? What would be the cons of dropping (2)?

> I understand the theory of this, but implementing it means that this plugins' code is doing Maven job in parallel to Maven (= mapping plugin execution configuration to objects): this is complex and fragile

@hboutemy, can we explicitly document this caveat of this feature and see how far we can go? Maybe even put a disclaimer stating the feature is experimental?
stevespringett: > 1. `$.metadata.component.externalReferences`
> 1. `$.components[].externalReferences` (only for $.components contained by the reactor)
> 
> @stevespringett, can we live with only (1)? What would be the cons of dropping (2)?

Granularity would be the only con that I can see. By limiting to `$.metadata.component.externalReferences`, the parent and all modules would be able to specify additional external references, but would not be able to specify them for any component dependencies they may include. 

If the assumption is that a supplier or open source project (e.g. Apache Logging) wants to specify external references for VDR/VEX, then limiting to the first option should have no impact on their ability to do so.
hboutemy: @ppkarwasz 

> I believe the makeAggregateBom goal logic would become much simpler if the execution order was:

Yes, but it's not what Maven does currently: I finally opened the Maven Jira issue that I wanted to open for a few years to better document the current issues and future solutions https://issues.apache.org/jira/browse/MNG-7991

@vy @stevespringett 

> Granularity would be the only con that I can see. By limiting to `$.metadata.component.externalReferences`, the parent and all modules would be able to specify additional external references, but would not be able to specify them for any component dependencies they may include.

to me, adding external references makes only sense for supplier on what he provides: I don't see why a supplier would add references for his dependencies, given it would be specific only to his own use of the dependencies

> If the assumption is that a supplier or open source project (e.g. Apache Logging) wants to specify external references for VDR/VEX, then limiting to the first option should have no impact on their ability to do so.

I think this is what the current Apache Logging case is about, and a very good step forward
vy: @hboutemy, I have updated the branch to only contain the `$.metadata.component.externalReferences` enrichment. I personally prefer the initial version, but I trust your judgement. Let me know if there is anything further I need to provide.
vy: > I can merge the PR and do the changes after, but it's better to not do the unwanted changes for dependency artifacts

@hboutemy, I have addressed your remarks. Would you mind checking again, please?

Patch:
diff --git a/src/main/java/org/cyclonedx/maven/BaseCycloneDxMojo.java b/src/main/java/org/cyclonedx/maven/BaseCycloneDxMojo.java
index ff304ea..8107d77 100644
--- a/src/main/java/org/cyclonedx/maven/BaseCycloneDxMojo.java
+++ b/src/main/java/org/cyclonedx/maven/BaseCycloneDxMojo.java
@@ -39,6 +39,7 @@ import org.cyclonedx.maven.ProjectDependenciesConverter.BomDependencies;
 import org.cyclonedx.model.Bom;
 import org.cyclonedx.model.Component;
 import org.cyclonedx.model.Dependency;
+import org.cyclonedx.model.ExternalReference;
 import org.cyclonedx.model.Metadata;
 import org.cyclonedx.model.Property;
 import org.cyclonedx.parsers.JsonParser;
@@ -217,6 +218,14 @@ public abstract class BaseCycloneDxMojo extends AbstractMojo {
     @Parameter( defaultValue = "${project.build.outputTimestamp}" )
     private String outputTimestamp;
 
+    /**
+     * External references to be added to <code>$.metadata.component.externalReferences[]</code>.
+     *
+     * @since 2.7.11
+     */
+    @Parameter
+    private ExternalReference[] externalReferences;
+
     @org.apache.maven.plugins.annotations.Component
     private MavenProjectHelper mavenProjectHelper;
 
@@ -301,7 +310,7 @@ public abstract class BaseCycloneDxMojo extends AbstractMojo {
 
         String analysis = extractComponentsAndDependencies(topLevelComponents, componentMap, dependencyMap);
         if (analysis != null) {
-            final Metadata metadata = modelConverter.convert(project, projectType, schemaVersion(), includeLicenseText);
+            final Metadata metadata = modelConverter.convert(project, projectType, schemaVersion(), includeLicenseText, externalReferences);
 
             if (schemaVersion().getVersion() >= 1.3) {
                 metadata.addProperty(newProperty("maven.goal", analysis));
diff --git a/src/main/java/org/cyclonedx/maven/DefaultModelConverter.java b/src/main/java/org/cyclonedx/maven/DefaultModelConverter.java
index b8aaa20..ae79e49 100644
--- a/src/main/java/org/cyclonedx/maven/DefaultModelConverter.java
+++ b/src/main/java/org/cyclonedx/maven/DefaultModelConverter.java
@@ -18,18 +18,8 @@
  */
 package org.cyclonedx.maven;
 
-import java.io.File;
-import java.io.IOException;
-import java.net.URI;
-import java.net.URISyntaxException;
-import java.util.List;
-import java.util.Properties;
-import java.util.TreeMap;
-
-import javax.inject.Inject;
-import javax.inject.Named;
-import javax.inject.Singleton;
-
+import com.github.packageurl.MalformedPackageURLException;
+import com.github.packageurl.PackageURL;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.maven.artifact.Artifact;
 import org.apache.maven.artifact.DefaultArtifact;
@@ -55,8 +45,18 @@ import org.eclipse.aether.artifact.ArtifactProperties;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.github.packageurl.MalformedPackageURLException;
-import com.github.packageurl.PackageURL;
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Properties;
+import java.util.TreeMap;
+import java.util.stream.Collectors;
 
 @Singleton
 @Named
@@ -187,6 +187,16 @@ public class DefaultModelConverter implements ModelConverter {
             }
         }
         return component;
+
+    }
+
+    private static void setExternalReferences(Component component, ExternalReference[] externalReferences) {
+        if (externalReferences == null || externalReferences.length == 0) {
+            return;
+        }
+        // We need a mutable `List`, hence `Arrays.asList()` won't work.
+        List<ExternalReference> externalReferences_ = Arrays.stream(externalReferences).collect(Collectors.toList());
+        component.setExternalReferences(externalReferences_);
     }
 
     private boolean isModified(Artifact artifact) {
@@ -341,7 +351,7 @@ public class DefaultModelConverter implements ModelConverter {
     }
 
     @Override
-    public Metadata convert(final MavenProject project, String projectType, CycloneDxSchema.Version schemaVersion, boolean includeLicenseText) {
+    public Metadata convert(final MavenProject project, String projectType, CycloneDxSchema.Version schemaVersion, boolean includeLicenseText, ExternalReference[] externalReferences) {
         final Tool tool = new Tool();
         final Properties properties = readPluginProperties();
         tool.setVendor(properties.getProperty("vendor"));
@@ -367,6 +377,7 @@ public class DefaultModelConverter implements ModelConverter {
         component.setType(resolveProjectType(projectType));
         component.setPurl(generatePackageUrl(project.getArtifact()));
         component.setBomRef(component.getPurl());
+        setExternalReferences(component, externalReferences);
         extractComponentMetadata(project, component, schemaVersion, includeLicenseText);
 
         final Metadata metadata = new Metadata();
diff --git a/src/main/java/org/cyclonedx/maven/ModelConverter.java b/src/main/java/org/cyclonedx/maven/ModelConverter.java
index 98721e2..77b9d72 100644
--- a/src/main/java/org/cyclonedx/maven/ModelConverter.java
+++ b/src/main/java/org/cyclonedx/maven/ModelConverter.java
@@ -22,6 +22,7 @@ import org.apache.maven.artifact.Artifact;
 import org.apache.maven.project.MavenProject;
 import org.cyclonedx.CycloneDxSchema;
 import org.cyclonedx.model.Component;
+import org.cyclonedx.model.ExternalReference;
 import org.cyclonedx.model.Metadata;
 
 /**
@@ -57,7 +58,9 @@ public interface ModelConverter {
      * @param projectType the target CycloneDX component type
      * @param schemaVersion the target CycloneDX schema version
      * @param includeLicenseText should license text be included in bom?
+     * @param externalReferences the external references
      * @return a CycloneDX Metadata object
      */
-    Metadata convert(MavenProject project, String projectType, CycloneDxSchema.Version schemaVersion, boolean includeLicenseText);
+    Metadata convert(MavenProject project, String projectType, CycloneDxSchema.Version schemaVersion, boolean includeLicenseText, ExternalReference[] externalReferences);
+
 }


Test patch:
diff --git a/src/test/java/org/cyclonedx/maven/ExternalReferenceTest.java b/src/test/java/org/cyclonedx/maven/ExternalReferenceTest.java
new file mode 100644
index 0000000..d725fbd
--- /dev/null
+++ b/src/test/java/org/cyclonedx/maven/ExternalReferenceTest.java
@@ -0,0 +1,65 @@
+package org.cyclonedx.maven;
+
+import io.takari.maven.testing.executor.MavenRuntime.MavenRuntimeBuilder;
+import io.takari.maven.testing.executor.MavenVersions;
+import io.takari.maven.testing.executor.junit.MavenJUnitTestRunner;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.util.Arrays;
+import java.util.Collections;
+
+import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson;
+
+/**
+ * Verifies external references are populated as expected.
+ */
+@RunWith(MavenJUnitTestRunner.class)
+@MavenVersions({"3.6.3"})
+public class ExternalReferenceTest extends BaseMavenVerifier {
+
+    public ExternalReferenceTest(MavenRuntimeBuilder runtimeBuilder) throws Exception {
+        super(runtimeBuilder);
+    }
+
+    @Test
+    public void testAddedExternalReferences() throws Exception {
+
+        // Create the verifier
+        File projDir = resources.getBasedir("external-reference");
+        verifier
+                .forProject(projDir)
+                .withCliOption("-Dcyclonedx-maven-plugin.version=" + getCurrentVersion())
+                .withCliOption("-X")
+                .withCliOption("-B")
+                .execute("clean", "verify")
+                .assertErrorFreeLog();
+
+        // Verify parent metadata
+        assertExternalReferences(
+                new File(projDir, "target/bom.json"),
+                "$.metadata.component.externalReferences[?(@.type=='chat')].url",
+                Collections.singleton("https://acme.com/parent"));
+
+        // Verify child metadata
+        assertExternalReferences(
+                new File(projDir, "child/target/bom.json"),
+                "$.metadata.component.externalReferences[?(@.type=='chat')].url",
+                Arrays.asList("https://acme.com/parent", "https://acme.com/child"));
+
+    }
+
+    private static void assertExternalReferences(File bomFile, String jsonPath, Iterable<Object> expectedValues) throws IOException {
+        byte[] bomJsonBytes = Files.readAllBytes(bomFile.toPath());
+        String bomJson = new String(bomJsonBytes, StandardCharsets.UTF_8);
+        assertThatJson(bomJson)
+                .inPath(jsonPath)
+                .isArray()
+                .containsOnlyOnceElementsOf(expectedValues);
+    }
+
+}
diff --git a/src/test/resources/external-reference/child/pom.xml b/src/test/resources/external-reference/child/pom.xml
new file mode 100644
index 0000000..1455508
--- /dev/null
+++ b/src/test/resources/external-reference/child/pom.xml
@@ -0,0 +1,42 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>com.acme</groupId>
+        <artifactId>parent</artifactId>
+        <version>${revision}</version>
+    </parent>
+
+    <artifactId>child</artifactId>
+
+    <dependencies>
+        <dependency>
+            <groupId>junit</groupId>
+            <artifactId>junit</artifactId>
+            <version>4.1</version>
+        </dependency>
+    </dependencies>
+
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.cyclonedx</groupId>
+                <artifactId>cyclonedx-maven-plugin</artifactId>
+                <version>${cyclonedx-maven-plugin.version}</version>
+                <configuration>
+                    <externalReferences combine.children="append">
+                        <externalReference>
+                            <type>CHAT</type>
+                            <url>https://acme.com/child</url>
+                        </externalReference>
+                    </externalReferences>
+                </configuration>
+            </plugin>
+        </plugins>
+    </build>
+
+</project>
diff --git a/src/test/resources/external-reference/pom.xml b/src/test/resources/external-reference/pom.xml
new file mode 100644
index 0000000..2e4c93d
--- /dev/null
+++ b/src/test/resources/external-reference/pom.xml
@@ -0,0 +1,60 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+
+    <modelVersion>4.0.0</modelVersion>
+
+    <groupId>com.acme</groupId>
+    <artifactId>parent</artifactId>
+    <packaging>pom</packaging>
+    <version>${revision}</version>
+
+    <licenses>
+        <license>
+            <name>Apache-2.0</name>
+            <url>http://www.apache.org/licenses/LICENSE-2.0.txt</url>
+        </license>
+    </licenses>
+
+    <properties>
+        <revision>1.0.0</revision>
+        <maven.compiler.source>1.8</maven.compiler.source>
+        <maven.compiler.target>1.8</maven.compiler.target>
+        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+        <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
+    </properties>
+
+    <modules>
+        <module>child</module>
+    </modules>
+
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.cyclonedx</groupId>
+                <artifactId>cyclonedx-maven-plugin</artifactId>
+                <version>${cyclonedx-maven-plugin.version}</version>
+                <executions>
+                    <execution>
+                        <id>generate-sbom</id>
+                        <goals>
+                            <goal>makeAggregateBom</goal>
+                        </goals>
+                        <phase>verify</phase>
+                        <configuration>
+                            <outputFormat>json</outputFormat>
+                            <externalReferences>
+                                <externalReference>
+                                    <type>CHAT</type>
+                                    <url>https://acme.com/parent</url>
+                                </externalReference>
+                            </externalReferences>
+                        </configuration>
+                    </execution>
+                </executions>
+            </plugin>
+        </plugins>
+    </build>
+
+</project>