Skip to content

Commit 022c485

Browse files
austenstoneaustenstone
committed
refactor: use native go-github functions for org runner group networking
- Use native NetworkConfigurationID on CreateRunnerGroupRequest and UpdateRunnerGroupRequest for org runner groups (already in go-github v83) - Read NetworkConfigurationID from RunnerGroup response instead of separate getRunnerGroupNetworking call for org runner groups - Exit early on 304 Not Modified in both org and enterprise Read - Add unit tests for 304 handling and NetworkConfigurationID deserialization - Enterprise runner groups still use raw API calls (go-github v83 lacks enterprise NetworkConfigurationID — will be addressed in v84 upgrade PR)
1 parent 352e4ed commit 022c485

3 files changed

Lines changed: 147 additions & 65 deletions

‎github/resource_github_actions_runner_group.go‎

Lines changed: 28 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,12 @@ func resourceGithubActionsRunnerGroupCreate(ctx context.Context, d *schema.Resou
190190
}
191191
}
192192

193+
var networkConfigurationIDPtr *string
194+
if networkConfigurationID, ok := d.GetOk("network_configuration_id"); ok {
195+
value := networkConfigurationID.(string)
196+
networkConfigurationIDPtr = &value
197+
}
198+
193199
runnerGroup, resp, err := client.Actions.CreateOrganizationRunnerGroup(ctx,
194200
orgName,
195201
github.CreateRunnerGroupRequest{
@@ -199,6 +205,7 @@ func resourceGithubActionsRunnerGroupCreate(ctx context.Context, d *schema.Resou
199205
SelectedRepositoryIDs: selectedRepositoryIDs,
200206
SelectedWorkflows: selectedWorkflows,
201207
AllowsPublicRepositories: &allowsPublicRepositories,
208+
NetworkConfigurationID: networkConfigurationIDPtr,
202209
},
203210
)
204211
if err != nil {
@@ -208,23 +215,7 @@ func resourceGithubActionsRunnerGroupCreate(ctx context.Context, d *schema.Resou
208215
if err = setGithubActionsRunnerGroupState(d, runnerGroup, normalizeEtag(resp.Header.Get("ETag")), selectedRepositoryIDs); err != nil {
209216
return diag.FromErr(err)
210217
}
211-
212-
if networkConfigurationID, ok := d.GetOk("network_configuration_id"); ok {
213-
networkConfigurationIDValue := networkConfigurationID.(string)
214-
// The create endpoint does not accept network_configuration_id, so private networking
215-
// must be attached with a follow-up PATCH after the runner group has been created.
216-
if _, err = updateRunnerGroupNetworking(client, ctx, fmt.Sprintf("orgs/%s/actions/runner-groups/%d", orgName, runnerGroup.GetID()), &networkConfigurationIDValue); err != nil {
217-
return diag.FromErr(err)
218-
}
219-
220-
if err = setRunnerGroupNetworkingState(d, &runnerGroupNetworking{NetworkConfigurationID: &networkConfigurationIDValue}); err != nil {
221-
return diag.FromErr(err)
222-
}
223-
224-
return nil
225-
}
226-
227-
if err = setRunnerGroupNetworkingState(d, nil); err != nil {
218+
if err = d.Set("network_configuration_id", runnerGroup.NetworkConfigurationID); err != nil {
228219
return diag.FromErr(err)
229220
}
230221

@@ -262,10 +253,8 @@ func resourceGithubActionsRunnerGroupRead(ctx context.Context, d *schema.Resourc
262253
return diag.FromErr(err)
263254
}
264255

265-
runnerGroupEtag := normalizeEtag(resp.Header.Get("ETag"))
266-
runnerGroupNetworking, _, err := getRunnerGroupNetworking(client, ctx, fmt.Sprintf("orgs/%s/actions/runner-groups/%d", orgName, runnerGroupID))
267-
if err != nil {
268-
return diag.FromErr(err)
256+
if runnerGroup == nil {
257+
return nil
269258
}
270259

271260
selectedRepositoryIDs := []int64{}
@@ -290,22 +279,12 @@ func resourceGithubActionsRunnerGroupRead(ctx context.Context, d *schema.Resourc
290279
options.Page = resp.NextPage
291280
}
292281

293-
if runnerGroup != nil {
294-
if err = setGithubActionsRunnerGroupState(d, runnerGroup, runnerGroupEtag, selectedRepositoryIDs); err != nil {
295-
return diag.FromErr(err)
296-
}
297-
} else {
298-
if err := d.Set("selected_repository_ids", selectedRepositoryIDs); err != nil {
299-
return diag.FromErr(err)
300-
}
301-
if err := d.Set("etag", runnerGroupEtag); err != nil {
302-
return diag.FromErr(err)
303-
}
282+
runnerGroupEtag := normalizeEtag(resp.Header.Get("ETag"))
283+
if err = setGithubActionsRunnerGroupState(d, runnerGroup, runnerGroupEtag, selectedRepositoryIDs); err != nil {
284+
return diag.FromErr(err)
304285
}
305-
if runnerGroupNetworking != nil {
306-
if err = setRunnerGroupNetworkingState(d, runnerGroupNetworking); err != nil {
307-
return diag.FromErr(err)
308-
}
286+
if err = d.Set("network_configuration_id", runnerGroup.NetworkConfigurationID); err != nil {
287+
return diag.FromErr(err)
309288
}
310289

311290
return nil
@@ -330,12 +309,24 @@ func resourceGithubActionsRunnerGroupUpdate(ctx context.Context, d *schema.Resou
330309
}
331310
}
332311

312+
var networkConfigurationIDPtr *string
313+
if networkConfigurationID, ok := d.GetOk("network_configuration_id"); ok {
314+
value := networkConfigurationID.(string)
315+
networkConfigurationIDPtr = &value
316+
} else if d.HasChange("network_configuration_id") {
317+
// Field was removed — send empty string to clear it.
318+
// go-github's omitempty omits nil pointers, so empty string is used as a workaround.
319+
empty := ""
320+
networkConfigurationIDPtr = &empty
321+
}
322+
333323
options := github.UpdateRunnerGroupRequest{
334324
Name: &name,
335325
Visibility: &visibility,
336326
RestrictedToWorkflows: &restrictedToWorkflows,
337327
SelectedWorkflows: selectedWorkflows,
338328
AllowsPublicRepositories: &allowsPublicRepositories,
329+
NetworkConfigurationID: networkConfigurationIDPtr,
339330
}
340331

341332
runnerGroupID, err := strconv.ParseInt(d.Id(), 10, 64)
@@ -349,23 +340,6 @@ func resourceGithubActionsRunnerGroupUpdate(ctx context.Context, d *schema.Resou
349340
return diag.FromErr(err)
350341
}
351342

352-
var networkConfigurationIDValue *string
353-
if networkConfigurationID, ok := d.GetOk("network_configuration_id"); ok {
354-
value := networkConfigurationID.(string)
355-
networkConfigurationIDValue = &value
356-
}
357-
358-
if d.HasChange("network_configuration_id") {
359-
if _, err := updateRunnerGroupNetworking(client, ctx, fmt.Sprintf("orgs/%s/actions/runner-groups/%d", orgName, runnerGroupID), networkConfigurationIDValue); err != nil {
360-
return diag.FromErr(err)
361-
}
362-
}
363-
364-
var networkingState *runnerGroupNetworking
365-
if networkConfigurationIDValue != nil {
366-
networkingState = &runnerGroupNetworking{NetworkConfigurationID: networkConfigurationIDValue}
367-
}
368-
369343
selectedRepositories, hasSelectedRepositories := d.GetOk("selected_repository_ids")
370344
selectedRepositoryIDs := []int64{}
371345

@@ -388,7 +362,7 @@ func resourceGithubActionsRunnerGroupUpdate(ctx context.Context, d *schema.Resou
388362
if err := setGithubActionsRunnerGroupState(d, runnerGroup, runnerGroupEtag, selectedRepositoryIDs); err != nil {
389363
return diag.FromErr(err)
390364
}
391-
if err := setRunnerGroupNetworkingState(d, networkingState); err != nil {
365+
if err := d.Set("network_configuration_id", runnerGroup.NetworkConfigurationID); err != nil {
392366
return diag.FromErr(err)
393367
}
394368

‎github/resource_github_actions_runner_group_unit_test.go‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
package github
2+
3+
import (
4+
"context"
5+
"net/http"
6+
"net/url"
7+
"testing"
8+
9+
"github.com/google/go-github/v83/github"
10+
)
11+
12+
func TestGetOrganizationRunnerGroup_ReturnsNilOn304(t *testing.T) {
13+
ts := githubApiMock([]*mockResponse{
14+
{
15+
ExpectedUri: "/orgs/test-org/actions/runner-groups/123",
16+
ExpectedMethod: "GET",
17+
ExpectedHeaders: map[string]string{
18+
"If-None-Match": "etag-abc",
19+
},
20+
ResponseBody: `{"message":"Not Modified"}`,
21+
StatusCode: http.StatusNotModified,
22+
},
23+
})
24+
defer ts.Close()
25+
26+
httpClient := http.DefaultClient
27+
httpClient.Transport = NewEtagTransport(http.DefaultTransport)
28+
client := github.NewClient(httpClient)
29+
u, _ := url.Parse(ts.URL + "/")
30+
client.BaseURL = u
31+
32+
ctx := context.WithValue(context.Background(), ctxEtag, "etag-abc")
33+
runnerGroup, resp, err := getOrganizationRunnerGroup(client, ctx, "test-org", 123)
34+
if err != nil {
35+
t.Fatalf("expected no error, got: %v", err)
36+
}
37+
if runnerGroup != nil {
38+
t.Fatalf("expected nil runner group on 304, got: %+v", runnerGroup)
39+
}
40+
if resp == nil || resp.StatusCode != http.StatusNotModified {
41+
t.Fatalf("expected 304 response, got: %+v", resp)
42+
}
43+
}
44+
45+
func TestGetOrganizationRunnerGroup_ReturnsRunnerGroup(t *testing.T) {
46+
ts := githubApiMock([]*mockResponse{
47+
{
48+
ExpectedUri: "/orgs/test-org/actions/runner-groups/42",
49+
ExpectedMethod: "GET",
50+
ResponseBody: `{"id":42,"name":"my-group","network_configuration_id":"nc-456"}`,
51+
StatusCode: http.StatusOK,
52+
},
53+
})
54+
defer ts.Close()
55+
56+
httpClient := http.DefaultClient
57+
client := github.NewClient(httpClient)
58+
u, _ := url.Parse(ts.URL + "/")
59+
client.BaseURL = u
60+
61+
runnerGroup, resp, err := getOrganizationRunnerGroup(client, context.Background(), "test-org", 42)
62+
if err != nil {
63+
t.Fatalf("expected no error, got: %v", err)
64+
}
65+
if runnerGroup == nil {
66+
t.Fatal("expected non-nil runner group")
67+
}
68+
if runnerGroup.GetID() != 42 {
69+
t.Fatalf("expected ID 42, got %d", runnerGroup.GetID())
70+
}
71+
if runnerGroup.GetName() != "my-group" {
72+
t.Fatalf("expected name 'my-group', got %q", runnerGroup.GetName())
73+
}
74+
if runnerGroup.GetNetworkConfigurationID() != "nc-456" {
75+
t.Fatalf("expected network_configuration_id 'nc-456', got %q", runnerGroup.GetNetworkConfigurationID())
76+
}
77+
if resp == nil || resp.StatusCode != http.StatusOK {
78+
t.Fatalf("expected 200 response, got: %+v", resp)
79+
}
80+
}
81+
82+
func TestGetEnterpriseRunnerGroup_ReturnsNilOn304(t *testing.T) {
83+
ts := githubApiMock([]*mockResponse{
84+
{
85+
ExpectedUri: "/enterprises/test-ent/actions/runner-groups/99",
86+
ExpectedMethod: "GET",
87+
ExpectedHeaders: map[string]string{
88+
"If-None-Match": "etag-xyz",
89+
},
90+
ResponseBody: `{"message":"Not Modified"}`,
91+
StatusCode: http.StatusNotModified,
92+
},
93+
})
94+
defer ts.Close()
95+
96+
httpClient := http.DefaultClient
97+
httpClient.Transport = NewEtagTransport(http.DefaultTransport)
98+
client := github.NewClient(httpClient)
99+
u, _ := url.Parse(ts.URL + "/")
100+
client.BaseURL = u
101+
102+
ctx := context.WithValue(context.Background(), ctxEtag, "etag-xyz")
103+
runnerGroup, resp, err := getEnterpriseRunnerGroup(client, ctx, "test-ent", 99)
104+
if err != nil {
105+
t.Fatalf("expected no error, got: %v", err)
106+
}
107+
if runnerGroup != nil {
108+
t.Fatalf("expected nil runner group on 304, got: %+v", runnerGroup)
109+
}
110+
if resp == nil || resp.StatusCode != http.StatusNotModified {
111+
t.Fatalf("expected 304 response, got: %+v", resp)
112+
}
113+
}

‎github/resource_github_enterprise_actions_runner_group.go‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,10 @@ func resourceGithubActionsEnterpriseRunnerGroupRead(ctx context.Context, d *sche
251251
return diag.FromErr(err)
252252
}
253253

254+
if enterpriseRunnerGroup == nil {
255+
return nil
256+
}
257+
254258
runnerGroupEtag := normalizeEtag(resp.Header.Get("ETag"))
255259

256260
runnerGroupNetworking, _, err := getRunnerGroupNetworking(client, ctx, fmt.Sprintf("enterprises/%s/actions/runner-groups/%d", enterpriseSlug, runnerGroupID))
@@ -280,17 +284,8 @@ func resourceGithubActionsEnterpriseRunnerGroupRead(ctx context.Context, d *sche
280284
optionsOrgs.Page = resp.NextPage
281285
}
282286

283-
if enterpriseRunnerGroup != nil {
284-
if err = setGithubActionsEnterpriseRunnerGroupState(d, enterpriseRunnerGroup, runnerGroupEtag, enterpriseSlug, selectedOrganizationIDs); err != nil {
285-
return diag.FromErr(err)
286-
}
287-
} else {
288-
if err := d.Set("selected_organization_ids", selectedOrganizationIDs); err != nil {
289-
return diag.FromErr(err)
290-
}
291-
if err := d.Set("etag", runnerGroupEtag); err != nil {
292-
return diag.FromErr(err)
293-
}
287+
if err = setGithubActionsEnterpriseRunnerGroupState(d, enterpriseRunnerGroup, runnerGroupEtag, enterpriseSlug, selectedOrganizationIDs); err != nil {
288+
return diag.FromErr(err)
294289
}
295290
if runnerGroupNetworking != nil {
296291
if err = setRunnerGroupNetworkingState(d, runnerGroupNetworking); err != nil {

0 commit comments

Comments
 (0)