[BUG] Ensure repository creation works if vulnerability-alerts isn't modifiable on repo level (#3024)

* `github_repository`: Convert to use Context-aware functions

Signed-off-by: Timo Sand <[email protected]>

* Add tests to ensure `vulnerability_alerts` behaviour keeps working

Signed-off-by: Timo Sand <[email protected]>

* Add error handling to ignore 422 response

In the case of `vulnerability_alerts = false` and the message indicates control from parent Org/Enterprise

Signed-off-by: Timo Sand <[email protected]>

* Improve descriptions to make interactions slightly clearer

Signed-off-by: Timo Sand <[email protected]>

* Add comment to describe this as a temporary workaround

Signed-off-by: Timo Sand <[email protected]>

---------

Signed-off-by: Timo Sand <[email protected]>

Author: deiga

github/resource_github_repository.go

@@ -10,19 +10,20 @@ import (
 	"strings"
 
 	"github.com/google/go-github/v67/github"
+	"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 resourceGithubRepository() *schema.Resource {
 	return &schema.Resource{
-		Create: resourceGithubRepositoryCreate,
-		Read:   resourceGithubRepositoryRead,
-		Update: resourceGithubRepositoryUpdate,
-		Delete: resourceGithubRepositoryDelete,
+		CreateContext: resourceGithubRepositoryCreate,
+		ReadContext:   resourceGithubRepositoryRead,
+		UpdateContext: resourceGithubRepositoryUpdate,
+		DeleteContext: resourceGithubRepositoryDelete,
 		Importer: &schema.ResourceImporter{
-			State: func(d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) {
+			StateContext: func(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) {
 				if err := d.Set("auto_init", false); err != nil {
 					return nil, err
 				}
@@ -390,7 +391,7 @@ func resourceGithubRepository() *schema.Resource {
 				Type:        schema.TypeBool,
 				Optional:    true,
 				Computed:    true,
-				Description: "Set to 'true' to enable security alerts for vulnerable dependencies. Enabling requires alerts to be enabled on the owner level. (Note for importing: GitHub enables the alerts on public repos but disables them on private repos by default). Note that vulnerability alerts have not been successfully tested on any GitHub Enterprise instance and may be unavailable in those settings.",
+				Description: "Set to 'true' to enable security alerts for vulnerable dependencies. Enabling requires alerts to be enabled on the owner level. (Note for importing: GitHub enables the alerts on all repos by default). Note that vulnerability alerts have not been successfully tested on any GitHub Enterprise instance and may be unavailable in those settings.",
 			},
 			"ignore_vulnerability_alerts_during_read": {
 				Type:        schema.TypeBool,
@@ -620,18 +621,17 @@ func resourceGithubRepositoryObject(d *schema.ResourceData) *github.Repository {
 	return repository
 }
 
-func resourceGithubRepositoryCreate(d *schema.ResourceData, meta any) error {
+func resourceGithubRepositoryCreate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
 	client := meta.(*Owner).v3client
 
 	if branchName, hasDefaultBranch := d.GetOk("default_branch"); hasDefaultBranch && (branchName != "main") {
-		return fmt.Errorf("cannot set the default branch on a new repository to something other than 'main'")
+		return diag.Errorf("cannot set the default branch on a new repository to something other than 'main'")
 	}
 
 	repoReq := resourceGithubRepositoryObject(d)
 	owner := meta.(*Owner).name
 
 	repoName := repoReq.GetName()
-	ctx := context.Background()
 
 	// determine if repository should be private. assume public to start
 	isPrivate := false
@@ -657,7 +657,7 @@ func resourceGithubRepositoryCreate(d *schema.ResourceData, meta any) error {
 		for _, templateConfigBlock := range templateConfigBlocks {
 			templateConfigMap, ok := templateConfigBlock.(map[string]any)
 			if !ok {
-				return errors.New("failed to unpack template configuration block")
+				return diag.FromErr(errors.New("failed to unpack template configuration block"))
 			}
 
 			templateRepo := templateConfigMap["repository"].(string)
@@ -678,7 +678,7 @@ func resourceGithubRepositoryCreate(d *schema.ResourceData, meta any) error {
 				&templateRepoReq,
 			)
 			if err != nil {
-				return err
+				return diag.FromErr(err)
 			}
 
 			d.SetId(*repo.Name)
@@ -691,7 +691,7 @@ func resourceGithubRepositoryCreate(d *schema.ResourceData, meta any) error {
 		log.Printf("[INFO] Creating fork of %s/%s in %s", sourceOwner, sourceRepo, owner)
 
 		if sourceOwner == "" || sourceRepo == "" {
-			return fmt.Errorf("source_owner and source_repo must be provided when forking a repository")
+			return diag.Errorf("source_owner and source_repo must be provided when forking a repository")
 		}
 
 		// Create the fork using the GitHub client library
@@ -712,18 +712,18 @@ func resourceGithubRepositoryCreate(d *schema.ResourceData, meta any) error {
 				log.Printf("[INFO] Fork is being created asynchronously")
 				// Despite the 202 status, the API should still return preliminary fork information
 				if fork == nil {
-					return fmt.Errorf("fork information not available after accepted status")
+					return diag.Errorf("fork information not available after accepted status")
 				}
 				log.Printf("[DEBUG] Fork name: %s", fork.GetName())
 			} else {
-				return fmt.Errorf("failed to create fork: %w", err)
+				return diag.Errorf("failed to create fork: %s", err.Error())
 			}
 		} else if resp != nil {
 			log.Printf("[DEBUG] Fork response status: %d", resp.StatusCode)
 		}
 
 		if fork == nil {
-			return fmt.Errorf("fork creation failed - no repository returned")
+			return diag.Errorf("fork creation failed - no repository returned")
 		}
 
 		log.Printf("[INFO] Fork created with name: %s", fork.GetName())
@@ -747,7 +747,7 @@ func resourceGithubRepositoryCreate(d *schema.ResourceData, meta any) error {
 			repo, _, err = client.Repositories.Create(ctx, "", repoReq)
 		}
 		if err != nil {
-			return err
+			return diag.FromErr(err)
 		}
 		d.SetId(repo.GetName())
 	}
@@ -756,27 +756,27 @@ func resourceGithubRepositoryCreate(d *schema.ResourceData, meta any) error {
 	if len(topics) > 0 {
 		_, _, err := client.Repositories.ReplaceAllTopics(ctx, owner, repoName, topics)
 		if err != nil {
-			return err
+			return diag.FromErr(err)
 		}
 	}
 
 	pages := expandPages(d.Get("pages").([]any))
 	if pages != nil {
 		_, _, err := client.Repositories.EnablePages(ctx, owner, repoName, pages)
 		if err != nil {
-			return err
+			return diag.FromErr(err)
 		}
 	}
 
 	err := updateVulnerabilityAlerts(d, client, ctx, owner, repoName)
 	if err != nil {
-		return err
+		return diag.FromErr(err)
 	}
 
-	return resourceGithubRepositoryUpdate(d, meta)
+	return resourceGithubRepositoryUpdate(ctx, d, meta)
 }
 
-func resourceGithubRepositoryRead(d *schema.ResourceData, meta any) error {
+func resourceGithubRepositoryRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
 	client := meta.(*Owner).v3client
 
 	owner := meta.(*Owner).name
@@ -788,7 +788,7 @@ func resourceGithubRepositoryRead(d *schema.ResourceData, meta any) error {
 		owner = explicitOwner
 	}
 
-	ctx := context.WithValue(context.Background(), ctxId, d.Id())
+	ctx = context.WithValue(ctx, ctxId, d.Id())
 	if !d.IsNewResource() {
 		ctx = context.WithValue(ctx, ctxEtag, d.Get("etag").(string))
 	}
@@ -807,7 +807,7 @@ func resourceGithubRepositoryRead(d *schema.ResourceData, meta any) error {
 				return nil
 			}
 		}
-		return err
+		return diag.FromErr(err)
 	}
 
 	_ = d.Set("etag", resp.Header.Get("ETag"))
@@ -853,10 +853,10 @@ func resourceGithubRepositoryRead(d *schema.ResourceData, meta any) error {
 	if repo.GetHasPages() {
 		pages, _, err := client.Repositories.GetPagesInfo(ctx, owner, repoName)
 		if err != nil {
-			return err
+			return diag.FromErr(err)
 		}
 		if err := d.Set("pages", flattenPages(pages)); err != nil {
-			return fmt.Errorf("error setting pages: %w", err)
+			return diag.Errorf("error setting pages: %s", err.Error())
 		}
 	}
 
@@ -882,32 +882,32 @@ func resourceGithubRepositoryRead(d *schema.ResourceData, meta any) error {
 				"repository": repo.TemplateRepository.Name,
 			},
 		}); err != nil {
-			return err
+			return diag.FromErr(err)
 		}
 	} else {
 		if err = d.Set("template", []any{}); err != nil {
-			return err
+			return diag.FromErr(err)
 		}
 	}
 
 	if !d.Get("ignore_vulnerability_alerts_during_read").(bool) {
 		vulnerabilityAlerts, _, err := client.Repositories.GetVulnerabilityAlerts(ctx, owner, repoName)
 		if err != nil {
-			return fmt.Errorf("error reading repository vulnerability alerts: %w", err)
+			return diag.Errorf("error reading repository vulnerability alerts: %s", err.Error())
 		}
 		if err = d.Set("vulnerability_alerts", vulnerabilityAlerts); err != nil {
-			return err
+			return diag.FromErr(err)
 		}
 	}
 
 	if err = d.Set("security_and_analysis", flattenSecurityAndAnalysis(repo.GetSecurityAndAnalysis())); err != nil {
-		return err
+		return diag.FromErr(err)
 	}
 
 	return nil
 }
 
-func resourceGithubRepositoryUpdate(d *schema.ResourceData, meta any) error {
+func resourceGithubRepositoryUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
 	// Can only update a repository if it is not archived or the update is to
 	// archive the repository (unarchiving is not supported by the GitHub API)
 	if d.Get("archived").(bool) && !d.HasChange("archived") {
@@ -937,7 +937,7 @@ func resourceGithubRepositoryUpdate(d *schema.ResourceData, meta any) error {
 
 	repoName := d.Id()
 	owner := meta.(*Owner).name
-	ctx := context.WithValue(context.Background(), ctxId, d.Id())
+	ctx = context.WithValue(ctx, ctxId, d.Id())
 
 	// When the organization has "Require sign off on web-based commits" enabled,
 	// the API doesn't allow you to send `web_commit_signoff_required` in order to
@@ -955,7 +955,7 @@ func resourceGithubRepositoryUpdate(d *schema.ResourceData, meta any) error {
 
 	repo, _, err := client.Repositories.Edit(ctx, owner, repoName, repoReq)
 	if err != nil {
-		return err
+		return diag.FromErr(err)
 	}
 	d.SetId(*repo.Name)
 
@@ -964,7 +964,7 @@ func resourceGithubRepositoryUpdate(d *schema.ResourceData, meta any) error {
 		if opts != nil {
 			pages, res, err := client.Repositories.GetPagesInfo(ctx, owner, repoName)
 			if res.StatusCode != http.StatusNotFound && err != nil {
-				return err
+				return diag.FromErr(err)
 			}
 
 			if pages == nil {
@@ -973,12 +973,12 @@ func resourceGithubRepositoryUpdate(d *schema.ResourceData, meta any) error {
 				_, err = client.Repositories.UpdatePages(ctx, owner, repoName, opts)
 			}
 			if err != nil {
-				return err
+				return diag.FromErr(err)
 			}
 		} else {
 			_, err := client.Repositories.DisablePages(ctx, owner, repoName)
 			if err != nil {
-				return err
+				return diag.FromErr(err)
 			}
 		}
 	}
@@ -987,23 +987,23 @@ func resourceGithubRepositoryUpdate(d *schema.ResourceData, meta any) error {
 		topics := repoReq.Topics
 		_, _, err = client.Repositories.ReplaceAllTopics(ctx, owner, *repo.Name, topics)
 		if err != nil {
-			return err
+			return diag.FromErr(err)
 		}
 		d.SetId(*repo.Name)
 
 		if d.HasChange("topics") {
 			topics := repoReq.Topics
 			_, _, err = client.Repositories.ReplaceAllTopics(ctx, owner, *repo.Name, topics)
 			if err != nil {
-				return err
+				return diag.FromErr(err)
 			}
 		}
 	}
 
 	if d.HasChange("vulnerability_alerts") {
 		err = updateVulnerabilityAlerts(d, client, ctx, owner, repoName)
 		if err != nil {
-			return err
+			return diag.FromErr(err)
 		}
 	}
 
@@ -1014,7 +1014,7 @@ func resourceGithubRepositoryUpdate(d *schema.ResourceData, meta any) error {
 		_, resp, err := client.Repositories.Edit(ctx, owner, repoName, repoReq)
 		if err != nil {
 			if resp.StatusCode != 422 || !strings.Contains(err.Error(), fmt.Sprintf("Visibility is already %s", n.(string))) {
-				return err
+				return diag.FromErr(err)
 			}
 		}
 	} else {
@@ -1028,21 +1028,21 @@ func resourceGithubRepositoryUpdate(d *schema.ResourceData, meta any) error {
 		_, _, err = client.Repositories.Edit(ctx, owner, repoName, repoReq)
 		if err != nil {
 			if !strings.Contains(err.Error(), "422 Privacy is already set") {
-				return err
+				return diag.FromErr(err)
 			}
 		}
 	} else {
 		log.Printf("[DEBUG] No privacy update required. private: %v", d.Get("private"))
 	}
 
-	return resourceGithubRepositoryRead(d, meta)
+	return resourceGithubRepositoryRead(ctx, d, meta)
 }
 
-func resourceGithubRepositoryDelete(d *schema.ResourceData, meta any) error {
+func resourceGithubRepositoryDelete(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
 	client := meta.(*Owner).v3client
 	repoName := d.Id()
 	owner := meta.(*Owner).name
-	ctx := context.WithValue(context.Background(), ctxId, d.Id())
+	ctx = context.WithValue(ctx, ctxId, d.Id())
 
 	archiveOnDestroy := d.Get("archive_on_destroy").(bool)
 	if archiveOnDestroy {
@@ -1051,20 +1051,20 @@ func resourceGithubRepositoryDelete(d *schema.ResourceData, meta any) error {
 			return nil
 		} else {
 			if err := d.Set("archived", true); err != nil {
-				return err
+				return diag.FromErr(err)
 			}
 			repoReq := resourceGithubRepositoryObject(d)
 			// Always remove `web_commit_signoff_required` when archiving, to avoid 422 error
 			repoReq.WebCommitSignoffRequired = nil
 			log.Printf("[DEBUG] Archiving repository on delete: %s/%s", owner, repoName)
 			_, _, err := client.Repositories.Edit(ctx, owner, repoName, repoReq)
-			return err
+			return diag.FromErr(err)
 		}
 	}
 
 	log.Printf("[DEBUG] Deleting repository: %s/%s", owner, repoName)
 	_, err := client.Repositories.Delete(ctx, owner, repoName)
-	return err
+	return diag.FromErr(err)
 }
 
 func expandPages(input []any) *github.Pages {
@@ -1243,11 +1243,22 @@ func resourceGithubParseFullName(resourceDataLike interface {
 }
 
 func updateVulnerabilityAlerts(d *schema.ResourceData, client *github.Client, ctx context.Context, owner, repoName string) error {
-	updateVulnerabilityAlerts := client.Repositories.DisableVulnerabilityAlerts
-	if vulnerabilityAlerts, ok := d.GetOk("vulnerability_alerts"); ok && vulnerabilityAlerts.(bool) {
-		updateVulnerabilityAlerts = client.Repositories.EnableVulnerabilityAlerts
+	updateVulnerabilityAlertsSDK := client.Repositories.DisableVulnerabilityAlerts
+	vulnerabilityAlerts, ok := d.GetOk("vulnerability_alerts")
+
+	// Only if the vulnerability alerts are specifically set to true, enable them.
+	// Otherwise, disable them as GitHub defaults to enabled and we have not wanted to introduce a breaking change for this yet.
+	if ok && vulnerabilityAlerts.(bool) {
+		updateVulnerabilityAlertsSDK = client.Repositories.EnableVulnerabilityAlerts
 	}
 
-	_, err := updateVulnerabilityAlerts(ctx, owner, repoName)
+	resp, err := updateVulnerabilityAlertsSDK(ctx, owner, repoName)
+	if err != nil {
+		// Check if the error is because an Organization or Enterprise policy is preventing the change
+		// This is a temporary workaround while we extract Vulnerability Alerts into a separate resource.
+		if resp.StatusCode == http.StatusUnprocessableEntity && strings.Contains(err.Error(), "An enforced security configuration prevented modifying") && !ok {
+			return nil
+		}
+	}
 	return err
 }