Skip detection of workspace projects in Yarn detector (#915)

* Potential fix for very large monorepos with yarn berry * Update detector version * Rename YarnLockVersion.V2 to YarnLockVersion.Berry * Update CONTRIBUTING.md * Add functional tests --------- Co-authored-by: OWA Framework <[email protected]>
microsoft · Nov 30, 2023 · 2455e9b · 2455e9b
1 parent 46cbc87
commit 2455e9b
Show file tree

Hide file tree

Showing 9 changed files with 169 additions and 29 deletions.
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -61,6 +61,6 @@ Analysis rulesets are defined in [analyzers.ruleset](analyzers.ruleset) and vali
 
 ### Testing
 
-L0s are defined in `MS.VS.Services.Governance.CD.*.L0.Tests`.
+L0s are defined in `Microsoft.ComponentDetection.*.Tests`.
 
 Verification tests are run on the sample projects defined in [microsoft/componentdetection-verification](https://github.com/microsoft/componentdetection-verification).
diff --git a/src/Microsoft.ComponentDetection.Detectors/yarn/Parsers/IYarnBlockFile.cs b/src/Microsoft.ComponentDetection.Detectors/yarn/Parsers/IYarnBlockFile.cs
@@ -7,7 +7,7 @@ public interface IYarnBlockFile : IEnumerable<YarnBlock>
     YarnLockVersion YarnLockVersion { get; set; }
 
     /// <summary>
-    /// The explicit version extracted from the `metadata` section of yarn lock files of <see cref="YarnLockVersion.V2"/>.
+    /// The explicit version extracted from the `metadata` section of yarn lock files of <see cref="YarnLockVersion.Berry"/>.
     /// </summary>
     string LockfileVersion { get; set; }
 }
diff --git a/src/Microsoft.ComponentDetection.Detectors/yarn/Parsers/YarnBlockFile.cs b/src/Microsoft.ComponentDetection.Detectors/yarn/Parsers/YarnBlockFile.cs
@@ -122,13 +122,13 @@ private void ReadVersionHeader()
             }
             else if (string.IsNullOrEmpty(this.fileLines[this.fileLineIndex]))
             {
-                // If the comment header does not specify V1, a V2 metadata block will follow a line break
+                // If the comment header does not specify V1, a Yarn Berry (>=v2) metadata block will follow a line break
                 if (this.IncrementIndex())
                 {
                     if (this.fileLines[this.fileLineIndex].StartsWith("__metadata:"))
                     {
                         this.VersionHeader = this.fileLines[this.fileLineIndex];
-                        this.YarnLockVersion = YarnLockVersion.V2;
+                        this.YarnLockVersion = YarnLockVersion.Berry;
 
                         var metadataBlock = this.ParseBlock();
 

diff --git a/src/Microsoft.ComponentDetection.Detectors/yarn/Parsers/YarnLockParser.cs b/src/Microsoft.ComponentDetection.Detectors/yarn/Parsers/YarnLockParser.cs
@@ -12,11 +12,13 @@ public class YarnLockParser : IYarnLockParser
 
     private const string Resolved = "resolved";
 
+    private const string Resolution = "resolution";
+
     private const string Dependencies = "dependencies";
 
     private const string OptionalDependencies = "optionalDependencies";
 
-    private static readonly List<YarnLockVersion> SupportedVersions = new List<YarnLockVersion> { YarnLockVersion.V1, YarnLockVersion.V2 };
+    private static readonly List<YarnLockVersion> SupportedVersions = new List<YarnLockVersion> { YarnLockVersion.V1, YarnLockVersion.Berry };
 
     private readonly ILogger<YarnLockParser> logger;
 
@@ -83,6 +85,18 @@ public YarnLockFile Parse(ISingleFileComponentRecorder singleFileComponentRecord
                 yarnEntry.Resolved = resolved;
             }
 
+            // Yarn berry renamed the "resolved" field to "resolution"
+            else if (block.Values.TryGetValue(Resolution, out var resolution))
+            {
+                yarnEntry.Resolved = resolution;
+
+                if (resolution.StartsWith(yarnEntry.Name + "@workspace:"))
+                {
+                    // Don't try to parse local workspace entries, which were never a part of the v1 lockfile
+                    continue;
+                }
+            }
+
             var dependencyBlock = block.Children.SingleOrDefault(x => string.Equals(x.Title, Dependencies, StringComparison.OrdinalIgnoreCase));
 
             if (dependencyBlock != null)

diff --git a/src/Microsoft.ComponentDetection.Detectors/yarn/YarnLockComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/yarn/YarnLockComponentDetector.cs
@@ -35,7 +35,7 @@ public YarnLockComponentDetector(
 
     public override IEnumerable<ComponentType> SupportedComponentTypes { get; } = new[] { ComponentType.Npm };
 
-    public override int Version => 7;
+    public override int Version => 8;
 
     public override IEnumerable<string> Categories => new[] { Enum.GetName(typeof(DetectorClass), DetectorClass.Npm) };
 

diff --git a/src/Microsoft.ComponentDetection.Detectors/yarn/YarnLockVersion.cs b/src/Microsoft.ComponentDetection.Detectors/yarn/YarnLockVersion.cs
@@ -4,5 +4,9 @@ public enum YarnLockVersion
 {
     Invalid = 0,
     V1 = 1,
-    V2 = 2,
+
+    // Berry is the public codename for the Yarn v2 rewrite.
+    // The lockfile has remained the same syntactically (YAML) since this rewrite,
+    // and all minor changes to the lockfile (up to Yarn v4/lockfile v8) have been irrelevant to CD
+    Berry = 2,
 }
diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/Yarn/YarnBerryTypeConverterTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/Yarn/YarnBerryTypeConverterTests.cs
@@ -41,6 +41,28 @@ public void Deserialize_WithLockfileWithMetadata_ShouldReturnLockfileWithMetadat
         result.Metadata.CacheKey.Should().Be("a");
     }
 
+    [TestMethod]
+    public void Deserialize_WithLockfileWithMetadata_ShouldNotReturnWorkspaceEntries()
+    {
+        var yaml = @"
+__metadata:
+  version: 8
+  cacheKey: a
+
+""internal-package@npm:0.0.0, internal-package@workspace:packages/internal-package"":
+  version: 0.0.0-use.local
+  resolution: ""internal-package@workspace:packages/internal-package""
+  languageName: unknown
+  linkType: soft
+";
+
+        var result = this.deserializer.Deserialize<YarnBerryLockfile>(yaml);
+
+        result.Should().NotBeNull();
+        result.Entries.Should().NotBeNull()
+            .And.HaveCount(1);
+    }
+
     [TestMethod]
     public void Deserialize_WithLockfileWithSingleEntry_ShouldReturnLockfileSingleEntry()
     {

diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/YarnBlockFileTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/YarnBlockFileTests.cs
@@ -169,7 +169,7 @@ public async Task BlockFileParserV2WithMetadataBlock_ParsesAsync()
 
         file.Should().BeEmpty();
         file.VersionHeader.Should().Be(yarnLockFileVersionString);
-        file.YarnLockVersion.Should().Be(YarnLockVersion.V2);
+        file.YarnLockVersion.Should().Be(YarnLockVersion.Berry);
     }
 
     [TestMethod]
@@ -208,7 +208,7 @@ public async Task BlockFileParserV2WithSingleBlock_ParsesAsync()
         block.Children.Single(x => x.Title == "block2").Values.Should().ContainKey("otherProperty");
         var value = block.Children.Single(x => x.Title == "block2").Values["otherProperty"];
         file.VersionHeader.Should().Be(yarnLockFileVersionString);
-        file.YarnLockVersion.Should().Be(YarnLockVersion.V2);
+        file.YarnLockVersion.Should().Be(YarnLockVersion.Berry);
     }
 
     [TestMethod]
@@ -247,7 +247,7 @@ public async Task BlockFileParserV2WithSingleBlock_ParsesWithQuotesAsync()
         block.Children.Single(x => x.Title == "block2").Values.Should().ContainKey("otherProperty");
         var value = block.Children.Single(x => x.Title == "block2").Values["otherProperty"];
         file.VersionHeader.Should().Be(yarnLockFileVersionString);
-        file.YarnLockVersion.Should().Be(YarnLockVersion.V2);
+        file.YarnLockVersion.Should().Be(YarnLockVersion.Berry);
     }
 
     [TestMethod]
@@ -294,6 +294,6 @@ public async Task BlockFileParserV2WithMultipleBlocks_ParsesAsync()
 
         file.Should().HaveCount(3);
         file.VersionHeader.Should().Be(yarnLockFileVersionString);
-        file.YarnLockVersion.Should().Be(YarnLockVersion.V2);
+        file.YarnLockVersion.Should().Be(YarnLockVersion.Berry);
     }
 }
diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/YarnParserTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/YarnParserTests.cs
@@ -51,7 +51,7 @@ public void YarnLockParser_CanParseV1LockFiles()
     [TestMethod]
     public void YarnLockParser_CanParseV2LockFiles()
     {
-        var yarnLockFileVersion = YarnLockVersion.V2;
+        var yarnLockFileVersion = YarnLockVersion.Berry;
 
         var parser = new YarnLockParser(this.loggerMock.Object);
 
@@ -80,23 +80,33 @@ public void YarnLockParser_ParsesEmptyFile()
     }
 
     [TestMethod]
-    public void YarnLockParser_ParsesBlocks()
+    public void YarnLockParser_V1_ParsesBlocks()
     {
         var yarnLockFileVersion = YarnLockVersion.V1;
 
         var parser = new YarnLockParser(this.loggerMock.Object);
 
         var blocks = new List<YarnBlock>
         {
-            this.CreateBlock("a@^1.0.0", "1.0.0", "https://a", new List<YarnBlock>
-            {
-                this.CreateDependencyBlock(new Dictionary<string, string> { { "xyz", "2" } }),
-            }),
-            this.CreateBlock("[email protected]", "2.4.6", "https://b", new List<YarnBlock>
-            {
-                this.CreateDependencyBlock(new Dictionary<string, string> { { "xyz", "2.4" }, { "a", "^1.0.0" } }),
-            }),
-            this.CreateBlock("xyz@2, [email protected]", "2.4.3", "https://xyz", Enumerable.Empty<YarnBlock>()),
+            this.CreateBlock(
+                "a@^1.0.0",
+                "1.0.0",
+                "https://a",
+                new List<YarnBlock>
+                {
+                    this.CreateDependencyBlock(new Dictionary<string, string> { { "xyz", "2" } }),
+                },
+                yarnLockFileVersion),
+            this.CreateBlock(
+                "[email protected]",
+                "2.4.6",
+                "https://b",
+                new List<YarnBlock>
+                {
+                    this.CreateDependencyBlock(new Dictionary<string, string> { { "xyz", "2.4" }, { "a", "^1.0.0" } }),
+                },
+                yarnLockFileVersion),
+            this.CreateBlock("xyz@2, [email protected]", "2.4.3", "https://xyz", Enumerable.Empty<YarnBlock>(), yarnLockFileVersion),
         };
 
         var blockFile = new Mock<IYarnBlockFile>();
@@ -105,14 +115,99 @@ public void YarnLockParser_ParsesBlocks()
 
         var file = parser.Parse(this.recorderMock.Object, blockFile.Object, this.loggerMock.Object);
 
-        file.LockVersion.Should().Be(YarnLockVersion.V1);
+        file.LockVersion.Should().Be(yarnLockFileVersion);
         file.Entries.Should().HaveCount(3);
 
         foreach (var entry in file.Entries)
         {
-            var block = blocks.Single(x => x.Values["resolved"] == entry.Resolved);
+            var block = blocks.Single(x => x.Values[this.GetResolvedEntryName(yarnLockFileVersion)] == entry.Resolved);
 
-            this.AssertBlockMatchesEntry(block, entry);
+            this.AssertBlockMatchesEntry(block, entry, yarnLockFileVersion);
+        }
+    }
+
+    [TestMethod]
+    public void YarnLockParser_Berry_ParsesBlocks()
+    {
+        var yarnLockFileVersion = YarnLockVersion.Berry;
+
+        var parser = new YarnLockParser(this.loggerMock.Object);
+
+        var blocks = new List<YarnBlock>
+        {
+            this.CreateBlock(
+                "a@^1.0.0",
+                "1.0.0",
+                "https://a",
+                new List<YarnBlock>
+                {
+                    this.CreateDependencyBlock(new Dictionary<string, string> { { "xyz", "2" } }),
+                },
+                yarnLockFileVersion),
+            this.CreateBlock(
+                "[email protected]",
+                "2.4.6",
+                "https://b",
+                new List<YarnBlock>
+                {
+                    this.CreateDependencyBlock(new Dictionary<string, string> { { "xyz", "2.4" }, { "a", "^1.0.0" } }),
+                },
+                yarnLockFileVersion),
+            this.CreateBlock("xyz@2, [email protected]", "2.4.3", "https://xyz", Enumerable.Empty<YarnBlock>(), yarnLockFileVersion),
+        };
+
+        var blockFile = new Mock<IYarnBlockFile>();
+        blockFile.Setup(x => x.YarnLockVersion).Returns(yarnLockFileVersion);
+        blockFile.Setup(x => x.GetEnumerator()).Returns(blocks.GetEnumerator());
+
+        var file = parser.Parse(this.recorderMock.Object, blockFile.Object, this.loggerMock.Object);
+
+        file.LockVersion.Should().Be(yarnLockFileVersion);
+        file.Entries.Should().HaveCount(3);
+
+        foreach (var entry in file.Entries)
+        {
+            var block = blocks.Single(x => x.Values[this.GetResolvedEntryName(yarnLockFileVersion)] == entry.Resolved);
+
+            this.AssertBlockMatchesEntry(block, entry, yarnLockFileVersion);
+        }
+    }
+
+    [TestMethod]
+    public void YarnLockParser_Berry_SkipsWorkspaceEntries()
+    {
+        var yarnLockFileVersion = YarnLockVersion.Berry;
+
+        var parser = new YarnLockParser(this.loggerMock.Object);
+
+        var blocks = new List<YarnBlock>
+        {
+            this.CreateBlock(
+                "internal-package@npm:0.0.0, internal-package@workspace:packages/internal-package",
+                "0.0.0-use.local",
+                "internal-package@workspace:packages/internal-package",
+                new List<YarnBlock>
+                {
+                    this.CreateDependencyBlock(new Dictionary<string, string> { { "xyz", "2" } }),
+                },
+                yarnLockFileVersion),
+            this.CreateBlock("xyz@2, [email protected]", "2.4.3", "https://xyz", Enumerable.Empty<YarnBlock>(), yarnLockFileVersion),
+        };
+
+        var blockFile = new Mock<IYarnBlockFile>();
+        blockFile.Setup(x => x.YarnLockVersion).Returns(yarnLockFileVersion);
+        blockFile.Setup(x => x.GetEnumerator()).Returns(blocks.GetEnumerator());
+
+        var file = parser.Parse(this.recorderMock.Object, blockFile.Object, this.loggerMock.Object);
+
+        file.LockVersion.Should().Be(yarnLockFileVersion);
+        file.Entries.Should().ContainSingle();
+
+        foreach (var entry in file.Entries)
+        {
+            var block = blocks.Single(x => x.Values[this.GetResolvedEntryName(yarnLockFileVersion)] == entry.Resolved);
+
+            this.AssertBlockMatchesEntry(block, entry, yarnLockFileVersion);
         }
     }
 
