fix: Fix repository environment deployment (#2993)

Signed-off-by: Steve Hipwell <[email protected]>

Author: stevehipwell

github/data_source_github_repository_environment_deployment_policies.go

@@ -4,12 +4,13 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
 
 func dataSourceGithubRepositoryEnvironmentDeploymentPolicies() *schema.Resource {
 	return &schema.Resource{
-		Read: dataSourceGithubRepositoryEnvironmentDeploymentPoliciesRead,
+		ReadContext: dataSourceGithubRepositoryEnvironmentDeploymentPoliciesRead,
 
 		Schema: map[string]*schema.Schema{
 			"repository": {
@@ -44,26 +45,31 @@ func dataSourceGithubRepositoryEnvironmentDeploymentPolicies() *schema.Resource
 	}
 }
 
-func dataSourceGithubRepositoryEnvironmentDeploymentPoliciesRead(d *schema.ResourceData, meta any) error {
+func dataSourceGithubRepositoryEnvironmentDeploymentPoliciesRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
 	client := meta.(*Owner).v3client
 	owner := meta.(*Owner).name
 	repoName := d.Get("repository").(string)
-	environmentName := d.Get("environment_name").(string)
+	environmentName := d.Get("environment").(string)
 
-	policies, _, err := client.Repositories.ListDeploymentBranchPolicies(context.Background(), owner, repoName, environmentName)
+	policies, _, err := client.Repositories.ListDeploymentBranchPolicies(ctx, owner, repoName, environmentName)
 	if err != nil {
-		return err
+		return diag.FromErr(err)
 	}
 
 	results := make([]map[string]any, 0)
 
 	for _, policy := range policies.BranchPolicies {
 		policyMap := make(map[string]any)
-		policyMap["type"] = policy.Type
+		policyMap["type"] = policy.GetType()
 		policyMap["pattern"] = policy.GetName()
 		results = append(results, policyMap)
 	}
 
 	d.SetId(fmt.Sprintf("%s:%s", repoName, environmentName))
-	return d.Set("policies", results)
+	err = d.Set("policies", results)
+	if err != nil {
+		return diag.FromErr(err)
+	}
+
+	return nil
 }

github/data_source_github_repository_environment_deployment_policies_test.go

@@ -36,23 +36,16 @@ func TestAccGithubRepositoryEnvironmentDeploymentPolicies(t *testing.T) {
 			resource "github_repository_environment_deployment_policy" "tag" {
 				repository       = github_repository.test.name
 				environment      = github_repository_environment.env.environment
-				tag_pattern   = "bar"
+				tag_pattern      = "bar"
 			}
-	`, randomID)
 
-		config2 := config + `
-			data "github_repository_environment_deployment_policies" "all" {
+			data "github_repository_environment_deployment_policies" "test" {
 				repository  = github_repository.test.name
 				environment = github_repository_environment.env.environment
+
+				depends_on = [github_repository_environment_deployment_policy.branch, github_repository_environment_deployment_policy.tag]
 			}
-		`
-		check := resource.ComposeTestCheckFunc(
-			resource.TestCheckResourceAttr("data.github_repository_environment_deployment_policies.all", "policies.#", "2"),
-			resource.TestCheckResourceAttr("data.github_repository_environment_deployment_policies.all", "policies.0.type", "branch"),
-			resource.TestCheckResourceAttr("data.github_repository_environment_deployment_policies.all", "policies.0.name", "foo"),
-			resource.TestCheckResourceAttr("data.github_repository_environment_deployment_policies.all", "policies.1.type", "tag"),
-			resource.TestCheckResourceAttr("data.github_repository_environment_deployment_policies.all", "policies.1.name", "bar"),
-		)
+	`, randomID)
 
 		testCase := func(t *testing.T, mode string) {
 			resource.Test(t, resource.TestCase{
@@ -63,8 +56,14 @@ func TestAccGithubRepositoryEnvironmentDeploymentPolicies(t *testing.T) {
 						Config: config,
 					},
 					{
-						Config: config2,
-						Check:  check,
+						Config: config,
+						Check: resource.ComposeTestCheckFunc(
+							resource.TestCheckResourceAttr("data.github_repository_environment_deployment_policies.test", "policies.#", "2"),
+							resource.TestCheckResourceAttr("data.github_repository_environment_deployment_policies.test", "policies.0.type", "branch"),
+							resource.TestCheckResourceAttr("data.github_repository_environment_deployment_policies.test", "policies.0.pattern", "foo"),
+							resource.TestCheckResourceAttr("data.github_repository_environment_deployment_policies.test", "policies.1.type", "tag"),
+							resource.TestCheckResourceAttr("data.github_repository_environment_deployment_policies.test", "policies.1.pattern", "bar"),
+						),
 					},
 				},
 			})

github/resource_github_actions_organization_secret.go

@@ -23,6 +23,9 @@ func resourceGithubActionsOrganizationSecret() *schema.Resource {
 				if err := d.Set("secret_name", d.Id()); err != nil {
 					return nil, err
 				}
+				if err := d.Set("destroy_on_drift", true); err != nil {
+					return nil, err
+				}
 				return []*schema.ResourceData{d}, nil
 			},
 		},

github/resource_github_repository_environment_deployment_policy.go

@@ -3,60 +3,74 @@ package github
 import (
 	"context"
 	"errors"
-	"fmt"
 	"log"
 	"net/http"
 	"net/url"
 	"strconv"
 
 	"github.com/google/go-github/v67/github"
+	"github.com/hashicorp/go-cty/cty"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
 
 func resourceGithubRepositoryEnvironmentDeploymentPolicy() *schema.Resource {
 	return &schema.Resource{
-		Create: resourceGithubRepositoryEnvironmentDeploymentPolicyCreate,
-		Read:   resourceGithubRepositoryEnvironmentDeploymentPolicyRead,
-		Update: resourceGithubRepositoryEnvironmentDeploymentPolicyUpdate,
-		Delete: resourceGithubRepositoryEnvironmentDeploymentPolicyDelete,
+		CreateContext: resourceGithubRepositoryEnvironmentDeploymentPolicyCreate,
+		ReadContext:   resourceGithubRepositoryEnvironmentDeploymentPolicyRead,
+		UpdateContext: resourceGithubRepositoryEnvironmentDeploymentPolicyUpdate,
+		DeleteContext: resourceGithubRepositoryEnvironmentDeploymentPolicyDelete,
 		Importer: &schema.ResourceImporter{
 			StateContext: schema.ImportStatePassthroughContext,
 		},
 		Schema: map[string]*schema.Schema{
 			"repository": {
+				Description: "The name of the GitHub repository.",
 				Type:        schema.TypeString,
 				Required:    true,
 				ForceNew:    true,
-				Description: "The name of the GitHub repository.",
 			},
 			"environment": {
+				Description: "The name of the environment.",
 				Type:        schema.TypeString,
 				Required:    true,
 				ForceNew:    true,
-				Description: "The name of the environment.",
 			},
 			"branch_pattern": {
-				Type:          schema.TypeString,
-				Optional:      true,
-				ForceNew:      false,
-				ConflictsWith: []string{"tag_pattern"},
-				Description:   "The name pattern that branches must match in order to deploy to the environment.",
+				Description:  "The name pattern that branches must match in order to deploy to the environment.",
+				Type:         schema.TypeString,
+				Optional:     true,
+				ForceNew:     false,
+				ExactlyOneOf: []string{"branch_pattern", "tag_pattern"},
+				ValidateDiagFunc: func(i any, _ cty.Path) diag.Diagnostics {
+					str, ok := i.(string)
+					if ok && len(str) > 0 {
+						return nil
+					}
+					return diag.Errorf("`branch_pattern` must be a valid non-empty string")
+				},
 			},
 			"tag_pattern": {
-				Type:          schema.TypeString,
-				Optional:      true,
-				ForceNew:      false,
-				ConflictsWith: []string{"branch_pattern"},
-				Description:   "The name pattern that tags must match in order to deploy to the environment.",
+				Description:  "The name pattern that tags must match in order to deploy to the environment.",
+				Type:         schema.TypeString,
+				Optional:     true,
+				ForceNew:     false,
+				ExactlyOneOf: []string{"branch_pattern", "tag_pattern"},
+				ValidateDiagFunc: func(i any, _ cty.Path) diag.Diagnostics {
+					str, ok := i.(string)
+					if ok && len(str) > 0 {
+						return nil
+					}
+					return diag.Errorf("`tag_pattern` must be a valid non-empty string")
+				},
 			},
 		},
 		CustomizeDiff: customDeploymentPolicyDiffFunction,
 	}
 }
 
-func resourceGithubRepositoryEnvironmentDeploymentPolicyCreate(d *schema.ResourceData, meta any) error {
+func resourceGithubRepositoryEnvironmentDeploymentPolicyCreate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
 	client := meta.(*Owner).v3client
-	ctx := context.Background()
 
 	owner := meta.(*Owner).name
 	repoName := d.Get("repository").(string)
@@ -75,31 +89,30 @@ func resourceGithubRepositoryEnvironmentDeploymentPolicyCreate(d *schema.Resourc
 			Type: github.String("tag"),
 		}
 	} else {
-		return fmt.Errorf("exactly one of %q and %q must be specified", "branch_pattern", "tag_pattern")
+		return diag.Errorf("only one of 'branch_pattern' or 'tag_pattern' must be specified")
 	}
 
 	resultKey, _, err := client.Repositories.CreateDeploymentBranchPolicy(ctx, owner, repoName, escapedEnvName, &createData)
 	if err != nil {
-		return err
+		return diag.FromErr(err)
 	}
 
 	d.SetId(buildThreePartID(repoName, escapedEnvName, strconv.FormatInt(resultKey.GetID(), 10)))
-	return resourceGithubRepositoryEnvironmentDeploymentPolicyRead(d, meta)
+	return nil
 }
 
-func resourceGithubRepositoryEnvironmentDeploymentPolicyRead(d *schema.ResourceData, meta any) error {
+func resourceGithubRepositoryEnvironmentDeploymentPolicyRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
 	client := meta.(*Owner).v3client
-	ctx := context.WithValue(context.Background(), ctxId, d.Id())
 
 	owner := meta.(*Owner).name
 	repoName, envName, branchPolicyIdString, err := parseThreePartID(d.Id(), "repository", "environment", "branchPolicyId")
 	if err != nil {
-		return err
+		return diag.FromErr(err)
 	}
 
 	branchPolicyId, err := strconv.ParseInt(branchPolicyIdString, 10, 64)
 	if err != nil {
-		return err
+		return diag.FromErr(err)
 	}
 
 	branchPolicy, _, err := client.Repositories.GetDeploymentBranchPolicy(ctx, owner, repoName, envName, branchPolicyId)
@@ -116,7 +129,7 @@ func resourceGithubRepositoryEnvironmentDeploymentPolicyRead(d *schema.ResourceD
 				return nil
 			}
 		}
-		return err
+		return diag.FromErr(err)
 	}
 
 	if branchPolicy.GetType() == "branch" {
@@ -127,9 +140,8 @@ func resourceGithubRepositoryEnvironmentDeploymentPolicyRead(d *schema.ResourceD
 	return nil
 }
 
-func resourceGithubRepositoryEnvironmentDeploymentPolicyUpdate(d *schema.ResourceData, meta any) error {
+func resourceGithubRepositoryEnvironmentDeploymentPolicyUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
 	client := meta.(*Owner).v3client
-	ctx := context.Background()
 
 	owner := meta.(*Owner).name
 	repoName := d.Get("repository").(string)
@@ -139,12 +151,12 @@ func resourceGithubRepositoryEnvironmentDeploymentPolicyUpdate(d *schema.Resourc
 	escapedEnvName := url.PathEscape(envName)
 	_, _, branchPolicyIdString, err := parseThreePartID(d.Id(), "repository", "environment", "branchPolicyId")
 	if err != nil {
-		return err
+		return diag.FromErr(err)
 	}
 
 	branchPolicyId, err := strconv.ParseInt(branchPolicyIdString, 10, 64)
 	if err != nil {
-		return err
+		return diag.FromErr(err)
 	}
 
 	pattern := branchPattern
@@ -158,56 +170,39 @@ func resourceGithubRepositoryEnvironmentDeploymentPolicyUpdate(d *schema.Resourc
 
 	resultKey, _, err := client.Repositories.UpdateDeploymentBranchPolicy(ctx, owner, repoName, escapedEnvName, branchPolicyId, &updateData)
 	if err != nil {
-		return err
+		return diag.FromErr(err)
 	}
 	d.SetId(buildThreePartID(repoName, escapedEnvName, strconv.FormatInt(resultKey.GetID(), 10)))
-	return resourceGithubRepositoryEnvironmentDeploymentPolicyRead(d, meta)
+	return nil
 }
 
-func resourceGithubRepositoryEnvironmentDeploymentPolicyDelete(d *schema.ResourceData, meta any) error {
+func resourceGithubRepositoryEnvironmentDeploymentPolicyDelete(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
 	client := meta.(*Owner).v3client
-	ctx := context.Background()
 
 	owner := meta.(*Owner).name
 	repoName, envName, branchPolicyIdString, err := parseThreePartID(d.Id(), "repository", "environment", "branchPolicyId")
 	if err != nil {
-		return err
+		return diag.FromErr(err)
 	}
 
 	branchPolicyId, err := strconv.ParseInt(branchPolicyIdString, 10, 64)
 	if err != nil {
-		return err
+		return diag.FromErr(err)
 	}
 
 	_, err = client.Repositories.DeleteDeploymentBranchPolicy(ctx, owner, repoName, envName, branchPolicyId)
 	if err != nil {
-		return err
+		return diag.FromErr(err)
 	}
 
 	return nil
 }
 
 func customDeploymentPolicyDiffFunction(_ context.Context, diff *schema.ResourceDiff, v any) error {
-	oldBranchPattern, newBranchPattern := diff.GetChange("branch_pattern")
-
-	if oldBranchPattern != "" && newBranchPattern == "" {
+	if diff.HasChange("branch_pattern") && diff.HasChange("tag_pattern") {
 		if err := diff.ForceNew("branch_pattern"); err != nil {
 			return err
 		}
-	}
-	if oldBranchPattern == "" && newBranchPattern != "" {
-		if err := diff.ForceNew("branch_pattern"); err != nil {
-			return err
-		}
-	}
-
-	oldTagPattern, newTagPattern := diff.GetChange("tag_pattern")
-	if oldTagPattern != "" && newTagPattern == "" {
-		if err := diff.ForceNew("tag_pattern"); err != nil {
-			return err
-		}
-	}
-	if oldTagPattern == "" && newTagPattern != "" {
 		if err := diff.ForceNew("tag_pattern"); err != nil {
 			return err
 		}