Skip to content
15 changes: 14 additions & 1 deletion github/data_source_github_organization_team_sync_groups.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ func dataSourceGithubOrganizationTeamSyncGroups() *schema.Resource {
ReadContext: dataSourceGithubOrganizationTeamSyncGroupsRead,

Schema: map[string]*schema.Schema{
"prefix_filter": {
Type: schema.TypeString,
Optional: true,
Description: "Filters the results to return only those that begin with the specified value.",
},
"groups": {
Type: schema.TypeList,
Computed: true,
Expand Down Expand Up @@ -48,6 +53,10 @@ func dataSourceGithubOrganizationTeamSyncGroupsRead(ctx context.Context, d *sche
},
}

if v, ok := d.GetOk("prefix_filter"); ok {
options.Query = v.(string)
}

groups := make([]any, 0)
for {
idpGroupList, resp, err := client.Teams.ListIDPGroupsInOrganization(ctx, orgName, options)
Expand All @@ -65,7 +74,11 @@ func dataSourceGithubOrganizationTeamSyncGroupsRead(ctx context.Context, d *sche
options.Page = resp.NextPageToken
}

d.SetId(fmt.Sprintf("%s/github-org-team-sync-groups", orgName))
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))
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 / 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Back to 3-part IDs: e21fe5d

if err := d.Set("groups", groups); err != nil {
return diag.Errorf("error setting groups: %v", err)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ func TestAccGithubOrganizationTeamSyncGroupsDataSource_existing(t *testing.T) {
resource.TestCheckResourceAttrSet("data.github_organization_team_sync_groups.test", "groups.0.group_name"),
),
},
{
Config: `data "github_organization_team_sync_groups" "test" { prefix_filter = "nonexistent_prefix_" }`,
Check: resource.ComposeTestCheckFunc(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use ConfigStateChecks instead of Check

Copy link
Copy Markdown
Contributor Author

@laughedelic laughedelic Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in ad18a98 + 49d1e01

resource.TestCheckResourceAttr("data.github_organization_team_sync_groups.test", "prefix_filter", "nonexistent_prefix_"),
resource.TestCheckResourceAttr("data.github_organization_team_sync_groups.test", "groups.#", "0"),
),
},
},
})
}
12 changes: 11 additions & 1 deletion website/docs/d/organization_team_sync_groups.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,19 @@ Use this data source to retrieve the identity provider (IdP) groups for an organ
## Example Usage

```hcl
data "github_organization_team_sync_groups" "test" {}
data "github_organization_team_sync_groups" "all" {}
```

```hcl
data "github_organization_team_sync_groups" "filtered" {
prefix_filter = "myprefix_"
}
```

## Argument Reference

* `prefix_filter` - (Optional) Filters the results to return only those groups whose names begin with this value.

## Attributes Reference

* `groups` - An Array of GitHub Identity Provider Groups. Each `group` block consists of the fields documented below.
Expand Down