Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 45 additions & 6 deletions pkg/argocd/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"crypto/sha256"
"encoding/hex"
"errors"
"fmt"
"os"
"path"
Expand All @@ -12,6 +13,7 @@ import (

"sigs.k8s.io/kustomize/api/konfig"
"sigs.k8s.io/kustomize/api/types"
"sigs.k8s.io/kustomize/kyaml/kio"
"sigs.k8s.io/kustomize/kyaml/order"
kyaml "sigs.k8s.io/kustomize/kyaml/yaml"

Expand Down Expand Up @@ -345,8 +347,8 @@ func writeKustomization(app *v1alpha1.Application, wbc *WriteBackConfig, gitC gi
}

// updateKustomizeFile reads the kustomization file at path, applies the filter to it, and writes the result back
// to the file. This is the same behavior as kyaml.UpdateFile, but it preserves the original order
// of YAML fields to minimize git diffs.
// to the file. This is the same behavior as kyaml.UpdateFile, but it preserves the original order of YAML fields
// and indentation of YAML sequences to minimize git diffs.
func updateKustomizeFile(filter kyaml.Filter, path string) (error, bool) {
// Read the yaml
y, err := kyaml.ReadFile(path)
Expand All @@ -359,8 +361,34 @@ func updateKustomizeFile(filter kyaml.Filter, path string) (error, bool) {
return err, false
}

// Open the input file for read
in, err := os.Open(path)
if err != nil {
return err, false
}
defer in.Close()

// Create a reader, preserving indentation of sequences
var out bytes.Buffer
rw := &kio.ByteReadWriter{
Reader: in,
Writer: &out,
PreserveSeqIndent: true,
}

// Read input file
Copy link
Collaborator

Choose a reason for hiding this comment

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

shall we change it to "// Read from input buffer" instead of file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated!

yCpySlice, err := rw.Read()
if err != nil {
return err, false
}

// We expect a single yaml
if len(yCpySlice) != 1 {
return errors.New("target parameter file should contain a single YAML document"), false
}
yCpy := yCpySlice[0]
Copy link
Collaborator

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

@AlessandroZanatta AlessandroZanatta Jan 10, 2025

Choose a reason for hiding this comment

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

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!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chengfang Just a heads up in case my commit got lost amidst other stuff. I've updated the code, feel free to review it when you've got the time to do so, thanks!


// Update the yaml
yCpy := y.Copy()
if err := yCpy.PipeE(filter); err != nil {
return err, false
}
Expand All @@ -370,18 +398,29 @@ func updateKustomizeFile(filter kyaml.Filter, path string) (error, bool) {
return err, false
}

override, err := yCpy.String()
// Write the yaml document to the output buffer
if err = rw.Write([]*kyaml.RNode{yCpy}); err != nil {
return err, false
}

// yCpy contains metadata used by kio to preserve sequence indentation,
Copy link
Collaborator

Choose a reason for hiding this comment

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

did you mean newY instead of yCpy in the above comment?
Just 2 optional nit comments. Overall looks good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I missed that, thanks. Updated!

// hence we need to parse the output buffer instead
parsed, err := kyaml.Parse(out.String())
if err != nil {
return err, false
}
override, err := parsed.String()
if err != nil {
return err, false
}

// Diff the updated document with the original
if originalData == override {
log.Debugf("target parameter file and marshaled data are the same, skipping commit.")
return nil, true
}

// Write the yaml
if err := os.WriteFile(path, []byte(override), 0600); err != nil {
if err := os.WriteFile(path, out.Bytes(), 0600); err != nil {
return err, false
}

Expand Down
12 changes: 12 additions & 0 deletions pkg/argocd/git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,18 @@ func Test_updateKustomizeFile(t *testing.T) {
wantContent: `images:
- name: foo
digest: sha23456
`,
filter: filter,
},
{
name: "indented",
content: `images:
- name: foo
digest: sha12345
`,
wantContent: `images:
- name: foo
digest: sha23456
`,
filter: filter,
},
Expand Down
Loading