Fix custom timeout handling under PlanResourceChange (#2390)

Under PlanResourceChange the way custom timeout option is handled is not
properly communicating the value to the underlying TF provider. This is
now fixed and the existing test made to pass.

See also: https://www.pulumi.com/docs/concepts/options/customtimeouts/

Fixes #2386

New warnings are introduced for the case when a user tries to set a
customTimeouts option on a resource that does not support it. The
customTiemeouts in this case are a no-op. I believe this should be a
warning to guide the user but we might also consider downgrading
severity to an INFO or debug level message.

---------

Co-authored-by: VenelinMartinov <[email protected]>

pf/tests/internal/pftfcheck/pftf_test.go

@@ -35,8 +35,10 @@ output "s_val" {
 }
 `)
 
-	plan := driver.Plan(t)
-	driver.Apply(t, plan)
+	plan, err := driver.Plan(t)
+	require.NoError(t, err)
+	err = driver.Apply(t, plan)
+	require.NoError(t, err)
 
 	require.Equal(t, "hello", driver.GetOutput(t, "s_val"))
 }
@@ -68,8 +70,10 @@ output "s_val" {
 }
 `)
 
-	plan := driver.Plan(t)
-	driver.Apply(t, plan)
+	plan, err := driver.Plan(t)
+	require.NoError(t, err)
+	err = driver.Apply(t, plan)
+	require.NoError(t, err)
 
 	require.Equal(t, "Default val", driver.GetOutput(t, "s_val"))
 }

pkg/tests/cross-tests/tf_driver.go

@@ -78,8 +78,10 @@ func (d *TfResDriver) writePlanApply(
 		t.Logf("empty config file")
 		d.driver.Write(t, "")
 	}
-	plan := d.driver.Plan(t)
-	d.driver.Apply(t, plan)
+	plan, err := d.driver.Plan(t)
+	require.NoError(t, err)
+	err = d.driver.Apply(t, plan)
+	require.NoError(t, err)
 	return plan
 }
 

pkg/tests/schema_pulumi_test.go

@@ -8,6 +8,7 @@ import (
 	"os"
 	"path/filepath"
 	"testing"
+	"time"
 
 	"github.com/hashicorp/go-cty/cty"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
@@ -1618,7 +1619,8 @@ func TestConfigureCrossTest(t *testing.T) {
 
 		tfdriver := tfcheck.NewTfDriver(t, t.TempDir(), "prov", tfp)
 		tfdriver.Write(t, tfProgram)
-		tfdriver.Plan(t)
+		_, err := tfdriver.Plan(t)
+		require.NoError(t, err)
 		require.NotNil(t, tfRd)
 		require.Nil(t, puRd)
 
@@ -2941,7 +2943,7 @@ runtime: yaml
 			} else {
 				assert.NotContains(t, imp.Stdout, "One or more imported inputs failed to validate")
 
-				f, err := os.OpenFile(filepath.Join(pt.CurrentStack().Workspace().WorkDir(), "Pulumi.yaml"), os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0600)
+				f, err := os.OpenFile(filepath.Join(pt.CurrentStack().Workspace().WorkDir(), "Pulumi.yaml"), os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0o600)
 				assert.NoError(t, err)
 				defer f.Close()
 				_, err = f.WriteString(string(contents))
@@ -2953,3 +2955,138 @@ runtime: yaml
 		})
 	}
 }
