From 6c10631aadfe414b0d9cac0e0adaf06cf12d8ce4 Mon Sep 17 00:00:00 2001 From: Ryan VanGundy Date: Sun, 12 Oct 2025 18:39:46 -0400 Subject: [PATCH] refactor(blueprint): Introduce postBuild substitutions via Features It is now possible to define kustomization postBuild substitutions directly on components in Features. These may use `expr` syntax in the same manner as Terraform input values. --- api/v1alpha1/feature_types.go | 15 + api/v1alpha1/feature_types_test.go | 17 + pkg/blueprint/blueprint_handler.go | 147 ++-- .../blueprint_handler_private_test.go | 706 ++++++------------ .../blueprint_handler_public_test.go | 52 +- pkg/config/schema_validator.go | 32 + 6 files changed, 416 insertions(+), 553 deletions(-) diff --git a/api/v1alpha1/feature_types.go b/api/v1alpha1/feature_types.go index 1a087e5ac..954d4f27b 100644 --- a/api/v1alpha1/feature_types.go +++ b/api/v1alpha1/feature_types.go @@ -52,6 +52,13 @@ type ConditionalKustomization struct { // When is a CEL expression that determines if this kustomization should be applied. // If empty, the kustomization is always applied when the parent feature matches. When string `yaml:"when,omitempty"` + + // Substitutions contains substitution values for post-build variable replacement. + // These values are collected and stored in ConfigMaps for use by Flux postBuild substitution. + // Values can be expressions using ${} syntax (e.g., "${dns.domain}") or literals (e.g., "example.com"). + // Values with ${} are evaluated as expressions, plain values are passed through as literals. + // All values are converted to strings as required by Flux variable substitution. + Substitutions map[string]string `yaml:"substitutions,omitempty"` } // DeepCopy creates a deep copy of the Feature object. @@ -92,9 +99,13 @@ func (f *Feature) DeepCopy() *Feature { kustomizationsCopy := make([]ConditionalKustomization, len(f.Kustomizations)) for i, kustomization := range f.Kustomizations { + substitutionsCopy := make(map[string]string, len(kustomization.Substitutions)) + maps.Copy(substitutionsCopy, kustomization.Substitutions) + kustomizationsCopy[i] = ConditionalKustomization{ Kustomization: *kustomization.Kustomization.DeepCopy(), When: kustomization.When, + Substitutions: substitutionsCopy, } } @@ -143,8 +154,12 @@ func (c *ConditionalKustomization) DeepCopy() *ConditionalKustomization { return nil } + substitutionsCopy := make(map[string]string, len(c.Substitutions)) + maps.Copy(substitutionsCopy, c.Substitutions) + return &ConditionalKustomization{ Kustomization: *c.Kustomization.DeepCopy(), When: c.When, + Substitutions: substitutionsCopy, } } diff --git a/api/v1alpha1/feature_types_test.go b/api/v1alpha1/feature_types_test.go index 2ef4a3979..74c5d49e7 100644 --- a/api/v1alpha1/feature_types_test.go +++ b/api/v1alpha1/feature_types_test.go @@ -45,6 +45,9 @@ func TestFeatureDeepCopy(t *testing.T) { DependsOn: []string{"pki-base"}, }, When: "ingress.enabled == true", + Substitutions: map[string]string{ + "host": "example.com", + }, }, }, } @@ -79,6 +82,11 @@ func TestFeatureDeepCopy(t *testing.T) { if copy.Kustomizations[0].Components[0] == "modified" { t.Error("Deep copy failed: kustomization components slice was not copied") } + + original.Kustomizations[0].Substitutions["host"] = "modified" + if copy.Kustomizations[0].Substitutions["host"] == "modified" { + t.Error("Deep copy failed: kustomization substitutions map was not copied") + } }) } @@ -146,6 +154,10 @@ func TestConditionalKustomizationDeepCopy(t *testing.T) { Interval: interval, }, When: "ingress.enabled == true", + Substitutions: map[string]string{ + "host": "example.com", + "replicas": "3", + }, } copy := original.DeepCopy() @@ -167,6 +179,11 @@ func TestConditionalKustomizationDeepCopy(t *testing.T) { if copy.DependsOn[0] == "modified" { t.Error("Deep copy failed: dependsOn slice was not copied") } + + original.Substitutions["host"] = "modified" + if copy.Substitutions["host"] == "modified" { + t.Error("Deep copy failed: substitutions map was not copied") + } }) } diff --git a/pkg/blueprint/blueprint_handler.go b/pkg/blueprint/blueprint_handler.go index 9a0a29a29..2f93e5601 100644 --- a/pkg/blueprint/blueprint_handler.go +++ b/pkg/blueprint/blueprint_handler.go @@ -65,24 +65,26 @@ var defaultJsonnetTemplate string type BaseBlueprintHandler struct { BlueprintHandler - injector di.Injector - configHandler config.ConfigHandler - shell shell.Shell - kubernetesManager kubernetes.KubernetesManager - blueprint blueprintv1alpha1.Blueprint - projectRoot string - shims *Shims - kustomizeData map[string]any - configLoaded bool + injector di.Injector + configHandler config.ConfigHandler + shell shell.Shell + kubernetesManager kubernetes.KubernetesManager + blueprint blueprintv1alpha1.Blueprint + projectRoot string + shims *Shims + kustomizeData map[string]any + featureSubstitutions map[string]map[string]string + configLoaded bool } // NewBlueprintHandler creates a new instance of BaseBlueprintHandler. // It initializes the handler with the provided dependency injector. func NewBlueprintHandler(injector di.Injector) *BaseBlueprintHandler { return &BaseBlueprintHandler{ - injector: injector, - shims: NewShims(), - kustomizeData: make(map[string]any), + injector: injector, + shims: NewShims(), + kustomizeData: make(map[string]any), + featureSubstitutions: make(map[string]map[string]string), } } @@ -491,7 +493,7 @@ func (b *BaseBlueprintHandler) GetLocalTemplateData() (map[string][]byte, error) config := make(map[string]any) for k, v := range contextValues { - if k != "substitution" { + if k != "substitutions" { config[k] = v } } @@ -515,8 +517,8 @@ func (b *BaseBlueprintHandler) GetLocalTemplateData() (map[string][]byte, error) } if contextValues != nil { - if substitutionValues, ok := contextValues["substitution"].(map[string]any); ok && len(substitutionValues) > 0 { - if existingValues, exists := templateData["substitution"]; exists { + if substitutionValues, ok := contextValues["substitutions"].(map[string]any); ok && len(substitutionValues) > 0 { + if existingValues, exists := templateData["substitutions"]; exists { var ociSubstitutionValues map[string]any if err := b.shims.YamlUnmarshal(existingValues, &ociSubstitutionValues); err == nil { substitutionValues = b.deepMergeMaps(ociSubstitutionValues, substitutionValues) @@ -526,7 +528,7 @@ func (b *BaseBlueprintHandler) GetLocalTemplateData() (map[string][]byte, error) if err != nil { return nil, fmt.Errorf("failed to marshal substitution values: %w", err) } - templateData["substitution"] = substitutionYAML + templateData["substitutions"] = substitutionYAML } } @@ -844,7 +846,22 @@ func (b *BaseBlueprintHandler) processFeatures(templateData map[string][]byte, c continue } } + kustomizationCopy := kustomization.Kustomization + + if len(kustomization.Substitutions) > 0 { + if b.featureSubstitutions[kustomizationCopy.Name] == nil { + b.featureSubstitutions[kustomizationCopy.Name] = make(map[string]string) + } + + evaluatedSubstitutions, err := b.evaluateSubstitutions(kustomization.Substitutions, config) + if err != nil { + return fmt.Errorf("failed to evaluate substitutions for kustomization '%s': %w", kustomizationCopy.Name, err) + } + + maps.Copy(b.featureSubstitutions[kustomizationCopy.Name], evaluatedSubstitutions) + } + tempBlueprint := &blueprintv1alpha1.Blueprint{ Kustomizations: []blueprintv1alpha1.Kustomization{kustomizationCopy}, } @@ -1337,15 +1354,7 @@ func (b *BaseBlueprintHandler) toFluxKustomization(k blueprintv1alpha1.Kustomiza var postBuild *kustomizev1.PostBuild substituteFrom := make([]kustomizev1.SubstituteReference, 0) - substituteFrom = append(substituteFrom, kustomizev1.SubstituteReference{ - Kind: "ConfigMap", - Name: "values-common", - Optional: false, - }) - - hasComponentValues := b.hasComponentValues(k.Name) - - if hasComponentValues { + if substitutions, hasSubstitutions := b.featureSubstitutions[k.Name]; hasSubstitutions && len(substitutions) > 0 { configMapName := fmt.Sprintf("values-%s", k.Name) substituteFrom = append(substituteFrom, kustomizev1.SubstituteReference{ Kind: "ConfigMap", @@ -1535,38 +1544,6 @@ func (b *BaseBlueprintHandler) extractTargetFromPatchContent(patchContent, defau return nil } -// hasComponentValues determines if component-specific values exist for the specified component name. -// It inspects both in-memory rendered template data and user-defined disk files, returning true if either contains the component section. -// User-defined values take precedence in the presence of both sources. -func (b *BaseBlueprintHandler) hasComponentValues(componentName string) bool { - hasTemplateComponent := false - if kustomizeValues, exists := b.kustomizeData["kustomize/values"]; exists { - if valuesMap, ok := kustomizeValues.(map[string]any); ok { - if _, hasComponent := valuesMap[componentName]; hasComponent { - hasTemplateComponent = true - } - } - } - - hasUserComponent := false - configRoot, err := b.configHandler.GetConfigRoot() - if err == nil { - valuesPath := filepath.Join(configRoot, "kustomize", "values.yaml") - if _, err := b.shims.Stat(valuesPath); err == nil { - if data, err := b.shims.ReadFile(valuesPath); err == nil { - var values map[string]any - if err := b.shims.YamlUnmarshal(data, &values); err == nil { - if _, hasComponent := values[componentName]; hasComponent { - hasUserComponent = true - } - } - } - } - } - - return hasTemplateComponent || hasUserComponent -} - // isOCISource returns true if the provided sourceNameOrURL is an OCI repository reference. // It checks if the input is a resolved OCI URL, matches the blueprint's main repository with an OCI URL, // or matches any additional source with an OCI URL. @@ -1625,23 +1602,28 @@ func (b *BaseBlueprintHandler) applyValuesConfigMaps() error { mergedCommonValues["BUILD_ID"] = buildID } - contextValues, err := b.configHandler.GetContextValues() - if err != nil { - return fmt.Errorf("failed to load context values: %w", err) - } + allValues := make(map[string]any) - renderedValues := make(map[string]any) - if substitutionData, exists := b.kustomizeData["substitution"]; exists { - if substitutionMap, ok := substitutionData.(map[string]any); ok { - renderedValues = substitutionMap + for kustomizationName, substitutions := range b.featureSubstitutions { + if len(substitutions) > 0 { + if allValues[kustomizationName] == nil { + allValues[kustomizationName] = make(map[string]any) + } + if componentMap, ok := allValues[kustomizationName].(map[string]any); ok { + for k, v := range substitutions { + componentMap[k] = v + } + } } } - allValues := make(map[string]any) - maps.Copy(allValues, renderedValues) + contextValues, err := b.configHandler.GetContextValues() + if err != nil { + return fmt.Errorf("failed to load context values: %w", err) + } if contextValues != nil { - if substitutionValues, ok := contextValues["substitution"].(map[string]any); ok { + if substitutionValues, ok := contextValues["substitutions"].(map[string]any); ok { allValues = b.deepMergeMaps(allValues, substitutionValues) } } @@ -1740,6 +1722,35 @@ func (b *BaseBlueprintHandler) createConfigMap(values map[string]any, configMapN return nil } +// evaluateSubstitutions evaluates expressions in substitution values and converts all results to strings. +// Values can use ${} syntax for expressions (e.g., "${dns.domain}") or be literals. +// All evaluated values are converted to strings as required by Flux postBuild substitution. +func (b *BaseBlueprintHandler) evaluateSubstitutions(substitutions map[string]string, config map[string]any) (map[string]string, error) { + result := make(map[string]string) + evaluator := NewFeatureEvaluator() + + for key, value := range substitutions { + if strings.Contains(value, "${") { + anyMap := map[string]any{key: value} + evaluated, err := evaluator.EvaluateDefaults(anyMap, config) + if err != nil { + return nil, fmt.Errorf("failed to evaluate substitution for key '%s': %w", key, err) + } + + evaluatedValue := evaluated[key] + if evaluatedValue == nil { + result[key] = "" + } else { + result[key] = fmt.Sprintf("%v", evaluatedValue) + } + } else { + result[key] = value + } + } + + return result, nil +} + // flattenValuesToConfigMap recursively flattens nested values into a flat map suitable for ConfigMap data. // Nested maps are flattened using dot notation (e.g., "ingress.host"). func (b *BaseBlueprintHandler) flattenValuesToConfigMap(values map[string]any, prefix string, result map[string]string) error { diff --git a/pkg/blueprint/blueprint_handler_private_test.go b/pkg/blueprint/blueprint_handler_private_test.go index a434d790b..40e4cb74c 100644 --- a/pkg/blueprint/blueprint_handler_private_test.go +++ b/pkg/blueprint/blueprint_handler_private_test.go @@ -4,7 +4,6 @@ import ( "fmt" "os" "path/filepath" - "reflect" "strings" "testing" "time" @@ -164,7 +163,7 @@ func TestBaseBlueprintHandler_applyValuesConfigMaps(t *testing.T) { } mockConfigHandler.GetContextValuesFunc = func() (map[string]any, error) { return map[string]any{ - "substitution": map[string]any{ + "substitutions": map[string]any{ "common": map[string]any{ "domain": "example.com", }, @@ -196,14 +195,14 @@ func TestBaseBlueprintHandler_applyValuesConfigMaps(t *testing.T) { // And mock file read for context values handler.shims.ReadFile = func(name string) ([]byte, error) { if name == filepath.Join(projectRoot, "contexts", "_template", "values.yaml") { - return []byte(`substitution: + return []byte(`substitutions: common: domain: template.com ingress: host: template.example.com`), nil } if name == filepath.Join(configRoot, "values.yaml") { - return []byte(`substitution: + return []byte(`substitutions: common: domain: example.com ingress: @@ -253,6 +252,215 @@ func TestBaseBlueprintHandler_applyValuesConfigMaps(t *testing.T) { } }) + t.Run("SuccessWithKustomizationSubstitutions", func(t *testing.T) { + handler := setup(t) + + configRoot := "/test/config" + projectRoot := "/test/project" + + mockConfigHandler := handler.configHandler.(*config.MockConfigHandler) + mockConfigHandler.GetConfigRootFunc = func() (string, error) { + return configRoot, nil + } + mockConfigHandler.GetStringFunc = func(key string, defaultValue ...string) string { + switch key { + case "dns.domain": + return "example.com" + case "network.loadbalancer_ips.start": + return "192.168.1.100" + case "network.loadbalancer_ips.end": + return "192.168.1.200" + case "docker.registry_url": + return "registry.example.com" + case "id": + return "test-id" + default: + return "" + } + } + mockConfigHandler.GetContextFunc = func() string { + return "test-context" + } + mockConfigHandler.GetStringSliceFunc = func(key string, defaultValue ...[]string) []string { + if key == "cluster.workers.volumes" { + return []string{"/host:/container"} + } + return []string{} + } + mockConfigHandler.GetContextValuesFunc = func() (map[string]any, error) { + return map[string]any{}, nil + } + + mockShell := handler.shell.(*shell.MockShell) + mockShell.GetProjectRootFunc = func() (string, error) { + return projectRoot, nil + } + + handler.shims.Stat = func(name string) (os.FileInfo, error) { + if name == filepath.Join(projectRoot, ".windsor", ".build-id") { + return nil, os.ErrNotExist + } + return nil, os.ErrNotExist + } + + handler.blueprint = blueprintv1alpha1.Blueprint{ + Kustomizations: []blueprintv1alpha1.Kustomization{ + { + Name: "ingress", + Path: "ingress", + }, + { + Name: "monitoring", + Path: "monitoring", + }, + }, + } + + handler.featureSubstitutions = map[string]map[string]string{ + "ingress": { + "host": "ingress.example.com", + "replicas": "3", + }, + "monitoring": { + "retention": "30d", + "enabled": "true", + }, + } + + var appliedConfigMaps []string + var configMapData = make(map[string]map[string]string) + mockKubernetesManager := handler.kubernetesManager.(*kubernetes.MockKubernetesManager) + mockKubernetesManager.ApplyConfigMapFunc = func(name, namespace string, data map[string]string) error { + appliedConfigMaps = append(appliedConfigMaps, name) + configMapData[name] = data + return nil + } + + err := handler.applyValuesConfigMaps() + + if err != nil { + t.Fatalf("expected applyValuesConfigMaps to succeed, got: %v", err) + } + + if len(appliedConfigMaps) != 3 { + t.Errorf("expected 3 ConfigMaps to be applied (common, ingress, monitoring), got %d: %v", len(appliedConfigMaps), appliedConfigMaps) + } + + ingressFound := false + monitoringFound := false + for _, name := range appliedConfigMaps { + if name == "values-ingress" { + ingressFound = true + } + if name == "values-monitoring" { + monitoringFound = true + } + } + + if !ingressFound { + t.Error("expected values-ingress ConfigMap to be applied") + } + if !monitoringFound { + t.Error("expected values-monitoring ConfigMap to be applied") + } + + if data, ok := configMapData["values-ingress"]; ok { + if data["host"] != "ingress.example.com" { + t.Errorf("expected ingress host to be 'ingress.example.com', got '%s'", data["host"]) + } + if data["replicas"] != "3" { + t.Errorf("expected ingress replicas to be '3', got '%s'", data["replicas"]) + } + } + + if data, ok := configMapData["values-monitoring"]; ok { + if data["retention"] != "30d" { + t.Errorf("expected monitoring retention to be '30d', got '%s'", data["retention"]) + } + if data["enabled"] != "true" { + t.Errorf("expected monitoring enabled to be 'true', got '%s'", data["enabled"]) + } + } + }) + + t.Run("EvaluatesKustomizationSubstitutionExpressions", func(t *testing.T) { + handler := setup(t) + + baseBlueprint := []byte(`kind: Blueprint +apiVersion: blueprints.windsorcli.dev/v1alpha1 +metadata: + name: base +kustomize: + - name: ingress + path: ingress +`) + + featureWithSubstitutions := []byte(`kind: Feature +apiVersion: blueprints.windsorcli.dev/v1alpha1 +metadata: + name: ingress-config +when: ingress.enabled == true +kustomize: + - name: ingress + path: ingress + substitutions: + host: "${dns.domain}" + replicas: "${cluster.workers.count}" + url: "https://${dns.domain}" + literal: "my-literal-value" +`) + + templateData := map[string][]byte{ + "blueprint": baseBlueprint, + "features/ingress-cfg.yaml": featureWithSubstitutions, + } + + config := map[string]any{ + "ingress": map[string]any{ + "enabled": true, + }, + "dns": map[string]any{ + "domain": "example.com", + }, + "cluster": map[string]any{ + "workers": map[string]any{ + "count": 3, + }, + }, + } + + err := handler.processFeatures(templateData, config) + + if err != nil { + t.Fatalf("Expected no error, got %v", err) + } + + if len(handler.featureSubstitutions) != 1 { + t.Fatalf("Expected 1 kustomization with substitutions, got %d", len(handler.featureSubstitutions)) + } + + ingressSubs, ok := handler.featureSubstitutions["ingress"] + if !ok { + t.Fatal("Expected ingress substitutions to be present") + } + + if ingressSubs["host"] != "example.com" { + t.Errorf("Expected host to be 'example.com', got '%s'", ingressSubs["host"]) + } + + if ingressSubs["replicas"] != "3" { + t.Errorf("Expected replicas to be '3', got '%s'", ingressSubs["replicas"]) + } + + if ingressSubs["url"] != "https://example.com" { + t.Errorf("Expected url to be 'https://example.com', got '%s'", ingressSubs["url"]) + } + + if ingressSubs["literal"] != "my-literal-value" { + t.Errorf("Expected literal to be 'my-literal-value', got '%s'", ingressSubs["literal"]) + } + }) + t.Run("NoKustomizeDirectory", func(t *testing.T) { // Given a handler handler := setup(t) @@ -455,176 +663,6 @@ ingress: t.Errorf("expected error about common ConfigMap creation, got: %v", err) } }) - - t.Run("SuccessWithRenderedSubstitutionValues", func(t *testing.T) { - // Given a handler with rendered substitution values from substitution.jsonnet - handler := setup(t) - - // Set up rendered substitution data (simulating substitution.jsonnet output) - handler.kustomizeData = map[string]any{ - "substitution": map[string]any{ - "common": map[string]any{ - "external_domain": "rendered.test", - "registry_url": "registry.rendered.test", - }, - "app_config": map[string]any{ - "replicas": 2, - }, - }, - } - - // Mock config handler - projectRoot := filepath.Join("test", "project") - configRoot := filepath.Join("test", "config") - mockConfigHandler := handler.configHandler.(*config.MockConfigHandler) - mockConfigHandler.GetConfigRootFunc = func() (string, error) { - return configRoot, nil - } - mockConfigHandler.GetStringFunc = func(key string, defaultValue ...string) string { - switch key { - case "dns.domain": - return "example.com" - case "network.loadbalancer_ips.start": - return "192.168.1.100" - case "network.loadbalancer_ips.end": - return "192.168.1.200" - case "docker.registry_url": - return "registry.example.com" - case "id": - return "test-id" - default: - return "" - } - } - mockConfigHandler.GetContextFunc = func() string { - return "test-context" - } - mockConfigHandler.GetStringSliceFunc = func(key string, defaultValue ...[]string) []string { - return []string{} - } - mockConfigHandler.GetContextValuesFunc = func() (map[string]any, error) { - return map[string]any{ - "substitution": map[string]any{ - "common": map[string]any{ - "external_domain": "context.test", - "context_key": "context_value", - }, - "app_config": map[string]any{ - "replicas": 5, - }, - }, - }, nil - } - - // Mock shell for project root - mockShell := handler.shell.(*shell.MockShell) - mockShell.GetProjectRootFunc = func() (string, error) { - return projectRoot, nil - } - - // Mock context values that override some rendered values - handler.shims.Stat = func(name string) (os.FileInfo, error) { - if name == filepath.Join(projectRoot, "contexts", "_template", "schema.yaml") { - return &mockFileInfo{name: "schema.yaml"}, nil - } - if name == filepath.Join(configRoot, "values.yaml") { - return &mockFileInfo{name: "values.yaml"}, nil - } - return nil, os.ErrNotExist - } - - handler.shims.ReadFile = func(name string) ([]byte, error) { - if name == filepath.Join(projectRoot, "contexts", "_template", "schema.yaml") { - return []byte(`$schema: https://json-schema.org/draft/2020-12/schema -type: object -properties: - substitution: - type: object - properties: - common: - type: object - properties: - template_key: - type: string - default: "template_value" - additionalProperties: true - default: - template_key: "template_value" - additionalProperties: true -required: [] -additionalProperties: true`), nil - } - if name == filepath.Join(configRoot, "values.yaml") { - return []byte(`substitution: - common: - external_domain: context.test - context_key: context_value - app_config: - replicas: 5`), nil - } - return nil, os.ErrNotExist - } - - // Mock Kubernetes manager to capture applied ConfigMaps - var appliedConfigMaps []string - var configMapData map[string]map[string]string = make(map[string]map[string]string) - mockKubernetesManager := handler.kubernetesManager.(*kubernetes.MockKubernetesManager) - mockKubernetesManager.ApplyConfigMapFunc = func(name, namespace string, data map[string]string) error { - appliedConfigMaps = append(appliedConfigMaps, name) - configMapData[name] = data - return nil - } - - // When applying values ConfigMaps - err := handler.applyValuesConfigMaps() - - // Then it should succeed - if err != nil { - t.Fatalf("expected applyValuesConfigMaps to succeed, got: %v", err) - } - - // And it should apply ConfigMaps for common and app_config - if len(appliedConfigMaps) != 2 { - t.Errorf("expected 2 ConfigMaps to be applied, got %d: %v", len(appliedConfigMaps), appliedConfigMaps) - } - - // Check common ConfigMap - should have rendered values merged with context overrides and system values - if commonData, exists := configMapData["values-common"]; exists { - // Context values should override rendered values - if commonData["external_domain"] != "context.test" { - t.Errorf("expected external_domain to be 'context.test' (context override), got '%s'", commonData["external_domain"]) - } - // Rendered values should be preserved when not overridden - if commonData["registry_url"] != "registry.rendered.test" { - t.Errorf("expected registry_url to be 'registry.rendered.test' (from rendered), got '%s'", commonData["registry_url"]) - } - // Context-only values should be included - if commonData["context_key"] != "context_value" { - t.Errorf("expected context_key to be 'context_value', got '%s'", commonData["context_key"]) - } - // Template-only values should be included - // Note: Schema defaults don't flow through rendered substitution values in this test scenario - // This is expected behavior - rendered values take precedence over schema defaults - if commonData["template_key"] != "" { - t.Logf("template_key value: '%s' (schema defaults don't override rendered values)", commonData["template_key"]) - } - // System values should be included - if commonData["DOMAIN"] != "example.com" { - t.Errorf("expected DOMAIN to be 'example.com', got '%s'", commonData["DOMAIN"]) - } - } else { - t.Error("expected values-common ConfigMap to be applied") - } - - // Check app_config ConfigMap - should have context override of rendered value - if appConfigData, exists := configMapData["values-app_config"]; exists { - if appConfigData["replicas"] != "5" { - t.Errorf("expected replicas to be '5' (context override), got '%s'", appConfigData["replicas"]) - } - } else { - t.Error("expected values-app_config ConfigMap to be applied") - } - }) } // ============================================================================= @@ -644,8 +682,13 @@ func TestBaseBlueprintHandler_toFluxKustomization(t *testing.T) { } t.Run("WithGlobalValuesConfigMap", func(t *testing.T) { - // Given a handler + // Given a handler with feature substitutions handler := setup(t) + handler.featureSubstitutions = map[string]map[string]string{ + "test-kustomization": { + "domain": "example.com", + }, + } // And initialize the blueprint handler.blueprint = blueprintv1alpha1.Blueprint{ @@ -657,20 +700,6 @@ func TestBaseBlueprintHandler_toFluxKustomization(t *testing.T) { }, } - // And mock config root - mockConfigHandler := handler.configHandler.(*config.MockConfigHandler) - mockConfigHandler.GetConfigRootFunc = func() (string, error) { - return "/test/config", nil - } - - // And mock that global config.yaml exists - handler.shims.Stat = func(name string) (os.FileInfo, error) { - if name == filepath.Join("/test/config", "kustomize", "config.yaml") { - return &mockFileInfo{name: "config.yaml"}, nil - } - return nil, os.ErrNotExist - } - // And a kustomization kustomization := blueprintv1alpha1.Kustomization{ Name: "test-kustomization", @@ -691,29 +720,34 @@ func TestBaseBlueprintHandler_toFluxKustomization(t *testing.T) { t.Fatal("expected PostBuild to be set") } - // And it should have the blueprint ConfigMap reference + // And it should have the component ConfigMap reference if len(result.Spec.PostBuild.SubstituteFrom) < 1 { t.Fatal("expected at least 1 SubstituteFrom reference") } - commonValuesFound := false + componentValuesFound := false for _, ref := range result.Spec.PostBuild.SubstituteFrom { - if ref.Kind == "ConfigMap" && ref.Name == "values-common" { - commonValuesFound = true + if ref.Kind == "ConfigMap" && ref.Name == "values-test-kustomization" { + componentValuesFound = true if ref.Optional != false { - t.Errorf("expected values-common ConfigMap to be Optional=false, got %v", ref.Optional) + t.Errorf("expected values-test-kustomization ConfigMap to be Optional=false, got %v", ref.Optional) } } } - if !commonValuesFound { - t.Error("expected values-common ConfigMap reference to be present") + if !componentValuesFound { + t.Error("expected values-test-kustomization ConfigMap reference to be present") } }) t.Run("WithComponentValuesConfigMap", func(t *testing.T) { - // Given a handler + // Given a handler with feature substitutions handler := setup(t) + handler.featureSubstitutions = map[string]map[string]string{ + "ingress": { + "domain": "example.com", + }, + } // And initialize the blueprint handler.blueprint = blueprintv1alpha1.Blueprint{ @@ -725,39 +759,6 @@ func TestBaseBlueprintHandler_toFluxKustomization(t *testing.T) { }, } - // And mock config root - mockConfigHandler := handler.configHandler.(*config.MockConfigHandler) - mockConfigHandler.GetConfigRootFunc = func() (string, error) { - return "/test/config", nil - } - - // And mock that global values.yaml exists - handler.shims.Stat = func(name string) (os.FileInfo, error) { - if name == filepath.Join("/test/config", "kustomize", "values.yaml") { - return &mockFileInfo{name: "values.yaml"}, nil - } - return nil, os.ErrNotExist - } - - // And mock the values.yaml content with ingress component - handler.shims.ReadFile = func(name string) ([]byte, error) { - if name == filepath.Join("/test/config", "kustomize", "values.yaml") { - return []byte(`ingress: - key: value`), nil - } - return nil, os.ErrNotExist - } - - handler.shims.YamlUnmarshal = func(data []byte, v interface{}) error { - values := map[string]any{ - "ingress": map[string]any{ - "key": "value", - }, - } - reflect.ValueOf(v).Elem().Set(reflect.ValueOf(values)) - return nil - } - // And a kustomization with component name kustomization := blueprintv1alpha1.Kustomization{ Name: "ingress", @@ -867,14 +868,10 @@ func TestBaseBlueprintHandler_toFluxKustomization(t *testing.T) { t.Errorf("expected VAR2 to be 'value2', got '%s'", result.Spec.PostBuild.Substitute["VAR2"]) } - // And it should have the correct SubstituteFrom references - commonValuesFound := false + // And it should preserve the existing SubstituteFrom reference existingConfigFound := false for _, ref := range result.Spec.PostBuild.SubstituteFrom { - if ref.Kind == "ConfigMap" && ref.Name == "values-common" { - commonValuesFound = true - } if ref.Kind == "ConfigMap" && ref.Name == "existing-config" { existingConfigFound = true if ref.Optional != true { @@ -883,16 +880,20 @@ func TestBaseBlueprintHandler_toFluxKustomization(t *testing.T) { } } - if !commonValuesFound { - t.Error("expected values-common ConfigMap reference to be present") - } if !existingConfigFound { t.Error("expected existing-config ConfigMap reference to be preserved") } + + // And it should not add a component ConfigMap without feature substitutions + for _, ref := range result.Spec.PostBuild.SubstituteFrom { + if ref.Kind == "ConfigMap" && ref.Name == "values-test-kustomization" { + t.Error("did not expect values-test-kustomization ConfigMap reference without feature substitutions") + } + } }) t.Run("WithoutValuesConfigMaps", func(t *testing.T) { - // Given a handler + // Given a handler without feature substitutions handler := setup(t) // And initialize the blueprint @@ -905,17 +906,6 @@ func TestBaseBlueprintHandler_toFluxKustomization(t *testing.T) { }, } - // And mock config root - mockConfigHandler := handler.configHandler.(*config.MockConfigHandler) - mockConfigHandler.GetConfigRootFunc = func() (string, error) { - return "/test/config", nil - } - - // And mock that no config.yaml files exist - handler.shims.Stat = func(name string) (os.FileInfo, error) { - return nil, os.ErrNotExist - } - // And a kustomization kustomization := blueprintv1alpha1.Kustomization{ Name: "test-kustomization", @@ -931,30 +921,14 @@ func TestBaseBlueprintHandler_toFluxKustomization(t *testing.T) { // When converting to Flux kustomization result := handler.toFluxKustomization(kustomization, "test-namespace") - // Then it should have PostBuild with only common ConfigMap reference - if result.Spec.PostBuild == nil { - t.Fatal("expected PostBuild to be set") - } - - // And it should only have the common ConfigMap reference - if len(result.Spec.PostBuild.SubstituteFrom) != 1 { - t.Errorf("expected 1 SubstituteFrom reference, got %d", len(result.Spec.PostBuild.SubstituteFrom)) - } - - ref := result.Spec.PostBuild.SubstituteFrom[0] - if ref.Kind != "ConfigMap" { - t.Errorf("expected Kind to be 'ConfigMap', got '%s'", ref.Kind) - } - if ref.Name != "values-common" { - t.Errorf("expected Name to be 'values-common', got '%s'", ref.Name) - } - if ref.Optional != false { - t.Errorf("expected Optional to be false, got %v", ref.Optional) + // Then it should not have PostBuild since no substitutions + if result.Spec.PostBuild != nil && len(result.Spec.PostBuild.SubstituteFrom) > 0 { + t.Errorf("expected no SubstituteFrom references without feature substitutions, got %d", len(result.Spec.PostBuild.SubstituteFrom)) } }) t.Run("ConfigRootError", func(t *testing.T) { - // Given a handler + // Given a handler without feature substitutions handler := setup(t) // And initialize the blueprint @@ -967,12 +941,6 @@ func TestBaseBlueprintHandler_toFluxKustomization(t *testing.T) { }, } - // And mock config root that fails - mockConfigHandler := handler.configHandler.(*config.MockConfigHandler) - mockConfigHandler.GetConfigRootFunc = func() (string, error) { - return "", os.ErrNotExist - } - // And a kustomization kustomization := blueprintv1alpha1.Kustomization{ Name: "test-kustomization", @@ -988,22 +956,9 @@ func TestBaseBlueprintHandler_toFluxKustomization(t *testing.T) { // When converting to Flux kustomization result := handler.toFluxKustomization(kustomization, "test-namespace") - // Then it should still have PostBuild with only blueprint ConfigMap reference - if result.Spec.PostBuild == nil { - t.Fatal("expected PostBuild to be set") - } - - // And it should only have the blueprint ConfigMap reference (no values ConfigMaps due to error) - if len(result.Spec.PostBuild.SubstituteFrom) != 1 { - t.Errorf("expected 1 SubstituteFrom reference, got %d", len(result.Spec.PostBuild.SubstituteFrom)) - } - - ref := result.Spec.PostBuild.SubstituteFrom[0] - if ref.Kind != "ConfigMap" { - t.Errorf("expected Kind to be 'ConfigMap', got '%s'", ref.Kind) - } - if ref.Name != "values-common" { - t.Errorf("expected Name to be 'values-common', got '%s'", ref.Name) + // Then it should not have PostBuild since no substitutions + if result.Spec.PostBuild != nil && len(result.Spec.PostBuild.SubstituteFrom) > 0 { + t.Errorf("expected no SubstituteFrom references without feature substitutions, got %d", len(result.Spec.PostBuild.SubstituteFrom)) } }) @@ -1814,173 +1769,6 @@ kind: ConfigMap }) } -func TestBaseBlueprintHandler_hasComponentValues(t *testing.T) { - setup := func(t *testing.T) *BaseBlueprintHandler { - t.Helper() - injector := di.NewInjector() - handler := NewBlueprintHandler(injector) - handler.shims = NewShims() - handler.configHandler = config.NewMockConfigHandler() - return handler - } - - t.Run("TemplateComponentExists", func(t *testing.T) { - // Given handler with component in template data - handler := setup(t) - handler.kustomizeData = map[string]any{ - "kustomize/values": map[string]any{ - "test-component": map[string]any{ - "key": "value", - }, - }, - } - // When checking if component values exist - exists := handler.hasComponentValues("test-component") - // Then it should return true - if !exists { - t.Error("Expected component to exist in template data") - } - }) - - t.Run("UserComponentExists", func(t *testing.T) { - // Given handler with component in user file - handler := setup(t) - handler.configHandler.(*config.MockConfigHandler).GetConfigRootFunc = func() (string, error) { - return "/test/config", nil - } - handler.shims.Stat = func(name string) (os.FileInfo, error) { - return &mockFileInfo{name: "config.yaml", isDir: false}, nil - } - handler.shims.ReadFile = func(name string) ([]byte, error) { - return []byte(`test-component: - key: value`), nil - } - handler.shims.YamlUnmarshal = func(data []byte, v any) error { - values := v.(*map[string]any) - *values = map[string]any{ - "test-component": map[string]any{ - "key": "value", - }, - } - return nil - } - // When checking if component values exist - exists := handler.hasComponentValues("test-component") - // Then it should return true - if !exists { - t.Error("Expected component to exist in user file") - } - }) - - t.Run("BothTemplateAndUserExist", func(t *testing.T) { - // Given handler with component in both template and user data - handler := setup(t) - handler.kustomizeData = map[string]any{ - "kustomize/values": map[string]any{ - "test-component": map[string]any{ - "template-key": "template-value", - }, - }, - } - handler.configHandler.(*config.MockConfigHandler).GetConfigRootFunc = func() (string, error) { - return "/test/config", nil - } - handler.shims.Stat = func(name string) (os.FileInfo, error) { - return &mockFileInfo{name: "config.yaml", isDir: false}, nil - } - handler.shims.ReadFile = func(name string) ([]byte, error) { - return []byte(`test-component: - user-key: user-value`), nil - } - handler.shims.YamlUnmarshal = func(data []byte, v any) error { - values := v.(*map[string]any) - *values = map[string]any{ - "test-component": map[string]any{ - "user-key": "user-value", - }, - } - return nil - } - // When checking if component values exist - exists := handler.hasComponentValues("test-component") - // Then it should return true - if !exists { - t.Error("Expected component to exist in both sources") - } - }) - - t.Run("NoComponentExists", func(t *testing.T) { - // Given handler with no component data - handler := setup(t) - handler.configHandler.(*config.MockConfigHandler).GetConfigRootFunc = func() (string, error) { - return "/test/config", nil - } - handler.shims.Stat = func(name string) (os.FileInfo, error) { - return nil, os.ErrNotExist - } - // When checking if component values exist - exists := handler.hasComponentValues("test-component") - // Then it should return false - if exists { - t.Error("Expected component to not exist") - } - }) - - t.Run("ConfigRootError", func(t *testing.T) { - // Given handler with config root error - handler := setup(t) - handler.configHandler.(*config.MockConfigHandler).GetConfigRootFunc = func() (string, error) { - return "", fmt.Errorf("config root error") - } - // When checking if component values exist - exists := handler.hasComponentValues("test-component") - // Then it should return false - if exists { - t.Error("Expected component to not exist when config root fails") - } - }) - - t.Run("FileNotExists", func(t *testing.T) { - // Given handler with file not existing - handler := setup(t) - handler.configHandler.(*config.MockConfigHandler).GetConfigRootFunc = func() (string, error) { - return "/test/config", nil - } - handler.shims.Stat = func(name string) (os.FileInfo, error) { - return nil, os.ErrNotExist - } - // When checking if component values exist - exists := handler.hasComponentValues("test-component") - // Then it should return false - if exists { - t.Error("Expected component to not exist when file doesn't exist") - } - }) - - t.Run("InvalidValuesFile", func(t *testing.T) { - // Given handler with invalid values file - handler := setup(t) - handler.configHandler.(*config.MockConfigHandler).GetConfigRootFunc = func() (string, error) { - return "/test/config", nil - } - handler.shims.Stat = func(name string) (os.FileInfo, error) { - return &mockFileInfo{name: "config.yaml", isDir: false}, nil - } - handler.shims.ReadFile = func(name string) ([]byte, error) { - return []byte("invalid yaml"), nil - } - handler.shims.YamlUnmarshal = func(data []byte, v any) error { - return fmt.Errorf("invalid yaml") - } - // When checking if component values exist - exists := handler.hasComponentValues("test-component") - // Then it should return false - if exists { - t.Error("Expected component to not exist when values file is invalid") - } - }) -} - func TestBaseBlueprintHandler_deepMergeMaps(t *testing.T) { setup := func(t *testing.T) *BaseBlueprintHandler { t.Helper() diff --git a/pkg/blueprint/blueprint_handler_public_test.go b/pkg/blueprint/blueprint_handler_public_test.go index d34899bca..9784d878f 100644 --- a/pkg/blueprint/blueprint_handler_public_test.go +++ b/pkg/blueprint/blueprint_handler_public_test.go @@ -65,7 +65,7 @@ properties: type: string default: "auto" additionalProperties: true - substitution: + substitutions: type: object properties: common: @@ -241,7 +241,7 @@ func setupShims(t *testing.T) *Shims { return []byte(mockSchemaContent()), nil case strings.Contains(name, "contexts") && strings.Contains(name, "values.yaml"): // Default context values for tests - return []byte(`substitution: + return []byte(`substitutions: common: external_domain: test.local`), nil default: @@ -2584,7 +2584,7 @@ properties: enabled: type: boolean default: true - substitution: + substitutions: type: object properties: common: @@ -2612,7 +2612,7 @@ additionalProperties: true`), nil return []byte(`external_domain: context.test context_only: enabled: true -substitution: +substitutions: common: external_domain: context.test context_only: @@ -2635,7 +2635,7 @@ substitution: "context_only": map[string]any{ "enabled": true, }, - "substitution": map[string]any{ + "substitutions": map[string]any{ "common": map[string]any{ "external_domain": "context.test", "registry_url": "registry.local.test", @@ -2668,10 +2668,10 @@ substitution: // Values validation is now done at the schema level, not in templateData - // Check for substitution (substitution section for ConfigMaps) - substitutionValuesData, exists := result["substitution"] + // Check for substitutions (substitutions section for ConfigMaps) + substitutionValuesData, exists := result["substitutions"] if !exists { - t.Fatal("Expected 'substitution' key to exist in result") + t.Fatal("Expected 'substitutions' key to exist in result") } var substitutionValues map[string]any @@ -2731,7 +2731,7 @@ substitution: return map[string]any{ "external_domain": "context.test", "context_only": "context_value", - "substitution": map[string]any{ + "substitutions": map[string]any{ "common": map[string]any{ "registry_url": "registry.context.test", }, @@ -2785,7 +2785,7 @@ properties: template_only: type: string default: "template_value" - substitution: + substitutions: type: object properties: common: @@ -2803,7 +2803,7 @@ additionalProperties: true`), nil return []byte(` external_domain: context.test context_only: context_value -substitution: +substitutions: common: registry_url: registry.context.test csi: @@ -2845,15 +2845,15 @@ substitution: // Values validation is now handled through schema processing - // Check that substitution values are merged and included - substitutionData, exists := result["substitution"] + // Check that substitutions values are merged and included + substitutionData, exists := result["substitutions"] if !exists { - t.Fatal("Expected 'substitution' key to exist in result") + t.Fatal("Expected 'substitutions' key to exist in result") } var substitution map[string]any if err := yaml.Unmarshal(substitutionData, &substitution); err != nil { - t.Fatalf("Failed to unmarshal substitution: %v", err) + t.Fatalf("Failed to unmarshal substitutions: %v", err) } // Check common section merging @@ -2906,7 +2906,7 @@ substitution: return map[string]any{ "external_domain": "context.test", "context_only": "context_value", - "substitution": map[string]any{ + "substitutions": map[string]any{ "common": map[string]any{ "registry_url": "registry.context.test", "context_sub": "context_sub_value", @@ -2943,7 +2943,7 @@ substitution: return []byte(` external_domain: context.test context_only: context_value -substitution: +substitutions: common: registry_url: registry.context.test context_sub: context_sub_value @@ -2973,15 +2973,15 @@ substitution: // This test doesn't include schema.yaml in the mock, so no schema key expected // Values processing is handled through the config context now - // Check substitution values - substitutionData, exists := result["substitution"] + // Check substitutions values + substitutionData, exists := result["substitutions"] if !exists { - t.Fatal("Expected 'substitution' key to exist in result") + t.Fatal("Expected 'substitutions' key to exist in result") } var substitution map[string]any if err := yaml.Unmarshal(substitutionData, &substitution); err != nil { - t.Fatalf("Failed to unmarshal substitution: %v", err) + t.Fatalf("Failed to unmarshal substitutions: %v", err) } common, exists := substitution["common"].(map[string]any) @@ -4111,7 +4111,7 @@ terraform: } mockConfigHandler.GetContextValuesFunc = func() (map[string]any, error) { return map[string]any{ - "substitution": map[string]any{ + "substitutions": map[string]any{ "domain": "example.com", "port": 8080, }, @@ -4139,14 +4139,14 @@ terraform: t.Fatalf("Expected no error, got %v", err) } - substitution, exists := templateData["substitution"] + substitutions, exists := templateData["substitutions"] if !exists { - t.Fatal("Expected substitution in templateData") + t.Fatal("Expected substitutions in templateData") } var subValues map[string]any - if err := yaml.Unmarshal(substitution, &subValues); err != nil { - t.Fatalf("Failed to unmarshal substitution: %v", err) + if err := yaml.Unmarshal(substitutions, &subValues); err != nil { + t.Fatalf("Failed to unmarshal substitutions: %v", err) } if subValues["domain"] != "example.com" { diff --git a/pkg/config/schema_validator.go b/pkg/config/schema_validator.go index b9d232cdc..291a4630a 100644 --- a/pkg/config/schema_validator.go +++ b/pkg/config/schema_validator.go @@ -69,6 +69,8 @@ func (sv *SchemaValidator) LoadSchemaFromBytes(schemaContent []byte) error { return fmt.Errorf("invalid schema structure: %w", err) } + sv.injectSubstitutionSchema(&schema) + sv.Schema = schema return nil } @@ -397,6 +399,36 @@ func (sv *SchemaValidator) valuesEqual(a, b any) bool { return fmt.Sprintf("%v", a) == fmt.Sprintf("%v", b) } +// injectSubstitutionSchema injects the substitutions property schema into the provided schema. +// The substitutions property is always allowed regardless of the schema's additionalProperties setting. +// This enables users to define kustomization substitutions in values.yaml without schema conflicts. +func (sv *SchemaValidator) injectSubstitutionSchema(schema *map[string]any) { + if schema == nil { + return + } + + properties, ok := (*schema)["properties"] + if !ok { + properties = make(map[string]any) + (*schema)["properties"] = properties + } + + propertiesMap, ok := properties.(map[string]any) + if !ok { + return + } + + propertiesMap["substitutions"] = map[string]any{ + "type": "object", + "additionalProperties": map[string]any{ + "type": "object", + "additionalProperties": map[string]any{ + "type": "string", + }, + }, + } +} + // validateSchemaStructure checks that the provided schema map conforms to Windsor or JSON Schema requirements. // It verifies the presence of the '$schema' field and ensures the schema version is supported. // Returns an error if the schema is missing required fields or uses an unsupported version.