feat: add prefix filter to team sync groups data source by laughedelic · Pull Request #3248 · integrations/terraform-provider-github

laughedelic · 2026-03-02T03:24:12Z

Resolves [FEAT]: Support filter parameter on github_organization_team_sync_groups data source #1809

Before the change?

Currently, the github_organization_team_sync_groups data source fetches all groups (in fixed page sizes of 100). This causes performance issues if a large number of groups are synced into GitHub (we have about 3000 groups being pushed to GitHub, so the refresh of github_organization_team_sync_groups takes 20+ seconds).

After the change?

Now it's possible to filter groups by prefix:

data "github_organization_team_sync_groups" "filtered" {
  q = "myprefix_"
}

Pull request checklist

Schema migrations have been created if needed (example)
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?

No

I followed the API and the SDK in naming it q, but I'm open to something more descriptive, e.g. prefix or prefix_filter

github-actions · 2026-03-02T03:24:23Z

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

deiga

Thanks for the PR. I left some comments that need addressing

github/data_source_github_organization_team_sync_groups.go

website/docs/d/organization_team_sync_groups.html.markdown

github/data_source_github_organization_team_sync_groups.go

deiga

Sorry, the ID will need to change and thus we need a schema migration

deiga · 2026-03-04T22:47:25Z

github/data_source_github_organization_team_sync_groups.go

+	if options.Query != "" {
+		d.SetId(fmt.Sprintf("%s/github-org-team-sync-groups/%s", orgName, options.Query))
+	} else {
+		d.SetId(fmt.Sprintf("%s/github-org-team-sync-groups", orgName))
+	}


Okay, looking at this makes my skin crawl a bit 😬

While we're at it, let's change to buildID to conform to our standards :)

Suggested change

if options.Query != "" {

d.SetId(fmt.Sprintf("%s/github-org-team-sync-groups/%s", orgName, options.Query))

} else {

d.SetId(fmt.Sprintf("%s/github-org-team-sync-groups", orgName))

}

id, err := buildID(orgName, "team-sync-groups", options.Query)

if err != nil {

return diag.FromErr(err)

}

d.SetId(id)

With this change we'll need you to make a StateUpgrader to migrate the old IDs to new IDs. Look at any of the existing ones to see how to go about it :)

Oh, I see what you mean 😅 I have to admit, I'm moving in the dark here, trying to make the minimal necessary changes, and I don't have a good overview of the whole codebase (yet). Thanks for the guidance.

I didn't know about buildID. But it uses : as a separator. I wonder why the original ID used / 🤔

If the query is empty, the id is going to look like orgname:team-sync-groups: with a trailing :. Isn't this odd? Should it be orgname:team-sync-groups instead?

So how about

Suggested change

if options.Query != "" {

d.SetId(fmt.Sprintf("%s/github-org-team-sync-groups/%s", orgName, options.Query))

} else {

d.SetId(fmt.Sprintf("%s/github-org-team-sync-groups", orgName))

}

idParts := []string{orgName, "team-sync-groups"}

if options.Query != "" {

idParts = append(idParts, options.Query)

}

id, err := buildID(idParts...)

if err != nil {

return diag.FromErr(err)

}

d.SetId(id)

?

The ID change is done in 0064745 and the migration added in a6e33d8. I hope I'm getting it right, but let me know if I still misunderstood.

Thanks. I don't think that conditional IDs are great, since that requires then custom logic when parsing them (if necessary). We already have other examples of IDs where they just end with : if the last part is empty

Ok. Back to 3-part IDs: e21fe5d

deiga · 2026-03-04T22:48:03Z

github/data_source_github_organization_team_sync_groups_test.go

 			},
+			{
+				Config: `data "github_organization_team_sync_groups" "test" { prefix_filter = "nonexistent_prefix_" }`,
+				Check: resource.ComposeTestCheckFunc(


Please use ConfigStateChecks instead of Check

Done in ad18a98 + 49d1e01

deiga · 2026-03-05T08:53:07Z

github/data_source_github_organization_team_sync_groups.go

+		SchemaVersion: 1,
+		StateUpgraders: []schema.StateUpgrader{
+			{
+				Type:    dataSourceGithubOrganizationTeamSyncGroupsV0().CoreConfigSchema().ImpliedType(),
+				Upgrade: dataSourceGithubOrganizationTeamSyncGroupsStateUpgradeV0,
+				Version: 0,
+			},
+		},


🤦 Silly me. Do data sources even need Schema Migrations? @stevehipwell

feat: add query filter to team sync groups data source

4a81574

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

deiga suggested changes Mar 2, 2026

View reviewed changes

laughedelic added 4 commits March 2, 2026 23:50

Rename q to prefix_filter

b477069

Replace prefix_filter Default with d.GetOk

50ddc7e

Change d.SetId only when the prefix_filter is set

d276ec2

Update docs to include both examples

ad7206e

laughedelic requested a review from deiga March 2, 2026 22:54

laughedelic changed the title ~~feat: add query filter to team sync groups data source~~ feat: add prefix filter to team sync groups data source Mar 4, 2026

deiga suggested changes Mar 4, 2026

View reviewed changes

laughedelic added 3 commits March 5, 2026 02:19

Change ID generation to use buildID

0064745

Use ConfigStateChecks in the tests

ad18a98

Add state migration for the new ID format

a6e33d8

laughedelic requested a review from deiga March 5, 2026 01:37

Use tflog instead

49d1e01

deiga reviewed Mar 5, 2026

View reviewed changes

Go back to 3-part IDs

e21fe5d

-	if options.Query != "" {
-		d.SetId(fmt.Sprintf("%s/github-org-team-sync-groups/%s", orgName, options.Query))
-	} else {
-		d.SetId(fmt.Sprintf("%s/github-org-team-sync-groups", orgName))
-	}
+	idParts := []string{orgName, "team-sync-groups"}
+	if options.Query != "" {
+		idParts = append(idParts, options.Query)
+	}
+	id, err := buildID(idParts...)
+	if err != nil {
+		return diag.FromErr(err)
+	}
+	d.SetId(id)

Conversation

laughedelic commented Mar 2, 2026 • 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!

github-actions bot commented Mar 2, 2026

Uh oh!

deiga left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

deiga 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!

laughedelic Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

laughedelic commented Mar 2, 2026 •

edited

Loading

laughedelic Mar 5, 2026 •

edited

Loading