+
+func TestCreateCustomTimeoutsCrossTest(t *testing.T) {
+	test := func(
+		t *testing.T,
+		schemaCreateTimeout *time.Duration,
+		programTimeout *string,
+		expected time.Duration,
+		ExpectFail bool,
+	) {
+		var pulumiCapturedTimeout *time.Duration
+		var tfCapturedTimeout *time.Duration
+		prov := &schema.Provider{
+			ResourcesMap: map[string]*schema.Resource{
+				"prov_test": {
+					Schema: map[string]*schema.Schema{
+						"prop": {
+							Type:     schema.TypeString,
+							Optional: true,
+						},
+					},
+					CreateContext: func(ctx context.Context, rd *schema.ResourceData, i interface{}) diag.Diagnostics {
+						t := rd.Timeout(schema.TimeoutCreate)
+						if pulumiCapturedTimeout == nil {
+							pulumiCapturedTimeout = &t
+						} else {
+							tfCapturedTimeout = &t
+						}
+						rd.SetId("id")
+						return diag.Diagnostics{}
+					},
+					Timeouts: &schema.ResourceTimeout{
+						Create: schemaCreateTimeout,
+					},
+				},
+			},
+		}
+
+		bridgedProvider := pulcheck.BridgedProvider(t, "prov", prov)
+		pulumiTimeout := `""`
+		if programTimeout != nil {
+			pulumiTimeout = fmt.Sprintf(`"%s"`, *programTimeout)
+		}
+
+		tfTimeout := "null"
+		if programTimeout != nil {
+			tfTimeout = fmt.Sprintf(`"%s"`, *programTimeout)
+		}
+
+		program := fmt.Sprintf(`
+name: test
+runtime: yaml
+resources:
+	mainRes:
+		type: prov:Test
+		properties:
+			prop: "val"
+		options:
+			customTimeouts:
+				create: %s
+`, pulumiTimeout)
+
+		pt := pulcheck.PulCheck(t, bridgedProvider, program)
+		pt.Up()
+		// We pass custom timeouts in the program if the resource does not support them.
+
+		require.NotNil(t, pulumiCapturedTimeout)
+		require.Nil(t, tfCapturedTimeout)
+
+		tfProgram := fmt.Sprintf(`
+resource "prov_test" "mainRes" {
+	prop = "val"
+	timeouts {
+		create = %s
+	}
+}`, tfTimeout)
+
+		tfdriver := tfcheck.NewTfDriver(t, t.TempDir(), "prov", prov)
+		tfdriver.Write(t, tfProgram)
+
+		plan, err := tfdriver.Plan(t)
+		if ExpectFail {
+			require.Error(t, err)
+			return
+		}
+		require.NoError(t, err)
+		err = tfdriver.Apply(t, plan)
+		require.NoError(t, err)
+		require.NotNil(t, tfCapturedTimeout)
+
+		assert.Equal(t, *pulumiCapturedTimeout, *tfCapturedTimeout)
+		assert.Equal(t, *pulumiCapturedTimeout, expected)
+	}
+
+	oneSecString := "1s"
+	oneSec := 1 * time.Second
+	// twoSecString := "2s"
+	twoSec := 2 * time.Second
+
+	tests := []struct {
+		name                string
+		schemaCreateTimeout *time.Duration
+		programTimeout      *string
+		expected            time.Duration
+		expectFail          bool
+	}{
+		{
+			"schema specified timeout",
+			&oneSec,
+			nil,
+			oneSec,
+			false,
+		},
+		{
+			"program specified timeout",
+			&twoSec,
+			&oneSecString,
+			oneSec,
+			false,
+		},
+		{
+			"program specified without schema timeout",
+			nil,
+			&oneSecString,
+			oneSec,
+			true,
+		},
+	}
+
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			test(t, tc.schemaCreateTimeout, tc.programTimeout, tc.expected, tc.expectFail)
+		})
+	}
+}

pkg/tests/tfcheck/exec.go

@@ -21,10 +21,9 @@ import (
 	"strings"
 
 	"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tests/pulcheck"
-	"github.com/stretchr/testify/require"
 )
 