@@ -161,15 +256,15 @@ private YarnBlock CreateDependencyBlock(IDictionary<string, string> dependencies
         return block;
     }
 
-    private YarnBlock CreateBlock(string title, string version, string resolved, IEnumerable<YarnBlock> dependencies)
+    private YarnBlock CreateBlock(string title, string version, string resolved, IEnumerable<YarnBlock> dependencies, YarnLockVersion lockfileVersion = YarnLockVersion.V1)
     {
         var block = new YarnBlock
         {
             Title = title,
             Values =
             {
                 ["version"] = version,
-                ["resolved"] = resolved,
+                [this.GetResolvedEntryName(lockfileVersion)] = resolved,
             },
         };
 
@@ -181,7 +276,7 @@ private YarnBlock CreateBlock(string title, string version, string resolved, IEn
         return block;
     }
 
-    private void AssertBlockMatchesEntry(YarnBlock block, YarnEntry entry)
+    private void AssertBlockMatchesEntry(YarnBlock block, YarnEntry entry, YarnLockVersion lockfileVersion = YarnLockVersion.V1)
     {
         var componentName = block.Title.Split(',').Select(x => x.Trim()).First().Split('@')[0];
         var blockVersions = block.Title.Split(',').Select(x => x.Trim()).Select(x => x.Split('@')[1]);
@@ -194,7 +289,7 @@ private void AssertBlockMatchesEntry(YarnBlock block, YarnEntry entry)
         }
 
         entry.Version.Should().Be(block.Values["version"]);
-        entry.Resolved.Should().Be(block.Values["resolved"]);
+        entry.Resolved.Should().Be(block.Values[this.GetResolvedEntryName(lockfileVersion)]);
 
         var dependencies = block.Children.SingleOrDefault(x => x.Title == "dependencies");
 
@@ -206,4 +301,9 @@ private void AssertBlockMatchesEntry(YarnBlock block, YarnEntry entry)
             }
         }
     }
+
+    private string GetResolvedEntryName(YarnLockVersion lockfileVersion)
+    {
+        return lockfileVersion == YarnLockVersion.Berry ? "resolution" : "resolved";
+    }
 }