diff --git a/internal/testprovider/schema.go b/internal/testprovider/schema.go index e78f07feb..771089d87 100644 --- a/internal/testprovider/schema.go +++ b/internal/testprovider/schema.go @@ -929,3 +929,131 @@ func MaxItemsOneProvider() *schemav2.Provider { }, } } + +func ConflictsWithValidationProvider() *schemav2.Provider { + return &schemav2.Provider{ + Schema: map[string]*schemav2.Schema{}, + ResourcesMap: map[string]*schemav2.Resource{ + "default_value_res": { + Schema: map[string]*schemav2.Schema{ + "conflicting_property": { + Type: schemav2.TypeString, + Optional: true, + // Some TF providers use this to mean not specified + // https://github.com/pulumi/pulumi-azuredevops/issues/38 + Default: "", + ConflictsWith: []string{"conflicting_property2"}, + }, + "conflicting_property2": { + Type: schemav2.TypeString, + Optional: true, + Default: "", + ConflictsWith: []string{"conflicting_property"}, + }, + + "conflicting_required_property": { + Type: schemav2.TypeString, + Required: true, + Default: "", + ConflictsWith: []string{"conflicting_nonrequired_property2"}, + }, + "conflicting_nonrequired_property2": { + Type: schemav2.TypeString, + Optional: true, + Default: "", + ConflictsWith: []string{"conflicting_required_property"}, + }, + }, + }, + }, + } +} + +func ExactlyOneOfValidationProvider() *schemav2.Provider { + return &schemav2.Provider{ + Schema: map[string]*schemav2.Schema{}, + ResourcesMap: map[string]*schemav2.Resource{ + "default_value_res": { + Schema: map[string]*schemav2.Schema{ + "exactly_one_of_property": { + Type: schemav2.TypeString, + Optional: true, + Default: "", + ExactlyOneOf: []string{"exactly_one_of_property", "exactly_one_of_property2"}, + }, + "exactly_one_of_property2": { + Type: schemav2.TypeString, + Optional: true, + Default: "", + ExactlyOneOf: []string{"exactly_one_of_property", "exactly_one_of_property2"}, + }, + + // This doesn't really make much sense as a schema + // but it should still behave as expected. + "exactly_one_of_required_property": { + Type: schemav2.TypeString, + Required: true, + Default: "", + ExactlyOneOf: []string{"exactly_one_of_required_property", "exactly_one_of_nonrequired_property2"}, + }, + "exactly_one_of_nonrequired_property2": { + Type: schemav2.TypeString, + Optional: true, + Default: "", + ExactlyOneOf: []string{"exactly_one_of_required_property", "exactly_one_of_nonrequired_property2"}, + }, + }, + }, + }, + } +} + +func RequiredWithValidationProvider() *schemav2.Provider { + return &schemav2.Provider{ + Schema: map[string]*schemav2.Schema{}, + ResourcesMap: map[string]*schemav2.Resource{ + "default_value_res": { + Schema: map[string]*schemav2.Schema{ + "required_with_property": { + Type: schemav2.TypeString, + Optional: true, + Default: "", + RequiredWith: []string{"required_with_property2"}, + }, + "required_with_property2": { + Type: schemav2.TypeString, + Optional: true, + Default: "", + RequiredWith: []string{"required_with_property"}, + }, + + "required_with_required_property": { + Type: schemav2.TypeString, + Required: true, + Default: "", + RequiredWith: []string{"required_with_nonrequired_property"}, + }, + "required_with_nonrequired_property": { + Type: schemav2.TypeString, + Optional: true, + Default: "", + RequiredWith: []string{"required_with_required_property"}, + }, + + "required_with_required_property2": { + Type: schemav2.TypeString, + Required: true, + Default: "", + RequiredWith: []string{"required_with_required_property3"}, + }, + "required_with_required_property3": { + Type: schemav2.TypeString, + Required: true, + Default: "", + RequiredWith: []string{"required_with_required_property2"}, + }, + }, + }, + }, + } +} diff --git a/pkg/tfbridge/provider.go b/pkg/tfbridge/provider.go index 54e898834..13f476ce8 100644 --- a/pkg/tfbridge/provider.go +++ b/pkg/tfbridge/provider.go @@ -664,7 +664,7 @@ func (p *Provider) Check(ctx context.Context, req *pulumirpc.CheckRequest) (*pul // Now fetch the default values so that (a) we can return them to the caller and (b) so that validation // includes the default values. Otherwise, the provider wouldn't be presented with its own defaults. tfname := res.TFName - inputs, assets, err := MakeTerraformInputs(ctx, + inputs, _, err := makeTerraformInputsWithoutTFDefaults(ctx, &PulumiResource{URN: urn, Properties: news, Seed: req.RandomSeed}, p.configValues, olds, news, res.TF.Schema(), res.Schema.Fields) if err != nil { @@ -683,6 +683,14 @@ func (p *Provider) Check(ctx context.Context, req *pulumirpc.CheckRequest) (*pul // Now produce CheckFalures for any properties that failed verification. failures := p.adaptCheckFailures(ctx, urn, false /*isProvider*/, res.TF.Schema(), res.Schema.GetFields(), errs) + // Now re-generate the inputs WITH the TF defaults + inputs, assets, err := MakeTerraformInputs(ctx, + &PulumiResource{URN: urn, Properties: news, Seed: req.RandomSeed}, + p.configValues, olds, news, res.TF.Schema(), res.Schema.Fields) + if err != nil { + return nil, err + } + // After all is said and done, we need to go back and return only what got populated as a diff from the origin. pinputs := MakeTerraformOutputs(p.tf, inputs, res.TF.Schema(), res.Schema.Fields, assets, false, p.supportsSecrets) diff --git a/pkg/tfbridge/provider_test.go b/pkg/tfbridge/provider_test.go index 8861c229c..455ba9155 100644 --- a/pkg/tfbridge/provider_test.go +++ b/pkg/tfbridge/provider_test.go @@ -18,6 +18,7 @@ import ( "context" "fmt" "sort" + "strings" "testing" "time" @@ -2303,3 +2304,252 @@ func TestMaxItemOneWrongStateDiff(t *testing.T) { }`) }) } + +// These should test that we validate resources before applying TF defaults, +// since this is what TF does. +// https://github.com/pulumi/pulumi-terraform-bridge/issues/1546 +func TestDefaultsAndConflictsWithValidationInteraction(t *testing.T) { + p := testprovider.ConflictsWithValidationProvider() + provider := &Provider{ + tf: shimv2.NewProvider(p), + config: shimv2.NewSchemaMap(p.Schema), + resources: map[tokens.Type]Resource{ + "DefaultValueRes": { + TF: shimv2.NewResource(p.ResourcesMap["default_value_res"]), + TFName: "default_value_res", + Schema: &ResourceInfo{}, + }, + }, + } + + t.Run("CheckMissingRequiredProp", func(t *testing.T) { + //nolint:lll + testutils.Replay(t, provider, ` + { + "method": "/pulumirpc.ResourceProvider/Check", + "request": { + "urn": "urn:pulumi:dev::teststack::DefaultValueRes::exres", + "olds": {}, + "news": {}, + "randomSeed": "iYRxB6/8Mm7pwKIs+yK6IyMDmW9JSSTM6klzRUgZhRk=" + }, + "response": { + "inputs": { + "__defaults": [] + }, + "failures": [ + { "reason": "Missing required argument. The argument \"conflicting_required_property\" is required, but no definition was found.. Examine values at 'exres.conflictingRequiredProperty'." + } + ] + } + }`) + }) + + t.Run("CheckRequiredDoesNotConflict", func(t *testing.T) { + testutils.Replay(t, provider, ` + { + "method": "/pulumirpc.ResourceProvider/Check", + "request": { + "urn": "urn:pulumi:dev::teststack::DefaultValueRes::exres", + "olds": {}, + "news": { + "conflicting_required_property": "required" + }, + "randomSeed": "iYRxB6/8Mm7pwKIs+yK6IyMDmW9JSSTM6klzRUgZhRk=" + }, + "response": { + "inputs": { + "__defaults": [], + "conflictingRequiredProperty": "required" + } + } + }`) + }) +} + +// https://github.com/pulumi/pulumi-terraform-bridge/issues/1546 +func TestDefaultsAndExactlyOneOfValidationInteraction(t *testing.T) { + p := testprovider.ExactlyOneOfValidationProvider() + provider := &Provider{ + tf: shimv2.NewProvider(p), + config: shimv2.NewSchemaMap(p.Schema), + resources: map[tokens.Type]Resource{ + "DefaultValueRes": { + TF: shimv2.NewResource(p.ResourcesMap["default_value_res"]), + TFName: "default_value_res", + Schema: &ResourceInfo{}, + }, + }, + } + t.Run("CheckFailsWhenExactlyOneOfNotSpecified", func(t *testing.T) { + //nolint:lll + testutils.Replay(t, provider, strings.ReplaceAll(` + { + "method": "/pulumirpc.ResourceProvider/Check", + "request": { + "urn": "urn:pulumi:dev::teststack::DefaultValueRes::exres", + "olds": {}, + "news": { + }, + "randomSeed": "iYRxB6/8Mm7pwKIs+yK6IyMDmW9JSSTM6klzRUgZhRk=" + }, + "response": { + "inputs": { + "__defaults": [] + }, + "failures": [ + {"reason": "Invalid combination of arguments. \"exactly_one_of_nonrequired_property2\": one of $exactly_one_of_nonrequired_property2,exactly_one_of_required_property$ must be specified. Examine values at 'exres.exactlyOneOfNonrequiredProperty2'."}, + {"reason": "Invalid combination of arguments. \"exactly_one_of_property\": one of $exactly_one_of_property,exactly_one_of_property2$ must be specified. Examine values at 'exres.exactlyOneOfProperty'."}, + {"reason": "Invalid combination of arguments. \"exactly_one_of_property2\": one of $exactly_one_of_property,exactly_one_of_property2$ must be specified. Examine values at 'exres.exactlyOneOfProperty2'."}, + {"reason": "Invalid combination of arguments. \"exactly_one_of_required_property\": one of $exactly_one_of_nonrequired_property2,exactly_one_of_required_property$ must be specified. Examine values at 'exres.exactlyOneOfRequiredProperty'."} + ] + } + }`, "$", "`")) + }) + + t.Run("Check", func(t *testing.T) { + testutils.Replay(t, provider, ` + { + "method": "/pulumirpc.ResourceProvider/Check", + "request": { + "urn": "urn:pulumi:dev::teststack::DefaultValueRes::exres", + "olds": {}, + "news": { + "exactlyOneOfProperty": "exactly_one_value", + "exactlyOneOfRequiredProperty": "exactly_one_req_value" + }, + "randomSeed": "iYRxB6/8Mm7pwKIs+yK6IyMDmW9JSSTM6klzRUgZhRk=" + }, + "response": { + "inputs": { + "__defaults": [], + "exactlyOneOfProperty": "exactly_one_value", + "exactlyOneOfRequiredProperty": "exactly_one_req_value" + } + } + }`) + }) +} + +// https://github.com/pulumi/pulumi-terraform-bridge/issues/1546 +func TestDefaultsAndRequiredWithValidationInteraction(t *testing.T) { + p := testprovider.RequiredWithValidationProvider() + provider := &Provider{ + tf: shimv2.NewProvider(p), + config: shimv2.NewSchemaMap(p.Schema), + resources: map[tokens.Type]Resource{ + "DefaultValueRes": { + TF: shimv2.NewResource(p.ResourcesMap["default_value_res"]), + TFName: "default_value_res", + Schema: &ResourceInfo{}, + }, + }, + } + + t.Run("CheckMissingRequiredPropErrors", func(t *testing.T) { + //nolint:lll + testutils.Replay(t, provider, ` + { + "method": "/pulumirpc.ResourceProvider/Check", + "request": { + "urn": "urn:pulumi:dev::teststack::DefaultValueRes::exres", + "olds": {}, + "news": {}, + "randomSeed": "iYRxB6/8Mm7pwKIs+yK6IyMDmW9JSSTM6klzRUgZhRk=" + }, + "response": { + "inputs": { + "__defaults": [ + "requiredWithNonrequiredProperty", + "requiredWithProperty", + "requiredWithProperty2", + "requiredWithRequiredProperty", + "requiredWithRequiredProperty2", + "requiredWithRequiredProperty3" + ], + "requiredWithNonrequiredProperty": "", + "requiredWithProperty": "", + "requiredWithProperty2": "", + "requiredWithRequiredProperty": "", + "requiredWithRequiredProperty2": "", + "requiredWithRequiredProperty3": "" + }, + "failures": [ + {"reason": "Missing required argument. The argument \"required_with_required_property\" is required, but no definition was found.. Examine values at 'exres.requiredWithRequiredProperty'."}, + {"reason": "Missing required argument. The argument \"required_with_required_property2\" is required, but no definition was found.. Examine values at 'exres.requiredWithRequiredProperty2'."}, + {"reason": "Missing required argument. The argument \"required_with_required_property3\" is required, but no definition was found.. Examine values at 'exres.requiredWithRequiredProperty3'."} + ] + } + }`) + }) + + t.Run("CheckHappyPath", func(t *testing.T) { + testutils.Replay(t, provider, ` + { + "method": "/pulumirpc.ResourceProvider/Check", + "request": { + "urn": "urn:pulumi:dev::teststack::DefaultValueRes::exres", + "olds": {}, + "news": { + "requiredWithNonrequiredProperty": "foo", + "requiredWithProperty": "foo", + "requiredWithProperty2": "foo", + "requiredWithRequiredProperty": "foo", + "requiredWithRequiredProperty2": "foo", + "requiredWithRequiredProperty3": "foo" + }, + "randomSeed": "iYRxB6/8Mm7pwKIs+yK6IyMDmW9JSSTM6klzRUgZhRk=" + }, + "response": { + "inputs": { + "__defaults": [], + "requiredWithNonrequiredProperty": "foo", + "requiredWithProperty": "foo", + "requiredWithProperty2": "foo", + "requiredWithRequiredProperty": "foo", + "requiredWithRequiredProperty2": "foo", + "requiredWithRequiredProperty3": "foo" + } + } + }`) + }) + + t.Run("CheckMissingRequiredWith", func(t *testing.T) { + //nolint:lll + testutils.Replay(t, provider, strings.ReplaceAll(` + { + "method": "/pulumirpc.ResourceProvider/Check", + "request": { + "urn": "urn:pulumi:dev::teststack::DefaultValueRes::exres", + "olds": {}, + "news": { + "requiredWithProperty": "foo", + "requiredWithRequiredProperty": "foo", + "requiredWithRequiredProperty2": "foo" + }, + "randomSeed": "iYRxB6/8Mm7pwKIs+yK6IyMDmW9JSSTM6klzRUgZhRk=" + }, + "response": { + "inputs": { + "__defaults": [ + "requiredWithNonrequiredProperty", + "requiredWithProperty2", + "requiredWithRequiredProperty3" + ], + "requiredWithNonrequiredProperty": "", + "requiredWithProperty": "foo", + "requiredWithProperty2": "", + "requiredWithRequiredProperty": "foo", + "requiredWithRequiredProperty2": "foo", + "requiredWithRequiredProperty3": "" + }, + "failures": [ + {"reason": "Missing required argument. \"required_with_property\": all of $required_with_property,required_with_property2$ must be specified. Examine values at 'exres.requiredWithProperty'."}, + {"reason": "Missing required argument. \"required_with_required_property\": all of $required_with_nonrequired_property,required_with_required_property$ must be specified. Examine values at 'exres.requiredWithRequiredProperty'."}, + {"reason": "Missing required argument. \"required_with_required_property2\": all of $required_with_required_property2,required_with_required_property3$ must be specified. Examine values at 'exres.requiredWithRequiredProperty2'."}, + {"reason": "Missing required argument. The argument \"required_with_required_property3\" is required, but no definition was found.. Examine values at 'exres.requiredWithRequiredProperty3'."} + ] + } + }`, "$", "`")) + }) +} diff --git a/pkg/tfbridge/schema.go b/pkg/tfbridge/schema.go index 823488d47..bb5c50ed1 100644 --- a/pkg/tfbridge/schema.go +++ b/pkg/tfbridge/schema.go @@ -292,14 +292,15 @@ type conversionContext struct { ComputeDefaultOptions ComputeDefaultOptions ProviderConfig resource.PropertyMap ApplyDefaults bool + ApplyTFDefaults bool Assets AssetTable } -func MakeTerraformInputs( +func makeTerraformInputsHelper( ctx context.Context, instance *PulumiResource, config resource.PropertyMap, olds, news resource.PropertyMap, tfs shim.SchemaMap, ps map[string]*SchemaInfo, + applyTFDefaults bool, ) (map[string]interface{}, AssetTable, error) { - cdOptions := ComputeDefaultOptions{} if instance != nil { cdOptions = ComputeDefaultOptions{ @@ -315,6 +316,7 @@ func MakeTerraformInputs( ComputeDefaultOptions: cdOptions, ProviderConfig: config, ApplyDefaults: true, + ApplyTFDefaults: applyTFDefaults, Assets: AssetTable{}, } inputs, err := cctx.makeTerraformInputs(olds, news, tfs, ps) @@ -324,6 +326,20 @@ func MakeTerraformInputs( return inputs, cctx.Assets, err } +func MakeTerraformInputs( + ctx context.Context, instance *PulumiResource, config resource.PropertyMap, + olds, news resource.PropertyMap, tfs shim.SchemaMap, ps map[string]*SchemaInfo, +) (map[string]interface{}, AssetTable, error) { + return makeTerraformInputsHelper(ctx, instance, config, olds, news, tfs, ps, true) +} + +func makeTerraformInputsWithoutTFDefaults( + ctx context.Context, instance *PulumiResource, config resource.PropertyMap, + olds, news resource.PropertyMap, tfs shim.SchemaMap, ps map[string]*SchemaInfo, +) (map[string]interface{}, AssetTable, error) { + return makeTerraformInputsHelper(ctx, instance, config, olds, news, tfs, ps, false) +} + // makeTerraformInput takes a single property plus custom schema info and does whatever is necessary // to prepare it for use by Terraform. Note that this function may have side effects, for instance // if it is necessary to spill an asset to disk in order to create a name out of it. Please take @@ -853,7 +869,7 @@ func (ctx *conversionContext) applyDefaults( } // Next, populate defaults from the Terraform schema. - if tfs != nil { + if tfs != nil && ctx.ApplyTFDefaults { var valueErr error tfs.Range(func(name string, sch shim.Schema) bool { if sch.Removed() != "" { diff --git a/pkg/tfbridge/schema_test.go b/pkg/tfbridge/schema_test.go index f11b762d5..cbd9ef136 100644 --- a/pkg/tfbridge/schema_test.go +++ b/pkg/tfbridge/schema_test.go @@ -56,8 +56,9 @@ func makeTerraformInputsWithDefaults(olds, news resource.PropertyMap, tfs shim.SchemaMap, ps map[string]*SchemaInfo, ) (map[string]interface{}, AssetTable, error) { ctx := &conversionContext{ - Assets: AssetTable{}, - ApplyDefaults: true, + Assets: AssetTable{}, + ApplyDefaults: true, + ApplyTFDefaults: true, } inputs, err := ctx.makeTerraformInputs(olds, news, tfs, ps) if err != nil { diff --git a/testing/x/json_match.go b/testing/x/json_match.go index 58daa52de..7d9a01cec 100644 --- a/testing/x/json_match.go +++ b/testing/x/json_match.go @@ -18,12 +18,25 @@ import ( "encoding/json" "fmt" "sort" + "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +type jsonMatchOptions struct { + UnorderedArrayPaths map[string]bool +} + +type JSONMatchOption func(*jsonMatchOptions) + +func WithUnorderedArrayPaths(unorderedArrayPaths map[string]bool) JSONMatchOption { + return func(opts *jsonMatchOptions) { + opts.UnorderedArrayPaths = unorderedArrayPaths + } +} + // Assert that a given JSON document structurally matches a pattern. // // The pattern language supports the following constructs: @@ -36,10 +49,15 @@ func AssertJSONMatchesPattern( t *testing.T, expectedPattern json.RawMessage, actual json.RawMessage, + opts ...JSONMatchOption, ) { - var p, a interface{} + options := jsonMatchOptions{} + for _, opt := range opts { + opt(&options) + } + if err := json.Unmarshal(expectedPattern, &p); err != nil { require.NoError(t, err) } @@ -77,6 +95,16 @@ func AssertJSONMatchesPattern( t.Errorf("[%s]: expected an array of length %d, but got %s", path, len(pp), prettyJSON(t, a)) } + + if options.UnorderedArrayPaths[path] { + sort.SliceStable(aa, func(i, j int) bool { + return strings.Compare( + fmt.Sprintf("%v", aa[i]), + fmt.Sprintf("%v", aa[j]), + ) < 0 + }) + } + for i, pv := range pp { av := aa[i] match(fmt.Sprintf("%s[%d]", path, i), pv, av) diff --git a/testing/x/json_match_test.go b/testing/x/json_match_test.go index e637a2b98..bedfdf02c 100644 --- a/testing/x/json_match_test.go +++ b/testing/x/json_match_test.go @@ -25,4 +25,21 @@ func TestJsonMatch(t *testing.T) { AssertJSONMatchesPattern(t, []byte(`{"\\": "*"}`), []byte(`"*"`)) AssertJSONMatchesPattern(t, []byte(`[1, "*", 3]`), []byte(`[1, 2, 3]`)) AssertJSONMatchesPattern(t, []byte(`{"foo": "*", "bar": 3}`), []byte(`{"foo": 1, "bar": 3}`)) + AssertJSONMatchesPattern(t, []byte(`[1, 2, 3]`), []byte(`[1, 3, 2]`), + WithUnorderedArrayPaths(map[string]bool{"#": true})) + AssertJSONMatchesPattern(t, + []byte(`[{"key1":"val"}, {"key2":"val"}]`), + []byte(`[{"key2":"val"}, {"key1":"val"}]`), + WithUnorderedArrayPaths(map[string]bool{"#": true}), + ) + AssertJSONMatchesPattern(t, + []byte(`[{"key":"val1"}, {"key":"val2"}]`), + []byte(`[{"key":"val2"}, {"key":"val1"}]`), + WithUnorderedArrayPaths(map[string]bool{"#": true}), + ) + AssertJSONMatchesPattern(t, + []byte(`{"arr":[{"key":"val1"}, {"key":"val2"}]}`), + []byte(`{"arr":[{"key":"val2"}, {"key":"val1"}]}`), + WithUnorderedArrayPaths(map[string]bool{`#["arr"]`: true}), + ) } diff --git a/testing/x/replay.go b/testing/x/replay.go index ca029d45f..7bf361f40 100644 --- a/testing/x/replay.go +++ b/testing/x/replay.go @@ -191,7 +191,7 @@ func replay[Req protoreflect.ProtoMessage, Resp protoreflect.ProtoMessage]( var expected, actual json.RawMessage = entry.Response, bytes - AssertJSONMatchesPattern(t, expected, actual) + AssertJSONMatchesPattern(t, expected, actual, WithUnorderedArrayPaths(map[string]bool{`#["failures"]`: true})) } // ReplayFile executes ReplaySequence on all pulumirpc.ResourceProvider events found in the file produced with