Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Standard Integration Or Reference For Use With Go VCR #190

Open
tgoodsell-tempus opened this issue Sep 21, 2023 · 2 comments
Open

Standard Integration Or Reference For Use With Go VCR #190

tgoodsell-tempus opened this issue Sep 21, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@tgoodsell-tempus
Copy link

terraform-plugin-testing version

latest

Use cases

Running acceptance tests requires access to a live provider which involves making API calls that may or may not cost a lot of money. Additionally, some platforms which offer or have had Terraform providers built, may have certain features or functions locked behind flags or paywall gates that aren't accessible to common users/developers.

Providers, such as terraform-provider-google and terraform-provider-okta, have implemented the use of Go VCR, to allow for cheaper, more efficient, and a more accessible contributor experience. Other providers have implemented other tools as well, some more specific to the particular API/service being modeled in Terraform.

Attempted solutions

This has already been accomplished in a ad-hoc manner in a variety of "similar" but also slightly different ways:

  1. Generally all providers using this logic have a "separate" set of provider init functions used with testing. These functions use some of the Go VCR environment variables to trigger logic to replace the HTTP clients used with the API/SDK the provider is making resources for.
  2. Nearly all situations involve a custom "acceptance test only" version of the Provider structs and override of the Configure function for "framework" and the ConfigureContextFunc to handle this HTTP configuration. Which then are passed into the various ProtoProviderFactories inputs on the the test case. This must be done since each test case will start a new instance of the configuration / providers by convention.
  3. Apart from that, there some level of custom code which really has to be provider / developer specific, related to customizing the VCR recording to strip sensitive fields, replacing/normalizing unique values to the original recorder, dealing with API quirks, etc. Additionally, there is "admin" type decisions, such as how the provider wants to manage the recording and storage of the VCRs, commonly seen "by acceptance test" due to the nature of the config reload, but it can be done in a variety of ways.

For the most part, this works out decently well provided the SDK being used is nice about replacing the HTTP client it uses, as well as good code cleanliness and standards. However, in it's current form, this does require a pretty significant effort and self-motivated desire to get this done.

Proposal

My personal statement would be that the benefits of a VCR/playback based acceptance test system are significant enough that the testing framework for providers should attempt to offer some level of "reduced boilerplate" tools to making the necessary changes during acceptance tests and some level of "encouragement" to include these types of acceptance tests tools in the provider.

My proposal for this would be.

  1. Inside of the "Test Case", there should be some sort of interface to allow us to modify/override the "provider client" AFTER the test case init process of the actual provider objects is done. Something similar to the "access" we get to the client inside of the various CRUD functions where things like pointers to a HTTP client on the given SDK client and other options could be modified. As some of the examples (see my references) show, there will have to be some consideration and guidance given to the developer to ensure the various "goroutines" aren't stepping on each other. Potentially this could be another top level configuration which takes a function making these changes, which is expected to be "goroutine" safe and has certain asks to ensure whatever is changed is also routine safe.
  2. Additionally, in some of the example code and Hashicorp documentation, references to using this interface and what it's intended purposes are (aka things like this). I would also request that the "example" providers and development/testing documentation have some toy examples of this being used in practice (using Go VCR or whatever Hashi / you decide you want to do), and some level of encouragement of this type of pattern when providers are being developed.

References

Google provider: https://github.com/hashicorp/terraform-provider-google/blob/main/google/acctest/vcr_utils.go
Okta provider: https://github.com/okta/terraform-provider-okta/blob/master/okta/provider_test.go

@tgoodsell-tempus tgoodsell-tempus added the enhancement New feature or request label Sep 21, 2023
@bflad
Copy link
Contributor

bflad commented Sep 22, 2023

Hi @tgoodsell-tempus 👋 Thank you for raising this feature request. It would certainly be useful if there was additional documentation about the general pattern of using a mock API client for provider acceptance testing.

