Merged
Conversation
0a3e4da to
2044772
Compare
2044772 to
dbbb1a9
Compare
dc561ba to
51181a3
Compare
…atches-only-newtag-could-we
Contributor
|
There's a few smells in the KustomizeUpdater which should be easy to fix. But a few general thoughts:
|
rain-on
reviewed
Apr 1, 2026
| } | ||
|
|
||
|
|
||
| internal PatchType? DeterminePatchTypeFromFile(string content) |
Contributor
There was a problem hiding this comment.
generally we don't use internal - I'd look at removing them across this whole PR I think
source/Calamari/ArgoCD/Conventions/UpdateImageTag/KustomizeUpdater.cs
Outdated
Show resolved
Hide resolved
source/Calamari/ArgoCD/Conventions/UpdateImageTag/KustomizeContainerImageReplacer.cs
Outdated
Show resolved
Hide resolved
source/Calamari/ArgoCD/Conventions/UpdateImageTag/KustomizeContainerImageReplacer.cs
Outdated
Show resolved
Hide resolved
…-files-do-not-support-patches-only-newtag-could-we
rain-on
approved these changes
Apr 2, 2026
| /// Handles updating container image references in inline patches within kustomization.yaml files. | ||
| /// The patches field contains an array of patch objects with inline patch operations. | ||
| /// </summary> | ||
| public class InlineJsonPatchImageReplacer : IContainerImageReplacer |
Contributor
There was a problem hiding this comment.
dont love the "InlineJsonPatch" - Maybe just InlinepatchImageReplacer?
| /// <summary> | ||
| /// Helper class for loading and parsing YAML content | ||
| /// </summary> | ||
| public static class YamlStreamLoader |
Contributor
There was a problem hiding this comment.
kinda surprised we need this class - but also, not really :)
| { | ||
| public static class YamlStreamLoader | ||
| { | ||
| public static YamlStream? TryLoad(string yamlContent, ILog log, string context) |
Contributor
There was a problem hiding this comment.
do we need this, if we have the YamlStreamLoader?
| // We know this won't be null after parse | ||
| new (ContainerImageReference.FromReferenceString("nginx:1.25", ArgoCDConstants.DefaultContainerRegistry)), | ||
| new (ContainerImageReference.FromReferenceString("busybox:stable", "my-registry.com")), | ||
| new (ContainerImageReference.FromReferenceString("my-registry.com/busybox:stable", ArgoCDConstants.DefaultContainerRegistry)), |
Contributor
There was a problem hiding this comment.
this shouldn't have needed changing - this example should still work
…atches-only-newtag-could-we
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds support for Kustomize patching in ArgoCD deployments. The PR implements patch discovery and image replacement capabilities for different patch types:
Testing:
Tested local Calamari with an local Octopus Server instance:
The changed commit a2defd6f2a showed that it consistently updated the image tags properly across all scenarios.