feat: update GitHub Actions runner group functions to use context and improve error handling

Author: austenstone

github/resource_github_actions_runner_group.go

@@ -9,16 +9,17 @@ import (
 	"strconv"
 
 	"github.com/google/go-github/v83/github"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
 )
 
 func resourceGithubActionsRunnerGroup() *schema.Resource {
 	return &schema.Resource{
-		Create: resourceGithubActionsRunnerGroupCreate,
-		Read:   resourceGithubActionsRunnerGroupRead,
-		Update: resourceGithubActionsRunnerGroupUpdate,
-		Delete: resourceGithubActionsRunnerGroupDelete,
+		CreateContext: resourceGithubActionsRunnerGroupCreate,
+		ReadContext:   resourceGithubActionsRunnerGroupRead,
+		UpdateContext: resourceGithubActionsRunnerGroupUpdate,
+		DeleteContext: resourceGithubActionsRunnerGroupDelete,
 		Importer: &schema.ResourceImporter{
 			StateContext: schema.ImportStatePassthroughContext,
 		},
@@ -93,10 +94,10 @@ func resourceGithubActionsRunnerGroup() *schema.Resource {
 				Description: "List of workflows the runner group should be allowed to run. This setting will be ignored unless restricted_to_workflows is set to 'true'.",
 			},
 			"network_configuration_id": {
-				Type:         schema.TypeString,
-				Optional:     true,
-				ValidateFunc: validation.StringLenBetween(1, 255),
-				Description:  "The identifier of the hosted compute network configuration to associate with this runner group for GitHub-hosted private networking.",
+				Type:             schema.TypeString,
+				Optional:         true,
+				ValidateDiagFunc: validation.ToDiagFunc(validation.StringLenBetween(1, 255)),
+				Description:      "The identifier of the hosted compute network configuration to associate with this runner group for GitHub-hosted private networking.",
 			},
 		},
 	}
@@ -108,7 +109,7 @@ func getOrganizationRunnerGroup(client *github.Client, ctx context.Context, org
 		var ghErr *github.ErrorResponse
 		if errors.As(err, &ghErr) && ghErr.Response != nil && ghErr.Response.StatusCode == http.StatusNotModified {
 			// ignore error StatusNotModified
-			return runnerGroup, resp, nil
+			return nil, resp, nil
 		}
 	}
 	return runnerGroup, resp, err
@@ -155,10 +156,9 @@ func setGithubActionsRunnerGroupState(d *schema.ResourceData, runnerGroup *githu
 	return nil
 }
 
-func resourceGithubActionsRunnerGroupCreate(d *schema.ResourceData, meta any) error {
-	err := checkOrganization(meta)
-	if err != nil {
-		return err
+func resourceGithubActionsRunnerGroupCreate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
+	if err := checkOrganization(meta); err != nil {
+		return diag.FromErr(err)
 	}
 
 	client := meta.(*Owner).v3client
@@ -177,7 +177,7 @@ func resourceGithubActionsRunnerGroupCreate(d *schema.ResourceData, meta any) er
 	}
 
 	if visibility != "selected" && hasSelectedRepositories {
-		return fmt.Errorf("cannot use selected_repository_ids without visibility being set to selected")
+		return diag.FromErr(fmt.Errorf("cannot use selected_repository_ids without visibility being set to selected"))
 	}
 
 	selectedRepositoryIDs := []int64{}
@@ -190,7 +190,7 @@ func resourceGithubActionsRunnerGroupCreate(d *schema.ResourceData, meta any) er
 		}
 	}
 
-	ctx := context.Background()
+	ctx = context.WithValue(ctx, ctxId, d.Id())
 
 	runnerGroup, resp, err := client.Actions.CreateOrganizationRunnerGroup(ctx,
 		orgName,
@@ -204,45 +204,49 @@ func resourceGithubActionsRunnerGroupCreate(d *schema.ResourceData, meta any) er
 		},
 	)
 	if err != nil {
-		return err
+		return diag.FromErr(err)
 	}
 	d.SetId(strconv.FormatInt(runnerGroup.GetID(), 10))
