feat: Adding github_enterprise_ip_allow_list_entry resource #2649

ErikElkins · 2025-05-02T14:08:29Z

Resolves #2648

Before the change?

None

After the change?

Adding the github_enterprise_ip_allow_list_entry resource.

Pull request checklist

Tests for the changes have been added (for bug fixes / features)
Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

Yes
No

nickfloyd · 2026-01-13T16:51:41Z

@ErikElkins Apologies for the delay on getting to this PR. Would you be willing to fix the lint issue? Thank you!

stevehipwell

Is there a REST API for this functionality?

github/resource_github_enterprise_ip_allow_list_entry.go

stevehipwell · 2026-01-13T18:21:16Z

github/resource_github_enterprise_ip_allow_list_entry.go

+}
+
+// Helper function to get Enterprise ID from slug
+func getEnterpriseID(ctx context.Context, client *githubv4.Client, enterpriseSlug string) (string, error) {


AFAIK enterprise slug is the ID, the GraphQL ID shouldn't pollute TF.

If i understand this correctly, you'd still want the resource to reference the slug (and not the GraphQL ID) like so:

resource "github_enterprise_ip_allow_list_entry" "example" { enterprise_slug = "my-enterprise-slug" ip = "1.2.3.4" }

If you pass enterprise_slug to the CreateIpAllowListEntry mutation it will fail with Error: Could not resolve to a node with the global id of 'my-enterprise-slug' (i tested with a real one). It looks to need the GraphQL ID, and not the slug. If we keep the lookup like this, we should be able to keep the GraphQL ID out of the terraform and have the lookup take place behind the scenes.

https://docs.github.com/en/enterprise-cloud@latest/graphql/reference/input-objects#createipallowlistentryinput

I think it makes sense that it's not the enterprise_slug as AFAIK that Mutation also works for Organizations, which could have the same slug as the enterprise

The GraphQL ID may be required, but it shouldn't be the resource ID as that should be the slug. I'm not fully familiar with the GitHub GraphQL implementation but isn't the whole point of GraphQL that you can do everything with a single query; in which case why can't you use the slug to lookup the GraphQL ID in the same query?

Yeah, you can combine multiple "requests" in a single GQL query, but directly dependant things aren't possible AFAIK.
But there one can then just use sequential queries. And one can't mix mutations and queries in a single request

I can't see a pending REST implementation. We may have to leak a GraphQL ID into the state after all. I'd suggest using <graphql-id>:<name> for the ID so if we get a REST implementation we can remove the GraphQL dependency.

I don't recommend using name as it's actually a description field and can contain long text.

And since name is mutable, it would change the ID even if the actual resource didn't change.

Existing IP entry in UI:
Add entry in UI:

Edit entry in UI:

Are we sure IP isn't unique in the context of an enterprise?

Yes, we have had multiple duplicate IPs. We could treat it as unique even if the API allows duplicate IPS. One reason for allowing duplicate IPs would be ease of management.

For example the IPs in api.github.com/meta have duplicates between products. If I'd be syncing these blindly to the allowlist with the product name (which we are doing) then it needs to enable duplicate IPS 😬

Not that it's relevant to this PR, but wouldn't you run them through distinct by default?

ErikElkins · 2026-01-13T20:15:44Z

Yep! Let me jump in here.

Co-authored-by: Steve Hipwell <[email protected]>

ErikElkins · 2026-01-13T20:32:10Z

Oh, i see the other PR. Should we just close this? EDIT: NEVERMIND

ErikElkins · 2026-01-13T20:36:33Z

Is there a REST API for this functionality?

Doesn't look like they've added it since I opened the PR.

gateixeira

hi @ErikElkins 👋

I was updating your PR to have the lint fixed and realized that the acceptance tests seem to also not pass. Can you please update the PR with the following? Feel free to validate on your end.

github/resource_github_enterprise_ip_allow_list_entry.go

github/resource_github_enterprise_ip_allow_list_entry_test.go

Co-authored-by: gateixeira <[email protected]>

gateixeira

@ErikElkins thanks for the changes!

deiga · 2026-01-13T21:56:41Z

github/resource_github_enterprise_ip_allow_list_entry.go

+			StateContext: schema.ImportStatePassthroughContext,
+		},
+
+		Schema: map[string]*schema.Schema{


Please add a top-level Description field

deiga · 2026-01-13T21:59:35Z

github/resource_github_enterprise_ip_allow_list_entry.go

+
+	d.SetId(string(mutation.CreateIpAllowListEntry.IpAllowListEntry.ID))
+
+	return resourceGithubEnterpriseIpAllowListEntryRead(ctx, d, meta)


Please remove the call to Read in Create. Instead set any computed values based on the response

deiga · 2026-01-13T21:59:50Z

github/resource_github_enterprise_ip_allow_list_entry.go

+		return diag.FromErr(err)
+	}
+
+	return resourceGithubEnterpriseIpAllowListEntryRead(ctx, d, meta)


Same for Update https://github.com/integrations/terraform-provider-github/pull/2649/changes#r2688260511

deiga · 2026-01-13T22:01:42Z

github/resource_github_enterprise_ip_allow_list_entry.go

+	err := client.Query(ctx, &query, variables)
+	if err != nil {
+		if strings.Contains(err.Error(), "Could not resolve to a node with the global id") {
+			log.Printf("[INFO] Removing IP allow list entry (%s) from state because it no longer exists in GitHub", d.Id())


Please use tflog.Info instead of log.Printf

deiga · 2026-01-13T22:04:55Z

github/resource_github_enterprise_ip_allow_list_entry.go

+}
+
+// Helper function to get Enterprise ID from slug
+func getEnterpriseID(ctx context.Context, client *githubv4.Client, enterpriseSlug string) (string, error) {


This function seems to exist already in resource_github_enterprise_organization.go, could you re-use that?

Maybe it could be moved to util_v4.go?

deiga · 2026-01-13T22:05:19Z

github/resource_github_enterprise_ip_allow_list_entry_test.go

+)
+
+func TestAccGithubEnterpriseIpAllowListEntry_basic(t *testing.T) {
+	t.Skip("Acceptance test requires a real GitHub Enterprise environment")


Suggested change

t.Skip("Acceptance test requires a real GitHub Enterprise environment")

deiga · 2026-01-13T22:05:54Z

github/resource_github_enterprise_ip_allow_list_entry_test.go

+		PreCheck: func() {
+			skipUnlessEnterprise(t)
+		},
+		Providers: testAccProviders,


Suggested change

Providers: testAccProviders,

ProviderFactories: providerFactories,

deiga · 2026-01-13T22:07:33Z

github/resource_github_enterprise_ip_allow_list_entry_test.go

+					resource.TestCheckResourceAttr(resourceName, "enterprise_slug", enterpriseSlug),
+					resource.TestCheckResourceAttr(resourceName, "ip", ip),
+					resource.TestCheckResourceAttr(resourceName, "name", name),
+					resource.TestCheckResourceAttr(resourceName, "is_active", fmt.Sprintf("%t", isActive)),


Suggested change

resource.TestCheckResourceAttr(resourceName, "is_active", fmt.Sprintf("%t", isActive)),

resource.TestCheckResourceAttr(resourceName, "is_active", strconv.FormatBool(isActive)),

deiga · 2026-01-13T22:07:43Z

github/resource_github_enterprise_ip_allow_list_entry_test.go

+}
+
+func TestAccGithubEnterpriseIpAllowListEntry_update(t *testing.T) {
+	t.Skip("Acceptance test requires a real GitHub Enterprise environment")


Suggested change

t.Skip("Acceptance test requires a real GitHub Enterprise environment")

deiga · 2026-01-13T22:07:57Z

github/resource_github_enterprise_ip_allow_list_entry_test.go

+		PreCheck: func() {
+			skipUnlessEnterprise(t)
+		},
+		Providers: testAccProviders,


Suggested change

Providers: testAccProviders,

ProviderFactories: providerFactories,

deiga · 2026-01-16T09:05:41Z

@ErikElkins Are you able to continue finalizing this PR? :)

Adding github_enterprise_ip_allow_list_entry resource

943df18

octokitbot added this to 🧰 Octokit Active May 2, 2025

github-project-automation bot moved this to 🆕 Triage in 🧰 Octokit Active May 2, 2025

nickfloyd moved this from 🆕 Triage to 👀 In review in 🧰 Octokit Active Jun 3, 2025

Merge branch 'main' into feat/enterprise-ip-allow-list

eb02154

ErikElkins changed the title ~~Adding github_enterprise_ip_allow_list_entry resource~~ feat: Adding github_enterprise_ip_allow_list_entry resource Oct 25, 2025

deiga mentioned this pull request Nov 4, 2025

[FEAT]: Additional Enterprise Settings support #2851

Open

1 task

Merge branch 'main' into feat/enterprise-ip-allow-list

935e689

github-actions bot added the Type: Feature New feature or request label Jan 13, 2026

nickfloyd previously approved these changes Jan 13, 2026

View reviewed changes

nickfloyd added this to the v6.10.0 Release milestone Jan 13, 2026

stevehipwell requested changes Jan 13, 2026

View reviewed changes

github-project-automation bot moved this from 👀 In review to 🏗 In progress in 🧰 Octokit Active Jan 13, 2026

stevehipwell modified the milestones: v6.10.0 Release, v6.11.0 Release Jan 13, 2026

gateixeira mentioned this pull request Jan 13, 2026

feat: Adding github_enterprise_ip_allow_list_entry resource #3078

Closed

4 tasks

Update github/resource_github_enterprise_ip_allow_list_entry.go

83a0055

Co-authored-by: Steve Hipwell <[email protected]>

ErikElkins dismissed nickfloyd’s stale review via 83a0055 January 13, 2026 20:29

Update github/resource_github_enterprise_ip_allow_list_entry.go

c3735ac

Co-authored-by: Steve Hipwell <[email protected]>

Merge branch 'main' into feat/enterprise-ip-allow-list

e1b06a2

gateixeira suggested changes Jan 13, 2026

View reviewed changes

ErikElkins and others added 3 commits January 13, 2026 14:52

Update github/resource_github_enterprise_ip_allow_list_entry.go

833f3f1

Co-authored-by: gateixeira <[email protected]>

Update github/resource_github_enterprise_ip_allow_list_entry_test.go

60c686b

Co-authored-by: gateixeira <[email protected]>

Update github/resource_github_enterprise_ip_allow_list_entry_test.go

f667dcc

Co-authored-by: gateixeira <[email protected]>

gateixeira approved these changes Jan 13, 2026

View reviewed changes

Code review fixes

06b2e43

deiga suggested changes Jan 13, 2026

View reviewed changes


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

		return resourceGithubEnterpriseIpAllowListEntryRead(ctx, d, meta)

	Providers: testAccProviders,
	ProviderFactories: providerFactories,

	resource.TestCheckResourceAttr(resourceName, "is_active", fmt.Sprintf("%t", isActive)),
	resource.TestCheckResourceAttr(resourceName, "is_active", strconv.FormatBool(isActive)),

feat: Adding github_enterprise_ip_allow_list_entry resource #2649

Are you sure you want to change the base?

feat: Adding github_enterprise_ip_allow_list_entry resource #2649

Conversation

ErikElkins commented May 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Before the change?

After the change?

Pull request checklist

Does this introduce a breaking change?

Uh oh!

nickfloyd commented Jan 13, 2026

Uh oh!

stevehipwell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ErikElkins commented Jan 13, 2026

Uh oh!

ErikElkins commented Jan 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ErikElkins commented Jan 13, 2026

Uh oh!

gateixeira left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gateixeira left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deiga commented Jan 16, 2026

Uh oh!

Reviewers

Assignees

Labels

ErikElkins commented May 2, 2025 •

edited

Loading

ErikElkins commented Jan 13, 2026 •

edited

Loading