Repository: klothoplatform/klotho
Pull request #827 (2023-12-14 22:24:21)

Sanitize environment variable names when added via consumer logic
Note: we should probably also sanitize for other automatically generated things (ie, that don't come from user constraints), but that's left for later.


Patch:
diff --git a/pkg/knowledge_base2/emitter.go b/pkg/knowledge_base2/emitter.go
index 6b9acfa5..21163300 100644
--- a/pkg/knowledge_base2/emitter.go
+++ b/pkg/knowledge_base2/emitter.go
@@ -31,23 +31,43 @@ type (
 	}
 )
 
-func ConsumeFromResource(consumer, emitter *construct.Resource, ctx DynamicValueContext) ([]DelayedConsumption, error) {
-	consumerTemplate, err := ctx.KnowledgeBase.GetResourceTemplate(consumer.ID)
+func sanitizeForConsumption(ctx DynamicContext, resource *construct.Resource, propTmpl Property, val any) (any, error) {
+	err := propTmpl.Validate(resource, val, ctx)
+	var sanErr *SanitizeError
+	if errors.As(err, &sanErr) {
+		val = sanErr.Sanitized
+	} else if err != nil {
+		return val, err
+	}
+	return val, nil
+}
+
+func ConsumeFromResource(consumer, emitter *construct.Resource, ctx DynamicContext) ([]DelayedConsumption, error) {
+	consumerTemplate, err := ctx.KB().GetResourceTemplate(consumer.ID)
 	if err != nil {
 		return nil, err
 	}
-	emitterTemplate, err := ctx.KnowledgeBase.GetResourceTemplate(emitter.ID)
+	emitterTemplate, err := ctx.KB().GetResourceTemplate(emitter.ID)
 	if err != nil {
 		return nil, err
 	}
 	var errs error
+	addErr := func(consume ConsumptionObject, emit ConsumptionObject, err error) {
+		if err == nil {
+			return
+		}
+		errs = errors.Join(errs, fmt.Errorf(
+			"error consuming %s from emitter %s: %w",
+			consume.PropertyPath, emit.PropertyPath, err,
+		))
+	}
 	delays := []DelayedConsumption{}
 	for _, consume := range consumerTemplate.Consumption.Consumed {
 		for _, emit := range emitterTemplate.Consumption.Emitted {
 			if consume.Model == emit.Model {
 				val, err := emit.Emit(ctx, emitter.ID)
 				if err != nil {
-					errs = errors.Join(errs, err)
+					addErr(consume, emit, err)
 					continue
 				}
 				id := consumer.ID
@@ -55,21 +75,36 @@ func ConsumeFromResource(consumer, emitter *construct.Resource, ctx DynamicValue
 					data := DynamicValueData{Resource: consumer.ID}
 					err = ctx.ExecuteDecode(consume.Resource, data, &id)
 					if err != nil {
-						errs = errors.Join(errs, err)
+						addErr(consume, emit, err)
 						continue
 					}
 				}
-				resource, err := ctx.Graph.Vertex(id)
+				consumeTmpl, err := ctx.KB().GetResourceTemplate(id)
 				if err != nil {
-					errs = errors.Join(errs, err)
+					addErr(consume, emit, err)
+					continue
+				}
+
+				resource, err := ctx.DAG().Vertex(id)
+				if err != nil {
+					addErr(consume, emit, err)
+					continue
+				}
+				val, err = sanitizeForConsumption(ctx, resource, consumeTmpl.GetProperty(consume.PropertyPath), val)
+				if err != nil {
+					addErr(consume, emit, err)
+					continue
+				}
+				pval, err := resource.GetProperty(consume.PropertyPath)
+				if err != nil {
+					addErr(consume, emit, err)
 					continue
 				}
-				pval, _ := resource.GetProperty(consume.PropertyPath)
 				if pval == nil {
 					if consume.Converter != "" {
 						val, err = consume.Convert(val, id, ctx)
 						if err != nil {
-							errs = errors.Join(errs, err)
+							addErr(consume, emit, err)
 							continue
 						}
 					}
@@ -83,7 +118,7 @@ func ConsumeFromResource(consumer, emitter *construct.Resource, ctx DynamicValue
 
 				err = consume.Consume(val, ctx, resource)
 				if err != nil {
-					errs = errors.Join(errs, err)
+					addErr(consume, emit, err)
 					continue
 				}
 			}
@@ -94,12 +129,12 @@ func ConsumeFromResource(consumer, emitter *construct.Resource, ctx DynamicValue
 
 // HasConsumedFromResource returns true if the consumer has consumed from the emitter
 // In order to return true, only one of the emitted values has to be set correctly
-func HasConsumedFromResource(consumer, emitter *construct.Resource, ctx DynamicValueContext) (bool, error) {
-	consumerTemplate, err := ctx.KnowledgeBase.GetResourceTemplate(consumer.ID)
+func HasConsumedFromResource(consumer, emitter *construct.Resource, ctx DynamicContext) (bool, error) {
+	consumerTemplate, err := ctx.KB().GetResourceTemplate(consumer.ID)
 	if err != nil {
 		return false, err
 	}
-	emitterTemplate, err := ctx.KnowledgeBase.GetResourceTemplate(emitter.ID)
+	emitterTemplate, err := ctx.KB().GetResourceTemplate(emitter.ID)
 	if err != nil {
 		return false, err
 	}
@@ -124,7 +159,7 @@ func HasConsumedFromResource(consumer, emitter *construct.Resource, ctx DynamicV
 						continue
 					}
 				}
-				resource, err := ctx.Graph.Vertex(id)
+				resource, err := ctx.DAG().Vertex(id)
 				if err != nil {
 					errs = errors.Join(errs, err)
 					continue
@@ -140,7 +175,7 @@ func HasConsumedFromResource(consumer, emitter *construct.Resource, ctx DynamicV
 						continue
 					}
 				}
-				rt, err := ctx.KnowledgeBase.GetResourceTemplate(resource.ID)
+				rt, err := ctx.KB().GetResourceTemplate(resource.ID)
 				if err != nil {
 					errs = errors.Join(errs, err)
 					continue
@@ -159,7 +194,7 @@ func HasConsumedFromResource(consumer, emitter *construct.Resource, ctx DynamicV
 	return noEmittedMatches, nil
 }
 
-func (c *ConsumptionObject) Convert(value any, res construct.ResourceId, ctx DynamicValueContext) (any, error) {
+func (c *ConsumptionObject) Convert(value any, res construct.ResourceId, ctx DynamicContext) (any, error) {
 	if c.Converter == "" {
 		return value, fmt.Errorf("no converter specified")
 	}
@@ -197,7 +232,7 @@ func (c *ConsumptionObject) Convert(value any, res construct.ResourceId, ctx Dyn
 	return val, nil
 }
 
-func (c *ConsumptionObject) Emit(ctx DynamicValueContext, resource construct.ResourceId) (any, error) {
+func (c *ConsumptionObject) Emit(ctx DynamicContext, resource construct.ResourceId) (any, error) {
 	if c.Value == "" {
 		return nil, fmt.Errorf("no value specified")
 	}
@@ -211,7 +246,7 @@ func (c *ConsumptionObject) Emit(ctx DynamicValueContext, resource construct.Res
 			return nil, err
 		}
 	}
-	model := ctx.KnowledgeBase.GetModel(c.Model)
+	model := ctx.KB().GetModel(c.Model)
 	data := DynamicValueData{Resource: resource}
 	val, err := model.GetObjectValue(c.Value, ctx, data)
 	if err != nil {
@@ -223,11 +258,11 @@ func (c *ConsumptionObject) Emit(ctx DynamicValueContext, resource construct.Res
 	return val, nil
 }
 
-func (c *ConsumptionObject) Consume(val any, ctx DynamicValueContext, resource *construct.Resource) error {
-	rt, err := ctx.KnowledgeBase.GetResourceTemplate(resource.ID)
+func (c *ConsumptionObject) Consume(val any, ctx DynamicContext, resource *construct.Resource) error {
+	rt, err := ctx.KB().GetResourceTemplate(resource.ID)
 	if err != nil {
 		return err
 	}
-	prop := rt.GetProperty(c.PropertyPath)
-	return prop.AppendProperty(resource, val)
+	propTmpl := rt.GetProperty(c.PropertyPath)
+	return propTmpl.AppendProperty(resource, val)
 }
diff --git a/pkg/knowledge_base2/model.go b/pkg/knowledge_base2/model.go
index cc550496..4a985051 100644
--- a/pkg/knowledge_base2/model.go
+++ b/pkg/knowledge_base2/model.go
@@ -13,7 +13,7 @@ type (
 )
 
 // GetObjectValue returns the value of the object as the model type
-func (m *Model) GetObjectValue(val any, ctx DynamicValueContext, data DynamicValueData) (any, error) {
+func (m *Model) GetObjectValue(val any, ctx DynamicContext, data DynamicValueData) (any, error) {
 
 	GetVal := func(p Property, val map[string]any) (any, error) {
 		propVal, found := val[p.Details().Name]
diff --git a/pkg/knowledge_base2/properties/list_property.go b/pkg/knowledge_base2/properties/list_property.go
index 31da2ba1..40c045c1 100644
--- a/pkg/knowledge_base2/properties/list_property.go
+++ b/pkg/knowledge_base2/properties/list_property.go
@@ -189,19 +189,34 @@ func (l *ListProperty) Validate(resource *construct.Resource, value any, ctx kno
 			return fmt.Errorf("list value %v is too long. max length is %d", value, *l.MaxLength)
 		}
 	}
-	var errs error
 	// Only validate values if its a primitive list, otherwise let the sub properties handle their own validation
 	if l.ItemProperty != nil {
-		for _, v := range listVal {
+		var errs error
+		hasSanitized := false
+		validList := make([]any, len(listVal))
+		for i, v := range listVal {
 			err := l.ItemProperty.Validate(resource, v, ctx)
 			if err != nil {
-				errs = errors.New(errs.Error() + "\n" + err.Error())
+				var sanitizeErr *knowledgebase.SanitizeError
+				if errors.As(err, &sanitizeErr) {
+					validList[i] = sanitizeErr.Sanitized
+					hasSanitized = true
+				} else {
+					errs = errors.Join(errs, err)
+				}
+			} else {
+				validList[i] = v
+			}
+		}
+		if errs != nil {
+			return errs
+		}
+		if hasSanitized {
+			return &knowledgebase.SanitizeError{
+				Input:     listVal,
+				Sanitized: validList,
 			}
 		}
-
-	}
-	if errs != nil {
-		return errs
 	}
 	return nil
 }
diff --git a/pkg/knowledge_base2/properties/map_property.go b/pkg/knowledge_base2/properties/map_property.go
index ee8a9b42..86998d61 100644
--- a/pkg/knowledge_base2/properties/map_property.go
+++ b/pkg/knowledge_base2/properties/map_property.go
@@ -198,20 +198,39 @@ func (m *MapProperty) Validate(resource *construct.Resource, value any, ctx know
 		}
 	}
 	var errs error
+	hasSanitized := false
+	validMap := make(map[string]any)
 	// Only validate values if its a primitive map, otherwise let the sub properties handle their own validation
-	if m.KeyProperty != nil && m.ValueProperty != nil {
-		for k, v := range mapVal {
-			if err := m.KeyProperty.Validate(resource, k, ctx); err != nil {
+	for k, v := range mapVal {
+		if m.KeyProperty != nil {
+			var sanitizeErr *knowledgebase.SanitizeError
+			if err := m.KeyProperty.Validate(resource, k, ctx); errors.As(err, &sanitizeErr) {
+				k = sanitizeErr.Sanitized.(string)
+				hasSanitized = true
+			} else if err != nil {
 				errs = errors.Join(errs, fmt.Errorf("invalid key %v for map property type %s: %w", k, m.KeyProperty.Type(), err))
 			}
-			if err := m.ValueProperty.Validate(resource, v, ctx); err != nil {
+		}
+		if m.ValueProperty != nil {
+			var sanitizeErr *knowledgebase.SanitizeError
+			if err := m.ValueProperty.Validate(resource, v, ctx); errors.As(err, &sanitizeErr) {
+				v = sanitizeErr.Sanitized
+				hasSanitized = true
+			} else if err != nil {
 				errs = errors.Join(errs, fmt.Errorf("invalid value %v for map property type %s: %w", v, m.ValueProperty.Type(), err))
 			}
 		}
+		validMap[k] = v
 	}
 	if errs != nil {
 		return errs
 	}
+	if hasSanitized {
+		return &knowledgebase.SanitizeError{
+			Input:     mapVal,
+			Sanitized: validMap,
+		}
+	}
 	return nil
 }
 
diff --git a/pkg/knowledge_base2/properties/set_property.go b/pkg/knowledge_base2/properties/set_property.go
index c0abd232..20d14890 100644
--- a/pkg/knowledge_base2/properties/set_property.go
+++ b/pkg/knowledge_base2/properties/set_property.go
@@ -181,14 +181,30 @@ func (s *SetProperty) Validate(resource *construct.Resource, value any, ctx know
 	// Only validate values if its a primitive list, otherwise let the sub properties handle their own validation
 	if s.ItemProperty != nil {
 		var errs error
+		hasSanitized := false
+		validSet := set.HashedSet[string, any]{Hasher: setVal.Hasher}
 		for _, item := range setVal.ToSlice() {
 			if err := s.ItemProperty.Validate(resource, item, ctx); err != nil {
-				errs = errors.Join(errs, fmt.Errorf("invalid item %v: %v", item, err))
+				var sanitizeErr *knowledgebase.SanitizeError
+				if errors.As(err, &sanitizeErr) {
+					validSet.Add(sanitizeErr.Sanitized)
+					hasSanitized = true
+				} else {
+					errs = errors.Join(errs, fmt.Errorf("invalid item %v: %v", item, err))
+				}
+			} else {
+				validSet.Add(item)
 			}
 		}
 		if errs != nil {
 			return errs
 		}
+		if hasSanitized {
+			return &knowledgebase.SanitizeError{
+				Input:     setVal,
+				Sanitized: validSet,
+			}
+		}
 	}
 	return nil
 }
diff --git a/pkg/knowledge_base2/properties/string_property.go b/pkg/knowledge_base2/properties/string_property.go
index e58980d4..1760cd91 100644
--- a/pkg/knowledge_base2/properties/string_property.go
+++ b/pkg/knowledge_base2/properties/string_property.go
@@ -123,14 +123,7 @@ func (s *StringProperty) Validate(resource *construct.Resource, value any, ctx k
 	}
 
 	if s.SanitizeTmpl != nil {
-		oldVal := stringVal
-		stringVal, err := s.SanitizeTmpl.Execute(stringVal)
-		if err != nil {
-			return err
-		}
-		if oldVal != stringVal {
-			return fmt.Errorf("value %s did not pass sanitization rules. suggested value: %s", oldVal, stringVal)
-		}
+		return s.SanitizeTmpl.Check(stringVal)
 	}
 	return nil
 }
diff --git a/pkg/knowledge_base2/reader/properties.go b/pkg/knowledge_base2/reader/properties.go
index f13e8ceb..386d946e 100644
--- a/pkg/knowledge_base2/reader/properties.go
+++ b/pkg/knowledge_base2/reader/properties.go
@@ -48,10 +48,10 @@ type (
 		SanitizeTmpl  string   `yaml:"sanitize"`
 		AllowedValues []string `yaml:"allowed_values"`
 
-		KeyProperty   knowledgebase.Property `yaml:"key_property"`
-		ValueProperty knowledgebase.Property `yaml:"value_property"`
+		KeyProperty   *Property `yaml:"key_property"`
+		ValueProperty *Property `yaml:"value_property"`
 
-		ItemProperty knowledgebase.Property `yaml:"item_property"`
+		ItemProperty *Property `yaml:"item_property"`
 
 		Path string `json:"-" yaml:"-"`
 	}
@@ -93,6 +93,7 @@ func (p *Property) Convert() (knowledgebase.Property, error) {
 	if err != nil {
 		return nil, err
 	}
+	propertyType.Details().Path = p.Path
 
 	srcVal := reflect.ValueOf(p).Elem()
 	dstVal := reflect.ValueOf(propertyType).Elem()
@@ -111,11 +112,9 @@ func (p *Property) Convert() (knowledgebase.Property, error) {
 			continue
 		}
 		// Handle sub properties so we can recurse down the tree
-		if fieldName == "Properties" {
-			properties, ok := srcField.Interface().(Properties)
-			if !ok {
-				return nil, fmt.Errorf("invalid properties")
-			}
+		switch fieldName {
+		case "Properties":
+			properties := srcField.Interface().(Properties)
 			var errs error
 			props := knowledgebase.Properties{}
 			for name, prop := range properties {
@@ -127,13 +126,67 @@ func (p *Property) Convert() (knowledgebase.Property, error) {
 				props[name] = propertyType
 			}
 			if errs != nil {
-				return nil, errs
+				return nil, fmt.Errorf("could not convert sub properties: %w", errs)
 			}
 			dstField.Set(reflect.ValueOf(props))
 			continue
+
+		case "KeyProperty", "ValueProperty":
+			if !strings.HasPrefix(p.Type, "map") {
+				return nil, fmt.Errorf("property must be 'map' (was %s) for %s", p.Type, fieldName)
+			}
+			keyType, valueType, hasElementTypes := strings.Cut(
+				strings.TrimSuffix(strings.TrimPrefix(p.Type, "map("), ")"),
+				",",
+			)
+			elemProp := srcField.Interface().(*Property)
+			// Add the element's type if it is not specified but is on the parent.
+			// For example, 'map(string,string)' on the parent means the key_property doesn't need 'type: string'
+			if hasElementTypes {
+				if fieldName == "KeyProperty" {
+					if elemProp.Type != "" && elemProp.Type != keyType {
+						return nil, fmt.Errorf("key property type must be %s (was %s)", keyType, elemProp.Type)
+					} else if elemProp.Type == "" {
+						elemProp.Type = keyType
+					}
+				} else {
+					if elemProp.Type != "" && elemProp.Type != valueType {
+						return nil, fmt.Errorf("value property type must be %s (was %s)", valueType, elemProp.Type)
+					} else if elemProp.Type == "" {
+						elemProp.Type = valueType
+					}
+				}
+			}
+			converted, err := elemProp.Convert()
+			if err != nil {
+				return nil, fmt.Errorf("could not convert %s: %w", fieldName, err)
+			}
+			srcField = reflect.ValueOf(converted)
+		case "ItemProperty":
+			if !strings.HasPrefix(p.Type, "list") && !strings.HasPrefix(p.Type, "set") {
+				return nil, fmt.Errorf("property must be 'list' or 'set' (was %s) for %s", p.Type, fieldName)
+			}
+			hasItemType := strings.Contains(p.Type, "(")
+			elemProp := srcField.Interface().(*Property)
+			if hasItemType {
+				itemType := strings.TrimSuffix(
+					strings.TrimPrefix(strings.TrimPrefix(p.Type, "list("), "set("),
+					")",
+				)
+				if elemProp.Type != "" && elemProp.Type != itemType {
+					return nil, fmt.Errorf("item property type must be %s (was %s)", itemType, elemProp.Type)
+				} else if elemProp.Type == "" {
+					elemProp.Type = itemType
+				}
+			}
+			converted, err := elemProp.Convert()
+			if err != nil {
+				return nil, fmt.Errorf("could not convert %s: %w", fieldName, err)
+			}
+			srcField = reflect.ValueOf(converted)
 		}
 
-		if dstField.Type().AssignableTo(srcField.Type()) {
+		if srcField.Type().AssignableTo(dstField.Type()) {
 			dstField.Set(srcField)
 			continue
 		}
@@ -159,12 +212,13 @@ func (p *Property) Convert() (knowledgebase.Property, error) {
 			continue
 		}
 
-		return nil, fmt.Errorf("invalid field name %s, for property %s of type %s", fieldName, p.Name, p.Type)
+		return nil, fmt.Errorf(
+			"could not assign %s#%s (%s) to field in %T (%s)",
+			p.Path, fieldName, srcField.Type(), propertyType, dstField.Type(),
+		)
 
 	}
 
-	details := propertyType.Details()
-	details.Path = p.Path
 	return propertyType, nil
 }
 
diff --git a/pkg/knowledge_base2/resource_template.go b/pkg/knowledge_base2/resource_template.go
index e4aaa5b5..75948939 100644
--- a/pkg/knowledge_base2/resource_template.go
+++ b/pkg/knowledge_base2/resource_template.go
@@ -79,15 +79,18 @@ type (
 		Clone() Property
 		// Type returns the string representation of the type of the property, as it should appear in the resource template
 		Type() string
-		// GetDefaultValue returns the default value for the property, pertaining to the specific data being passed in for execution
+		// GetDefaultValue returns the default value for the property,
+		// pertaining to the specific data being passed in for execution
 		GetDefaultValue(ctx DynamicContext, data DynamicValueData) (any, error)
-		// Validate ensures the value is valid for the property and returns an error if it is not
+		// Validate ensures the value is valid for the property to `Set` (not `Append` for collection types)
+		// and returns an error if it is not
 		Validate(resource *construct.Resource, value any, ctx DynamicContext) error
-		// SubProperties returns the sub properties of the property, if any. This is used for properties that are complex structures,
-		// such as lists, sets, or maps
+		// SubProperties returns the sub properties of the property, if any.
+		// This is used for properties that are complex structures, such as lists, sets, or maps
 		SubProperties() Properties
-		// Parse parses a given value to ensure it is the correct type for the property. If the given value cannot be converted
-		// to the respective property type an error is returned. The returned value will always be the correct type for the property
+		// Parse parses a given value to ensure it is the correct type for the property.
+		// If the given value cannot be converted to the respective property type an error is returned.
+		// The returned value will always be the correct type for the property
 		Parse(value any, ctx DynamicContext, data DynamicValueData) (any, error)
 		// ZeroValue returns the zero value for the property type
 		ZeroValue() any
diff --git a/pkg/knowledge_base2/sanitization.go b/pkg/knowledge_base2/sanitization.go
index 40881aa3..0332859a 100644
--- a/pkg/knowledge_base2/sanitization.go
+++ b/pkg/knowledge_base2/sanitization.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"regexp"
 	"strings"
+	"sync"
 	"text/template"
 )
 
@@ -13,6 +14,13 @@ type (
 	SanitizeTmpl struct {
 		template *template.Template
 	}
+
+	// SanitizeError is returned when a value is sanitized if the input is not valid. The Sanitized field
+	// is always the same type as the Input field.
+	SanitizeError struct {
+		Input     any
+		Sanitized any
+	}
 )
 
 func NewSanitizationTmpl(name string, tmpl string) (*SanitizeTmpl, error) {
@@ -49,11 +57,38 @@ func NewSanitizationTmpl(name string, tmpl string) (*SanitizeTmpl, error) {
 	}, err
 }
 
-func (t SanitizeTmpl) Execute(name string) (string, error) {
-	buf := new(bytes.Buffer)
-	err := t.template.Execute(buf, name)
+var sanitizeBufs = sync.Pool{
+	New: func() interface{} {
+		return new(bytes.Buffer)
+	},
+}
+
+func (t SanitizeTmpl) Execute(value string) (string, error) {
+	buf := sanitizeBufs.Get().(*bytes.Buffer)
+	defer sanitizeBufs.Put(buf)
+	buf.Reset()
+
+	err := t.template.Execute(buf, value)
 	if err != nil {
-		return name, fmt.Errorf("could not execute sanitize name template on %q: %w", name, err)
+		return value, fmt.Errorf("could not execute sanitize name template on %q: %w", value, err)
 	}
 	return strings.TrimSpace(buf.String()), nil
 }
+
+func (t SanitizeTmpl) Check(value string) error {
+	sanitized, err := t.Execute(value)
+	if err != nil {
+		return err
+	}
+	if sanitized != value {
+		return &SanitizeError{
+			Input:     value,
+			Sanitized: sanitized,
+		}
+	}
+	return nil
+}
+
+func (err SanitizeError) Error() string {
+	return fmt.Sprintf("invalid value %q, suggested value: %q", err.Input, err.Sanitized)
+}
diff --git a/pkg/templates/aws/resources/app_runer_service.yaml b/pkg/templates/aws/resources/app_runer_service.yaml
index 2b19e999..d6953a2a 100644
--- a/pkg/templates/aws/resources/app_runer_service.yaml
+++ b/pkg/templates/aws/resources/app_runer_service.yaml
@@ -35,6 +35,13 @@ properties:
     type: map(string,string)
     description: Key-value pairs of environment variables that are provided to the
       App Runner service
+    key_property:
+      sanitize: |
+        {{ .
+          | replace `[^[:alnum:]_]+` "_"
+          | replace `^[^a-zA-Z]+` ""
+          | upper
+        }}
   Port:
     type: int
     default_value: 8080
diff --git a/pkg/templates/aws/resources/ecs_task_definition.yaml b/pkg/templates/aws/resources/ecs_task_definition.yaml
index 9d47612c..b3316c7e 100644
--- a/pkg/templates/aws/resources/ecs_task_definition.yaml
+++ b/pkg/templates/aws/resources/ecs_task_definition.yaml
@@ -28,6 +28,13 @@ properties:
   EnvironmentVariables:
     type: map(string,string)
     description: A map of the environment variables to pass to the container
+    key_property:
+      sanitize: |
+        {{ .
+          | replace `[^[:alnum:]_]+` "_"
+          | replace `^[^a-zA-Z]+` ""
+          | upper
+        }}
   Cpu:
     type: string
     default_value: '256'
diff --git a/pkg/templates/aws/resources/lambda_function.yaml b/pkg/templates/aws/resources/lambda_function.yaml
index 2e788242..329da459 100644
--- a/pkg/templates/aws/resources/lambda_function.yaml
+++ b/pkg/templates/aws/resources/lambda_function.yaml
@@ -20,6 +20,13 @@ properties:
         unique: true
   EnvironmentVariables:
     type: map(string,string)
+    key_property:
+      sanitize: |
+        {{ .
+          | replace `[^[:alnum:]_]+` "_"
+          | replace `^[^a-zA-Z]+` ""
+          | upper
+        }}
   SecurityGroups:
     type: list(resource(aws:security_group))
     operational_rule:
@@ -80,7 +87,7 @@ path_satisfaction:
     - permissions
   as_source:
     - network#Subnets
-    - network    # This is required as well in case the lambda does not already exist in a vpc, we need the path evaluation done in 2 step
+    - network # This is required as well in case the lambda does not already exist in a vpc, we need the path evaluation done in 2 step
 
 consumption:
   consumed:
diff --git a/pkg/templates/kubernetes/models/env_var.yaml b/pkg/templates/kubernetes/models/env_var.yaml
index 86227966..dfef949b 100644
--- a/pkg/templates/kubernetes/models/env_var.yaml
+++ b/pkg/templates/kubernetes/models/env_var.yaml
@@ -2,5 +2,11 @@ name: kubernetes:EnvVar
 properties:
   name:
     type: string
+    sanitize: |
+      {{ .
+        | replace `[^[:alnum:]_]+` "_"
+        | replace `^[^a-zA-Z]+` ""
+        | upper
+      }}
   value:
     type: string


Test patch:
diff --git a/pkg/engine2/testdata/ecs_rds.expect.yaml b/pkg/engine2/testdata/ecs_rds.expect.yaml
index ff4d63c2..7a20d6dc 100755
--- a/pkg/engine2/testdata/ecs_rds.expect.yaml
+++ b/pkg/engine2/testdata/ecs_rds.expect.yaml
@@ -30,10 +30,10 @@ resources:
     aws:ecs_task_definition:ecs_service_0:
         Cpu: "256"
         EnvironmentVariables:
-            rds-instance-2_RDS_CONNECTION_ARN: aws:rds_instance:rds-instance-2#RdsConnectionArn
-            rds-instance-2_RDS_ENDPOINT: aws:rds_instance:rds-instance-2#Endpoint
-            rds-instance-2_RDS_PASSWORD: aws:rds_instance:rds-instance-2#Password
-            rds-instance-2_RDS_USERNAME: aws:rds_instance:rds-instance-2#Username
+            RDS_INSTANCE_2_RDS_CONNECTION_ARN: aws:rds_instance:rds-instance-2#RdsConnectionArn
+            RDS_INSTANCE_2_RDS_ENDPOINT: aws:rds_instance:rds-instance-2#Endpoint
+            RDS_INSTANCE_2_RDS_PASSWORD: aws:rds_instance:rds-instance-2#Password
+            RDS_INSTANCE_2_RDS_USERNAME: aws:rds_instance:rds-instance-2#Username
         ExecutionRole: aws:iam_role:ecs_service_0-execution-role
         Image: aws:ecr_image:ecs_service_0-image
         LogGroup: aws:log_group:ecs_service_0-log-group
diff --git a/pkg/engine2/testdata/ecs_rds.input.yaml b/pkg/engine2/testdata/ecs_rds.input.yaml
index a0f78ecc..7deae529 100644
--- a/pkg/engine2/testdata/ecs_rds.input.yaml
+++ b/pkg/engine2/testdata/ecs_rds.input.yaml
@@ -2,11 +2,11 @@ constraints:
   - node: aws:ecs_service:ecs_service_0
     operator: add
     scope: application
-  - node: aws:rds_instance:rds_instance_2
+  - node: aws:rds_instance:rds_instance_2 # deliberately use underscores to run name sanitization
     operator: add
     scope: application
   - operator: must_exist
     scope: edge
     target:
       source: aws:ecs_service:ecs_service_0
-      target: aws:rds_instance:rds-instance-2
+      target: aws:rds_instance:rds_instance_2