Add logging to some of the utils in DockerReferenceUtility for parse failures

Co-authored-by: Copilot <copilot@github.com>

Author: jpinz

src/Microsoft.ComponentDetection.Common/DockerReference/DockerReferenceUtility.cs

@@ -29,6 +29,7 @@ namespace Microsoft.ComponentDetection.Common;
 using System;
 using System.Diagnostics.CodeAnalysis;
 using Microsoft.ComponentDetection.Contracts;
+using Microsoft.Extensions.Logging;
 
 public static class DockerReferenceUtility
 {
@@ -49,32 +50,53 @@ public static bool HasUnresolvedVariables(string reference) =>
 
     /// <summary>
     /// Attempts to parse an image reference string into a <see cref="DockerReference"/>.
-    /// Returns <c>null</c> if the reference contains unresolved variables.
+    /// Returns <c>null</c> if the reference contains unresolved variables or cannot be parsed.
     /// </summary>
     /// <param name="imageReference">The image reference string to parse.</param>
-    /// <returns>A <see cref="DockerReference"/> if parsing succeeds; otherwise <c>null</c> if it has unresolved variables, or an exception is thrown.</returns>
-    public static DockerReference? TryParseImageReference(string imageReference)
+    /// <param name="logger">Optional logger for recording parse failures.</param>
+    /// <returns>A <see cref="DockerReference"/> if parsing succeeds; otherwise <c>null</c>.</returns>
+    public static DockerReference? TryParseImageReference(string imageReference, ILogger? logger = null)
     {
         if (HasUnresolvedVariables(imageReference))
         {
             return null;
         }
 
-        return ParseFamiliarName(imageReference);
+        try
+        {
+            return ParseFamiliarName(imageReference);
+        }
+        catch (DockerReferenceException ex)
+        {
+            logger?.LogWarning(ex, "Failed to parse image reference '{ImageReference}'.", imageReference);
+            return null;
+        }
     }
 
     /// <summary>
     /// Parses an image reference and registers it with the recorder if valid.
-    /// Silently skips references with unresolved variables or that cannot be parsed.
+    /// Skips references with unresolved variables or that cannot be parsed,
+    /// logging a warning for parse failures so that remaining entries continue to be processed.
     /// </summary>
     /// <param name="imageReference">The image reference string to parse.</param>
     /// <param name="recorder">The component recorder to register the image with.</param>
-    public static void TryRegisterImageReference(string imageReference, ISingleFileComponentRecorder recorder)
+    /// <param name="logger">Optional logger for recording parse failures.</param>
+    public static void TryRegisterImageReference(string imageReference, ISingleFileComponentRecorder recorder, ILogger? logger = null)
+    {
+        var dockerRef = TryParseImageReference(imageReference, logger);
+        TryRegisterImageReference(dockerRef, recorder);
+    }
+
+    /// <summary>
+    /// Registers a pre-parsed <see cref="DockerReference"/> with the recorder if non-null.
+    /// </summary>
+    /// <param name="dockerReference">The parsed docker reference, or <c>null</c> to skip.</param>
+    /// <param name="recorder">The component recorder to register the image with.</param>
+    public static void TryRegisterImageReference(DockerReference? dockerReference, ISingleFileComponentRecorder recorder)
     {
-        var dockerRef = TryParseImageReference(imageReference);
-        if (dockerRef != null)
+        if (dockerReference != null)
         {
-            recorder.RegisterUsage(new DetectedComponent(dockerRef.ToTypedDockerReferenceComponent()));
+            recorder.RegisterUsage(new DetectedComponent(dockerReference.ToTypedDockerReferenceComponent()));
         }
     }
 

src/Microsoft.ComponentDetection.Detectors/dockercompose/DockerComposeComponentDetector.cs

@@ -114,7 +114,7 @@ private void ExtractImageReferences(YamlMappingNode rootMapping, ISingleFileComp
                     var imageRef = (entry.Value as YamlScalarNode)?.Value;
                     if (!string.IsNullOrWhiteSpace(imageRef))
                     {
-                        DockerReferenceUtility.TryRegisterImageReference(imageRef, recorder);
+                        DockerReferenceUtility.TryRegisterImageReference(imageRef, recorder, this.Logger);
                     }
                 }
             }

