Adding ApplyJSON(), DestroyJSON(), PlanJSON() and RefreshJSON() functions

[bendbennett]

Closes: #353

[bflad]

This looks good to me although I would prefer to hear an opinion from @radeksimko / @kmoe 👍

[radeksimko]

Aside from my mostly minor in-line comments I have three higher-level questions:

• Can you confirm the assumption that with JSON output, the error matching does not work as expected (i.e. we produce generic error types, rather than the ones from https://github.com/hashicorp/terraform-exec/blob/main/tfexec/errors.go )? It's fine if it doesn't - that is one of the reasons we wanted to make it a separate method, but it would be great to have that theory confirmed + more importantly have it documented - ideally both in the comment above the method and in the Changelog. We should have an answer for those who'd rightly be confused about the similar methods.
• The other reason (in addition to ^) we wanted to make separate *JSON methods is that all methods should work with JSON by default. I'm assuming the (in)stability is already communicated through the 0.x.x version, but is it worth mentioning the (likely) future of these methods in the comment? i.e. that they're most likely to be removed in the future major version in favour of Apply/Refresh/Destroy?
• Have you tried using this API without buffering? I'm guessing it should work just fine with the help of io.Pipe? Just trying to get an answer for @thrashr888 in Plan/Apply -json support missing #183 who I believe might want to avoid buffering.

Thank you for adding all the tests. I'm assuming the 3 failures are related to the upstream hc-install bug, which is something I'm working on, so you can ignore these for now.

[bendbennett]

Can you confirm the assumption that with JSON output, the error matching does not work as expected (i.e. we produce generic error types, rather than the ones from https://github.com/hashicorp/terraform-exec/blob/main/tfexec/errors.go )? It's fine if it doesn't - that is one of the reasons we wanted to make it a separate method, but it would be great to have that theory confirmed + more importantly have it documented - ideally both in the comment above the method and in the Changelog. We should have an answer for those who'd rightly be confused about the similar methods.

With the ...JSON() functions the original errors (e.g., ErrVersionMismatch) are preserved.
We make use of this in terraform-plugin-testing to fallback to calling the original functions (e.g., Apply() if ApplyJSON() generates a ErrVersionMismatch error).

We're using code such as the following in terraform-plugin-testing to detect such errors:

	if err != nil {
		target := &tfexec.ErrVersionMismatch{}
		if errors.As(err, &target) {
			...

See https://github.com/hashicorp/terraform-plugin-testing/pull/17/files#diff-fb41773504c4caa3fef8e2f58c5839a3959cac7c99ad150aca6acf8e1d276bb0R188 for an example implementation.

@radeksimko given the original errors are preserved can you confirm that the documentation and changelog do not need altering?

The other reason (in addition to ^) we wanted to make separate *JSON methods is that all methods should work with JSON by default. I'm assuming the (in)stability is already communicated through the 0.x.x version, but is it worth mentioning the (likely) future of these methods in the comment? i.e. that they're most likely to be removed in the future major version in favour of Apply/Refresh/Destroy?

I have updated the Go docs for ApplyJSON(), DestroyJSON(), PlanJSON() and RefreshJSON() to reflect that the ...JSON() functions will likely be removed in future in favour of updating Apply() etc to return JSON.

Have you tried using this API without buffering? I'm guessing it should work just fine with the help of io.Pipe? Just trying to get an answer for @thrashr888 in #183 who I believe might want to avoid buffering.

I have tested the API without buffering (i.e., using io.Pipe).

The following test produces the output below:

func TestApplyJSONCmdPipe(t *testing.T) {
	td := t.TempDir()

	tf, err := NewTerraform(td, tfVersion(t, testutil.Latest_v1))
	if err != nil {
		t.Fatal(err)
	}

	// empty env, to avoid environ mismatch in testing
	tf.SetEnv(map[string]string{})

	pipeReader, pipeWriter := io.Pipe()
	defer pipeWriter.Close()

	go func() {
		defer pipeReader.Close()
		if _, err := io.Copy(os.Stdout, pipeReader); err != nil {
			log.Fatal(err)
		}
	}()

	err = tf.ApplyJSON(context.Background(), pipeWriter)
	if err == nil {
		t.Fatal("error expected, got nil")
	}
}

{"@level":"info","@message":"Terraform 1.0.11","@module":"terraform.ui","@timestamp":"2023-01-23T09:38:37.659360Z","terraform":"1.0.11","type":"version","ui":"0.1.0"}
{"@level":"error","@message":"Error: No configuration files","@module":"terraform.ui","@timestamp":"2023-01-23T09:38:37.660401Z","diagnostic":{"severity":"error","summary":"No configuration files","detail":"Apply requires configuration to be present. Applying without a configuration would mark everything for destruction, which is normally not what is desired. If you would like to destroy everything, run 'terraform destroy' instead."},"type":"diagnostic"}
--- PASS: TestApplyJSONCmdPipe (9.76s)

[radeksimko]

given the original errors are preserved can you confirm that the documentation and changelog do not need altering?

Yep, we're good on that front.

Thank you for making the changes. LGTM