resource/time_static: Enhance plan output for config-defined rfc3339 values

.changes/unreleased/ENHANCEMENTS-20240223-162424.yaml

@@ -0,0 +1,6 @@
+kind: ENHANCEMENTS
+body: 'resource/time_static: If the `rfc3339` value is set in config and known at
+  plan-time, all other attributes will also be known during plan.'
+time: 2024-02-23T16:24:24.067014-05:00
+custom:
+  Issue: "255"

go.mod

@@ -5,6 +5,8 @@ go 1.21
 toolchain go1.21.6
 
 require (
+	github.com/google/go-cmp v0.6.0
+	github.com/hashicorp/terraform-json v0.22.1
 	github.com/hashicorp/terraform-plugin-framework v1.10.0
 	github.com/hashicorp/terraform-plugin-framework-timetypes v0.4.0
 	github.com/hashicorp/terraform-plugin-framework-validators v0.13.0
@@ -20,7 +22,6 @@ require (
 	github.com/cloudflare/circl v1.3.7 // indirect
 	github.com/fatih/color v1.16.0 // indirect
 	github.com/golang/protobuf v1.5.4 // indirect
-	github.com/google/go-cmp v0.6.0 // indirect
 	github.com/hashicorp/errwrap v1.1.0 // indirect
 	github.com/hashicorp/go-checkpoint v0.5.0 // indirect
 	github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
@@ -34,7 +35,6 @@ require (
 	github.com/hashicorp/hcl/v2 v2.21.0 // indirect
 	github.com/hashicorp/logutils v1.0.0 // indirect
 	github.com/hashicorp/terraform-exec v0.21.0 // indirect
-	github.com/hashicorp/terraform-json v0.22.1 // indirect
 	github.com/hashicorp/terraform-plugin-sdk/v2 v2.34.0 // indirect
 	github.com/hashicorp/terraform-registry-address v0.2.3 // indirect
 	github.com/hashicorp/terraform-svchost v0.1.1 // indirect

internal/provider/provider_test.go

@@ -4,15 +4,11 @@
 package provider
 
 import (
-	"fmt"
-	"time"
-
 	"github.com/hashicorp/terraform-plugin-framework/providerserver"
 
 	"github.com/hashicorp/terraform-plugin-go/tfprotov5"
 
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
-	"github.com/hashicorp/terraform-plugin-testing/terraform"
 )
 
 func protoV5ProviderFactories() map[string]func() (tfprotov5.ProviderServer, error) {
@@ -29,62 +25,3 @@ func providerVersion080() map[string]resource.ExternalProvider {
 		},
 	}
 }
-
-func testCheckAttributeValuesDiffer(i *string, j *string) resource.TestCheckFunc {
-	return func(s *terraform.State) error {
-		if testStringValue(i) == testStringValue(j) {
-			return fmt.Errorf("attribute values are the same")
-		}
-
-		return nil
-	}
-}
-
-func testCheckAttributeValuesSame(i *string, j *string) resource.TestCheckFunc {
-	return func(s *terraform.State) error {
-		if testStringValue(i) != testStringValue(j) {
-			return fmt.Errorf("attribute values are different")
-		}
-
-		return nil
-	}
-}
-
-//nolint:unparam
-func testExtractResourceAttr(resourceName string, attributeName string, attributeValue *string) resource.TestCheckFunc {
-	return func(s *terraform.State) error {
-		rs, ok := s.RootModule().Resources[resourceName]
-
-		if !ok {
-			return fmt.Errorf("resource name %s not found in state", resourceName)
-		}
-
-		attrValue, ok := rs.Primary.Attributes[attributeName]
-
-		if !ok {
-			return fmt.Errorf("attribute %s not found in resource %s state", attributeName, resourceName)
-		}
-
-		*attributeValue = attrValue
-
-		return nil
-	}
-}
-
-// Certain testing requires time differences that are too fast for unit testing.
-// Sleeping for a second or two seems pragmatic in our testing.
-func testSleep(seconds int) resource.TestCheckFunc {
-	return func(s *terraform.State) error {
-		time.Sleep(time.Duration(seconds) * time.Second)
-
-		return nil
-	}
-}
-
-func testStringValue(sPtr *string) string {
-	if sPtr == nil {
-		return ""
-	}
-
-	return *sPtr
-}

internal/provider/resource_time_sleep_test.go

@@ -14,6 +14,10 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/tfsdk"
 	"github.com/hashicorp/terraform-plugin-go/tftypes"
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
+	"github.com/hashicorp/terraform-plugin-testing/knownvalue"
+	"github.com/hashicorp/terraform-plugin-testing/statecheck"
+	"github.com/hashicorp/terraform-plugin-testing/tfjsonpath"
+	"github.com/hashicorp/terraform-provider-time/internal/timetesting"
 )
 
 // Since the acceptance testing framework can introduce uncontrollable time delays,
@@ -155,20 +159,24 @@ func TestResourceTimeSleepDelete(t *testing.T) {
 }
 
 func TestAccTimeSleep_CreateDuration(t *testing.T) {
-	var time1, time2 string
 	resourceName := "time_sleep.test"
 
+	// These ID comparisons can eventually be replaced by the multiple value checks once released
+	// in terraform-plugin-testing: https://github.com/hashicorp/terraform-plugin-testing/issues/295
+	captureTimeState1 := timetesting.NewExtractState(resourceName, tfjsonpath.New("id"))
+	captureTimeState2 := timetesting.NewExtractState(resourceName, tfjsonpath.New("id"))
+
 	resource.UnitTest(t, resource.TestCase{
 		ProtoV5ProviderFactories: protoV5ProviderFactories(),
 		CheckDestroy:             nil,
 		Steps: []resource.TestStep{
 			{
 				Config: testAccConfigTimeSleepCreateDuration("1ms"),
-				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttr(resourceName, "create_duration", "1ms"),
-					resource.TestCheckResourceAttrSet(resourceName, "id"),
-					testExtractResourceAttr(resourceName, "id", &time1),
-				),
+				ConfigStateChecks: []statecheck.StateCheck{
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("create_duration"), knownvalue.StringExact("1ms")),
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("id"), knownvalue.NotNull()),
+					captureTimeState1,
+				},
 			},
 			// This test may work in local execution but typically does not work in CI because of its reliance
 			// on the current time stamp in the ID. We will also need to revisit this test later once TF core allows
@@ -181,32 +189,40 @@ func TestAccTimeSleep_CreateDuration(t *testing.T) {
 			//},
 			{
 				Config: testAccConfigTimeSleepCreateDuration("2ms"),
-				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttr(resourceName, "create_duration", "2ms"),
-					resource.TestCheckResourceAttrSet(resourceName, "id"),
-					testExtractResourceAttr(resourceName, "id", &time2),
-					testCheckAttributeValuesSame(&time1, &time2),
-				),
+				ConfigStateChecks: []statecheck.StateCheck{
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("create_duration"), knownvalue.StringExact("2ms")),
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("id"), knownvalue.NotNull()),
+					captureTimeState2,
+				},
 			},
 		},
 	})
