Preserve file formatting when updating ArgoCD image tags (SF-3581)

build/Build.Docker.cs

@@ -1,12 +1,15 @@
 using Calamari.Build.Utilities;
 using JetBrains.Annotations;
+using Nuke.Common.Tooling;
 using Nuke.Common.Tools.Docker;
-using Nuke.Common.Tools.PowerShell;
 
 namespace Calamari.Build;
 
 public partial class Build
 {
+    //Resolved from PATH so a missing gzip fails by name, rather than as a mystery exit code
+    static Tool Gzip => ToolResolver.GetPathTool("gzip");
+
     [PublicAPI]
     Target BuildDockerImages =>
         d =>
@@ -82,10 +85,10 @@ public partial class Build
                                                                                               return settings;
                                                                                           });
 
-                                                            //compress with gzip
-                                                            PowerShellTasks.PowerShell(_ => _
-                                                                                            .EnableNoProfile()
-                                                                                            .SetCommand($"gzip -k -9 -f '{outputFile}'"));
+                                                            //compress with gzip. Invoked directly rather than via pwsh, which only
+                                                            //added a dependency on whatever .NET runtime the agent's `pwsh` global tool
+                                                            //was built against - not the SDK this build pins.
+                                                            Gzip($"-k -9 -f \"{outputFile}\"");
 
                                                             //gzip always uses the .gz suffix
                                                             var compressedZipPath = $"{outputFile}.gz";

source/Calamari.Common/Plumbing/Extensions/StringExtensions.cs

@@ -99,6 +99,11 @@ public static string ToCamelCase(this string text)
                         : null;
         }
 
+        public static bool HasTrailingNewLine(this string? input)
+        {
+            return input != null && (input.EndsWith("\n") || input.EndsWith("\r"));
+        }
+
         public static string EnsureDoubleQuoteIfContainsSpaces(this string text) => EnsureDoubleQuote(text, t => t.Contains(" "));
         public static string EnsureDoubleQuote(this string text) => EnsureDoubleQuote(text, t => !t.EndsWith("\"") && !t.StartsWith("\""));
         public static string EnsureDoubleQuote(this string text, Predicate<string> shouldQuote) => shouldQuote(text) ? $"\"{text}\"" : text;

source/Calamari.Tests/ArgoCD/Helm/HelmValuesEditorTests.cs

@@ -98,29 +98,32 @@ public void GenerateVariableDictionary_ReturnsDictionaryOfNodeValuesWithValues()
             result.Should().BeEquivalentTo(expected);
         }
 
-        [Test]
-        public void UpdateNodeValue_ReturnsModifiedYaml()
+        [TestCase("\n")]
+        [TestCase("\r\n")]
+        public void UpdateNodeValue_ReturnsModifiedYaml(string newLine)
         {
-            const string yamlContent = @"root:
-  node1: ""node1value""
-  node2:
-     node2Nest:
-         node2nestedValue: ""banana""
-     node2Child1: ""node2child1value""
-     node2Child2: 42
-";
+            var yamlContent = string.Join(newLine,
+                                          "root:",
+                                          "  node1: \"node1value\"",
+                                          "  node2:",
+                                          "     node2Nest:",
+                                          "         node2nestedValue: \"banana\"",
+                                          "     node2Child1: \"node2child1value\"",
+                                          "     node2Child2: 42",
+                                          "");
+
             var result = HelmValuesEditor.UpdateNodeValue(yamlContent, "root.node1", "awesome new value");
 
-            const string expected = @"root:
-  node1: ""awesome new value""
-  node2:
-     node2Nest:
-         node2nestedValue: ""banana""
-     node2Child1: ""node2child1value""
-     node2Child2: 42
-";
-            //ensure platform-agnostic multiline comparison
-            result.ReplaceLineEndings().Should().Be(expected.ReplaceLineEndings());
+            result.Should()
+                  .Be(string.Join(newLine,
+                                  "root:",
+                                  "  node1: \"awesome new value\"",
+                                  "  node2:",
+                                  "     node2Nest:",
+                                  "         node2nestedValue: \"banana\"",
+                                  "     node2Child1: \"node2child1value\"",
+                                  "     node2Child2: 42",
+                                  ""));
         }
     }
 }

source/Calamari.Tests/ArgoCD/Helm/HelmValuesImageReplaceStepVariablesTests.cs

@@ -1,4 +1,3 @@
-using System;
 using System.Collections.Generic;
 using Calamari.ArgoCD;
 using Calamari.ArgoCD.Conventions;
@@ -119,16 +118,37 @@ public void StructuredValue_ImageOnNonDefaultRegistry_UpdatesFullRefAndTracksWit
         result.UpdatedContents.Should().Contain("name: us-docker.pkg.dev/shared-gke-dev-gqtrxy/argo-test/helloworld:v2");
     }
 
