From 03c5b5650555eb934279f9c3e1394229563c610b Mon Sep 17 00:00:00 2001 From: Coby Allred Date: Tue, 14 Feb 2023 09:33:35 -0800 Subject: [PATCH 1/5] Initial set of skipped pkg logs --- .../DependencyGraph/ComponentRecorder.cs | 46 ++++++++++++++++++- .../IComponentRecorder.cs | 8 ++++ .../cocoapods/PodComponentDetector.cs | 1 + .../dockerfile/DockerfileComponentDetector.cs | 3 +- .../go/GoComponentDetector.cs | 9 +++- .../ivy/IvyDetector.cs | 1 + .../maven/MavenCommandService.cs | 1 + .../npm/NpmComponentDetector.cs | 1 + .../nuget/NuGetComponentDetector.cs | 1 + .../vcpkg/VcpkgComponentDetector.cs | 3 +- .../yarn/IYarnLockParser.cs | 2 +- .../yarn/Parsers/YarnLockParser.cs | 3 +- .../yarn/YarnLockComponentDetector.cs | 2 +- .../yarn/YarnLockFileFactory.cs | 4 +- .../Services/DetectorProcessingService.cs | 27 +++++++++++ 15 files changed, 102 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Common/DependencyGraph/ComponentRecorder.cs b/src/Microsoft.ComponentDetection.Common/DependencyGraph/ComponentRecorder.cs index 268eb6564..b1205b959 100644 --- a/src/Microsoft.ComponentDetection.Common/DependencyGraph/ComponentRecorder.cs +++ b/src/Microsoft.ComponentDetection.Common/DependencyGraph/ComponentRecorder.cs @@ -62,6 +62,24 @@ public IEnumerable GetDetectedComponents() return detectedComponents; } + public IEnumerable GetSkippedComponents() + { + IEnumerable skippedComponents; + if (this.singleFileRecorders == null) + { + return Enumerable.Empty(); + } + + skippedComponents = this.singleFileRecorders + .Select(singleFileRecorder => singleFileRecorder.GetSkippedComponents().Keys) + .SelectMany(x => x) + .GroupBy(x => x) + .Select(grouping => grouping.First()) + .ToImmutableList(); + + return skippedComponents; + } + public ISingleFileComponentRecorder CreateSingleFileComponentRecorder(string location) { if (string.IsNullOrWhiteSpace(location)) @@ -90,16 +108,23 @@ internal DependencyGraph GetDependencyGraphForLocation(string location) return this.singleFileRecorders.Single(x => x.ManifestFileLocation == location).DependencyGraph; } - public sealed class SingleFileComponentRecorder : ISingleFileComponentRecorder + public class SingleFileComponentRecorder : ISingleFileComponentRecorder { private readonly ILogger log; private readonly ConcurrentDictionary detectedComponentsInternal = new ConcurrentDictionary(); + /// + /// Dictionary of components which had an error during parsing and a dummy data value that only allocates 1 byte. + /// + private readonly ConcurrentDictionary skippedComponentsInternal = new ConcurrentDictionary(); + private readonly ComponentRecorder recorder; private readonly object registerUsageLock = new object(); + private readonly object registerSkippedLock = new object(); + public SingleFileComponentRecorder(string location, ComponentRecorder recorder, bool enableManualTrackingOfExplicitReferences, ILogger log) { this.ManifestFileLocation = location; @@ -130,6 +155,12 @@ public IReadOnlyDictionary GetDetectedComponents() return this.detectedComponentsInternal; } + public IReadOnlyDictionary GetSkippedComponents() + { + // Should this be immutable? + return this.skippedComponentsInternal; + } + public void RegisterUsage( DetectedComponent detectedComponent, bool isExplicitReferencedDependency = false, @@ -173,6 +204,19 @@ public void RegisterUsage( } } + public void RegisterPackageParseFailure(string skippedComponent) + { + if (skippedComponent == null) + { + throw new ArgumentNullException(paramName: nameof(skippedComponent)); + } + + lock (this.registerSkippedLock) + { + _ = this.skippedComponentsInternal[skippedComponent] = default; + } + } + public void AddAdditionalRelatedFile(string relatedFilePath) { this.DependencyGraph.AddAdditionalRelatedFile(relatedFilePath); diff --git a/src/Microsoft.ComponentDetection.Contracts/IComponentRecorder.cs b/src/Microsoft.ComponentDetection.Contracts/IComponentRecorder.cs index ad009fda4..e9f346b18 100644 --- a/src/Microsoft.ComponentDetection.Contracts/IComponentRecorder.cs +++ b/src/Microsoft.ComponentDetection.Contracts/IComponentRecorder.cs @@ -8,6 +8,8 @@ public interface IComponentRecorder IEnumerable GetDetectedComponents(); + IEnumerable GetSkippedComponents(); + ISingleFileComponentRecorder CreateSingleFileComponentRecorder(string location); IReadOnlyDictionary GetDependencyGraphsByLocation(); @@ -35,6 +37,12 @@ void RegisterUsage( bool? isDevelopmentDependency = null, DependencyScope? dependencyScope = null); + /// + /// Register that a package was unable to be processed. + /// + /// Component version identifier. + void RegisterPackageParseFailure(string skippedComponent); + DetectedComponent GetComponent(string componentId); void AddAdditionalRelatedFile(string relatedFilePath); diff --git a/src/Microsoft.ComponentDetection.Detectors/cocoapods/PodComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/cocoapods/PodComponentDetector.cs index fd4246236..3caf716ed 100644 --- a/src/Microsoft.ComponentDetection.Detectors/cocoapods/PodComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/cocoapods/PodComponentDetector.cs @@ -229,6 +229,7 @@ private void ProcessPodfileLock( else { this.Logger.LogWarning($"Missing podspec declaration. podspec={dependency.Podspec}, version={dependency.PodVersion}"); + singleFileComponentRecorder.RegisterPackageParseFailure($"{dependency.Podspec} - {dependency.PodVersion}"); } } } diff --git a/src/Microsoft.ComponentDetection.Detectors/dockerfile/DockerfileComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/dockerfile/DockerfileComponentDetector.cs index dbd58175a..a9e2abd18 100644 --- a/src/Microsoft.ComponentDetection.Detectors/dockerfile/DockerfileComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/dockerfile/DockerfileComponentDetector.cs @@ -72,7 +72,7 @@ private Task ParseDockerFileAsync(string fileContents, string fileLocation, ISin return Task.CompletedTask; } - private DockerReference ProcessDockerfileConstruct(DockerfileConstruct construct, char escapeChar, Dictionary stageNameMap) + private DockerReference ProcessDockerfileConstruct(ISingleFileComponentRecorder singleFileComponentRecorder, DockerfileConstruct construct, char escapeChar, Dictionary stageNameMap) { try { @@ -100,6 +100,7 @@ private DockerReference ProcessDockerfileConstruct(DockerfileConstruct construct { this.Logger.LogError($"Failed to detect a DockerReference component, the component will not be registered. \n Error Message: <{e.Message}>"); this.Logger.LogException(e, isError: true, printException: true); + singleFileComponentRecorder.RegisterPackageParseFailure(); return null; } } diff --git a/src/Microsoft.ComponentDetection.Detectors/go/GoComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/go/GoComponentDetector.cs index af8c9cf0b..d696096f3 100644 --- a/src/Microsoft.ComponentDetection.Detectors/go/GoComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/go/GoComponentDetector.cs @@ -161,7 +161,9 @@ private void ParseGoModFile( } else { - this.Logger.LogWarning($"Line could not be parsed for component [{line.Trim()}]"); + var lineTrim = line.Trim(); + this.Logger.LogWarning($"Line could not be parsed for component [{lineTrim}]"); + singleFileComponentRecorder.RegisterPackageParseFailure(lineTrim); } } } @@ -200,7 +202,9 @@ private void ParseGoSumFile( } else { - this.Logger.LogWarning($"Line could not be parsed for component [{line.Trim()}]"); + var lineTrim = line.Trim(); + this.Logger.LogWarning($"Line could not be parsed for component [{lineTrim}]"); + singleFileComponentRecorder.RegisterPackageParseFailure(lineTrim); } } } @@ -261,6 +265,7 @@ private void PopulateDependencyGraph(string goGraphOutput, ISingleFileComponentR else { this.Logger.LogWarning($"Failed to parse components from relationship string {relationship}"); + componentRecorder.RegisterPackageParseFailure(relationship); } } } diff --git a/src/Microsoft.ComponentDetection.Detectors/ivy/IvyDetector.cs b/src/Microsoft.ComponentDetection.Detectors/ivy/IvyDetector.cs index 2268c28d7..f237abf29 100644 --- a/src/Microsoft.ComponentDetection.Detectors/ivy/IvyDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/ivy/IvyDetector.cs @@ -228,6 +228,7 @@ private void RegisterUsagesFromFile(ISingleFileComponentRecorder singleFileCompo else { this.Logger.LogWarning($"Dependency \"{component.Id}\" could not be resolved by Ivy, and so has not been recorded by Component Detection."); + singleFileComponentRecorder.RegisterPackageParseFailure(component.Id); } } } diff --git a/src/Microsoft.ComponentDetection.Detectors/maven/MavenCommandService.cs b/src/Microsoft.ComponentDetection.Detectors/maven/MavenCommandService.cs index c7cf20bee..0fd95dc9b 100644 --- a/src/Microsoft.ComponentDetection.Detectors/maven/MavenCommandService.cs +++ b/src/Microsoft.ComponentDetection.Detectors/maven/MavenCommandService.cs @@ -40,6 +40,7 @@ public async Task GenerateDependenciesFileAsync(ProcessRequest processRequest) { this.Logger.LogVerbose($"Mvn execution failed for pom file: {pomFile.Location}"); this.Logger.LogError(string.IsNullOrEmpty(result.StdErr) ? result.StdOut : result.StdErr); + processRequest.SingleFileComponentRecorder.RegisterPackageParseFailure(pomFile.Location); } } diff --git a/src/Microsoft.ComponentDetection.Detectors/npm/NpmComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/npm/NpmComponentDetector.cs index 3f01497a9..97e66833f 100644 --- a/src/Microsoft.ComponentDetection.Detectors/npm/NpmComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/npm/NpmComponentDetector.cs @@ -72,6 +72,7 @@ protected virtual bool ProcessIndividualPackageJTokens(string filePath, ISingleF if (!SemanticVersion.TryParse(version, out _)) { this.Logger.LogWarning($"Unable to parse version \"{version}\" for package \"{name}\" found at path \"{filePath}\". This may indicate an invalid npm package component and it will not be registered."); + singleFileComponentRecorder.RegisterPackageParseFailure($"{name} - {version}"); return false; } diff --git a/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetComponentDetector.cs index 037eb1122..fdb4931a7 100644 --- a/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetComponentDetector.cs @@ -118,6 +118,7 @@ private async Task ProcessFileAsync(ProcessRequest processRequest) if (!NuGetVersion.TryParse(version, out var parsedVer)) { this.Logger.LogInfo($"Version '{version}' from {stream.Location} could not be parsed as a NuGet version"); + singleFileComponentRecorder.RegisterPackageParseFailure(stream.Location); return; } diff --git a/src/Microsoft.ComponentDetection.Detectors/vcpkg/VcpkgComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/vcpkg/VcpkgComponentDetector.cs index b9eafcb41..6e1e534e4 100644 --- a/src/Microsoft.ComponentDetection.Detectors/vcpkg/VcpkgComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/vcpkg/VcpkgComponentDetector.cs @@ -37,7 +37,7 @@ protected override async Task OnFileFoundAsync(ProcessRequest processRequest, ID var singleFileComponentRecorder = processRequest.SingleFileComponentRecorder; var file = processRequest.ComponentStream; - this.Logger.LogInfo($"vcpkg detector found {file}"); + this.Logger.LogVerbose($"vcpkg detector found {file}"); var projectRootDirectory = Directory.GetParent(file.Location); if (this.projectRoots.Any(path => projectRootDirectory.FullName.StartsWith(path))) @@ -107,6 +107,7 @@ private async Task ParseSpdxFileAsync( catch (Exception) { this.Logger.LogWarning($"failed while handling {item.Name}"); + singleFileComponentRecorder.RegisterPackageParseFailure(item.Name); } } } diff --git a/src/Microsoft.ComponentDetection.Detectors/yarn/IYarnLockParser.cs b/src/Microsoft.ComponentDetection.Detectors/yarn/IYarnLockParser.cs index 088bcca6b..6336058ef 100644 --- a/src/Microsoft.ComponentDetection.Detectors/yarn/IYarnLockParser.cs +++ b/src/Microsoft.ComponentDetection.Detectors/yarn/IYarnLockParser.cs @@ -6,5 +6,5 @@ public interface IYarnLockParser { bool CanParse(YarnLockVersion yarnLockVersion); - YarnLockFile Parse(IYarnBlockFile fileLines, ILogger logger); + YarnLockFile Parse(ISingleFileComponentRecorder singleFileComponentRecorder, IYarnBlockFile fileLines, ILogger logger); } diff --git a/src/Microsoft.ComponentDetection.Detectors/yarn/Parsers/YarnLockParser.cs b/src/Microsoft.ComponentDetection.Detectors/yarn/Parsers/YarnLockParser.cs index 15c39a948..e44587840 100644 --- a/src/Microsoft.ComponentDetection.Detectors/yarn/Parsers/YarnLockParser.cs +++ b/src/Microsoft.ComponentDetection.Detectors/yarn/Parsers/YarnLockParser.cs @@ -30,7 +30,7 @@ public bool CanParse(YarnLockVersion yarnLockVersion) return SupportedVersions.Contains(yarnLockVersion); } - public YarnLockFile Parse(IYarnBlockFile fileLines, ILogger logger) + public YarnLockFile Parse(ISingleFileComponentRecorder singleFileComponentRecorder, IYarnBlockFile fileLines, ILogger logger) { if (fileLines == null) { @@ -70,6 +70,7 @@ public YarnLockFile Parse(IYarnBlockFile fileLines, ILogger logger) if (!block.Values.TryGetValue(VersionString, out var version)) { logger.LogWarning($"Failed to read a version for {yarnEntry.Name}. The entry will be skipped."); + singleFileComponentRecorder.RegisterPackageParseFailure(yarnEntry.Name); continue; } diff --git a/src/Microsoft.ComponentDetection.Detectors/yarn/YarnLockComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/yarn/YarnLockComponentDetector.cs index fda52e8af..41844daee 100644 --- a/src/Microsoft.ComponentDetection.Detectors/yarn/YarnLockComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/yarn/YarnLockComponentDetector.cs @@ -45,7 +45,7 @@ protected override async Task OnFileFoundAsync(ProcessRequest processRequest, ID try { - var parsed = await YarnLockFileFactory.ParseYarnLockFileAsync(file.Stream, this.Logger); + var parsed = await YarnLockFileFactory.ParseYarnLockFileAsync(singleFileComponentRecorder, file.Stream, this.Logger); this.DetectComponents(parsed, file.Location, singleFileComponentRecorder); } catch (Exception ex) diff --git a/src/Microsoft.ComponentDetection.Detectors/yarn/YarnLockFileFactory.cs b/src/Microsoft.ComponentDetection.Detectors/yarn/YarnLockFileFactory.cs index 740b74bf4..820eddbd9 100644 --- a/src/Microsoft.ComponentDetection.Detectors/yarn/YarnLockFileFactory.cs +++ b/src/Microsoft.ComponentDetection.Detectors/yarn/YarnLockFileFactory.cs @@ -11,7 +11,7 @@ public static class YarnLockFileFactory public static IList Parsers { get; } - public static async Task ParseYarnLockFileAsync(Stream file, ILogger logger) + public static async Task ParseYarnLockFileAsync(ISingleFileComponentRecorder singleFileComponentRecorder, Stream file, ILogger logger) { var blockFile = await YarnBlockFile.CreateBlockFileAsync(file); @@ -19,7 +19,7 @@ public static async Task ParseYarnLockFileAsync(Stream file, ILogg { if (parser.CanParse(blockFile.YarnLockVersion)) { - return parser.Parse(blockFile, logger); + return parser.Parse(singleFileComponentRecorder, blockFile, logger); } } diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorProcessingService.cs b/src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorProcessingService.cs index f1c2765e8..60ede4179 100644 --- a/src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorProcessingService.cs +++ b/src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorProcessingService.cs @@ -121,6 +121,33 @@ public async Task ProcessDetectorsAsync(IDetectionArgu var totalElapsedTime = stopwatch.Elapsed.TotalSeconds; this.LogTabularOutput(this.Logger, providerElapsedTime, totalElapsedTime); + // If there are components which are skipped due to connection or parsing + // errors, log them by detector. + var parseWarningShown = false; + foreach (var (_, recorder, detector) in results) + { + var skippedComponents = recorder.GetSkippedComponents(); + if (!skippedComponents.Any()) + { + continue; + } + + if (!parseWarningShown) + { + this.Logger.LogCreateLoggingGroup(); + this.Logger.LogWarning($"Some components or files were not detected due to parsing failures or connectivity issues."); + this.Logger.LogWarning($"Please review the logs above for more detailed information."); + parseWarningShown = true; + } + + this.Logger.LogCreateLoggingGroup(); + this.Logger.LogWarning($"Components skipped for {detector.Id} detector:"); + foreach (var component in skippedComponents) + { + this.Logger.LogWarning($"- {component}"); + } + } + this.Logger.LogCreateLoggingGroup(); this.Logger.LogInfo($"Detection time: {totalElapsedTime} seconds."); From c1cfb68634b85d33dbe12301ec1ea5e8f701c0f2 Mon Sep 17 00:00:00 2001 From: Coby Allred Date: Tue, 14 Feb 2023 13:37:27 -0800 Subject: [PATCH 2/5] Additional detector logging --- .../dockerfile/DockerfileComponentDetector.cs | 3 +-- .../linux/LinuxContainerDetector.cs | 6 ++++++ .../nuget/NuGetComponentDetector.cs | 1 + .../ruby/RubyComponentDetector.cs | 11 ++++++----- .../rust/RustCrateDetector.cs | 1 + .../yarn/YarnLockComponentDetector.cs | 6 ++++-- 6 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/dockerfile/DockerfileComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/dockerfile/DockerfileComponentDetector.cs index a9e2abd18..dbd58175a 100644 --- a/src/Microsoft.ComponentDetection.Detectors/dockerfile/DockerfileComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/dockerfile/DockerfileComponentDetector.cs @@ -72,7 +72,7 @@ private Task ParseDockerFileAsync(string fileContents, string fileLocation, ISin return Task.CompletedTask; } - private DockerReference ProcessDockerfileConstruct(ISingleFileComponentRecorder singleFileComponentRecorder, DockerfileConstruct construct, char escapeChar, Dictionary stageNameMap) + private DockerReference ProcessDockerfileConstruct(DockerfileConstruct construct, char escapeChar, Dictionary stageNameMap) { try { @@ -100,7 +100,6 @@ private DockerReference ProcessDockerfileConstruct(ISingleFileComponentRecorder { this.Logger.LogError($"Failed to detect a DockerReference component, the component will not be registered. \n Error Message: <{e.Message}>"); this.Logger.LogException(e, isError: true, printException: true); - singleFileComponentRecorder.RegisterPackageParseFailure(); return null; } } diff --git a/src/Microsoft.ComponentDetection.Detectors/linux/LinuxContainerDetector.cs b/src/Microsoft.ComponentDetection.Detectors/linux/LinuxContainerDetector.cs index ee19f3c88..9dfc94d58 100644 --- a/src/Microsoft.ComponentDetection.Detectors/linux/LinuxContainerDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/linux/LinuxContainerDetector.cs @@ -153,6 +153,9 @@ await this.DockerService.TryPullImageAsync(image, cancellationToken))) StackTrace = e.StackTrace, ImageId = image, }; + + var singleFileComponentRecorder = componentRecorder.CreateSingleFileComponentRecorder(image); + singleFileComponentRecorder.RegisterPackageParseFailure(image); } }); @@ -196,6 +199,9 @@ await this.DockerService.TryPullImageAsync(image, cancellationToken))) StackTrace = e.StackTrace, ImageId = kvp.Value.ImageId, }; + + var singleFileComponentRecorder = componentRecorder.CreateSingleFileComponentRecorder(kvp.Value.ImageId); + singleFileComponentRecorder.RegisterPackageParseFailure(kvp.Key); } return EmptyImageScanningResult(); diff --git a/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetComponentDetector.cs index fdb4931a7..99563e6f6 100644 --- a/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetComponentDetector.cs @@ -133,6 +133,7 @@ private async Task ProcessFileAsync(ProcessRequest processRequest) { // If something went wrong, just ignore the component this.Logger.LogFailedReadingFile(stream.Location, e); + singleFileComponentRecorder.RegisterPackageParseFailure(stream.Location); } } diff --git a/src/Microsoft.ComponentDetection.Detectors/ruby/RubyComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/ruby/RubyComponentDetector.cs index 855251867..611d858ed 100644 --- a/src/Microsoft.ComponentDetection.Detectors/ruby/RubyComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/ruby/RubyComponentDetector.cs @@ -110,13 +110,13 @@ private void ParseGemLockFile(ISingleFileComponentRecorder singleFileComponentRe switch (heading) { case "GIT": - this.ParseSection(SectionType.GIT, sublines, components, dependencies, file); + this.ParseSection(singleFileComponentRecorder, SectionType.GIT, sublines, components, dependencies, file); break; case "GEM": - this.ParseSection(SectionType.GEM, sublines, components, dependencies, file); + this.ParseSection(singleFileComponentRecorder, SectionType.GEM, sublines, components, dependencies, file); break; case "PATH": - this.ParseSection(SectionType.PATH, sublines, components, dependencies, file); + this.ParseSection(singleFileComponentRecorder, SectionType.PATH, sublines, components, dependencies, file); break; case "BUNDLED WITH": var line = sublines[0].Trim(); @@ -136,7 +136,7 @@ private void ParseGemLockFile(ISingleFileComponentRecorder singleFileComponentRe { // Throw this line away. Is this malformed? We were expecting a header this.Logger.LogVerbose(lines[0]); - this.Logger.LogVerbose("Appears to be malformed/is not expected here. Expected heading."); + this.Logger.LogVerbose("Appears to be malformed/is not expected here. Expected heading."); lines.RemoveAt(0); } } @@ -164,7 +164,7 @@ private void ParseGemLockFile(ISingleFileComponentRecorder singleFileComponentRe } } - private void ParseSection(SectionType sectionType, List lines, Dictionary components, Dictionary> dependencies, IComponentStream file) + private void ParseSection(ISingleFileComponentRecorder singleFileComponentRecorder, SectionType sectionType, List lines, Dictionary components, Dictionary> dependencies, IComponentStream file) { string name, remote, revision; name = remote = revision = string.Empty; @@ -214,6 +214,7 @@ private void ParseSection(SectionType sectionType, List lines, Dictionar if (this.IsVersionRelative(version)) { this.Logger.LogWarning($"Found component with invalid version, name = {name} and version = {version}"); + singleFileComponentRecorder.RegisterPackageParseFailure($"{name} - {version}"); wasParentDependencyExcluded = true; continue; } diff --git a/src/Microsoft.ComponentDetection.Detectors/rust/RustCrateDetector.cs b/src/Microsoft.ComponentDetection.Detectors/rust/RustCrateDetector.cs index 70b57dc8c..0a2d31a6c 100644 --- a/src/Microsoft.ComponentDetection.Detectors/rust/RustCrateDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/rust/RustCrateDetector.cs @@ -233,6 +233,7 @@ private void ProcessDependency( record.Dependencies = dependency; this.Logger.LogFailedReadingFile(cargoLockFile.Location, e); + singleFileComponentRecorder.RegisterPackageParseFailure(record.PackageInfo); } } } diff --git a/src/Microsoft.ComponentDetection.Detectors/yarn/YarnLockComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/yarn/YarnLockComponentDetector.cs index 41844daee..39ba5520d 100644 --- a/src/Microsoft.ComponentDetection.Detectors/yarn/YarnLockComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/yarn/YarnLockComponentDetector.cs @@ -77,7 +77,7 @@ private void DetectComponents(YarnLockFile file, string location, ISingleFileCom } } - if (yarnPackages.Count == 0 || !this.TryReadPeerPackageJsonRequestsAsYarnEntries(location, yarnPackages, out var yarnRoots)) + if (yarnPackages.Count == 0 || !this.TryReadPeerPackageJsonRequestsAsYarnEntries(singleFileComponentRecorder, location, yarnPackages, out var yarnRoots)) { return; } @@ -166,11 +166,12 @@ private void ParseTreeWithAssignedRoot(YarnEntry root, Dictionary + /// The component recorder for file that is been processed. /// The file location of the yarn.lock file. /// All the yarn entries that we know about. /// The output yarnRoots that we care about using as starting points. /// False if no package.json file was found at location, otherwise it returns true. - private bool TryReadPeerPackageJsonRequestsAsYarnEntries(string location, Dictionary yarnEntries, out List yarnRoots) + private bool TryReadPeerPackageJsonRequestsAsYarnEntries(ISingleFileComponentRecorder singleFileComponentRecorder, string location, Dictionary yarnEntries, out List yarnRoots) { yarnRoots = new List(); @@ -209,6 +210,7 @@ private bool TryReadPeerPackageJsonRequestsAsYarnEntries(string location, Dictio if (!yarnEntries.ContainsKey(entryKey)) { this.Logger.LogWarning($"A package was requested in the package.json file that was a peer of {location} but was not contained in the lockfile. {name} - {version.Key}"); + singleFileComponentRecorder.RegisterPackageParseFailure($"{name} - {version.Key}"); continue; } From e89309dcbc458a7dca59965b0d3fded371e58494 Mon Sep 17 00:00:00 2001 From: Coby Allred Date: Tue, 14 Feb 2023 13:43:57 -0800 Subject: [PATCH 3/5] Add Python detector --- .../pip/IPythonResolver.cs | 4 +++- .../pip/PipComponentDetector.cs | 2 +- .../pip/PythonResolver.cs | 9 ++++++--- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/pip/IPythonResolver.cs b/src/Microsoft.ComponentDetection.Detectors/pip/IPythonResolver.cs index c45b2e057..e3a6e5fdc 100644 --- a/src/Microsoft.ComponentDetection.Detectors/pip/IPythonResolver.cs +++ b/src/Microsoft.ComponentDetection.Detectors/pip/IPythonResolver.cs @@ -1,13 +1,15 @@ namespace Microsoft.ComponentDetection.Detectors.Pip; using System.Collections.Generic; using System.Threading.Tasks; +using Microsoft.ComponentDetection.Contracts; public interface IPythonResolver { /// /// Resolves the root Python packages from the initial list of packages. /// + /// The component recorder for file that is been processed. /// The initial list of packages. /// The root packages, with dependencies associated as children. - Task> ResolveRootsAsync(IList initialPackages); + Task> ResolveRootsAsync(ISingleFileComponentRecorder singleFileComponentRecorder, IList initialPackages); } diff --git a/src/Microsoft.ComponentDetection.Detectors/pip/PipComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/pip/PipComponentDetector.cs index 2841ad496..edbcdfec9 100644 --- a/src/Microsoft.ComponentDetection.Detectors/pip/PipComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/pip/PipComponentDetector.cs @@ -57,7 +57,7 @@ protected override async Task OnFileFoundAsync(ProcessRequest processRequest, ID .Where(x => !x.PackageIsUnsafe()) .ToList(); - var roots = await this.PythonResolver.ResolveRootsAsync(listedPackage); + var roots = await this.PythonResolver.ResolveRootsAsync(singleFileComponentRecorder, listedPackage); RecordComponents( singleFileComponentRecorder, diff --git a/src/Microsoft.ComponentDetection.Detectors/pip/PythonResolver.cs b/src/Microsoft.ComponentDetection.Detectors/pip/PythonResolver.cs index eac967401..5a9c4c7dc 100644 --- a/src/Microsoft.ComponentDetection.Detectors/pip/PythonResolver.cs +++ b/src/Microsoft.ComponentDetection.Detectors/pip/PythonResolver.cs @@ -19,9 +19,10 @@ public class PythonResolver : IPythonResolver /// /// Resolves the root Python packages from the initial list of packages. /// + /// The component recorder for file that is been processed. /// The initial list of packages. /// The root packages, with dependencies associated as children. - public async Task> ResolveRootsAsync(IList initialPackages) + public async Task> ResolveRootsAsync(ISingleFileComponentRecorder singleFileComponentRecorder, IList initialPackages) { var state = new PythonResolverState(); @@ -52,15 +53,16 @@ public async Task> ResolveRootsAsync(IList(); + return await this.ProcessQueueAsync(singleFileComponentRecorder, state) ?? new List(); } - private async Task> ProcessQueueAsync(PythonResolverState state) + private async Task> ProcessQueueAsync(ISingleFileComponentRecorder singleFileComponentRecorder, PythonResolverState state) { while (state.ProcessingQueue.Count > 0) { @@ -109,6 +111,7 @@ private async Task> ProcessQueueAsync(PythonResolverState st else { this.Logger.LogWarning($"Dependency Package {dependencyNode.Name} not found in Pypi. Skipping package"); + singleFileComponentRecorder.RegisterPackageParseFailure(dependencyNode.Name); } } } From b701c7292b796104b6a35cf3adb44790a2b8335f Mon Sep 17 00:00:00 2001 From: Coby Allred Date: Tue, 14 Feb 2023 14:25:37 -0800 Subject: [PATCH 4/5] Resolve test and merge failures --- .../DependencyGraph/ComponentRecorder.cs | 4 ++-- .../yarn/IYarnLockFileFactory.cs | 2 +- .../PipComponentDetectorTests.cs | 12 ++++++------ .../PipResolverTests.cs | 10 ++++++---- .../YarnLockDetectorTests.cs | 10 +++++++--- .../YarnParserTests.cs | 17 +++++++++++------ 6 files changed, 33 insertions(+), 22 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Common/DependencyGraph/ComponentRecorder.cs b/src/Microsoft.ComponentDetection.Common/DependencyGraph/ComponentRecorder.cs index b1205b959..3f940032d 100644 --- a/src/Microsoft.ComponentDetection.Common/DependencyGraph/ComponentRecorder.cs +++ b/src/Microsoft.ComponentDetection.Common/DependencyGraph/ComponentRecorder.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Concurrent; using System.Collections.Generic; using System.Collections.Immutable; @@ -108,7 +108,7 @@ internal DependencyGraph GetDependencyGraphForLocation(string location) return this.singleFileRecorders.Single(x => x.ManifestFileLocation == location).DependencyGraph; } - public class SingleFileComponentRecorder : ISingleFileComponentRecorder + public sealed class SingleFileComponentRecorder : ISingleFileComponentRecorder { private readonly ILogger log; diff --git a/src/Microsoft.ComponentDetection.Detectors/yarn/IYarnLockFileFactory.cs b/src/Microsoft.ComponentDetection.Detectors/yarn/IYarnLockFileFactory.cs index f2dc248ec..593dc7fd0 100644 --- a/src/Microsoft.ComponentDetection.Detectors/yarn/IYarnLockFileFactory.cs +++ b/src/Microsoft.ComponentDetection.Detectors/yarn/IYarnLockFileFactory.cs @@ -6,5 +6,5 @@ namespace Microsoft.ComponentDetection.Detectors.Yarn; public interface IYarnLockFileFactory { - Task ParseYarnLockFileAsync(Stream file, ILogger logger); + Task ParseYarnLockFileAsync(ISingleFileComponentRecorder singleFileComponentRecorder, Stream file, ILogger logger); } diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/PipComponentDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/PipComponentDetectorTests.cs index 1a8ad2b65..936e2afba 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/PipComponentDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/PipComponentDetectorTests.cs @@ -81,8 +81,8 @@ public async Task TestPipDetector_SetupPyAndRequirementsTxtAsync() new PipGraphNode(new PipComponent("f", "1.1")), }; - this.pythonResolver.Setup(x => x.ResolveRootsAsync(It.Is>(p => p.Any(d => d.Name == "b")))).ReturnsAsync(setupPyRoots); - this.pythonResolver.Setup(x => x.ResolveRootsAsync(It.Is>(p => p.Any(d => d.Name == "d")))).ReturnsAsync(requirementsTxtRoots); + this.pythonResolver.Setup(x => x.ResolveRootsAsync(It.IsAny(), It.Is>(p => p.Any(d => d.Name == "b")))).ReturnsAsync(setupPyRoots); + this.pythonResolver.Setup(x => x.ResolveRootsAsync(It.IsAny(), It.Is>(p => p.Any(d => d.Name == "d")))).ReturnsAsync(requirementsTxtRoots); var (result, componentRecorder) = await this.DetectorTestUtility .WithFile("setup.py", string.Empty) @@ -137,8 +137,8 @@ public async Task TestPipDetector_ComponentsDedupedAcrossFilesAsync() new PipGraphNode(new PipComponent("g", "1.2")), }; - this.pythonResolver.Setup(x => x.ResolveRootsAsync(It.Is>(p => p.Any(d => d.Name == "h")))).ReturnsAsync(requirementsTxtRoots); - this.pythonResolver.Setup(x => x.ResolveRootsAsync(It.Is>(p => p.Any(d => d.Name == "g")))).ReturnsAsync(requirementsTxtRoots2); + this.pythonResolver.Setup(x => x.ResolveRootsAsync(It.IsAny(), It.Is>(p => p.Any(d => d.Name == "h")))).ReturnsAsync(requirementsTxtRoots); + this.pythonResolver.Setup(x => x.ResolveRootsAsync(It.IsAny(), It.Is>(p => p.Any(d => d.Name == "g")))).ReturnsAsync(requirementsTxtRoots2); var (result, componentRecorder) = await this.DetectorTestUtility .WithFile("requirements.txt", string.Empty) @@ -186,11 +186,11 @@ public async Task TestPipDetector_ComponentRecorderAsync() blue.Children.Add(dog); this.pythonResolver.Setup(x => - x.ResolveRootsAsync(It.Is>(p => p.Any(d => d.Name == "a")))) + x.ResolveRootsAsync(It.IsAny(), It.Is>(p => p.Any(d => d.Name == "a")))) .ReturnsAsync(new List { rootA, rootB, }); this.pythonResolver.Setup(x => - x.ResolveRootsAsync(It.Is>(p => p.Any(d => d.Name == "c")))) + x.ResolveRootsAsync(It.IsAny(), It.Is>(p => p.Any(d => d.Name == "c")))) .ReturnsAsync(new List { rootC, rootD, rootE, }); var (result, componentRecorder) = await this.DetectorTestUtility diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/PipResolverTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/PipResolverTests.cs index 17d925c3f..756da8f2b 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/PipResolverTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/PipResolverTests.cs @@ -16,12 +16,14 @@ public class PipResolverTests { private Mock loggerMock; private Mock pyPiClient; + private Mock recorderMock; [TestInitialize] public void TestInitialize() { this.loggerMock = new Mock(); this.pyPiClient = new Mock(); + this.recorderMock = new Mock(); } [TestMethod] @@ -49,7 +51,7 @@ public async Task TestPipResolverSimpleGraphAsync() var resolver = new PythonResolver(this.pyPiClient.Object, this.loggerMock.Object); - var resolveResult = await resolver.ResolveRootsAsync(dependencies); + var resolveResult = await resolver.ResolveRootsAsync(this.recorderMock.Object, dependencies); Assert.IsNotNull(resolveResult); @@ -92,7 +94,7 @@ public async Task TestPipResolverNonExistantRootAsync() var resolver = new PythonResolver(this.pyPiClient.Object, this.loggerMock.Object); - var resolveResult = await resolver.ResolveRootsAsync(dependencies); + var resolveResult = await resolver.ResolveRootsAsync(this.recorderMock.Object, dependencies); Assert.IsNotNull(resolveResult); @@ -132,7 +134,7 @@ public async Task TestPipResolverNonExistantLeafAsync() var resolver = new PythonResolver(this.pyPiClient.Object, this.loggerMock.Object); - var resolveResult = await resolver.ResolveRootsAsync(dependencies); + var resolveResult = await resolver.ResolveRootsAsync(this.recorderMock.Object, dependencies); Assert.IsNotNull(resolveResult); @@ -175,7 +177,7 @@ public async Task TestPipResolverBacktrackAsync() var resolver = new PythonResolver(this.pyPiClient.Object, this.loggerMock.Object); - var resolveResult = await resolver.ResolveRootsAsync(dependencies); + var resolveResult = await resolver.ResolveRootsAsync(this.recorderMock.Object, dependencies); Assert.IsNotNull(resolveResult); diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/YarnLockDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/YarnLockDetectorTests.cs index 5f5d52b34..ed80ca71d 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/YarnLockDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/YarnLockDetectorTests.cs @@ -1,4 +1,4 @@ -namespace Microsoft.ComponentDetection.Detectors.Tests; +namespace Microsoft.ComponentDetection.Detectors.Tests; using System; using System.Collections.Generic; using System.IO; @@ -33,8 +33,12 @@ public YarnLockDetectorTests() this.yarnLockParser = new YarnLockParser(loggerMock.Object); this.yarnLockFileFactory = new YarnLockFileFactory(new[] { this.yarnLockParser }); var yarnLockFileFactoryMock = new Mock(); - yarnLockFileFactoryMock.Setup(x => x.ParseYarnLockFileAsync(It.IsAny(), It.IsAny())) - .Returns((Stream stream, ILogger logger) => this.yarnLockFileFactory.ParseYarnLockFileAsync(stream, logger)); + + var recorderMock = new Mock(); + + yarnLockFileFactoryMock.Setup(x => x.ParseYarnLockFileAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((ISingleFileComponentRecorder recorder, Stream stream, ILogger logger) => this.yarnLockFileFactory.ParseYarnLockFileAsync(recorder, stream, logger)); + this.DetectorTestUtility.AddServiceMock(yarnLockFileFactoryMock); } diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/YarnParserTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/YarnParserTests.cs index d06eb5c30..8bdaa515f 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/YarnParserTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/YarnParserTests.cs @@ -1,4 +1,4 @@ -namespace Microsoft.ComponentDetection.Detectors.Tests; +namespace Microsoft.ComponentDetection.Detectors.Tests; using System; using System.Collections.Generic; using System.Linq; @@ -14,15 +14,20 @@ public class YarnParserTests { private readonly Mock loggerMock; + private readonly Mock recorderMock; - public YarnParserTests() => this.loggerMock = new Mock(); + public YarnParserTests() + { + this.loggerMock = new Mock(); + this.recorderMock = new Mock(); + } [TestMethod] public void YarnLockParserWithNullBlockFile_Fails() { var parser = new YarnLockParser(this.loggerMock.Object); - void Action() => parser.Parse(null, this.loggerMock.Object); + void Action() => parser.Parse(this.recorderMock.Object, null, this.loggerMock.Object); Assert.ThrowsException(Action); } @@ -65,7 +70,7 @@ public void YarnLockParser_ParsesEmptyFile() blockFile.Setup(x => x.YarnLockVersion).Returns(yarnLockFileVersion); blockFile.Setup(x => x.GetEnumerator()).Returns(blocks.GetEnumerator()); - var file = parser.Parse(blockFile.Object, this.loggerMock.Object); + var file = parser.Parse(this.recorderMock.Object, blockFile.Object, this.loggerMock.Object); Assert.AreEqual(YarnLockVersion.V1, file.LockVersion); Assert.AreEqual(0, file.Entries.Count()); @@ -95,7 +100,7 @@ public void YarnLockParser_ParsesBlocks() blockFile.Setup(x => x.YarnLockVersion).Returns(yarnLockFileVersion); blockFile.Setup(x => x.GetEnumerator()).Returns(blocks.GetEnumerator()); - var file = parser.Parse(blockFile.Object, this.loggerMock.Object); + var file = parser.Parse(this.recorderMock.Object, blockFile.Object, this.loggerMock.Object); Assert.AreEqual(YarnLockVersion.V1, file.LockVersion); Assert.AreEqual(3, file.Entries.Count()); @@ -131,7 +136,7 @@ public void YarnLockParser_ParsesNoVersionInTitleBlock() blockFile.Setup(x => x.YarnLockVersion).Returns(yarnLockFileVersion); blockFile.Setup(x => x.GetEnumerator()).Returns(blocks.GetEnumerator()); - var file = parser.Parse(blockFile.Object, this.loggerMock.Object); + var file = parser.Parse(this.recorderMock.Object, blockFile.Object, this.loggerMock.Object); Assert.AreEqual(YarnLockVersion.V1, file.LockVersion); Assert.AreEqual(2, file.Entries.Count()); From b8376905b5b712646327c0301980bc39fd0cd09d Mon Sep 17 00:00:00 2001 From: Coby Allred Date: Fri, 17 Feb 2023 10:29:06 -0800 Subject: [PATCH 5/5] Address PR comments --- .../DependencyGraph/ComponentRecorder.cs | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Common/DependencyGraph/ComponentRecorder.cs b/src/Microsoft.ComponentDetection.Common/DependencyGraph/ComponentRecorder.cs index 3f940032d..8b35d226e 100644 --- a/src/Microsoft.ComponentDetection.Common/DependencyGraph/ComponentRecorder.cs +++ b/src/Microsoft.ComponentDetection.Common/DependencyGraph/ComponentRecorder.cs @@ -64,20 +64,16 @@ public IEnumerable GetDetectedComponents() public IEnumerable GetSkippedComponents() { - IEnumerable skippedComponents; if (this.singleFileRecorders == null) { return Enumerable.Empty(); } - skippedComponents = this.singleFileRecorders - .Select(singleFileRecorder => singleFileRecorder.GetSkippedComponents().Keys) + return this.singleFileRecorders + .Select(x => x.GetSkippedComponents().Keys) .SelectMany(x => x) - .GroupBy(x => x) - .Select(grouping => grouping.First()) + .Distinct() .ToImmutableList(); - - return skippedComponents; } public ISingleFileComponentRecorder CreateSingleFileComponentRecorder(string location) @@ -123,8 +119,6 @@ public sealed class SingleFileComponentRecorder : ISingleFileComponentRecorder private readonly object registerUsageLock = new object(); - private readonly object registerSkippedLock = new object(); - public SingleFileComponentRecorder(string location, ComponentRecorder recorder, bool enableManualTrackingOfExplicitReferences, ILogger log) { this.ManifestFileLocation = location; @@ -157,7 +151,6 @@ public IReadOnlyDictionary GetDetectedComponents() public IReadOnlyDictionary GetSkippedComponents() { - // Should this be immutable? return this.skippedComponentsInternal; } @@ -211,10 +204,7 @@ public void RegisterPackageParseFailure(string skippedComponent) throw new ArgumentNullException(paramName: nameof(skippedComponent)); } - lock (this.registerSkippedLock) - { - _ = this.skippedComponentsInternal[skippedComponent] = default; - } + _ = this.skippedComponentsInternal[skippedComponent] = default; } public void AddAdditionalRelatedFile(string relatedFilePath)