Fix malformed External ID generation, obtuse warning message and unmatched version resolution in Yarn detector #1290

gopannabd · 2024-11-05T15:06:07Z

Description

The root cause for the issue is JSON: why are forward slashes escaped?

This issue is surfaced by the dependency on Apache Commons : ReflectionToStringBuilder.toString(this, ToStringStyle.JSON_STYLE)

This returns a string with escaped forward slash in the JSON format. This is used in Stringable class of integration-common library which in turn is extended by ExternalID class in integration-bdio library.

When external ID is generated from name and version of entries in yarn.lock file, we use a factory method that generates a String. This same string is recorded and referred to again for potential matches as we build the dependency graph. When we do, we do not match because the String with escaped forward slash does not return the same component name for comparison.

To fix this, we have addressed it in the Yarn Detect code to remove the escape character for forward slash whenever we generate the external ID as String for comparison.

While the malformed ID is fixed, the warning message itself was not intuitive and is confusing. It is now reworded for better clarity to mean that the entries in the yarn.lock file that have not so far been resolved through package.json files, could not be resolved with any standard NPM packages either.

Additionally, the default workflow now provides advanced version resolution for unmatched components, if any, using the root yarn lock file. This addresses the requirement of IDETECT-4342 indirectly by resolving the warning messages of unresolved versions when default Yarn detector is used. Yarn 4 is support is unaffected.

Github Issues

IDETECT-4447, IDETECT-4314, IDETECT-4342

gopannabd · 2024-11-07T13:52:44Z

...e/src/main/java/com/blackduck/integration/detectable/detectables/yarn/YarnLockExtractor.java

