Merge pull request #33018 from hashicorp/tf-5529-sro-tfe-version-check

3 years ago · e6c3aab6c5
parent 0e0db2d206 300a60f393
commit e6c3aab6c5
7 changed files with 204 additions and 18 deletions
--- a/go.mod
+++ b/go.mod
@ -39,7 +39,7 @@ require (
 	github.com/hashicorp/go-multierror v1.1.1
 	github.com/hashicorp/go-plugin v1.4.3
 	github.com/hashicorp/go-retryablehttp v0.7.2
-	github.com/hashicorp/go-tfe v1.18.0
+	github.com/hashicorp/go-tfe v1.21.0
 	github.com/hashicorp/go-uuid v1.0.3
 	github.com/hashicorp/go-version v1.6.0
 	github.com/hashicorp/hcl v0.0.0-20170504190234-a4b07c25de5f
@ -147,7 +147,7 @@ require (
 	github.com/hashicorp/go-msgpack v0.5.4 // indirect
 	github.com/hashicorp/go-rootcerts v1.0.2 // indirect
 	github.com/hashicorp/go-safetemp v1.0.0 // indirect
-	github.com/hashicorp/go-slug v0.10.1 // indirect
+	github.com/hashicorp/go-slug v0.11.0 // indirect
 	github.com/hashicorp/golang-lru v0.5.1 // indirect
 	github.com/hashicorp/serf v0.9.5 // indirect
 	github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d // indirect
--- a/go.sum
+++ b/go.sum
@ -537,13 +537,13 @@ github.com/hashicorp/go-rootcerts v1.0.2 h1:jzhAVGtqPKbwpyCPELlgNWhE1znq+qwJtW5O
 github.com/hashicorp/go-rootcerts v1.0.2/go.mod h1:pqUvnprVnM5bf7AOirdbb01K4ccR319Vf4pU3K5EGc8=
 github.com/hashicorp/go-safetemp v1.0.0 h1:2HR189eFNrjHQyENnQMMpCiBAsRxzbTMIgBhEyExpmo=
 github.com/hashicorp/go-safetemp v1.0.0/go.mod h1:oaerMy3BhqiTbVye6QuFhFtIceqFoDHxNAB65b+Rj1I=
-github.com/hashicorp/go-slug v0.10.1 h1:05SCRWCBpCxOeP7stQHvMgOz0raCBCekaytu8Rg/RZ4=
-github.com/hashicorp/go-slug v0.10.1/go.mod h1:Ib+IWBYfEfJGI1ZyXMGNbu2BU+aa3Dzu41RKLH301v4=
+github.com/hashicorp/go-slug v0.11.0 h1:l7cHWiBk8cnnskjheloW9h8PwXhihvwXbQiiFw2KqkY=
+github.com/hashicorp/go-slug v0.11.0/go.mod h1:Ib+IWBYfEfJGI1ZyXMGNbu2BU+aa3Dzu41RKLH301v4=
 github.com/hashicorp/go-sockaddr v1.0.0 h1:GeH6tui99pF4NJgfnhp+L6+FfobzVW3Ah46sLo0ICXs=
 github.com/hashicorp/go-sockaddr v1.0.0/go.mod h1:7Xibr9yA9JjQq1JpNB2Vw7kxv8xerXegt+ozgdvDeDU=
 github.com/hashicorp/go-syslog v1.0.0/go.mod h1:qPfqrKkXGihmCqbJM2mZgkZGvKG1dFdvsLplgctolz4=
-github.com/hashicorp/go-tfe v1.18.0 h1:AjyZe2KSAyGHD1kbGYlY64PVYQPnJJON24qr97IjIqA=
-github.com/hashicorp/go-tfe v1.18.0/go.mod h1:T76X7dHKNEPEugPCZI3gDdaDdxUU4P4sqMZO60W57cQ=
+github.com/hashicorp/go-tfe v1.21.0 h1:sTZXf/MaC/iQ8HxKwYSL0xJSEVDwY+h4ngh/+na8vdk=
+github.com/hashicorp/go-tfe v1.21.0/go.mod h1:jedlLiHHiDeBKKpON4aIpTdsKbc2OaVbklEPI7XEHiY=
 github.com/hashicorp/go-uuid v1.0.0/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro=
 github.com/hashicorp/go-uuid v1.0.1/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro=
 github.com/hashicorp/go-uuid v1.0.2/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro=
@ -762,7 +762,7 @@ github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
-github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk=
+github.com/stretchr/testify v1.8.2 h1:+h33VjcLVPDHtOdpUCuF+7gSuG3yGIftsP1YvFihtJ8=
 github.com/tencentcloud/tencentcloud-sdk-go/tencentcloud/common v1.0.194/go.mod h1:7sCQWVkxcsR38nffDW057DRGk8mUjK1Ing/EFOK8s8Y=
 github.com/tencentcloud/tencentcloud-sdk-go/tencentcloud/common v1.0.588 h1:DYtBXB7sVc3EOW5horg8j55cLZynhsLYhHrvQ/jXKKM=
 github.com/tencentcloud/tencentcloud-sdk-go/tencentcloud/common v1.0.588/go.mod h1:7sCQWVkxcsR38nffDW057DRGk8mUjK1Ing/EFOK8s8Y=
--- a/internal/cloud/backend_plan.go
+++ b/internal/cloud/backend_plan.go
@ -11,6 +11,7 @@ import (
 	"log"
 	"os"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"syscall"
 	"time"
@ -428,10 +429,10 @@ func (b *Cloud) renderPlanLogs(ctx context.Context, op *backend.Operation, run *
 		return nil
 	}

-	// Only call the renderer if the remote workspace has structured run output
-	// enabled. The plan output will have already been rendered when the logs
-	// were read if this wasn't the case.
-	if run.Workspace.StructuredRunOutputEnabled && b.renderer != nil {
+	// Determine whether we should call the renderer to generate the plan output
+	// in human readable format. Otherwise we risk duplicate plan output since
+	// plan output may be contained in the streamed log file.
+	if ok, err := b.shouldRenderStructuredRunOutput(run); ok {
 		// Fetch the redacted plan.
 		redacted, err := readRedactedPlan(ctx, b.client.BaseURL(), b.token, run.Plan.ID)
 		if err != nil {
@ -440,11 +441,53 @@ func (b *Cloud) renderPlanLogs(ctx context.Context, op *backend.Operation, run *

 		// Render plan output.
 		b.renderer.RenderHumanPlan(*redacted, op.PlanMode)
+	} else if err != nil {
+		return err
 	}

 	return nil
 }

+// shouldRenderStructuredRunOutput ensures the remote workspace has structured
+// run output enabled and, if using Terraform Enterprise, ensures it is a release
+// that supports enabling SRO for CLI-driven runs. The plan output will have
+// already been rendered when the logs were read if this wasn't the case.
+func (b *Cloud) shouldRenderStructuredRunOutput(run *tfe.Run) (bool, error) {
+	if b.renderer == nil || !run.Workspace.StructuredRunOutputEnabled {
+		return false, nil
+	}
+
+	// If the cloud backend is configured against TFC, we only require that
+	// the workspace has structured run output enabled.
+	if b.client.IsCloud() && run.Workspace.StructuredRunOutputEnabled {
+		return true, nil
+	}
+
+	// If the cloud backend is configured against TFE, ensure the release version
+	// supports enabling SRO for CLI runs.
+	if b.client.IsEnterprise() {
+		tfeVersion := b.client.RemoteTFEVersion()
+		if tfeVersion != "" {
+			v := strings.Split(tfeVersion[1:], "-")
+			releaseDate, err := strconv.Atoi(v[0])
+			if err != nil {
+				return false, err
+			}
+
+			// Any release older than 202302-1 will not support enabling SRO for
+			// CLI-driven runs
+			if releaseDate < 202302 {
+				return false, nil
+			} else if run.Workspace.StructuredRunOutputEnabled {
+				return true, nil
+			}
+		}
+	}
+
+	// Version of TFE is unknowable
+	return false, nil
+}
+
 const planDefaultHeader = `
 [reset][yellow]Running plan in Terraform Cloud. Output will stream here. Pressing Ctrl-C
 will stop streaming the logs, but will not stop the plan running remotely.[reset]
--- a/internal/cloud/backend_plan_test.go
+++ b/internal/cloud/backend_plan_test.go
@ -2,6 +2,7 @@ package cloud

 import (
 	"context"
+	"net/http"
 	"os"
 	"os/signal"
 	"strings"
@ -1250,3 +1251,120 @@ func TestCloud_planOtherError(t *testing.T) {
 		t.Fatalf("expected error message, got: %s", err.Error())
 	}
 }
+
+func TestCloud_planShouldRenderSRO(t *testing.T) {
+	t.Run("when instance is TFC", func(t *testing.T) {
+		handlers := map[string]func(http.ResponseWriter, *http.Request){
+			"/api/v2/ping": func(w http.ResponseWriter, r *http.Request) {
+				w.Header().Set("Content-Type", "application/json")
+				w.Header().Set("TFP-API-Version", "2.5")
+				w.Header().Set("TFP-AppName", "Terraform Cloud")
+			},
+		}
+		b, bCleanup := testBackendWithHandlers(t, handlers)
+		t.Cleanup(bCleanup)
+		b.renderer = &jsonformat.Renderer{}
+
+		t.Run("and SRO is enabled", func(t *testing.T) {
+			r := &tfe.Run{
+				Workspace: &tfe.Workspace{
+					StructuredRunOutputEnabled: true,
+				},
+			}
+			assertSRORendered(t, b, r, true)
+		})
+
+		t.Run("and SRO is not enabled", func(t *testing.T) {
+			r := &tfe.Run{
+				Workspace: &tfe.Workspace{
+					StructuredRunOutputEnabled: false,
+				},
+			}
+			assertSRORendered(t, b, r, false)
+		})
+
+	})
+
+	t.Run("when instance is TFE and version supports CLI SRO", func(t *testing.T) {
+		handlers := map[string]func(http.ResponseWriter, *http.Request){
+			"/api/v2/ping": func(w http.ResponseWriter, r *http.Request) {
+				w.Header().Set("Content-Type", "application/json")
+				w.Header().Set("TFP-API-Version", "2.5")
+				w.Header().Set("TFP-AppName", "Terraform Enterprise")
+				w.Header().Set("X-TFE-Version", "v202303-1")
+			},
+		}
+		b, bCleanup := testBackendWithHandlers(t, handlers)
+		t.Cleanup(bCleanup)
+		b.renderer = &jsonformat.Renderer{}
+
+		t.Run("and SRO is enabled", func(t *testing.T) {
+			r := &tfe.Run{
+				Workspace: &tfe.Workspace{
+					StructuredRunOutputEnabled: true,
+				},
+			}
+			assertSRORendered(t, b, r, true)
+		})
+
+		t.Run("and SRO is not enabled", func(t *testing.T) {
+			r := &tfe.Run{
+				Workspace: &tfe.Workspace{
+					StructuredRunOutputEnabled: false,
+				},
+			}
+			assertSRORendered(t, b, r, false)
+		})
+	})
+
+	t.Run("when instance is a known unsupported TFE release", func(t *testing.T) {
+		handlers := map[string]func(http.ResponseWriter, *http.Request){
+			"/api/v2/ping": func(w http.ResponseWriter, r *http.Request) {
+				w.Header().Set("Content-Type", "application/json")
+				w.Header().Set("TFP-API-Version", "2.5")
+				w.Header().Set("TFP-AppName", "Terraform Enterprise")
+				w.Header().Set("X-TFE-Version", "v202208-1")
+			},
+		}
+		b, bCleanup := testBackendWithHandlers(t, handlers)
+		t.Cleanup(bCleanup)
+		b.renderer = &jsonformat.Renderer{}
+
+		r := &tfe.Run{
+			Workspace: &tfe.Workspace{
+				StructuredRunOutputEnabled: true,
+			},
+		}
+		assertSRORendered(t, b, r, false)
+	})
+
+	t.Run("when instance is an unknown TFE release", func(t *testing.T) {
+		handlers := map[string]func(http.ResponseWriter, *http.Request){
+			"/api/v2/ping": func(w http.ResponseWriter, r *http.Request) {
+				w.Header().Set("Content-Type", "application/json")
+				w.Header().Set("TFP-API-Version", "2.5")
+			},
+		}
+		b, bCleanup := testBackendWithHandlers(t, handlers)
+		t.Cleanup(bCleanup)
+		b.renderer = &jsonformat.Renderer{}
+
+		r := &tfe.Run{
+			Workspace: &tfe.Workspace{
+				StructuredRunOutputEnabled: true,
+			},
+		}
+		assertSRORendered(t, b, r, false)
+	})
+
+}
+
+func assertSRORendered(t *testing.T, b *Cloud, r *tfe.Run, shouldRender bool) {
+	got, err := b.shouldRenderStructuredRunOutput(r)
+	if err != nil {
+		t.Fatalf("expected no error: %v", err)
+	}
+	if shouldRender != got {
+		t.Fatalf("expected SRO to be rendered: %t, got %t", shouldRender, got)
+	}
+}
--- a/internal/cloud/backend_test.go
+++ b/internal/cloud/backend_test.go
@ -647,7 +647,7 @@ func TestCloud_setUnavailableTerraformVersion(t *testing.T) {
 		}),
 	})

-	b, bCleanup := testBackend(t, config)
+	b, bCleanup := testBackend(t, config, nil)
 	defer bCleanup()

 	// Make sure the workspace doesn't exist yet -- otherwise, we can't test what
--- a/internal/cloud/testing.go
+++ b/internal/cloud/testing.go
@ -43,6 +43,13 @@ var (
 		tfeHost: {"token": testCred},
 	})
 	testBackendSingleWorkspaceName = "app-prod"
+	defaultTFCPing                 = map[string]func(http.ResponseWriter, *http.Request){
+		"/api/v2/ping": func(w http.ResponseWriter, r *http.Request) {
+			w.Header().Set("Content-Type", "application/json")
+			w.Header().Set("TFP-API-Version", "2.5")
+			w.Header().Set("TFP-AppName", "Terraform Cloud")
+		},
+	}
 )

 // mockInput is a mock implementation of terraform.UIInput.
@ -79,7 +86,7 @@ func testBackendWithName(t *testing.T) (*Cloud, func()) {
 			"tags": cty.NullVal(cty.Set(cty.String)),
 		}),
 	})
-	return testBackend(t, obj)
+	return testBackend(t, obj, defaultTFCPing)
 }

 func testBackendWithTags(t *testing.T) (*Cloud, func()) {
@ -96,7 +103,7 @@ func testBackendWithTags(t *testing.T) (*Cloud, func()) {
 			),
 		}),
 	})
-	return testBackend(t, obj)
+	return testBackend(t, obj, nil)
 }

 func testBackendNoOperations(t *testing.T) (*Cloud, func()) {
@ -109,7 +116,20 @@ func testBackendNoOperations(t *testing.T) (*Cloud, func()) {
 			"tags": cty.NullVal(cty.Set(cty.String)),
 		}),
 	})
-	return testBackend(t, obj)
+	return testBackend(t, obj, nil)
+}
+
+func testBackendWithHandlers(t *testing.T, handlers map[string]func(http.ResponseWriter, *http.Request)) (*Cloud, func()) {
+	obj := cty.ObjectVal(map[string]cty.Value{
+		"hostname":     cty.NullVal(cty.String),
+		"organization": cty.StringVal("hashicorp"),
+		"token":        cty.NullVal(cty.String),
+		"workspaces": cty.ObjectVal(map[string]cty.Value{
+			"name": cty.StringVal(testBackendSingleWorkspaceName),
+			"tags": cty.NullVal(cty.Set(cty.String)),
+		}),
+	})
+	return testBackend(t, obj, handlers)
 }

 func testCloudState(t *testing.T) *State {
@ -186,8 +206,13 @@ func testBackendWithOutputs(t *testing.T) (*Cloud, func()) {
 	return b, cleanup
 }

-func testBackend(t *testing.T, obj cty.Value) (*Cloud, func()) {
-	s := testServer(t)
+func testBackend(t *testing.T, obj cty.Value, handlers map[string]func(http.ResponseWriter, *http.Request)) (*Cloud, func()) {
+	var s *httptest.Server
+	if handlers != nil {
+		s = testServerWithHandlers(handlers)
+	} else {
+		s = testServer(t)
+	}
 	b := New(testDisco(s))

 	// Configure the backend so the client is created.
--- a/internal/command/jsonformat/renderer.go
+++ b/internal/command/jsonformat/renderer.go
@ -58,7 +58,7 @@ func (renderer Renderer) RenderHumanPlan(plan Plan, mode plans.Mode, opts ...Pla
 	// version differences. This should work for alpha testing in the meantime.
 	if plan.PlanFormatVersion != jsonplan.FormatVersion || plan.ProviderFormatVersion != jsonprovider.FormatVersion {
 		renderer.Streams.Println(format.WordWrap(
-			renderer.Colorize.Color("\n[bold][red]Warning:[reset][bold] This plan was generated using a different version of Terraform, the diff presented here maybe missing representations of recent features."),
+			renderer.Colorize.Color("\n[bold][red]Warning:[reset][bold] This plan was generated using a different version of Terraform, the diff presented here may be missing representations of recent features."),
 			renderer.Streams.Stdout.Columns()))
 	}