+
+	// Ensure the id time value is different due to the sleep
+	if captureTimeState1.Value == captureTimeState2.Value {
+		t.Fatal("attribute values are the same")
+	}
 }
 
 func TestAccTimeSleep_DestroyDuration(t *testing.T) {
-	var time1, time2 string
 	resourceName := "time_sleep.test"
 
+	// These ID comparisons can eventually be replaced by the multiple value checks once released
+	// in terraform-plugin-testing: https://github.com/hashicorp/terraform-plugin-testing/issues/295
+	captureTimeState1 := timetesting.NewExtractState(resourceName, tfjsonpath.New("id"))
+	captureTimeState2 := timetesting.NewExtractState(resourceName, tfjsonpath.New("id"))
+
 	resource.UnitTest(t, resource.TestCase{
 		ProtoV5ProviderFactories: protoV5ProviderFactories(),
 		CheckDestroy:             nil,
 		Steps: []resource.TestStep{
 			{
 				Config: testAccConfigTimeSleepDestroyDuration("1ms"),
-				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttr(resourceName, "destroy_duration", "1ms"),
-					resource.TestCheckResourceAttrSet(resourceName, "id"),
-					testExtractResourceAttr(resourceName, "id", &time1),
-				),
+				ConfigStateChecks: []statecheck.StateCheck{
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("destroy_duration"), knownvalue.StringExact("1ms")),
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("id"), knownvalue.NotNull()),
+					captureTimeState1,
+				},
 			},
 			// This test may work in local execution but typically does not work in CI because of its reliance
 			// on the current time stamp in the ID. We will also need to revisit this test later once TF core allows
@@ -219,34 +235,42 @@ func TestAccTimeSleep_DestroyDuration(t *testing.T) {
 			//},
 			{
 				Config: testAccConfigTimeSleepDestroyDuration("2ms"),
-				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttr(resourceName, "destroy_duration", "2ms"),
-					resource.TestCheckResourceAttrSet(resourceName, "id"),
-					testExtractResourceAttr(resourceName, "id", &time2),
-					testCheckAttributeValuesSame(&time1, &time2),
-				),
+				ConfigStateChecks: []statecheck.StateCheck{
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("destroy_duration"), knownvalue.StringExact("2ms")),
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("id"), knownvalue.NotNull()),
+					captureTimeState2,
+				},
 			},
 		},
 	})
