feat: Fixup repository environment logic (#3162)

Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>

Author: stevehipwell

github/resource_github_repository_environment.go

@@ -3,32 +3,40 @@ package github
 import (
 	"context"
 	"errors"
-	"log"
+	"fmt"
 	"net/http"
 	"net/url"
 
 	"github.com/google/go-github/v83/github"
+	"github.com/hashicorp/terraform-plugin-log/tflog"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
 )
 
 func resourceGithubRepositoryEnvironment() *schema.Resource {
 	return &schema.Resource{
-		CreateContext: resourceGithubRepositoryEnvironmentCreate,
-		ReadContext:   resourceGithubRepositoryEnvironmentRead,
-		UpdateContext: resourceGithubRepositoryEnvironmentUpdate,
-		DeleteContext: resourceGithubRepositoryEnvironmentDelete,
-		Importer: &schema.ResourceImporter{
-			StateContext: resourceGithubRepositoryEnvironmentImport,
+		SchemaVersion: 1,
+		StateUpgraders: []schema.StateUpgrader{
+			{
+				Type:    resourceGithubRepositoryEnvironmentV0().CoreConfigSchema().ImpliedType(),
+				Upgrade: resourceGithubRepositoryEnvironmentStateUpgradeV0,
+				Version: 0,
+			},
 		},
+
 		Schema: map[string]*schema.Schema{
 			"repository": {
 				Type:        schema.TypeString,
 				Required:    true,
-				ForceNew:    true,
 				Description: "The repository of the environment.",
 			},
+			"repository_id": {
+				Description: "The ID of the GitHub repository.",
+				Type:        schema.TypeInt,
+				Computed:    true,
+			},
 			"environment": {
 				Type:        schema.TypeString,
 				Required:    true,
@@ -56,20 +64,22 @@ func resourceGithubRepositoryEnvironment() *schema.Resource {
 			"reviewers": {
 				Type:        schema.TypeList,
 				Optional:    true,
-				MaxItems:    6,
+				MaxItems:    1,
 				Description: "The environment reviewers configuration.",
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"teams": {
 							Type:        schema.TypeSet,
-							Optional:    true,
 							Elem:        &schema.Schema{Type: schema.TypeInt},
+							Optional:    true,
+							MaxItems:    6,
 							Description: "Up to 6 IDs for teams who may review jobs that reference the environment. Reviewers must have at least read access to the repository. Only one of the required reviewers needs to approve the job for it to proceed.",
 						},
 						"users": {
 							Type:        schema.TypeSet,
-							Optional:    true,
 							Elem:        &schema.Schema{Type: schema.TypeInt},
+							Optional:    true,
+							MaxItems:    6,
 							Description: "Up to 6 IDs for users who may review jobs that reference the environment. Reviewers must have at least read access to the repository. Only one of the required reviewers needs to approve the job for it to proceed.",
 						},
 					},
@@ -96,12 +106,50 @@ func resourceGithubRepositoryEnvironment() *schema.Resource {
 				},
 			},
 		},
+
+		CustomizeDiff: customdiff.All(
+			diffRepository,
+			resourceGithubRepositoryEnvironmentDiff,
+		),
+
+		CreateContext: resourceGithubRepositoryEnvironmentCreate,
+		ReadContext:   resourceGithubRepositoryEnvironmentRead,
+		UpdateContext: resourceGithubRepositoryEnvironmentUpdate,
+		DeleteContext: resourceGithubRepositoryEnvironmentDelete,
+		Importer: &schema.ResourceImporter{
+			StateContext: resourceGithubRepositoryEnvironmentImport,
+		},
+	}
+}
+
+func resourceGithubRepositoryEnvironmentDiff(_ context.Context, d *schema.ResourceDiff, _ any) error {
+	if d.Id() == "" {
+		return nil
+	}
+
+	if v, ok := d.GetOk("reviewers"); ok {
+		count := 0
+		o := v.([]any)[0]
+		if t, ok := o.(map[string]any)["teams"]; ok {
+			count += t.(*schema.Set).Len()
+		}
+
+		if t, ok := o.(map[string]any)["users"]; ok {
+			count += t.(*schema.Set).Len()
+		}
+
+		if count > 6 {
+			return fmt.Errorf("a maximum of 6 reviewers (users and teams combined) can be set for an environment")
+		}
 	}
+
+	return nil
 }
 
-func resourceGithubRepositoryEnvironmentCreate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
-	client := meta.(*Owner).v3client
-	owner := meta.(*Owner).name
+func resourceGithubRepositoryEnvironmentCreate(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics {
+	meta := m.(*Owner)
+	client := meta.v3client
+	owner := meta.name
 
 	repoName := d.Get("repository").(string)
 	envName := d.Get("environment").(string)
@@ -118,41 +166,50 @@ func resourceGithubRepositoryEnvironmentCreate(ctx context.Context, d *schema.Re
 	}
 	d.SetId(id)
 
+	repo, _, err := client.Repositories.Get(ctx, owner, repoName)
+	if err != nil {
+		return diag.FromErr(err)
+	}
+
+	if err := d.Set("repository_id", int(repo.GetID())); err != nil {
+		return diag.FromErr(err)
+	}
+
 	return nil
 }
 
-func resourceGithubRepositoryEnvironmentRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
-	client := meta.(*Owner).v3client
-	owner := meta.(*Owner).name
+func resourceGithubRepositoryEnvironmentRead(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics {
+	ctx = tflog.SetField(ctx, "id", d.Id())
 
-	repoName, envNamePart, err := parseID2(d.Id())
-	if err != nil {
-		return diag.FromErr(err)
-	}
+	meta := m.(*Owner)
+	client := meta.v3client
+	owner := meta.name
 
-	envName := unescapeIDPart(envNamePart)
+	repoName := d.Get("repository").(string)
+	envName := d.Get("environment").(string)
 
 	env, _, err := client.Repositories.GetEnvironment(ctx, owner, repoName, url.PathEscape(envName))
 	if err != nil {
 		var ghErr *github.ErrorResponse
 		if errors.As(err, &ghErr) {
 			if ghErr.Response.StatusCode == http.StatusNotFound {
-				log.Printf("[INFO] Removing repository environment %s from state because it no longer exists in GitHub",
-					d.Id())
+				tflog.Info(ctx, "Repository environment not found, removing from state.", map[string]any{"repository": repoName, "environment": envName})
 				d.SetId("")
 				return nil
 			}
 		}
 		return diag.FromErr(err)
 	}
 
-	_ = d.Set("repository", repoName)
-	_ = d.Set("environment", envName)
-	_ = d.Set("wait_timer", nil)
-	_ = d.Set("can_admins_bypass", env.CanAdminsBypass)
+	if err := d.Set("wait_timer", nil); err != nil {
+		return diag.FromErr(err)
+	}
+	if err := d.Set("can_admins_bypass", env.CanAdminsBypass); err != nil {
+		return diag.FromErr(err)
+	}
 
 	for _, pr := range env.ProtectionRules {
-		switch *pr.Type {
+		switch pr.GetType() {
 		case "wait_timer":
 			if err = d.Set("wait_timer", pr.WaitTimer); err != nil {
 				return diag.FromErr(err)
@@ -199,15 +256,18 @@ func resourceGithubRepositoryEnvironmentRead(ctx context.Context, d *schema.Reso
 			return diag.FromErr(err)
 		}
 	} else {
-		_ = d.Set("deployment_branch_policy", []any{})
+		if err := d.Set("deployment_branch_policy", []any{}); err != nil {
+			return diag.FromErr(err)
+		}
 	}
 
 	return nil
 }
 
-func resourceGithubRepositoryEnvironmentUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
-	client := meta.(*Owner).v3client
-	owner := meta.(*Owner).name
+func resourceGithubRepositoryEnvironmentUpdate(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics {
+	meta := m.(*Owner)
+	client := meta.v3client
+	owner := meta.name
 
 	repoName := d.Get("repository").(string)
 	envName := d.Get("environment").(string)
@@ -227,34 +287,43 @@ func resourceGithubRepositoryEnvironmentUpdate(ctx context.Context, d *schema.Re
 	return nil
 }
 
-func resourceGithubRepositoryEnvironmentDelete(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
-	client := meta.(*Owner).v3client
-	owner := meta.(*Owner).name
+func resourceGithubRepositoryEnvironmentDelete(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics {
+	meta := m.(*Owner)
+	client := meta.v3client
+	owner := meta.name
 
-	repoName, envNamePart, err := parseID2(d.Id())
-	if err != nil {
-		return diag.FromErr(err)
-	}
-
-	envName := unescapeIDPart(envNamePart)
+	repoName := d.Get("repository").(string)
+	envName := d.Get("environment").(string)
 
-	_, err = client.Repositories.DeleteEnvironment(ctx, owner, repoName, url.PathEscape(envName))
+	_, err := client.Repositories.DeleteEnvironment(ctx, owner, repoName, url.PathEscape(envName))
 	if err != nil {
 		return diag.FromErr(deleteResourceOn404AndSwallow304OtherwiseReturnError(err, d, "environment (%s)", envName))
 	}
 
 	return nil
 }
 
-func resourceGithubRepositoryEnvironmentImport(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) {
+func resourceGithubRepositoryEnvironmentImport(ctx context.Context, d *schema.ResourceData, m any) ([]*schema.ResourceData, error) {
+	meta := m.(*Owner)
+	client := meta.v3client
+	owner := meta.name
+
 	repoName, envNamePart, err := parseID2(d.Id())
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("invalid id (%s), expected format <repository>:<environment>", d.Id())
+	}
+
+	repo, _, err := client.Repositories.Get(ctx, owner, repoName)
+	if err != nil {
+		return nil, fmt.Errorf("failed to retrieve repository %s: %w", repoName, err)
 	}
 
 	if err := d.Set("repository", repoName); err != nil {
 		return nil, err
 	}
+	if err := d.Set("repository_id", int(repo.GetID())); err != nil {
+		return nil, err
+	}
 	if err := d.Set("environment", unescapeIDPart(envNamePart)); err != nil {
 		return nil, err
 	}