Repository: pulumi/pulumi-terraform-bridge
Pull request #1310 (2023-07-31 11:31:44)

Add resource option PreCheckCallback
Add the ability to override the check behavior for resources.

github-actions[bot]: Diff for [pulumi-azuread](https://github.com/pulumi-bot/pulumi-azuread/compare/6458f6b..9f04d99) with merge commit 539459f266c5f8c3c17b26889c2eacaad7b3a4fb
github-actions[bot]: Diff for [pulumi-random](https://github.com/pulumi-bot/pulumi-random/compare/da03c65..fc79a12) with merge commit 539459f266c5f8c3c17b26889c2eacaad7b3a4fb
t0yv0: https://go.dev/play/p/-eNaTRqOVhu perhaps consider a way to override it without making changes to this codebase? It seems Go language has the facility to do this via embedded structs.
t0yv0: IN addition, FWIW there's PreConfigureCallback machinery that is allowed to modify the data at Pulumi layer before it drops to TF; not sure if that is sufficient for the use case you have.
iwahbe: > https://go.dev/play/p/-eNaTRqOVhu perhaps consider a way to override it without making changes to this codebase? It seems Go language has the facility to do this via embedded structs.

I don't see how this is applicable to this case. Where would this come into play? Which interface would I be hijacking here? 

`PreConfigureCallback` is for provider configuration, not resource configuration. It is not sufficient for my use case.
t0yv0: Ok, my bad you're right about PreConfigureCallback belonging to a different part of the system.

Regarding overriding via Go embedding I guess it can be too tricky to be useful if we rely on calling into Main/ having multiple layers.

So let's discuss making a change to this codebase to support customizing Check. One sec.

t0yv0: Giving it a second read, I think this can be useful to have, I wanted to reach out for something like this in other work. Perhaps let's call it PreCheckCallback? Add some quick tests to make sure it's covered, similar to PreConfigureCallback. 

github-actions[bot]: Diff for [pulumi-random](https://github.com/pulumi-bot/pulumi-random/compare/a29a0b4..dbaef84) with merge commit b97edc475273b7e8c1c2a57ef9636cde8d01abcb
github-actions[bot]: Diff for [pulumi-azuread](https://github.com/pulumi-bot/pulumi-azuread/compare/442d734..62a67e1) with merge commit b97edc475273b7e8c1c2a57ef9636cde8d01abcb
t0yv0: This is great just needs a little more test TLC.
github-actions[bot]: Diff for [pulumi-random](https://github.com/pulumi-bot/pulumi-random/compare/faf4d9b..5746ab5) with merge commit 011045c572b2a7c8024df140c42dbedd810c6c92
github-actions[bot]: Diff for [pulumi-azuread](https://github.com/pulumi-bot/pulumi-azuread/compare/94fa943..2d14d89) with merge commit 011045c572b2a7c8024df140c42dbedd810c6c92
github-actions[bot]: Diff for [pulumi-azuread](https://github.com/pulumi-bot/pulumi-azuread/compare/6b8d602..fe3f0dc) with merge commit 6266eaa8cd2848c7dc2be600b37940b9b96389f0
github-actions[bot]: Diff for [pulumi-random](https://github.com/pulumi-bot/pulumi-random/compare/29b2ec9..e0eeeec) with merge commit 6266eaa8cd2848c7dc2be600b37940b9b96389f0
github-actions[bot]: Diff for [pulumi-azuread](https://github.com/pulumi-bot/pulumi-azuread/compare/449f72b..5436780) with merge commit ea090d28db9acbac27ee500e74df9101190cc81b
github-actions[bot]: Diff for [pulumi-azuread](https://github.com/pulumi-bot/pulumi-azuread/compare/b57695f..b9c19df) with merge commit 3b96d9e9df68ddae81237963c35c21c8b37d6eff

Patch:
diff --git a/pf/tfbridge/provider_check.go b/pf/tfbridge/provider_check.go
index 03a3049d..811e5fec 100644
--- a/pf/tfbridge/provider_check.go
+++ b/pf/tfbridge/provider_check.go
@@ -46,6 +46,15 @@ func (p *provider) CheckWithContext(
 		return checkedInputs, []plugin.CheckFailure{}, err
 	}
 
+	if info := rh.pulumiResourceInfo; info != nil {
+		if check := info.PreCheckCallback; check != nil {
+			checkedInputs, err := check(ctx, checkedInputs, p.lastKnownProviderConfig.Copy())
+			if err != nil {
+				return checkedInputs, []plugin.CheckFailure{}, err
+			}
+		}
+	}
+
 	// Transform checkedInputs to apply Pulumi-level defaults.
 	news := defaults.ApplyDefaultInfoValues(ctx, defaults.ApplyDefaultInfoValuesArgs{
 		SchemaMap:   rh.schemaOnlyShimResource.Schema(),
diff --git a/pf/tfbridge/provider_configure.go b/pf/tfbridge/provider_configure.go
index 48a63db8..fc506cb2 100644
--- a/pf/tfbridge/provider_configure.go
+++ b/pf/tfbridge/provider_configure.go
@@ -29,6 +29,8 @@ import (
 func (p *provider) ConfigureWithContext(ctx context.Context, inputs resource.PropertyMap) error {
 	ctx = p.initLogging(ctx, p.logSink, "")
 
+	p.lastKnownProviderConfig = inputs
+
 	config, err := convert.EncodePropertyMapToDynamic(p.configEncoder, p.configType, inputs)
 	if err != nil {
 		return fmt.Errorf("cannot encode provider configuration to call ConfigureProvider: %w", err)
diff --git a/pkg/tfbridge/info.go b/pkg/tfbridge/info.go
index f0fa5d13..d11b280a 100644
--- a/pkg/tfbridge/info.go
+++ b/pkg/tfbridge/info.go
@@ -292,8 +292,15 @@ type ResourceInfo struct {
 	// Optional hook to run before upgrading the state. TODO[pulumi/pulumi-terraform-bridge#864] this is currently
 	// only supported for Plugin-Framework based providers.
 	PreStateUpgradeHook PreStateUpgradeHook
+
+	// An experimental way to augment the Check function in the Pulumi life cycle.
+	PreCheckCallback PreCheckCallback
 }
 
+type PreCheckCallback = func(
+	ctx context.Context, config resource.PropertyMap, meta resource.PropertyMap,
+) (resource.PropertyMap, error)
+
 // GetTok returns a resource type token
 func (info *ResourceInfo) GetTok() tokens.Token { return tokens.Token(info.Tok) }
 
diff --git a/pkg/tfbridge/provider.go b/pkg/tfbridge/provider.go
index 9655a25b..004dd6eb 100644
--- a/pkg/tfbridge/provider.go
+++ b/pkg/tfbridge/provider.go
@@ -567,6 +567,13 @@ func (p *Provider) Check(ctx context.Context, req *pulumirpc.CheckRequest) (*pul
 		return nil, err
 	}
 
+	if check := res.Schema.PreCheckCallback; check != nil {
+		news, err = check(ctx, news, p.configValues.Copy())
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	// 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


Test patch:
diff --git a/pf/tests/provider_check_test.go b/pf/tests/provider_check_test.go
index 03785358..52964992 100644
--- a/pf/tests/provider_check_test.go
+++ b/pf/tests/provider_check_test.go
@@ -15,14 +15,16 @@
 package tfbridgetests
 
 import (
+	"context"
 	"fmt"
 	"testing"
 
+	"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"
 	pschema "github.com/hashicorp/terraform-plugin-framework/provider/schema"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema"
-
-	"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"
 	"github.com/hashicorp/terraform-plugin-framework/schema/validator"
+	"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
+
 	"github.com/pulumi/pulumi-terraform-bridge/pf/tests/internal/providerbuilder"
 	"github.com/pulumi/pulumi-terraform-bridge/pf/tfbridge"
 	testutils "github.com/pulumi/pulumi-terraform-bridge/testing/x"
@@ -36,6 +38,7 @@ func TestCheck(t *testing.T) {
 		schema      schema.Schema
 		replay      string
 		replayMulti string
+		callback    tfbridge0.PreCheckCallback
 	}
 
 	testCases := []testCase{
@@ -204,6 +207,56 @@ func TestCheck(t *testing.T) {
 			  }
 			]`,
 		},
+		{
+			name: "callback",
+			schema: schema.Schema{
+				Attributes: map[string]schema.Attribute{
+					"id":   schema.StringAttribute{Computed: true},
+					"prop": schema.StringAttribute{Required: true},
+				},
+			},
+			replayMulti: `
+			[
+			  {
+			    "method": "/pulumirpc.ResourceProvider/Configure",
+			    "request": {
+			      "args": {
+				"prop": "global"
+			      },
+			      "variables": {
+				"prop": "global"
+			      },
+			      "acceptSecrets": true,
+			      "acceptResources": true
+			    },
+			    "response": {
+			      "supportsPreview": true,
+			      "acceptResources": true
+			    }
+			  },
+			  {
+			    "method": "/pulumirpc.ResourceProvider/Check",
+			    "request": {
+			      "urn": "urn:pulumi:st::pg::testprovider:index/res:Res::r",
+			      "olds": {},
+			      "news": {},
+			      "randomSeed": "wqZZaHWVfsS1ozo3bdauTfZmjslvWcZpUjn7BzpS79c="
+			    },
+			    "response": {
+			      "inputs": {
+				"prop": "global"
+			      }
+			    }
+			  }
+			]`,
+			callback: func(
+				_ context.Context, config, meta resource.PropertyMap,
+			) (resource.PropertyMap, error) {
+				t.Logf("Meta: %#v", meta)
+				config["prop"] = meta["prop"]
+				return config, nil
+			},
+		},
 	}
 
 	for _, tc := range testCases {
@@ -211,9 +264,15 @@ func TestCheck(t *testing.T) {
 
 		t.Run(tc.name, func(t *testing.T) {
 			testProvider := &providerbuilder.Provider{
-				TypeName:       "testprovider",
-				Version:        "0.0.1",
-				ProviderSchema: pschema.Schema{},
+				TypeName: "testprovider",
+				Version:  "0.0.1",
+				ProviderSchema: pschema.Schema{
+					Attributes: map[string]pschema.Attribute{
+						"prop": pschema.StringAttribute{
+							Optional: true,
+						},
+					},
+				},
 				AllResources: []providerbuilder.Resource{{
 					Name:           "res",
 					ResourceSchema: tc.schema,
@@ -230,6 +289,7 @@ func TestCheck(t *testing.T) {
 						Docs: &tfbridge0.DocInfo{
 							Markdown: []byte("OK"),
 						},
+						PreCheckCallback: tc.callback,
 					},
 				},
 			}
diff --git a/pkg/tfbridge/provider_test.go b/pkg/tfbridge/provider_test.go
index 448f5671..dbe3b448 100644
--- a/pkg/tfbridge/provider_test.go
+++ b/pkg/tfbridge/provider_test.go
@@ -495,25 +495,10 @@ func testCheckFailures(t *testing.T, provider *Provider, typeName tokens.Type) [
 		News: pulumiIns,
 	})
 	assert.NoError(t, err)
-	assert.Len(t, checkResp.Failures, 3)
 	return checkResp.Failures
 }
 
-func TestProviderCheck(t *testing.T) {
-	provider := &Provider{
-		tf:     shimv1.NewProvider(testTFProvider),
-		config: shimv1.NewSchemaMap(testTFProvider.Schema),
-	}
-	provider.resources = map[tokens.Type]Resource{
-		"SecondResource": {
-			TF:     shimv1.NewResource(testTFProvider.ResourcesMap["second_resource"]),
-			TFName: "second_resource",
-			Schema: &ResourceInfo{Tok: "SecondResource"},
-		},
-	}
-
-	failures := testCheckFailures(t, provider, "SecondResource")
-	sort.SliceStable(failures, func(i, j int) bool { return failures[i].Reason < failures[j].Reason })
+func testCheckFailuresV1(t *testing.T, failures []*pulumirpc.CheckFailure) {
 	assert.Equal(t, "\"conflicting_property\": conflicts with conflicting_property2."+
 		" Examine values at 'name.conflictingProperty'.", failures[0].Reason)
 	assert.Equal(t, "", failures[0].Property)
@@ -524,21 +509,7 @@ func TestProviderCheck(t *testing.T) {
 	assert.Equal(t, "", failures[2].Property)
 }
 
-func TestProviderCheckV2(t *testing.T) {
-	provider := &Provider{
-		tf:     shimv2.NewProvider(testTFProviderV2),
-		config: shimv2.NewSchemaMap(testTFProviderV2.Schema),
-	}
-	provider.resources = map[tokens.Type]Resource{
-		"SecondResource": {
-			TF:     shimv2.NewResource(testTFProviderV2.ResourcesMap["second_resource"]),
-			TFName: "second_resource",
-			Schema: &ResourceInfo{Tok: "SecondResource"},
-		},
-	}
-
-	failures := testCheckFailures(t, provider, "SecondResource")
-	sort.SliceStable(failures, func(i, j int) bool { return failures[i].Reason < failures[j].Reason })
+func testCheckFailuresV2(t *testing.T, failures []*pulumirpc.CheckFailure) {
 	assert.Equal(t, "Conflicting configuration arguments: \"conflicting_property\": conflicts with "+
 		"conflicting_property2. Examine values at 'name.conflictingProperty'.", failures[0].Reason)
 	assert.Equal(t, "", failures[0].Property)
@@ -550,6 +521,107 @@ func TestProviderCheckV2(t *testing.T) {
 	assert.Equal(t, "", failures[2].Property)
 }
 
+func TestProviderCheck(t *testing.T) {
+	testFailures := map[string]func(*testing.T, []*pulumirpc.CheckFailure){
+		"v1": testCheckFailuresV1,
+		"v2": testCheckFailuresV2,
+	}
+	for _, f := range factories {
+		provider := &Provider{
+			tf:     f.NewTestProvider(),
+			config: f.NewTestProvider().Schema(),
+		}
+
+		provider.resources = map[tokens.Type]Resource{
+			"SecondResource": {
+				TF:     provider.tf.ResourcesMap().Get("second_resource"),
+				TFName: "second_resource",
+				Schema: &ResourceInfo{Tok: "SecondResource"},
+			},
+		}
+
+		t.Run(f.SDKVersion(), func(t *testing.T) {
+			t.Run("failures", func(t *testing.T) {
+				failures := testCheckFailures(t, provider, "SecondResource")
+				assert.Len(t, failures, 3)
+				sort.SliceStable(failures, func(i, j int) bool {
+					return failures[i].Reason < failures[j].Reason
+				})
+				testFailures[f.SDKVersion()](t, failures)
+			})
+		})
+	}
+}
+
+func TestCheckCallback(t *testing.T) {
+	t.Parallel()
+	test := func(t *testing.T, p *Provider) {
+		testutils.ReplaySequence(t, p, `
+			[
+			  {
+			    "method": "/pulumirpc.ResourceProvider/Configure",
+			    "request": {
+			      "args": {
+				"prop": "global"
+			      },
+			      "variables": {
+				"prop": "global"
+			      }
+			    },
+			    "response": {
+			      "supportsPreview": true
+			    }
+			  },
+			  {
+			    "method": "/pulumirpc.ResourceProvider/Check",
+			    "request": {
+			      "urn": "urn:pulumi:st::pg::testprovider:index/res:Res::r",
+			      "olds": {},
+			      "news": {},
+			      "randomSeed": "wqZZaHWVfsS1ozo3bdauTfZmjslvWcZpUjn7BzpS79c="
+			    },
+			    "response": {
+			      "inputs": {
+				"__defaults": [],
+				"arrayPropertyValues": ["global"]
+			      }
+			    }
+			  }
+			]`)
+	}
+
+	callback := func(
+		_ context.Context, config, meta resource.PropertyMap,
+	) (resource.PropertyMap, error) {
+		config["arrayPropertyValues"] = resource.NewArrayProperty(
+			[]resource.PropertyValue{meta["prop"]},
+		)
+		return config, nil
+	}
+
+	for _, f := range factories {
+		f := f
+		t.Run(f.SDKVersion(), func(t *testing.T) {
+			t.Parallel()
+			p := &Provider{
+				tf:     f.NewTestProvider(),
+				config: f.NewTestProvider().Schema(),
+			}
+			p.resources = map[tokens.Type]Resource{
+				"testprovider:index/res:Res": {
+					TF:     p.tf.ResourcesMap().Get("example_resource"),
+					TFName: "example_resource",
+					Schema: &ResourceInfo{
+						Tok:              "testprovider:index/res:Res",
+						PreCheckCallback: callback,
+					},
+				},
+			}
+			test(t, p)
+		})
+	}
+}
+
 func testProviderRead(t *testing.T, provider *Provider, typeName tokens.Type, checkRawConfig bool) {
 	urn := resource.NewURN("stack", "project", "", typeName, "name")
 	props, err := structpb.NewStruct(map[string]interface{}{