From c4d4c88a0824862ec88482653470d61336506f9a Mon Sep 17 00:00:00 2001 From: Austen Stone Date: Thu, 10 Sep 2026 15:02:26 -0700 Subject: [PATCH 1/4] Allow removing runner group network configurations Add explicit removal flags for organization and enterprise runner group updates, preserving existing omission and string behavior. --- github/actions_runner_groups.go | 19 ++++ github/actions_runner_groups_test.go | 88 +++++++++++++++++++ github/enterprise_actions_runner_groups.go | 19 ++++ .../enterprise_actions_runner_groups_test.go | 88 +++++++++++++++++++ github/github-accessors.go | 16 ++++ github/github-accessors_test.go | 16 ++++ 6 files changed, 246 insertions(+) diff --git a/github/actions_runner_groups.go b/github/actions_runner_groups.go index 40da3fd5a0b..67309cd29cc 100644 --- a/github/actions_runner_groups.go +++ b/github/actions_runner_groups.go @@ -7,6 +7,7 @@ package github import ( "context" + "encoding/json" "fmt" ) @@ -59,6 +60,24 @@ type UpdateRunnerGroupRequest struct { RestrictedToWorkflows *bool `json:"restricted_to_workflows,omitempty"` SelectedWorkflows []string `json:"selected_workflows,omitempty"` NetworkConfigurationID *string `json:"network_configuration_id,omitempty"` + + // If true, the network configuration is removed by sending null. + // This takes precedence over NetworkConfigurationID. + RemoveNetworkConfiguration bool `json:"-"` +} + +// MarshalJSON implements the json.Marshaler interface. +func (r UpdateRunnerGroupRequest) MarshalJSON() ([]byte, error) { + type alias UpdateRunnerGroupRequest + if !r.RemoveNetworkConfiguration { + return json.Marshal(alias(r)) + } + return json.Marshal(&struct { + alias + NetworkConfigurationID *string `json:"network_configuration_id"` + }{ + alias: alias(r), + }) } // SetRepoAccessRunnerGroupRequest represents a request to replace the list of repositories diff --git a/github/actions_runner_groups_test.go b/github/actions_runner_groups_test.go index 820021d0bbf..203ad03ecec 100644 --- a/github/actions_runner_groups_test.go +++ b/github/actions_runner_groups_test.go @@ -6,6 +6,7 @@ package github import ( + "encoding/json" "fmt" "net/http" "testing" @@ -289,6 +290,93 @@ func TestActionsService_UpdateOrganizationRunnerGroup(t *testing.T) { }) } +func TestActionsService_UpdateOrganizationRunnerGroup_NetworkConfiguration(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + body UpdateRunnerGroupRequest + want string + }{ + { + name: "omitted", + body: UpdateRunnerGroupRequest{}, + want: `{}`, + }, + { + name: "set", + body: UpdateRunnerGroupRequest{NetworkConfigurationID: new("network-id")}, + want: `{"network_configuration_id":"network-id"}`, + }, + { + name: "empty string", + body: UpdateRunnerGroupRequest{NetworkConfigurationID: new("")}, + want: `{"network_configuration_id":""}`, + }, + { + name: "rename only", + body: UpdateRunnerGroupRequest{Name: new("renamed")}, + want: `{"name":"renamed"}`, + }, + { + name: "remove", + body: UpdateRunnerGroupRequest{RemoveNetworkConfiguration: true}, + want: `{"network_configuration_id":null}`, + }, + { + name: "remove overrides ID", + body: UpdateRunnerGroupRequest{ + NetworkConfigurationID: new("network-id"), + RemoveNetworkConfiguration: true, + }, + want: `{"network_configuration_id":null}`, + }, + { + name: "remove with other fields", + body: UpdateRunnerGroupRequest{ + Name: new("renamed"), + Visibility: new("selected"), + AllowsPublicRepositories: new(false), + RestrictedToWorkflows: new(true), + SelectedWorkflows: []string{"o/r/.github/workflows/build.yml@refs/heads/main"}, + RemoveNetworkConfiguration: true, + }, + want: `{"name":"renamed","visibility":"selected","allows_public_repositories":false,"restricted_to_workflows":true,"selected_workflows":["o/r/.github/workflows/build.yml@refs/heads/main"],"network_configuration_id":null}`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + before := Stringify(tt.body) + testJSONMarshalOnly(t, tt.body, tt.want) + testJSONMarshalOnly(t, &tt.body, tt.want) + if got := Stringify(tt.body); got != before { + t.Errorf("json.Marshal changed request to %v, want %v", got, before) + } + + var want map[string]json.RawMessage + if err := json.Unmarshal([]byte(tt.want), &want); err != nil { + t.Fatalf("json.Unmarshal returned error: %v", err) + } + + client, mux, _ := setup(t) + mux.HandleFunc("/orgs/o/actions/runner-groups/2", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "PATCH") + testJSONBody(t, r, want) + fmt.Fprint(w, `{"id":2}`) + }) + + if _, _, err := client.Actions.UpdateOrganizationRunnerGroup(t.Context(), "o", 2, tt.body); err != nil { + t.Fatalf("Actions.UpdateOrganizationRunnerGroup returned error: %v", err) + } + if got := Stringify(tt.body); got != before { + t.Errorf("Actions.UpdateOrganizationRunnerGroup changed request to %v, want %v", got, before) + } + }) + } +} + func TestActionsService_ListRepositoryAccessRunnerGroup(t *testing.T) { t.Parallel() client, mux, _ := setup(t) diff --git a/github/enterprise_actions_runner_groups.go b/github/enterprise_actions_runner_groups.go index 7cf75c33ae3..a42f789fd92 100644 --- a/github/enterprise_actions_runner_groups.go +++ b/github/enterprise_actions_runner_groups.go @@ -7,6 +7,7 @@ package github import ( "context" + "encoding/json" "fmt" ) @@ -65,6 +66,24 @@ type UpdateEnterpriseRunnerGroupRequest struct { RestrictedToWorkflows *bool `json:"restricted_to_workflows,omitempty"` SelectedWorkflows []string `json:"selected_workflows,omitempty"` NetworkConfigurationID *string `json:"network_configuration_id,omitempty"` + + // If true, the network configuration is removed by sending null. + // This takes precedence over NetworkConfigurationID. + RemoveNetworkConfiguration bool `json:"-"` +} + +// MarshalJSON implements the json.Marshaler interface. +func (r UpdateEnterpriseRunnerGroupRequest) MarshalJSON() ([]byte, error) { + type alias UpdateEnterpriseRunnerGroupRequest + if !r.RemoveNetworkConfiguration { + return json.Marshal(alias(r)) + } + return json.Marshal(&struct { + alias + NetworkConfigurationID *string `json:"network_configuration_id"` + }{ + alias: alias(r), + }) } // SetOrgAccessRunnerGroupRequest represents a request to replace the list of organizations diff --git a/github/enterprise_actions_runner_groups_test.go b/github/enterprise_actions_runner_groups_test.go index 92b8bfef510..16f0b30e850 100644 --- a/github/enterprise_actions_runner_groups_test.go +++ b/github/enterprise_actions_runner_groups_test.go @@ -6,6 +6,7 @@ package github import ( + "encoding/json" "fmt" "net/http" "testing" @@ -281,6 +282,93 @@ func TestEnterpriseService_UpdateEnterpriseRunnerGroup(t *testing.T) { }) } +func TestEnterpriseService_UpdateEnterpriseRunnerGroup_NetworkConfiguration(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + body UpdateEnterpriseRunnerGroupRequest + want string + }{ + { + name: "omitted", + body: UpdateEnterpriseRunnerGroupRequest{}, + want: `{}`, + }, + { + name: "set", + body: UpdateEnterpriseRunnerGroupRequest{NetworkConfigurationID: new("network-id")}, + want: `{"network_configuration_id":"network-id"}`, + }, + { + name: "empty string", + body: UpdateEnterpriseRunnerGroupRequest{NetworkConfigurationID: new("")}, + want: `{"network_configuration_id":""}`, + }, + { + name: "rename only", + body: UpdateEnterpriseRunnerGroupRequest{Name: new("renamed")}, + want: `{"name":"renamed"}`, + }, + { + name: "remove", + body: UpdateEnterpriseRunnerGroupRequest{RemoveNetworkConfiguration: true}, + want: `{"network_configuration_id":null}`, + }, + { + name: "remove overrides ID", + body: UpdateEnterpriseRunnerGroupRequest{ + NetworkConfigurationID: new("network-id"), + RemoveNetworkConfiguration: true, + }, + want: `{"network_configuration_id":null}`, + }, + { + name: "remove with other fields", + body: UpdateEnterpriseRunnerGroupRequest{ + Name: new("renamed"), + Visibility: new("selected"), + AllowsPublicRepositories: new(false), + RestrictedToWorkflows: new(true), + SelectedWorkflows: []string{"o/r/.github/workflows/build.yml@refs/heads/main"}, + RemoveNetworkConfiguration: true, + }, + want: `{"name":"renamed","visibility":"selected","allows_public_repositories":false,"restricted_to_workflows":true,"selected_workflows":["o/r/.github/workflows/build.yml@refs/heads/main"],"network_configuration_id":null}`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + before := Stringify(tt.body) + testJSONMarshalOnly(t, tt.body, tt.want) + testJSONMarshalOnly(t, &tt.body, tt.want) + if got := Stringify(tt.body); got != before { + t.Errorf("json.Marshal changed request to %v, want %v", got, before) + } + + var want map[string]json.RawMessage + if err := json.Unmarshal([]byte(tt.want), &want); err != nil { + t.Fatalf("json.Unmarshal returned error: %v", err) + } + + client, mux, _ := setup(t) + mux.HandleFunc("/enterprises/o/actions/runner-groups/2", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "PATCH") + testJSONBody(t, r, want) + fmt.Fprint(w, `{"id":2}`) + }) + + if _, _, err := client.Enterprise.UpdateEnterpriseRunnerGroup(t.Context(), "o", 2, tt.body); err != nil { + t.Fatalf("Enterprise.UpdateEnterpriseRunnerGroup returned error: %v", err) + } + if got := Stringify(tt.body); got != before { + t.Errorf("Enterprise.UpdateEnterpriseRunnerGroup changed request to %v, want %v", got, before) + } + }) + } +} + func TestEnterpriseService_ListOrganizationAccessRunnerGroup(t *testing.T) { t.Parallel() client, mux, _ := setup(t) diff --git a/github/github-accessors.go b/github/github-accessors.go index 50ac38eb513..7d36dd968f5 100644 --- a/github/github-accessors.go +++ b/github/github-accessors.go @@ -45094,6 +45094,14 @@ func (u *UpdateEnterpriseRunnerGroupRequest) GetNetworkConfigurationID() string return *u.NetworkConfigurationID } +// GetRemoveNetworkConfiguration returns the RemoveNetworkConfiguration field. +func (u *UpdateEnterpriseRunnerGroupRequest) GetRemoveNetworkConfiguration() bool { + if u == nil { + return false + } + return u.RemoveNetworkConfiguration +} + // GetRestrictedToWorkflows returns the RestrictedToWorkflows field if it's non-nil, zero value otherwise. func (u *UpdateEnterpriseRunnerGroupRequest) GetRestrictedToWorkflows() bool { if u == nil || u.RestrictedToWorkflows == nil { @@ -45750,6 +45758,14 @@ func (u *UpdateRunnerGroupRequest) GetNetworkConfigurationID() string { return *u.NetworkConfigurationID } +// GetRemoveNetworkConfiguration returns the RemoveNetworkConfiguration field. +func (u *UpdateRunnerGroupRequest) GetRemoveNetworkConfiguration() bool { + if u == nil { + return false + } + return u.RemoveNetworkConfiguration +} + // GetRestrictedToWorkflows returns the RestrictedToWorkflows field if it's non-nil, zero value otherwise. func (u *UpdateRunnerGroupRequest) GetRestrictedToWorkflows() bool { if u == nil || u.RestrictedToWorkflows == nil { diff --git a/github/github-accessors_test.go b/github/github-accessors_test.go index 921473bf3bd..204b50a1ad7 100644 --- a/github/github-accessors_test.go +++ b/github/github-accessors_test.go @@ -56291,6 +56291,14 @@ func TestUpdateEnterpriseRunnerGroupRequest_GetNetworkConfigurationID(tt *testin u.GetNetworkConfigurationID() } +func TestUpdateEnterpriseRunnerGroupRequest_GetRemoveNetworkConfiguration(tt *testing.T) { + tt.Parallel() + u := &UpdateEnterpriseRunnerGroupRequest{} + u.GetRemoveNetworkConfiguration() + u = nil + u.GetRemoveNetworkConfiguration() +} + func TestUpdateEnterpriseRunnerGroupRequest_GetRestrictedToWorkflows(tt *testing.T) { tt.Parallel() var zeroValue bool @@ -57163,6 +57171,14 @@ func TestUpdateRunnerGroupRequest_GetNetworkConfigurationID(tt *testing.T) { u.GetNetworkConfigurationID() } +func TestUpdateRunnerGroupRequest_GetRemoveNetworkConfiguration(tt *testing.T) { + tt.Parallel() + u := &UpdateRunnerGroupRequest{} + u.GetRemoveNetworkConfiguration() + u = nil + u.GetRemoveNetworkConfiguration() +} + func TestUpdateRunnerGroupRequest_GetRestrictedToWorkflows(tt *testing.T) { tt.Parallel() var zeroValue bool From 6cf899342ab6a3f82601df34a50fcc69d8eda4cc Mon Sep 17 00:00:00 2001 From: Austen Stone Date: Tue, 15 Sep 2026 16:40:41 -0600 Subject: [PATCH 2/4] Clarify runner group network removal semantics Keep explicit JSON null removal separate from empty-string compatibility. Document the primitive-pointer encoding contract and exercise the removal flag through organization and enterprise update requests. --- CONTRIBUTING.md | 11 ++++++++-- github/actions_runner_groups.go | 2 +- github/actions_runner_groups_test.go | 21 +++++++++++-------- github/enterprise_actions_runner_groups.go | 2 +- .../enterprise_actions_runner_groups_test.go | 21 +++++++++++-------- 5 files changed, 35 insertions(+), 22 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3cdb417b766..9c417b3c593 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -413,8 +413,15 @@ type RepositoryRuleset struct { } ``` -For optional boolean fields where you need to distinguish between `false` -and "not set", use `*bool` with `omitzero`. +For optional primitive fields where the zero value has API semantics, use a +pointer with `omitempty`. A nil pointer omits the field, while a pointer to the +zero value includes it, such as `false`, `0`, or `""`. + +Neither `omitempty` nor `omitzero` makes a pointer to a zero value encode as +JSON `null`. When an update must distinguish omission, assignment, and explicit +removal using `null`, follow `UpdateTeamRequest.RemoveParentTeam`: add a Go-only +removal flag and a value-receiver `MarshalJSON` method. This supports marshaling +both request values and pointers without changing the request. #### Response Bodies diff --git a/github/actions_runner_groups.go b/github/actions_runner_groups.go index 67309cd29cc..337a592b43a 100644 --- a/github/actions_runner_groups.go +++ b/github/actions_runner_groups.go @@ -61,7 +61,7 @@ type UpdateRunnerGroupRequest struct { SelectedWorkflows []string `json:"selected_workflows,omitempty"` NetworkConfigurationID *string `json:"network_configuration_id,omitempty"` - // If true, the network configuration is removed by sending null. + // If true, send a null network_configuration_id to remove the network configuration. // This takes precedence over NetworkConfigurationID. RemoveNetworkConfiguration bool `json:"-"` } diff --git a/github/actions_runner_groups_test.go b/github/actions_runner_groups_test.go index 203ad03ecec..e5538b1ac02 100644 --- a/github/actions_runner_groups_test.go +++ b/github/actions_runner_groups_test.go @@ -309,22 +309,20 @@ func TestActionsService_UpdateOrganizationRunnerGroup_NetworkConfiguration(t *te want: `{"network_configuration_id":"network-id"}`, }, { - name: "empty string", - body: UpdateRunnerGroupRequest{NetworkConfigurationID: new("")}, + name: "empty ID without removal", + body: UpdateRunnerGroupRequest{ + NetworkConfigurationID: new(""), + RemoveNetworkConfiguration: false, + }, want: `{"network_configuration_id":""}`, }, - { - name: "rename only", - body: UpdateRunnerGroupRequest{Name: new("renamed")}, - want: `{"name":"renamed"}`, - }, { name: "remove", body: UpdateRunnerGroupRequest{RemoveNetworkConfiguration: true}, want: `{"network_configuration_id":null}`, }, { - name: "remove overrides ID", + name: "remove takes precedence", body: UpdateRunnerGroupRequest{ NetworkConfigurationID: new("network-id"), RemoveNetworkConfiguration: true, @@ -332,7 +330,7 @@ func TestActionsService_UpdateOrganizationRunnerGroup_NetworkConfiguration(t *te want: `{"network_configuration_id":null}`, }, { - name: "remove with other fields", + name: "remove preserves other fields", body: UpdateRunnerGroupRequest{ Name: new("renamed"), Visibility: new("selected"), @@ -343,6 +341,11 @@ func TestActionsService_UpdateOrganizationRunnerGroup_NetworkConfiguration(t *te }, want: `{"name":"renamed","visibility":"selected","allows_public_repositories":false,"restricted_to_workflows":true,"selected_workflows":["o/r/.github/workflows/build.yml@refs/heads/main"],"network_configuration_id":null}`, }, + { + name: "rename only", + body: UpdateRunnerGroupRequest{Name: new("renamed")}, + want: `{"name":"renamed"}`, + }, } for _, tt := range tests { diff --git a/github/enterprise_actions_runner_groups.go b/github/enterprise_actions_runner_groups.go index a42f789fd92..d6a766b423e 100644 --- a/github/enterprise_actions_runner_groups.go +++ b/github/enterprise_actions_runner_groups.go @@ -67,7 +67,7 @@ type UpdateEnterpriseRunnerGroupRequest struct { SelectedWorkflows []string `json:"selected_workflows,omitempty"` NetworkConfigurationID *string `json:"network_configuration_id,omitempty"` - // If true, the network configuration is removed by sending null. + // If true, send a null network_configuration_id to remove the network configuration. // This takes precedence over NetworkConfigurationID. RemoveNetworkConfiguration bool `json:"-"` } diff --git a/github/enterprise_actions_runner_groups_test.go b/github/enterprise_actions_runner_groups_test.go index 16f0b30e850..0558284dffd 100644 --- a/github/enterprise_actions_runner_groups_test.go +++ b/github/enterprise_actions_runner_groups_test.go @@ -301,22 +301,20 @@ func TestEnterpriseService_UpdateEnterpriseRunnerGroup_NetworkConfiguration(t *t want: `{"network_configuration_id":"network-id"}`, }, { - name: "empty string", - body: UpdateEnterpriseRunnerGroupRequest{NetworkConfigurationID: new("")}, + name: "empty ID without removal", + body: UpdateEnterpriseRunnerGroupRequest{ + NetworkConfigurationID: new(""), + RemoveNetworkConfiguration: false, + }, want: `{"network_configuration_id":""}`, }, - { - name: "rename only", - body: UpdateEnterpriseRunnerGroupRequest{Name: new("renamed")}, - want: `{"name":"renamed"}`, - }, { name: "remove", body: UpdateEnterpriseRunnerGroupRequest{RemoveNetworkConfiguration: true}, want: `{"network_configuration_id":null}`, }, { - name: "remove overrides ID", + name: "remove takes precedence", body: UpdateEnterpriseRunnerGroupRequest{ NetworkConfigurationID: new("network-id"), RemoveNetworkConfiguration: true, @@ -324,7 +322,7 @@ func TestEnterpriseService_UpdateEnterpriseRunnerGroup_NetworkConfiguration(t *t want: `{"network_configuration_id":null}`, }, { - name: "remove with other fields", + name: "remove preserves other fields", body: UpdateEnterpriseRunnerGroupRequest{ Name: new("renamed"), Visibility: new("selected"), @@ -335,6 +333,11 @@ func TestEnterpriseService_UpdateEnterpriseRunnerGroup_NetworkConfiguration(t *t }, want: `{"name":"renamed","visibility":"selected","allows_public_repositories":false,"restricted_to_workflows":true,"selected_workflows":["o/r/.github/workflows/build.yml@refs/heads/main"],"network_configuration_id":null}`, }, + { + name: "rename only", + body: UpdateEnterpriseRunnerGroupRequest{Name: new("renamed")}, + want: `{"name":"renamed"}`, + }, } for _, tt := range tests { From 4eb1ab636964293403f3c9f59a86bc98998b0ec8 Mon Sep 17 00:00:00 2001 From: Austen Stone Date: Mon, 21 Sep 2026 09:34:42 -0700 Subject: [PATCH 3/4] docs: apply suggested pointer encoding guidance --- CONTRIBUTING.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9c417b3c593..ec258964e3d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -413,15 +413,15 @@ type RepositoryRuleset struct { } ``` -For optional primitive fields where the zero value has API semantics, use a -pointer with `omitempty`. A nil pointer omits the field, while a pointer to the -zero value includes it, such as `false`, `0`, or `""`. - -Neither `omitempty` nor `omitzero` makes a pointer to a zero value encode as -JSON `null`. When an update must distinguish omission, assignment, and explicit -removal using `null`, follow `UpdateTeamRequest.RemoveParentTeam`: add a Go-only -removal flag and a value-receiver `MarshalJSON` method. This supports marshaling -both request values and pointers without changing the request. +Optional pointer fields should use `omitempty`: a nil pointer omits the field, +while a pointer to a zero value such as `false`, `0`, or `""` includes it. +`omitzero` behaves identically for pointers, so prefer `omitempty`. + +Neither tag can send JSON `null`, so an update that must distinguish omission, +assignment, and removal follows `UpdateTeamRequest.RemoveParentTeam` in +`github/teams.go`: a Go-only removal flag tagged `json:"-"`, plus a +value-receiver `MarshalJSON` that also serves `*T` call sites. `new("")` sends +an empty string, not `null`. #### Response Bodies From 999afeb0dd199544aa95e5f11cb04dfdefc49cc7 Mon Sep 17 00:00:00 2001 From: Austen Stone Date: Wed, 30 Sep 2026 20:48:14 -0700 Subject: [PATCH 4/4] docs: Apply requested JSON tagging guidance --- CONTRIBUTING.md | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ec258964e3d..4b2eee5bfb6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -413,15 +413,8 @@ type RepositoryRuleset struct { } ``` -Optional pointer fields should use `omitempty`: a nil pointer omits the field, -while a pointer to a zero value such as `false`, `0`, or `""` includes it. -`omitzero` behaves identically for pointers, so prefer `omitempty`. - -Neither tag can send JSON `null`, so an update that must distinguish omission, -assignment, and removal follows `UpdateTeamRequest.RemoveParentTeam` in -`github/teams.go`: a Go-only removal flag tagged `json:"-"`, plus a -value-receiver `MarshalJSON` that also serves `*T` call sites. `new("")` sends -an empty string, not `null`. +If you need to differentiate between an unset pointer to a basic type and a `nil` value you can add an un-marshaled struct field to control this behaviour and provide a custom `MarshalJSON` implementation for the struct (see `UpdateTeamRequest.RemoveParentTeam` in +`github/teams.go`). #### Response Bodies