Skip to content

Commit 06b9a9c

Browse files
nico34638nico34638
committed
resource_github_enterprise_actions_hosted_runner: fix review
1 parent 498d39f commit 06b9a9c

2 files changed

Lines changed: 60 additions & 85 deletions

File tree

‎github/resource_github_enterprise_actions_hosted_runner.go‎

Lines changed: 58 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,13 @@ package github
33
import (
44
"context"
55
"fmt"
6-
"log"
76
"net/http"
87
"regexp"
98
"strconv"
109
"time"
1110

1211
"github.com/google/go-github/v82/github"
12+
"github.com/hashicorp/terraform-plugin-log/tflog"
1313
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
1414
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry"
1515
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
@@ -25,6 +25,11 @@ func resourceGithubEnterpriseActionsHostedRunner() *schema.Resource {
2525
Importer: &schema.ResourceImporter{
2626
StateContext: schema.ImportStatePassthroughContext,
2727
},
28+
29+
Timeouts: &schema.ResourceTimeout{
30+
Delete: schema.DefaultTimeout(15 * time.Minute),
31+
},
32+
2833
Schema: map[string]*schema.Schema{
2934
"enterprise_slug": {
3035
Type: schema.TypeString,
@@ -228,48 +233,46 @@ func resourceGithubEnterpriseActionsHostedRunnerCreate(ctx context.Context, d *s
228233
}
229234

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

233242
// Populate computed fields directly from API response
234243
if err := d.Set("enterprise_slug", enterpriseSlug); err != nil {
235244
return diag.FromErr(err)
236245
}
237246

238247
// runner.ID is guaranteed non-nil due to check above
239-
if err := d.Set("runner_id", int(*runner.ID)); err != nil {
248+
if err := d.Set("runner_id", int(runner.GetID())); err != nil {
240249
return diag.FromErr(err)
241250
}
242251

243-
if runner.Name != nil {
244-
if err := d.Set("name", *runner.Name); err != nil {
245-
return diag.FromErr(err)
246-
}
252+
if err := d.Set("name", runner.GetName()); err != nil {
253+
return diag.FromErr(err)
247254
}
248-
if runner.Status != nil {
249-
if err := d.Set("status", *runner.Status); err != nil {
250-
return diag.FromErr(err)
251-
}
255+
256+
if err := d.Set("status", runner.GetStatus()); err != nil {
257+
return diag.FromErr(err)
252258
}
253-
if runner.Platform != nil {
254-
if err := d.Set("platform", *runner.Platform); err != nil {
255-
return diag.FromErr(err)
256-
}
259+
260+
if err := d.Set("platform", runner.GetPlatform()); err != nil {
261+
return diag.FromErr(err)
257262
}
263+
258264
if runner.LastActiveOn != nil {
259265
if err := d.Set("last_active_on", runner.LastActiveOn.Format(time.RFC3339)); err != nil {
260266
return diag.FromErr(err)
261267
}
262268
}
263-
if runner.PublicIPEnabled != nil {
264-
if err := d.Set("public_ip_enabled", *runner.PublicIPEnabled); err != nil {
265-
return diag.FromErr(err)
266-
}
269+
270+
if err := d.Set("public_ip_enabled", runner.GetPublicIPEnabled()); err != nil {
271+
return diag.FromErr(err)
267272
}
268273

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

275278
if runner.MachineSizeDetails != nil {
@@ -281,16 +284,12 @@ func resourceGithubEnterpriseActionsHostedRunnerCreate(ctx context.Context, d *s
281284
}
282285
}
283286

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

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

296295
if runner.PublicIPs != nil {
@@ -305,7 +304,7 @@ func resourceGithubEnterpriseActionsHostedRunnerCreate(ctx context.Context, d *s
305304
func resourceGithubEnterpriseActionsHostedRunnerRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
306305
client := meta.(*Owner).v3client
307306

308-
enterpriseSlug, runnerIDStr, err := parseTwoPartID(d.Id(), "enterprise_slug", "runner_id")
307+
enterpriseSlug, runnerIDStr, err := parseID2(d.Id())
309308
if err != nil {
310309
return diag.FromErr(err)
311310
}
@@ -318,57 +317,49 @@ func resourceGithubEnterpriseActionsHostedRunnerRead(ctx context.Context, d *sch
318317
runner, resp, err := client.Enterprise.GetHostedRunner(ctx, enterpriseSlug, runnerID)
319318
if err != nil {
320319
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())
320+
tflog.Info(ctx, "Removing enterprise hosted runner from state because it no longer exists in GitHub", map[string]any{
321+
"id": d.Id(),
322+
"enterprise_slug": enterpriseSlug,
323+
"runner_id": runnerID,
324+
})
322325
d.SetId("")
323326
return nil
324327
}
325328
return diag.FromErr(fmt.Errorf("error reading enterprise hosted runner: %w", err))
326329
}
327330

328-
if runner == nil {
329-
return diag.Errorf("no runner data returned from API")
330-
}
331-
332331
if err := d.Set("enterprise_slug", enterpriseSlug); err != nil {
333332
return diag.FromErr(err)
334333
}
335334

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

342-
if runner.Name != nil {
343-
if err := d.Set("name", *runner.Name); err != nil {
344-
return diag.FromErr(err)
345-
}
339+
if err := d.Set("name", runner.GetName()); err != nil {
340+
return diag.FromErr(err)
346341
}
347-
if runner.Status != nil {
348-
if err := d.Set("status", *runner.Status); err != nil {
349-
return diag.FromErr(err)
350-
}
342+
343+
if err := d.Set("status", runner.GetStatus()); err != nil {
344+
return diag.FromErr(err)
351345
}
352-
if runner.Platform != nil {
353-
if err := d.Set("platform", *runner.Platform); err != nil {
354-
return diag.FromErr(err)
355-
}
346+
347+
if err := d.Set("platform", runner.GetPlatform()); err != nil {
348+
return diag.FromErr(err)
356349
}
350+
357351
if runner.LastActiveOn != nil {
358352
if err := d.Set("last_active_on", runner.LastActiveOn.Format(time.RFC3339)); err != nil {
359353
return diag.FromErr(err)
360354
}
361355
}
362-
if runner.PublicIPEnabled != nil {
363-
if err := d.Set("public_ip_enabled", *runner.PublicIPEnabled); err != nil {
364-
return diag.FromErr(err)
365-
}
356+
357+
if err := d.Set("public_ip_enabled", runner.GetPublicIPEnabled()); err != nil {
358+
return diag.FromErr(err)
366359
}
367360

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

374365
if runner.MachineSizeDetails != nil {
@@ -380,16 +371,12 @@ func resourceGithubEnterpriseActionsHostedRunnerRead(ctx context.Context, d *sch
380371
}
381372
}
382373

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

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

395382
if runner.PublicIPs != nil {
@@ -404,7 +391,7 @@ func resourceGithubEnterpriseActionsHostedRunnerRead(ctx context.Context, d *sch
404391
func resourceGithubEnterpriseActionsHostedRunnerUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
405392
client := meta.(*Owner).v3client
406393

407-
enterpriseSlug, runnerIDStr, err := parseTwoPartID(d.Id(), "enterprise_slug", "runner_id")
394+
enterpriseSlug, runnerIDStr, err := parseID2(d.Id())
408395
if err != nil {
409396
return diag.FromErr(err)
410397
}
@@ -415,33 +402,21 @@ func resourceGithubEnterpriseActionsHostedRunnerUpdate(ctx context.Context, d *s
415402
}
416403

417404
request := &github.HostedRunnerRequest{}
418-
hasChanges := false
419405

420406
if d.HasChange("name") {
421407
request.Name = d.Get("name").(string)
422-
hasChanges = true
423408
}
424409
if d.HasChange("size") {
425410
request.Size = d.Get("size").(string)
426-
hasChanges = true
427411
}
428412
if d.HasChange("runner_group_id") {
429413
request.RunnerGroupID = int64(d.Get("runner_group_id").(int))
430-
hasChanges = true
431414
}
432415
if d.HasChange("maximum_runners") {
433416
request.MaximumRunners = int64(d.Get("maximum_runners").(int))
434-
hasChanges = true
435417
}
436418
if d.HasChange("public_ip_enabled") {
437419
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
445420
}
446421

447422
_, _, err = client.Enterprise.UpdateHostedRunner(ctx, enterpriseSlug, runnerID, *request)
@@ -455,7 +430,7 @@ func resourceGithubEnterpriseActionsHostedRunnerUpdate(ctx context.Context, d *s
455430
func resourceGithubEnterpriseActionsHostedRunnerDelete(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
456431
client := meta.(*Owner).v3client
457432

458-
enterpriseSlug, runnerIDStr, err := parseTwoPartID(d.Id(), "enterprise_slug", "runner_id")
433+
enterpriseSlug, runnerIDStr, err := parseID2(d.Id())
459434
if err != nil {
460435
return diag.FromErr(err)
461436
}

‎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)