Skip to content

Support patching for kustomize#1835

Open
octonautcal wants to merge 51 commits intomainfrom
cal/md-1462-kustomize-files-do-not-support-patches-only-newtag-could-we
Open

Support patching for kustomize#1835
octonautcal wants to merge 51 commits intomainfrom
cal/md-1462-kustomize-files-do-not-support-patches-only-newtag-could-we

Conversation

@octonautcal
Copy link
Copy Markdown
Contributor

@octonautcal octonautcal commented Mar 17, 2026

Adds support for Kustomize patching in ArgoCD deployments. The PR implements patch discovery and image replacement capabilities for different patch types:

  • JSON6902 patch
  • Strategic merge patch
  • Inline JSON patch

Testing:

Tested local Calamari with an local Octopus Server instance:

Screenshot 2026-03-24 at 2 01 42 pm

The changed commit a2defd6f2a showed that it consistently updated the image tags properly across all scenarios.

@octonautcal octonautcal force-pushed the cal/md-1462-kustomize-files-do-not-support-patches-only-newtag-could-we branch from 0a3e4da to 2044772 Compare March 17, 2026 05:01
@octonautcal octonautcal force-pushed the cal/md-1462-kustomize-files-do-not-support-patches-only-newtag-could-we branch from 2044772 to dbbb1a9 Compare March 17, 2026 05:10
@octonautcal octonautcal force-pushed the cal/md-1462-kustomize-files-do-not-support-patches-only-newtag-could-we branch from dc561ba to 51181a3 Compare March 23, 2026 04:28
@octonautcal octonautcal marked this pull request as ready for review March 24, 2026 03:03
@rain-on
Copy link
Copy Markdown
Contributor

rain-on commented Mar 24, 2026

There's a few smells in the KustomizeUpdater which should be easy to fix. But a few general thoughts:

  1. The selection of files process is really nice 👍
  2. Pulling Update into the KustomizeUpdater is not great - I know its needed for this design to work, as you need currentFile - but it'd be good to avoid.
  3. The currentFile property feels a bit dangerous - esp given that it needs to set and unset - means we've got something wrong with lifetime management I suspect.
    3.1 On that topic - we shouldn't need currentFile - yes it can be used as a short-circuit to determine the type of file being updated - but we could infer the file-type from the provided content (and hide it behind a function so we don't need to look at it too often).
  4. KustomizeUpdater might need to grow a bit, but going from ~64 --> ~450 is probably too much, hopefully a lot of that could be extracted to a separate class.

@octonautcal octonautcal force-pushed the cal/md-1462-kustomize-files-do-not-support-patches-only-newtag-could-we branch from 0991d49 to c42805b Compare March 25, 2026 00:46
Copy link
Copy Markdown
Contributor

@rain-on rain-on left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still need to review the tests - but generally like the layout 👍
If you find any of the comments disingenuous, leave them out, we can work through them in person

}
else
{
var patchType = DeterminePatchTypeFromFile(input);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is nice :)
You COULD break this block out to a separate function too but that's just an opinion.

{

var hasOpField = System.Text.RegularExpressions.Regex.IsMatch(content,
@"[""']?op[""']?\s*:\s*[""']?(add|remove|replace|move|copy|test)[""']?",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I LOVE regular expressions :) But no one else does - is there any easy way to do this without the regex?

@octonautcal octonautcal force-pushed the cal/md-1462-kustomize-files-do-not-support-patches-only-newtag-could-we branch from db12f10 to 81b7372 Compare March 30, 2026 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants