[BUG] Enable importing `github_emu_group_mapping` for Group with multiple teams by deiga · Pull Request #3054 · integrations/terraform-provider-github

deiga · 2026-01-07T14:22:12Z

Superseded #3043

Resolves #3030

Before the change?

No tests for github_emu_group_mapping
Import set wrong ID
Not able to import Groups with multiple teams mapped
Using legacy CRUD provider functions

After the change?

Adds tests for github_emu_group_mapping
Fixes import logic
Enabled importing Group with multiple team mappings
Refactor to use Context-aware functions

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?

Please see our docs on breaking changes to help!

Yes
No

github-actions · 2026-01-07T14:22:24Z

👋 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! 🚀

stevehipwell

As we as the inline comments, we need to be careful when using tflog that it can only be used with contexts injected by TF (e.g. CreateContext).

github/resource_github_emu_group_mapping.go

github/util_import.go

github/resource_github_emu_group_mapping.go

stevehipwell

I've just realized what you're doing here, which is what I've been working on with the other team resources. I think you need to make the following changes (none of which I'd consider breaking).

Resource ID format to <team-id>:<team-slug>:<group-id>
- State migration to update the ID
Import to accept <team-slug>:<group-id>
- Lookup team ID and update resource ID to correct format
Make group ID force new
- Otherwise changing the group would leave orphaned links
Add custom diff function
- Force new if team slug points at new ID
Separate create and update functions

The top 3 steps would be good enough to fix the bug and stop records being orphaned (unless the team slug was changed to point at a completely different team).

deiga · 2026-01-15T13:11:23Z

I've just realized what you're doing here, which is what I've been working on with the other team resources. I think you need to make the following changes (none of which I'd consider breaking).

Resource ID format to <team-id>:<team-slug>:<group-id>

State migration to update the ID

Import to accept <team-slug>:<group-id>

Lookup team ID and update resource ID to correct format

Make group ID force new

Otherwise changing the group would leave orphaned links

Add custom diff function

Force new if team slug points at new ID

Separate create and update functions

The top 3 steps would be good enough to fix the bug and stop records being orphaned (unless the team slug was changed to point at a completely different team).

@stevehipwell These definitely sound like worthwhile changes, but I wonder if we need to do them before merging this?
I'd rather get this PR merged now, to enable these imports and then create a follow-up PR to address those changes.

stevehipwell · 2026-01-15T13:24:58Z

@deiga you need the first 2 steps above to fix the bug as otherwise you'll have duplicate IDs when teams have multiple groups attached. The 3rd step costs nothing and fixes another set of current defects. I think maybe we move this PR and the issue to the v6.11.0 milestone to give us a bit more time?

deiga · 2026-01-15T18:08:59Z

@deiga you need the first 2 steps above to fix the bug as otherwise you'll have duplicate IDs when teams have multiple groups attached. The 3rd step costs nothing and fixes another set of current defects. I think maybe we move this PR and the issue to the v6.11.0 milestone to give us a bit more time?

@stevehipwell The ID uses team slug as the ID, so duplicate IDs shouldn't be an issue as each team can have exactly 1 mapping.

Like I said, I'll happily address those in a follow-up as I don't see any of them necessary for getting this merged

stevehipwell

Ok, at a bare minimum can we please only support two part import IDs. That way import doesn't have to lookup the team and we don't get any drift. It can save the original ID pattern for now and we can fix up the rest of my points later

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Which was found by our new tests! Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

stevehipwell

Sorry @deiga I don't see how this resource works at all? The read just checks that the group has a team linked, not that the relationship is the one being managed by TF. It looks like the SDK is missing the required endpoint.

Let's push this PR into the next release. I'll open a PR to get the endpoint supported in the SDK.

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

stevehipwell

Thanks @deiga. This PR fixes the import bug and the other defects are pre existing so I think we should get it merged and fix the bugs in a separate PR.

github-actions bot added the Type: Bug Something isn't working as documented label Jan 7, 2026

deiga changed the title ~~Fix emu group mapping import multiple teams 3030~~ [BUG] Enable importing github_emu_group_mapping for Group with multiple teams Jan 7, 2026

