From 3f50167e7b34b5696886b862f5cb97b95644178a Mon Sep 17 00:00:00 2001 From: Frank Lin Date: Wed, 16 Sep 2026 21:10:31 +1000 Subject: [PATCH 1/7] Preserve line endings and trailing newline in Helm values files ReplaceNodeContent rebuilt the file with StringBuilder.AppendLine, which emits Environment.NewLine, so a file whose convention differed from the agent's had every line rewritten and a one-line tag change became a whole-file diff. It now appends the line ending detected from the file itself. The trailing-newline check compared against Environment.NewLine too. On a Windows worker an LF file therefore looked as though it had no trailing newline and TrimEnd removed it, which together with the rewrite matches the reported symptom of CRLF endings and no trailing newline. It now tests for an actual line break. That second fix cannot be covered on Linux, where Environment.NewLine is already "\n" and the old comparison happens to work. Co-Authored-By: Claude Opus 5 (1M context) --- .../ArgoCD/Helm/HelmYamlParserTests.cs | 60 +++++++++++++++++++ source/Calamari/ArgoCD/Helm/HelmYamlParser.cs | 8 ++- 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/source/Calamari.Tests/ArgoCD/Helm/HelmYamlParserTests.cs b/source/Calamari.Tests/ArgoCD/Helm/HelmYamlParserTests.cs index fff879ad7d..88f110e0b6 100644 --- a/source/Calamari.Tests/ArgoCD/Helm/HelmYamlParserTests.cs +++ b/source/Calamari.Tests/ArgoCD/Helm/HelmYamlParserTests.cs @@ -143,6 +143,66 @@ public void UpdateNodeValue_RespectsTrailingWhitespaceFromInput() result.ReplaceLineEndings().Should().Be(expectedUpdate.ReplaceLineEndings()); } + [Test] + public void UpdateNodeValue_WithCrlfLineEndings_PreservesCrlfOnEveryLine() + { + const string yamlContent = "root:\r\n node1: 42\r\n node2: stable\r\n"; + + var sut = new HelmYamlParser(yamlContent); + + var result = sut.UpdateContentForPath("root.node1", "69"); + + 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"); + + result.Should().Be("root:\n node1: 69\n node2: stable\n"); + } + + [Test] + public void UpdateNodeValue_WithNoTrailingNewline_DoesNotAddOne() + { + const string yamlContent = "root:\n node1: 42"; + + var sut = new HelmYamlParser(yamlContent); + + 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"); + + result.Should().Be("root:\r\n node1: 42\r\n node2: \"stable\""); + } + + [Test] + public void UpdateNodeValue_WithUnchangedPath_ReturnsContentByteForByte() + { + const string yamlContent = "root:\r\n node1: 42\r\n"; + + var sut = new HelmYamlParser(yamlContent); + + var result = sut.UpdateContentForPath("root.missing", "69"); + + result.Should().Be(yamlContent); + } + [Test] public void CreateDotPathsForNodes_WithExistingDotNotationKeys_IgnoresThoseKeys() { diff --git a/source/Calamari/ArgoCD/Helm/HelmYamlParser.cs b/source/Calamari/ArgoCD/Helm/HelmYamlParser.cs index 8c1f2cd014..1d8061a30a 100644 --- a/source/Calamari/ArgoCD/Helm/HelmYamlParser.cs +++ b/source/Calamari/ArgoCD/Helm/HelmYamlParser.cs @@ -4,6 +4,7 @@ using System.IO; using System.Linq; using System.Text; +using Calamari.Common.Plumbing.Extensions; using YamlDotNet.Core; using YamlDotNet.RepresentationModel; using YamlDotNet.Serialization; @@ -22,7 +23,7 @@ public HelmYamlParser(string yamlContent) var reader = new StringReader(yamlString); yamlStream = new YamlStream(); yamlStream.Load(reader); - endsWithNewline = yamlString.EndsWith(Environment.NewLine); + endsWithNewline = yamlString.EndsWith("\n") || yamlString.EndsWith("\r"); } readonly string yamlString; @@ -87,6 +88,7 @@ string ReplaceNodeContent(YamlScalarNode node, string newValue) { var result = new StringBuilder(); using var reader = new StringReader(yamlString); + var newLine = yamlString.DetectLineEnding() ?? "\n"; var targetLine = (int)node.Start.Line; int startColumn; @@ -115,11 +117,11 @@ string ReplaceNodeContent(YamlScalarNode node, string newValue) // Replace in this line var before = line[..startColumn]; var after = line[endColumn..]; - result.AppendLine(before + newValue + after); + result.Append(before + newValue + after).Append(newLine); } else { - result.AppendLine(line); + result.Append(line).Append(newLine); } currentLine++; } From 9e18692862a9cbf5c69e677132d77c47af0d4074 Mon Sep 17 00:00:00 2001 From: Frank Lin Date: Tue, 15 Sep 2026 16:19:03 +1000 Subject: [PATCH 2/7] Route inline patch replacers through newline-aware serialization SerializeDocuments applied the detected line ending only to the separator between documents; each document body still came out of a plain StringWriter as LF. It also had no callers, while the three inline patch replacers each did their own Save(writer) plus TrimEnd, losing the file's line endings and trailing newline. SerializeDocuments now reapplies the line ending after emitting (the emitter writes '\n' directly and ignores TextWriter.NewLine) and restores the trailing newline when the original had one. The three replacers call it instead of serializing themselves. This removes the line-ending and trailing-newline noise only. These replacers still round-trip the document through YamlDotNet, so quoting, indentation and comments are still reflowed. Co-Authored-By: Claude Opus 5 (1M context) --- .../ArgoCD/YamlStreamLoaderTests.cs | 56 +++++++++++++++++++ .../InlineStrategicMergeImageReplacer.cs | 5 +- .../ArgoCD/InlineJsonPatchReplacer.cs | 4 +- .../ArgoCD/YamlJson6902PatchImageReplacer.cs | 11 +--- source/Calamari/ArgoCD/YamlStreamLoader.cs | 37 ++++++------ 5 files changed, 79 insertions(+), 34 deletions(-) create mode 100644 source/Calamari.Tests/ArgoCD/YamlStreamLoaderTests.cs diff --git a/source/Calamari.Tests/ArgoCD/YamlStreamLoaderTests.cs b/source/Calamari.Tests/ArgoCD/YamlStreamLoaderTests.cs new file mode 100644 index 0000000000..0937cac8c4 --- /dev/null +++ b/source/Calamari.Tests/ArgoCD/YamlStreamLoaderTests.cs @@ -0,0 +1,56 @@ +using Calamari.ArgoCD; +using FluentAssertions; +using NUnit.Framework; + +namespace Calamari.Tests.ArgoCD +{ + [TestFixture] + public class YamlStreamLoaderTests + { + [Test] + public void SerializeDocuments_WithCrlfOriginal_EmitsCrlfOnEveryLine() + { + const string original = "kind: Kustomization\r\nnamespace: dev\r\n"; + + var result = Serialize(original); + + result.Should().Be("kind: Kustomization\r\nnamespace: dev\r\n"); + } + + [Test] + public void SerializeDocuments_WithLfOriginal_EmitsLfOnEveryLine() + { + const string original = "kind: Kustomization\nnamespace: dev\n"; + + var result = Serialize(original); + + result.Should().Be("kind: Kustomization\nnamespace: dev\n"); + } + + [Test] + public void SerializeDocuments_WithOriginalMissingTrailingNewline_DoesNotAddOne() + { + const string original = "kind: Kustomization\nnamespace: dev"; + + var result = Serialize(original); + + result.Should().Be("kind: Kustomization\nnamespace: dev"); + } + + [Test] + public void SerializeDocuments_WithMultipleDocuments_SeparatesUsingTheOriginalLineEnding() + { + const string original = "kind: First\r\n---\r\nkind: Second\r\n"; + + var result = Serialize(original); + + result.Should().Be("kind: First\r\n---\r\nkind: Second\r\n"); + } + + static string Serialize(string original) + { + var stream = YamlStreamLoader.TryLoadSilent(original); + return YamlStreamLoader.SerializeDocuments(stream!.Documents, original); + } + } +} diff --git a/source/Calamari/ArgoCD/Conventions/UpdateImageTag/InlineStrategicMergeImageReplacer.cs b/source/Calamari/ArgoCD/Conventions/UpdateImageTag/InlineStrategicMergeImageReplacer.cs index 484e7c72ed..3e37462ecc 100644 --- a/source/Calamari/ArgoCD/Conventions/UpdateImageTag/InlineStrategicMergeImageReplacer.cs +++ b/source/Calamari/ArgoCD/Conventions/UpdateImageTag/InlineStrategicMergeImageReplacer.cs @@ -1,5 +1,4 @@ using System.Collections.Generic; -using System.IO; using System.Linq; using Calamari.ArgoCD.Models; using Calamari.Common.Plumbing.Logging; @@ -56,9 +55,7 @@ public ImageReplacementResult UpdateImages(IReadOnlyCollection(), new HashSet()); } - using var writer = new StringWriter(); - yamlStream.Save(writer, false); - var modifiedContent = writer.ToString().TrimEnd(); + var modifiedContent = YamlStreamLoader.SerializeDocuments(yamlStream.Documents, input); return new ImageReplacementResult(modifiedContent, allUpdatedImages, new HashSet()); } diff --git a/source/Calamari/ArgoCD/InlineJsonPatchReplacer.cs b/source/Calamari/ArgoCD/InlineJsonPatchReplacer.cs index e7a0ec585c..72484df8c4 100644 --- a/source/Calamari/ArgoCD/InlineJsonPatchReplacer.cs +++ b/source/Calamari/ArgoCD/InlineJsonPatchReplacer.cs @@ -91,9 +91,7 @@ public ImageReplacementResult UpdateImages(IReadOnlyCollection()); } diff --git a/source/Calamari/ArgoCD/YamlJson6902PatchImageReplacer.cs b/source/Calamari/ArgoCD/YamlJson6902PatchImageReplacer.cs index 0796ea902c..426c28e44e 100644 --- a/source/Calamari/ArgoCD/YamlJson6902PatchImageReplacer.cs +++ b/source/Calamari/ArgoCD/YamlJson6902PatchImageReplacer.cs @@ -1,7 +1,6 @@ #nullable enable using System; using System.Collections.Generic; -using System.IO; using System.Linq; using Calamari.ArgoCD.Conventions; using Calamari.ArgoCD.Models; @@ -72,15 +71,9 @@ public ImageReplacementResult UpdateImages(IReadOnlyCollection 0) - { - var singleDocStream = new YamlStream(stream.Documents[0]); - singleDocStream.Save(writer, false); - } - var modifiedContent = writer.ToString().TrimEnd(); + // Take just the first document to avoid unwanted document separators. + var modifiedContent = YamlStreamLoader.SerializeDocuments(stream.Documents.Take(1), yamlContent); return new ImageReplacementResult(modifiedContent, combinedResult.UpdatedImageReferences, combinedResult.AlreadyUpToDateImages); } diff --git a/source/Calamari/ArgoCD/YamlStreamLoader.cs b/source/Calamari/ArgoCD/YamlStreamLoader.cs index 83965b535a..c9a3ad5394 100644 --- a/source/Calamari/ArgoCD/YamlStreamLoader.cs +++ b/source/Calamari/ArgoCD/YamlStreamLoader.cs @@ -109,27 +109,28 @@ public static string SerializeDocuments(IEnumerable documents, str return string.Empty; var newLine = originalContent?.DetectLineEnding() ?? "\n"; - var serializedDocs = new List(); + var serializedDocs = documentList.Select(doc => SerializeDocument(doc, newLine)); - foreach (var doc in documentList) - { - using var writer = new StringWriter(); - var tempStream = new YamlStream(doc); - tempStream.Save(writer, false); - var serialized = writer.ToString(); - - serialized = serialized.TrimEnd(); - if (serialized.EndsWith("...")) - { - serialized = serialized.Substring(0, serialized.Length - 3).TrimEnd(); - } + var joined = string.Join($"{newLine}---{newLine}", serializedDocs); + return EndsWithNewLine(originalContent) ? joined + newLine : joined; + } - serializedDocs.Add(serialized); - } + static string SerializeDocument(YamlDocument document, string newLine) + { + // The emitter always writes '\n' regardless of the writer's NewLine, so the document's + // own line ending has to be reapplied afterwards. + using var writer = new StringWriter(); + new YamlStream(document).Save(writer, false); + + var serialized = writer.ToString().TrimEnd().ReplaceLineEndings(newLine); + return serialized.EndsWith("...") + ? serialized.Substring(0, serialized.Length - 3).TrimEnd() + : serialized; + } - return documentList.Count == 1 - ? serializedDocs[0] - : string.Join($"{newLine}---{newLine}", serializedDocs); + static bool EndsWithNewLine(string? content) + { + return content != null && (content.EndsWith("\n") || content.EndsWith("\r")); } } } \ No newline at end of file From 1743cd5c06e32a119f97a08b67ab782ae0015b0d Mon Sep 17 00:00:00 2001 From: Frank Lin Date: Thu, 17 Sep 2026 07:58:28 +1000 Subject: [PATCH 3/7] Extract HasTrailingNewLine alongside DetectLineEnding The trailing-newline check had two copies, one in each replacer path. It now sits next to DetectLineEnding in StringExtensions, which is where the line-ending handling already lives, and tolerates null so callers holding optional content do not need their own guard. Co-Authored-By: Claude Opus 5 (1M context) --- .../Plumbing/Extensions/StringExtensions.cs | 5 +++++ .../Fixtures/Util/StringExtensionsFixture.cs | 14 ++++++++++++++ source/Calamari/ArgoCD/Helm/HelmYamlParser.cs | 2 +- source/Calamari/ArgoCD/YamlStreamLoader.cs | 7 +------ 4 files changed, 21 insertions(+), 7 deletions(-) diff --git a/source/Calamari.Common/Plumbing/Extensions/StringExtensions.cs b/source/Calamari.Common/Plumbing/Extensions/StringExtensions.cs index 4ea21405bc..8d2dbb9739 100644 --- a/source/Calamari.Common/Plumbing/Extensions/StringExtensions.cs +++ b/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 shouldQuote) => shouldQuote(text) ? $"\"{text}\"" : text; diff --git a/source/Calamari.Tests/Fixtures/Util/StringExtensionsFixture.cs b/source/Calamari.Tests/Fixtures/Util/StringExtensionsFixture.cs index ef72f9a9c1..9c48a6b7a4 100644 --- a/source/Calamari.Tests/Fixtures/Util/StringExtensionsFixture.cs +++ b/source/Calamari.Tests/Fixtures/Util/StringExtensionsFixture.cs @@ -44,6 +44,20 @@ public void AsRelativePathFrom(string source, string baseDirectory, string expec Assert.AreEqual(expected, source.AsRelativePathFrom(baseDirectory)); } + [TestCase("a\n", true)] + [TestCase("a\r\n", true)] + [TestCase("a\r", true)] + [TestCase("a\n\n", true)] + [TestCase("a", false)] + [TestCase("a ", false)] + [TestCase("", false)] + [TestCase(null, false)] + [Test] + public void HasTrailingNewLine_DetectsATrailingLineBreak(string input, bool expected) + { + input.HasTrailingNewLine().Should().Be(expected); + } + [TestCase("to_camel_case_function", "toCamelCaseFunction")] [TestCase("My S3 Bucket", "myS3Bucket")] [TestCase("-only-$AlphaNUMERIC-characters%^", "onlyAlphanumericCharacters")] diff --git a/source/Calamari/ArgoCD/Helm/HelmYamlParser.cs b/source/Calamari/ArgoCD/Helm/HelmYamlParser.cs index 1d8061a30a..27948ca441 100644 --- a/source/Calamari/ArgoCD/Helm/HelmYamlParser.cs +++ b/source/Calamari/ArgoCD/Helm/HelmYamlParser.cs @@ -23,7 +23,7 @@ public HelmYamlParser(string yamlContent) var reader = new StringReader(yamlString); yamlStream = new YamlStream(); yamlStream.Load(reader); - endsWithNewline = yamlString.EndsWith("\n") || yamlString.EndsWith("\r"); + endsWithNewline = yamlString.HasTrailingNewLine(); } readonly string yamlString; diff --git a/source/Calamari/ArgoCD/YamlStreamLoader.cs b/source/Calamari/ArgoCD/YamlStreamLoader.cs index c9a3ad5394..e7b5cc00d9 100644 --- a/source/Calamari/ArgoCD/YamlStreamLoader.cs +++ b/source/Calamari/ArgoCD/YamlStreamLoader.cs @@ -112,7 +112,7 @@ public static string SerializeDocuments(IEnumerable documents, str var serializedDocs = documentList.Select(doc => SerializeDocument(doc, newLine)); var joined = string.Join($"{newLine}---{newLine}", serializedDocs); - return EndsWithNewLine(originalContent) ? joined + newLine : joined; + return originalContent.HasTrailingNewLine() ? joined + newLine : joined; } static string SerializeDocument(YamlDocument document, string newLine) @@ -127,10 +127,5 @@ static string SerializeDocument(YamlDocument document, string newLine) ? serialized.Substring(0, serialized.Length - 3).TrimEnd() : serialized; } - - static bool EndsWithNewLine(string? content) - { - return content != null && (content.EndsWith("\n") || content.EndsWith("\r")); - } } } \ No newline at end of file From 9ccd29135663e311b5435bcd593f63eb9e656d83 Mon Sep 17 00:00:00 2001 From: Frank Lin Date: Thu, 17 Sep 2026 08:40:18 +1000 Subject: [PATCH 4/7] Assert line endings are preserved, not that they match the agent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TwoImagesWithSameTag_OnlyUpdatesConfiguredPath asserted the output contained Environment.NewLine. Because .gitattributes checks .cs files out with native endings, the YAML literal's endings always equalled Environment.NewLine, so the assertion held on every platform and could not tell "preserved the file's endings" from "rewrote them to the agent's" — the two only differ for content that does not match the platform, which is exactly the reported case of an LF file on a Windows agent. It now spells the line endings out, runs for both LF and CRLF, and compares the whole result, so only the configured tag may change. A second case covers the reported symptom directly: CRLF with no trailing newline. Reverting the parser fix now fails both, where previously it failed neither. Co-Authored-By: Claude Opus 5 (1M context) --- ...elmValuesImageReplaceStepVariablesTests.cs | 39 +++++++++++++------ 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/source/Calamari.Tests/ArgoCD/Helm/HelmValuesImageReplaceStepVariablesTests.cs b/source/Calamari.Tests/ArgoCD/Helm/HelmValuesImageReplaceStepVariablesTests.cs index e68151e7b3..0d5242d0b2 100644 --- a/source/Calamari.Tests/ArgoCD/Helm/HelmValuesImageReplaceStepVariablesTests.cs +++ b/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,17 @@ public void StructuredValue_ImageOnNonDefaultRegistry_UpdatesFullRefAndTracksWit result.UpdatedContents.Should().Contain("name: us-docker.pkg.dev/shared-gke-dev-gqtrxy/argo-test/helloworld:v2"); } + // 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. [Test] - public void TwoImagesWithSameTag_OnlyUpdatesConfiguredPath() + [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 { @@ -139,8 +139,25 @@ 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] + public void UpdatesTag_PreservingCrlfAndTheAbsenceOfATrailingNewline() + { + const string yaml = "image:\r\n tag: 1.0"; + + var replacer = new HelmValuesImageReplaceStepVariables(yaml, DefaultRegistry, log); + var images = new List + { + new(ContainerImageReference.FromReferenceString("nginx:1.27.1", DefaultRegistry), "image.tag") + }; + + var result = replacer.UpdateImages(images); + + result.UpdatedContents.Should().Be("image:\r\n tag: 1.27.1"); } [Test] From 8d01ff896560d47075c1d5042ce13424a7af7231 Mon Sep 17 00:00:00 2001 From: Frank Lin Date: Thu, 17 Sep 2026 08:54:51 +1000 Subject: [PATCH 5/7] Assert line endings exactly in the remaining Helm value tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five tests compared both sides through ReplaceLineEndings, so they passed whatever the replacer did to line endings. They now spell the endings out, run for both LF and CRLF, and compare the whole result, which keeps their original subjects — quote preservation and trailing whitespace — while also pinning the endings. UpdatesTag_PreservingCrlfAndTheAbsenceOfATrailingNewline now mirrors TwoImagesWithSameTag_OnlyUpdatesConfiguredPath without the trailing newline, so the pair differs in one variable rather than in shape, and covers both endings instead of only CRLF. Reverting the parser fix fails nine of these, where before this branch it failed none. Co-Authored-By: Claude Opus 5 (1M context) --- .../ArgoCD/Helm/HelmValuesEditorTests.cs | 43 +++++----- ...elmValuesImageReplaceStepVariablesTests.cs | 16 ++-- .../ArgoCD/Helm/HelmYamlParserTests.cs | 80 +++++-------------- 3 files changed, 53 insertions(+), 86 deletions(-) diff --git a/source/Calamari.Tests/ArgoCD/Helm/HelmValuesEditorTests.cs b/source/Calamari.Tests/ArgoCD/Helm/HelmValuesEditorTests.cs index de6d2736c0..443bd28da2 100644 --- a/source/Calamari.Tests/ArgoCD/Helm/HelmValuesEditorTests.cs +++ b/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", + "")); } } } diff --git a/source/Calamari.Tests/ArgoCD/Helm/HelmValuesImageReplaceStepVariablesTests.cs b/source/Calamari.Tests/ArgoCD/Helm/HelmValuesImageReplaceStepVariablesTests.cs index 0d5242d0b2..c063f5e888 100644 --- a/source/Calamari.Tests/ArgoCD/Helm/HelmValuesImageReplaceStepVariablesTests.cs +++ b/source/Calamari.Tests/ArgoCD/Helm/HelmValuesImageReplaceStepVariablesTests.cs @@ -122,7 +122,6 @@ public void StructuredValue_ImageOnNonDefaultRegistry_UpdatesFullRefAndTracksWit // .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. - [Test] [TestCase("\n")] [TestCase("\r\n")] public void TwoImagesWithSameTag_OnlyUpdatesConfiguredPath(string newLine) @@ -144,20 +143,25 @@ public void TwoImagesWithSameTag_OnlyUpdatesConfiguredPath(string newLine) .Be(string.Join(newLine, "", "nginx:", " tag: 1.27.1", "redis:", " tag: 1.0", "")); } - [Test] - public void UpdatesTag_PreservingCrlfAndTheAbsenceOfATrailingNewline() + [TestCase("\n")] + [TestCase("\r\n")] + public void TwoImagesWithSameTag_WithoutATrailingNewline_OnlyUpdatesConfiguredPath(string newLine) { - const string yaml = "image:\r\n 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 { - new(ContainerImageReference.FromReferenceString("nginx:1.27.1", DefaultRegistry), "image.tag") + new(ContainerImageReference.FromReferenceString("nginx:1.27.1", DefaultRegistry), "nginx.tag") }; var result = replacer.UpdateImages(images); - result.UpdatedContents.Should().Be("image:\r\n tag: 1.27.1"); + 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")); } [Test] diff --git a/source/Calamari.Tests/ArgoCD/Helm/HelmYamlParserTests.cs b/source/Calamari.Tests/ArgoCD/Helm/HelmYamlParserTests.cs index 88f110e0b6..5ba552469f 100644 --- a/source/Calamari.Tests/ArgoCD/Helm/HelmYamlParserTests.cs +++ b/source/Calamari.Tests/ArgoCD/Helm/HelmYamlParserTests.cs @@ -51,96 +51,56 @@ 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"); - //ensure platform-agnostic multiline comparison - result.ReplaceLineEndings().Should().Be(expectedUpdate.ReplaceLineEndings()); + result.Should().Be(string.Join(newLine, "", "root:", " node1: 69", " node2: stable", "")); } - [Test] - public void UpdateNodeValue_WithDoubleQuoteDelimitedNodeValue_PreservesDelimitersWithNewValue() + [TestCase("\n")] + [TestCase("\r\n")] + public void UpdateNodeValue_WithDoubleQuoteDelimitedNodeValue_PreservesDelimitersWithNewValue(string newLine) { - const string yamlContent = @" -root: - node1: 42 - node2: ""latest"" -"; + var yamlContent = string.Join(newLine, "", "root:", " node1: 42", " node2: \"latest\"", ""); var sut = new HelmYamlParser(yamlContent); - const string expectedUpdate = @" -root: - node1: 42 - node2: ""stable"" -"; - var result = sut.UpdateContentForPath("root.node2", "stable"); - //ensure platform-agnostic multiline comparison - result.ReplaceLineEndings().Should().Be(expectedUpdate.ReplaceLineEndings()); + result.Should().Be(string.Join(newLine, "", "root:", " node1: 42", " node2: \"stable\"", "")); } - [Test] - public void UpdateNodeValue_WithSingleQuoteDelimitedNodeValue_PreservesDelimitersWithNewValue() + [TestCase("\n")] + [TestCase("\r\n")] + public void UpdateNodeValue_WithSingleQuoteDelimitedNodeValue_PreservesDelimitersWithNewValue(string newLine) { - const string yamlContent = @" -root: - node1: 42 - node2: 'latest' -"; + var yamlContent = string.Join(newLine, "", "root:", " node1: 42", " node2: 'latest'", ""); var sut = new HelmYamlParser(yamlContent); - const string expectedUpdate = @" -root: - node1: 42 - node2: 'stable' -"; - var result = sut.UpdateContentForPath("root.node2", "stable"); - //ensure platform-agnostic multiline comparison - result.ReplaceLineEndings().Should().Be(expectedUpdate.ReplaceLineEndings()); + result.Should().Be(string.Join(newLine, "", "root:", " node1: 42", " node2: 'stable'", "")); } - [Test] - public void UpdateNodeValue_RespectsTrailingWhitespaceFromInput() + [TestCase("\n")] + [TestCase("\r\n")] + public void UpdateNodeValue_RespectsTrailingWhitespaceFromInput(string newLine) { - const string yamlContent = @" -root: - node1: 42 - -"; + var yamlContent = string.Join(newLine, "", "root:", " node1: 42", " ", ""); var sut = new HelmYamlParser(yamlContent); - const string expectedUpdate = @" -root: - node1: 69 - -"; - 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] From bad625f6084984a2d53f270e8f0bc2297c057439 Mon Sep 17 00:00:00 2001 From: Nick Josevski Date: Tue, 8 Sep 2026 21:01:50 +1000 Subject: [PATCH 6/7] Run gzip directly in BuildDockerImages instead of through pwsh BuildDockerImages launched PowerShell for exactly one thing: running `gzip -k -9 -f` on the OCI tar. That made the step depend on whichever .NET runtime the agent's `pwsh` global tool was built against, and on main's build agent those no longer line up: App: /root/.dotnet/tools/pwsh Framework: 'Microsoft.NETCore.App', version '10.0.0' .NET location: .../.nuke/temp/dotnet-unix The following frameworks were found: 8.0.30 The agent's `pwsh` needs .NET 10, and the only runtime on offer is the .NET 8 SDK that build.sh bootstraps into .nuke/temp and puts on PATH. Calling gzip directly removes pwsh from the equation. This was the build's only use of PowerShellTasks, so nothing else in the build cares about the agent's pwsh now. Failures also surface properly. `pwsh -Command` exits 0 regardless of the native exit code, so a failed gzip used to show up later as a confusing missing-artifact error from PublishArtifacts. A Nuke Tool asserts a zero exit code, so it now fails at the gzip call with gzip's stderr attached. Verified with a throwaway target: resolves /usr/bin/gzip from PATH, arguments pass through intact, -k keeps the .tar alongside the .gz, and a deliberate failure raises `ProcessException: Process 'gzip' exited with code 1`. Not addressed here: reaching the SDK bootstrap at all means `dotnet --version` failed, so the agent no longer satisfies global.json's 8.0.419 pin. That costs every build a full SDK download and belongs with the .NET 10 work. Co-Authored-By: Claude Opus 5 (1M context) (cherry picked from commit edee954078116d951e06f51db3c767056765b1f1) --- build/Build.Docker.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/build/Build.Docker.cs b/build/Build.Docker.cs index c9bf2006f4..585b83b93a 100644 --- a/build/Build.Docker.cs +++ b/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"; From 3e2599006be49cedb8d2ae5f8013cabce0fd6d7a Mon Sep 17 00:00:00 2001 From: Frank Lin Date: Thu, 17 Sep 2026 11:29:00 +1000 Subject: [PATCH 7/7] Declare test YAML as literals instead of joining lines Reads the YAML fixtures from raw string literals with ReplaceLineEndings rather than string.Join, so the documents read as YAML and the quote delimiters need no escaping. Also splits the line-ending cases out of TwoImagesWithSameTag_OnlyUpdatesConfiguredPath, which no longer varies them. Co-Authored-By: Claude Opus 5 (1M context) --- .../ArgoCD/Helm/HelmValuesEditorTests.cs | 40 ++++---- ...elmValuesImageReplaceStepVariablesTests.cs | 94 +++++++++++++++---- .../ArgoCD/Helm/HelmYamlParserTests.cs | 58 ++++++++++-- 3 files changed, 145 insertions(+), 47 deletions(-) diff --git a/source/Calamari.Tests/ArgoCD/Helm/HelmValuesEditorTests.cs b/source/Calamari.Tests/ArgoCD/Helm/HelmValuesEditorTests.cs index 443bd28da2..847cce44ee 100644 --- a/source/Calamari.Tests/ArgoCD/Helm/HelmValuesEditorTests.cs +++ b/source/Calamari.Tests/ArgoCD/Helm/HelmValuesEditorTests.cs @@ -102,28 +102,30 @@ public void GenerateVariableDictionary_ReturnsDictionaryOfNodeValuesWithValues() [TestCase("\r\n")] public void UpdateNodeValue_ReturnsModifiedYaml(string newLine) { - var yamlContent = string.Join(newLine, - "root:", - " node1: \"node1value\"", - " node2:", - " node2Nest:", - " node2nestedValue: \"banana\"", - " node2Child1: \"node2child1value\"", - " node2Child2: 42", - ""); + var yamlContent = """ + root: + node1: "node1value" + node2: + node2Nest: + node2nestedValue: "banana" + node2Child1: "node2child1value" + node2Child2: 42 + + """.ReplaceLineEndings(newLine); var result = HelmValuesEditor.UpdateNodeValue(yamlContent, "root.node1", "awesome new value"); - result.Should() - .Be(string.Join(newLine, - "root:", - " node1: \"awesome new value\"", - " node2:", - " node2Nest:", - " node2nestedValue: \"banana\"", - " node2Child1: \"node2child1value\"", - " node2Child2: 42", - "")); + var expected = """ + root: + node1: "awesome new value" + node2: + node2Nest: + node2nestedValue: "banana" + node2Child1: "node2child1value" + node2Child2: 42 + + """.ReplaceLineEndings(newLine); + result.Should().Be(expected); } } } diff --git a/source/Calamari.Tests/ArgoCD/Helm/HelmValuesImageReplaceStepVariablesTests.cs b/source/Calamari.Tests/ArgoCD/Helm/HelmValuesImageReplaceStepVariablesTests.cs index c063f5e888..9089edf749 100644 --- a/source/Calamari.Tests/ArgoCD/Helm/HelmValuesImageReplaceStepVariablesTests.cs +++ b/source/Calamari.Tests/ArgoCD/Helm/HelmValuesImageReplaceStepVariablesTests.cs @@ -1,4 +1,4 @@ -using System.Collections.Generic; +using System.Collections.Generic; using Calamari.ArgoCD; using Calamari.ArgoCD.Conventions; using Calamari.ArgoCD.Models; @@ -118,15 +118,17 @@ public void StructuredValue_ImageOnNonDefaultRegistry_UpdatesFullRefAndTracksWit result.UpdatedContents.Should().Contain("name: us-docker.pkg.dev/shared-gke-dev-gqtrxy/argo-test/helloworld:v2"); } - // 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) + [Test] + public void TwoImagesWithSameTag_OnlyUpdatesConfiguredPath() { - var yaml = string.Join(newLine, "", "nginx:", " tag: 1.0", "redis:", " tag: 1.0", ""); + const string yaml = """ + + nginx: + tag: 1.0 + redis: + tag: 1.0 + + """; var replacer = new HelmValuesImageReplaceStepVariables(yaml, DefaultRegistry, log); var images = new List @@ -136,18 +138,36 @@ public void TwoImagesWithSameTag_OnlyUpdatesConfiguredPath(string newLine) var result = replacer.UpdateImages(images); + const string expectedYaml = """ + + nginx: + tag: 1.27.1 + redis: + tag: 1.0 + + """; + 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", "")); + result.UpdatedContents.ReplaceLineEndings("\n").Should().Be(expectedYaml.ReplaceLineEndings("\n")); } + // The endings are forced with ReplaceLineEndings rather than inherited from the literal: + // .gitattributes checks .cs files out with native endings, so an inherited ending always matches + // Environment.NewLine and the assertion cannot distinguish "preserved the input's endings" from + // "used the agent's". The customer's case was an LF file on a Windows agent. [TestCase("\n")] [TestCase("\r\n")] - public void TwoImagesWithSameTag_WithoutATrailingNewline_OnlyUpdatesConfiguredPath(string newLine) + public void UpdatedYaml_PreservesTheInputLineEndings(string newLine) { - var yaml = string.Join(newLine, "", "nginx:", " tag: 1.0", "redis:", " tag: 1.0"); + var yaml = """ + + nginx: + tag: 1.0 + redis: + tag: 1.0 + + """.ReplaceLineEndings(newLine); var replacer = new HelmValuesImageReplaceStepVariables(yaml, DefaultRegistry, log); var images = new List @@ -157,11 +177,47 @@ public void TwoImagesWithSameTag_WithoutATrailingNewline_OnlyUpdatesConfiguredPa 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")); + var expectedYaml = """ + + nginx: + tag: 1.27.1 + redis: + tag: 1.0 + + """.ReplaceLineEndings(newLine); + + result.UpdatedContents.Should().Be(expectedYaml); + } + + [TestCase("\n")] + [TestCase("\r\n")] + public void UpdatedYaml_WithoutATrailingNewline_DoesNotAddOne(string newLine) + { + var yaml = """ + + nginx: + tag: 1.0 + redis: + tag: 1.0 + """.ReplaceLineEndings(newLine); + + var replacer = new HelmValuesImageReplaceStepVariables(yaml, DefaultRegistry, log); + var images = new List + { + new(ContainerImageReference.FromReferenceString("nginx:1.27.1", DefaultRegistry), "nginx.tag") + }; + + var result = replacer.UpdateImages(images); + + var expectedYaml = """ + + nginx: + tag: 1.27.1 + redis: + tag: 1.0 + """.ReplaceLineEndings(newLine); + + result.UpdatedContents.Should().Be(expectedYaml); } [Test] diff --git a/source/Calamari.Tests/ArgoCD/Helm/HelmYamlParserTests.cs b/source/Calamari.Tests/ArgoCD/Helm/HelmYamlParserTests.cs index 5ba552469f..c26b92895a 100644 --- a/source/Calamari.Tests/ArgoCD/Helm/HelmYamlParserTests.cs +++ b/source/Calamari.Tests/ArgoCD/Helm/HelmYamlParserTests.cs @@ -1,4 +1,4 @@ -using System; +using System; using Calamari.ArgoCD.Helm; using FluentAssertions; using NUnit.Framework; @@ -55,52 +55,92 @@ public void GetValueAtPath_ReturnsTheValueOfTheSpecifiedNode(string path, string [TestCase("\r\n")] public void UpdateNodeValue_WithNonDelimitedNodeValue_ReplacesValueInDocument(string newLine) { - var yamlContent = string.Join(newLine, "", "root:", " node1: 42", " node2: stable", ""); + var yamlContent = """ + + root: + node1: 42 + node2: stable + + """.ReplaceLineEndings(newLine); var sut = new HelmYamlParser(yamlContent); var result = sut.UpdateContentForPath("root.node1", "69"); - result.Should().Be(string.Join(newLine, "", "root:", " node1: 69", " node2: stable", "")); + var expected = """ + + root: + node1: 69 + node2: stable + + """.ReplaceLineEndings(newLine); + result.Should().Be(expected); } [TestCase("\n")] [TestCase("\r\n")] public void UpdateNodeValue_WithDoubleQuoteDelimitedNodeValue_PreservesDelimitersWithNewValue(string newLine) { - var yamlContent = string.Join(newLine, "", "root:", " node1: 42", " node2: \"latest\"", ""); + var yamlContent = """ + + root: + node1: 42 + node2: "latest" + + """.ReplaceLineEndings(newLine); var sut = new HelmYamlParser(yamlContent); var result = sut.UpdateContentForPath("root.node2", "stable"); - result.Should().Be(string.Join(newLine, "", "root:", " node1: 42", " node2: \"stable\"", "")); + var expected = """ + + root: + node1: 42 + node2: "stable" + + """.ReplaceLineEndings(newLine); + result.Should().Be(expected); } [TestCase("\n")] [TestCase("\r\n")] public void UpdateNodeValue_WithSingleQuoteDelimitedNodeValue_PreservesDelimitersWithNewValue(string newLine) { - var yamlContent = string.Join(newLine, "", "root:", " node1: 42", " node2: 'latest'", ""); + var yamlContent = """ + + root: + node1: 42 + node2: 'latest' + + """.ReplaceLineEndings(newLine); var sut = new HelmYamlParser(yamlContent); var result = sut.UpdateContentForPath("root.node2", "stable"); - result.Should().Be(string.Join(newLine, "", "root:", " node1: 42", " node2: 'stable'", "")); + var expected = """ + + root: + node1: 42 + node2: 'stable' + + """.ReplaceLineEndings(newLine); + result.Should().Be(expected); } [TestCase("\n")] [TestCase("\r\n")] public void UpdateNodeValue_RespectsTrailingWhitespaceFromInput(string newLine) { - var yamlContent = string.Join(newLine, "", "root:", " node1: 42", " ", ""); + var yamlContent = "\nroot:\n node1: 42\n \n".ReplaceLineEndings(newLine); var sut = new HelmYamlParser(yamlContent); var result = sut.UpdateContentForPath("root.node1", "69"); - result.Should().Be(string.Join(newLine, "", "root:", " node1: 69", " ", "")); + var expected = "\nroot:\n node1: 69\n \n".ReplaceLineEndings(newLine); + result.Should().Be(expected); } [Test]