-    [Test]
-    public void TwoImagesWithSameTag_OnlyUpdatesConfiguredPath()
+    // Line endings are spelled out rather than taken from a verbatim literal: .gitattributes checks
+    // .cs files out with native endings, so a literal's endings always match Environment.NewLine and
+    // an assertion against it cannot distinguish "preserved the file's endings" from "used the
+    // agent's". The customer's case was an LF file on a Windows agent, where those differ.
+    [TestCase("\n")]
+    [TestCase("\r\n")]
+    public void TwoImagesWithSameTag_OnlyUpdatesConfiguredPath(string newLine)
     {
-        const string yaml = @"
-nginx:
-  tag: 1.0
-redis:
-  tag: 1.0
-";
-
+        var yaml = string.Join(newLine, "", "nginx:", "  tag: 1.0", "redis:", "  tag: 1.0", "");
+
+        var replacer = new HelmValuesImageReplaceStepVariables(yaml, DefaultRegistry, log);
+        var images = new List<ContainerImageReferenceAndHelmReference>
+        {
+            new(ContainerImageReference.FromReferenceString("nginx:1.27.1", DefaultRegistry), "nginx.tag")
+        };
+
+        var result = replacer.UpdateImages(images);
+
+        using var scope = new AssertionScope();
+        result.UpdatedImageReferences.Should().BeEquivalentTo(["nginx:1.27.1"]);
+        result.UpdatedContents
+              .Should()
+              .Be(string.Join(newLine, "", "nginx:", "  tag: 1.27.1", "redis:", "  tag: 1.0", ""));
+    }
+
+    [TestCase("\n")]
+    [TestCase("\r\n")]
+    public void TwoImagesWithSameTag_WithoutATrailingNewline_OnlyUpdatesConfiguredPath(string newLine)
+    {
+        var yaml = string.Join(newLine, "", "nginx:", "  tag: 1.0", "redis:", "  tag: 1.0");
+
         var replacer = new HelmValuesImageReplaceStepVariables(yaml, DefaultRegistry, log);
         var images = new List<ContainerImageReferenceAndHelmReference>
         {
@@ -139,8 +159,9 @@ public void TwoImagesWithSameTag_OnlyUpdatesConfiguredPath()
 
         using var scope = new AssertionScope();
         result.UpdatedImageReferences.Should().BeEquivalentTo(["nginx:1.27.1"]);
-        result.UpdatedContents.Should().Contain($"nginx:{Environment.NewLine}  tag: 1.27.1");
-        result.UpdatedContents.Should().Contain($"redis:{Environment.NewLine}  tag: 1.0");
+        result.UpdatedContents
+              .Should()
+              .Be(string.Join(newLine, "", "nginx:", "  tag: 1.27.1", "redis:", "  tag: 1.0"));
     }
 
     [Test]

source/Calamari.Tests/ArgoCD/Helm/HelmYamlParserTests.cs

@@ -1,5 +1,6 @@
 using System;
 using Calamari.ArgoCD.Helm;
+using Calamari.Common.Commands;
 using FluentAssertions;
 using NUnit.Framework;
 
@@ -51,96 +52,130 @@ public void GetValueAtPath_ReturnsTheValueOfTheSpecifiedNode(string path, string
             result.Should().Be(expected);
         }
 
-        [Test]
-        public void UpdateNodeValue_WithNonDelimitedNodeValue_ReplacesValueInDocument()
+        [TestCase("\n")]
+        [TestCase("\r\n")]
+        public void UpdateNodeValue_WithNonDelimitedNodeValue_ReplacesValueInDocument(string newLine)
         {
-            const string yamlContent = @"
-root:
-  node1: 42
-  node2: stable
-";
+            var yamlContent = string.Join(newLine, "", "root:", "  node1: 42", "  node2: stable", "");
 
             var sut = new HelmYamlParser(yamlContent);
 
-            const string expectedUpdate = @"
-root:
-  node1: 69
-  node2: stable
-";
+            var result = sut.UpdateContentForPath("root.node1", "69");
+
+            result.Should().Be(string.Join(newLine, "", "root:", "  node1: 69", "  node2: stable", ""));
+        }
+
+        [TestCase("\n")]
+        [TestCase("\r\n")]
+        public void UpdateNodeValue_WithDoubleQuoteDelimitedNodeValue_PreservesDelimitersWithNewValue(string newLine)
+        {
+            var yamlContent = string.Join(newLine, "", "root:", "  node1: 42", "  node2: \"latest\"", "");
+
+            var sut = new HelmYamlParser(yamlContent);
+
+            var result = sut.UpdateContentForPath("root.node2", "stable");
+
+            result.Should().Be(string.Join(newLine, "", "root:", "  node1: 42", "  node2: \"stable\"", ""));
+        }
+
+        [TestCase("\n")]
+        [TestCase("\r\n")]
+        public void UpdateNodeValue_WithSingleQuoteDelimitedNodeValue_PreservesDelimitersWithNewValue(string newLine)
+        {
+            var yamlContent = string.Join(newLine, "", "root:", "  node1: 42", "  node2: 'latest'", "");
+
+            var sut = new HelmYamlParser(yamlContent);
+
+            var result = sut.UpdateContentForPath("root.node2", "stable");
+
+            result.Should().Be(string.Join(newLine, "", "root:", "  node1: 42", "  node2: 'stable'", ""));
+        }
+
+        [TestCase("\n")]
+        [TestCase("\r\n")]
+        public void UpdateNodeValue_RespectsTrailingWhitespaceFromInput(string newLine)
+        {
+            var yamlContent = string.Join(newLine, "", "root:", "  node1: 42", "  ", "");
+
+            var sut = new HelmYamlParser(yamlContent);
 
             var result = sut.UpdateContentForPath("root.node1", "69");
 
-            //ensure platform-agnostic multiline comparison
-            result.ReplaceLineEndings().Should().Be(expectedUpdate.ReplaceLineEndings());
+            result.Should().Be(string.Join(newLine, "", "root:", "  node1: 69", "  ", ""));
         }
 
         [Test]
-        public void UpdateNodeValue_WithDoubleQuoteDelimitedNodeValue_PreservesDelimitersWithNewValue()
+        public void UpdateNodeValue_WithCrlfLineEndings_PreservesCrlfOnEveryLine()
         {
-            const string yamlContent = @"
-root:
-  node1: 42
-  node2: ""latest""
-";
+            const string yamlContent = "root:\r\n  node1: 42\r\n  node2: stable\r\n";
 
             var sut = new HelmYamlParser(yamlContent);
 
-            const string expectedUpdate = @"
-root:
-  node1: 42
-  node2: ""stable""
-";
+            var result = sut.UpdateContentForPath("root.node1", "69");
 
-            var result = sut.UpdateContentForPath("root.node2", "stable");
+            result.Should().Be("root:\r\n  node1: 69\r\n  node2: stable\r\n");
+        }
+
+        [Test]
+        public void UpdateNodeValue_WithLfLineEndings_PreservesLfOnEveryLine()
+        {
+            const string yamlContent = "root:\n  node1: 42\n  node2: stable\n";
+
+            var sut = new HelmYamlParser(yamlContent);
+
+            var result = sut.UpdateContentForPath("root.node1", "69");
 
-            //ensure platform-agnostic multiline comparison
-            result.ReplaceLineEndings().Should().Be(expectedUpdate.ReplaceLineEndings());
+            result.Should().Be("root:\n  node1: 69\n  node2: stable\n");
         }
 
         [Test]
-        public void UpdateNodeValue_WithSingleQuoteDelimitedNodeValue_PreservesDelimitersWithNewValue()
+        public void UpdateNodeValue_WithNoTrailingNewline_DoesNotAddOne()
         {
-            const string yamlContent = @"
-root:
-  node1: 42
-  node2: 'latest'
-";
+            const string yamlContent = "root:\n  node1: 42";
 
             var sut = new HelmYamlParser(yamlContent);
 
-            const string expectedUpdate = @"
-root:
-  node1: 42
-  node2: 'stable'
-";
+            var result = sut.UpdateContentForPath("root.node1", "69");
+
+            result.Should().Be("root:\n  node1: 69");
+        }
+
+        [Test]
+        public void UpdateNodeValue_WithCrlfAndNoTrailingNewline_PreservesBoth()
+        {
+            const string yamlContent = "root:\r\n  node1: 42\r\n  node2: \"latest\"";
+
+            var sut = new HelmYamlParser(yamlContent);
 
             var result = sut.UpdateContentForPath("root.node2", "stable");
 
-            //ensure platform-agnostic multiline comparison
-            result.ReplaceLineEndings().Should().Be(expectedUpdate.ReplaceLineEndings());
+            result.Should().Be("root:\r\n  node1: 42\r\n  node2: \"stable\"");
         }
 
         [Test]
-        public void UpdateNodeValue_RespectsTrailingWhitespaceFromInput()
+        public void UpdateNodeValue_WithUnchangedPath_ReturnsContentByteForByte()
         {
-            const string yamlContent = @"
-root:
-  node1: 42
-
-";
+            const string yamlContent = "root:\r\n  node1: 42\r\n";
 
             var sut = new HelmYamlParser(yamlContent);
 
-            const string expectedUpdate = @"
-root:
-  node1: 69
-
-";
+            var result = sut.UpdateContentForPath("root.missing", "69");
 
-            var result = sut.UpdateContentForPath("root.node1", "69");
+            result.Should().Be(yamlContent);
+        }
+
+        [Test]
+        public void UpdateNodeValue_WithAFoldedValue_FailsWithAMessageNamingThePathAndTheFix()
+        {
+            const string yamlContent = "image:\n  tag: >\n    1.21\n";
+
+            var sut = new HelmYamlParser(yamlContent);
+
+            var act = () => sut.UpdateContentForPath("image.tag", "1.25");
 
-            //ensure platform-agnostic multiline comparison
-            result.ReplaceLineEndings().Should().Be(expectedUpdate.ReplaceLineEndings());
+            act.Should()
+               .Throw<CommandException>()
+               .WithMessage("*image.tag*line 2*folded block scalar (>)*literal block (|)*");
         }
 
         [Test]