Skip to content

Commit

Permalink
Generate singleton lists as embedded objects
Browse files Browse the repository at this point in the history
- Terraform configuration blocks, even if they have a MaxItems
  constraint of 1, are (almost) always generated as lists. We
  now generate the lists with a MaxItems constraint of 1 as
  embedded objects in our MR APIs.
- This also helps when updating or patching via SSA the
  (previously list) objects. The merging strategy implemented
  by SSA requires configuration for associative lists and
  converting the singleton lists into embedded objects removes
  the configuration need.
- A schema traverser is introduced, which can decouple the Terraform
  schema traversal logic from the actions (such as code generation,
  inspection, or singleton-list-to-embedded-object conversion)
  taken while traversing the schema.

Signed-off-by: Alper Rifat Ulucinar <[email protected]>
  • Loading branch information
ulucinar committed Apr 17, 2024
1 parent 4c67d8e commit d078959
Show file tree
Hide file tree
Showing 9 changed files with 388 additions and 39 deletions.
4 changes: 2 additions & 2 deletions pkg/config/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ func (r *Resource) MarkAsRequired(fieldpaths ...string) {
// Deprecated: Use Resource.MarkAsRequired instead.
// This function will be removed in future versions.
func MarkAsRequired(sch *schema.Resource, fieldpaths ...string) {
for _, fieldpath := range fieldpaths {
if s := GetSchema(sch, fieldpath); s != nil {
for _, fp := range fieldpaths {
if s := GetSchema(sch, fp); s != nil {
s.Computed = false
s.Optional = false
}
Expand Down
27 changes: 27 additions & 0 deletions pkg/config/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"fmt"
"regexp"

"github.com/crossplane/upjet/pkg/schema/traverser"

Check failure on line 12 in pkg/config/provider.go

View workflow job for this annotation

GitHub Actions / lint

File is not `gci`-ed with --skip-generated -s standard -s default -s prefix(github.com/crossplane/upjet) -s blank -s dot --custom-order (gci)

tfjson "github.com/hashicorp/terraform-json"
fwprovider "github.com/hashicorp/terraform-plugin-framework/provider"
fwresource "github.com/hashicorp/terraform-plugin-framework/resource"
Expand Down Expand Up @@ -351,6 +353,13 @@ func NewProvider(schema []byte, prefix string, modulePath string, metadata []byt
p.Resources[name] = DefaultResource(name, terraformResource, terraformPluginFrameworkResource, providerMetadata.Resources[name], p.DefaultResourceOptions...)
p.Resources[name].useTerraformPluginSDKClient = isTerraformPluginSDK
p.Resources[name].useTerraformPluginFrameworkClient = isPluginFrameworkResource
// traverse the Terraform resource schema to initialize the upjet Resource
// configuration using:
// - listEmbedder: This traverser marks lists of length at most 1
// (singleton lists) as embedded objects.
if err := traverser.Traverse(terraformResource, listEmbedder{r: p.Resources[name]}); err != nil {
panic(err)
}
}
for i, refInjector := range p.refInjectors {
if err := refInjector.InjectReferences(p.Resources); err != nil {
Expand Down Expand Up @@ -432,3 +441,21 @@ func terraformPluginFrameworkResourceFunctionsMap(provider fwprovider.Provider)

return resourceFunctionsMap
}

type listEmbedder struct {
traverser.NoopTraverser
r *Resource
}

func (l listEmbedder) VisitResource(r *traverser.ResourceNode) error {
// this visitor only works on sets and lists with the MaxItems constraint
// of 1.
if r.Schema.Type != schema.TypeList && r.Schema.Type != schema.TypeSet {
return nil
}
if r.Schema.MaxItems != 1 {
return nil
}
l.r.AddSingletonListConversion(traverser.FieldPathWithWildcard(r.TFPath))
return nil
}
41 changes: 41 additions & 0 deletions pkg/config/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package config
import (
"context"
"fmt"
"strings"
"time"

xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1"
Expand Down Expand Up @@ -435,6 +436,15 @@ type Resource struct {
// SchemaElementOption for configuring options for schema elements.
SchemaElementOptions SchemaElementOptions

// listConversionPaths is the Terraform field paths of embedded objects that
// need to be converted into singleton lists (lists of at most one element)
// at runtime.
// Such fields are lists in the Terraform schema, however upjet generates
// them as nested objects, and we need to convert them back to lists
// at runtime before passing them to the Terraform stack and lists into
// embedded objects after reading the state from the Terraform stack.
listConversionPaths []string

// TerraformConfigurationInjector allows a managed resource to inject
// configuration values in the Terraform configuration map obtained by
// deserializing its `spec.forProvider` value. Managed resources can
Expand Down Expand Up @@ -541,7 +551,38 @@ func (m SchemaElementOptions) AddToObservation(el string) bool {
return m[el] != nil && m[el].AddToObservation
}

// ListConversionPaths returns the Resource's runtime Terraform list
// conversion paths in fieldpath syntax.
func (r *Resource) ListConversionPaths() []string {
l := make([]string, 0, len(r.listConversionPaths))
for _, v := range r.listConversionPaths {
l = append(l, v)
}
return l
}

// AddSingletonListConversion configures the list at the specified CRD
// field path and the specified Terraform field path as an embedded object.
// crdPath is the field path expression for the CRD schema and tfPath is
// the field path expression for the Terraform schema corresponding to the
// singleton list to be converted to an embedded object.
// At runtime, upjet will convert such objects back and forth
// from/to singleton lists while communicating with the Terraform stack.
// The specified fieldpath expression must be a wildcard expression such as
// `conditions[*]` or a 0-indexed expression such as `conditions[0]`. Other
// index values are not allowed as this function deals with singleton lists.
func (r *Resource) AddSingletonListConversion(tfPath string) {
// SchemaElementOptions.SetEmbeddedObject does not expect the indices and
// because we are dealing with singleton lists here, we only expect wildcards
// or the zero-index.
nPath := strings.ReplaceAll(tfPath, "[*]", "")
nPath = strings.ReplaceAll(nPath, "[0]", "")
r.SchemaElementOptions.SetEmbeddedObject(nPath)
r.listConversionPaths = append(r.listConversionPaths, tfPath)
}

// SetEmbeddedObject sets the EmbeddedObject for the specified key.
// The key is a Terraform field path without the wildcard segments.
func (m SchemaElementOptions) SetEmbeddedObject(el string) {
if m[el] == nil {
m[el] = &SchemaElementOption{}
Expand Down
115 changes: 115 additions & 0 deletions pkg/controller/conversion.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
// SPDX-FileCopyrightText: 2023 The Crossplane Authors <https://crossplane.io>
//
// SPDX-License-Identifier: Apache-2.0

package controller

import (
"slices"
"sort"
"strings"

"github.com/crossplane/crossplane-runtime/pkg/fieldpath"
"github.com/pkg/errors"
)

type conversionMode int

const (
toEmbeddedObject conversionMode = iota
toSingletonList
)

// String returns a string representation of the conversion mode.
func (m conversionMode) String() string {
switch m {
case toSingletonList:
return "toSingletonList"
case toEmbeddedObject:
return "toEmbeddedObject"
default:
return "unknown"
}
}

// setValue sets the value, in pv, to v at the specified path fp.
// It's implemented on top of the fieldpath library by accessing
// the parent map in fp and directly setting v as a value in the
// parent map. We don't use fieldpath.Paved.SetValue because the
// JSON value validation performed by it potentially changes types.
func setValue(pv *fieldpath.Paved, v any, fp string) error {
segments := strings.Split(fp, ".")
p := fp
var pm any = pv.UnstructuredContent()
var err error
if len(segments) > 1 {
p = strings.Join(segments[:len(segments)-1], ".")
pm, err = pv.GetValue(p)
if err != nil {
return errors.Wrapf(err, "cannot get the parent value at field path %s", p)
}
}
parent, ok := pm.(map[string]any)
if !ok {
return errors.Errorf("parent at field path %s must be a map[string]any", p)
}
parent[segments[len(segments)-1]] = v
return nil
}

// convert performs conversion between singleton lists and embedded objects
// while passing the CRD parameters to the Terraform layer and while reading
// state from the Terraform layer at runtime. The paths where the conversion
// will be performed are specified using paths and the conversion mode (whether
// an embedded object will be converted into a singleton list or a singleton
// list will be converted into an embedded object) is determined by the mode
// parameter.
func convert(params map[string]any, paths []string, mode conversionMode) (map[string]any, error) {

Check failure on line 67 in pkg/controller/conversion.go

View workflow job for this annotation

GitHub Actions / lint

cyclomatic complexity 15 of func `convert` is high (> 10) (gocyclo)
switch mode {
case toSingletonList:
slices.Sort(paths)
case toEmbeddedObject:
sort.Slice(paths, func(i, j int) bool {
return paths[i] > paths[j]
})
}

pv := fieldpath.Pave(params)
for _, fp := range paths {
exp, err := pv.ExpandWildcards(fp)
if err != nil {
return nil, errors.Wrapf(err, "cannot expand wildcards for the field path expression %s", fp)
}
switch len(exp) {
case 0:
continue
case 1:
v, err := pv.GetValue(exp[0])
if err != nil {
return nil, errors.Wrapf(err, "cannot get the value at the field path %s with the conversion mode set to %q", exp[0], mode)
}
switch mode {
case toSingletonList:
if err := setValue(pv, []any{v}, exp[0]); err != nil {
return nil, errors.Wrapf(err, "cannot set the singleton list's value at the field path %s", exp[0])
}
case toEmbeddedObject:
s, ok := v.([]any)
if !ok || len(s) > 1 {
// if len(s) is 0, then it's not a slice
return nil, errors.Errorf("singleton list, at the field path %s, must have a length of 1 but it has a length of %d", exp[0], len(s))
}
var newVal any = map[string]any{}
if len(s) > 0 {
newVal = s[0]
}
if err := setValue(pv, newVal, exp[0]); err != nil {
return nil, errors.Wrapf(err, "cannot set the embedded object's value at the field path %s", exp[0])
}
}
default:
return nil, errors.Errorf("unexpected number of expansions (%d) for the wildcard field path expression %s", len(exp), fp)
}
}
return params, nil
}
14 changes: 13 additions & 1 deletion pkg/controller/external_tfpluginsdk.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ func getExtendedParameters(ctx context.Context, tr resource.Terraformed, externa
params["tags_all"] = params["tags"]
}
}
return params, nil
return convert(params, config.ListConversionPaths(), toSingletonList)
}

func (c *TerraformPluginSDKConnector) processParamsWithHCLParser(schemaMap map[string]*schema.Schema, params map[string]any) map[string]any {
Expand Down Expand Up @@ -255,6 +255,10 @@ func (c *TerraformPluginSDKConnector) Connect(ctx context.Context, mg xpresource
if err != nil {
return nil, errors.Wrap(err, "failed to get the observation")
}
tfState, err = convert(tfState, c.config.ListConversionPaths(), toSingletonList)
if err != nil {
return nil, err
}
copyParams := len(tfState) == 0
if err = resource.GetSensitiveParameters(ctx, &APISecretClient{kube: c.kube}, tr, tfState, tr.GetConnectionDetailsMapping()); err != nil {
return nil, errors.Wrap(err, "cannot store sensitive parameters into tfState")
Expand Down Expand Up @@ -511,6 +515,10 @@ func (n *terraformPluginSDKExternal) Observe(ctx context.Context, mg xpresource.
}
mg.SetConditions(xpv1.Available())

stateValueMap, err = convert(stateValueMap, n.config.ListConversionPaths(), toEmbeddedObject)
if err != nil {
return managed.ExternalObservation{}, err
}
buff, err := json.TFParser.Marshal(stateValueMap)
if err != nil {
return managed.ExternalObservation{}, errors.Wrap(err, "cannot marshal the attributes of the new state for late-initialization")
Expand Down Expand Up @@ -623,6 +631,10 @@ func (n *terraformPluginSDKExternal) Create(ctx context.Context, mg xpresource.M
if _, err := n.setExternalName(mg, stateValueMap); err != nil {
return managed.ExternalCreation{}, errors.Wrap(err, "failed to set the external-name of the managed resource during create")
}
stateValueMap, err = convert(stateValueMap, n.config.ListConversionPaths(), toEmbeddedObject)
if err != nil {
return managed.ExternalCreation{}, err
}
err = mg.(resource.Terraformed).SetObservation(stateValueMap)
if err != nil {
return managed.ExternalCreation{}, errors.Errorf("could not set observation: %v", err)
Expand Down
34 changes: 34 additions & 0 deletions pkg/schema/traverser/fieldpath.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// SPDX-FileCopyrightText: 2023 The Crossplane Authors <https://crossplane.io>
//
// SPDX-License-Identifier: Apache-2.0

package traverser

import "github.com/crossplane/crossplane-runtime/pkg/fieldpath"

const (
// FieldPathWildcard is the wildcard expression in fieldpath indices.
FieldPathWildcard = "*"
)

// FieldPathWithWildcard joins the given segment strings into a field path.
func FieldPathWithWildcard(parts []string) string {
seg := make(fieldpath.Segments, len(parts))
for i, p := range parts {
seg[i] = fieldpath.Field(p)
}
return seg.String()
}

// FieldPath joins the given segment strings into a field path eliminating
// the wildcard index segments.
func FieldPath(parts []string) string {
seg := make(fieldpath.Segments, len(parts))
for i, p := range parts {
if p == FieldPathWildcard {
continue
}
seg[i] = fieldpath.Field(p)
}
return seg.String()
}
Loading

0 comments on commit d078959

Please sign in to comment.