Refactor Template Step to use App and AppSource Updates by rain-on · Pull Request #1829 · OctopusDeploy/Calamari

rain-on · 2026-03-12T05:43:00Z

This splits up the UpdateArgoCDApplicationManifestsInstallConvention into 2 extra classes:

UpdateArgoCDApplicationManifestsInstallConvention
- Responsible for parsing in put data, and spawning the applicationUpdater
ApplicationUpdater
- Validating application content, and triggering the update of each internal source
ApplicationSourceUpdater
- Create repository
- Copy templates files into repository
- Purge files if necessary
- Raise output variables

This should be reviewed when prior refactoring is merged (is built atop).

⚠️ Does this change require a corresponding Server Change?
⚠️ If so - please add a "Requires Server Change" label to this PR!

flin-8 · 2026-03-13T07:25:41Z

source/Calamari/ArgoCD/Git/RepositoryWrapper.cs

-
-        public void StageFiles(string[] filesToStage)
+        
+        public void UnstageFiles(string[] filesToRemove)


Suggest we rename StageFiles/UnstageFiles to AddFiles/RemoveFiles

Unstage means you have local changes but you are excluding them from the commit

flin-8 · 2026-03-13T07:27:31Z

source/Calamari/ArgoCD/Conventions/UpdateImageTag/FileUpdateResult.cs

 namespace Calamari.ArgoCD.Conventions.UpdateImageTag;

-public record FileUpdateResult(HashSet<string> UpdatedImages, List<FilePathContent> PatchedFileContent);
+public record FileUpdateResult(bool hasChanges, HashSet<string> UpdatedImages, List<FilePathContent> PatchedFileContent, string[]? FilesRemoved = null);


The HasChanges flag should be computed right?

🤔 Prior to this, if there were UpdatedImages - then we determined that a change was made and should be commited.
Except that doesn't work for templating step (which will never results in changed images) - but maybe we could just say "if either of the files-changed list is non-empty)... which would make this much cleaner.

flin-8 · 2026-03-13T07:29:56Z

source/Calamari/ArgoCD/Conventions/UpdateImageTag/FileUpdateResult.cs

 namespace Calamari.ArgoCD.Conventions.UpdateImageTag;

-public record FileUpdateResult(HashSet<string> UpdatedImages, List<FilePathContent> PatchedFileContent);
+public record FileUpdateResult(bool hasChanges, HashSet<string> UpdatedImages, List<FilePathContent> PatchedFileContent, string[]? FilesRemoved = null);


Also should we be sharing FileUpdateResult ? UpdatedImages will never be populated for the manifest step etc.

It was really useful to find a common type so that we could use for both steps :/ Maybe I can find a nicer solution 👍

So turns out - the UpdatedImages ends up in Octopus, to be added to an event in AMplitude - and defaults to "0" if the UpdatedImages Variable is not populated.

source/Calamari/ArgoCD/Conventions/ManifestTemplating/CopyTemplatesSourceUpdater.cs

flin-8

LGTM

flin-8 · 2026-03-17T00:16:19Z

source/Calamari/ArgoCD/Conventions/ManifestTemplating/CopyTemplatesSourceUpdater.cs

+        log.Info($"Removing files recursively from {outputDirectory}");
        //TODO(tmm): Knowing we're in a git repository is a bit of a smell :(
-        var gitDir = Path.Combine(cleansedSubPath, ".git");
+        // var gitDir = Path.Combine(cleansedSubPath, ".git");


Commented code

flin-8 · 2026-03-17T00:17:08Z

source/Calamari/ArgoCD/Conventions/UpdateImageTag/FileUpdateResult.cs

 namespace Calamari.ArgoCD.Conventions.UpdateImageTag;

-public record FileUpdateResult(bool hasChanges, HashSet<string> UpdatedImages, List<FilePathContent> PatchedFileContent, string[]? FilesRemoved = null);
+public record FileUpdateResult(HashSet<string> UpdatedImages, List<FilePathContent> PatchedFileContent, string[]? FilesRemoved = null)


