Merge pull request #607 from hashicorp/uk1288-fix-plan-failure-with-e…

…xec-mode-change

Fix ability to detect and apply agent_pool_id and execution_mode changes

CHANGELOG.md

@@ -1,4 +1,5 @@
 ## Unreleased
+ * r/tfe_workspace: Changes in `agent_pool_id` and `execution_mode` attributes are now detected and applied. ([#607](https://github.com/hashicorp/terraform-provider-tfe/pull/607))
 
 FEATURES:
 * r/tfe_workspace_run_task, d/tfe_workspace_run_task: Add `stage` attribute to workspace run tasks. ([#555](https://github.com/hashicorp/terraform-provider-tfe/pull/555))

tfe/resource_tfe_workspace.go

@@ -34,7 +34,13 @@ func resourceTFEWorkspace() *schema.Resource {
 		},
 
 		CustomizeDiff: func(c context.Context, d *schema.ResourceDiff, meta interface{}) error {
-			err := validateAgentExecution(c, d)
+			// NOTE: execution mode must be set to default first before calling the validation functions
+			err := setExecutionModeDefault(c, d)
+			if err != nil {
+				return err
+			}
+
+			err = validateAgentExecution(c, d)
 			if err != nil {
 				return err
 			}
@@ -67,6 +73,7 @@ func resourceTFEWorkspace() *schema.Resource {
 			"agent_pool_id": {
 				Type:          schema.TypeString,
 				Optional:      true,
+				Default:       "",
 				ConflictsWith: []string{"operations"},
 			},
 
@@ -120,6 +127,7 @@ func resourceTFEWorkspace() *schema.Resource {
 				Type:          schema.TypeBool,
 				Optional:      true,
 				Computed:      true,
+				Deprecated:    "Use execution_mode instead.",
 				ConflictsWith: []string{"execution_mode", "agent_pool_id"},
 			},
 
@@ -254,7 +262,7 @@ func resourceTFEWorkspaceCreate(d *schema.ResourceData, meta interface{}) error
 		options.ExecutionMode = tfe.String(v.(string))
 	}
 
-	if v, ok := d.GetOk("operations"); ok {
+	if v, ok := d.GetOkExists("operations"); ok {
 		options.Operations = tfe.Bool(v.(bool))
 	}
 
@@ -657,13 +665,34 @@ func resourceTFEWorkspaceDelete(d *schema.ResourceData, meta interface{}) error
 	return nil
 }
 
+// since execution_mode is marked as Optional: true, and Computed: true,
+// unsetting the execution_mode in the config after it's been set to a valid
+// value is not detected by ResourceDiff so read value from RawConfig instead
+func setExecutionModeDefault(_ context.Context, d *schema.ResourceDiff) error {
+	configMap := d.GetRawConfig().AsValueMap()
+	operations, operationsReadOk := configMap["operations"]
+	executionMode, executionModeReadOk := configMap["execution_mode"]
+	executionModeState := d.Get("execution_mode")
+	if operationsReadOk && executionModeReadOk {
+		if operations.IsNull() && executionMode.IsNull() && executionModeState != "remote" {
+			err := d.SetNew("execution_mode", "remote")
+			if err != nil {
+				return fmt.Errorf("failed to set execution_mode: %w", err)
+			}
+		}
+	}
+
+	return nil
+}
+
 // An agent pool can only be specified when execution_mode is set to "agent". You currently cannot specify a
 // schema validation based on a different argument's value, so we do so here at plan time instead.
 func validateAgentExecution(_ context.Context, d *schema.ResourceDiff) error {
 	if executionMode, ok := d.GetOk("execution_mode"); ok {
-		if executionMode.(string) != "agent" && d.Get("agent_pool_id") != "" {
+		executionModeIsAgent := executionMode.(string) == "agent"
+		if !executionModeIsAgent && d.Get("agent_pool_id") != "" {
 			return fmt.Errorf("execution_mode must be set to 'agent' to assign agent_pool_id")
-		} else if executionMode.(string) == "agent" && d.NewValueKnown("agent_pool_id") && d.Get("agent_pool_id") == "" {
+		} else if executionModeIsAgent && d.NewValueKnown("agent_pool_id") && d.Get("agent_pool_id") == "" {
 			return fmt.Errorf("agent_pool_id must be provided when execution_mode is 'agent'")
 		}
 	}

tfe/resource_tfe_workspace_test.go

@@ -1624,6 +1624,54 @@ func TestAccTFEWorkspace_operationsAndExecutionModeInteroperability(t *testing.T
 	})
 }
 
+func TestAccTFEWorkspace_unsetExecutionMode(t *testing.T) {
+	skipIfEnterprise(t)
+
+	tfeClient, err := getClientUsingEnv()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	org, orgCleanup := createBusinessOrganization(t, tfeClient)
+	t.Cleanup(orgCleanup)
+
+	workspace := &tfe.Workspace{}
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckTFEWorkspaceDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccTFEWorkspace_executionModeAgent(org.Name),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckTFEWorkspaceExists(
+						"tfe_workspace.foobar", workspace),
+					resource.TestCheckResourceAttr(
+						"tfe_workspace.foobar", "operations", "true"),
+					resource.TestCheckResourceAttr(
+						"tfe_workspace.foobar", "execution_mode", "agent"),
+					resource.TestCheckResourceAttrSet(
+						"tfe_workspace.foobar", "agent_pool_id"),
+				),
+			},
+			{
+				Config: testAccTFEWorkspace_executionModeNull(org.Name),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckTFEWorkspaceExists(
+						"tfe_workspace.foobar", workspace),
+					resource.TestCheckResourceAttr(
+						"tfe_workspace.foobar", "operations", "true"),
+					resource.TestCheckResourceAttr(
+						"tfe_workspace.foobar", "execution_mode", "remote"),
+					resource.TestCheckResourceAttr(
+						"tfe_workspace.foobar", "agent_pool_id", ""),
+				),
+			},
+		},
+	})
+}
+
 func TestAccTFEWorkspace_globalRemoteState(t *testing.T) {
 	workspace := &tfe.Workspace{}
 	rInt := rand.New(rand.NewSource(time.Now().UnixNano())).Int()
@@ -2249,6 +2297,22 @@ resource "tfe_workspace" "foobar" {
 }`, organization, organization)
 }
 
+// while testing the flow of unsetting execution mode as in TestAccTFEWorkspace_unsetExecutionMode
+// the resource "tfe_agent_pool" has been kept in both configs(testAccTFEWorkspace_executionModeAgent & testAccTFEWorkspace_executionModeNull)
+// this prevents an attempt to destroy the agent pool before dissasociating it from the workspace
+func testAccTFEWorkspace_executionModeNull(organization string) string {
+	return fmt.Sprintf(`
+resource "tfe_agent_pool" "foobar" {
+  name = "agent-pool-test"
+  organization = "%s"
+}
+
+resource "tfe_workspace" "foobar" {
+  name         = "workspace-test"
+  organization = "%s"
+}`, organization, organization)
+}
+
 func testAccTFEWorkspace_basicSpeculativeOff(rInt int) string {
 	return fmt.Sprintf(`
 resource "tfe_organization" "foobar" {
@@ -2702,15 +2766,15 @@ func testAccTFEWorkspace_updateRemoveVCSBlockFromTagsRegex(rInt int) string {
 		name  = "tst-tf-%d-git-tag-ff-on"
 		email = "[email protected]"
 	}
-	
+
 	resource "tfe_oauth_client" "test" {
 		organization     = tfe_organization.foobar.id
 		api_url          = "https://api.github.com"
 		http_url         = "https://github.com"
 		oauth_token      = "%s"
 		service_provider = "github"
 	}
-	
+
 	resource "tfe_workspace" "foobar" {
 		name         			= "workspace-test"
 		description  			= "workspace-test-update-vcs-repo-tags-regex"