Conversation
scme0
force-pushed
the
scme/refactor-helm-image-replacement
branch
from
ef9c058 to
f1da315
Compare
| AssertNotUpdatedWithExpectedPatch(getCapturedResults, | ||
| expectedPatchPointer: "/0/image/tag", | ||
| expectedPatchValue: "1.27.1", | ||
| expectedPatchedFilePath: Path.Combine("otherRepoPath", "values.yaml")); |
There was a problem hiding this comment.
We had a weird round trip in the original code where we would take file paths from relative -> absolute -> relative which meant that the paths were normalised to the machine (windows and nix). We don't actually need to do this because calamari handles that down stream automatically. This refactor removes the round-trip so the path here can be a normal string with explicit file path separator.
|
|
||
| public static class HelmHelpers | ||
| { | ||
| public static void LogHelmSourceConfigurationProblems(ILog log, IReadOnlyCollection<HelmSourceConfigurationProblem> helmSourceConfigurationProblems) |
There was a problem hiding this comment.
After refactor, it was only used in AbstractHelmUpdater once so I made it a private static method in that file.
| { | ||
| if (valueToUpdate == newImageTag.ContainerReference.Tag) | ||
| { | ||
| alreadyUpToDate.Add(newImageTag.ContainerReference.Tag); |
There was a problem hiding this comment.
Can you see if this is a real bug? Add test?
There was a problem hiding this comment.
I've extracted the tests I added to find this and the fix into this PR: #1896
scme0
force-pushed
the
scme/refactor-helm-image-replacement
branch
from
002cad1 to
9cbfdd3
Compare
AbstractHelmUpdater.cs retained its filename (rename didn't apply cleanly) and needed MakePlaceholderRef updated to JsonPatchUtils.MakePlaceholderRef. HelmValuesImageReplaceStepVariablesTests.cs was missing the Calamari.ArgoCD using directive added during conflict resolution. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
scme0
force-pushed
the
scme/refactor-helm-image-replacement
branch
from
9cbfdd3 to
8798cf3
Compare
2 participants
We had two distinct processing paths for Helm - one via annotations and one via variables. This refactor unifies them - the only difference is the annotations are preprocessed into image paths so the image replacer can do its thing!
I realised as well that the AbstractHelmUpdater didn't have an "is-a" relationship with BaseUpdater anymore, so I removed the inheritance and move the one static helper method to a static utils class so it can be shared. Also renamed to BaseHelmUpdater because it's now the base.