Do not error when reading task related resource objects (hashicorp#1500)

Previously if Workspace Run Tasks, Organiaztion Run Tasks or global settings were managed by TF and deleted server-side the TF apply/plan would error. However this stopped resources being removed or ignored. This commit modifies the Read methods to remove the previous resource state when the object no longer exists (a 404 response). The Read methods still raise if another error type is received (e.g. 500 or Network error).
bschaatsbergen · Oct 22, 2024 · 4fa21b9 · 4fa21b9
1 parent 2fe452d
commit 4fa21b9
Show file tree

Hide file tree

Showing 7 changed files with 260 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,12 @@
 ## Unreleased
 
+BUG FIXES:
+* `r/tfe_workspace_run_task`: Do not error when reading workspace tasks that no longer exist by @glennsarti [#1500](https://github.com/hashicorp/terraform-provider-tfe/pull/1459)
+* `r/tfe_organization_run_task`: Do not error when reading organization tasks that no longer exist by @glennsarti [#1500](https://github.com/hashicorp/terraform-provider-tfe/pull/1459)
+* `r/tfe_organization_run_task_global_settings`: Do not error when reading organization task global settings that no longer exist by @glennsarti [#1500](https://github.com/hashicorp/terraform-provider-tfe/pull/1459)
+
+## v0.59.0
+
 ## BREAKING CHANGES
 
 * `r/tfe_team`: Default "secret" visibility has been removed from tfe_team because it now requires explicit or owner access. The default, "organization", is now computed by the platform. by @brandonc [#1439](https://github.com/hashicorp/terraform-provider-tfe/pull/1439)

diff --git a/internal/provider/helper_test.go b/internal/provider/helper_test.go
@@ -187,6 +187,22 @@ func createProject(t *testing.T, client *tfe.Client, orgName string, options tfe
 	return proj
 }
 
+func createRunTask(t *testing.T, client *tfe.Client, orgName string, options tfe.RunTaskCreateOptions) *tfe.RunTask {
+	ctx := context.Background()
+
+	if options.Category == "" {
+		options.Category = "task"
+	}
+
+	task, err := client.RunTasks.Create(ctx, orgName, options)
+	if err != nil {
+		t.Fatal(err)
+		return nil
+	}
+
+	return task
+}
+
 func skipIfCloud(t *testing.T) {
 	if !enterpriseEnabled() {
 		t.Skip("Skipping test for a feature unavailable in HCP Terraform. Set 'ENABLE_TFE=1' to run.")
@@ -270,6 +286,17 @@ func testCheckResourceAttrUnlessEnterprise(name, key, value string) resource.Tes
 	return resource.TestCheckResourceAttr(name, key, value)
 }
 
+// Tests whether a resource exists in the state
+func testCheckResourceNotExist(resourceName string) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		if item, ok := s.RootModule().Resources[resourceName]; ok {
+			return fmt.Errorf("Resource %s should not exist but found a resource with id %s", resourceName, item.Primary.ID)
+		}
+
+		return nil
+	}
+}
+
 func randomString(t *testing.T) string {
 	v, err := uuid.GenerateUUID()
 	if err != nil {

diff --git a/internal/provider/resource_tfe_organization_run_task.go b/internal/provider/resource_tfe_organization_run_task.go
@@ -160,7 +160,11 @@ func (r *resourceOrgRunTask) Read(ctx context.Context, req resource.ReadRequest,
 	tflog.Debug(ctx, "Reading organization run task")
 	task, err := r.config.Client.RunTasks.Read(ctx, taskID)
 	if err != nil {
-		resp.Diagnostics.AddError("Error reading Organization Run Task", "Could not read Organization Run Task, unexpected error: "+err.Error())
+		if errors.Is(err, tfe.ErrResourceNotFound) {
+			resp.State.RemoveResource(ctx)
+		} else {
+			resp.Diagnostics.AddError("Error reading Organization Run Task", "Could not read Organization Run Task, unexpected error: "+err.Error())
+		}
 		return
 	}
 

diff --git a/internal/provider/resource_tfe_organization_run_task_global_settings_test.go b/internal/provider/resource_tfe_organization_run_task_global_settings_test.go
@@ -145,6 +145,78 @@ func TestAccTFEOrganizationRunTaskGlobalSettings_import(t *testing.T) {
 	})
 }
 
+func TestAccTFEOrganizationRunTaskGlobalSettings_Read(t *testing.T) {
+	skipUnlessRunTasksDefined(t)
+
+	tfeClient, err := getClientUsingEnv()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	org, orgCleanup := createBusinessOrganization(t, tfeClient)
+	t.Cleanup(orgCleanup)
+	key := runTasksHMACKey()
+	task := createRunTask(t, tfeClient, org.Name, tfe.RunTaskCreateOptions{
+		Name:    fmt.Sprintf("tst-task-%s", randomString(t)),
+		URL:     runTasksURL(),
+		HMACKey: &key,
+	})
+
+	org_tf := fmt.Sprintf(`data "tfe_organization" "orgtask" { name = %q }`, org.Name)
+
+	create_settings_tf := fmt.Sprintf(`
+		%s
+		resource "tfe_organization_run_task_global_settings" "sut" {
+			task_id = %q
+
+			enabled           = true
+			enforcement_level = "mandatory"
+			stages            = ["post_plan"]
+		}
+		`, org_tf, task.ID)
+
+	delete_task_settings := func() {
+		_, err := tfeClient.RunTasks.Update(ctx, task.ID, tfe.RunTaskUpdateOptions{
+			Global: &tfe.GlobalRunTaskOptions{
+				Enabled: tfe.Bool(false),
+			},
+		})
+		if err != nil {
+			t.Fatalf("Error updating task: %s", err)
+		}
+	}
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:                 func() { testAccPreCheck(t) },
+		ProtoV5ProviderFactories: testAccMuxedProviders,
+		CheckDestroy:             testAccCheckTFEOrganizationRunTaskDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: create_settings_tf,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("tfe_organization_run_task_global_settings.sut", "enabled", "true"),
+				),
+			},
+			{
+				// Delete the created run task settings and ensure we can re-create it
+				PreConfig: delete_task_settings,
+				Config:    create_settings_tf,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("tfe_organization_run_task_global_settings.sut", "enabled", "true"),
+				),
+			},
+			{
+				// Delete the created run task settings and ensure we can ignore it if we no longer need to manage it
+				PreConfig: delete_task_settings,
+				Config:    org_tf,
+				Check: resource.ComposeTestCheckFunc(
+					testCheckResourceNotExist("tfe_organization_run_task_global_settings.sut"),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckTFEOrganizationRunTaskGlobalEnabled(resourceName string, expectedEnabled bool) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		config := testAccProvider.Meta().(ConfiguredClient)

diff --git a/internal/provider/resource_tfe_organization_run_task_test.go b/internal/provider/resource_tfe_organization_run_task_test.go
@@ -118,6 +118,72 @@ func TestAccTFEOrganizationRunTask_import(t *testing.T) {
 	})
 }
 
+func TestAccTFEOrganizationRunTask_Read(t *testing.T) {
+	skipUnlessRunTasksDefined(t)
+
+	tfeClient, err := getClientUsingEnv()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	org, orgCleanup := createBusinessOrganization(t, tfeClient)
+	t.Cleanup(orgCleanup)
+	rInt := rand.New(rand.NewSource(time.Now().UnixNano())).Int()
+	hmacKey := runTasksHMACKey()
+
+	org_tf := fmt.Sprintf(`data "tfe_organization" "orgtask" { name = %q }`, org.Name)
+
+	create_task_tf := fmt.Sprintf(`
+		%s
+		%s
+		`, org_tf, testAccTFEOrganizationRunTask_basic(org.Name, rInt, runTasksURL(), hmacKey))
+
+	delete_tasks := func() {
+		tasks, err := tfeClient.RunTasks.List(ctx, org.Name, nil)
+		if err != nil || tasks == nil {
+			t.Fatalf("Error listing tasks: %s", err)
+			return
+		}
+		// There shouldn't be more that 25 run tasks so we don't need to worry about pagination
+		for _, task := range tasks.Items {
+			if task != nil {
+				if err := tfeClient.RunTasks.Delete(ctx, task.ID); err != nil {
+					t.Fatalf("Error deleting task: %s", err)
+				}
+			}
+		}
+	}
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:                 func() { testAccPreCheck(t) },
+		ProtoV5ProviderFactories: testAccMuxedProviders,
+		Steps: []resource.TestStep{
+			{
+				Config: create_task_tf,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("tfe_organization_run_task.foobar", "name", fmt.Sprintf("foobar-task-%d", rInt)),
+				),
+			},
+			{
+				// Delete the created run task and ensure we can re-create it
+				PreConfig: delete_tasks,
+				Config:    create_task_tf,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("tfe_organization_run_task.foobar", "name", fmt.Sprintf("foobar-task-%d", rInt)),
+				),
+			},
+			{
+				// Delete the created run task and ensure we can ignore it if we no longer need to manage it
+				PreConfig: delete_tasks,
+				Config:    org_tf,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckTFEOrganizationRunTaskDestroy,
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckTFEOrganizationRunTaskExists(n string, runTask *tfe.RunTask) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		config := testAccProvider.Meta().(ConfiguredClient)

diff --git a/internal/provider/resource_tfe_workspace_run_task.go b/internal/provider/resource_tfe_workspace_run_task.go
@@ -122,7 +122,11 @@ func (r *resourceWorkspaceRunTask) Read(ctx context.Context, req resource.ReadRe
 	tflog.Debug(ctx, "Reading workspace run task")
 	wstask, err := r.config.Client.WorkspaceRunTasks.Read(ctx, workspaceID, wstaskID)
 	if err != nil {
-		resp.Diagnostics.AddError("Error reading Workspace Run Task", "Could not read Workspace Run Task, unexpected error: "+err.Error())
+		if errors.Is(err, tfe.ErrResourceNotFound) {
+			resp.State.RemoveResource(ctx)
+		} else {
+			resp.Diagnostics.AddError("Error reading Workspace Run Task", "Could not read Workspace Run Task, unexpected error: "+err.Error())
+		}
 		return
 	}
 

diff --git a/internal/provider/resource_tfe_workspace_run_task_test.go b/internal/provider/resource_tfe_workspace_run_task_test.go
@@ -182,6 +182,84 @@ func TestAccTFEWorkspaceRunTask_import(t *testing.T) {
 	})
 }
 
+func TestAccTFEWorkspaceRunTask_Read(t *testing.T) {
+	skipUnlessRunTasksDefined(t)
+
+	tfeClient, err := getClientUsingEnv()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Create test fixtures
+	org, orgCleanup := createBusinessOrganization(t, tfeClient)
+	t.Cleanup(orgCleanup)
+	ws := createTempWorkspace(t, tfeClient, org.Name)
+	key := runTasksHMACKey()
+	task := createRunTask(t, tfeClient, org.Name, tfe.RunTaskCreateOptions{
+		Name:    fmt.Sprintf("tst-task-%s", randomString(t)),
+		URL:     runTasksURL(),
+		HMACKey: &key,
+	})
+
+	org_tf := fmt.Sprintf(`data "tfe_organization" "orgtask" { name = %q }`, org.Name)
+
+	create_wstask_tf := fmt.Sprintf(`
+		%s
+		resource "tfe_workspace_run_task" "foobar" {
+			workspace_id      = %q
+			task_id           = %q
+			enforcement_level = "advisory"
+			stage             = "post_plan"
+		}
+		`, org_tf, ws.ID, task.ID)
+
+	delete_wstasks := func() {
+		wstasks, err := tfeClient.WorkspaceRunTasks.List(ctx, ws.ID, nil)
+		if err != nil || wstasks == nil {
+			t.Fatalf("Error listing tasks: %s", err)
+			return
+		}
+		// There shouldn't be more that 25 run tasks so we don't need to worry about pagination
+		for _, wstask := range wstasks.Items {
+			if wstask != nil {
+				if err := tfeClient.WorkspaceRunTasks.Delete(ctx, ws.ID, wstask.ID); err != nil {
+					t.Fatalf("Error deleting workspace task: %s", err)
+				}
+			}
+		}
+	}
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:                 func() { testAccPreCheck(t) },
+		ProtoV5ProviderFactories: testAccMuxedProviders,
+		CheckDestroy:             testAccCheckTFETeamAccessDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: create_wstask_tf,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("tfe_workspace_run_task.foobar", "enforcement_level", "advisory"),
+				),
+			},
+			{
+				// Delete the created workspace run task and ensure we can re-create it
+				PreConfig: delete_wstasks,
+				Config:    create_wstask_tf,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("tfe_workspace_run_task.foobar", "enforcement_level", "advisory"),
+				),
+			},
+			{
+				// Delete the created workspace run task and ensure we can ignore it if we no longer need to manage it
+				PreConfig: delete_wstasks,
+				Config:    org_tf,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckTFEWorkspaceRunTaskDestroy,
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckTFEWorkspaceRunTaskExists(n string, runTask *tfe.WorkspaceRunTask) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		config := testAccProvider.Meta().(ConfiguredClient)