src/Microsoft.ComponentDetection.Detectors/dockerfile/DockerfileComponentDetector.cs

@@ -72,10 +72,7 @@ private Task ParseDockerFileAsync(string fileContents, string fileLocation, ISin
         foreach (var instruction in instructions)
         {
             var imageReference = this.ProcessDockerfileConstruct(instruction, dockerfileModel.EscapeChar, stageNameMap);
-            if (imageReference != null)
-            {
-                singleFileComponentRecorder.RegisterUsage(new DetectedComponent(imageReference.ToTypedDockerReferenceComponent()));
-            }
+            DockerReferenceUtility.TryRegisterImageReference(imageReference, singleFileComponentRecorder);
         }
 
         return Task.CompletedTask;
@@ -85,14 +82,12 @@ private Task ParseDockerFileAsync(string fileContents, string fileLocation, ISin
     {
         try
         {
-            var baseImage = construct switch
+            return construct switch
             {
                 FromInstruction => this.ParseFromInstruction(construct, escapeChar, stageNameMap),
                 CopyInstruction => this.ParseCopyInstruction(construct, escapeChar, stageNameMap),
                 _ => null,
             };
-
-            return baseImage;
         }
         catch (Exception e)
         {

test/Microsoft.ComponentDetection.Common.Tests/DockerReferenceUtilityTests.cs

@@ -5,6 +5,7 @@ namespace Microsoft.ComponentDetection.Common.Tests;
 using AwesomeAssertions;
 using Microsoft.ComponentDetection.Contracts;
 using Microsoft.ComponentDetection.Contracts.BcdeModels;
+using Microsoft.Extensions.Logging;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using Moq;
 
@@ -296,11 +297,9 @@ public void TryParseImageReference_ReturnsNullForUnresolvedVariables()
     }
 
     [TestMethod]
-    public void TryParseImageReference_ThrowsForInvalidReference()
+    public void TryParseImageReference_ReturnsNullForInvalidReference()
     {
-        var func = () => DockerReferenceUtility.TryParseImageReference("docker.io/library/Nginx");
-
-        func.Should().Throw<ReferenceNameContainsUppercaseException>();
+        DockerReferenceUtility.TryParseImageReference("docker.io/library/Nginx").Should().BeNull();
     }
 
     [TestMethod]
@@ -357,13 +356,31 @@ public void TryRegisterImageReference_SkipsUnresolvedVariables()
     }
 
     [TestMethod]
-    public void TryRegisterImageReference_ThrowsForInvalidReference()
+    public void TryRegisterImageReference_SkipsInvalidReference()
     {
         var recorder = new Mock<ISingleFileComponentRecorder>();
 
-        var func = () => DockerReferenceUtility.TryRegisterImageReference("docker.io/library/Nginx", recorder.Object);
+        DockerReferenceUtility.TryRegisterImageReference("docker.io/library/Nginx", recorder.Object);
+
+        recorder.Verify(r => r.RegisterUsage(It.IsAny<DetectedComponent>(), It.IsAny<bool>(), It.IsAny<string>(), It.IsAny<bool?>(), It.IsAny<DependencyScope?>(), It.IsAny<string>()), Times.Never);
+    }
+
+    [TestMethod]
+    public void TryRegisterImageReference_LogsWarningForInvalidReference()
+    {
+        var recorder = new Mock<ISingleFileComponentRecorder>();
+        var logger = new Mock<ILogger>();
+
+        DockerReferenceUtility.TryRegisterImageReference("docker.io/library/Nginx", recorder.Object, logger.Object);
 
-        func.Should().Throw<ReferenceNameContainsUppercaseException>();
         recorder.Verify(r => r.RegisterUsage(It.IsAny<DetectedComponent>(), It.IsAny<bool>(), It.IsAny<string>(), It.IsAny<bool?>(), It.IsAny<DependencyScope?>(), It.IsAny<string>()), Times.Never);
+        logger.Verify(
+            l => l.Log(
+                LogLevel.Warning,
+                It.IsAny<EventId>(),
+                It.IsAny<It.IsAnyType>(),
+                It.IsAny<Exception>(),
+                It.IsAny<Func<It.IsAnyType, Exception, string>>()),
+            Times.Once);
     }
 }