Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use the appropriate ID when trying to import github_team_members objects #1074

Merged
merged 1 commit into from
Mar 11, 2022

Conversation

brandon-at-wrk
Copy link
Contributor

When importing a github_team_member, the import command passes in the data as the
"Id", not the "team_id", causing the import step to fail when parsing an empty string
as a number. Should fix #608, as I hit the same error.

…s` objects

When importing a `github_team_member`, the import command passes in team ID as the
"Id", not the "team_id" captured in the state for existing resources, and this empty string is
causing the import step to fail when parsing as a number. Should fix integrations#608, as I hit the same error.
@brandon-at-wrk
Copy link
Contributor Author

I tested this by following these notes and running terraform import github_team_members.team_members <ID>. I do not know how to unit test this, since it seems like the current tests ought to have caught it.

@SpencerMalone
Copy link

Good catch! I actually had also just dug in and realized this was the problem. The generic error actually had me all tripped up in thinking it was the schema validation at first. I think that old issue from 2020 is something else, but this definitely solves a very active problem in the recent release.

My only Q (and this comes from a place of ignorance w/ the codebase):

Do we need to repeat the pattern from resource_github_team_membership.go of...

	// We intentionally set these early to allow reconciliation
	// from an upstream bug which emptied team_id in state
	// See https:/integrations/terraform-provider-github/issues/323
	d.Set("team_id", teamIdString)
	d.Set("username", username)

@ https:/integrations/terraform-provider-github/blob/main/github/resource_github_team_membership.go#L85-L99 ?

@brandon-at-wrk
Copy link
Contributor Author

I believe line 85 using the split on d.Id() makes it safe in this case, since it seems like the id in import x <id> is set as the value of d.Id() -- I haven't tested that import case, so I cannot be sure.

@SpencerMalone
Copy link

@kfcampbell any chance this could get bumped up in priority? It effectively makes the newest resource added unusable until this is merged. If there's another individual or group you'd like tagged, lemme know!

@kfcampbell kfcampbell added this to the v4.21.0 milestone Mar 3, 2022
@brandon-at-wrk
Copy link
Contributor Author

For anyone blocked by this, you can take the patch, follow these steps, and run the imports. Since this only breaks imports, the rest of your workflow can remain the same after your import step.

Copy link
Member

@kfcampbell kfcampbell left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution! I'll get this released shortly.

@kfcampbell kfcampbell merged commit 2dcabfb into integrations:main Mar 11, 2022
kfcampbell pushed a commit to kfcampbell/terraform-provider-github that referenced this pull request Jul 26, 2022
…s` objects (integrations#1074)

When importing a `github_team_member`, the import command passes in team ID as the
"Id", not the "team_id" captured in the state for existing resources, and this empty string is
causing the import step to fail when parsing as a number. Should fix integrations#608, as I hit the same error.
kazaker pushed a commit to auto1-oss/terraform-provider-github that referenced this pull request Dec 28, 2022
…s` objects (integrations#1074)

When importing a `github_team_member`, the import command passes in team ID as the
"Id", not the "team_id" captured in the state for existing resources, and this empty string is
causing the import step to fail when parsing as a number. Should fix integrations#608, as I hit the same error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error: Unexpected ID format (""), expected numerical ID. strconv.ParseInt: parsing "": invalid syntax
3 participants