Skip to content

Commit 3b2f0f6

Browse files
nico34638nico34638
committed
resource_github_enterprise_actions_hosted_runner: fix review
1 parent c48fddf commit 3b2f0f6

2 files changed

Lines changed: 69 additions & 105 deletions

File tree

‎github/resource_github_enterprise_actions_hosted_runner.go‎

Lines changed: 67 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,13 @@ package github
22

33
import (
44
"context"
5-
"fmt"
6-
"log"
75
"net/http"
86
"regexp"
97
"strconv"
108
"time"
119

1210
"github.com/google/go-github/v82/github"
11+
"github.com/hashicorp/terraform-plugin-log/tflog"
1312
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
1413
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry"
1514
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
@@ -25,6 +24,11 @@ func resourceGithubEnterpriseActionsHostedRunner() *schema.Resource {
2524
Importer: &schema.ResourceImporter{
2625
StateContext: schema.ImportStatePassthroughContext,
2726
},
27+
28+
Timeouts: &schema.ResourceTimeout{
29+
Delete: schema.DefaultTimeout(15 * time.Minute),
30+
},
31+
2832
Schema: map[string]*schema.Schema{
2933
"enterprise_slug": {
3034
Type: schema.TypeString,
@@ -228,48 +232,46 @@ func resourceGithubEnterpriseActionsHostedRunnerCreate(ctx context.Context, d *s
228232
}
229233

230234
// Set the ID in the format enterprise_slug/runner_id
231-
d.SetId(fmt.Sprintf("%s/%d", enterpriseSlug, *runner.ID))
235+
id, err := buildID(enterpriseSlug, strconv.FormatInt(runner.GetID(), 10))
236+
if err != nil {
237+
return diag.FromErr(err)
238+
}
239+
d.SetId(id)
232240

233241
// Populate computed fields directly from API response
234242
if err := d.Set("enterprise_slug", enterpriseSlug); err != nil {
235243
return diag.FromErr(err)
236244
}
237245

238246
// runner.ID is guaranteed non-nil due to check above
239-
if err := d.Set("runner_id", int(*runner.ID)); err != nil {
247+
if err := d.Set("runner_id", int(runner.GetID())); err != nil {
240248
return diag.FromErr(err)
241249
}
242250

243-
if runner.Name != nil {
244-
if err := d.Set("name", *runner.Name); err != nil {
245-
return diag.FromErr(err)
246-
}
251+
if err := d.Set("name", runner.GetName()); err != nil {
252+
return diag.FromErr(err)
247253
}
248-
if runner.Status != nil {
249-
if err := d.Set("status", *runner.Status); err != nil {
250-
return diag.FromErr(err)
251-
}
254+
255+
if err := d.Set("status", runner.GetStatus()); err != nil {
256+
return diag.FromErr(err)
252257
}
253-
if runner.Platform != nil {
254-
if err := d.Set("platform", *runner.Platform); err != nil {
255-
return diag.FromErr(err)
256-
}
258+
259+
if err := d.Set("platform", runner.GetPlatform()); err != nil {
260+
return diag.FromErr(err)
257261
}
262+
258263
if runner.LastActiveOn != nil {
259264
if err := d.Set("last_active_on", runner.LastActiveOn.Format(time.RFC3339)); err != nil {
260265
return diag.FromErr(err)
261266
}
262267
}
263-
if runner.PublicIPEnabled != nil {
264-
if err := d.Set("public_ip_enabled", *runner.PublicIPEnabled); err != nil {
265-
return diag.FromErr(err)
266-
}
268+
269+
if err := d.Set("public_ip_enabled", runner.GetPublicIPEnabled()); err != nil {
270+
return diag.FromErr(err)
267271
}
268272

269-
if runner.ImageDetails != nil {
270-
if err := d.Set("image", flattenHostedRunnerImage(runner.ImageDetails)); err != nil {
271-
return diag.FromErr(err)
272-
}
273+
if err := d.Set("image", flattenHostedRunnerImage(runner.ImageDetails)); err != nil {
274+
return diag.FromErr(err)
273275
}
274276

275277
if runner.MachineSizeDetails != nil {
@@ -281,16 +283,12 @@ func resourceGithubEnterpriseActionsHostedRunnerCreate(ctx context.Context, d *s
281283
}
282284
}
283285

284-
if runner.RunnerGroupID != nil {
285-
if err := d.Set("runner_group_id", int(*runner.RunnerGroupID)); err != nil {
286-
return diag.FromErr(err)
287-
}
286+
if err := d.Set("runner_group_id", int(runner.GetRunnerGroupID())); err != nil {
287+
return diag.FromErr(err)
288288
}
289289

290-
if runner.MaximumRunners != nil {
291-
if err := d.Set("maximum_runners", int(*runner.MaximumRunners)); err != nil {
292-
return diag.FromErr(err)
293-
}
290+
if err := d.Set("maximum_runners", int(runner.GetMaximumRunners())); err != nil {
291+
return diag.FromErr(err)
294292
}
295293

296294
if runner.PublicIPs != nil {
@@ -305,70 +303,62 @@ func resourceGithubEnterpriseActionsHostedRunnerCreate(ctx context.Context, d *s
305303
func resourceGithubEnterpriseActionsHostedRunnerRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
306304
client := meta.(*Owner).v3client
307305

308-
enterpriseSlug, runnerIDStr, err := parseTwoPartID(d.Id(), "enterprise_slug", "runner_id")
306+
enterpriseSlug, runnerIDStr, err := parseID2(d.Id())
309307
if err != nil {
310308
return diag.FromErr(err)
311309
}
312310

313311
runnerID, err := strconv.ParseInt(runnerIDStr, 10, 64)
314312
if err != nil {
315-
return diag.FromErr(fmt.Errorf("invalid runner ID %q: %w", runnerIDStr, err))
313+
return diag.Errorf("invalid runner ID %q: %s", runnerIDStr, err.Error())
316314
}
317315

318316
runner, resp, err := client.Enterprise.GetHostedRunner(ctx, enterpriseSlug, runnerID)
319317
if err != nil {
320318
if resp != nil && resp.StatusCode == http.StatusNotFound {
321-
log.Printf("[WARN] Removing enterprise hosted runner %s from state because it no longer exists in GitHub", d.Id())
319+
tflog.Info(ctx, "Removing enterprise hosted runner from state because it no longer exists in GitHub", map[string]any{
320+
"id": d.Id(),
321+
"enterprise_slug": enterpriseSlug,
322+
"runner_id": runnerID,
323+
})
322324
d.SetId("")
323325
return nil
324326
}
325-
return diag.FromErr(fmt.Errorf("error reading enterprise hosted runner: %w", err))
326-
}
327-
328-
if runner == nil {
329-
return diag.Errorf("no runner data returned from API")
327+
return diag.Errorf("error reading enterprise hosted runner: %s", err.Error())
330328
}
331329

332330
if err := d.Set("enterprise_slug", enterpriseSlug); err != nil {
333331
return diag.FromErr(err)
334332
}
335333

336-
if runner.ID != nil {
337-
if err := d.Set("runner_id", int(*runner.ID)); err != nil {
338-
return diag.FromErr(err)
339-
}
334+
if err := d.Set("runner_id", int(runner.GetID())); err != nil {
335+
return diag.FromErr(err)
340336
}
341337

342-
if runner.Name != nil {
343-
if err := d.Set("name", *runner.Name); err != nil {
344-
return diag.FromErr(err)
345-
}
338+
if err := d.Set("name", runner.GetName()); err != nil {
339+
return diag.FromErr(err)
346340
}
347-
if runner.Status != nil {
348-
if err := d.Set("status", *runner.Status); err != nil {
349-
return diag.FromErr(err)
350-
}
341+
342+
if err := d.Set("status", runner.GetStatus()); err != nil {
343+
return diag.FromErr(err)
351344
}
352-
if runner.Platform != nil {
353-
if err := d.Set("platform", *runner.Platform); err != nil {
354-
return diag.FromErr(err)
355-
}
345+
346+
if err := d.Set("platform", runner.GetPlatform()); err != nil {
347+
return diag.FromErr(err)
356348
}
349+
357350
if runner.LastActiveOn != nil {
358351
if err := d.Set("last_active_on", runner.LastActiveOn.Format(time.RFC3339)); err != nil {
359352
return diag.FromErr(err)
360353
}
361354
}
362-
if runner.PublicIPEnabled != nil {
363-
if err := d.Set("public_ip_enabled", *runner.PublicIPEnabled); err != nil {
364-
return diag.FromErr(err)
365-
}
355+
356+
if err := d.Set("public_ip_enabled", runner.GetPublicIPEnabled()); err != nil {
357+
return diag.FromErr(err)
366358
}
367359

368-
if runner.ImageDetails != nil {
369-
if err := d.Set("image", flattenHostedRunnerImage(runner.ImageDetails)); err != nil {
370-
return diag.FromErr(err)
371-
}
360+
if err := d.Set("image", flattenHostedRunnerImage(runner.ImageDetails)); err != nil {
361+
return diag.FromErr(err)
372362
}
373363

374364
if runner.MachineSizeDetails != nil {
@@ -380,16 +370,12 @@ func resourceGithubEnterpriseActionsHostedRunnerRead(ctx context.Context, d *sch
380370
}
381371
}
382372

383-
if runner.RunnerGroupID != nil {
384-
if err := d.Set("runner_group_id", int(*runner.RunnerGroupID)); err != nil {
385-
return diag.FromErr(err)
386-
}
373+
if err := d.Set("runner_group_id", int(runner.GetRunnerGroupID())); err != nil {
374+
return diag.FromErr(err)
387375
}
388376

389-
if runner.MaximumRunners != nil {
390-
if err := d.Set("maximum_runners", int(*runner.MaximumRunners)); err != nil {
391-
return diag.FromErr(err)
392-
}
377+
if err := d.Set("maximum_runners", int(runner.GetMaximumRunners())); err != nil {
378+
return diag.FromErr(err)
393379
}
394380

395381
if runner.PublicIPs != nil {
@@ -404,7 +390,7 @@ func resourceGithubEnterpriseActionsHostedRunnerRead(ctx context.Context, d *sch
404390
func resourceGithubEnterpriseActionsHostedRunnerUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
405391
client := meta.(*Owner).v3client
406392

407-
enterpriseSlug, runnerIDStr, err := parseTwoPartID(d.Id(), "enterprise_slug", "runner_id")
393+
enterpriseSlug, runnerIDStr, err := parseID2(d.Id())
408394
if err != nil {
409395
return diag.FromErr(err)
410396
}
@@ -414,48 +400,26 @@ func resourceGithubEnterpriseActionsHostedRunnerUpdate(ctx context.Context, d *s
414400
return diag.Errorf("invalid runner ID %q: %s", runnerIDStr, err.Error())
415401
}
416402

417-
request := &github.HostedRunnerRequest{}
418-
hasChanges := false
419-
420-
if d.HasChange("name") {
421-
request.Name = d.Get("name").(string)
422-
hasChanges = true
423-
}
424-
if d.HasChange("size") {
425-
request.Size = d.Get("size").(string)
426-
hasChanges = true
427-
}
428-
if d.HasChange("runner_group_id") {
429-
request.RunnerGroupID = int64(d.Get("runner_group_id").(int))
430-
hasChanges = true
431-
}
432-
if d.HasChange("maximum_runners") {
433-
request.MaximumRunners = int64(d.Get("maximum_runners").(int))
434-
hasChanges = true
435-
}
436-
if d.HasChange("public_ip_enabled") {
437-
request.EnableStaticIP = d.Get("public_ip_enabled").(bool)
438-
hasChanges = true
439-
}
440-
441-
// This should rarely happen as Terraform only calls Update when there are changes in the plan.
442-
// However, computed fields or external changes could trigger Update without actual user-configurable changes.
443-
if !hasChanges {
444-
return nil
403+
request := &github.HostedRunnerRequest{
404+
Name: d.Get("name").(string),
405+
Size: d.Get("size").(string),
406+
RunnerGroupID: int64(d.Get("runner_group_id").(int)),
407+
MaximumRunners: int64(d.Get("maximum_runners").(int)),
408+
EnableStaticIP: d.Get("public_ip_enabled").(bool),
445409
}
446410

447411
_, _, err = client.Enterprise.UpdateHostedRunner(ctx, enterpriseSlug, runnerID, *request)
448412
if err != nil {
449413
return diag.Errorf("error updating enterprise hosted runner: %s", err.Error())
450414
}
451415

452-
return nil
416+
return resourceGithubEnterpriseActionsHostedRunnerRead(ctx, d, meta)
453417
}
454418

455419
func resourceGithubEnterpriseActionsHostedRunnerDelete(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
456420
client := meta.(*Owner).v3client
457421

458-
enterpriseSlug, runnerIDStr, err := parseTwoPartID(d.Id(), "enterprise_slug", "runner_id")
422+
enterpriseSlug, runnerIDStr, err := parseID2(d.Id())
459423
if err != nil {
460424
return diag.FromErr(err)
461425
}

‎github/resource_github_enterprise_actions_hosted_runner_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ import (
44
"fmt"
55
"testing"
66

7-
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest"
8-
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
7+
"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
8+
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
99
)
1010

1111
func TestAccGithubEnterpriseActionsHostedRunner(t *testing.T) {

0 commit comments

Comments
 (0)