+
+	// Ensure the id time value is different due to the sleep
+	if captureTimeState1.Value == captureTimeState2.Value {
+		t.Fatal("attribute values are the same")
+	}
 }
 
 func TestAccTimeSleep_Triggers(t *testing.T) {
-	var time1, time2 string
 	resourceName := "time_sleep.test"
 
+	// These ID comparisons can eventually be replaced by the multiple value checks once released
+	// in terraform-plugin-testing: https://github.com/hashicorp/terraform-plugin-testing/issues/295
+	captureTimeState1 := timetesting.NewExtractState(resourceName, tfjsonpath.New("id"))
+	captureTimeState2 := timetesting.NewExtractState(resourceName, tfjsonpath.New("id"))
+
 	resource.UnitTest(t, resource.TestCase{
 		ProtoV5ProviderFactories: protoV5ProviderFactories(),
 		CheckDestroy:             nil,
 		Steps: []resource.TestStep{
 			{
 				Config: testAccConfigTimeSleepTriggers1("key1", "value1"),
-				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttr(resourceName, "triggers.%", "1"),
-					resource.TestCheckResourceAttr(resourceName, "triggers.key1", "value1"),
-					resource.TestCheckResourceAttrSet(resourceName, "id"),
-					resource.TestCheckResourceAttrSet(resourceName, "create_duration"),
-					testExtractResourceAttr(resourceName, "id", &time1),
-				),
+				ConfigStateChecks: []statecheck.StateCheck{
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("triggers"), knownvalue.MapSizeExact(1)),
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("triggers").AtMapKey("key1"), knownvalue.StringExact("value1")),
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("id"), knownvalue.NotNull()),
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("create_duration"), knownvalue.NotNull()),
+					captureTimeState1,
+				},
 			},
 			// This test may work in local execution but typically does not work in CI because of its reliance
 			// on the current time stamp in the ID. We will also need to revisit this test later once TF core allows
@@ -260,17 +284,21 @@ func TestAccTimeSleep_Triggers(t *testing.T) {
 			//},
 			{
 				Config: testAccConfigTimeSleepTriggers1("key1", "value1updated"),
-				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttr(resourceName, "triggers.%", "1"),
-					resource.TestCheckResourceAttr(resourceName, "triggers.key1", "value1updated"),
-					resource.TestCheckResourceAttrSet(resourceName, "id"),
-					resource.TestCheckResourceAttrSet(resourceName, "create_duration"),
-					testExtractResourceAttr(resourceName, "id", &time2),
-					testCheckAttributeValuesDiffer(&time1, &time2),
-				),
+				ConfigStateChecks: []statecheck.StateCheck{
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("triggers"), knownvalue.MapSizeExact(1)),
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("triggers").AtMapKey("key1"), knownvalue.StringExact("value1updated")),
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("id"), knownvalue.NotNull()),
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("create_duration"), knownvalue.NotNull()),
+					captureTimeState2,
+				},
 			},
 		},
 	})
