Consider Multiple Value Checking Interface

[bflad]

terraform-plugin-testing version

1.6.0

Use cases

There are certain patterns of provider acceptance testing which involving checking/comparing multiple values, typically computed resource attribute values from multiple TestStep states, including:

• Value Should Change: For example, ensuring that a computed attribute value is different after resource replacement, to assert that the resource replacement occurred as expected.
• Value Should Not Change: For example, ensuring that a computed attribute value is the same after resource update.
• Integer Value Should Increment: For example, ensuring that a computed attribute value, such as an API-handled resource-wide version number, increments after resource update/replacement. (Although now that plan checks exist for known values, this could be considered moot since checking that the resource generated the incremented known value during planning is just as valid/correct since Terraform will error if there is a mismatch after apply.)

When provider developers encounter this situation, there is not really a native/prescriptive way to handle the situation. It would be nice if at all possible to encourage developers towards more supported/prescriptive testing patterns. Having those patterns be baked into the provider acceptance testing API and extensible would better position us to document what developers should do or try in these situations, rather than potentially point at example code that may differ between providers and require extra explanation/re-familiarization each time.

Attempted solutions

Overload the abstraction of the StateCheckFunc-based checking (TestStep.Checks) to extract API object types or resource attribute state values and potentially another function to implement value comparisons. Since there is no a canonical way to extract values out of API/state during each test step, developers are left to figure out this solution on their own. The additional "problem" with this setup is that it then is no longer a "check", even though much of the internal logic is the same as required for a state check to get to a resource attribute value.

From the hashicorp/aws provider, this example extracts API objects for comparison (which should instead be possible via state value comparison):

// definitions

func testAccCheckBucketObjectExists(ctx context.Context, n string, v *s3.GetObjectOutput) resource.TestCheckFunc {
	return func(s *terraform.State) error {
		rs, ok := s.RootModule().Resources[n]
		if !ok {
			return fmt.Errorf("Not Found: %s", n)
		}

		conn := acctest.Provider.Meta().(*conns.AWSClient).S3Client(ctx)

		input := &s3.GetObjectInput{
			Bucket:  aws.String(rs.Primary.Attributes["bucket"]),
			Key:     aws.String(tfs3.SDKv1CompatibleCleanKey(rs.Primary.Attributes["key"])),
			IfMatch: aws.String(rs.Primary.Attributes["etag"]),
		}

		output, err := conn.GetObject(ctx, input)

		if err != nil {
			return err
		}

		*v = *output

		return nil
	}
}

func testAccCheckObjectVersionIDDiffers(first, second *s3.GetObjectOutput) resource.TestCheckFunc {
	return func(s *terraform.State) error {
		if aws.ToString(first.VersionId) == aws.ToString(second.VersionId) {
			return errors.New("S3 Object version IDs are equal")
		}

		return nil
	}
}

func testAccCheckObjectVersionIDEquals(first, second *s3.GetObjectOutput) resource.TestCheckFunc {
	return func(s *terraform.State) error {
		if aws.ToString(first.VersionId) != aws.ToString(second.VersionId) {
			return errors.New("S3 Object version IDs differ")
		}

		return nil
	}
}

// example usage

func TestAccS3BucketObject_updatesWithVersioning(t *testing.T) {
	ctx := acctest.Context(t)
	var originalObj, modifiedObj s3.GetObjectOutput
	resourceName := "aws_s3_bucket_object.object"
	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)

	sourceInitial := testAccObjectCreateTempFile(t, "initial versioned object state")
	defer os.Remove(sourceInitial)
	sourceModified := testAccObjectCreateTempFile(t, "modified versioned object")
	defer os.Remove(sourceInitial)

	resource.ParallelTest(t, resource.TestCase{
		PreCheck:                 func() { acctest.PreCheck(ctx, t) },
		ErrorCheck:               acctest.ErrorCheck(t, names.S3ServiceID),
		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
		CheckDestroy:             testAccCheckBucketObjectDestroy(ctx),
		Steps: []resource.TestStep{
			{
				Config: testAccBucketObjectConfig_updateable(rName, true, sourceInitial),
				Check: resource.ComposeTestCheckFunc(
					testAccCheckBucketObjectExists(ctx, resourceName, &originalObj),
					testAccCheckObjectBody(&originalObj, "initial versioned object state"),
					resource.TestCheckResourceAttr(resourceName, "etag", "cee4407fa91906284e2a5e5e03e86b1b"),
				),
			},
			{
				Config: testAccBucketObjectConfig_updateable(rName, true, sourceModified),
				Check: resource.ComposeTestCheckFunc(
					testAccCheckBucketObjectExists(ctx, resourceName, &modifiedObj),
					testAccCheckObjectBody(&modifiedObj, "modified versioned object"),
					resource.TestCheckResourceAttr(resourceName, "etag", "00b8c73b1b50e7cc932362c7225b8e29"),
					testAccCheckObjectVersionIDDiffers(&modifiedObj, &originalObj),
				),
			},
			// ...
		},
	})
}

