From 89139ed73985752fb3312255a86cb1e5e7ed2e2a Mon Sep 17 00:00:00 2001 From: Ryan VanGundy Date: Tue, 15 Jul 2025 10:20:56 -0400 Subject: [PATCH 1/2] Use goccy/go-yaml --- pkg/blueprint/blueprint_handler_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/blueprint/blueprint_handler_test.go b/pkg/blueprint/blueprint_handler_test.go index b05c62352..057f78cd9 100644 --- a/pkg/blueprint/blueprint_handler_test.go +++ b/pkg/blueprint/blueprint_handler_test.go @@ -12,6 +12,7 @@ import ( helmv2 "github.com/fluxcd/helm-controller/api/v2" kustomizev1 "github.com/fluxcd/kustomize-controller/api/v1" sourcev1 "github.com/fluxcd/source-controller/api/v1" + "github.com/goccy/go-yaml" blueprintv1alpha1 "github.com/windsorcli/cli/api/v1alpha1" "github.com/windsorcli/cli/pkg/artifact" "github.com/windsorcli/cli/pkg/config" From b5fc817254fe4796e0548c46c2cce5525819eb1b Mon Sep 17 00:00:00 2001 From: Ryan VanGundy Date: Tue, 15 Jul 2025 12:27:58 -0400 Subject: [PATCH 2/2] Omit empty postBuild --- api/v1alpha1/blueprint_types.go | 51 +++++-------------------- api/v1alpha1/blueprint_types_test.go | 51 +++++++++++++++++++++++++ pkg/blueprint/blueprint_handler_test.go | 10 ++++- 3 files changed, 70 insertions(+), 42 deletions(-) diff --git a/api/v1alpha1/blueprint_types.go b/api/v1alpha1/blueprint_types.go index 419040a53..f9b81eef9 100644 --- a/api/v1alpha1/blueprint_types.go +++ b/api/v1alpha1/blueprint_types.go @@ -246,38 +246,7 @@ func (b *Blueprint) DeepCopy() *Blueprint { kustomizationsCopy := make([]Kustomization, len(b.Kustomizations)) for i, kustomization := range b.Kustomizations { - var substituteFromCopy []SubstituteReference - if kustomization.PostBuild != nil { - substituteFromCopy = make([]SubstituteReference, len(kustomization.PostBuild.SubstituteFrom)) - copy(substituteFromCopy, kustomization.PostBuild.SubstituteFrom) - } - - postBuildCopy := &PostBuild{ - Substitute: make(map[string]string), - SubstituteFrom: substituteFromCopy, - } - if kustomization.PostBuild != nil { - for key, value := range kustomization.PostBuild.Substitute { - postBuildCopy.Substitute[key] = value - } - } - - kustomizationsCopy[i] = Kustomization{ - Name: kustomization.Name, - Path: kustomization.Path, - Source: kustomization.Source, - DependsOn: slices.Clone(kustomization.DependsOn), - Interval: kustomization.Interval, - RetryInterval: kustomization.RetryInterval, - Timeout: kustomization.Timeout, - Patches: slices.Clone(kustomization.Patches), - Wait: kustomization.Wait, - Force: kustomization.Force, - Prune: kustomization.Prune, - Components: slices.Clone(kustomization.Components), - Cleanup: slices.Clone(kustomization.Cleanup), - PostBuild: postBuildCopy, - } + kustomizationsCopy[i] = *kustomization.DeepCopy() } return &Blueprint{ @@ -366,9 +335,7 @@ func (b *Blueprint) Merge(overlay *Blueprint) { if mergedComponent.Values == nil { mergedComponent.Values = make(map[string]any) } - for k, v := range overlayComponent.Values { - mergedComponent.Values[k] = v - } + maps.Copy(mergedComponent.Values, overlayComponent.Values) if overlayComponent.FullPath != "" { mergedComponent.FullPath = overlayComponent.FullPath @@ -402,12 +369,14 @@ func (k *Kustomization) DeepCopy() *Kustomization { var postBuildCopy *PostBuild if k.PostBuild != nil { - postBuildCopy = &PostBuild{ - Substitute: make(map[string]string), - SubstituteFrom: slices.Clone(k.PostBuild.SubstituteFrom), - } - for key, value := range k.PostBuild.Substitute { - postBuildCopy.Substitute[key] = value + substituteCopy := maps.Clone(k.PostBuild.Substitute) + substituteFromCopy := slices.Clone(k.PostBuild.SubstituteFrom) + + if len(substituteCopy) > 0 || len(substituteFromCopy) > 0 { + postBuildCopy = &PostBuild{ + Substitute: substituteCopy, + SubstituteFrom: substituteFromCopy, + } } } diff --git a/api/v1alpha1/blueprint_types_test.go b/api/v1alpha1/blueprint_types_test.go index 33b8aad17..e6f68e0e5 100644 --- a/api/v1alpha1/blueprint_types_test.go +++ b/api/v1alpha1/blueprint_types_test.go @@ -1023,3 +1023,54 @@ func TestBlueprint_DeepCopy(t *testing.T) { } }) } + +// TestPostBuildOmitEmpty verifies that empty PostBuild objects are omitted from YAML serialization +func TestPostBuildOmitEmpty(t *testing.T) { + t.Run("EmptyPostBuildOmitted", func(t *testing.T) { + kustomization := Kustomization{ + Name: "test-kustomization", + Path: "test/path", + PostBuild: &PostBuild{ + Substitute: map[string]string{}, + SubstituteFrom: []SubstituteReference{}, + }, + } + + // Create a copy using DeepCopy which should omit empty PostBuild + copied := kustomization.DeepCopy() + + // Verify that PostBuild is nil for empty content + if copied.PostBuild != nil { + t.Errorf("Expected PostBuild to be nil for empty content, but got %v", copied.PostBuild) + } + }) + + t.Run("NonEmptyPostBuildPreserved", func(t *testing.T) { + kustomization := Kustomization{ + Name: "test-kustomization", + Path: "test/path", + PostBuild: &PostBuild{ + Substitute: map[string]string{ + "key": "value", + }, + SubstituteFrom: []SubstituteReference{ + {Kind: "ConfigMap", Name: "test"}, + }, + }, + } + + // Create a copy using DeepCopy which should preserve non-empty PostBuild + copied := kustomization.DeepCopy() + + // Verify that PostBuild is preserved for non-empty content + if copied.PostBuild == nil { + t.Error("Expected PostBuild to be preserved for non-empty content, but got nil") + } + if copied.PostBuild.Substitute["key"] != "value" { + t.Errorf("Expected substitute key to be 'value', but got %s", copied.PostBuild.Substitute["key"]) + } + if len(copied.PostBuild.SubstituteFrom) != 1 { + t.Errorf("Expected 1 substitute reference, but got %d", len(copied.PostBuild.SubstituteFrom)) + } + }) +} diff --git a/pkg/blueprint/blueprint_handler_test.go b/pkg/blueprint/blueprint_handler_test.go index 057f78cd9..9ef5721b9 100644 --- a/pkg/blueprint/blueprint_handler_test.go +++ b/pkg/blueprint/blueprint_handler_test.go @@ -20,7 +20,6 @@ import ( "github.com/windsorcli/cli/pkg/di" "github.com/windsorcli/cli/pkg/kubernetes" "github.com/windsorcli/cli/pkg/shell" - "gopkg.in/yaml.v3" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -3327,6 +3326,15 @@ func TestBlueprintHandler_Write(t *testing.T) { if len(component.Values) != 0 { t.Errorf("Expected all values to be cleared, but got %d values: %v", len(component.Values), component.Values) } + + // Also verify kustomizations have postBuild cleared + if len(writtenBlueprint.Kustomizations) > 0 { + for i, kustomization := range writtenBlueprint.Kustomizations { + if kustomization.PostBuild != nil { + t.Errorf("Expected PostBuild to be cleared for kustomization %d, but got %v", i, kustomization.PostBuild) + } + } + } }) t.Run("ErrorWritingFile", func(t *testing.T) {