Add support for Windows artifacts to Deliverables Analyzer #4137
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,7 +20,6 @@ | |
| import com.github.packageurl.MalformedPackageURLException; | ||
| import com.github.packageurl.PackageURL; | ||
| import com.github.packageurl.PackageURLBuilder; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import org.jboss.pnc.api.deliverablesanalyzer.dto.Artifact; | ||
| import org.jboss.pnc.api.deliverablesanalyzer.dto.ArtifactType; | ||
|
|
@@ -29,6 +28,7 @@ | |
| import org.jboss.pnc.api.deliverablesanalyzer.dto.FinderResult; | ||
| import org.jboss.pnc.api.deliverablesanalyzer.dto.LicenseInfo; | ||
| import org.jboss.pnc.api.deliverablesanalyzer.dto.MavenArtifact; | ||
| import org.jboss.pnc.api.deliverablesanalyzer.dto.WindowsArtifact; | ||
| import org.jboss.pnc.api.dto.Request; | ||
| import org.jboss.pnc.api.enums.DeliverableAnalyzerReportLabel; | ||
| import org.jboss.pnc.api.enums.LabelOperation; | ||
|
|
@@ -48,7 +48,6 @@ | |
| import org.jboss.pnc.enums.RepositoryType; | ||
| import org.jboss.pnc.facade.OperationsManager; | ||
| import org.jboss.pnc.facade.deliverables.api.AnalysisResult; | ||
| import org.jboss.pnc.mapper.api.ArtifactMapper; | ||
| import org.jboss.pnc.mapper.api.DeliverableAnalyzerOperationMapper; | ||
| import org.jboss.pnc.model.Base32LongID; | ||
|
|
@@ -95,6 +94,7 @@ | |
| import java.util.HashSet; | ||
| import java.util.Iterator; | ||
| import java.util.List; | ||
| import java.util.Locale; | ||
| import java.util.Map; | ||
| import java.util.Objects; | ||
| import java.util.Optional; | ||
|
|
@@ -543,9 +543,8 @@ private org.jboss.pnc.model.Artifact findOrCreateNotFoundArtifact( | |
| private org.jboss.pnc.model.Artifact createArtifact( | ||
| org.jboss.pnc.model.Artifact artifact, | ||
| TargetRepository targetRepo) { | ||
| artifact.setTargetRepository(targetRepo); | ||
| artifact.setPurl(createGenericPurl(artifact.getFilename(), artifact.getSha256())); | ||
| org.jboss.pnc.model.Artifact savedArtifact = artifactRepository.save(artifact); | ||
| targetRepo.getArtifacts().add(savedArtifact); | ||
| return savedArtifact; | ||
|
|
@@ -567,17 +566,12 @@ private org.jboss.pnc.model.Artifact mapBrewArtifact( | |
| String nvr, | ||
| TargetRepository targetRepository, | ||
| User user) { | ||
| org.jboss.pnc.model.Artifact.Builder builder = mapArtifact(artifact, user); | ||
| builder.identifier(createIdentifier(artifact)); | ||
| builder.filename(createFileName(artifact)); | ||
| builder.deployPath(createDeployPath(artifact)); | ||
| builder.originUrl(createBrewOriginURL(artifact, nvr)); | ||
| builder.purl(createPURL(artifact)); | ||
| builder.targetRepository(targetRepository); | ||
|
|
||
| return builder.build(); | ||
|
|
@@ -603,49 +597,82 @@ private org.jboss.pnc.model.Artifact.Builder mapArtifact(Artifact artifact, User | |
| return builder; | ||
| } | ||
|
|
||
| private static String createDeployPath(Artifact artifact) { | ||
| String filename = createFileName(artifact); | ||
| String deployPath; | ||
|
|
||
| if (artifact instanceof MavenArtifact) { | ||
| MavenArtifact mavenArtifact = (MavenArtifact) artifact; | ||
| deployPath = "/" + mavenArtifact.getGroupId().replace('.', '/') + "/" + mavenArtifact.getArtifactId() + "/" | ||
| + mavenArtifact.getVersion() + "/" + filename; | ||
| } else if (artifact instanceof WindowsArtifact) { | ||
| deployPath = "/" + filename; | ||
| } else { | ||
| throw new IllegalArgumentException("Unsupported artifact type: " + artifact.getArtifactType()); | ||
| } | ||
|
|
||
| return deployPath; | ||
| } | ||
|
|
||
| private static String createFileName(Artifact artifact) { | ||
| String filename; | ||
|
|
||
| if (artifact instanceof MavenArtifact) { | ||
| MavenArtifact mavenArtifact = (MavenArtifact) artifact; | ||
| filename = mavenArtifact.getArtifactId() + "-" + mavenArtifact.getVersion(); | ||
| if (!Strings.isEmpty(mavenArtifact.getClassifier())) { | ||
| filename += "-" + mavenArtifact.getClassifier(); | ||
| } | ||
| filename += "." + mavenArtifact.getType(); | ||
| } else if (artifact instanceof WindowsArtifact) { | ||
| WindowsArtifact windowsArtifact = (WindowsArtifact) artifact; | ||
| filename = windowsArtifact.getFilename(); | ||
| } else { | ||
| throw new IllegalArgumentException("Unsupported artifact type: " + artifact.getArtifactType()); | ||
| } | ||
|
|
||
| return filename; | ||
| } | ||
|
|
||
| private String createBrewOriginURL(Artifact artifact, String nvr) { | ||
| String brewContentUrl = globalConfig.getBrewContentUrl(); | ||
| Matcher matcher = NVR_PATTERN.matcher(nvr); | ||
| if (!matcher.matches()) { | ||
| throw new IllegalArgumentException("NVR " + nvr + " does not match expected format."); | ||
| } | ||
| String name = matcher.group(1); | ||
| String version = matcher.group(2); | ||
| String release = matcher.group(3); | ||
| return brewContentUrl + "/" + name + "/" + version + "/" + release + "/" | ||
| + artifact.getArtifactType().name().toLowerCase(Locale.ENGLISH) + createDeployPath(artifact); | ||
| } | ||
|
|
||
| private String createPURL(Artifact artifact) { | ||
| try { | ||
| PackageURLBuilder purlBuilder; | ||
| if (artifact instanceof MavenArtifact) { | ||
| MavenArtifact mavenArtifact = (MavenArtifact) artifact; | ||
| purlBuilder = PackageURLBuilder.aPackageURL() | ||
| .withType(PackageURL.StandardTypes.MAVEN) | ||
| .withNamespace(mavenArtifact.getGroupId()) | ||
| .withName(mavenArtifact.getArtifactId()) | ||
| .withVersion(mavenArtifact.getVersion()) | ||
| .withQualifier( | ||
| "type", | ||
| StringUtils.isEmpty(mavenArtifact.getType()) ? "jar" : mavenArtifact.getType()); | ||
|
|
||
| if (!StringUtils.isEmpty(mavenArtifact.getClassifier())) { | ||
| purlBuilder.withQualifier("classifier", mavenArtifact.getClassifier()); | ||
| } | ||
|
|
||
| } else if (artifact instanceof WindowsArtifact) { | ||
| WindowsArtifact windowsArtifact = (WindowsArtifact) artifact; | ||
| purlBuilder = PackageURLBuilder.aPackageURL() | ||
| .withType(PackageURL.StandardTypes.GENERIC) | ||
| .withName(windowsArtifact.getName()) | ||
| .withVersion(windowsArtifact.getVersion()); | ||
|
There was a problem hiding this comment. @pkocandr Should the namespace be empty? Type is |
||
| } else { | ||
| throw new IllegalArgumentException("Unsupported artifact type: " + artifact.getArtifactType()); | ||
| } | ||
| return purlBuilder.build().toString(); | ||
| } catch (MalformedPackageURLException e) { | ||
|
|
@@ -673,17 +700,25 @@ private String createGenericPurl(String filename, String sha256) { | |
| } | ||
| } | ||
|
|
||
| private String createIdentifier(Artifact artifact) { | ||
| if (artifact instanceof MavenArtifact) { | ||
| MavenArtifact mavenArtifact = (MavenArtifact) artifact; | ||
| return Arrays | ||
| .asList( | ||
| mavenArtifact.getGroupId(), | ||
| mavenArtifact.getArtifactId(), | ||
| mavenArtifact.getType(), | ||
| mavenArtifact.getVersion(), | ||
| mavenArtifact.getClassifier()) | ||
| .stream() | ||
| .filter(Objects::nonNull) | ||
| .collect(Collectors.joining(":")); | ||
| } else if (artifact instanceof WindowsArtifact) { | ||
| WindowsArtifact windowsArtifact = (WindowsArtifact) artifact; | ||
| return String.join("-", windowsArtifact.getName(), windowsArtifact.getVersion()); | ||
|
There was a problem hiding this comment. @pkocandr For Windows, I just chose |
||
| } else { | ||
| throw new IllegalArgumentException("Unsupported artifact type: " + artifact.getArtifactType()); | ||
| } | ||
| } | ||
|
|
||
| private TargetRepository getDistributionRepository(String distURL) { | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pkocandr I am not sure how the deploy path would be if we got that artifact via brew pull (is it even possible?) and if this matches.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I understand the code, this isn't the Maven repository path, e.g., /maven2. This is the original Brew path (see
createBrewOriginURL()).Maven artifacts start with /maven and Windows artifacts start with /win, so I think it just uses the artifact type to start the path. For Windows, since there is no group identifier, etc., the rest of the path is just the filename itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We only have support to pull Maven dependecies from Brew, so it would only be possible if a Windows "artifact" was deployed as Maven artifact in a Maven repo probably as a result of a native build. The deploy path would then normally forllow the groupId artifactId coordinates as any other Maven artifact.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But, the whole point of this code was is add support for Windows artifacts in addition to Maven artifacts. So, when you say that we only support pulling Maven dependencies from Brew I am not sure that I understand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am under the impression that this change only applies to Deliverables Analyzer and that it shouldn't matter if other parts of PNC support Maven only.