From 017f4f25377e28c4e0f025d7adf8e60901878a2b Mon Sep 17 00:00:00 2001 From: Alper Rifat Ulucinar Date: Thu, 18 Apr 2024 15:24:41 +0300 Subject: [PATCH] Add conversion.singletonConverter conversion function for CRD API conversions between singleton list & embedded object API versions. - Export conversion.Convert to be reused in embedded singleton list webhook API conversions. Signed-off-by: Alper Rifat Ulucinar --- pkg/config/common.go | 1 + pkg/config/conversion/conversions.go | 46 +++++++++++++++++++ .../conversion/runtime_conversion.go} | 37 +++++++++------ pkg/config/resource.go | 36 ++++++++++----- pkg/config/schema_conversions.go | 2 +- pkg/controller/external_tfpluginsdk.go | 9 ++-- 6 files changed, 100 insertions(+), 31 deletions(-) rename pkg/{controller/conversion.go => config/conversion/runtime_conversion.go} (73%) diff --git a/pkg/config/common.go b/pkg/config/common.go index c123a9c7..d09f9e6c 100644 --- a/pkg/config/common.go +++ b/pkg/config/common.go @@ -92,6 +92,7 @@ func DefaultResource(name string, terraformSchema *schema.Resource, terraformPlu SchemaElementOptions: make(SchemaElementOptions), ServerSideApplyMergeStrategies: make(ServerSideApplyMergeStrategies), MarkStorageVersion: true, + listConversionPaths: make(map[string]string), } for _, f := range opts { f(r) diff --git a/pkg/config/conversion/conversions.go b/pkg/config/conversion/conversions.go index ba5a2da1..b91e7a27 100644 --- a/pkg/config/conversion/conversions.go +++ b/pkg/config/conversion/conversions.go @@ -5,6 +5,8 @@ package conversion import ( + "fmt" + "github.com/crossplane/crossplane-runtime/pkg/fieldpath" "github.com/crossplane/crossplane-runtime/pkg/resource" "github.com/pkg/errors" @@ -19,6 +21,10 @@ const ( AllVersions = "*" ) +const ( + pathForProvider = "spec.forProvider" +) + // Conversion is the interface for the API version converters. // Conversion implementations registered for a source, target // pair are called in chain so Conversion implementations can be modular, e.g., @@ -68,6 +74,10 @@ type baseConversion struct { targetVersion string } +func (c *baseConversion) String() string { + return fmt.Sprintf("source API version %q, target API version %q", c.sourceVersion, c.targetVersion) +} + func newBaseConversion(sourceVersion, targetVersion string) baseConversion { return baseConversion{ sourceVersion: sourceVersion, @@ -141,3 +151,39 @@ func NewCustomConverter(sourceVersion, targetVersion string, converter func(src, customConverter: converter, } } + +type singletonListConverter struct { + baseConversion + crdPaths []string + mode Mode +} + +// NewSingletonListConversion returns a new Conversion from the specified +// sourceVersion of an API to the specified targetVersion and uses the +// CRD field paths given in crdPaths to convert between the singleton +// lists and embedded objects in the given conversion mode. +func NewSingletonListConversion(sourceVersion, targetVersion string, crdPaths []string, mode Mode) Conversion { + return &singletonListConverter{ + baseConversion: newBaseConversion(sourceVersion, targetVersion), + crdPaths: crdPaths, + mode: mode, + } +} + +func (s *singletonListConverter) ConvertPaved(src, target *fieldpath.Paved) (bool, error) { + if len(s.crdPaths) == 0 { + return false, nil + } + v, err := src.GetValue(pathForProvider) + if err != nil { + return true, errors.Wrapf(err, "failed to read the %s value for conversion in mode %q", pathForProvider, s.mode) + } + m, ok := v.(map[string]any) + if !ok { + return true, errors.Errorf("value at path %s is not a map[string]any", pathForProvider) + } + if _, err := Convert(m, s.crdPaths, s.mode); err != nil { + return true, errors.Wrapf(err, "failed to convert the source map in mode %q with %s", s.mode, s.baseConversion) + } + return true, errors.Wrapf(target.SetValue(pathForProvider, m), "failed to set the %s value for conversion in mode %q", pathForProvider, s.mode) +} diff --git a/pkg/controller/conversion.go b/pkg/config/conversion/runtime_conversion.go similarity index 73% rename from pkg/controller/conversion.go rename to pkg/config/conversion/runtime_conversion.go index 455e2cd6..4f2217a6 100644 --- a/pkg/controller/conversion.go +++ b/pkg/config/conversion/runtime_conversion.go @@ -1,8 +1,8 @@ -// SPDX-FileCopyrightText: 2023 The Crossplane Authors +// SPDX-FileCopyrightText: 2024 The Crossplane Authors // // SPDX-License-Identifier: Apache-2.0 -package controller +package conversion import ( "slices" @@ -13,19 +13,28 @@ import ( "github.com/pkg/errors" ) -type conversionMode int +// Mode denotes the mode of the runtime API conversion, e.g., +// conversion of embedded objects into singleton lists. +type Mode int const ( - toEmbeddedObject conversionMode = iota - toSingletonList + // ToEmbeddedObject represents a runtime conversion from a singleton list + // to an embedded object, i.e., the runtime conversions needed while + // reading from the Terraform state and updating the CRD + // (for status, late-initialization, etc.) + ToEmbeddedObject Mode = iota + // ToSingletonList represents a runtime conversion from an embedded object + // to a singleton list, i.e., the runtime conversions needed while passing + // the configuration data to the underlying Terraform layer. + ToSingletonList ) // String returns a string representation of the conversion mode. -func (m conversionMode) String() string { +func (m Mode) String() string { switch m { - case toSingletonList: + case ToSingletonList: return "toSingletonList" - case toEmbeddedObject: + case ToEmbeddedObject: return "toEmbeddedObject" default: return "unknown" @@ -57,18 +66,18 @@ func setValue(pv *fieldpath.Paved, v any, fp string) error { return nil } -// convert performs conversion between singleton lists and embedded objects +// 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) { //nolint:gocyclo // easier to follow as a unit +func Convert(params map[string]any, paths []string, mode Mode) (map[string]any, error) { //nolint:gocyclo // easier to follow as a unit switch mode { - case toSingletonList: + case ToSingletonList: slices.Sort(paths) - case toEmbeddedObject: + case ToEmbeddedObject: sort.Slice(paths, func(i, j int) bool { return paths[i] > paths[j] }) @@ -89,11 +98,11 @@ func convert(params map[string]any, paths []string, mode conversionMode) (map[st 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: + 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: + case ToEmbeddedObject: s, ok := v.([]any) if !ok || len(s) > 1 { // if len(s) is 0, then it's not a slice diff --git a/pkg/config/resource.go b/pkg/config/resource.go index 7f257ca4..9fb6e7dc 100644 --- a/pkg/config/resource.go +++ b/pkg/config/resource.go @@ -441,14 +441,14 @@ type Resource struct { // version is by default the storage version. MarkStorageVersion bool - // 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. + // listConversionPaths maps the Terraform field paths of embedded objects + // that need to be converted into singleton lists (lists of + // at most one element) at runtime, to the corresponding CRD paths. // 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 + listConversionPaths map[string]string // TerraformConfigurationInjector allows a managed resource to inject // configuration values in the Terraform configuration map obtained by @@ -556,16 +556,28 @@ func (m SchemaElementOptions) AddToObservation(el string) bool { return m[el] != nil && m[el].AddToObservation } -// ListConversionPaths returns the Resource's runtime Terraform list +// TFListConversionPaths returns the Resource's runtime Terraform list // conversion paths in fieldpath syntax. -func (r *Resource) ListConversionPaths() []string { - l := make([]string, len(r.listConversionPaths)) - copy(l, r.listConversionPaths) +func (r *Resource) TFListConversionPaths() []string { + l := make([]string, 0, len(r.listConversionPaths)) + for k := range r.listConversionPaths { + l = append(l, k) + } + return l +} + +// CRDListConversionPaths returns the Resource's runtime CRD list +// conversion paths in fieldpath syntax. +func (r *Resource) CRDListConversionPaths() []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. +// AddSingletonListConversion configures the list at the specified Terraform +// field path and the specified CRD 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. @@ -574,14 +586,14 @@ func (r *Resource) ListConversionPaths() []string { // 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) { +func (r *Resource) AddSingletonListConversion(tfPath, crdPath 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) + r.listConversionPaths[tfPath] = crdPath } // SetEmbeddedObject sets the EmbeddedObject for the specified key. diff --git a/pkg/config/schema_conversions.go b/pkg/config/schema_conversions.go index 30504a44..3258a409 100644 --- a/pkg/config/schema_conversions.go +++ b/pkg/config/schema_conversions.go @@ -52,6 +52,6 @@ func (l *SingletonListEmbedder) VisitResource(r *traverser.ResourceNode) error { if r.Schema.MaxItems != 1 { return nil } - l.r.AddSingletonListConversion(traverser.FieldPathWithWildcard(r.TFPath)) + l.r.AddSingletonListConversion(traverser.FieldPathWithWildcard(r.TFPath), traverser.FieldPathWithWildcard(r.CRDPath)) return nil } diff --git a/pkg/controller/external_tfpluginsdk.go b/pkg/controller/external_tfpluginsdk.go index 548b3677..0262e0f0 100644 --- a/pkg/controller/external_tfpluginsdk.go +++ b/pkg/controller/external_tfpluginsdk.go @@ -26,6 +26,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "github.com/crossplane/upjet/pkg/config" + "github.com/crossplane/upjet/pkg/config/conversion" "github.com/crossplane/upjet/pkg/metrics" "github.com/crossplane/upjet/pkg/resource" "github.com/crossplane/upjet/pkg/resource/json" @@ -155,7 +156,7 @@ func getExtendedParameters(ctx context.Context, tr resource.Terraformed, externa params["tags_all"] = params["tags"] } } - return convert(params, config.ListConversionPaths(), toSingletonList) + return conversion.Convert(params, config.TFListConversionPaths(), conversion.ToSingletonList) } func (c *TerraformPluginSDKConnector) processParamsWithHCLParser(schemaMap map[string]*schema.Schema, params map[string]any) map[string]any { @@ -255,7 +256,7 @@ 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) + tfState, err = conversion.Convert(tfState, c.config.TFListConversionPaths(), conversion.ToSingletonList) if err != nil { return nil, err } @@ -515,7 +516,7 @@ func (n *terraformPluginSDKExternal) Observe(ctx context.Context, mg xpresource. } mg.SetConditions(xpv1.Available()) - stateValueMap, err = convert(stateValueMap, n.config.ListConversionPaths(), toEmbeddedObject) + stateValueMap, err = conversion.Convert(stateValueMap, n.config.TFListConversionPaths(), conversion.ToEmbeddedObject) if err != nil { return managed.ExternalObservation{}, err } @@ -631,7 +632,7 @@ 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) + stateValueMap, err = conversion.Convert(stateValueMap, n.config.TFListConversionPaths(), conversion.ToEmbeddedObject) if err != nil { return managed.ExternalCreation{}, err }