deiga force-pushed the fix-emu-group-mapping-import-multiple-teams-3030 branch from beb7f4d to abb3de3 Compare January 7, 2026 21:24

stevehipwell requested changes Jan 8, 2026

View reviewed changes

stevehipwell added this to the v6.10.0 Release milestone Jan 8, 2026

deiga force-pushed the fix-emu-group-mapping-import-multiple-teams-3030 branch from abb3de3 to a523161 Compare January 8, 2026 21:44

deiga requested a review from stevehipwell January 8, 2026 21:44

deiga force-pushed the fix-emu-group-mapping-import-multiple-teams-3030 branch from a523161 to e8cceb1 Compare January 9, 2026 22:50

stevehipwell requested changes Jan 9, 2026

View reviewed changes

github/resource_github_emu_group_mapping.go Outdated Show resolved Hide resolved

github/resource_github_emu_group_mapping.go Outdated Show resolved Hide resolved

deiga requested a review from stevehipwell January 10, 2026 21:32

stevehipwell requested changes Jan 13, 2026

View reviewed changes

github/resource_github_emu_group_mapping.go Outdated Show resolved Hide resolved

github/resource_github_emu_group_mapping.go Outdated Show resolved Hide resolved

github/util_import.go Outdated Show resolved Hide resolved

deiga requested a review from stevehipwell January 13, 2026 20:10

stevehipwell requested changes Jan 14, 2026

View reviewed changes

github/resource_github_emu_group_mapping.go Outdated Show resolved Hide resolved

deiga force-pushed the fix-emu-group-mapping-import-multiple-teams-3030 branch from b572afb to 5302481 Compare January 14, 2026 20:25

deiga requested a review from stevehipwell January 14, 2026 21:16

stevehipwell requested changes Jan 15, 2026

View reviewed changes

deiga requested a review from stevehipwell January 15, 2026 18:09

stevehipwell requested changes Jan 15, 2026

View reviewed changes

deiga added 8 commits January 16, 2026 15:26

Add tests for github_emu_group_mapping

f67624f

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Fix broken import of github_emu_group_mapping`

15b25f8

Which was found by our new tests! Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Add failing test to verify problem in integrations#3030

4c2eff8

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Update import function to support a single ID and a two part ID

4a535aa

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Refactor to use Context-aware functions

d993892

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Add detailed logging for INFO, DEBUG and TRACE levels

d9d35cf

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Extract import functions into util_import.go

38cda7e

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Address review comments

0390b35

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

deiga added 6 commits January 16, 2026 15:26

Merge Create and Update and remove call to Read

daac2fb

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Address review comments

4c34bfc

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Use SDK accessors instead of pointers

786c360

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Add getTeamSlugContext func

d5d66f1

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Replace TeamName with fetching TeamSlug from API

71fae66

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Only support import with two part ID

c853582

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

deiga force-pushed the fix-emu-group-mapping-import-multiple-teams-3030 branch from 3e24afb to c853582 Compare January 16, 2026 13:30

stevehipwell requested changes Jan 16, 2026

View reviewed changes

Add warning about Read misbehaving

01c6ae1

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

stevehipwell approved these changes Jan 16, 2026

View reviewed changes

nickfloyd approved these changes Jan 16, 2026

View reviewed changes

nickfloyd merged commit bfb6a49 into integrations:main Jan 16, 2026
8 checks passed

Conversation

deiga commented Jan 7, 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 Jan 7, 2026

Uh oh!

stevehipwell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stevehipwell left a comment

Choose a reason for hiding this comment

Uh oh!

deiga commented Jan 15, 2026

Uh oh!

stevehipwell commented Jan 15, 2026

Uh oh!

deiga commented Jan 15, 2026

Uh oh!

stevehipwell left a comment

Choose a reason for hiding this comment

Uh oh!

stevehipwell left a comment

Choose a reason for hiding this comment

Uh oh!

stevehipwell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

deiga commented Jan 7, 2026 •

edited

Loading