+
+	// Ensure the id time value is different due to the sleep
+	if captureTimeState1.Value == captureTimeState2.Value {
+		t.Fatal("attribute values are the same")
+	}
 }
 
 func TestAccTimeSleep_Upgrade(t *testing.T) {
@@ -282,10 +310,10 @@ func TestAccTimeSleep_Upgrade(t *testing.T) {
 			{
 				ExternalProviders: providerVersion080(),
 				Config:            testAccConfigTimeSleepCreateDuration("1ms"),
-				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttr(resourceName, "create_duration", "1ms"),
-					resource.TestCheckResourceAttrSet(resourceName, "id"),
-				),
+				ConfigStateChecks: []statecheck.StateCheck{
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("create_duration"), knownvalue.StringExact("1ms")),
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("id"), knownvalue.NotNull()),
+				},
 			},
 			{
 				ProtoV5ProviderFactories: protoV5ProviderFactories(),
@@ -295,10 +323,10 @@ func TestAccTimeSleep_Upgrade(t *testing.T) {
 			{
 				ProtoV5ProviderFactories: protoV5ProviderFactories(),
 				Config:                   testAccConfigTimeSleepCreateDuration("1ms"),
-				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttr(resourceName, "create_duration", "1ms"),
-					resource.TestCheckResourceAttrSet(resourceName, "id"),
-				),
+				ConfigStateChecks: []statecheck.StateCheck{
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("create_duration"), knownvalue.StringExact("1ms")),
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("id"), knownvalue.NotNull()),
+				},
 			},
 		},
 	})

internal/provider/resource_time_static.go

@@ -20,6 +20,7 @@ import (
 
 var (
 	_ resource.Resource                = (*timeStaticResource)(nil)
+	_ resource.ResourceWithModifyPlan  = (*timeStaticResource)(nil)
 	_ resource.ResourceWithImportState = (*timeStaticResource)(nil)
 )
 
@@ -29,6 +30,53 @@ func NewTimeStaticResource() resource.Resource {
 
 type timeStaticResource struct{}
 
+func (t timeStaticResource) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) {
+	// Skip plan modification unless it's a create operation
+	if req.Plan.Raw.IsNull() || !req.State.Raw.IsNull() {
+		return
+	}
+
+	var plan timeStaticModelV0
+
+	resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...)
+	if resp.Diagnostics.HasError() {
+		return
+	}
+
+	// Currently, it is only possible to enhance the plan when the rfc3339 value is defined in configuration (i.e. value is not null and known in plan).
+	//
+	// Terraform calls the PlanResourceChange RPC twice (initial planned state and final planned state) and currently has no mechanism for sharing information between
+	// the initial plan call and final plan call. This means that we can't create a final plan that matches the initial plan using something like time.Now()
+	// which will differ between the two calls and result in a "Provider produced inconsistent final plan" error from Terraform.
+	//
+	// If functionality is introduced in the future that allows us to create consistent final and initial plans, we'd likely want to introduce a new managed resource that
+	// always determines its results at plan time. Changing this resource to adopt that behavior would be a breaking change for practitioners who are relying on the time being
+	// determined at apply time.
+	//
+	// There is no time provider feature request currently for this behavior, but a similar long-standing issue exists on the random provider:
+	// - https://github.com/hashicorp/terraform-provider-random/issues/121
+	if plan.RFC3339.IsNull() || plan.RFC3339.IsUnknown() {
+		return
+	}
+
+	rfc3339, diags := plan.RFC3339.ValueRFC3339Time()
+	resp.Diagnostics.Append(diags...)
+	if resp.Diagnostics.HasError() {
+		return
+	}
+
+	plan.Year = types.Int64Value(int64(rfc3339.Year()))
+	plan.Month = types.Int64Value(int64(rfc3339.Month()))
+	plan.Day = types.Int64Value(int64(rfc3339.Day()))
+	plan.Hour = types.Int64Value(int64(rfc3339.Hour()))
+	plan.Minute = types.Int64Value(int64(rfc3339.Minute()))
+	plan.Second = types.Int64Value(int64(rfc3339.Second()))
+	plan.Unix = types.Int64Value(rfc3339.Unix())
+	plan.ID = plan.RFC3339
+
+	resp.Diagnostics.Append(resp.Plan.Set(ctx, &plan)...)
+}
+
 func (t timeStaticResource) Metadata(ctx context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) {
 	resp.TypeName = req.ProviderTypeName + "_static"
 }
@@ -139,7 +187,7 @@ func (t timeStaticResource) Create(ctx context.Context, req resource.CreateReque
 
 	timestamp := time.Now().UTC()
 
-	if plan.RFC3339.ValueString() != "" {
+	if !plan.RFC3339.IsNull() && !plan.RFC3339.IsUnknown() {
 		rfc3339, diags := plan.RFC3339.ValueRFC3339Time()
 
 		resp.Diagnostics.Append(diags...)