Add guard for invalid deploymentId in team creation (#144)

astronomer · Aug 28, 2024 · 180adf9 · 180adf9
1 parent a82d7a0
commit 180adf9
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 8 deletions.
diff --git a/internal/provider/common/role.go b/internal/provider/common/role.go
@@ -94,14 +94,15 @@ func ValidateWorkspaceDeploymentRoles(ctx context.Context, input ValidateWorkspa
 		return nil
 	}
 
-	// get list of deployment ids
-	deploymentIds := lo.Map(input.DeploymentRoles, func(role iam.DeploymentRole, _ int) string {
+	// get list of deploymentRole ids
+	deploymentRoleIds := lo.Map(input.DeploymentRoles, func(role iam.DeploymentRole, _ int) string {
 		return role.DeploymentId
 	})
+	deploymentRoleIds = lo.Uniq(deploymentRoleIds)
 
 	// get list of deployments
 	listDeployments, err := input.PlatformClient.ListDeploymentsWithResponse(ctx, input.OrganizationId, &platform.ListDeploymentsParams{
-		DeploymentIds: &deploymentIds,
+		DeploymentIds: &deploymentRoleIds,
 	})
 	if err != nil {
 		tflog.Error(ctx, "failed to mutate roles", map[string]interface{}{"error": err})
@@ -116,20 +117,36 @@ func ValidateWorkspaceDeploymentRoles(ctx context.Context, input ValidateWorkspa
 		return diag.Diagnostics{diagnostic}
 	}
 
+	// get list of deployment ids
+	deploymentIds := lo.Map(listDeployments.JSON200.Deployments, func(deployment platform.Deployment, _ int) string {
+		return deployment.Id
+	})
+
+	// check if deploymentRole ids are in list of deployments
+	invalidDeploymentIds, _ := lo.Difference(deploymentRoleIds, deploymentIds)
+	if len(invalidDeploymentIds) > 0 {
+		tflog.Error(ctx, "failed to mutate roles")
+		return diag.Diagnostics{diag.NewErrorDiagnostic(
+			"Unable to mutate roles, not every deployment role has a corresponding valid deployment",
+			fmt.Sprintf("Please ensure that every deployment role has a corresponding deployment, got invalid deployment ids: %v", invalidDeploymentIds),
+		),
+		}
+	}
+
 	// get list of workspace ids from deployments
 	deploymentWorkspaceIds := lo.Map(listDeployments.JSON200.Deployments, func(deployment platform.Deployment, _ int) string {
 		return deployment.WorkspaceId
 	})
 	deploymentWorkspaceIds = lo.Uniq(deploymentWorkspaceIds)
 
-	// get list of workspaceIds
-	workspaceIds := lo.Map(input.WorkspaceRoles, func(role iam.WorkspaceRole, _ int) string {
+	// get list of workspaceRole ids
+	workspaceRoleIds := lo.Map(input.WorkspaceRoles, func(role iam.WorkspaceRole, _ int) string {
 		return role.WorkspaceId
 	})
 
-	// check if deploymentWorkspaceIds are in workspaceIds
-	workspaceIds = lo.Intersect(lo.Uniq(workspaceIds), deploymentWorkspaceIds)
-	if len(workspaceIds) != len(deploymentWorkspaceIds) {
+	// check if deploymentWorkspaceIds are in workspaceRoleIds
+	workspaceRoleIds = lo.Intersect(lo.Uniq(workspaceRoleIds), deploymentWorkspaceIds)
+	if len(workspaceRoleIds) != len(deploymentWorkspaceIds) {
 		tflog.Error(ctx, "failed to mutate roles")
 		return diag.Diagnostics{diag.NewErrorDiagnostic(
 			"Unable to mutate roles, not every deployment role has a corresponding workspace role",

diff --git a/internal/provider/resources/resource_team.go b/internal/provider/resources/resource_team.go
@@ -197,6 +197,28 @@ func (r *TeamResource) Create(
 		diags = r.MutateRoles(ctx, &data, teamId)
 		if diags.HasError() {
 			resp.Diagnostics.Append(diags...)
+
+			// if there is an error in creating team with workspace or deployment roles, delete the team
+			team, err := r.IamClient.DeleteTeamWithResponse(
+				ctx,
+				r.OrganizationId,
+				teamId,
+			)
+			if err != nil {
+				tflog.Error(ctx, "failed to delete Team", map[string]interface{}{"error": err})
+				resp.Diagnostics.AddError(
+					"Client Error",
+					fmt.Sprintf("Unable to delete Team, got error: %s", err),
+				)
+				return
+			}
+			statusCode, diagnostic := clients.NormalizeAPIError(ctx, team.HTTPResponse, team.Body)
+			// It is recommended to ignore 404 Resource Not Found errors when deleting a resource
+			if statusCode != http.StatusNotFound && diagnostic != nil {
+				resp.Diagnostics.Append(diagnostic)
+				return
+			}
+
 			return
 		}
 	}

diff --git a/internal/provider/resources/resource_team_test.go b/internal/provider/resources/resource_team_test.go
@@ -8,6 +8,8 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/lucsky/cuid"
+
 	"github.com/astronomer/terraform-provider-astro/internal/clients"
 	"github.com/astronomer/terraform-provider-astro/internal/clients/iam"
 	astronomerprovider "github.com/astronomer/terraform-provider-astro/internal/provider"
@@ -110,6 +112,28 @@ func TestAcc_ResourceTeam(t *testing.T) {
 				}),
 				ExpectError: regexp.MustCompile("Invalid Configuration: Cannot have multiple roles with the same workspace id"),
 			},
+			// Test failure: check for invalid deploymentId
+			{
+				Config: astronomerprovider.ProviderConfig(t, astronomerprovider.HOSTED) + team(teamInput{
+					Name:             failTeamName,
+					Description:      utils.TestResourceDescription,
+					MemberIds:        []string{userId},
+					OrganizationRole: string(iam.ORGANIZATIONOWNER),
+					WorkspaceRoles: []utils.Role{
+						{
+							Role:     string(iam.WORKSPACEOWNER),
+							EntityId: workspaceId,
+						},
+					},
+					DeploymentRoles: []utils.Role{
+						{
+							Role:     "DEPLOYMENT_ADMIN",
+							EntityId: cuid.New(),
+						},
+					},
+				}),
+				ExpectError: regexp.MustCompile("Unable to mutate roles, not every deployment role has a corresponding valid deployment"),
+			},
 			// Create team with all fields
 			{
 				Config: astronomerprovider.ProviderConfig(t, astronomerprovider.HOSTED) + team(teamInput{