Construct Template Enhancements #993

DavidSeptimus-Klotho · 2024-08-15T05:18:02Z

This PR make substantial enhancements to construct templates

Features

Interpolation enhancements

You can mix and match go template {{}} and existing ${} interpolation syntax in construct/binding templates.
Additionally, referencing properties containing a . is now possible using [part1.part2] syntax.

prop1: '{{ toUpper .Inputs.Input1 }}'
prop2: '${inputs:Input1}-{{ .Inputs.Input1 }}-${resources:[{{ .Inputs.Name }}]}'
prop3: '{{ if .Inputs.ReadOnly }}readonly{{ else }}readwrite{{ end }}'

Input rule enhancements

You can now write for_each-do oriented rules (mixing for_each and if in a single rule will cause an error). The do component of the rule will be executed once for each index or entry in the item selected in the for_each go template using the .Select function.
Resources added by input rules include a customizable prefix.
In a for_each, rule, use the .Select function to select a value from the current interpolation context to iterate over and reference the current selection with .Selected.
You can referenced the following within .Selected where applicable:
- .Key
- .Index
- .Value

input_rules:
  - for_each: '{{ .Select "inputs.Routes" }}'
    prefix: '{{ toLower (replace `[^\w]` "-"  .Selected.Value.Path) }}'
    do:
      resources:
        ...

Input rules now contain a rules field that enables nested rules. Nested rules inherit the prefix of their parent by default. Prefix hierarchies are merged with a . separator (e.g., MyRoute.Method)

input_rules:
  - if: {{ .Inputs.SomeBool }}
    then:
      resources:
        Resource1:
        ...
      rules:
        - for_each: {{ .Select "inputs.DependentInput" }}
          do: ...

Structured Inputs

Structured inputs are now supported using the same syntax as resource template properties. Notably, model inputs are not available and resource refs (e.g. resource:Id#property are not supported)

No validation enhancements were made to this functionality -- that will be a separate effort, so most validation declared in templates is still ignored.

Inputs:
  HeathCheck:
    type: map
    properties:
      Protocol:
        type: string
        required: true
        allowed_values:
          - TCP
          - UDP
          - HTTPS
          - HTTP
      Port:
        type: int
        default_value: 80
      Path:
        type: string
        default_value: /health

gordon-klotho · 2024-08-15T16:39:32Z

pkg/construct/properties.go

+	if value, ok := path.Get(); ok {
+		return value, nil
+	}
+	// Backwards compatibility: if the property does not exist, return nil instead of an error.


Was this causing problems? I don't mind breaking and internal API's backwards compatibility. If it does break the functional tests, then can you throw up a ticket for this cleanup?

IIRC, I did this, so I wouldn't have to introduce error handling in places that were relying on the previous behavior.

gordon-klotho · 2024-08-15T16:44:18Z

pkg/construct/properties.go

+		v, _ := path.Get()
+		otherV, _ := otherPath.Get()


Shouldn't this check the error to see if they match? Eg, if both are non-existent

pkg/construct/properties.go

pkg/construct/properties_test.go

pkg/k2/constructs/import_resources.go

pkg/k2/constructs/interpolation.go

pkg/k2/constructs/template/construct_template_test.go

gordon-klotho · 2024-08-15T19:49:47Z

pkg/k2/constructs/template/execution/context.go

+		Unmarshal(data *bytes.Buffer, v any) error
+	}
+
+	DefaultExecutionContext struct{}


