Refactor Sentry Project resource (#228)

* Refactor tests

* Refactor resourceSentryProjectUpdate

Author: jianyuan

docs/resources/project.md

@@ -41,8 +41,8 @@ resource "sentry_project" "default" {
 - `platform` (String) The optional platform for this project.
 - `resolve_age` (Number) Hours in which an issue is automatically resolve if not seen after this amount of time.
 - `slug` (String) The optional slug for this project.
-- `team` (String, Deprecated) The slug of the team to create the project for. One of 'team' or 'teams' must be set.
-- `teams` (Set of String) The slugs of the teams to create the project for. One of 'team' or 'teams' must be set.
+- `team` (String, Deprecated) The slug of the team to create the project for. **Deprecated** Use `teams` instead.
+- `teams` (Set of String) The slugs of the teams to create the project for.
 
 ### Read-Only
 

sentry/data_source_sentry_issue_alert_test.go

@@ -61,7 +61,7 @@ func TestAccSentryIssueAlertDataSource_basic(t *testing.T) {
 }
 
 func testAccSentryIssueAlertDataSourceConfig(teamName, projectName, alertName string) string {
-	return testAccSentryProjectConfig(teamName, projectName) + fmt.Sprintf(`
+	return testAccSentryProjectConfig_team(teamName, projectName) + fmt.Sprintf(`
 resource "sentry_issue_alert" "test" {
 	organization = sentry_project.test.organization
 	project      = sentry_project.test.id

sentry/data_source_sentry_key_test.go

@@ -94,7 +94,7 @@ func testAccCheckSentryKeyDataSourceID(n string) resource.TestCheckFunc {
 }
 
 func testAccSentryKeyDataSourceConfig(teamName, projectName string) string {
-	return testAccSentryProjectConfig(teamName, projectName) + `
+	return testAccSentryProjectConfig_team(teamName, projectName) + `
 data "sentry_key" "test" {
 	organization = sentry_project.test.organization
 	project      = sentry_project.test.id
@@ -104,7 +104,7 @@ data "sentry_key" "test" {
 
 // Testing first parameter
 func testAccSentryKeyDataSourceConfig_first(teamName, projectName string) string {
-	return testAccSentryProjectConfig(teamName, projectName) + `
+	return testAccSentryProjectConfig_team(teamName, projectName) + `
 resource "sentry_key" "test_2" {
 	organization = sentry_project.test.organization
 	project      = sentry_project.test.id
@@ -123,7 +123,7 @@ data "sentry_key" "test" {
 
 // Testing name parameter
 func testAccSentryKeyDataSourceConfig_name(teamName, projectName, keyName string) string {
-	return testAccSentryProjectConfig(teamName, projectName) + fmt.Sprintf(`
+	return testAccSentryProjectConfig_team(teamName, projectName) + fmt.Sprintf(`
 resource "sentry_key" "test_2" {
 	organization = sentry_project.test.organization
 	project      = sentry_project.test.id

sentry/data_source_sentry_metric_alert_test.go

@@ -69,7 +69,7 @@ func TestAccSentryMetricAlertDataSource_basic(t *testing.T) {
 }
 
 func testAccSentryMetricAlertDataSourceConfig(teamName, projectName, alertName string) string {
-	return testAccSentryProjectConfig(teamName, projectName) + fmt.Sprintf(`
+	return testAccSentryProjectConfig_team(teamName, projectName) + fmt.Sprintf(`
 resource "sentry_metric_alert" "test" {
 	organization      = sentry_project.test.organization
 	project           = sentry_project.test.id

sentry/provider.go

@@ -22,7 +22,7 @@ func NewProvider(version string) func() *schema.Provider {
 					Description: "The authentication token used to connect to Sentry. The value can be sourced from " +
 						"the `SENTRY_AUTH_TOKEN` environment variable.",
 					Type:        schema.TypeString,
-					Required:    true,
+					Optional:    true,
 					DefaultFunc: schema.MultiEnvDefaultFunc([]string{"SENTRY_AUTH_TOKEN", "SENTRY_TOKEN"}, nil),
 					Sensitive:   true,
 				},

sentry/resource_sentry_issue_alert_test.go

@@ -221,7 +221,7 @@ func testAccCheckSentryIssueAlertExists(n string, alertID *string) resource.Test
 }
 
 func testAccSentryIssueAlertConfig(teamName, projectName, alertName string) string {
-	return testAccSentryProjectConfig(teamName, projectName) + fmt.Sprintf(`
+	return testAccSentryProjectConfig_team(teamName, projectName) + fmt.Sprintf(`
 resource "sentry_issue_alert" "test" {
 	organization = sentry_project.test.organization
 	project      = sentry_project.test.id

sentry/resource_sentry_key_test.go

@@ -179,7 +179,7 @@ func testAccSentryKeyImportStateIdFunc(n string) resource.ImportStateIdFunc {
 }
 
 func testAccSentryKeyConfig(teamName, projectName, keyName string) string {
-	return testAccSentryProjectConfig(teamName, projectName) + fmt.Sprintf(`
+	return testAccSentryProjectConfig_team(teamName, projectName) + fmt.Sprintf(`
 resource "sentry_key" "test" {
 	organization      = sentry_project.test.organization
 	project           = sentry_project.test.id
@@ -189,7 +189,7 @@ resource "sentry_key" "test" {
 }
 
 func testAccSentryKeyConfig_rateLimit(teamName, projectName, keyName, rateLimitWindow, rateLimitCount string) string {
-	return testAccSentryProjectConfig(teamName, projectName) + fmt.Sprintf(`
+	return testAccSentryProjectConfig_team(teamName, projectName) + fmt.Sprintf(`
 resource "sentry_key" "test" {
 	organization      = sentry_project.test.organization
 	project           = sentry_project.test.id

sentry/resource_sentry_metric_alert_test.go

@@ -115,7 +115,7 @@ func testAccCheckSentryMetricAlertExists(n string, gotAlertID *string) resource.
 }
 
 func testAccSentryMetricAlertConfig(teamName, projectName, alertName string) string {
-	return testAccSentryProjectConfig(teamName, projectName) + fmt.Sprintf(`
+	return testAccSentryProjectConfig_team(teamName, projectName) + fmt.Sprintf(`
 resource "sentry_metric_alert" "test" {
 	organization      = sentry_project.test.organization
 	project           = sentry_project.test.id

sentry/resource_sentry_project.go

@@ -2,6 +2,7 @@ package sentry
 
 import (
 	"context"
+	"errors"
 	"net/http"
 
 	"github.com/hashicorp/go-multierror"
@@ -31,19 +32,20 @@ func resourceSentryProject() *schema.Resource {
 				Required:    true,
 			},
 			"team": {
-				Description: "The slug of the team to create the project for. One of 'team' or 'teams' must be set.",
-				Type:        schema.TypeString,
-				Deprecated:  "To be replaced by 'teams' in a future release.",
-				Optional:    true,
+				Description:   "The slug of the team to create the project for. **Deprecated** Use `teams` instead.",
+				Type:          schema.TypeString,
+				Deprecated:    "Use `teams` instead.",
+				ConflictsWith: []string{"teams"},
+				Optional:      true,
 			},
 			"teams": {
-				Description: "The slugs of the teams to create the project for. One of 'team' or 'teams' must be set.",
+				Description: "The slugs of the teams to create the project for.",
 				Type:        schema.TypeSet,
 				Elem: &schema.Schema{
 					Type: schema.TypeString,
 				},
-				ExactlyOneOf: []string{"team"},
-				Optional:     true,
+				ConflictsWith: []string{"team"},
+				Optional:      true,
 			},
 			"name": {
 				Description: "The name for the project.",
@@ -124,32 +126,42 @@ func resourceSentryProjectCreate(ctx context.Context, d *schema.ResourceData, me
 	client := meta.(*sentry.Client)
 
 	org := d.Get("organization").(string)
-	team := d.Get("team").(string)
-	var teams []interface{}
-	if team == "" {
+
+	team, teamOk := d.GetOk("team")
+	teams, teamsOk := d.GetOk("teams")
+	if !teamOk && !teamsOk {
+		return diag.FromErr(errors.New("one of team or teams must be configured"))
+	}
+
+	var initialTeam string
+	if teamOk {
+		initialTeam = team.(string)
+	} else {
 		// Since `Set.List()` produces deterministic ordering, `teams[0]` should always
 		// resolve to the same value given the same `teams`.
-		teams = d.Get("teams").(*schema.Set).List()
-		team = teams[0].(string)
+		// Pick the first team when creating the project.
+		initialTeam = teams.(*schema.Set).List()[0].(string)
 	}
+
 	params := &sentry.CreateProjectParams{
 		Name: d.Get("name").(string),
 		Slug: d.Get("slug").(string),
 	}
 
 	tflog.Debug(ctx, "Creating Sentry project", map[string]interface{}{
-		"team":  team,
-		"teams": teams,
-		"org":   org,
+		"team":        team,
+		"teams":       teams,
+		"org":         org,
+		"initialTeam": initialTeam,
 	})
-	proj, _, err := client.Projects.Create(ctx, org, team, params)
+	proj, _, err := client.Projects.Create(ctx, org, initialTeam, params)
 	if err != nil {
 		return diag.FromErr(err)
 	}
 	tflog.Debug(ctx, "Created Sentry project", map[string]interface{}{
 		"projectSlug": proj.Slug,
 		"projectID":   proj.ID,
-		"team":        team,
+		"team":        initialTeam,
 		"org":         org,
 	})
 
@@ -177,31 +189,11 @@ func resourceSentryProjectRead(ctx context.Context, d *schema.ResourceData, meta
 		"org":         org,
 	})
 
-	setTeams := func() error {
-		if len(proj.Teams) <= 1 {
-			return multierror.Append(
-				d.Set("team", proj.Team.Slug),
-				d.Set("teams", nil),
-			)
-		}
-
-		teams := make([]string, len(proj.Teams))
-		for i, team := range proj.Teams {
-			teams[i] = *team.Slug
-		}
-
-		return multierror.Append(
-			d.Set("team", nil),
-			d.Set("teams", teams),
-		)
-	}
-
 	d.SetId(proj.Slug)
 	retErr := multierror.Append(
 		d.Set("organization", proj.Organization.Slug),
 		d.Set("name", proj.Name),
 		d.Set("slug", proj.Slug),
-		setTeams(),
 		d.Set("platform", proj.Platform),
 		d.Set("internal_id", proj.ID),
 		d.Set("is_public", proj.IsPublic),
@@ -213,6 +205,15 @@ func resourceSentryProjectRead(ctx context.Context, d *schema.ResourceData, meta
 		d.Set("resolve_age", proj.ResolveAge),
 		d.Set("project_id", proj.ID), // Deprecated
 	)
+	if _, ok := d.GetOk("team"); ok {
+		retErr = multierror.Append(retErr, d.Set("team", proj.Team.Slug))
+	} else {
+		teams := make([]string, 0, len(proj.Teams))
+		for _, team := range proj.Teams {
+			teams = append(teams, *team.Slug)
+		}
+		retErr = multierror.Append(retErr, d.Set("teams", flattenStringSet(teams)))
+	}
 
 	// TODO: Project options
 
@@ -285,35 +286,37 @@ func resourceSentryProjectUpdate(ctx context.Context, d *schema.ResourceData, me
 
 	// Ensure old teams and new teams do not overlap.
 	for newTeam := range newTeams {
-		if _, exists := oldTeams[newTeam]; exists {
-			delete(oldTeams, newTeam)
-		}
+		delete(oldTeams, newTeam)
 	}
 
-	tflog.Debug(ctx, "Adding teams to project", map[string]interface{}{
-		"org":        org,
-		"project":    project,
-		"teamsToAdd": newTeams,
-	})
+	if len(newTeams) > 0 {
+		tflog.Debug(ctx, "Adding teams to project", map[string]interface{}{
+			"org":        org,
+			"project":    project,
+			"teamsToAdd": newTeams,
+		})
 
-	for newTeam := range newTeams {
-		_, _, err = client.Projects.AddTeam(ctx, org, project, newTeam)
-		if err != nil {
-			return diag.FromErr(err)
+		for newTeam := range newTeams {
+			_, _, err = client.Projects.AddTeam(ctx, org, project, newTeam)
+			if err != nil {
+				return diag.FromErr(err)
+			}
 		}
 	}
 
-	tflog.Debug(ctx, "Removing teams from project", map[string]interface{}{
-		"org":           org,
-		"project":       project,
-		"teamsToRemove": oldTeams,
-	})
-
-	for oldTeam := range oldTeams {
-		resp, err := client.Projects.RemoveTeam(ctx, org, project, oldTeam)
-		if err != nil {
-			if resp.Response.StatusCode != http.StatusNotFound {
-				return diag.FromErr(err)
+	if len(oldTeams) > 0 {
+		tflog.Debug(ctx, "Removing teams from project", map[string]interface{}{
+			"org":           org,
+			"project":       project,
+			"teamsToRemove": oldTeams,
+		})
+
+		for oldTeam := range oldTeams {
+			resp, err := client.Projects.RemoveTeam(ctx, org, project, oldTeam)
+			if err != nil {
+				if resp.Response.StatusCode != http.StatusNotFound {
+					return diag.FromErr(err)
+				}
 			}
 		}
 	}