From 32c36a3bfd9246aa83ec7b1aabd37f1549feee30 Mon Sep 17 00:00:00 2001 From: Ryan VanGundy Date: Tue, 15 Jul 2025 10:15:48 -0400 Subject: [PATCH 1/2] fix(blueprint): Don't include values in generated blueprint.yaml Values are OK on incomping blueprint templates, but they shouldn't be written to file. Values should go in their respective tfvars. --- pkg/blueprint/blueprint_handler.go | 18 +++++- pkg/blueprint/blueprint_handler_test.go | 78 +++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 3 deletions(-) diff --git a/pkg/blueprint/blueprint_handler.go b/pkg/blueprint/blueprint_handler.go index 26b7f9d6e..8533296d8 100644 --- a/pkg/blueprint/blueprint_handler.go +++ b/pkg/blueprint/blueprint_handler.go @@ -162,6 +162,7 @@ func (b *BaseBlueprintHandler) LoadData(data map[string]any, ociInfo ...*artifac // If overwrite is true, the file is overwritten regardless of existence. If overwrite is false or omitted, // the file is only written if it does not already exist. The method ensures the target directory exists, // marshals the blueprint to YAML, and writes the file using the configured shims. +// Terraform variables are filtered out to prevent them from appearing in the final blueprint.yaml. func (b *BaseBlueprintHandler) Write(overwrite ...bool) error { shouldOverwrite := false if len(overwrite) > 0 { @@ -185,7 +186,9 @@ func (b *BaseBlueprintHandler) Write(overwrite ...bool) error { return fmt.Errorf("error creating directory: %w", err) } - data, err := b.shims.YamlMarshal(&b.blueprint) + cleanedBlueprint := b.createCleanedBlueprint() + + data, err := b.shims.YamlMarshal(cleanedBlueprint) if err != nil { return fmt.Errorf("error marshalling blueprint data: %w", err) } @@ -581,8 +584,17 @@ func (b *BaseBlueprintHandler) Down() error { // Private Methods // ============================================================================= -// walkAndCollectTemplates recursively walks through the template directory and collects only .jsonnet files. -// It maintains the relative path structure from the template directory root. +// createCleanedBlueprint returns a deep copy of the blueprint with all Terraform component Values fields removed. +// All Values maps are cleared, as these should not be persisted in the final blueprint.yaml. +func (b *BaseBlueprintHandler) createCleanedBlueprint() *blueprintv1alpha1.Blueprint { + cleaned := b.blueprint.DeepCopy() + for i := range cleaned.TerraformComponents { + cleaned.TerraformComponents[i].Values = map[string]any{} + } + return cleaned +} + +// walkAndCollectTemplates recursively walks through template directories and collects .jsonnet files. func (b *BaseBlueprintHandler) walkAndCollectTemplates(templateDir, templateRoot string, templateData map[string][]byte) error { entries, err := b.shims.ReadDir(templateDir) if err != nil { diff --git a/pkg/blueprint/blueprint_handler_test.go b/pkg/blueprint/blueprint_handler_test.go index 25454d6a8..b05c62352 100644 --- a/pkg/blueprint/blueprint_handler_test.go +++ b/pkg/blueprint/blueprint_handler_test.go @@ -19,6 +19,7 @@ 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" ) @@ -3250,6 +3251,83 @@ func TestBlueprintHandler_Write(t *testing.T) { } }) + t.Run("ClearsAllValues", func(t *testing.T) { + // Given a blueprint handler with terraform components containing values + handler, mocks := setup(t) + + // Set up a terraform component with both values and terraform variables + handler.blueprint = blueprintv1alpha1.Blueprint{ + Kind: "Blueprint", + ApiVersion: "v1alpha1", + Metadata: blueprintv1alpha1.Metadata{ + Name: "test-blueprint", + }, + TerraformComponents: []blueprintv1alpha1.TerraformComponent{ + { + Source: "core", + Path: "cluster/talos", + Values: map[string]any{ + "cluster_name": "test-cluster", // Should be kept (not a terraform variable) + "cluster_endpoint": "https://test:6443", // Should be filtered if it's a terraform variable + "controlplanes": []string{"node1"}, // Should be filtered if it's a terraform variable + "custom_config": "some-value", // Should be kept (not a terraform variable) + }, + }, + }, + } + + // Set up file system mocks + var writtenContent []byte + mocks.Shims.WriteFile = func(name string, data []byte, perm os.FileMode) error { + writtenContent = data + return nil + } + + mocks.Shims.Stat = func(name string) (os.FileInfo, error) { + return nil, os.ErrNotExist // blueprint.yaml doesn't exist + } + + mocks.Shims.MkdirAll = func(path string, perm os.FileMode) error { + return nil + } + + mocks.Shims.YamlMarshal = func(v any) ([]byte, error) { + return yaml.Marshal(v) + } + + // When Write is called + err := handler.Write() + + // Then no error should be returned + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + + // And the written content should have all values cleared + if len(writtenContent) == 0 { + t.Errorf("Expected content to be written, got empty content") + } + + // Parse the written YAML to verify all values are cleared + var writtenBlueprint blueprintv1alpha1.Blueprint + err = yaml.Unmarshal(writtenContent, &writtenBlueprint) + if err != nil { + t.Errorf("Failed to unmarshal written YAML: %v", err) + } + + // Verify that the terraform component exists + if len(writtenBlueprint.TerraformComponents) != 1 { + t.Errorf("Expected 1 terraform component, got %d", len(writtenBlueprint.TerraformComponents)) + } + + component := writtenBlueprint.TerraformComponents[0] + + // Verify all values are cleared from the blueprint.yaml + if len(component.Values) != 0 { + t.Errorf("Expected all values to be cleared, but got %d values: %v", len(component.Values), component.Values) + } + }) + t.Run("ErrorWritingFile", func(t *testing.T) { // Given a blueprint handler handler, mocks := setup(t) From 075feec3a2ad8c39445654d4f2b0103b09c5f154 Mon Sep 17 00:00:00 2001 From: rmvangun <85766511+rmvangun@users.noreply.github.com> Date: Tue, 15 Jul 2025 12:43:22 -0400 Subject: [PATCH 2/2] fix(blueprint): Clean blueprint before writing (#1441) --- api/v1alpha1/blueprint_types.go | 51 +++++-------------------- api/v1alpha1/blueprint_types_test.go | 51 +++++++++++++++++++++++++ pkg/blueprint/blueprint_handler_test.go | 11 +++++- 3 files changed, 71 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 b05c62352..9ef5721b9 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" @@ -19,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" ) @@ -3326,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) {