Generally, interfaces are defined where they're used, not implemented. I also don't see any other implementations (maybe I'm missing it), so probably not necessary to preemptively make an interface for it.

It's used in the properties package, which contains all the property implementations. I'm open to moving it if you feel strongly about it.

I don't feel strongly, but the general practice of putting interfaces at the usage site makes them be way less bloated over time. It's a little weird compare to other languages like Java / TS, but it also makes it much easier to test since the surface area is smaller (the interface is what's used, not the union of all the uses).

gordon-klotho · 2024-08-15T19:50:22Z

pkg/k2/constructs/template/execution/context.go

+
+	return err
+
+}


Publishing WIP review. This comment is a bookmark so I can return to it.

gordon-klotho · 2024-08-16T18:19:21Z

pkg/k2/constructs/template/inputs/properties_template.go

+}
+
+// fieldConversion is a map providing functionality on how to convert inputs into our internal types if they are not inherently the same structure
+var fieldConversion = map[string]func(val reflect.Value, p *InputTemplate, kp property.Property) error{


nit: for readability, would be nice to type alias the func

This bit is copied verbatim from the knowledgebase properties implementation, but I can take a closer look at the points you raised in this comment and related subsequent comments.

gordon-klotho · 2024-08-16T18:21:22Z

pkg/k2/constructs/template/inputs/properties_template.go

+		// generate random uuid as the name of the template
+		name := uuid.New().String()


Why use a uuid and not the property's name? Since we don't have delegate templates, the only place this is relevant is in making the error messages.

gordon-klotho · 2024-08-16T18:38:28Z

pkg/k2/constructs/template/inputs/properties_template_test.go

+	"github.com/stretchr/testify/assert"
+)
+
+func Test_ConvertProperty(t *testing.T) {


non-blocking: would be nice to have more test cases

gordon-klotho · 2024-08-16T18:39:21Z

pkg/k2/constructs/template/properties.go

+type Properties struct {
+	propertyMap property.PropertyMap


Is there a need for a dedicated type for this?

PropertyMap is largely functionality extracted from ResourceTemplate and applied more generically to a map instead of to a template's Properties field. Its primary use is for working with nested properties. Interestingly, it looks like it also inherits a lack of support for seg1[seg2.seg3] path syntax in its GetProperty() method.

gordon-klotho · 2024-08-16T18:42:48Z

pkg/k2/constructs/template/property/construct_type.go

+
+func (c *ConstructType) UnmarshalYAML(value *yaml.Node) error {
+	// Split the value into parts
+	parts := strings.Split(value.Value, ".")


is value.Value safe to assume being the string? How does it behave if using a multiline string or other cases that have characters that wouldn't be present when unmarshalling to a string

nit: reduce the 3x copy of the parsing to one and reuse

I don't understand what you mean by the nit. For the rest, I can decode the value to a string and validate it with a regex.

Oh, the nit is that UnmarshalYAML, ParseConstrucType and FromString all do the same thing. Decoding to string is the most safe, since I know it'd behave well in those edge cases.

pkg/k2/constructs/template/templates/aws/bucket/bucket.yaml

DavidSeptimus-Klotho force-pushed the enhanced-inputs branch 3 times, most recently from 431cdaf to 37a28b8 Compare August 15, 2024 17:10

gordon-klotho reviewed Aug 15, 2024

View reviewed changes

gordon-klotho reviewed Aug 16, 2024

View reviewed changes

gordon-klotho approved these changes Aug 21, 2024

View reviewed changes

Adds support for structured inputs and for-each rules

896c5fe

DavidSeptimus-Klotho force-pushed the enhanced-inputs branch from 79d3ab6 to 896c5fe Compare August 21, 2024 22:09

DavidSeptimus-Klotho merged commit 58d73f6 into main Aug 21, 2024
5 checks passed

DavidSeptimus-Klotho deleted the enhanced-inputs branch August 21, 2024 22:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Construct Template Enhancements #993

Construct Template Enhancements #993

DavidSeptimus-Klotho commented Aug 15, 2024

gordon-klotho Aug 15, 2024

DavidSeptimus-Klotho Aug 16, 2024

gordon-klotho Aug 15, 2024

gordon-klotho Aug 15, 2024

DavidSeptimus-Klotho Aug 16, 2024

gordon-klotho Aug 16, 2024

gordon-klotho Aug 15, 2024

gordon-klotho Aug 16, 2024

DavidSeptimus-Klotho Aug 16, 2024

gordon-klotho Aug 16, 2024

gordon-klotho Aug 16, 2024

gordon-klotho Aug 16, 2024

DavidSeptimus-Klotho Aug 16, 2024

gordon-klotho Aug 16, 2024

DavidSeptimus-Klotho Aug 16, 2024

gordon-klotho Aug 16, 2024

		// generate random uuid as the name of the template
		name := uuid.New().String()

Construct Template Enhancements #993

Construct Template Enhancements #993

Conversation

DavidSeptimus-Klotho commented Aug 15, 2024

Features

Interpolation enhancements

Input rule enhancements

Structured Inputs

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment