From caf6eec88a0ca55afeb4fd77df5927be00731532 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Fri, 15 May 2026 06:41:00 +0800 Subject: [PATCH 1/3] feat: add SortOptions support for kustomize resource ordering Add ability to set kustomize's sortOptions.order (e.g. "fifo") to control resource ordering. This reduces diff noise when applying patches by preserving the original resource order instead of using kustomize's default "legacy" sorting. Adds SortOptions to ChartifyOpts, KustomizeBuildOpts, KustomizeOpts, and PatchOpts. The sortOptions is written to the generated kustomization.yaml in both kustomize-based and patch-based code paths. Also fixes a pre-existing nilness lint warning (tautological condition). Closes #188 Signed-off-by: yxxhero --- chartify.go | 9 ++++++++- kustomize.go | 31 +++++++++++++++++++++++++++---- patch.go | 11 +++++++++++ tempdir_test.go | 8 ++++---- 4 files changed, 50 insertions(+), 9 deletions(-) diff --git a/chartify.go b/chartify.go index ec13432..d44cdd9 100644 --- a/chartify.go +++ b/chartify.go @@ -108,6 +108,11 @@ type ChartifyOpts struct { // and it my produce output unexpected to you. KubeVersion string + // SortOptions configures kustomize's sortOptions for resource ordering. + // Use &SortOptions{Order: "fifo"} to preserve resource order and minimize diff noise. + // See https://kubectl.docs.kubernetes.io/references/kustomize/kustomization/sortoptions/ + SortOptions *SortOptions + // ApiVersions is a string of kubernetes APIVersions and passed to helm template via --api-versions // It is required if your chart contains any template that relies on Capabilities.APIVersion for rendering // resources depending on the API resources and versions available in a target cluster. @@ -267,6 +272,7 @@ func (r *Runner) Chartify(release, dirOrChart string, opts ...ChartifyOption) (s EnableAlphaPlugins: u.EnableKustomizeAlphaPlugins, Namespace: u.Namespace, HelmBinary: r.helmBin(), + SortOptions: u.SortOptions, } kustomizeFile, err := r.KustomizeBuild(dirOrChart, tempDir, kustomizeOpts) if err != nil { @@ -466,6 +472,7 @@ func (r *Runner) Chartify(release, dirOrChart string, opts ...ChartifyOption) (s StrategicMergePatches: u.StrategicMergePatches, Transformers: u.Transformers, EnableAlphaPlugins: u.EnableKustomizeAlphaPlugins, + SortOptions: u.SortOptions, } if err := r.Patch(tempDir, generatedManifestFiles, patchOpts); err != nil { return "", err @@ -684,7 +691,7 @@ func createDirForFile(f string) error { dstFileDir := filepath.Dir(f) if _, err := os.Lstat(dstFileDir); err == nil { - } else if err != nil && os.IsNotExist(err) { + } else if os.IsNotExist(err) { if err := os.MkdirAll(dstFileDir, 0755); err != nil { return fmt.Errorf("creating directory %s: %v", dstFileDir, err) } diff --git a/kustomize.go b/kustomize.go index f81f3fd..0b71a0e 100644 --- a/kustomize.go +++ b/kustomize.go @@ -10,11 +10,16 @@ import ( "gopkg.in/yaml.v3" ) +type SortOptions struct { + Order string `yaml:"order"` +} + type KustomizeOpts struct { - Images []KustomizeImage `yaml:"images"` - NamePrefix string `yaml:"namePrefix"` - NameSuffix string `yaml:"nameSuffix"` - Namespace string `yaml:"namespace"` + Images []KustomizeImage `yaml:"images"` + NamePrefix string `yaml:"namePrefix"` + NameSuffix string `yaml:"nameSuffix"` + Namespace string `yaml:"namespace"` + SortOptions *SortOptions `yaml:"sortOptions,omitempty"` } type KustomizeImage struct { @@ -45,6 +50,7 @@ type KustomizeBuildOpts struct { EnableAlphaPlugins bool Namespace string HelmBinary string + SortOptions *SortOptions } func (o *KustomizeBuildOpts) SetKustomizeBuildOption(opts *KustomizeBuildOpts) error { @@ -80,6 +86,10 @@ func (r *Runner) KustomizeBuild(srcDir string, tempDir string, opts ...Kustomize kustomizeOpts.Namespace = u.Namespace } + if u.SortOptions != nil { + kustomizeOpts.SortOptions = u.SortOptions + } + if len(u.SetValues) > 0 || len(u.SetFlags) > 0 { panic("--set is not yet supported for kustomize-based apps! Use -f/--values flag instead.") } @@ -139,6 +149,19 @@ func (r *Runner) KustomizeBuild(srcDir string, tempDir string, opts ...Kustomize return "", err } } + if kustomizeOpts.SortOptions != nil { + sortOptsBytes, err := yaml.Marshal(map[string]*SortOptions{"sortOptions": kustomizeOpts.SortOptions}) + if err != nil { + return "", fmt.Errorf("marshaling sortOptions: %w", err) + } + f, err := os.ReadFile(kustomizationPath) + if err != nil { + return "", fmt.Errorf("reading kustomization.yaml for sortOptions: %w", err) + } + if err := r.WriteFile(kustomizationPath, append(f, sortOptsBytes...), 0644); err != nil { + return "", err + } + } outputFile := filepath.Join(tempDir, "templates", "kustomized.yaml") kustomizeArgs := []string{"-o", outputFile, "build"} diff --git a/patch.go b/patch.go index e41c2eb..2983240 100644 --- a/patch.go +++ b/patch.go @@ -22,6 +22,9 @@ type PatchOpts struct { // Above Kustomize v3, it is `--enable-alpha-plugins`. // Below Kustomize v3 (including v3), it is `--enable_alpha_plugins`. EnableAlphaPlugins bool + + // SortOptions configures kustomize's sortOptions for resource ordering. + SortOptions *SortOptions } func (o *PatchOpts) SetPatchOption(opts *PatchOpts) error { @@ -173,6 +176,14 @@ resources: } } + if u.SortOptions != nil { + sortOptsBytes, err := yaml.Marshal(map[string]*SortOptions{"sortOptions": u.SortOptions}) + if err != nil { + return fmt.Errorf("marshaling sortOptions: %w", err) + } + kustomizationYamlContent += string(sortOptsBytes) + } + if err := r.WriteFile(filepath.Join(tempDir, "kustomization.yaml"), []byte(kustomizationYamlContent), 0644); err != nil { return err } diff --git a/tempdir_test.go b/tempdir_test.go index 441a8c5..67d7c38 100644 --- a/tempdir_test.go +++ b/tempdir_test.go @@ -35,21 +35,21 @@ func TestGenerateID(t *testing.T) { release: "foo", chart: "incubator/raw", opts: ChartifyOpts{}, - want: "foo-57669d77b", + want: "foo-5586b9d54d", }) run(testcase{ release: "foo", chart: "stable/envoy", opts: ChartifyOpts{}, - want: "foo-6c769b499", + want: "foo-748fb9844f", }) run(testcase{ release: "bar", chart: "incubator/raw", opts: ChartifyOpts{}, - want: "bar-7d49bf498c", + want: "bar-77ddc8bd65", }) run(testcase{ @@ -57,7 +57,7 @@ func TestGenerateID(t *testing.T) { opts: ChartifyOpts{ Namespace: "myns", }, - want: "myns-foo-9c6f7fb79", + want: "myns-foo-5dbbf694b5", }) for id, n := range ids { From b78952e68467f4787835071f5beed1d2b3bd7f2c Mon Sep 17 00:00:00 2001 From: yxxhero Date: Fri, 15 May 2026 06:54:31 +0800 Subject: [PATCH 2/3] refactor: extract marshalSortOptions helper, use r.ReadFile, add tests - Extract duplicated sortOptions marshaling into marshalSortOptions helper used by both KustomizeBuild and Patch (review comment) - Use r.ReadFile instead of os.ReadFile for consistency with the Runner abstraction (review comment) - Add unit tests for marshalSortOptions and SortOptions behavior in generated kustomization.yaml (review comment) Signed-off-by: yxxhero --- kustomize.go | 14 ++++++-- kustomize_test.go | 86 +++++++++++++++++++++++++++++++++++++++++++++++ patch.go | 4 +-- 3 files changed, 99 insertions(+), 5 deletions(-) create mode 100644 kustomize_test.go diff --git a/kustomize.go b/kustomize.go index 0b71a0e..43e8d45 100644 --- a/kustomize.go +++ b/kustomize.go @@ -14,6 +14,14 @@ type SortOptions struct { Order string `yaml:"order"` } +func marshalSortOptions(opts *SortOptions) ([]byte, error) { + sortOptsBytes, err := yaml.Marshal(map[string]*SortOptions{"sortOptions": opts}) + if err != nil { + return nil, fmt.Errorf("marshaling sortOptions: %w", err) + } + return sortOptsBytes, nil +} + type KustomizeOpts struct { Images []KustomizeImage `yaml:"images"` NamePrefix string `yaml:"namePrefix"` @@ -150,11 +158,11 @@ func (r *Runner) KustomizeBuild(srcDir string, tempDir string, opts ...Kustomize } } if kustomizeOpts.SortOptions != nil { - sortOptsBytes, err := yaml.Marshal(map[string]*SortOptions{"sortOptions": kustomizeOpts.SortOptions}) + sortOptsBytes, err := marshalSortOptions(kustomizeOpts.SortOptions) if err != nil { - return "", fmt.Errorf("marshaling sortOptions: %w", err) + return "", err } - f, err := os.ReadFile(kustomizationPath) + f, err := r.ReadFile(kustomizationPath) if err != nil { return "", fmt.Errorf("reading kustomization.yaml for sortOptions: %w", err) } diff --git a/kustomize_test.go b/kustomize_test.go new file mode 100644 index 0000000..05f26ce --- /dev/null +++ b/kustomize_test.go @@ -0,0 +1,86 @@ +package chartify + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestMarshalSortOptions(t *testing.T) { + t.Run("nil returns no sortOptions content", func(t *testing.T) { + got, err := marshalSortOptions(nil) + require.NoError(t, err) + result := string(got) + require.Contains(t, result, "sortOptions: null\n") + }) + + t.Run("fifo order", func(t *testing.T) { + got, err := marshalSortOptions(&SortOptions{Order: "fifo"}) + require.NoError(t, err) + result := string(got) + require.Contains(t, result, "sortOptions:") + require.Contains(t, result, "order: fifo") + }) + + t.Run("legacy order", func(t *testing.T) { + got, err := marshalSortOptions(&SortOptions{Order: "legacy"}) + require.NoError(t, err) + result := string(got) + require.Contains(t, result, "sortOptions:") + require.Contains(t, result, "order: legacy") + }) +} + +func TestKustomizationYamlWithSortOptions(t *testing.T) { + t.Run("Patch generates sortOptions in kustomization.yaml", func(t *testing.T) { + opts := &SortOptions{Order: "fifo"} + sortOptsBytes, err := marshalSortOptions(opts) + require.NoError(t, err) + + kustomizationYamlContent := `kind: "" +apiversion: "" +resources: +- templates/deployment.yaml +patches: +- path: strategicmergepatches/patch.0.yaml +` + kustomizationYamlContent += string(sortOptsBytes) + + require.True(t, strings.Contains(kustomizationYamlContent, "sortOptions:")) + require.True(t, strings.Contains(kustomizationYamlContent, "order: fifo")) + }) + + t.Run("Patch without SortOptions has no sortOptions in kustomization.yaml", func(t *testing.T) { + kustomizationYamlContent := `kind: "" +apiversion: "" +resources: +- templates/deployment.yaml +patches: +- path: strategicmergepatches/patch.0.yaml +` + + require.False(t, strings.Contains(kustomizationYamlContent, "sortOptions:")) + }) + + t.Run("KustomizeBuild merges SortOptions from KustomizeBuildOpts", func(t *testing.T) { + u := &KustomizeBuildOpts{ + SortOptions: &SortOptions{Order: "fifo"}, + } + kustomizeOpts := KustomizeOpts{} + if u.SortOptions != nil { + kustomizeOpts.SortOptions = u.SortOptions + } + require.NotNil(t, kustomizeOpts.SortOptions) + require.Equal(t, "fifo", kustomizeOpts.SortOptions.Order) + }) + + t.Run("KustomizeBuild without SortOptions leaves kustomizeOpts.SortOptions nil", func(t *testing.T) { + u := &KustomizeBuildOpts{} + kustomizeOpts := KustomizeOpts{} + if u.SortOptions != nil { + kustomizeOpts.SortOptions = u.SortOptions + } + require.Nil(t, kustomizeOpts.SortOptions) + }) +} diff --git a/patch.go b/patch.go index 2983240..32ada5f 100644 --- a/patch.go +++ b/patch.go @@ -177,9 +177,9 @@ resources: } if u.SortOptions != nil { - sortOptsBytes, err := yaml.Marshal(map[string]*SortOptions{"sortOptions": u.SortOptions}) + sortOptsBytes, err := marshalSortOptions(u.SortOptions) if err != nil { - return fmt.Errorf("marshaling sortOptions: %w", err) + return err } kustomizationYamlContent += string(sortOptsBytes) } From 65d008a21a2b286411178d4ffa1d107c11bfb8c5 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Fri, 15 May 2026 07:39:04 +0800 Subject: [PATCH 3/3] fix: add SortOptions validation, improve tests per review feedback - Guard marshalSortOptions(nil) to return nil/empty instead of 'sortOptions: null' (review comment) - Add validate() on SortOptions.Order to reject invalid values early, accepted values: legacy, fifo (review comment) - Call validation from Chartify() before any kustomize operations - Rewrite tests to exercise real Runner.Patch code path via WriteFile capture, instead of inline string assertions (review comment) - Add comment explaining sortOptions uses byte append because kustomize has no 'edit set sortoptions' command (review comment) - Fix PR description to only list accepted values: legacy, fifo Signed-off-by: yxxhero --- chartify.go | 6 ++ kustomize.go | 23 +++++++ kustomize_test.go | 156 ++++++++++++++++++++++++++++++++-------------- 3 files changed, 137 insertions(+), 48 deletions(-) diff --git a/chartify.go b/chartify.go index d44cdd9..492fa19 100644 --- a/chartify.go +++ b/chartify.go @@ -168,6 +168,12 @@ func (r *Runner) Chartify(release, dirOrChart string, opts ...ChartifyOption) (s } } + if u.SortOptions != nil { + if err := u.SortOptions.validate(); err != nil { + return "", err + } + } + isLocal, _ := r.Exists(dirOrChart) var isKustomization bool diff --git a/kustomize.go b/kustomize.go index 43e8d45..f157076 100644 --- a/kustomize.go +++ b/kustomize.go @@ -10,11 +10,32 @@ import ( "gopkg.in/yaml.v3" ) +var validSortOrders = map[string]bool{ + "legacy": true, + "fifo": true, +} + type SortOptions struct { Order string `yaml:"order"` } +func (o *SortOptions) validate() error { + if o.Order == "" { + return fmt.Errorf("sortOptions.order must not be empty") + } + if !validSortOrders[o.Order] { + return fmt.Errorf("sortOptions.order %q is not valid; accepted values are: legacy, fifo", o.Order) + } + return nil +} + func marshalSortOptions(opts *SortOptions) ([]byte, error) { + if opts == nil { + return nil, nil + } + if err := opts.validate(); err != nil { + return nil, err + } sortOptsBytes, err := yaml.Marshal(map[string]*SortOptions{"sortOptions": opts}) if err != nil { return nil, fmt.Errorf("marshaling sortOptions: %w", err) @@ -157,6 +178,8 @@ func (r *Runner) KustomizeBuild(srcDir string, tempDir string, opts ...Kustomize return "", err } } + // sortOptions is appended directly to kustomization.yaml because kustomize + // has no `edit set sortoptions` command unlike images, nameprefix, etc. if kustomizeOpts.SortOptions != nil { sortOptsBytes, err := marshalSortOptions(kustomizeOpts.SortOptions) if err != nil { diff --git a/kustomize_test.go b/kustomize_test.go index 05f26ce..50ca8fa 100644 --- a/kustomize_test.go +++ b/kustomize_test.go @@ -1,18 +1,41 @@ package chartify import ( + "os" + "path/filepath" "strings" "testing" "github.com/stretchr/testify/require" ) +func TestSortOptionsValidate(t *testing.T) { + t.Run("empty order is invalid", func(t *testing.T) { + err := (&SortOptions{Order: ""}).validate() + require.Error(t, err) + require.Contains(t, err.Error(), "must not be empty") + }) + + t.Run("invalid order is rejected", func(t *testing.T) { + err := (&SortOptions{Order: "unknown"}).validate() + require.Error(t, err) + require.Contains(t, err.Error(), "is not valid") + }) + + t.Run("fifo is valid", func(t *testing.T) { + require.NoError(t, (&SortOptions{Order: "fifo"}).validate()) + }) + + t.Run("legacy is valid", func(t *testing.T) { + require.NoError(t, (&SortOptions{Order: "legacy"}).validate()) + }) +} + func TestMarshalSortOptions(t *testing.T) { - t.Run("nil returns no sortOptions content", func(t *testing.T) { + t.Run("nil returns empty bytes", func(t *testing.T) { got, err := marshalSortOptions(nil) require.NoError(t, err) - result := string(got) - require.Contains(t, result, "sortOptions: null\n") + require.Nil(t, got) }) t.Run("fifo order", func(t *testing.T) { @@ -23,64 +46,101 @@ func TestMarshalSortOptions(t *testing.T) { require.Contains(t, result, "order: fifo") }) - t.Run("legacy order", func(t *testing.T) { - got, err := marshalSortOptions(&SortOptions{Order: "legacy"}) - require.NoError(t, err) - result := string(got) - require.Contains(t, result, "sortOptions:") - require.Contains(t, result, "order: legacy") + t.Run("invalid order returns error", func(t *testing.T) { + _, err := marshalSortOptions(&SortOptions{Order: "bogus"}) + require.Error(t, err) + require.Contains(t, err.Error(), "is not valid") }) } -func TestKustomizationYamlWithSortOptions(t *testing.T) { - t.Run("Patch generates sortOptions in kustomization.yaml", func(t *testing.T) { - opts := &SortOptions{Order: "fifo"} - sortOptsBytes, err := marshalSortOptions(opts) - require.NoError(t, err) +func TestPatch_SortOptions(t *testing.T) { + t.Run("Patch writes sortOptions to kustomization.yaml", func(t *testing.T) { + tempDir := t.TempDir() - kustomizationYamlContent := `kind: "" -apiversion: "" -resources: -- templates/deployment.yaml -patches: -- path: strategicmergepatches/patch.0.yaml -` - kustomizationYamlContent += string(sortOptsBytes) + templatesDir := filepath.Join(tempDir, "templates") + require.NoError(t, os.MkdirAll(templatesDir, 0755)) - require.True(t, strings.Contains(kustomizationYamlContent, "sortOptions:")) - require.True(t, strings.Contains(kustomizationYamlContent, "order: fifo")) - }) + manifest := filepath.Join(templatesDir, "deployment.yaml") + require.NoError(t, os.WriteFile(manifest, []byte(`apiVersion: apps/v1 +kind: Deployment +metadata: + name: test-deployment +spec: + replicas: 1 +`), 0644)) - t.Run("Patch without SortOptions has no sortOptions in kustomization.yaml", func(t *testing.T) { - kustomizationYamlContent := `kind: "" -apiversion: "" -resources: -- templates/deployment.yaml -patches: -- path: strategicmergepatches/patch.0.yaml + patchContent := `apiVersion: apps/v1 +kind: Deployment +metadata: + name: test-deployment +spec: + replicas: 3 ` + patchFile := filepath.Join(tempDir, "patch.yaml") + require.NoError(t, os.WriteFile(patchFile, []byte(patchContent), 0644)) - require.False(t, strings.Contains(kustomizationYamlContent, "sortOptions:")) - }) + r := New(HelmBin(helm)) - t.Run("KustomizeBuild merges SortOptions from KustomizeBuildOpts", func(t *testing.T) { - u := &KustomizeBuildOpts{ - SortOptions: &SortOptions{Order: "fifo"}, + var capturedKustomization string + origWriteFile := r.WriteFile + r.WriteFile = func(filename string, data []byte, perm os.FileMode) error { + if strings.HasSuffix(filename, "kustomization.yaml") { + capturedKustomization = string(data) + } + return origWriteFile(filename, data, perm) } - kustomizeOpts := KustomizeOpts{} - if u.SortOptions != nil { - kustomizeOpts.SortOptions = u.SortOptions + + patchOpts := &PatchOpts{ + StrategicMergePatches: []string{patchFile}, + SortOptions: &SortOptions{Order: "fifo"}, } - require.NotNil(t, kustomizeOpts.SortOptions) - require.Equal(t, "fifo", kustomizeOpts.SortOptions.Order) + err := r.Patch(tempDir, []string{manifest}, patchOpts) + require.NoError(t, err) + require.Contains(t, capturedKustomization, "sortOptions:") + require.Contains(t, capturedKustomization, "order: fifo") }) - t.Run("KustomizeBuild without SortOptions leaves kustomizeOpts.SortOptions nil", func(t *testing.T) { - u := &KustomizeBuildOpts{} - kustomizeOpts := KustomizeOpts{} - if u.SortOptions != nil { - kustomizeOpts.SortOptions = u.SortOptions + t.Run("Patch without SortOptions omits sortOptions from kustomization.yaml", func(t *testing.T) { + tempDir := t.TempDir() + + templatesDir := filepath.Join(tempDir, "templates") + require.NoError(t, os.MkdirAll(templatesDir, 0755)) + + manifest := filepath.Join(templatesDir, "deployment.yaml") + require.NoError(t, os.WriteFile(manifest, []byte(`apiVersion: apps/v1 +kind: Deployment +metadata: + name: test-deployment +spec: + replicas: 1 +`), 0644)) + + patchContent := `apiVersion: apps/v1 +kind: Deployment +metadata: + name: test-deployment +spec: + replicas: 3 +` + patchFile := filepath.Join(tempDir, "patch.yaml") + require.NoError(t, os.WriteFile(patchFile, []byte(patchContent), 0644)) + + r := New(HelmBin(helm)) + + var capturedKustomization string + origWriteFile := r.WriteFile + r.WriteFile = func(filename string, data []byte, perm os.FileMode) error { + if strings.HasSuffix(filename, "kustomization.yaml") { + capturedKustomization = string(data) + } + return origWriteFile(filename, data, perm) } - require.Nil(t, kustomizeOpts.SortOptions) + + patchOpts := &PatchOpts{ + StrategicMergePatches: []string{patchFile}, + } + err := r.Patch(tempDir, []string{manifest}, patchOpts) + require.NoError(t, err) + require.NotContains(t, capturedKustomization, "sortOptions:") }) }