Might as well make FilesRemoved non-nullable

Maybe create static factory methods to aid creating this record for image-tag vs. manifest use

going to avoid the factory method for now - being explicit in the "no files removed" is not a bad thin to have.
The field was nullable to minimise the files changed - but its probably sensible to make it explicit.

flin-8 · 2026-03-17T12:39:30Z

source/Calamari.Tests/ArgoCD/Git/RepositoryExtensionMethods.cs

        {
-            var fileTreeEntry = repo.Branches[branchName.Value].Tip[filename];
+            //filename will always be posix-compliant
+            var fileTreeEntry = repo.Branches[branchName.Value].Tip[filename.Replace(Path.DirectorySeparatorChar, '/')];


Do you know if this filename also needs to be stripped of the leading "./"?

I think, the way we're using it in the tests - the answer is "no" - but that's luck.
The reason we have a problem in production code is when the argo app's path is "./" :(

rain-on added 30 commits March 11, 2026 14:47

its looking pretty good

0269ee0

sort of?

765eb80

DirectoryUpdater looks about right

86fb93e

added empty updaters

c70942b

half way with ref updating

35e6d26

Kustomize is in - now for ref

b07dfd9

created updaterHelpers

1f67816

ref is compiling

51df92d

created an intermediate helmupdater its nasty

8e74fa2

compiling - wonder what happens with the main convention

b0ef6a3

moved everything

fbe1a5d

added a repositoryAdapter

d07ff17

split out the repo and outputvariables stuff

fe5dc4a

sort out compilation

bdaaff8

fix the null whoopsie

ca8752f

yay

690e254

clean up

e10f532

catch the lost convention

4ed8def

slow tidy up after review

023f9a8

tidy it up even more

2aa02c3

and more

535baed

cleaning

54558bd

removed updatdfiles and rely on patches

233d297

make file list distinct

766ab76

added authenticatingrepositoryfactor

f449e19

moved things around a bit

20cd4c8

clean up imports

ae52c8e

move files into a subdirectory under Conventions

96bdae1

fix namespacing

e75926c

its mostly good

a55c7f9

rain-on added 2 commits March 13, 2026 17:03

And the Templating is way nicer

3c10736

add comment

dbf87e8

flin-8 reviewed Mar 13, 2026

View reviewed changes

flin-8 reviewed Mar 16, 2026

View reviewed changes

source/Calamari/ArgoCD/Conventions/ManifestTemplating/CopyTemplatesSourceUpdater.cs Outdated Show resolved Hide resolved

flin-8 approved these changes Mar 16, 2026

View reviewed changes

rain-on added 10 commits March 16, 2026 12:55

looks ok

5046d60

minor tidy up

1966820

fix compilation issues

bd7df91

sorted

f55822d

back it out

7a817e3

remove the slash which wasn't going to work

8cfe54f

I hate windows

d082bfe

fix compilation

8103b86

tidy up

d5a0e37

fixed the pathing issues, wild.

799533e

flin-8 reviewed Mar 17, 2026

View reviewed changes

rain-on added 4 commits March 17, 2026 15:32

create a test for pathing

fb68452

make everything posix-ee

53b5c3d

normalize to posix deliberately

15f4ecf

force removed files

57322e9

flin-8 reviewed Mar 17, 2026

View reviewed changes

rain-on added 4 commits March 18, 2026 09:38

fix up the relative path error

c05bc45

revert the normalizePath in repositorywrapper

c6b9a72

always force repositoryWrapper to use forward slash

92a193b

fix normalize yet again

526c6e2

rain-on merged commit c8522f8 into main Mar 19, 2026
33 checks passed

rain-on deleted the tmm/refactor_other_step branch March 19, 2026 02:21


		public void StageFiles(string[] filesToStage)

		public void UnstageFiles(string[] filesToRemove)

Conversation

rain-on commented Mar 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

flin-8 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants