Fix migrating tag across sets (#755)

octopusdeploy_framework/resource_tag.go

@@ -110,17 +110,18 @@ func (t *tagTypeResource) Update(ctx context.Context, req resource.UpdateRequest
 	tagSetID := data.TagSetId.ValueString()
 	tagSetSpaceID := data.TagSetSpaceId.ValueString()
 
-	tflog.Info(ctx, fmt.Sprintf("updating tag (%s)", data.ID))
+	tflog.Info(ctx, fmt.Sprintf("updating tag (%s)", state.ID))
 
 	// if the tag is reassigned to another tag set
 	if !data.TagSetId.Equal(state.TagSetId) {
 		sourceTagSetID, destinationTagSetID := state.TagSetId.ValueString(), data.TagSetId.ValueString()
-		sourceTagSetSpaceID, destinationTagSetSpaceID := state.TagSetSpaceId.ValueString(), data.TagSetSpaceId.ValueString()
+		targetSpaceId := util.Ternary(data.TagSetSpaceId.ValueString() == "", data.TagSetSpaceId, state.TagSetSpaceId)
+		sourceTagSetSpaceID, destinationTagSetSpaceID := state.TagSetSpaceId.ValueString(), targetSpaceId.ValueString()
 
 		sourceTagSet, err := tagsets.GetByID(t.Client, sourceTagSetSpaceID, sourceTagSetID)
 		if err != nil {
 			// if spaceID has changed, tag has been deleted, recreate required
-			if !data.TagSetSpaceId.Equal(state.TagSetSpaceId) {
+			if !targetSpaceId.Equal(state.TagSetSpaceId) {
 				tagCreate(ctx, data, resp.Diagnostics, t.Client)
 				if !resp.Diagnostics.HasError() {
 					resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
@@ -146,15 +147,20 @@ func (t *tagTypeResource) Update(ctx context.Context, req resource.UpdateRequest
 		}
 
 		tag := schemas.MapFromStateToTag(data)
+		if tag.ID == "" {
+			tag.ID = destinationTagSet.GetID() + "/" + strings.Split(state.ID.ValueString(), "/")[1]
+		}
 
 		// check to see that the tag is not applied to a tenant
 		isUsed, err := isTagUsedByTenants(ctx, t.Client, sourceTagSetSpaceID, tag)
 		if err != nil {
+			data.ID = types.StringValue("")
 			resp.Diagnostics.AddError("Failed to check if tag is used by tenants", err.Error())
 			return
 		}
 
 		if isUsed {
+			data.ID = types.StringValue("")
 			resp.Diagnostics.AddError("Tag in use", "the tag may not be transferred; it is being used by one or more tenant(s)")
 			return
 		}
@@ -163,7 +169,7 @@ func (t *tagTypeResource) Update(ctx context.Context, req resource.UpdateRequest
 
 		// remove the tag from the source tag set and update through the API
 		for i := 0; i < len(sourceTagSet.Tags); i++ {
-			if sourceTagSet.Tags[i].ID == data.ID.ValueString() {
+			if sourceTagSet.Tags[i].ID == state.ID.ValueString() {
 				sourceTagSet.Tags = slices.Delete(sourceTagSet.Tags, i, i+1)
 				if _, err := tagsets.Update(t.Client, sourceTagSet); err != nil {
 					resp.Diagnostics.AddError("Failed to update source tag set", err.Error())
@@ -201,7 +207,7 @@ func (t *tagTypeResource) Update(ctx context.Context, req resource.UpdateRequest
 	// find and update the tag that matches the one updated in configuration
 	var updatedTag *tagsets.Tag
 	for i := 0; i < len(tagSet.Tags); i++ {
-		if tagSet.Tags[i].ID == data.ID.ValueString() || tagSet.Tags[i].Name == data.Name.ValueString() {
+		if tagSet.Tags[i].ID == state.ID.ValueString() || tagSet.Tags[i].Name == data.Name.ValueString() {
 			tagSet.Tags[i] = schemas.MapFromStateToTag(data)
 
 			updatedTagSet, err := tagsets.Update(t.Client, tagSet)
@@ -212,7 +218,7 @@ func (t *tagTypeResource) Update(ctx context.Context, req resource.UpdateRequest
 
 			// Find the updated tag in the updatedTagSet
 			for _, t := range updatedTagSet.Tags {
-				if t.ID == data.ID.ValueString() || t.Name == data.Name.ValueString() {
+				if t.ID == state.ID.ValueString() || t.Name == data.Name.ValueString() {
 					updatedTag = t
 					break
 				}

octopusdeploy_framework/resource_tag_set_test.go

@@ -110,7 +110,8 @@ func testTagExists(n string) resource.TestCheckFunc {
 		}
 
 		tagSetID := rs.Primary.Attributes["tag_set_id"]
-		tagSet, err := tagsets.GetByID(octoClient, rs.Primary.Attributes["space_id"], tagSetID)
+		spaceID := rs.Primary.Attributes["tag_set_space_id"]
+		tagSet, err := tagsets.GetByID(octoClient, spaceID, tagSetID)
 		if err != nil {
 			return fmt.Errorf("error retrieving tag set: %s", err)
 		}

octopusdeploy_framework/resource_tag_test.go

@@ -2,6 +2,7 @@ package octopusdeploy_framework
 
 import (
 	"fmt"
+	"github.com/stretchr/testify/assert"
 	"testing"
 
 	"github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/tagsets"
@@ -12,7 +13,9 @@ import (
 
 func TestAccTag(t *testing.T) {
 	tagSetName := acctest.RandStringFromCharSet(20, acctest.CharSetAlpha)
+	tagSetMigrationName := acctest.RandStringFromCharSet(20, acctest.CharSetAlpha)
 	tagName := acctest.RandStringFromCharSet(20, acctest.CharSetAlpha)
+	tenantName := acctest.RandStringFromCharSet(20, acctest.CharSetAlpha)
 	tagColor := "#6e6e6e"
 	tagResourceName := "octopusdeploy_tag." + tagName
 
@@ -43,6 +46,12 @@ func TestAccTag(t *testing.T) {
 					resource.TestCheckResourceAttrSet(tagResourceName, "tag_set_space_id"),
 				),
 			},
+			{
+				Config: testTagMigrationConfigUpdate(tenantName, tagSetName, tagSetMigrationName, tagName, "#ff0000"),
+				Check: resource.ComposeTestCheckFunc(
+					testTagMigrationWorksAsExpected(t, tagResourceName, tagSetMigrationName),
+				),
+			},
 			{
 				ResourceName:      tagResourceName,
 				ImportState:       true,
@@ -53,6 +62,35 @@ func TestAccTag(t *testing.T) {
 	})
 }
 
+func testTagMigrationWorksAsExpected(t *testing.T, n string, tagSetMigrationName string) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		rs, ok := s.RootModule().Resources[n]
+		if !ok {
+			return fmt.Errorf("not found: %s", n)
+		}
+
+		if rs.Primary.ID == "" {
+			return fmt.Errorf("no ID is set")
+		}
+
+		tagSetID := rs.Primary.Attributes["tag_set_id"]
+		tagSet, err := tagsets.GetByID(octoClient, rs.Primary.Attributes["tag_set_space_id"], tagSetID)
+		assert.Equal(t, tagSet.Name, tagSetMigrationName)
+
+		if err != nil {
+			return fmt.Errorf("error retrieving tag set: %s", err)
+		}
+
+		for _, tag := range tagSet.Tags {
+			if tag.ID == rs.Primary.ID {
+				return nil
+			}
+		}
+
+		return fmt.Errorf("tag not found in tag set")
+	}
+}
+
 func testTagConfig(tagSetName, tagName, tagColor string) string {
 	var tfConfig = fmt.Sprintf(`
 		resource "octopusdeploy_tag_set" "%s" {
@@ -87,6 +125,35 @@ func testTagConfigUpdate(tagSetName, tagName, tagColor string) string {
 	return tfConfig
 }
 
+func testTagMigrationConfigUpdate(tenantName, tagSetName, tagSetMigrationName, tagName, tagColor string) string {
+	var tfConfig = fmt.Sprintf(`
+		resource "octopusdeploy_tenant" "tenant1" {
+			name        = "%s"
+		}
+
+		resource "octopusdeploy_tag_set" "%s" {
+			name        = "%s"
+			description = "Test tag set"
+  			depends_on = [ octopusdeploy_tenant.tenant1 ]
+		}
+
+		resource "octopusdeploy_tag_set" "%s" {
+			name        = "%s"
+			description = "Test tag set"
+			depends_on = [ octopusdeploy_tenant.tenant1 ]
+		}
+
+		resource "octopusdeploy_tag" "%s" {
+			name        = "%s"
+			color       = "%s"
+			description = "Updated test tag"
+			tag_set_id  = octopusdeploy_tag_set.%s.id
+			depends_on = [ octopusdeploy_tenant.tenant1 ]
+		}
+	`, tenantName, tagSetName, tagSetName, tagSetMigrationName, tagSetMigrationName, tagName, tagName, tagColor, tagSetMigrationName)
+	return tfConfig
+}
+
 func testAccTagDestroy(s *terraform.State) error {
 
 	for _, rs := range s.RootModule().Resources {