From 4c327a686edc9bc87cc5c80ed629d73a35ba3cf8 Mon Sep 17 00:00:00 2001 From: Ryan VanGundy <85766511+rmvangun@users.noreply.github.com> Date: Thu, 20 Nov 2025 22:05:24 -0500 Subject: [PATCH 1/3] enhancement(blueprint): Hide templated content If content is templated, it will remain hidden and not generated to `contexts//...`. This includes both `terraform/` and `patches/`. These are now written to hidden folders or handled in-memory only. This change makes it so that the user only needs to concern themselves with overrides, and the tool is less focused on "templating" as its means of operation. Signed-off-by: Ryan VanGundy <85766511+rmvangun@users.noreply.github.com> --- pkg/composer/blueprint/blueprint_handler.go | 152 +--- .../blueprint_handler_private_test.go | 697 ------------------ .../blueprint_handler_public_test.go | 22 +- pkg/composer/terraform/module_resolver.go | 31 +- .../terraform/module_resolver_test.go | 121 ++- pkg/runtime/env/terraform_env.go | 8 +- pkg/runtime/env/terraform_env_test.go | 39 +- 7 files changed, 137 insertions(+), 933 deletions(-) diff --git a/pkg/composer/blueprint/blueprint_handler.go b/pkg/composer/blueprint/blueprint_handler.go index 070709155..2943b54c1 100644 --- a/pkg/composer/blueprint/blueprint_handler.go +++ b/pkg/composer/blueprint/blueprint_handler.go @@ -165,7 +165,7 @@ func (b *BaseBlueprintHandler) LoadBlueprint() error { // 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 inputs and kustomization substitutions are manually cleared to prevent them from appearing in the final blueprint.yaml. -// Also writes patches from Features and local templates to the context patches directory. +// Patches from _template, OCI artifacts, and contexts/ are processed in memory only and not written to disk. func (b *BaseBlueprintHandler) Write(overwrite ...bool) error { shouldOverwrite := false if len(overwrite) > 0 { @@ -187,17 +187,6 @@ func (b *BaseBlueprintHandler) Write(overwrite ...bool) error { return fmt.Errorf("error setting repository defaults: %w", err) } - for _, kustomization := range b.blueprint.Kustomizations { - strategicMergePatchesToWrite, _ := b.categorizePatches(kustomization) - if len(strategicMergePatchesToWrite) > 0 { - kustomizationWithPatches := kustomization - kustomizationWithPatches.Patches = strategicMergePatchesToWrite - if err := b.writeLocalTemplatePatches(kustomizationWithPatches, shouldOverwrite); err != nil { - return fmt.Errorf("error writing patches to context: %w", err) - } - } - } - if !shouldOverwrite { if _, err := b.shims.Stat(yamlPath); err == nil { return nil @@ -1208,145 +1197,6 @@ func (b *BaseBlueprintHandler) categorizePatches(kustomization blueprintv1alpha1 return strategicMergePatchesToWrite, inlinePatches } -// writeLocalTemplatePatches writes patches from local _template to the context patches directory. -// Patches are written to contexts//patches// as individual YAML files. -// Each patch file is named using kind-name.yaml format extracted from the patch metadata. -// Patch content is evaluated (jsonnet expressions are processed) before writing. -// If overwrite is false, existing patch files are not overwritten. -// Returns an error if directory creation or file writing fails. -func (b *BaseBlueprintHandler) writeLocalTemplatePatches(kustomization blueprintv1alpha1.Kustomization, overwrite bool) error { - if len(kustomization.Patches) == 0 { - return nil - } - - configRoot := b.runtime.ConfigRoot - if configRoot == "" { - return nil - } - - patchesDir := filepath.Join(configRoot, "patches", kustomization.Name) - if err := b.shims.MkdirAll(patchesDir, 0755); err != nil { - return fmt.Errorf("failed to create patches directory: %w", err) - } - - config := make(map[string]any) - contextValues, err := b.runtime.ConfigHandler.GetContextValues() - if err == nil { - for k, v := range contextValues { - if k != "substitutions" { - config[k] = v - } - } - } - - patchMap := make(map[string]string) - for i, patch := range kustomization.Patches { - if patch.Path == "" && patch.Patch == "" { - continue - } - - var patchContent string - - if patch.Patch != "" { - patchContent = patch.Patch - } else if patch.Path != "" { - patchFileFullPath := filepath.Join(b.runtime.TemplateRoot, patch.Path) - data, err := b.shims.ReadFile(patchFileFullPath) - if err != nil { - continue - } - patchContent = string(data) - evaluated, err := b.featureEvaluator.InterpolateString(patchContent, config, filepath.Dir(patchFileFullPath)) - if err != nil { - return fmt.Errorf("failed to evaluate patch file %s: %w", patch.Path, err) - } - patchContent = evaluated - } else { - continue - } - - if strings.TrimSpace(patchContent) == "" { - continue - } - - var patchFileName string - var doc map[string]any - if err := b.shims.YamlUnmarshal([]byte(patchContent), &doc); err == nil { - if kind, ok := doc["kind"].(string); ok { - if metadata, ok := doc["metadata"].(map[string]any); ok { - if name, ok := metadata["name"].(string); ok { - filename := fmt.Sprintf("%s-%s.yaml", strings.ToLower(kind), name) - invalidChars := []string{"/", "\\", ":", "*", "?", "\"", "<", ">", "|"} - for _, char := range invalidChars { - filename = strings.ReplaceAll(filename, char, "-") - } - patchFileName = filename - } - } - } - } - if patchFileName == "" { - patchFileName = fmt.Sprintf("%d.yaml", i) - } - existingContent, exists := patchMap[patchFileName] - if exists { - var existingDoc, newDoc map[string]any - if err := b.shims.YamlUnmarshal([]byte(existingContent), &existingDoc); err != nil { - return fmt.Errorf("failed to parse existing patch: %w", err) - } - if err := b.shims.YamlUnmarshal([]byte(patchContent), &newDoc); err != nil { - return fmt.Errorf("failed to parse new patch: %w", err) - } - existingKind, existingKindOk := existingDoc["kind"].(string) - var existingName string - var existingNameOk bool - if metadata, ok := existingDoc["metadata"].(map[string]any); ok { - if name, ok := metadata["name"].(string); ok { - existingName = name - existingNameOk = true - } - } - newKind, newKindOk := newDoc["kind"].(string) - var newName string - var newNameOk bool - if metadata, ok := newDoc["metadata"].(map[string]any); ok { - if name, ok := metadata["name"].(string); ok { - newName = name - newNameOk = true - } - } - if existingKindOk && existingNameOk && newKindOk && newNameOk && - existingKind == newKind && existingName == newName { - merged := b.deepMergeMaps(existingDoc, newDoc) - mergedYAML, err := b.shims.YamlMarshal(merged) - if err != nil { - return fmt.Errorf("failed to marshal merged patch: %w", err) - } - patchMap[patchFileName] = strings.TrimSpace(string(mergedYAML)) - } else { - patchMap[patchFileName] = strings.TrimSpace(existingContent) + "\n---\n" + strings.TrimSpace(patchContent) - } - } else { - patchMap[patchFileName] = strings.TrimSpace(patchContent) - } - } - - for patchFileName, patchContent := range patchMap { - patchFilePath := filepath.Join(patchesDir, patchFileName) - if !overwrite { - if _, err := b.shims.Stat(patchFilePath); err == nil { - continue - } - } - - if err := b.shims.WriteFile(patchFilePath, []byte(patchContent), 0644); err != nil { - return fmt.Errorf("failed to write patch file %s: %w", patchFilePath, err) - } - } - - return nil -} - // mergeLegacySpecialVariables merges legacy special variables into the common values map for backward compatibility. // These variables were previously extracted from config and kustomize/values.yaml and are now merged from the config handler. // This method can be removed when legacy variable support is no longer needed. diff --git a/pkg/composer/blueprint/blueprint_handler_private_test.go b/pkg/composer/blueprint/blueprint_handler_private_test.go index 2fb46028c..0828cbd4a 100644 --- a/pkg/composer/blueprint/blueprint_handler_private_test.go +++ b/pkg/composer/blueprint/blueprint_handler_private_test.go @@ -4290,703 +4290,6 @@ func TestNewShims(t *testing.T) { }) } -func TestBaseBlueprintHandler_writeLocalTemplatePatches(t *testing.T) { - setup := func(t *testing.T) (*BaseBlueprintHandler, *BlueprintTestMocks) { - t.Helper() - mocks := setupBlueprintMocks(t) - mockArtifactBuilder := artifact.NewMockArtifact() - handler, err := NewBlueprintHandler(mocks.Runtime, mockArtifactBuilder) - if err != nil { - t.Fatalf("NewBlueprintHandler() failed: %v", err) - } - handler.shims = mocks.Shims - return handler, mocks - } - - t.Run("WritesPatchWithKindAndName", func(t *testing.T) { - handler, mocks := setup(t) - mocks.Runtime.ConfigRoot = "/test/config" - mocks.Runtime.TemplateRoot = "/test/template" - - kustomization := blueprintv1alpha1.Kustomization{ - Name: "test-kustomization", - Patches: []blueprintv1alpha1.BlueprintPatch{ - { - Patch: `apiVersion: v1 -kind: ConfigMap -metadata: - name: test-config -data: - key: value`, - }, - }, - } - - var writtenPath string - var writtenContent []byte - mocks.Shims.WriteFile = func(name string, data []byte, perm os.FileMode) error { - normalizedName := filepath.ToSlash(name) - if strings.Contains(normalizedName, "patches/") { - writtenPath = name - writtenContent = data - } - return nil - } - - mocks.Shims.Stat = func(name string) (os.FileInfo, error) { - return nil, os.ErrNotExist - } - - mocks.Shims.MkdirAll = func(path string, perm os.FileMode) error { - return nil - } - - mocks.Shims.YamlUnmarshal = yaml.Unmarshal - mocks.Shims.YamlMarshal = yaml.Marshal - - mocks.ConfigHandler.(*config.MockConfigHandler).GetContextValuesFunc = func() (map[string]any, error) { - return make(map[string]any), nil - } - - err := handler.writeLocalTemplatePatches(kustomization, true) - - if err != nil { - t.Errorf("Expected no error, got %v", err) - } - - expectedPath := filepath.Join(mocks.Runtime.ConfigRoot, "patches", "test-kustomization", "configmap-test-config.yaml") - if filepath.ToSlash(writtenPath) != filepath.ToSlash(expectedPath) { - t.Errorf("Expected patch file at %s, got %s", expectedPath, writtenPath) - } - - if len(writtenContent) == 0 { - t.Error("Expected patch content to be written") - } - }) - - t.Run("HandlesEmptyPatches", func(t *testing.T) { - handler, mocks := setup(t) - mocks.Runtime.ConfigRoot = "/test/config" - - kustomization := blueprintv1alpha1.Kustomization{ - Name: "test-kustomization", - Patches: []blueprintv1alpha1.BlueprintPatch{}, - } - - var writeFileCalled bool - mocks.Shims.WriteFile = func(name string, data []byte, perm os.FileMode) error { - normalizedName := filepath.ToSlash(name) - if strings.Contains(normalizedName, "patches/") { - writeFileCalled = true - } - return nil - } - - err := handler.writeLocalTemplatePatches(kustomization, true) - - if err != nil { - t.Errorf("Expected no error, got %v", err) - } - - if writeFileCalled { - t.Error("Expected WriteFile not to be called for empty patches") - } - }) - - t.Run("HandlesEmptyConfigRoot", func(t *testing.T) { - handler, mocks := setup(t) - mocks.Runtime.ConfigRoot = "" - - kustomization := blueprintv1alpha1.Kustomization{ - Name: "test-kustomization", - Patches: []blueprintv1alpha1.BlueprintPatch{ - { - Patch: `apiVersion: v1 -kind: ConfigMap -metadata: - name: test-config`, - }, - }, - } - - var writeFileCalled bool - mocks.Shims.WriteFile = func(name string, data []byte, perm os.FileMode) error { - normalizedName := filepath.ToSlash(name) - if strings.Contains(normalizedName, "patches/") { - writeFileCalled = true - } - return nil - } - - err := handler.writeLocalTemplatePatches(kustomization, true) - - if err != nil { - t.Errorf("Expected no error, got %v", err) - } - - if writeFileCalled { - t.Error("Expected WriteFile not to be called when ConfigRoot is empty") - } - }) - - t.Run("HandlesMkdirAllError", func(t *testing.T) { - handler, mocks := setup(t) - mocks.Runtime.ConfigRoot = "/test/config" - - kustomization := blueprintv1alpha1.Kustomization{ - Name: "test-kustomization", - Patches: []blueprintv1alpha1.BlueprintPatch{ - { - Patch: `apiVersion: v1 -kind: ConfigMap -metadata: - name: test-config`, - }, - }, - } - - expectedError := fmt.Errorf("mkdir error") - mocks.Shims.MkdirAll = func(path string, perm os.FileMode) error { - normalizedPath := filepath.ToSlash(path) - if strings.Contains(normalizedPath, "patches/") { - return expectedError - } - return nil - } - - mocks.ConfigHandler.(*config.MockConfigHandler).GetContextValuesFunc = func() (map[string]any, error) { - return make(map[string]any), nil - } - - err := handler.writeLocalTemplatePatches(kustomization, true) - - if err == nil { - t.Fatal("Expected error from MkdirAll, got nil") - } - - if !strings.Contains(err.Error(), "failed to create patches directory") { - t.Errorf("Expected error about creating patches directory, got: %v", err) - } - }) - - t.Run("HandlesWriteFileError", func(t *testing.T) { - handler, mocks := setup(t) - mocks.Runtime.ConfigRoot = "/test/config" - - kustomization := blueprintv1alpha1.Kustomization{ - Name: "test-kustomization", - Patches: []blueprintv1alpha1.BlueprintPatch{ - { - Patch: `apiVersion: v1 -kind: ConfigMap -metadata: - name: test-config`, - }, - }, - } - - expectedError := fmt.Errorf("write file error") - mocks.Shims.WriteFile = func(name string, data []byte, perm os.FileMode) error { - normalizedName := filepath.ToSlash(name) - if strings.Contains(normalizedName, "patches/") { - return expectedError - } - return nil - } - - mocks.Shims.Stat = func(name string) (os.FileInfo, error) { - return nil, os.ErrNotExist - } - - mocks.Shims.MkdirAll = func(path string, perm os.FileMode) error { - return nil - } - - mocks.Shims.YamlUnmarshal = yaml.Unmarshal - - mocks.ConfigHandler.(*config.MockConfigHandler).GetContextValuesFunc = func() (map[string]any, error) { - return make(map[string]any), nil - } - - err := handler.writeLocalTemplatePatches(kustomization, true) - - if err == nil { - t.Fatal("Expected error from WriteFile, got nil") - } - - if !strings.Contains(err.Error(), "failed to write patch file") { - t.Errorf("Expected error about writing patch file, got: %v", err) - } - }) - - t.Run("RespectsOverwriteFlag", func(t *testing.T) { - handler, mocks := setup(t) - mocks.Runtime.ConfigRoot = "/test/config" - - kustomization := blueprintv1alpha1.Kustomization{ - Name: "test-kustomization", - Patches: []blueprintv1alpha1.BlueprintPatch{ - { - Patch: `apiVersion: v1 -kind: ConfigMap -metadata: - name: test-config`, - }, - }, - } - - var writeFileCalled bool - mocks.Shims.WriteFile = func(name string, data []byte, perm os.FileMode) error { - normalizedName := filepath.ToSlash(name) - if strings.Contains(normalizedName, "patches/") { - writeFileCalled = true - } - return nil - } - - mocks.Shims.Stat = func(name string) (os.FileInfo, error) { - normalizedName := filepath.ToSlash(name) - if strings.Contains(normalizedName, "patches/") { - return &mockFileInfo{name: "configmap-test-config.yaml", isDir: false}, nil - } - return nil, os.ErrNotExist - } - - mocks.Shims.MkdirAll = func(path string, perm os.FileMode) error { - return nil - } - - mocks.Shims.YamlUnmarshal = yaml.Unmarshal - - mocks.ConfigHandler.(*config.MockConfigHandler).GetContextValuesFunc = func() (map[string]any, error) { - return make(map[string]any), nil - } - - err := handler.writeLocalTemplatePatches(kustomization, false) - - if err != nil { - t.Errorf("Expected no error, got %v", err) - } - - if writeFileCalled { - t.Error("Expected WriteFile not to be called when overwrite is false and file exists") - } - - writeFileCalled = false - err = handler.writeLocalTemplatePatches(kustomization, true) - - if err != nil { - t.Errorf("Expected no error, got %v", err) - } - - if !writeFileCalled { - t.Error("Expected WriteFile to be called when overwrite is true") - } - }) - - t.Run("EvaluatesPatchContentFromPath", func(t *testing.T) { - handler, mocks := setup(t) - tmpDir := t.TempDir() - mocks.Runtime.ConfigRoot = "/test/config" - mocks.Runtime.TemplateRoot = tmpDir - - patchFile := filepath.Join(tmpDir, "kustomize", "patches", "test-patch.yaml") - if err := os.MkdirAll(filepath.Dir(patchFile), 0755); err != nil { - t.Fatalf("Failed to create patch directory: %v", err) - } - - patchContent := `apiVersion: v1 -kind: ConfigMap -metadata: - name: test-config` - if err := os.WriteFile(patchFile, []byte(patchContent), 0644); err != nil { - t.Fatalf("Failed to write patch file: %v", err) - } - - kustomization := blueprintv1alpha1.Kustomization{ - Name: "test-kustomization", - Patches: []blueprintv1alpha1.BlueprintPatch{ - { - Path: "kustomize/patches/test-patch.yaml", - }, - }, - } - - var writtenContent []byte - mocks.Shims.WriteFile = func(name string, data []byte, perm os.FileMode) error { - normalizedName := filepath.ToSlash(name) - if strings.Contains(normalizedName, "patches/") { - writtenContent = data - } - return nil - } - - mocks.Shims.Stat = func(name string) (os.FileInfo, error) { - return nil, os.ErrNotExist - } - - mocks.Shims.MkdirAll = func(path string, perm os.FileMode) error { - return nil - } - - mocks.Shims.ReadFile = os.ReadFile - mocks.Shims.YamlUnmarshal = yaml.Unmarshal - mocks.Shims.YamlMarshal = yaml.Marshal - - mocks.ConfigHandler.(*config.MockConfigHandler).GetContextValuesFunc = func() (map[string]any, error) { - return make(map[string]any), nil - } - - err := handler.writeLocalTemplatePatches(kustomization, true) - - if err != nil { - t.Errorf("Expected no error, got %v", err) - } - - if len(writtenContent) == 0 { - t.Error("Expected patch content to be written") - } - - if !strings.Contains(string(writtenContent), "test-config") { - t.Error("Expected written content to contain patch data") - } - }) - - t.Run("MergesPatchesWithSameKindAndName", func(t *testing.T) { - handler, mocks := setup(t) - mocks.Runtime.ConfigRoot = "/test/config" - - kustomization := blueprintv1alpha1.Kustomization{ - Name: "test-kustomization", - Patches: []blueprintv1alpha1.BlueprintPatch{ - { - Patch: `apiVersion: v1 -kind: ConfigMap -metadata: - name: test-config -data: - key1: value1`, - }, - { - Patch: `apiVersion: v1 -kind: ConfigMap -metadata: - name: test-config -data: - key2: value2`, - }, - }, - } - - var writtenContent []byte - mocks.Shims.WriteFile = func(name string, data []byte, perm os.FileMode) error { - normalizedName := filepath.ToSlash(name) - if strings.Contains(normalizedName, "patches/") { - writtenContent = data - } - return nil - } - - mocks.Shims.Stat = func(name string) (os.FileInfo, error) { - return nil, os.ErrNotExist - } - - mocks.Shims.MkdirAll = func(path string, perm os.FileMode) error { - return nil - } - - mocks.Shims.YamlUnmarshal = yaml.Unmarshal - mocks.Shims.YamlMarshal = yaml.Marshal - - mocks.ConfigHandler.(*config.MockConfigHandler).GetContextValuesFunc = func() (map[string]any, error) { - return make(map[string]any), nil - } - - err := handler.writeLocalTemplatePatches(kustomization, true) - - if err != nil { - t.Errorf("Expected no error, got %v", err) - } - - if len(writtenContent) == 0 { - t.Fatal("Expected merged patch content to be written") - } - - var mergedDoc map[string]any - if err := yaml.Unmarshal(writtenContent, &mergedDoc); err != nil { - t.Fatalf("Failed to unmarshal merged patch: %v", err) - } - - if data, ok := mergedDoc["data"].(map[string]any); ok { - if data["key1"] != "value1" { - t.Error("Expected key1 to be in merged patch") - } - if data["key2"] != "value2" { - t.Error("Expected key2 to be in merged patch") - } - } else { - t.Error("Expected data section in merged patch") - } - }) - - t.Run("AppendsPatchesWithDifferentKindOrName", func(t *testing.T) { - handler, mocks := setup(t) - mocks.Runtime.ConfigRoot = "/test/config" - - kustomization := blueprintv1alpha1.Kustomization{ - Name: "test-kustomization", - Patches: []blueprintv1alpha1.BlueprintPatch{ - { - Patch: `apiVersion: v1 -kind: ConfigMap -metadata: - name: config-1 -data: - key1: value1`, - }, - { - Patch: `apiVersion: v1 -kind: Deployment -metadata: - name: deployment-1 -spec: - replicas: 3`, - }, - }, - } - - var writtenFiles = make(map[string][]byte) - mocks.Shims.WriteFile = func(name string, data []byte, perm os.FileMode) error { - normalizedName := filepath.ToSlash(name) - if strings.Contains(normalizedName, "patches/") { - writtenFiles[name] = data - } - return nil - } - - mocks.Shims.Stat = func(name string) (os.FileInfo, error) { - return nil, os.ErrNotExist - } - - mocks.Shims.MkdirAll = func(path string, perm os.FileMode) error { - return nil - } - - mocks.Shims.YamlUnmarshal = yaml.Unmarshal - mocks.Shims.YamlMarshal = yaml.Marshal - - mocks.ConfigHandler.(*config.MockConfigHandler).GetContextValuesFunc = func() (map[string]any, error) { - return make(map[string]any), nil - } - - err := handler.writeLocalTemplatePatches(kustomization, true) - - if err != nil { - t.Errorf("Expected no error, got %v", err) - } - - if len(writtenFiles) == 0 { - t.Fatal("Expected patch files to be written") - } - - foundConfigMap := false - foundDeployment := false - for path, content := range writtenFiles { - contentStr := string(content) - if strings.Contains(path, "configmap-config-1.yaml") { - foundConfigMap = true - if !strings.Contains(contentStr, "config-1") { - t.Error("Expected config-1 patch content") - } - } - if strings.Contains(path, "deployment-deployment-1.yaml") { - foundDeployment = true - if !strings.Contains(contentStr, "deployment-1") { - t.Error("Expected deployment-1 patch content") - } - } - } - - if !foundConfigMap { - t.Error("Expected ConfigMap patch file to be written") - } - if !foundDeployment { - t.Error("Expected Deployment patch file to be written") - } - }) - - t.Run("SanitizesInvalidFilenameCharacters", func(t *testing.T) { - handler, mocks := setup(t) - mocks.Runtime.ConfigRoot = "/test/config" - - kustomization := blueprintv1alpha1.Kustomization{ - Name: "test-kustomization", - Patches: []blueprintv1alpha1.BlueprintPatch{ - { - Patch: `apiVersion: v1 -kind: ConfigMap -metadata: - name: test/config:name*with?invalid"chars<| -data: - key: value`, - }, - }, - } - - var writtenPath string - mocks.Shims.WriteFile = func(name string, data []byte, perm os.FileMode) error { - normalizedName := filepath.ToSlash(name) - if strings.Contains(normalizedName, "patches/") { - writtenPath = name - } - return nil - } - - mocks.Shims.Stat = func(name string) (os.FileInfo, error) { - return nil, os.ErrNotExist - } - - mocks.Shims.MkdirAll = func(path string, perm os.FileMode) error { - return nil - } - - mocks.Shims.YamlUnmarshal = yaml.Unmarshal - - mocks.ConfigHandler.(*config.MockConfigHandler).GetContextValuesFunc = func() (map[string]any, error) { - return make(map[string]any), nil - } - - err := handler.writeLocalTemplatePatches(kustomization, true) - - if err != nil { - t.Errorf("Expected no error, got %v", err) - } - - if writtenPath == "" { - t.Fatal("Expected patch file to be written") - } - - fileName := filepath.Base(writtenPath) - invalidChars := []string{"/", "\\", ":", "*", "?", "\"", "<", "|"} - for _, char := range invalidChars { - if strings.Contains(fileName, char) { - t.Errorf("Expected invalid character '%s' to be sanitized from filename, got: %s", char, fileName) - } - } - - if !strings.Contains(fileName, "configmap-test") { - t.Errorf("Expected sanitized filename to contain 'configmap-test', got: %s", fileName) - } - - if !strings.HasSuffix(fileName, ".yaml") { - t.Errorf("Expected filename to end with .yaml, got: %s", fileName) - } - }) - - t.Run("SkipsPatchWhenReadFileFails", func(t *testing.T) { - handler, mocks := setup(t) - mocks.Runtime.ConfigRoot = "/test/config" - mocks.Runtime.TemplateRoot = "/test/template" - - kustomization := blueprintv1alpha1.Kustomization{ - Name: "test-kustomization", - Patches: []blueprintv1alpha1.BlueprintPatch{ - { - Path: "kustomize/patches/test-patch.yaml", - }, - }, - } - - mocks.Shims.Stat = func(name string) (os.FileInfo, error) { - return nil, os.ErrNotExist - } - - mocks.Shims.MkdirAll = func(path string, perm os.FileMode) error { - return nil - } - - mocks.Shims.ReadFile = func(name string) ([]byte, error) { - normalizedName := filepath.ToSlash(name) - if strings.Contains(normalizedName, "patches/test-patch.yaml") { - return nil, fmt.Errorf("read file error") - } - return nil, os.ErrNotExist - } - - mocks.ConfigHandler.(*config.MockConfigHandler).GetContextValuesFunc = func() (map[string]any, error) { - return make(map[string]any), nil - } - - var writeFileCalled bool - mocks.Shims.WriteFile = func(name string, data []byte, perm os.FileMode) error { - normalizedName := filepath.ToSlash(name) - if strings.Contains(normalizedName, "patches/") { - writeFileCalled = true - } - return nil - } - - err := handler.writeLocalTemplatePatches(kustomization, true) - - if err != nil { - t.Errorf("Expected no error (patch skipped), got %v", err) - } - - if writeFileCalled { - t.Error("Expected WriteFile not to be called when patch file cannot be read") - } - }) - - t.Run("UsesIndexWhenNoMetadata", func(t *testing.T) { - handler, mocks := setup(t) - mocks.Runtime.ConfigRoot = "/test/config" - - kustomization := blueprintv1alpha1.Kustomization{ - Name: "test-kustomization", - Patches: []blueprintv1alpha1.BlueprintPatch{ - { - Patch: `apiVersion: v1 -data: - key: value`, - }, - }, - } - - var writtenPath string - mocks.Shims.WriteFile = func(name string, data []byte, perm os.FileMode) error { - normalizedName := filepath.ToSlash(name) - if strings.Contains(normalizedName, "patches/") { - writtenPath = name - } - return nil - } - - mocks.Shims.Stat = func(name string) (os.FileInfo, error) { - return nil, os.ErrNotExist - } - - mocks.Shims.MkdirAll = func(path string, perm os.FileMode) error { - return nil - } - - mocks.Shims.YamlUnmarshal = yaml.Unmarshal - - mocks.ConfigHandler.(*config.MockConfigHandler).GetContextValuesFunc = func() (map[string]any, error) { - return make(map[string]any), nil - } - - err := handler.writeLocalTemplatePatches(kustomization, true) - - if err != nil { - t.Errorf("Expected no error, got %v", err) - } - - if !strings.Contains(writtenPath, "0.yaml") { - t.Errorf("Expected filename with index 0, got: %s", filepath.Base(writtenPath)) - } - }) -} - func TestBaseBlueprintHandler_categorizePatches(t *testing.T) { setup := func(t *testing.T) (*BaseBlueprintHandler, *BlueprintTestMocks) { t.Helper() diff --git a/pkg/composer/blueprint/blueprint_handler_public_test.go b/pkg/composer/blueprint/blueprint_handler_public_test.go index 9f1c208d4..097db5c4d 100644 --- a/pkg/composer/blueprint/blueprint_handler_public_test.go +++ b/pkg/composer/blueprint/blueprint_handler_public_test.go @@ -2057,7 +2057,7 @@ func TestBlueprintHandler_Write(t *testing.T) { } }) - t.Run("WritesStrategicMergePatchesToDisk", func(t *testing.T) { + t.Run("DoesNotWritePatchesToDisk", func(t *testing.T) { // Given a blueprint handler with kustomization containing strategic merge patches handler, mocks := setup(t) mocks.Runtime.ConfigRoot = "/test/config" @@ -2087,12 +2087,10 @@ data: } var writtenPatchPaths []string - var writtenPatchContents []string mocks.Shims.WriteFile = func(name string, data []byte, perm os.FileMode) error { normalizedName := filepath.ToSlash(name) if strings.Contains(normalizedName, "patches/") { writtenPatchPaths = append(writtenPatchPaths, name) - writtenPatchContents = append(writtenPatchContents, string(data)) } return nil } @@ -2124,21 +2122,9 @@ data: t.Errorf("Expected no error, got %v", err) } - // And patch file should be written - if len(writtenPatchPaths) == 0 { - t.Error("Expected patch file to be written, but no patch files were written") - } - - expectedPatchPath := filepath.Join(mocks.Runtime.ConfigRoot, "patches", "test-kustomization", "configmap-test-config.yaml") - found := false - for _, path := range writtenPatchPaths { - if filepath.ToSlash(path) == filepath.ToSlash(expectedPatchPath) { - found = true - break - } - } - if !found { - t.Errorf("Expected patch file to be written at %s, but got: %v", expectedPatchPath, writtenPatchPaths) + // And no patch files should be written (patches are processed in memory only) + if len(writtenPatchPaths) != 0 { + t.Errorf("Expected no patch files to be written, but got: %v", writtenPatchPaths) } }) diff --git a/pkg/composer/terraform/module_resolver.go b/pkg/composer/terraform/module_resolver.go index 8d94ee611..fcbd16df9 100644 --- a/pkg/composer/terraform/module_resolver.go +++ b/pkg/composer/terraform/module_resolver.go @@ -73,10 +73,10 @@ func NewBaseModuleResolver(rt *runtime.Runtime, blueprintHandler blueprint.Bluep // in the blueprint, it ensures a tfvars file is generated if not already handled by the input data. // The method uses the blueprint handler to retrieve TerraformComponents and determines the variables.tf // location based on component source (remote or local). Module resolution is handled by pkg/terraform. +// Before generating new tfvars files, it cleans up orphaned tfvars files for components that no longer exist. func (h *BaseModuleResolver) GenerateTfvars(overwrite bool) error { h.reset = overwrite - contextPath := h.runtime.ConfigRoot projectRoot := h.runtime.ProjectRoot components := h.blueprintHandler.GetTerraformComponents() @@ -87,7 +87,7 @@ func (h *BaseModuleResolver) GenerateTfvars(overwrite bool) error { componentValues = make(map[string]any) } - if err := h.generateComponentTfvars(projectRoot, contextPath, component, componentValues); err != nil { + if err := h.generateComponentTfvars(projectRoot, component, componentValues); err != nil { return fmt.Errorf("failed to generate tfvars for component %s: %w", component.Path, err) } } @@ -191,29 +191,22 @@ func (h *BaseModuleResolver) parseVariablesFile(variablesTfPath string, protecte } // generateComponentTfvars generates tfvars files for a single Terraform component. -// For components with a non-empty Source, only the module tfvars file is generated at .windsor/.tf_modules//terraform.tfvars. -// For components with an empty Source, only the context tfvars file is generated at /terraform/.tfvars. +// All components write tfvars files to .windsor/.tf_modules//terraform.tfvars, +// regardless of whether they have a Source (remote) or not (local). This unifies the behavior +// between local templates and OCI artifacts, preventing writes to the contexts folder. // Returns an error if variables.tf cannot be found or if tfvars file generation fails. -func (h *BaseModuleResolver) generateComponentTfvars(projectRoot, contextPath string, component blueprintv1alpha1.TerraformComponent, componentValues map[string]any) error { +func (h *BaseModuleResolver) generateComponentTfvars(projectRoot string, component blueprintv1alpha1.TerraformComponent, componentValues map[string]any) error { variablesTfPath, err := h.findVariablesTfFileForComponent(projectRoot, component) if err != nil { return fmt.Errorf("failed to find variables.tf for component %s: %w", component.Path, err) } - if component.Source != "" { - moduleTfvarsPath := filepath.Join(projectRoot, ".windsor", ".tf_modules", component.Path, "terraform.tfvars") - if err := h.removeTfvarsFiles(filepath.Dir(moduleTfvarsPath)); err != nil { - return fmt.Errorf("failed cleaning existing .tfvars in module dir %s: %w", filepath.Dir(moduleTfvarsPath), err) - } - if err := h.generateTfvarsFile(moduleTfvarsPath, variablesTfPath, componentValues, component.Source); err != nil { - return fmt.Errorf("failed to generate module tfvars file: %w", err) - } - } else { - terraformKey := "terraform/" + component.Path - tfvarsFilePath := filepath.Join(contextPath, terraformKey+".tfvars") - if err := h.generateTfvarsFile(tfvarsFilePath, variablesTfPath, componentValues, component.Source); err != nil { - return fmt.Errorf("failed to generate context tfvars file: %w", err) - } + moduleTfvarsPath := filepath.Join(projectRoot, ".windsor", ".tf_modules", component.Path, "terraform.tfvars") + if err := h.removeTfvarsFiles(filepath.Dir(moduleTfvarsPath)); err != nil { + return fmt.Errorf("failed cleaning existing .tfvars in module dir %s: %w", filepath.Dir(moduleTfvarsPath), err) + } + if err := h.generateTfvarsFile(moduleTfvarsPath, variablesTfPath, componentValues, component.Source); err != nil { + return fmt.Errorf("failed to generate module tfvars file: %w", err) } return nil diff --git a/pkg/composer/terraform/module_resolver_test.go b/pkg/composer/terraform/module_resolver_test.go index dee7a9131..83729d5bb 100644 --- a/pkg/composer/terraform/module_resolver_test.go +++ b/pkg/composer/terraform/module_resolver_test.go @@ -99,6 +99,7 @@ contexts: rt := &runtime.Runtime{ ConfigHandler: configHandler, Shell: mockShell, + ProjectRoot: tmpDir, } mocks := &TerraformTestMocks{ @@ -1340,8 +1341,8 @@ variable "disabled" { type = bool }` } }) - t.Run("HandlesExistingTfvarsFileReadError", func(t *testing.T) { - // Given a resolver with an existing tfvars file that cannot be read (component without Source) + t.Run("HandlesRemoveTfvarsFilesReadDirError", func(t *testing.T) { + // Given a resolver with a ReadDir error when removing tfvars files resolver, mocks := setup(t) mocks.BlueprintHandler.GetTerraformComponentsFunc = func() []blueprintv1alpha1.TerraformComponent { @@ -1365,44 +1366,36 @@ variable "disabled" { type = bool }` t.Fatalf("Failed to write variables.tf: %v", err) } - contextPath := mocks.Runtime.ConfigRoot - tfvarsPath := filepath.Join(contextPath, "terraform", "local-module.tfvars") - if err := os.MkdirAll(filepath.Dir(tfvarsPath), 0755); err != nil { - t.Fatalf("Failed to create context dir: %v", err) - } - if err := os.WriteFile(tfvarsPath, []byte("name = \"old\""), 0644); err != nil { - t.Fatalf("Failed to write existing tfvars: %v", err) + moduleDir := filepath.Join(projectRoot, ".windsor", ".tf_modules", "local-module") + if err := os.MkdirAll(moduleDir, 0755); err != nil { + t.Fatalf("Failed to create module dir: %v", err) } - // Mock ReadFile to return error when checking existing file - originalReadFile := resolver.shims.ReadFile - resolver.shims.ReadFile = func(path string) ([]byte, error) { - if path == tfvarsPath { - return nil, fmt.Errorf("read error") - } - return originalReadFile(path) - } - - // Mock Stat to return success (file exists) - originalStat := resolver.shims.Stat - resolver.shims.Stat = func(path string) (os.FileInfo, error) { - if path == tfvarsPath { - return nil, nil + // Mock ReadDir to return error when removing tfvars files + originalReadDir := resolver.shims.ReadDir + resolver.shims.ReadDir = func(path string) ([]os.DirEntry, error) { + normalizedPath := filepath.Clean(path) + normalizedModuleDir := filepath.Clean(moduleDir) + if normalizedPath == normalizedModuleDir { + return nil, fmt.Errorf("readdir error") } - return originalStat(path) + return originalReadDir(path) } - // When generating tfvars without overwrite + // When generating tfvars err := resolver.GenerateTfvars(false) // Then it should return an error if err == nil { - t.Error("Expected error when existing file cannot be read") + t.Error("Expected error when ReadDir fails during tfvars removal") + } + if err != nil && !strings.Contains(err.Error(), "failed cleaning existing .tfvars") { + t.Errorf("Expected error about cleaning tfvars, got: %v", err) } }) - t.Run("HandlesExistingTfvarsFileStatError", func(t *testing.T) { - // Given a resolver with a stat error on existing tfvars file (component without Source) + t.Run("HandlesRemoveTfvarsFilesStatError", func(t *testing.T) { + // Given a resolver with a stat error when checking module directory resolver, mocks := setup(t) mocks.BlueprintHandler.GetTerraformComponentsFunc = func() []blueprintv1alpha1.TerraformComponent { @@ -1426,13 +1419,14 @@ variable "disabled" { type = bool }` t.Fatalf("Failed to write variables.tf: %v", err) } - contextPath := mocks.Runtime.ConfigRoot - tfvarsPath := filepath.Join(contextPath, "terraform", "local-module.tfvars") + moduleDir := filepath.Join(projectRoot, ".windsor", ".tf_modules", "local-module") - // Mock Stat to return non-NotExist error for the context tfvars file + // Mock Stat to return non-NotExist error for the module directory originalStat := resolver.shims.Stat resolver.shims.Stat = func(path string) (os.FileInfo, error) { - if path == tfvarsPath { + normalizedPath := filepath.Clean(path) + normalizedModuleDir := filepath.Clean(moduleDir) + if normalizedPath == normalizedModuleDir { return nil, fmt.Errorf("stat error") } return originalStat(path) @@ -1443,7 +1437,10 @@ variable "disabled" { type = bool }` // Then it should return an error if err == nil { - t.Error("Expected error when stat fails") + t.Error("Expected error when stat fails during tfvars removal") + } + if err != nil && !strings.Contains(err.Error(), "failed cleaning existing .tfvars") { + t.Errorf("Expected error about cleaning tfvars, got: %v", err) } }) @@ -1901,6 +1898,64 @@ variable "cluster_name" { type = string }` } }) + t.Run("CleansUpOrphanedTfvars", func(t *testing.T) { + // Given a resolver with an orphaned tfvars file for a component that no longer exists + resolver, mocks := setup(t) + + projectRoot, _ := mocks.Shell.GetProjectRootFunc() + + // Create a component directory for a component that will not be in the blueprint + orphanedDir := filepath.Join(projectRoot, ".windsor", ".tf_modules", "orphaned-component") + if err := os.MkdirAll(orphanedDir, 0755); err != nil { + t.Fatalf("Failed to create orphaned dir: %v", err) + } + orphanedTfvars := filepath.Join(orphanedDir, "terraform.tfvars") + if err := os.WriteFile(orphanedTfvars, []byte("old_value = \"stale\""), 0644); err != nil { + t.Fatalf("Failed to write orphaned tfvars: %v", err) + } + + // And a valid component that exists in the blueprint + mocks.BlueprintHandler.GetTerraformComponentsFunc = func() []blueprintv1alpha1.TerraformComponent { + return []blueprintv1alpha1.TerraformComponent{ + { + Path: "test-module", + Source: "git::https://github.com/test/module.git", + FullPath: filepath.Join(projectRoot, "terraform", "test-module"), + Inputs: map[string]any{ + "cluster_name": "test-cluster", + }, + }, + } + } + + variablesDir := filepath.Join(projectRoot, ".windsor", ".tf_modules", "test-module") + if err := os.MkdirAll(variablesDir, 0755); err != nil { + t.Fatalf("Failed to create dir: %v", err) + } + if err := os.WriteFile(filepath.Join(variablesDir, "variables.tf"), []byte(`variable "cluster_name" { type = string }`), 0644); err != nil { + t.Fatalf("Failed to write variables.tf: %v", err) + } + + // When generating tfvars + err := resolver.GenerateTfvars(false) + + // Then it should succeed + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + + // And the orphaned tfvars file should be removed + if _, err := os.Stat(orphanedTfvars); !os.IsNotExist(err) { + t.Errorf("Expected orphaned tfvars file to be removed, but it still exists") + } + + // And the valid component's tfvars should still exist + validTfvars := filepath.Join(projectRoot, ".windsor", ".tf_modules", "test-module", "terraform.tfvars") + if _, err := os.Stat(validTfvars); err != nil { + t.Errorf("Expected valid component tfvars to exist, got error: %v", err) + } + }) + t.Run("HandlesWriteVariableWithDescriptionInInfo", func(t *testing.T) { // Given a resolver with a variable that has description in VariableInfo passed to writeVariable resolver, mocks := setup(t) diff --git a/pkg/runtime/env/terraform_env.go b/pkg/runtime/env/terraform_env.go index f85324a81..91cdc2b99 100644 --- a/pkg/runtime/env/terraform_env.go +++ b/pkg/runtime/env/terraform_env.go @@ -118,11 +118,15 @@ func (e *TerraformEnvPrinter) GenerateTerraformArgs(projectPath, modulePath stri return nil, fmt.Errorf("error getting config root: %w", err) } + projectRoot, err := e.shell.GetProjectRoot() + if err != nil { + return nil, fmt.Errorf("error getting project root: %w", err) + } + patterns := []string{ + filepath.Join(projectRoot, ".windsor", ".tf_modules", projectPath, "terraform.tfvars"), filepath.Join(configRoot, "terraform", projectPath+".tfvars"), filepath.Join(configRoot, "terraform", projectPath+".tfvars.json"), - filepath.Join(configRoot, "terraform", projectPath+"_generated.tfvars"), - filepath.Join(configRoot, "terraform", projectPath+"_generated.tfvars.json"), } var varFileArgs []string diff --git a/pkg/runtime/env/terraform_env_test.go b/pkg/runtime/env/terraform_env_test.go index 69f5e64c1..9f0a568d9 100644 --- a/pkg/runtime/env/terraform_env_test.go +++ b/pkg/runtime/env/terraform_env_test.go @@ -48,12 +48,10 @@ func setupTerraformEnvMocks(t *testing.T, overrides ...*EnvTestMocks) *EnvTestMo if strings.Contains(nameSlash, "project/path.tfvars") || strings.Contains(nameSlash, "project/path.tfvars.json") || strings.Contains(nameSlash, "project\\path.tfvars") || - strings.Contains(nameSlash, "project\\path.tfvars.json") { + strings.Contains(nameSlash, "project\\path.tfvars.json") || + strings.Contains(nameSlash, ".windsor/.tf_modules/project/path/terraform.tfvars") { return nil, nil } - if strings.Contains(nameSlash, "project/path_generated.tfvars") { - return nil, os.ErrNotExist - } return nil, os.ErrNotExist } @@ -84,24 +82,31 @@ func TestTerraformEnv_GetEnvVars(t *testing.T) { osType = "windows" } - // Get the actual config root + // Get the actual config root and project root configRoot, err := mocks.ConfigHandler.GetConfigRoot() if err != nil { t.Fatalf("Failed to get config root: %v", err) } + projectRoot, err := mocks.Shell.GetProjectRoot() + if err != nil { + t.Fatalf("Failed to get project root: %v", err) + } expectedEnvVars := map[string]string{ "TF_DATA_DIR": filepath.ToSlash(filepath.Join(configRoot, ".terraform/project/path")), "TF_CLI_ARGS_init": fmt.Sprintf(`-backend=true -force-copy -upgrade -backend-config="path=%s"`, filepath.ToSlash(filepath.Join(configRoot, ".tfstate/project/path/terraform.tfstate"))), - "TF_CLI_ARGS_plan": fmt.Sprintf(`-out="%s" -var-file="%s" -var-file="%s"`, + "TF_CLI_ARGS_plan": fmt.Sprintf(`-out="%s" -var-file="%s" -var-file="%s" -var-file="%s"`, filepath.ToSlash(filepath.Join(configRoot, ".terraform/project/path/terraform.tfplan")), + filepath.ToSlash(filepath.Join(projectRoot, ".windsor", ".tf_modules", "project/path", "terraform.tfvars")), filepath.ToSlash(filepath.Join(configRoot, "terraform/project/path.tfvars")), filepath.ToSlash(filepath.Join(configRoot, "terraform/project/path.tfvars.json"))), "TF_CLI_ARGS_apply": fmt.Sprintf(`"%s"`, filepath.ToSlash(filepath.Join(configRoot, ".terraform/project/path/terraform.tfplan"))), - "TF_CLI_ARGS_import": fmt.Sprintf(`-var-file="%s" -var-file="%s"`, + "TF_CLI_ARGS_import": fmt.Sprintf(`-var-file="%s" -var-file="%s" -var-file="%s"`, + filepath.ToSlash(filepath.Join(projectRoot, ".windsor", ".tf_modules", "project/path", "terraform.tfvars")), filepath.ToSlash(filepath.Join(configRoot, "terraform/project/path.tfvars")), filepath.ToSlash(filepath.Join(configRoot, "terraform/project/path.tfvars.json"))), - "TF_CLI_ARGS_destroy": fmt.Sprintf(`-var-file="%s" -var-file="%s"`, + "TF_CLI_ARGS_destroy": fmt.Sprintf(`-var-file="%s" -var-file="%s" -var-file="%s"`, + filepath.ToSlash(filepath.Join(projectRoot, ".windsor", ".tf_modules", "project/path", "terraform.tfvars")), filepath.ToSlash(filepath.Join(configRoot, "terraform/project/path.tfvars")), filepath.ToSlash(filepath.Join(configRoot, "terraform/project/path.tfvars.json"))), "TF_VAR_context_path": filepath.ToSlash(configRoot), @@ -275,11 +280,15 @@ func TestTerraformEnv_GetEnvVars(t *testing.T) { return nil, nil } - // Get the actual config root + // Get the actual config root and project root configRoot, err := mocks.ConfigHandler.GetConfigRoot() if err != nil { t.Fatalf("Failed to get config root: %v", err) } + projectRoot, err := mocks.Shell.GetProjectRoot() + if err != nil { + t.Fatalf("Failed to get project root: %v", err) + } // Mock Stat to handle both tfvars files mocks.Shims.Stat = func(name string) (os.FileInfo, error) { @@ -290,7 +299,8 @@ func TestTerraformEnv_GetEnvVars(t *testing.T) { if strings.Contains(nameSlash, "project/path.tfvars") || strings.Contains(nameSlash, "project/path.tfvars.json") || strings.Contains(nameSlash, "project\\path.tfvars") || - strings.Contains(nameSlash, "project\\path.tfvars.json") { + strings.Contains(nameSlash, "project\\path.tfvars.json") || + strings.Contains(nameSlash, ".windsor/.tf_modules/project/path/terraform.tfvars") { return nil, nil } return nil, os.ErrNotExist @@ -308,15 +318,18 @@ func TestTerraformEnv_GetEnvVars(t *testing.T) { expectedEnvVars := map[string]string{ "TF_DATA_DIR": filepath.ToSlash(filepath.Join(configRoot, ".terraform/project/path")), "TF_CLI_ARGS_init": fmt.Sprintf(`-backend=true -force-copy -upgrade -backend-config="path=%s"`, filepath.ToSlash(filepath.Join(configRoot, ".tfstate/project/path/terraform.tfstate"))), - "TF_CLI_ARGS_plan": fmt.Sprintf(`-out="%s" -var-file="%s" -var-file="%s"`, + "TF_CLI_ARGS_plan": fmt.Sprintf(`-out="%s" -var-file="%s" -var-file="%s" -var-file="%s"`, filepath.ToSlash(filepath.Join(configRoot, ".terraform/project/path/terraform.tfplan")), + filepath.ToSlash(filepath.Join(projectRoot, ".windsor", ".tf_modules", "project/path", "terraform.tfvars")), filepath.ToSlash(filepath.Join(configRoot, "terraform/project/path.tfvars")), filepath.ToSlash(filepath.Join(configRoot, "terraform/project/path.tfvars.json"))), "TF_CLI_ARGS_apply": fmt.Sprintf(`"%s"`, filepath.ToSlash(filepath.Join(configRoot, ".terraform/project/path/terraform.tfplan"))), - "TF_CLI_ARGS_import": fmt.Sprintf(`-var-file="%s" -var-file="%s"`, + "TF_CLI_ARGS_import": fmt.Sprintf(`-var-file="%s" -var-file="%s" -var-file="%s"`, + filepath.ToSlash(filepath.Join(projectRoot, ".windsor", ".tf_modules", "project/path", "terraform.tfvars")), filepath.ToSlash(filepath.Join(configRoot, "terraform/project/path.tfvars")), filepath.ToSlash(filepath.Join(configRoot, "terraform/project/path.tfvars.json"))), - "TF_CLI_ARGS_destroy": fmt.Sprintf(`-var-file="%s" -var-file="%s"`, + "TF_CLI_ARGS_destroy": fmt.Sprintf(`-var-file="%s" -var-file="%s" -var-file="%s"`, + filepath.ToSlash(filepath.Join(projectRoot, ".windsor", ".tf_modules", "project/path", "terraform.tfvars")), filepath.ToSlash(filepath.Join(configRoot, "terraform/project/path.tfvars")), filepath.ToSlash(filepath.Join(configRoot, "terraform/project/path.tfvars.json"))), "TF_VAR_context_path": filepath.ToSlash(configRoot), From 93aefec727783d709415389ad7f7cae7dcc7b116 Mon Sep 17 00:00:00 2001 From: Ryan VanGundy <85766511+rmvangun@users.noreply.github.com> Date: Thu, 20 Nov 2025 22:05:39 -0500 Subject: [PATCH 2/3] autolint Signed-off-by: Ryan VanGundy <85766511+rmvangun@users.noreply.github.com> --- pkg/composer/terraform/module_resolver_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/composer/terraform/module_resolver_test.go b/pkg/composer/terraform/module_resolver_test.go index 83729d5bb..e3164b1f1 100644 --- a/pkg/composer/terraform/module_resolver_test.go +++ b/pkg/composer/terraform/module_resolver_test.go @@ -1903,7 +1903,7 @@ variable "cluster_name" { type = string }` resolver, mocks := setup(t) projectRoot, _ := mocks.Shell.GetProjectRootFunc() - + // Create a component directory for a component that will not be in the blueprint orphanedDir := filepath.Join(projectRoot, ".windsor", ".tf_modules", "orphaned-component") if err := os.MkdirAll(orphanedDir, 0755); err != nil { From 9a61b39c420d83a18d71915c066be74c051bf36a Mon Sep 17 00:00:00 2001 From: Ryan VanGundy <85766511+rmvangun@users.noreply.github.com> Date: Thu, 20 Nov 2025 22:41:35 -0500 Subject: [PATCH 3/3] Fix tests Signed-off-by: Ryan VanGundy <85766511+rmvangun@users.noreply.github.com> --- pkg/composer/terraform/module_resolver.go | 1 - .../terraform/module_resolver_test.go | 58 ------------------- 2 files changed, 59 deletions(-) diff --git a/pkg/composer/terraform/module_resolver.go b/pkg/composer/terraform/module_resolver.go index fcbd16df9..f875e49e5 100644 --- a/pkg/composer/terraform/module_resolver.go +++ b/pkg/composer/terraform/module_resolver.go @@ -73,7 +73,6 @@ func NewBaseModuleResolver(rt *runtime.Runtime, blueprintHandler blueprint.Bluep // in the blueprint, it ensures a tfvars file is generated if not already handled by the input data. // The method uses the blueprint handler to retrieve TerraformComponents and determines the variables.tf // location based on component source (remote or local). Module resolution is handled by pkg/terraform. -// Before generating new tfvars files, it cleans up orphaned tfvars files for components that no longer exist. func (h *BaseModuleResolver) GenerateTfvars(overwrite bool) error { h.reset = overwrite diff --git a/pkg/composer/terraform/module_resolver_test.go b/pkg/composer/terraform/module_resolver_test.go index e3164b1f1..25ea70c97 100644 --- a/pkg/composer/terraform/module_resolver_test.go +++ b/pkg/composer/terraform/module_resolver_test.go @@ -1898,64 +1898,6 @@ variable "cluster_name" { type = string }` } }) - t.Run("CleansUpOrphanedTfvars", func(t *testing.T) { - // Given a resolver with an orphaned tfvars file for a component that no longer exists - resolver, mocks := setup(t) - - projectRoot, _ := mocks.Shell.GetProjectRootFunc() - - // Create a component directory for a component that will not be in the blueprint - orphanedDir := filepath.Join(projectRoot, ".windsor", ".tf_modules", "orphaned-component") - if err := os.MkdirAll(orphanedDir, 0755); err != nil { - t.Fatalf("Failed to create orphaned dir: %v", err) - } - orphanedTfvars := filepath.Join(orphanedDir, "terraform.tfvars") - if err := os.WriteFile(orphanedTfvars, []byte("old_value = \"stale\""), 0644); err != nil { - t.Fatalf("Failed to write orphaned tfvars: %v", err) - } - - // And a valid component that exists in the blueprint - mocks.BlueprintHandler.GetTerraformComponentsFunc = func() []blueprintv1alpha1.TerraformComponent { - return []blueprintv1alpha1.TerraformComponent{ - { - Path: "test-module", - Source: "git::https://github.com/test/module.git", - FullPath: filepath.Join(projectRoot, "terraform", "test-module"), - Inputs: map[string]any{ - "cluster_name": "test-cluster", - }, - }, - } - } - - variablesDir := filepath.Join(projectRoot, ".windsor", ".tf_modules", "test-module") - if err := os.MkdirAll(variablesDir, 0755); err != nil { - t.Fatalf("Failed to create dir: %v", err) - } - if err := os.WriteFile(filepath.Join(variablesDir, "variables.tf"), []byte(`variable "cluster_name" { type = string }`), 0644); err != nil { - t.Fatalf("Failed to write variables.tf: %v", err) - } - - // When generating tfvars - err := resolver.GenerateTfvars(false) - - // Then it should succeed - if err != nil { - t.Errorf("Expected no error, got: %v", err) - } - - // And the orphaned tfvars file should be removed - if _, err := os.Stat(orphanedTfvars); !os.IsNotExist(err) { - t.Errorf("Expected orphaned tfvars file to be removed, but it still exists") - } - - // And the valid component's tfvars should still exist - validTfvars := filepath.Join(projectRoot, ".windsor", ".tf_modules", "test-module", "terraform.tfvars") - if _, err := os.Stat(validTfvars); err != nil { - t.Errorf("Expected valid component tfvars to exist, got error: %v", err) - } - }) - t.Run("HandlesWriteVariableWithDescriptionInInfo", func(t *testing.T) { // Given a resolver with a variable that has description in VariableInfo passed to writeVariable resolver, mocks := setup(t)