Skip to content

Commit aed945b

Browse files
ErikElkinsErikElkins
committed
Fixes from code review
1 parent f8d8807 commit aed945b

4 files changed

Lines changed: 33 additions & 46 deletions

‎github/resource_github_enterprise_ip_allow_list_entry.go‎

Lines changed: 7 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,17 @@ package github
22

33
import (
44
"context"
5-
"log"
65
"strings"
76

7+
"github.com/hashicorp/terraform-plugin-log/tflog"
88
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
99
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
1010
"github.com/shurcooL/githubv4"
1111
)
1212

1313
func resourceGithubEnterpriseIpAllowListEntry() *schema.Resource {
1414
return &schema.Resource{
15+
Description: "Manage a GitHub Enterprise IP Allow List Entry.",
1516
CreateContext: resourceGithubEnterpriseIpAllowListEntryCreate,
1617
ReadContext: resourceGithubEnterpriseIpAllowListEntryRead,
1718
UpdateContext: resourceGithubEnterpriseIpAllowListEntryUpdate,
@@ -98,7 +99,7 @@ func resourceGithubEnterpriseIpAllowListEntryCreate(ctx context.Context, d *sche
9899

99100
d.SetId(string(mutation.CreateIpAllowListEntry.IpAllowListEntry.ID))
100101

101-
return resourceGithubEnterpriseIpAllowListEntryRead(ctx, d, meta)
102+
return nil
102103
}
103104

104105
func resourceGithubEnterpriseIpAllowListEntryRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
@@ -129,7 +130,9 @@ func resourceGithubEnterpriseIpAllowListEntryRead(ctx context.Context, d *schema
129130
err := client.Query(ctx, &query, variables)
130131
if err != nil {
131132
if strings.Contains(err.Error(), "Could not resolve to a node with the global id") {
132-
log.Printf("[INFO] Removing IP allow list entry (%s) from state because it no longer exists in GitHub", d.Id())
133+
tflog.Info(ctx, "[INFO] Removing IP allow list entry (%s) from state because it no longer exists in GitHub", map[string]any{
134+
"id": d.Id(),
135+
})
133136
d.SetId("")
134137
return nil
135138
}
@@ -179,7 +182,7 @@ func resourceGithubEnterpriseIpAllowListEntryUpdate(ctx context.Context, d *sche
179182
return diag.FromErr(err)
180183
}
181184

182-
return resourceGithubEnterpriseIpAllowListEntryRead(ctx, d, meta)
185+
return nil
183186
}
184187

185188
func resourceGithubEnterpriseIpAllowListEntryDelete(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
@@ -203,23 +206,3 @@ func resourceGithubEnterpriseIpAllowListEntryDelete(ctx context.Context, d *sche
203206
d.SetId("")
204207
return nil
205208
}
206-
207-
// Helper function to get Enterprise ID from slug
208-
func getEnterpriseID(ctx context.Context, client *githubv4.Client, enterpriseSlug string) (string, error) {
209-
var query struct {
210-
Enterprise struct {
211-
ID githubv4.ID
212-
} `graphql:"enterprise(slug: $slug)"`
213-
}
214-
215-
variables := map[string]interface{}{
216-
"slug": githubv4.String(enterpriseSlug),
217-
}
218-
219-
err := client.Query(ctx, &query, variables)
220-
if err != nil {
221-
return "", err
222-
}
223-
224-
return query.Enterprise.ID.(string), nil
225-
}

‎github/resource_github_enterprise_ip_allow_list_entry_test.go‎

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

33
import (
44
"fmt"
5+
"strconv"
56
"testing"
67

78
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
89
)
910

1011
func TestAccGithubEnterpriseIpAllowListEntry_basic(t *testing.T) {
11-
t.Skip("Acceptance test requires a real GitHub Enterprise environment")
12-
1312
resourceName := "github_enterprise_ip_allow_list_entry.test"
1413
enterpriseSlug := "test-enterprise"
1514
ip := "192.168.1.0/24"
@@ -20,15 +19,15 @@ func TestAccGithubEnterpriseIpAllowListEntry_basic(t *testing.T) {
2019
PreCheck: func() {
2120
skipUnlessEnterprise(t)
2221
},
23-
Providers: testAccProviders,
22+
ProviderFactories: providerFactories,
2423
Steps: []resource.TestStep{
2524
{
2625
Config: testAccGithubEnterpriseIpAllowListEntryConfig(enterpriseSlug, ip, name, isActive),
2726
Check: resource.ComposeTestCheckFunc(
2827
resource.TestCheckResourceAttr(resourceName, "enterprise_slug", enterpriseSlug),
2928
resource.TestCheckResourceAttr(resourceName, "ip", ip),
3029
resource.TestCheckResourceAttr(resourceName, "name", name),
31-
resource.TestCheckResourceAttr(resourceName, "is_active", fmt.Sprintf("%t", isActive)),
30+
resource.TestCheckResourceAttr(resourceName, "is_active", strconv.FormatBool(isActive)),
3231
),
3332
},
3433
{
@@ -41,8 +40,6 @@ func TestAccGithubEnterpriseIpAllowListEntry_basic(t *testing.T) {
4140
}
4241

4342
func TestAccGithubEnterpriseIpAllowListEntry_update(t *testing.T) {
44-
t.Skip("Acceptance test requires a real GitHub Enterprise environment")
45-
4643
resourceName := "github_enterprise_ip_allow_list_entry.test"
4744
enterpriseSlug := "test-enterprise"
4845
ip := "192.168.1.0/24"
@@ -57,7 +54,7 @@ func TestAccGithubEnterpriseIpAllowListEntry_update(t *testing.T) {
5754
PreCheck: func() {
5855
skipUnlessEnterprise(t)
5956
},
60-
Providers: testAccProviders,
57+
ProviderFactories: providerFactories,
6158
Steps: []resource.TestStep{
6259
{
6360
Config: testAccGithubEnterpriseIpAllowListEntryConfig(enterpriseSlug, ip, name, isActive),

‎github/resource_github_enterprise_organization.go‎

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ func resourceGithubEnterpriseOrganizationImport(data *schema.ResourceData, meta
268268
v4 := meta.(*Owner).v4client
269269
ctx := context.Background()
270270

271-
enterpriseId, err := getEnterpriseId(ctx, v4, parts[0])
271+
enterpriseId, err := getEnterpriseID(ctx, v4, parts[0])
272272
if err != nil {
273273
return nil, err
274274
}
@@ -287,20 +287,6 @@ func resourceGithubEnterpriseOrganizationImport(data *schema.ResourceData, meta
287287
return []*schema.ResourceData{data}, nil
288288
}
289289

290-
func getEnterpriseId(ctx context.Context, v4 *githubv4.Client, enterpriseSlug string) (string, error) {
291-
var query struct {
292-
Enterprise struct {
293-
ID githubv4.String
294-
} `graphql:"enterprise(slug: $enterpriseSlug)"`
295-
}
296-
297-
err := v4.Query(ctx, &query, map[string]any{"enterpriseSlug": githubv4.String(enterpriseSlug)})
298-
if err != nil {
299-
return "", err
300-
}
301-
return string(query.Enterprise.ID), nil
302-
}
303-
304290
func getOrganizationId(ctx context.Context, v4 *githubv4.Client, orgName string) (string, error) {
305291
var query struct {
306292
Organization struct {

‎github/util_v4.go‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package github
22

33
import (
4+
"context"
5+
46
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
57
"github.com/shurcooL/githubv4"
68
)
@@ -48,3 +50,22 @@ func githubv4IDSliceEmpty(ss []string) []githubv4.ID {
4850
func githubv4NewStringSlice(v []githubv4.String) *[]githubv4.String { return &v }
4951

5052
func githubv4NewIDSlice(v []githubv4.ID) *[]githubv4.ID { return &v }
53+
54+
func getEnterpriseID(ctx context.Context, client *githubv4.Client, enterpriseSlug string) (string, error) {
55+
var query struct {
56+
Enterprise struct {
57+
ID githubv4.ID
58+
} `graphql:"enterprise(slug: $slug)"`
59+
}
60+
61+
variables := map[string]interface{}{
62+
"slug": githubv4.String(enterpriseSlug),
63+
}
64+
65+
err := client.Query(ctx, &query, variables)
66+
if err != nil {
67+
return "", err
68+
}
69+
70+
return query.Enterprise.ID.(string), nil
71+
}

0 commit comments

Comments
 (0)