From 4771f98feb35e3a2d666489a82c39f4e2260d536 Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Mon, 25 Nov 2024 12:24:51 -0500 Subject: [PATCH] Fixes union type conversions to CloudControl This change fixes the conversion of Pulumi input properties to the format expected by the CloudControl API in the case when Pulumi schema specifies a union type (OneOf) without a discriminator. Previously the first matching case would be picked which could erroneously send empty data to CloudControl. With this change a heuristic is run instead to pick the non-error case with the largest map or array. In the long term it would be better to support discriminators and manage their metadata (#1849). Fixes #1846 --- provider/pkg/naming/convert.go | 53 +++++++++++++++++++++++-- provider/pkg/naming/convert_test.go | 61 +++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 4 deletions(-) diff --git a/provider/pkg/naming/convert.go b/provider/pkg/naming/convert.go index 39de03f1fa..9b499a5297 100644 --- a/provider/pkg/naming/convert.go +++ b/provider/pkg/naming/convert.go @@ -3,16 +3,19 @@ package naming import ( + "errors" "fmt" "reflect" "slices" "strings" + "github.com/golang/glog" "github.com/mattbaird/jsonpatch" "github.com/pulumi/pulumi-aws-native/provider/pkg/metadata" "github.com/pulumi/pulumi-go-provider/resourcex" pschema "github.com/pulumi/pulumi/pkg/v3/codegen/schema" "github.com/pulumi/pulumi/sdk/v3/go/common/resource" + "github.com/pulumi/pulumi/sdk/v3/go/common/util/contract" ) // SdkToCfn converts Pulumi-SDK-shaped state to CloudFormation-shaped payload. In particular, SDK properties @@ -94,14 +97,56 @@ func (c *sdkToCfnConverter) sdkTypedValueToCfn(spec *pschema.TypeSpec, v interfa } if spec.OneOf != nil { + contract.Assertf(len(spec.OneOf) >= 1, "at least one union case is required") + + results := []any{} + errs := []error{} + for _, item := range spec.OneOf { converted, err := c.sdkTypedValueToCfn(&item, v) - // return the first successful conversion - if err == nil { - return converted, nil + if err != nil { + errs = append(errs, err) + } else { + results = append(results, converted) + } + } + + switch { + case len(results) == 0: + glog.V(9).Infof("conversion error: all union variants failed: %v", errors.Join(errs...)) + return nil, &ConversionError{fmt.Sprintf("%v", *spec), v} + case len(results) == 1: + return results[0], nil + default: + // Properties such as aws-native:datazone:DataSource configuration do not specify a + // discriminator yet. Without a discriminator in case of multiple successful results we could + // instead pick the largest by length. + + sizeOf := func(x any) int { + switch x := x.(type) { + case map[string]any: + return len(x) + case []any: + return len(x) + case nil: + return 0 + default: + return 1 + } } + + var bestResult any + bestResultSize := -1 + for _, r := range results { + s := sizeOf(r) + if s > bestResultSize { + bestResult = r + bestResultSize = s + } + } + return bestResult, nil } - return nil, &ConversionError{fmt.Sprintf("%v", *spec), v} + } switch spec.Type { diff --git a/provider/pkg/naming/convert_test.go b/provider/pkg/naming/convert_test.go index 3a3a1f4a75..ff80aad8a3 100644 --- a/provider/pkg/naming/convert_test.go +++ b/provider/pkg/naming/convert_test.go @@ -12,6 +12,7 @@ import ( pschema "github.com/pulumi/pulumi/pkg/v3/codegen/schema" "github.com/pulumi/pulumi/sdk/v3/go/common/resource" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestCfnToSdk(t *testing.T) { @@ -83,6 +84,66 @@ func TestSdkToCfnOneOf(t *testing.T) { assert.Equal(t, expected, actual) } +// Check that ambiguous OneOf without a discriminator will pick the largest match. +func TestSdkToCfnOneOfAmbiguous(t *testing.T) { + res := metadata.CloudAPIResource{ + Inputs: map[string]pschema.PropertySpec{ + "configuration": {TypeSpec: pschema.TypeSpec{ + OneOf: []pschema.TypeSpec{ + {Ref: "#/types/aws-native:datazone:DataSourceConfigurationInput0Properties"}, + {Ref: "#/types/aws-native:datazone:DataSourceConfigurationInput1Properties"}, + }, + }}, + }, + } + types := map[string]metadata.CloudAPIType{ + "aws-native:datazone:DataSourceConfigurationInput0Properties": metadata.CloudAPIType{ + Type: "object", + Properties: map[string]pschema.PropertySpec{ + "glueRunConfiguration": pschema.PropertySpec{ + TypeSpec: pschema.TypeSpec{ + Ref: "#/types/aws-native:datazone:DataSourceGlueRunConfigurationInput", + }, + }, + }, + }, + "aws-native:datazone:DataSourceGlueRunConfigurationInput": metadata.CloudAPIType{ + Type: "object", + Properties: map[string]pschema.PropertySpec{ + "dataAccessRole": pschema.PropertySpec{TypeSpec: pschema.TypeSpec{Type: "string"}}, + }, + }, + "aws-native:datazone:DataSourceConfigurationInput1Properties": metadata.CloudAPIType{ + Type: "object", + Properties: map[string]pschema.PropertySpec{"redshiftRunConfiguration": pschema.PropertySpec{ + TypeSpec: pschema.TypeSpec{ + Ref: "#/types/aws-native:datazone:DataSourceRedshiftRunConfigurationInput", + }, + }}, + }, + "aws-native:datazone:DataSourceRedshiftRunConfigurationInput": metadata.CloudAPIType{ + Type: "object", + Properties: map[string]pschema.PropertySpec{ + "dataAccessRole": pschema.PropertySpec{TypeSpec: pschema.TypeSpec{Type: "string"}}, + }, + }, + } + + result, err := SdkToCfn(&res, types, map[string]any{ + "configuration": map[string]any{ + "redshiftRunConfiguration": map[string]any{ + "dataAccessRole": "myrole", + }, + }, + }) + require.NoError(t, err) + + actualConfig := result["Configuration"].(map[string]any) + actualRedshiftConfig := actualConfig["RedshiftRunConfiguration"].(map[string]any) + actualDataAccessRole := actualRedshiftConfig["DataAccessRole"].(string) + require.Equal(t, "myrole", actualDataAccessRole) +} + func TestDiffToPatch(t *testing.T) { test := func(t *testing.T, diff resource.ObjectDiff, expected []jsonpatch.JsonPatchOperation) { res := sampleSchema.Resources["aws-native:ecs:Service"]