-func execCmd(t pulcheck.T, wdir string, environ []string, program string, args ...string) *exec.Cmd {
+func execCmd(t pulcheck.T, wdir string, environ []string, program string, args ...string) (*exec.Cmd, error) {
 	t.Logf("%s %s", program, strings.Join(args, " "))
 	cmd := exec.Command(program, args...)
 	var stdout, stderr bytes.Buffer
@@ -34,7 +33,7 @@ func execCmd(t pulcheck.T, wdir string, environ []string, program string, args .
 	cmd.Stdout = &stdout
 	cmd.Stderr = &stderr
 	err := cmd.Run()
-	require.NoError(t, err, "error from `%s %s`\n\nStdout:\n%s\n\nStderr:\n%s\n\n",
+	t.Logf("error from `%s %s`\n\nStdout:\n%s\n\nStderr:\n%s\n\n",
 		program, strings.Join(args, " "), stdout.String(), stderr.String())
-	return cmd
+	return cmd, err
 }

pkg/tests/tfcheck/tf_test.go

@@ -7,6 +7,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestTfComputed(t *testing.T) {
@@ -39,17 +40,21 @@ resource "test_resource" "test" {
 `,
 	)
 
-	plan := driver.Plan(t)
+	plan, err := driver.Plan(t)
+	require.NoError(t, err)
 	t.Log(driver.Show(t, plan.PlanFile))
-	driver.Apply(t, plan)
+	err = driver.Apply(t, plan)
+	require.NoError(t, err)
 
 	t.Log(driver.GetState(t))
 
-	newPlan := driver.Plan(t)
+	newPlan, err := driver.Plan(t)
+	require.NoError(t, err)
 
 	t.Log(driver.Show(t, plan.PlanFile))
 
-	driver.Apply(t, newPlan)
+	err = driver.Apply(t, newPlan)
+	require.NoError(t, err)
 
 	t.Log(driver.GetState(t))
 }
@@ -100,17 +105,22 @@ resource "test_resource" "test" {
 `,
 	)
 
-	plan := driver.Plan(t)
+	plan, err := driver.Plan(t)
+	require.NoError(t, err)
+
 	t.Log(driver.Show(t, plan.PlanFile))
-	driver.Apply(t, plan)
+	err = driver.Apply(t, plan)
+	require.NoError(t, err)
 
 	t.Log(driver.GetState(t))
 
-	newPlan := driver.Plan(t)
+	newPlan, err := driver.Plan(t)
+	require.NoError(t, err)
 
 	t.Log(driver.Show(t, plan.PlanFile))
 
-	driver.Apply(t, newPlan)
+	err = driver.Apply(t, newPlan)
+	require.NoError(t, err)
 
 	t.Log(driver.GetState(t))
 }
@@ -185,16 +195,22 @@ resource "test_resource" "test" {
     }
 }`
 	driver.Write(t, knownProgram)
-	plan := driver.Plan(t)
+	plan, err := driver.Plan(t)
+	require.NoError(t, err)
+
 	t.Log(driver.Show(t, plan.PlanFile))
 
-	driver.Apply(t, plan)
+	err = driver.Apply(t, plan)
+	require.NoError(t, err)
 	t.Log(driver.GetState(t))
 
 	driver.Write(t, unknownProgram)
-	plan = driver.Plan(t)
+	plan, err = driver.Plan(t)
+	require.NoError(t, err)
+
 	t.Log(driver.Show(t, plan.PlanFile))
 
-	driver.Apply(t, plan)
+	err = driver.Apply(t, plan)
+	require.NoError(t, err)
 	t.Log(driver.GetState(t))
 }

pkg/tests/tfcheck/tfcheck.go

@@ -119,30 +119,36 @@ func (d *TfDriver) Write(t pulcheck.T, program string) {
 	require.NoErrorf(t, err, "writing test.tf")
 }
 
-func (d *TfDriver) Plan(t pulcheck.T) *TfPlan {
+func (d *TfDriver) Plan(t pulcheck.T) (*TfPlan, error) {
 	planFile := filepath.Join(d.cwd, "test.tfplan")
 	env := []string{d.formatReattachEnvVar()}
 	tfCmd := getTFCommand()
-	execCmd(t, d.cwd, env, tfCmd, "plan", "-refresh=false", "-out", planFile)
-	cmd := execCmd(t, d.cwd, env, tfCmd, "show", "-json", planFile)
+	_, err := execCmd(t, d.cwd, env, tfCmd, "plan", "-refresh=false", "-out", planFile)
+	if err != nil {
+		return nil, err
+	}
+	cmd, err := execCmd(t, d.cwd, env, tfCmd, "show", "-json", planFile)
+	require.NoError(t, err)
 	tp := TfPlan{PlanFile: planFile}
-	err := json.Unmarshal(cmd.Stdout.(*bytes.Buffer).Bytes(), &tp.RawPlan)
+	err = json.Unmarshal(cmd.Stdout.(*bytes.Buffer).Bytes(), &tp.RawPlan)
 	require.NoErrorf(t, err, "failed to unmarshal terraform plan")
-	return &tp
+	return &tp, nil
 }
 
-func (d *TfDriver) Apply(t pulcheck.T, plan *TfPlan) {
+func (d *TfDriver) Apply(t pulcheck.T, plan *TfPlan) error {
 	tfCmd := getTFCommand()
-	execCmd(t, d.cwd, []string{d.formatReattachEnvVar()},
+	_, err := execCmd(t, d.cwd, []string{d.formatReattachEnvVar()},
 		tfCmd, "apply", "-auto-approve", "-refresh=false", plan.PlanFile)
+	return err
 }
 
 func (d *TfDriver) Show(t pulcheck.T, planFile string) string {
 	tfCmd := getTFCommand()
-	cmd := execCmd(t, d.cwd, []string{d.formatReattachEnvVar()}, tfCmd, "show", "-json", planFile)
+	cmd, err := execCmd(t, d.cwd, []string{d.formatReattachEnvVar()}, tfCmd, "show", "-json", planFile)
+	require.NoError(t, err)
 	res := cmd.Stdout.(*bytes.Buffer)
 	buf := bytes.NewBuffer(nil)
-	err := json.Indent(buf, res.Bytes(), "", "    ")
+	err = json.Indent(buf, res.Bytes(), "", "    ")
 	require.NoError(t, err)
 	return buf.String()
 }
@@ -158,7 +164,8 @@ func (d *TfDriver) GetState(t pulcheck.T) string {
 
 func (d *TfDriver) GetOutput(t pulcheck.T, outputName string) string {
 	tfCmd := getTFCommand()
-	cmd := execCmd(t, d.cwd, []string{d.formatReattachEnvVar()}, tfCmd, "output", outputName)
+	cmd, err := execCmd(t, d.cwd, []string{d.formatReattachEnvVar()}, tfCmd, "output", outputName)
+	require.NoError(t, err)
 	res := cmd.Stdout.(*bytes.Buffer).String()
 	res = strings.TrimSuffix(res, "\n")
 	res = strings.Trim(res, "\"")