+	ctx = context.WithValue(ctx, ctxId, d.Id())
 	if err = setGithubActionsRunnerGroupState(d, runnerGroup, normalizeEtag(resp.Header.Get("ETag")), selectedRepositoryIDs); err != nil {
-		return err
+		return diag.FromErr(err)
 	}
 
 	if networkConfigurationID, ok := d.GetOk("network_configuration_id"); ok {
 		networkConfigurationIDValue := networkConfigurationID.(string)
 		// The create endpoint does not accept network_configuration_id, so private networking
 		// must be attached with a follow-up PATCH after the runner group has been created.
 		if _, err = updateRunnerGroupNetworking(client, ctx, fmt.Sprintf("orgs/%s/actions/runner-groups/%d", orgName, runnerGroup.GetID()), &networkConfigurationIDValue); err != nil {
-			return err
+			return diag.FromErr(err)
 		}
 
-		return resourceGithubActionsRunnerGroupRead(d, meta)
+		if err = setRunnerGroupNetworkingState(d, &runnerGroupNetworking{NetworkConfigurationID: &networkConfigurationIDValue}); err != nil {
+			return diag.FromErr(err)
+		}
+
+		return nil
 	}
 
 	if err = setRunnerGroupNetworkingState(d, nil); err != nil {
-		return err
+		return diag.FromErr(err)
 	}
 
 	return nil
 }
 
-func resourceGithubActionsRunnerGroupRead(d *schema.ResourceData, meta any) error {
-	err := checkOrganization(meta)
-	if err != nil {
-		return err
+func resourceGithubActionsRunnerGroupRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
+	if err := checkOrganization(meta); err != nil {
+		return diag.FromErr(err)
 	}
 
 	client := meta.(*Owner).v3client
 	orgName := meta.(*Owner).name
 
 	runnerGroupID, err := strconv.ParseInt(d.Id(), 10, 64)
 	if err != nil {
-		return err
+		return diag.FromErr(err)
 	}
-	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))
 	}
@@ -258,7 +262,7 @@ func resourceGithubActionsRunnerGroupRead(d *schema.ResourceData, meta any) erro
 				return nil
 			}
 		}
-		return err
+		return diag.FromErr(err)
 	}
 
 	runnerGroupEtag := normalizeEtag(d.Get("etag").(string))
@@ -268,7 +272,7 @@ func resourceGithubActionsRunnerGroupRead(d *schema.ResourceData, meta any) erro
 
 	runnerGroupNetworking, _, err := getRunnerGroupNetworking(client, ctx, fmt.Sprintf("orgs/%s/actions/runner-groups/%d", orgName, runnerGroupID))
 	if err != nil {
-		return err
+		return diag.FromErr(err)
 	}
 
 	selectedRepositoryIDs := []int64{}