@@ -46,7 +46,7 @@ public Extraction extract(File projectDir, File yarnLockFile, File rootPackageJs
            YarnLock yarnLock = readYarnLock(yarnLockFile);

            YarnResult yarnResult;
-            if (yarnLockOptions.getMonorepoMode()) {
+            if (yarnLockOptions.getYarnIgnoreAllWorkspacesMode()) {


Renamed for consistency.

gopannabd · 2024-11-07T13:52:51Z

...ble/src/main/java/com/blackduck/integration/detectable/detectables/yarn/YarnLockOptions.java

@@ -8,17 +8,17 @@ public class YarnLockOptions {
    private final EnumListFilter<YarnDependencyType> yarnDependencyTypeFilter;
    private final List<String> excludedWorkspaceNamePatterns;
    private final List<String> includedWorkspaceNamePatterns;
-    private final Boolean monorepoMode;
+    private final Boolean yarnIgnoreAllWorkspacesMode;


Renamed for consistency.

gopannabd · 2024-11-07T13:52:57Z

...ble/src/main/java/com/blackduck/integration/detectable/detectables/yarn/YarnLockOptions.java


    public YarnLockOptions(
            EnumListFilter<YarnDependencyType> yarnDependencyTypeFilter, 
            List<String> excludedWorkspaceNamePatterns, 
            List<String> includedWorkspaceNamePatterns,
-            Boolean monorepoMode) {
+            Boolean yarnIgnoreAllWorkspacesMode) {


Renamed for consistency.

gopannabd · 2024-11-07T13:53:01Z

...ble/src/main/java/com/blackduck/integration/detectable/detectables/yarn/YarnLockOptions.java

        this.yarnDependencyTypeFilter = yarnDependencyTypeFilter;
        this.excludedWorkspaceNamePatterns = excludedWorkspaceNamePatterns;
        this.includedWorkspaceNamePatterns = includedWorkspaceNamePatterns;
-        this.monorepoMode = monorepoMode;
+        this.yarnIgnoreAllWorkspacesMode = yarnIgnoreAllWorkspacesMode;


Renamed for consistency.

gopannabd · 2024-11-07T13:53:06Z

...ble/src/main/java/com/blackduck/integration/detectable/detectables/yarn/YarnLockOptions.java

@@ -33,7 +33,7 @@ public List<String> getIncludedWorkspaceNamePatterns() {
        return includedWorkspaceNamePatterns;
    }

-    public Boolean getMonorepoMode() {
-        return monorepoMode;
+    public Boolean getYarnIgnoreAllWorkspacesMode() {


Renamed for consistency.

gopannabd · 2024-11-07T13:57:05Z

...ble/src/main/java/com/blackduck/integration/detectable/detectables/yarn/YarnTransformer.java

@@ -39,7 +37,7 @@ public class YarnTransformer {
    private final Logger logger = LoggerFactory.getLogger(this.getClass());
    public static final String STRING_ID_NAME_VERSION_SEPARATOR = "@";
    private final ExternalIdFactory externalIdFactory;
-    private final Set<LazyId> unMatchedDependencies = new HashSet<>();
+    private final Map<LazyId, Optional<NameVersion>> unMatchedDependencies = new HashMap<>();


Used a map to retain NameVersion corresponding to LazyID to avoid lossy reconversion later.

gopannabd · 2024-11-07T13:57:48Z

...ble/src/main/java/com/blackduck/integration/detectable/detectables/yarn/YarnTransformer.java

@@ -50,10 +48,10 @@ public YarnTransformer(ExternalIdFactory externalIdFactory, EnumListFilter<YarnD
    public List<CodeLocation> generateCodeLocations(YarnLockResult yarnLockResult, List<NameVersion> externalDependencies)
        throws MissingExternalIdException {
        List<CodeLocation> codeLocations = new LinkedList<>();
-        logger.debug("Adding root dependencies for project: {}:{}", yarnLockResult.getRootPackageJson().getNameString(), yarnLockResult.getRootPackageJson().getVersionString());
+        logger.debug("Adding root dependencies for project: {}:{}, externalDeps: {}", yarnLockResult.getRootPackageJson().getNameString(), yarnLockResult.getRootPackageJson().getVersionString(), externalDependencies.size());


Additional data in debug logger on external dependencies available.

gopannabd · 2024-11-07T13:58:48Z

...ble/src/main/java/com/blackduck/integration/detectable/detectables/yarn/YarnTransformer.java

        LazyBuilderMissingExternalIdHandler lazyBuilderHandler = getLazyBuilderHandler(externalDependencies);
        ExternalIdDependencyGraphBuilder rootGraphBuilder = new ExternalIdDependencyGraphBuilder();
-        addRootDependenciesForProjectOrWorkspace(yarnLockResult, yarnLockResult.getRootPackageJson(), rootGraphBuilder);
+        addRootDependenciesForProject(yarnLockResult, yarnLockResult.getRootPackageJson(), rootGraphBuilder);


Method addRootDependenciesForProject(...) belongs to 'ignore all workspaces' workflow.

gopannabd · 2024-11-07T13:59:16Z

...ble/src/main/java/com/blackduck/integration/detectable/detectables/yarn/YarnTransformer.java

@@ -83,29 +81,33 @@ private DependencyGraph buildGraphForProjectOrWorkspace(
        List<NameVersion> externalDependencies
    ) throws MissingExternalIdException {
        LazyExternalIdDependencyGraphBuilder graphBuilder = new LazyExternalIdDependencyGraphBuilder();
-        addRootNodesToGraph(graphBuilder, projectOrWorkspacePackageJson, yarnLockResult.getWorkspaceData());
+        addRootDependenciesForProjectOrWorkspace(graphBuilder, projectOrWorkspacePackageJson, yarnLockResult.getWorkspaceData());


Refactored code by removing need for addRootNodesToGraph(...)

gopannabd · 2024-11-07T14:00:02Z

...ble/src/main/java/com/blackduck/integration/detectable/detectables/yarn/YarnTransformer.java

        for (YarnLockDependency dependency : entry.getDependencies()) {
            if (!isWorkspace(yarnLockResult.getWorkspaceData(), dependency)) {
                LazyId stringDependencyId = generateComponentDependencyId(dependency.getName(), dependency.getVersion());

                if (yarnDependencyTypeFilter.shouldInclude(YarnDependencyType.NON_PRODUCTION) || !dependency.isOptional()) {
+                    yarnDependencies.put(stringDependencyId, new NameVersion(dependency.getName(), entry.getVersion()));


Retained NameVersion pairs for LazyId.

gopannabd · 2024-11-07T14:01:09Z

...ble/src/main/java/com/blackduck/integration/detectable/detectables/yarn/YarnTransformer.java

-                    unMatchedDependencies.add(dependencyId);
+                Optional<NameVersion> nameVersionOptional;
+                if (!unMatchedDependencies.containsKey(dependencyId)) {
+                    String dependencyIdClean = dependencyId.toString().replace("\\/","/");


Removed escape character from string of dependency ID before use to reconvert to NameVersion.

gopannabd · 2024-11-07T14:01:52Z

...ble/src/main/java/com/blackduck/integration/detectable/detectables/yarn/YarnTransformer.java

+                if (!unMatchedDependencies.containsKey(dependencyId)) {
+                    String dependencyIdClean = dependencyId.toString().replace("\\/","/");
+                    nameVersionOptional = getNameVersion(dependencyIdClean);
+                    logger.warn("No standard NPM package identification details are available for '{}' from the yarn.lock file", dependencyIdClean);


Improved warning message about lack of standard NPM details to resolve remaining unmatched components.

gopannabd · 2024-11-07T14:02:37Z

...ble/src/main/java/com/blackduck/integration/detectable/detectables/yarn/YarnTransformer.java

+                    logger.warn("No standard NPM package identification details are available for '{}' from the yarn.lock file", dependencyIdClean);
+                    unMatchedDependencies.put(dependencyId, nameVersionOptional);
+                } else {
+                    nameVersionOptional = unMatchedDependencies.get(dependencyId);


If matched, then use the retained NameVersion from map.

gopannabd · 2024-11-07T14:03:08Z

...ble/src/main/java/com/blackduck/integration/detectable/detectables/yarn/YarnTransformer.java

                }
-                lazilyGeneratedExternalId = generateComponentExternalId(dependencyId);
-                return lazilyGeneratedExternalId;
+                return generateComponentExternalId(nameVersionOptional.get().getName(), nameVersionOptional.get().getVersion());


ExternalID generated from NameVersion avoids escape character issue.

gopannabd · 2024-11-07T14:04:28Z

...ble/src/main/java/com/blackduck/integration/detectable/detectables/yarn/YarnTransformer.java

-            Optional<YarnWorkspace> dependencyWorkspace = workspaceData.lookup(rootDependency.getKey(), rootDependency.getValue());
-            if (dependencyWorkspace.isPresent()) {
-                logger.trace("Omitting dependency {}/{} because it's a workspace", rootDependency.getKey(), rootDependency.getValue());
+    private LazyBuilderMissingExternalIdHandler getLazyBuilderHandler(List<NameVersion> externalDependencies, Map<LazyId, NameVersion> yarnLockDependencies) {


Separate method for default Yarn detector.

gopannabd · 2024-11-07T14:05:08Z

...ble/src/main/java/com/blackduck/integration/detectable/detectables/yarn/YarnTransformer.java

@@ -210,42 +212,68 @@ private LazyBuilderMissingExternalIdHandler getLazyBuilderHandler(List<NameVersi
            if (externalId.isPresent()) {


Separate method for ignore-all-workspaces Yarn detector workflow.

gopannabd · 2024-11-07T14:07:49Z

...ble/src/main/java/com/blackduck/integration/detectable/detectables/yarn/YarnTransformer.java

+        };
+    }
+
+    private Optional<NameVersion> getNameVersion(String dependencyIdString) {


Converts given dependency ID string to NameVersion pair object.

gopannabd · 2024-11-07T14:11:06Z

...ble/src/main/java/com/blackduck/integration/detectable/detectables/yarn/YarnTransformer.java

    }

-    private void addRootNodesToGraph(LazyExternalIdDependencyGraphBuilder graphBuilder, NullSafePackageJson projectOrWorkspacePackageJson, YarnWorkspaces workspaceData) {
-        populateGraphWithRootDependencies(graphBuilder, projectOrWorkspacePackageJson, workspaceData);
+    private void addRootDependenciesForProject(YarnLockResult yarnLockResult, NullSafePackageJson projectPackageJson, LazyExternalIdDependencyGraphBuilder graphBuilder) throws MissingExternalIdException {


Method refactored.

gopannabd · 2024-11-07T14:11:17Z

...ble/src/main/java/com/blackduck/integration/detectable/detectables/yarn/YarnTransformer.java

    }

-    private void populateGraphWithRootDependencies(LazyExternalIdDependencyGraphBuilder graphBuilder, NullSafePackageJson rootPackageJson, YarnWorkspaces workspaceData) {
+    private void addRootDependenciesForProjectOrWorkspace(LazyExternalIdDependencyGraphBuilder graphBuilder, NullSafePackageJson rootPackageJson, YarnWorkspaces workspaceData) {


Method refactored.

gopannabd · 2024-11-07T14:11:42Z

...ble/src/main/java/com/blackduck/integration/detectable/detectables/yarn/YarnTransformer.java

@@ -266,14 +294,10 @@ private void addRootDependenciesToGraph(LazyExternalIdDependencyGraphBuilder gra
    }

    private LazyId generateComponentDependencyId(String name, String version) {
-        return LazyId.fromString(name + STRING_ID_NAME_VERSION_SEPARATOR + version);
+        return LazyId.fromNameAndVersion(name, version);


Switch to NameVersion method to generate external ID.

gopannabd · 2024-11-07T14:11:55Z

src/main/java/com/blackduck/integration/detect/configuration/DetectProperties.java

@@ -1715,7 +1715,7 @@ private DetectProperties() {
            .setGroups(DetectGroup.PATHS, DetectGroup.GLOBAL)
            .build();

-    public static final BooleanProperty DETECT_YARN_MONOREPO_MODE =
+    public static final BooleanProperty DETECT_YARN_IGNORE_ALL_WORKSPACES_MODE =


Renamed for consistency.

gopannabd · 2024-11-07T14:12:04Z

src/main/java/com/blackduck/integration/detect/configuration/DetectableOptionFactory.java

@@ -284,12 +284,12 @@ public SbtDetectableOptions createSbtDetectableOptions() {

    public YarnLockOptions createYarnLockOptions() {
        Set<YarnDependencyType> excludedDependencyTypes = detectConfiguration.getValue(DetectProperties.DETECT_YARN_DEPENDENCY_TYPES_EXCLUDED).representedValueSet();
-        Boolean monorepoMode = detectConfiguration.getValue(DetectProperties.DETECT_YARN_MONOREPO_MODE);


Renamed for consistency.

gopannabd · 2024-11-07T14:12:13Z

src/main/java/com/blackduck/integration/detect/configuration/DetectableOptionFactory.java

        EnumListFilter<YarnDependencyType> yarnDependencyTypeFilter = EnumListFilter.fromExcluded(excludedDependencyTypes);

        List<String> excludedWorkspaces = detectConfiguration.getValue(DetectProperties.DETECT_YARN_EXCLUDED_WORKSPACES);
        List<String> includedWorkspaces = detectConfiguration.getValue(DetectProperties.DETECT_YARN_INCLUDED_WORKSPACES);
-        return new YarnLockOptions(yarnDependencyTypeFilter, excludedWorkspaces, includedWorkspaces, monorepoMode);
+        return new YarnLockOptions(yarnDependencyTypeFilter, excludedWorkspaces, includedWorkspaces, yarnIgnoreAllWorkspacesMode);


Renamed for consistency.

...ble/src/main/java/com/blackduck/integration/detectable/detectables/yarn/YarnTransformer.java

Replaced string constants with existing enum references.

gopannabd · 2024-11-19T20:52:51Z

@andrian-sevastyanov @devmehtabd Please review the last committed changes.

andrian-sevastyanov · 2024-11-20T18:18:20Z

detectable/src/main/java/com/blackduck/integration/detectable/detectables/yarn/Version.java

+        @Override
+        public int compareTo(Version o) {
+            if (this.major == o.major
+                    && this.minor == o.minor) {


Are we missing the patch comparison here or is this for some reason purposeful?

gopannabd added 3 commits November 4, 2024 09:18

Fix for IDETECT-4447

737c08f

Removed unused line.

7b41a53

Removed unused line.

2a0e096

gopannabd self-assigned this Nov 5, 2024

Extended version look up when Yarn detector is run in default mode.

8200cfb

gopannabd changed the title ~~Idetect 4447 test~~ Fix malformed External ID generation, obtuse warning message and unmatched version resolution in Yarn detector Nov 7, 2024

gopannabd commented Nov 7, 2024

View reviewed changes

Removed unwanted loggers.

858ef47

gopannabd commented Nov 7, 2024

View reviewed changes

gopannabd requested review from dterrysynopsys, shantyk, niravrsynopsys, devmehtabd and andrian-sevastyanov November 7, 2024 14:13

gopannabd marked this pull request as ready for review November 7, 2024 14:16

andrian-sevastyanov reviewed Nov 7, 2024

View reviewed changes

...ble/src/main/java/com/blackduck/integration/detectable/detectables/yarn/YarnTransformer.java Outdated Show resolved Hide resolved

gopannabd added 2 commits November 7, 2024 13:14

Merge branch 'master' into idetect-4447-test

5620859

Added external dependency look up as fall back for default workflow.

4ae6fcb

Replaced string constants with existing enum references.

andrian-sevastyanov approved these changes Nov 7, 2024

View reviewed changes

Updated tests and modified reverse Yarn version resolution.

eaacb2f

devmehtabd approved these changes Nov 14, 2024

View reviewed changes

Updated tests and modified reverse Yarn version resolution.

33f8f2a

andrian-sevastyanov reviewed Nov 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix malformed External ID generation, obtuse warning message and unmatched version resolution in Yarn detector #1290

Fix malformed External ID generation, obtuse warning message and unmatched version resolution in Yarn detector #1290

gopannabd commented Nov 5, 2024 •

edited

Loading

gopannabd Nov 7, 2024

gopannabd Nov 7, 2024

gopannabd Nov 7, 2024

gopannabd Nov 7, 2024

gopannabd Nov 7, 2024

gopannabd Nov 7, 2024 •

edited

Loading

gopannabd Nov 7, 2024

gopannabd Nov 7, 2024

gopannabd Nov 7, 2024

gopannabd Nov 7, 2024

gopannabd Nov 7, 2024

gopannabd Nov 7, 2024

gopannabd Nov 7, 2024 •

edited

Loading

gopannabd Nov 7, 2024

gopannabd Nov 7, 2024

gopannabd Nov 7, 2024

gopannabd Nov 7, 2024

gopannabd Nov 7, 2024

gopannabd Nov 7, 2024

gopannabd Nov 7, 2024

gopannabd Nov 7, 2024

gopannabd Nov 7, 2024

gopannabd Nov 7, 2024

gopannabd commented Nov 19, 2024

andrian-sevastyanov Nov 20, 2024

		@@ -210,42 +212,68 @@ private LazyBuilderMissingExternalIdHandler getLazyBuilderHandler(List<NameVersi
		if (externalId.isPresent()) {

Fix malformed External ID generation, obtuse warning message and unmatched version resolution in Yarn detector #1290

Are you sure you want to change the base?

Fix malformed External ID generation, obtuse warning message and unmatched version resolution in Yarn detector #1290

Conversation

gopannabd commented Nov 5, 2024 • edited Loading

Description

Github Issues

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gopannabd Nov 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gopannabd Nov 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gopannabd commented Nov 19, 2024

Choose a reason for hiding this comment

gopannabd commented Nov 5, 2024 •

edited

Loading

gopannabd Nov 7, 2024 •

edited

Loading

gopannabd Nov 7, 2024 •

edited

Loading