From 767b88f554daac859800cefcdfb3f4ec3e1b69a2 Mon Sep 17 00:00:00 2001 From: Alper Rifat Ulucinar Date: Wed, 17 Apr 2024 12:50:57 +0300 Subject: [PATCH] Make singleton-list-to-embedded-object API conversions optional Signed-off-by: Alper Rifat Ulucinar --- pkg/config/common_test.go | 1 + pkg/config/provider.go | 50 +++++++++++----------- pkg/config/resource.go | 6 +-- pkg/config/schema_conversions.go | 57 ++++++++++++++++++++++++++ pkg/controller/conversion.go | 2 +- pkg/controller/external_tfpluginsdk.go | 2 +- pkg/schema/traverser/traverse.go | 22 ++++++---- pkg/types/builder.go | 3 +- pkg/types/field.go | 3 +- 9 files changed, 102 insertions(+), 44 deletions(-) create mode 100644 pkg/config/schema_conversions.go diff --git a/pkg/config/common_test.go b/pkg/config/common_test.go index 3bdfc553..879f6b8d 100644 --- a/pkg/config/common_test.go +++ b/pkg/config/common_test.go @@ -129,6 +129,7 @@ func TestDefaultResource(t *testing.T) { cmpopts.IgnoreFields(Resource{}, "useTerraformPluginSDKClient"), cmpopts.IgnoreFields(Resource{}, "useTerraformPluginFrameworkClient"), cmpopts.IgnoreFields(Resource{}, "requiredFields"), + cmpopts.IgnoreFields(Resource{}, "listConversionPaths"), } for name, tc := range cases { diff --git a/pkg/config/provider.go b/pkg/config/provider.go index 85917e6a..9ae89ecb 100644 --- a/pkg/config/provider.go +++ b/pkg/config/provider.go @@ -9,8 +9,6 @@ import ( "fmt" "regexp" - "github.com/crossplane/upjet/pkg/schema/traverser" - tfjson "github.com/hashicorp/terraform-json" fwprovider "github.com/hashicorp/terraform-plugin-framework/provider" fwresource "github.com/hashicorp/terraform-plugin-framework/resource" @@ -18,6 +16,7 @@ import ( "github.com/pkg/errors" "github.com/crossplane/upjet/pkg/registry" + "github.com/crossplane/upjet/pkg/schema/traverser" conversiontfjson "github.com/crossplane/upjet/pkg/types/conversion/tfjson" ) @@ -157,6 +156,12 @@ type Provider struct { // resourceConfigurators is a map holding resource configurators where key // is Terraform resource name. resourceConfigurators map[string]ResourceConfiguratorChain + + // schemaTraversers is a chain of schema traversers to be used with + // this Provider configuration. Schema traversers can be used to inspect or + // modify the Provider configuration based on the underlying Terraform + // resource schemas. + schemaTraversers []traverser.SchemaTraverser } // ReferenceInjector injects cross-resource references across the resources @@ -259,19 +264,32 @@ func WithFeaturesPackage(s string) ProviderOption { } } +// WithMainTemplate configures the provider family main module file's path. +// This template file will be used to generate the main modules of the +// family's members. func WithMainTemplate(template string) ProviderOption { return func(p *Provider) { p.MainTemplate = template } } +// WithSchemaTraversers configures a chain of schema traversers to be used with +// this Provider configuration. Schema traversers can be used to inspect or +// modify the Provider configuration based on the underlying Terraform +// resource schemas. +func WithSchemaTraversers(traversers ...traverser.SchemaTraverser) ProviderOption { + return func(p *Provider) { + p.schemaTraversers = traversers + } +} + // NewProvider builds and returns a new Provider from provider // tfjson schema, that is generated using Terraform CLI with: // `terraform providers schema --json` func NewProvider(schema []byte, prefix string, modulePath string, metadata []byte, opts ...ProviderOption) *Provider { //nolint:gocyclo ps := tfjson.ProviderSchemas{} if err := ps.UnmarshalJSON(schema); err != nil { - panic(err) + panic(errors.Wrap(err, "failed to unmarshal the Terraform JSON schema")) } if len(ps.Schemas) != 1 { panic(fmt.Sprintf("there should exactly be 1 provider schema but there are %d", len(ps.Schemas))) @@ -354,11 +372,9 @@ func NewProvider(schema []byte, prefix string, modulePath string, metadata []byt 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) + // configurations + if err := traverseSchemas(name, terraformResource, p.Resources[name], p.schemaTraversers...); err != nil { + panic(errors.Wrap(err, "failed to execute the Terraform schema traverser chain")) } } for i, refInjector := range p.refInjectors { @@ -441,21 +457,3 @@ 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 -} diff --git a/pkg/config/resource.go b/pkg/config/resource.go index 14389c44..9d8fadea 100644 --- a/pkg/config/resource.go +++ b/pkg/config/resource.go @@ -554,10 +554,8 @@ func (m SchemaElementOptions) AddToObservation(el string) bool { // 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) - } + l := make([]string, len(r.listConversionPaths)) + copy(l, r.listConversionPaths) return l } diff --git a/pkg/config/schema_conversions.go b/pkg/config/schema_conversions.go new file mode 100644 index 00000000..30504a44 --- /dev/null +++ b/pkg/config/schema_conversions.go @@ -0,0 +1,57 @@ +// SPDX-FileCopyrightText: 2024 The Crossplane Authors +// +// SPDX-License-Identifier: Apache-2.0 + +package config + +import ( + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + + "github.com/crossplane/upjet/pkg/schema/traverser" +) + +var _ ResourceSetter = &SingletonListEmbedder{} + +// ResourceSetter allows the context Resource to be set for a traverser. +type ResourceSetter interface { + SetResource(r *Resource) +} + +func traverseSchemas(tfName string, tfResource *schema.Resource, r *Resource, visitors ...traverser.SchemaTraverser) error { + // set the upjet Resource configuration as context for the visitors that + // satisfy the ResourceSetter interface. + for _, v := range visitors { + if rs, ok := v.(ResourceSetter); ok { + rs.SetResource(r) + } + } + return traverser.Traverse(tfName, tfResource, visitors...) +} + +type resourceContext struct { + r *Resource +} + +func (rc *resourceContext) SetResource(r *Resource) { + rc.r = r +} + +// SingletonListEmbedder is a schema traverser for embedding singleton lists +// in the Terraform schema as objects. +type SingletonListEmbedder struct { + resourceContext + traverser.NoopTraverser +} + +func (l *SingletonListEmbedder) 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 +} diff --git a/pkg/controller/conversion.go b/pkg/controller/conversion.go index eb9c8cd8..455e2cd6 100644 --- a/pkg/controller/conversion.go +++ b/pkg/controller/conversion.go @@ -64,7 +64,7 @@ func setValue(pv *fieldpath.Paved, v any, fp string) error { // 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) { +func convert(params map[string]any, paths []string, mode conversionMode) (map[string]any, error) { //nolint:gocyclo // easier to follow as a unit switch mode { case toSingletonList: slices.Sort(paths) diff --git a/pkg/controller/external_tfpluginsdk.go b/pkg/controller/external_tfpluginsdk.go index 8cb8ef54..548b3677 100644 --- a/pkg/controller/external_tfpluginsdk.go +++ b/pkg/controller/external_tfpluginsdk.go @@ -581,7 +581,7 @@ func (n *terraformPluginSDKExternal) setExternalName(mg xpresource.Managed, stat return oldName != newName, nil } -func (n *terraformPluginSDKExternal) Create(ctx context.Context, mg xpresource.Managed) (managed.ExternalCreation, error) { +func (n *terraformPluginSDKExternal) Create(ctx context.Context, mg xpresource.Managed) (managed.ExternalCreation, error) { //nolint:gocyclo // easier to follow as a unit n.logger.Debug("Creating the external resource") start := time.Now() newState, diag := n.resourceSchema.Apply(ctx, n.opTracker.GetTfState(), n.instanceDiff, n.ts.Meta) diff --git a/pkg/schema/traverser/traverse.go b/pkg/schema/traverser/traverse.go index 774b75b9..8846f60f 100644 --- a/pkg/schema/traverser/traverse.go +++ b/pkg/schema/traverser/traverse.go @@ -19,12 +19,16 @@ const ( var _ Element = &SchemaNode{} var _ Element = &ResourceNode{} -// Traverse traverses the Terraform schema of the given Terraform resource and -// visits each of the specified visitors on the traversed schema's nodes. +// Traverse traverses the Terraform schema of the given Terraform resource +// with the given Terraform resource name and visits each of the specified +// visitors on the traversed schema's nodes. // If any of the visitors in the chain reports an error, // it stops the traversal. -func Traverse(tfResource *schema.Resource, visitors ...SchemaTraverser) error { - return traverse(tfResource, Node{}, visitors...) +func Traverse(tfName string, tfResource *schema.Resource, visitors ...SchemaTraverser) error { + if len(visitors) == 0 { + return nil + } + return traverse(tfResource, Node{TFName: tfName}, visitors...) } // SchemaTraverser represents a visitor on the schema.Schema and @@ -44,6 +48,8 @@ type Element interface { // Node represents a schema node that's being traversed. type Node struct { + // TFName is the Terraform resource name + TFName string // Schema is the Terraform schema associated with the visited node during a // traversal. Schema *schema.Schema @@ -77,7 +83,7 @@ func (r *ResourceNode) Accept(v SchemaTraverser) error { return v.VisitResource(r) } -func traverse(tfResource *schema.Resource, pNode Node, visitors ...SchemaTraverser) error { +func traverse(tfResource *schema.Resource, pNode Node, visitors ...SchemaTraverser) error { //nolint:gocyclo // traverse logic is easier to follow in a unit m := tfResource.Schema if m == nil && tfResource.SchemaFunc != nil { m = tfResource.SchemaFunc() @@ -85,10 +91,10 @@ func traverse(tfResource *schema.Resource, pNode Node, visitors ...SchemaTravers if m == nil { return nil } - var node Node + node := Node{TFName: pNode.TFName} for k, s := range m { - node.CRDPath = append(pNode.CRDPath, name.NewFromSnake(k).LowerCamelComputed) - node.TFPath = append(pNode.TFPath, k) + node.CRDPath = append(pNode.CRDPath, name.NewFromSnake(k).LowerCamelComputed) //nolint:gocritic // the parent node's path must not be modified + node.TFPath = append(pNode.TFPath, k) //nolint:gocritic // the parent node's path must not be modified node.Schema = s switch e := s.Elem.(type) { case *schema.Schema: diff --git a/pkg/types/builder.go b/pkg/types/builder.go index 86b098d9..b6dcd9f9 100644 --- a/pkg/types/builder.go +++ b/pkg/types/builder.go @@ -11,14 +11,13 @@ import ( "sort" "strings" - "github.com/crossplane/upjet/pkg/schema/traverser" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" twtypes "github.com/muvaf/typewriter/pkg/types" "github.com/pkg/errors" "k8s.io/utils/ptr" "github.com/crossplane/upjet/pkg/config" + "github.com/crossplane/upjet/pkg/schema/traverser" ) const ( diff --git a/pkg/types/field.go b/pkg/types/field.go index d468609d..b85812be 100644 --- a/pkg/types/field.go +++ b/pkg/types/field.go @@ -12,14 +12,13 @@ import ( "sort" "strings" - "github.com/crossplane/upjet/pkg/schema/traverser" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/pkg/errors" "k8s.io/utils/ptr" "github.com/crossplane/upjet/pkg" "github.com/crossplane/upjet/pkg/config" + "github.com/crossplane/upjet/pkg/schema/traverser" "github.com/crossplane/upjet/pkg/types/comments" "github.com/crossplane/upjet/pkg/types/name" )