From the hashicorp/time provider, for example:

// definitions

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
	}
}

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
	}
}

// example usage

func TestAccTimeStatic_Triggers(t *testing.T) {
	var time1, time2 string
	resourceName := "time_static.test"

	resource.UnitTest(t, resource.TestCase{
		ProtoV5ProviderFactories: protoV5ProviderFactories(),
		CheckDestroy:             nil,
		Steps: []resource.TestStep{
			{
				Config: testAccConfigTimeStaticTriggers1("key1", "value1"),
				Check: resource.ComposeTestCheckFunc(
					// ...
					testExtractResourceAttr(resourceName, "rfc3339", &time1),
					// ...
				),
			},
			// ...
			{
				Config: testAccConfigTimeStaticTriggers1("key1", "value1updated"),
				Check: resource.ComposeTestCheckFunc(
					// ...
					testExtractResourceAttr(resourceName, "rfc3339", &time2),
					testCheckAttributeValuesDiffer(&time1, &time2),
				),
			},
		},
	})
}

Proposal

An idea could be terraform-plugin-testing offering multiple value checking helpers that can work like the following:

• Before the test case starts, the developer initializes the multiple value check
• In each step, have a method on the multiple value check that implements the state check interface and enables developers to point at a resource address and attribute path. That logic could extract the attribute value to include it in the multiple value check.
• (Optional) After the test case is ended, the developer calls a method on the multiple value check that returns an error or not.

By pseudo-code example (naming, interfaces, etc. debatable!):

idValuesDiffer := multivaluecheck.ValuesDiffer()

resource.Test(t, resource.TestCase{
  // ...
  Steps: []resource.TestStep{
    {
      // ...
      ConfigStateChecks: []statecheck.StateCheck{
        idValuesDiffer.AddStateValue(resourceName, tfjsonpath.New("id")),
      },
    },
    {
      // ...
      ConfigStateChecks: []statecheck.StateCheck{
        idValuesDiffer.AddStateValue(resourceName, tfjsonpath.New("id")),
      },
    },
  },
})

// Optional
if err := idValuesDiffer.Check(); err != nil {
  t.Fatal(err)
}

The post test case logic feels optional as this check could be written so the checking logic runs each time a value is "added" (raising an error immediately, on the failing test step, if the added value doesn't conform to the check). The benefit being less logic that needs to be written each time, but the tradeoff being that it could have less implementation error handling (e.g. returning an error if only one value is ever provided) unless the additional method is provided to do so after the acceptance testing should have ran.

The developer is declaring the check as a single unit and leaving the resource address and path information within each step, likely near other declarative checks doing the same, so the interface should feel familiar. The check can save step numbers, resource addresses, attribute paths, etc. associated with each value for error messaging.

It may be possible to further enhance this by having a generic value comparison interface:

// ValueComparer defines an interface that is implemented to run comparison logic on multiple values. Individual
// implementations determine how the comparison is performed (e.g., values differ, values equal).
type ValueComparer interface {
	// CompareValues should assert the given known values against any expectations.
	// Values are always ordered in the order they were added. Use the error
	// return to signal unexpected values or implementation errors.
	CompareValues(values ...any) error

	// String should return a string representation of the comparison.
	String() string
}

// example implementation

type ValuesDiffer struct {}

func (d ValuesDiffer) CompareValues(values ...any) error {
	// logic to ensure all values differ
}

So implementations could look like:

idValuesDiffer := multivaluecheck.New(xxx.ValuesDiffer{})

And provider developers could implement custom value comparison logic. Its not clear if making the check itself an interface would be beneficial or unnecessary abstraction, but that could likely be teased out during further design/implementation.

References

• resource/time_static: Enhance plan output for config-defined rfc3339 values terraform-provider-time#295

[bflad]

For what its worth, the proposed ValueComparer interface could also be something that helps with a re-implementation of TestCheckResourceAttrPair() for state checks as mentioned in #282.