Merge pull request #967 from timofurrer/feature/delete-topic

resource/gitlab_topic: support proper deletion of topics

Author: timofurrer

docs/resources/topic.md

@@ -4,18 +4,18 @@ page_title: "gitlab_topic Resource - terraform-provider-gitlab"
 subcategory: ""
 description: |-
   The gitlab_topic resource allows to manage the lifecycle of topics that are then assignable to projects.
-  Topics are the successors for project tags. Aside from avoiding terminology collisions with Git tags, they are more descriptive and better searchable.
-  ~> Deleting a resource doesn't delete the corresponding topic as the GitLab API doesn't support deleting topics yet. You can set soft_destroy to true if you want the topics description to be emptied instead.
+  -> Topics are the successors for project tags. Aside from avoiding terminology collisions with Git tags, they are more descriptive and better searchable.
+  ~> Deleting a topic was implemented in GitLab 14.9. For older versions of GitLab set soft_destroy = true to empty out a topic instead of deleting it.
   Upstream API: GitLab REST API docs for topics https://docs.gitlab.com/ee/api/topics.html
 ---
 
 # gitlab_topic (Resource)
 
 The `gitlab_topic` resource allows to manage the lifecycle of topics that are then assignable to projects.
 
-Topics are the successors for project tags. Aside from avoiding terminology collisions with Git tags, they are more descriptive and better searchable.
+-> Topics are the successors for project tags. Aside from avoiding terminology collisions with Git tags, they are more descriptive and better searchable.
 
-~> Deleting a resource doesn't delete the corresponding topic as the GitLab API doesn't support deleting topics yet. You can set soft_destroy to true if you want the topics description to be emptied instead.
+~> Deleting a topic was implemented in GitLab 14.9. For older versions of GitLab set `soft_destroy = true` to empty out a topic instead of deleting it.
 
 **Upstream API**: [GitLab REST API docs for topics](https://docs.gitlab.com/ee/api/topics.html)
 
@@ -34,12 +34,12 @@ resource "gitlab_topic" "functional_programming" {
 ### Required
 
 - `name` (String) The topic's name
-- `soft_destroy` (Boolean) Empty the topics fields instead of deleting it
 
 ### Optional
 
 - `description` (String) A text describing the topic
 - `id` (String) The ID of this resource.
+- `soft_destroy` (Boolean, Deprecated) Empty the topics fields instead of deleting it
 
 ## Import
 

internal/provider/resource_gitlab_topic.go

@@ -15,9 +15,9 @@ var _ = registerResource("gitlab_topic", func() *schema.Resource {
 	return &schema.Resource{
 		Description: `The ` + "`gitlab_topic`" + ` resource allows to manage the lifecycle of topics that are then assignable to projects.
 
-Topics are the successors for project tags. Aside from avoiding terminology collisions with Git tags, they are more descriptive and better searchable.
+-> Topics are the successors for project tags. Aside from avoiding terminology collisions with Git tags, they are more descriptive and better searchable.
 
-~> Deleting a resource doesn't delete the corresponding topic as the GitLab API doesn't support deleting topics yet. You can set soft_destroy to true if you want the topics description to be emptied instead.
+~> Deleting a topic was implemented in GitLab 14.9. For older versions of GitLab set ` + "`soft_destroy = true`" + ` to empty out a topic instead of deleting it.
 
 **Upstream API**: [GitLab REST API docs for topics](https://docs.gitlab.com/ee/api/topics.html)
 `,
@@ -39,7 +39,8 @@ Topics are the successors for project tags. Aside from avoiding terminology coll
 			"soft_destroy": {
 				Description: "Empty the topics fields instead of deleting it",
 				Type:        schema.TypeBool,
-				Required:    true,
+				Optional:    true,
+				Deprecated:  "GitLab 14.9 introduced the proper deletion of topics. This field is no longer needed.",
 			},
 			"description": {
 				Description: "A text describing the topic",
@@ -125,27 +126,43 @@ func resourceGitlabTopicUpdate(ctx context.Context, d *schema.ResourceData, meta
 }
 
 func resourceGitlabTopicDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
-
+	client := meta.(*gitlab.Client)
+	topicID, err := strconv.Atoi(d.Id())
+	if err != nil {
+		return diag.Errorf("Failed to convert topic id %s to int: %s", d.Id(), err)
+	}
 	softDestroy := d.Get("soft_destroy").(bool)
 
-	if !softDestroy {
-		return diag.Errorf("Destroying a topic is not yet supported. You can set soft_destroy=true to suppress this error")
+	deleteNotSupported, err := isGitLabVersionLessThan(client, "14.9")()
+	if err != nil {
+		return diag.FromErr(err)
+	}
+	if !softDestroy && deleteNotSupported {
+		return diag.Errorf("GitLab 14.9 introduced the proper deletion of topics. Set `soft_destroy = true` to empty out a topic instead of deleting it.")
 	}
 
-	log.Printf("[WARN] Not deleting gitlab topic %s. Instead emptying its description", d.Id())
+	// NOTE: the `soft_destroy` field is deprecated and will be removed in a future version.
+	//       It was only introduced because GitLab prior to 14.9 didn't support topic deletion.
+	if softDestroy {
+		log.Printf("[WARN] Not deleting gitlab topic %s. Instead emptying its description", d.Id())
 
-	client := meta.(*gitlab.Client)
-	options := &gitlab.UpdateTopicOptions{
-		Description: gitlab.String(""),
-	}
+		options := &gitlab.UpdateTopicOptions{
+			Description: gitlab.String(""),
+		}
 
-	topicID, err := strconv.Atoi(d.Id())
-	if err != nil {
-		return diag.Errorf("Failed to convert topic id %s to int: %s", d.Id(), err)
+		_, _, err = client.Topics.UpdateTopic(topicID, options, gitlab.WithContext(ctx))
+		if err != nil {
+			return diag.Errorf("Failed to update topic %d: %s", topicID, err)
+		}
+
+		return nil
 	}
-	_, _, err = client.Topics.UpdateTopic(topicID, options, gitlab.WithContext(ctx))
-	if err != nil {
-		return diag.Errorf("Failed to update topic %d: %s", topicID, err)
+
+	log.Printf("[DEBUG] delete gitlab topic %s", d.Id())
+
+	if _, err = client.Topics.DeleteTopic(topicID, gitlab.WithContext(ctx)); err != nil {
+		return diag.Errorf("Failed to delete topic %d: %s", topicID, err)
 	}
+
 	return nil
 }

internal/provider/resource_gitlab_topic_test.go

@@ -30,6 +30,15 @@ func TestAccGitlabTopic_basic(t *testing.T) {
 					}),
 				),
 			},
+			// Verify import
+			{
+				ResourceName:      "gitlab_topic.foo",
+				ImportState:       true,
+				ImportStateVerify: true,
+				ImportStateVerifyIgnore: []string{
+					"soft_destroy",
+				},
+			},
 			// Update the topics values
 			{
 				Config: testAccGitlabTopicFullConfig(rInt),
@@ -41,6 +50,15 @@ func TestAccGitlabTopic_basic(t *testing.T) {
 					}),
 				),
 			},
+			// Verify import
+			{
+				ResourceName:      "gitlab_topic.foo",
+				ImportState:       true,
+				ImportStateVerify: true,
+				ImportStateVerifyIgnore: []string{
+					"soft_destroy",
+				},
+			},
 			// Update the topics values back to their initial state
 			{
 				Config: testAccGitlabTopicRequiredConfig(rInt),
@@ -51,6 +69,15 @@ func TestAccGitlabTopic_basic(t *testing.T) {
 					}),
 				),
 			},
+			// Verify import
+			{
+				ResourceName:      "gitlab_topic.foo",
+				ImportState:       true,
+				ImportStateVerify: true,
+				ImportStateVerifyIgnore: []string{
+					"soft_destroy",
+				},
+			},
 			// Updating the topic to have a description before it is deleted
 			{
 				Config: testAccGitlabTopicFullConfig(rInt),
@@ -75,6 +102,26 @@ func TestAccGitlabTopic_basic(t *testing.T) {
 	})
 }
 
+func TestAccGitlabTopic_softDestroy(t *testing.T) {
+	var topic gitlab.Topic
+	rInt := acctest.RandInt()
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:          func() { testAccPreCheck(t) },
+		ProviderFactories: providerFactories,
+		CheckDestroy:      testAccCheckGitlabTopicSoftDestroy,
+		Steps: []resource.TestStep{
+			// Create a topic with soft_destroy enabled
+			{
+				Config: testAccGitlabTopicSoftDestroyConfig(rInt),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckGitlabTopicExists("gitlab_topic.foo", &topic),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckGitlabTopicExists(n string, assign *gitlab.Topic) resource.TestCheckFunc {
 	return func(s *terraform.State) (err error) {
 
@@ -139,19 +186,48 @@ func testAccCheckGitlabTopicDestroy(s *terraform.State) (err error) {
 			return err
 		}
 
-		topic, resp, err := testGitlabClient.Topics.GetTopic(id)
+		topic, _, err := testGitlabClient.Topics.GetTopic(id)
 		if err == nil {
 			if topic != nil && fmt.Sprintf("%d", topic.ID) == rs.Primary.ID {
+				return fmt.Errorf("topic %s still exists", rs.Primary.ID)
+			}
+		}
+		if !is404(err) {
+			return err
+		}
+		return nil
+	}
+	return nil
+}
+
+func testAccCheckGitlabTopicSoftDestroy(s *terraform.State) (err error) {
+
+	defer func() {
+		if err != nil {
+			err = fmt.Errorf("destroying gitlab topic failed: %w", err)
+		}
+	}()
+
+	for _, rs := range s.RootModule().Resources {
+		if rs.Type != "gitlab_topic" {
+			continue
+		}
+
+		id, err := strconv.Atoi(rs.Primary.ID)
+		if err != nil {
+			return err
+		}
 
+		topic, _, err := testGitlabClient.Topics.GetTopic(id)
+		if err == nil {
+			if topic != nil && fmt.Sprintf("%d", topic.ID) == rs.Primary.ID {
 				if topic.Description != "" {
 					return fmt.Errorf("topic still has a description")
 				}
-
-				// TODO: Return error as soon as deleting a topic is supported
 				return nil
 			}
 		}
-		if resp.StatusCode != 404 {
+		if !is404(err) {
 			return err
 		}
 		return nil
@@ -163,7 +239,6 @@ func testAccGitlabTopicRequiredConfig(rInt int) string {
 	return fmt.Sprintf(`
 resource "gitlab_topic" "foo" {
   name             = "foo-req-%d"
-  soft_destroy     = true
 }`, rInt)
 }
 
@@ -172,6 +247,15 @@ func testAccGitlabTopicFullConfig(rInt int) string {
 resource "gitlab_topic" "foo" {
   name             = "foo-full-%d"
   description      = "Terraform acceptance tests"
+}`, rInt)
+}
+
+func testAccGitlabTopicSoftDestroyConfig(rInt int) string {
+	return fmt.Sprintf(`
+resource "gitlab_topic" "foo" {
+  name             = "foo-soft-destroy-%d"
+  description      = "Terraform acceptance tests"
+
   soft_destroy     = true
 }`, rInt)
 }