Cloud: refactor setting attributes, add tests

internal/cloud/backend.go

@@ -183,41 +183,14 @@ func (b *Cloud) Configure(obj cty.Value) tfdiags.Diagnostics {
 	if obj.IsNull() {
 		return diags
 	}
-
+	diagErr := b.setConfigurationFields(obj)
-	// Get the hostname.
+	if diagErr.HasErrors() {
-	if val := obj.GetAttr("hostname"); !val.IsNull() && val.AsString() != "" {
+		return diagErr
-		b.hostname = val.AsString()
-	} else {
-		b.hostname = defaultHostname
-	}
-
-	// Get the organization.
-	if val := obj.GetAttr("organization"); !val.IsNull() {
-		b.organization = val.AsString()
-	}
-
-	// Get the workspaces configuration block and retrieve the
-	// default workspace name and prefix.
-	if workspaces := obj.GetAttr("workspaces"); !workspaces.IsNull() {
-		if val := workspaces.GetAttr("name"); !val.IsNull() {
-			b.workspace = val.AsString()
-		}
-		if val := workspaces.GetAttr("prefix"); !val.IsNull() {
-			b.prefix = val.AsString()
-		}
-	}
-
-	// Determine if we are forced to use the local backend.
-	b.forceLocal = os.Getenv("TF_FORCE_LOCAL_BACKEND") != ""
-
-	serviceID := tfeServiceID
-	if b.forceLocal {
-		serviceID = stateServiceID
 	}
 
 	// Discover the service URL to confirm that it provides the Terraform Cloud/Enterprise API
 	// and to get the version constraints.
-	service, constraints, err := b.discover(serviceID)
+	service, constraints, err := b.discover()
 
 	// First check any contraints we might have received.
 	if constraints != nil {
@@ -332,8 +305,47 @@ func (b *Cloud) Configure(obj cty.Value) tfdiags.Diagnostics {
 	return diags
 }
 
+func (b *Cloud) setConfigurationFields(obj cty.Value) tfdiags.Diagnostics {
+	var diags tfdiags.Diagnostics
+
+	// Get the hostname.
+	if val := obj.GetAttr("hostname"); !val.IsNull() && val.AsString() != "" {
+		b.hostname = val.AsString()
+	} else {
+		b.hostname = defaultHostname
+	}
+
+	// Get the organization.
+	if val := obj.GetAttr("organization"); !val.IsNull() {
+		b.organization = val.AsString()
+	}
+
+	// Get the workspaces configuration block and retrieve the
+	// default workspace name and prefix.
+	if workspaces := obj.GetAttr("workspaces"); !workspaces.IsNull() {
+
+		// PrepareConfig checks that you cannot set both of these.
+		if val := workspaces.GetAttr("name"); !val.IsNull() {
+			b.workspace = val.AsString()
+		}
+		if val := workspaces.GetAttr("prefix"); !val.IsNull() {
+			b.prefix = val.AsString()
+		}
+	}
+
+	// Determine if we are forced to use the local backend.
+	b.forceLocal = os.Getenv("TF_FORCE_LOCAL_BACKEND") != ""
+
+	return diags
+}
+
 // discover the TFC/E API service URL and version constraints.
-func (b *Cloud) discover(serviceID string) (*url.URL, *disco.Constraints, error) {
+func (b *Cloud) discover() (*url.URL, *disco.Constraints, error) {
+	serviceID := tfeServiceID
+	if b.forceLocal {
+		serviceID = stateServiceID
+	}
+
 	hostname, err := svchost.ForComparison(b.hostname)
 	if err != nil {
 		return nil, nil, err

internal/cloud/backend_test.go

@@ -3,6 +3,7 @@ package cloud
 import (
 	"context"
 	"fmt"
+	"os"
 	"reflect"
 	"strings"
 	"testing"
@@ -212,6 +213,127 @@ func TestCloud_config(t *testing.T) {
 	}
 }
 
+func TestCloud_setConfigurationFields(t *testing.T) {
+	originalForceBackendEnv := os.Getenv("TF_FORCE_LOCAL_BACKEND")
+
+	cases := map[string]struct {
+		obj                     cty.Value
+		expectedHostname        string
+		expectedOrganziation    string
+		expectedWorkspacePrefix string
+		expectedWorkspaceName   string
+		expectedForceLocal      bool
+		setEnv                  func()
+		resetEnv                func()
+		expectedErr             string
+	}{
+		"with hostname set": {
+			obj: cty.ObjectVal(map[string]cty.Value{
+				"organization": cty.StringVal("hashicorp"),
+				"hostname":     cty.StringVal("hashicorp.com"),
+				"workspaces": cty.ObjectVal(map[string]cty.Value{
+					"name":   cty.StringVal("prod"),
+					"prefix": cty.NullVal(cty.String),
+				}),
+			}),
+			expectedHostname:     "hashicorp.com",
+			expectedOrganziation: "hashicorp",
+		},
+		"with hostname not set, set to default hostname": {
+			obj: cty.ObjectVal(map[string]cty.Value{
+				"organization": cty.StringVal("hashicorp"),
+				"hostname":     cty.NullVal(cty.String),
+				"workspaces": cty.ObjectVal(map[string]cty.Value{
+					"name":   cty.StringVal("prod"),
+					"prefix": cty.NullVal(cty.String),
+				}),
+			}),
+			expectedHostname:     defaultHostname,
+			expectedOrganziation: "hashicorp",
+		},
+		"with workspace name set": {
+			obj: cty.ObjectVal(map[string]cty.Value{
+				"organization": cty.StringVal("hashicorp"),
+				"hostname":     cty.StringVal("hashicorp.com"),
+				"workspaces": cty.ObjectVal(map[string]cty.Value{
+					"name":   cty.StringVal("prod"),
+					"prefix": cty.NullVal(cty.String),
+				}),
+			}),
+			expectedHostname:      "hashicorp.com",
+			expectedOrganziation:  "hashicorp",
+			expectedWorkspaceName: "prod",
+		},
+		"with workspace prefix set": {
+			obj: cty.ObjectVal(map[string]cty.Value{
+				"organization": cty.StringVal("hashicorp"),
+				"hostname":     cty.StringVal("hashicorp.com"),
+				"workspaces": cty.ObjectVal(map[string]cty.Value{
+					"name":   cty.NullVal(cty.String),
+					"prefix": cty.StringVal("prod"),
+				}),
+			}),
+			expectedHostname:        "hashicorp.com",
+			expectedOrganziation:    "hashicorp",
+			expectedWorkspacePrefix: "prod",
+		},
+		"with force local set": {
+			obj: cty.ObjectVal(map[string]cty.Value{
+				"organization": cty.StringVal("hashicorp"),
+				"hostname":     cty.StringVal("hashicorp.com"),
+				"workspaces": cty.ObjectVal(map[string]cty.Value{
+					"name":   cty.NullVal(cty.String),
+					"prefix": cty.StringVal("prod"),
+				}),
+			}),
+			expectedHostname:        "hashicorp.com",
+			expectedOrganziation:    "hashicorp",
+			expectedWorkspacePrefix: "prod",
+			setEnv: func() {
+				os.Setenv("TF_FORCE_LOCAL_BACKEND", "1")
+			},
+			resetEnv: func() {
+				os.Setenv("TF_FORCE_LOCAL_BACKEND", originalForceBackendEnv)
+			},
+			expectedForceLocal: true,
+		},
+	}
+
+	for name, tc := range cases {
+		b := &Cloud{}
+
+		// if `setEnv` is set, then we expect `resetEnv` to also be set
+		if tc.setEnv != nil {
+			tc.setEnv()
+			defer tc.resetEnv()
+		}
+
+		errDiags := b.setConfigurationFields(tc.obj)
+		if errDiags.HasErrors() || tc.expectedErr != "" {
+			actualErr := errDiags.Err().Error()
+			if !strings.Contains(actualErr, tc.expectedErr) {
+				t.Fatalf("%s: unexpected validation result: %v", name, errDiags.Err())
+			}
+		}
+
+		if tc.expectedHostname != "" && b.hostname != tc.expectedHostname {
+			t.Fatalf("%s: expected hostname %s to match actual hostname %s", name, tc.expectedHostname, b.hostname)
+		}
+		if tc.expectedOrganziation != "" && b.organization != tc.expectedOrganziation {
+			t.Fatalf("%s: expected organization %s to match actual organization %s", name, tc.expectedOrganziation, b.organization)
+		}
+		if tc.expectedWorkspacePrefix != "" && b.prefix != tc.expectedWorkspacePrefix {
+			t.Fatalf("%s: expected workspace prefix %s to match actual workspace prefix %s", name, tc.expectedWorkspacePrefix, b.prefix)
+		}
+		if tc.expectedWorkspaceName != "" && b.workspace != tc.expectedWorkspaceName {
+			t.Fatalf("%s: expected workspace name %s to match actual workspace name %s", name, tc.expectedWorkspaceName, b.workspace)
+		}
+		if tc.expectedForceLocal != false && b.forceLocal != tc.expectedForceLocal {
+			t.Fatalf("%s: expected force local backend to be set ", name)
+		}
+	}
+}
+
 func TestCloud_versionConstraints(t *testing.T) {
 	cases := map[string]struct {
 		config     cty.Value