@@ -279,7 +283,7 @@ func resourceGithubActionsRunnerGroupRead(d *schema.ResourceData, meta any) erro
 	for {
 		runnerGroupRepositories, resp, err := client.Actions.ListRepositoryAccessRunnerGroup(ctx, orgName, runnerGroupID, &options)
 		if err != nil {
-			return err
+			return diag.FromErr(err)
 		}
 
 		for _, repo := range runnerGroupRepositories.Repositories {
@@ -295,29 +299,28 @@ func resourceGithubActionsRunnerGroupRead(d *schema.ResourceData, meta any) erro
 
 	if runnerGroup != nil {
 		if err = setGithubActionsRunnerGroupState(d, runnerGroup, runnerGroupEtag, selectedRepositoryIDs); err != nil {
-			return err
+			return diag.FromErr(err)
 		}
 	} else {
 		if err := d.Set("selected_repository_ids", selectedRepositoryIDs); err != nil {
-			return err
+			return diag.FromErr(err)
 		}
 		if err := d.Set("etag", runnerGroupEtag); err != nil {
-			return err
+			return diag.FromErr(err)
 		}
 	}
 	if runnerGroupNetworking != nil {
 		if err = setRunnerGroupNetworkingState(d, runnerGroupNetworking); err != nil {
-			return err
+			return diag.FromErr(err)
 		}
 	}
 
 	return nil
 }
 
-func resourceGithubActionsRunnerGroupUpdate(d *schema.ResourceData, meta any) error {
-	err := checkOrganization(meta)
-	if err != nil {
-		return err
+func resourceGithubActionsRunnerGroupUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
+	if err := checkOrganization(meta); err != nil {
+		return diag.FromErr(err)
 	}
 
 	client := meta.(*Owner).v3client
@@ -344,26 +347,32 @@ func resourceGithubActionsRunnerGroupUpdate(d *schema.ResourceData, meta any) er
 
 	runnerGroupID, err := strconv.ParseInt(d.Id(), 10, 64)
 	if err != nil {
-		return err
+		return diag.FromErr(err)
 	}
-	ctx := context.WithValue(context.Background(), ctxId, d.Id())
+	ctx = context.WithValue(ctx, ctxId, d.Id())
 
-	if _, _, err := client.Actions.UpdateOrganizationRunnerGroup(ctx, orgName, runnerGroupID, options); err != nil {
-		return err
+	runnerGroup, resp, err := client.Actions.UpdateOrganizationRunnerGroup(ctx, orgName, runnerGroupID, options)
+	if err != nil {
+		return diag.FromErr(err)
 	}
 
-	if d.HasChange("network_configuration_id") {
-		var networkConfigurationIDValue *string
-		if networkConfigurationID, ok := d.GetOk("network_configuration_id"); ok {
-			value := networkConfigurationID.(string)
-			networkConfigurationIDValue = &value
-		}
+	var networkConfigurationIDValue *string
+	if networkConfigurationID, ok := d.GetOk("network_configuration_id"); ok {
+		value := networkConfigurationID.(string)
+		networkConfigurationIDValue = &value
+	}
 
+	if d.HasChange("network_configuration_id") {
 		if _, err := updateRunnerGroupNetworking(client, ctx, fmt.Sprintf("orgs/%s/actions/runner-groups/%d", orgName, runnerGroupID), networkConfigurationIDValue); err != nil {
-			return err
+			return diag.FromErr(err)
 		}
 	}
 
+	var networkingState *runnerGroupNetworking
+	if networkConfigurationIDValue != nil {
+		networkingState = &runnerGroupNetworking{NetworkConfigurationID: networkConfigurationIDValue}
+	}
+
 	selectedRepositories, hasSelectedRepositories := d.GetOk("selected_repository_ids")
 	selectedRepositoryIDs := []int64{}
 
@@ -378,27 +387,42 @@ func resourceGithubActionsRunnerGroupUpdate(d *schema.ResourceData, meta any) er
 	reposOptions := github.SetRepoAccessRunnerGroupRequest{SelectedRepositoryIDs: selectedRepositoryIDs}
 
 	if _, err := client.Actions.SetRepositoryAccessRunnerGroup(ctx, orgName, runnerGroupID, reposOptions); err != nil {
-		return err
+		return diag.FromErr(err)
 	}
 
-	return resourceGithubActionsRunnerGroupRead(d, meta)
+	runnerGroupEtag := normalizeEtag(d.Get("etag").(string))
+	if resp != nil {
+		runnerGroupEtag = normalizeEtag(resp.Header.Get("ETag"))
+	}
+
+	if err := setGithubActionsRunnerGroupState(d, runnerGroup, runnerGroupEtag, selectedRepositoryIDs); err != nil {
+		return diag.FromErr(err)
+	}
+	if err := setRunnerGroupNetworkingState(d, networkingState); err != nil {
+		return diag.FromErr(err)
+	}
+
+	return nil
 }
 
-func resourceGithubActionsRunnerGroupDelete(d *schema.ResourceData, meta any) error {
-	err := checkOrganization(meta)
-	if err != nil {
-		return err
+func resourceGithubActionsRunnerGroupDelete(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
+	if err := checkOrganization(meta); err != nil {
+		return diag.FromErr(err)
 	}
 
 	client := meta.(*Owner).v3client
 	orgName := meta.(*Owner).name
 	runnerGroupID, err := strconv.ParseInt(d.Id(), 10, 64)
 	if err != nil {
-		return err
+		return diag.FromErr(err)
 	}
-	ctx := context.WithValue(context.Background(), ctxId, d.Id())
+	ctx = context.WithValue(ctx, ctxId, d.Id())
 
 	log.Printf("[INFO] Deleting organization runner group: %s (%s)", d.Id(), orgName)
 	_, err = client.Actions.DeleteOrganizationRunnerGroup(ctx, orgName, runnerGroupID)
-	return err
+	if err != nil {
+		return diag.FromErr(err)
+	}
+
+	return nil
 }

github/resource_github_actions_runner_group_networking.go

@@ -52,15 +52,10 @@ func updateRunnerGroupNetworking(client *github.Client, ctx context.Context, pat
 }
 
 func setRunnerGroupNetworkingState(d *schema.ResourceData, runnerGroup *runnerGroupNetworking) error {
-	if runnerGroup != nil && runnerGroup.NetworkConfigurationID != nil && *runnerGroup.NetworkConfigurationID != "" {
-		if err := d.Set("network_configuration_id", *runnerGroup.NetworkConfigurationID); err != nil {
-			return err
-		}
-	} else {
-		if err := d.Set("network_configuration_id", nil); err != nil {
-			return err
-		}
+	var networkConfigurationID any
+	if runnerGroup != nil {
+		networkConfigurationID = runnerGroup.NetworkConfigurationID
 	}
 
-	return nil
+	return d.Set("network_configuration_id", networkConfigurationID)
 }