To help with the testing code related portion of your proposal it might help to provide a code snippet to highlight what you think can/should be done in this regard. One important thing to note is that newer testing code (ProtoV5ProviderFactories and ProtoV6ProviderFactories fields) intentionally only accept provider servers, while the direct terraform-plugin-sdk provider code fields Providers and ProviderFactories will be deprecated and removed at some point in the future as part of #186.

@tgoodsell-tempus
Copy link
Author

tgoodsell-tempus commented Sep 22, 2023

Hi @tgoodsell-tempus 👋 Thank you for raising this feature request. It would certainly be useful if there was additional documentation about the general pattern of using a mock API client for provider acceptance testing.

To help with the testing code related portion of your proposal it might help to provide a code snippet to highlight what you think can/should be done in this regard. One important thing to note is that newer testing code (ProtoV5ProviderFactories and ProtoV6ProviderFactories fields) intentionally only accept provider servers, while the direct terraform-plugin-sdk provider code fields Providers and ProviderFactories will be deprecated and removed at some point in the future as part of #186.

Hi @bflad,

Yes this is a bit of a odd / difficult one to think about concretely. I thought up of a theoretical example based on overriding the Configure method of a resource. For example, based on the scaffolding framework's basic test, we'd add in code interfaces/override structs for the resource in a manner like:

import (
	"context"
	"fmt"
	"net/http"
	"testing"

	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
	fwresource "github.com/hashicorp/terraform-plugin-framework/resource"
)

type OverideResource interface {
	Configure(ctx context.Context, req fwresource.ConfigureRequest, resp *fwresource.ConfigureResponse)
}

type OverideResourceImpl struct {
	ExampleResource
}

func (r *OverideResourceImpl) Configure(ctx context.Context, req fwresource.ConfigureRequest, resp *fwresource.ConfigureResponse) {
	// Prevent panic if the provider has not been configured.
	if req.ProviderData == nil {
		return
	}

	client, ok := req.ProviderData.(*http.Client)

	client.Transport = &http.Transport{} // EXAMPLE REPLACEMENT OF TRANSPORT WITH MOCK OR OTHER CUSTOM TRANSPORT

	if !ok {
		resp.Diagnostics.AddError(
			"Unexpected Resource Configure Type",
			fmt.Sprintf("Expected *http.Client, got: %T. Please report this issue to the provider developers.", req.ProviderData),
		)

		return
	}

	r.client = client
}

Then perhaps have a new field in the resource.TestCase struct which accepts a override for the resource struct for that case, something like:

func TestAccExampleResource(t *testing.T) {
	resource.Test(t, resource.TestCase{
		PreCheck:                 func() { testAccPreCheck(t) },
		ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
		// Replace the instance of the resource with the OverideResourceImpl used for this test
		// ResourceOverride: map[string]fwresource.Resource{
		// 	"scaffolding_example": &OverideResourceImpl{},
		// },
		Steps: []resource.TestStep{
			// Create and Read testing
			{
				Config: testAccExampleResourceConfig("one"),
				Check: resource.ComposeAggregateTestCheckFunc(
					resource.TestCheckResourceAttr("scaffolding_example.test", "configurable_attribute", "one"),
					resource.TestCheckResourceAttr("scaffolding_example.test", "defaulted", "example value when not configured"),
					resource.TestCheckResourceAttr("scaffolding_example.test", "id", "example-id"),
				),
			},
			// ImportState testing
			{
				ResourceName:      "scaffolding_example.test",
				ImportState:       true,
				ImportStateVerify: true,
				// This is not normally necessary, but is here because this
				// example code does not have an actual upstream service.
				// Once the Read method is able to refresh information from
				// the upstream service, this can be removed.
				ImportStateVerifyIgnore: []string{"configurable_attribute", "defaulted"},
			},
			// Update and Read testing
			{
				Config: testAccExampleResourceConfig("two"),
				Check: resource.ComposeAggregateTestCheckFunc(
					resource.TestCheckResourceAttr("scaffolding_example.test", "configurable_attribute", "two"),
				),
			},
			// Delete testing automatically occurs in TestCase
		},
	})
}

While I came up with this at the Resource level, it probably does make more sense for that to happen for a provider.Provider object globally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants