Keep yaml sequence indentation when writing back to Git to a kustomize file

[AlessandroZanatta]

Hi,

I've created a PR that should solve #501. First time I (really) contribute to an open-source project, so feel free to criticize anything I may have done wrong!

I have based this PR on the v0.15.2 branch instead of master as master was failing for me (throwing panics at runtime in my cluster). Let me know whether this is fine.

I've basically changed the way the updateKustomizeFile function in pkg/argocd/git.go works. It now uses kio.ByteReadWriter, which allows to preserve sequence indentation.
I also had to slightly modify how the comparison is done: kio adds some metadata.annotations to the yaml document, hence my approach was to get the output from the writer (which clears those annotations), and then parse it again with kyaml, hence ensuring that indentation is the same across the two files.

I'm also planning of sending a PR to kyaml to add a boolean that optionally retains the initial triple dashes of a document, if present (though I have yet to dig on kyaml code). To me, that would be a very nice achievement, as git diffs would become truly the smallest they can be!

I hope that my explanation (and code) make sense!

EDIT: added a test case for the change

[AlessandroZanatta]

I can already see that I screwed up, as all the commits from v0.15.2 are listed above.

Also, should I add a Signed-off-by: ... to my commit message to make the DCO action pass?

EDIT: branched from master and added Signed-off-by: ...

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 53.84615% with 18 lines in your changes missing coverage. Please review.

Project coverage is 63.26%. Comparing base (941e369) to head (5f1b81a).
Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
pkg/argocd/git.go	53.84%	10 Missing and 8 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1002      +/-   ##
==========================================
- Coverage   63.42%   63.26%   -0.17%     
==========================================
  Files          15       15              
  Lines        2212     2243      +31     
==========================================
+ Hits         1403     1419      +16     
- Misses        723      733      +10     
- Partials       86       91       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[AlessandroZanatta]

Sorry about the spam with the commit on my repo. I was hoping for GitHub not to mention a commit PR link on an unrelated repository in the actual PR... seems I was wrong!

[chengfang]

Thanks for implementing this enhancement. Will take a closer look.

[chengfang]

In the test, can you use a different indentation, for example,

		{
			name: "indented",
			content: `
images:
  - name: foo
    digest: sha12345
`,
			wantContent: `
images:
  - name: foo
    digest: sha23456
`,
			filter: filter,
		},
`,

so the list element is indented by 2 spaces, or some other variations. I think the current behavior is to write list elements without indentation, so test data of list elements without indentation will always pass even without this fix.

[AlessandroZanatta]

I'm not sure I understand what you mean. I have included a test that checks that a given indentation is retained after an update, and it looks like your example. Could you please clarify?

[chengfang]

in the beginning of the func, we called kyaml.ReadFile to get the originl RNode and originalData. Then here we call os.Open to get the input stream to kio ReadWriter. I think we could remove the first read, and use the 2nd read to get the original RNode and originalData before any modification. WDTY?

[AlessandroZanatta]

Makes sense. It might be a bit verbose, as we cannot simply use the kio.ByteReadWriter due to it adding its own metadata.annotations. I agree though that we should avoid having two reads back to back, I'll try to work out a readable solution!