From ff70090ee92d0e82f187667447e6da3cc410ac67 Mon Sep 17 00:00:00 2001 From: Isaac Calligeros <101079287+IsaacCalligeros95@users.noreply.github.com> Date: Mon, 19 Aug 2024 17:13:14 +0930 Subject: [PATCH] Fix migrating tag across sets (#755) --- octopusdeploy_framework/resource_tag.go | 18 +++-- .../resource_tag_set_test.go | 3 +- octopusdeploy_framework/resource_tag_test.go | 67 +++++++++++++++++++ 3 files changed, 81 insertions(+), 7 deletions(-) diff --git a/octopusdeploy_framework/resource_tag.go b/octopusdeploy_framework/resource_tag.go index fcc2cc2fd..f8b46dad2 100644 --- a/octopusdeploy_framework/resource_tag.go +++ b/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 } diff --git a/octopusdeploy_framework/resource_tag_set_test.go b/octopusdeploy_framework/resource_tag_set_test.go index 49b8b04ad..a631235b2 100644 --- a/octopusdeploy_framework/resource_tag_set_test.go +++ b/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) } diff --git a/octopusdeploy_framework/resource_tag_test.go b/octopusdeploy_framework/resource_tag_test.go index 710f8d235..6959d3735 100644 --- a/octopusdeploy_framework/resource_tag_test.go +++ b/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 {