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

Don't override imported connection_client id #562

Merged
merged 2 commits into from
May 2, 2023
Merged

Don't override imported connection_client id #562

merged 2 commits into from
May 2, 2023

Conversation

WesleyKlop
Copy link
Contributor

🔧 Changes

This PR changes the importConnectionClient function which handles the importing of auth0_connection_client resources. When repeatedly importing this resource (like is possible when using a tool like Pulumi) the imported connection client has a new id every time even though it has not changed. This causes the IaC tool to want to recreate the resource, which breaks your environment. Using just the combination of connection_id:client_id is unique enough and should be fine to use the identifier for this resource, since I don't think there is a situation where duplicates would exist. (and if they do, that sounds like a misconfiguration)

The only change is that the internal id is now the same each time the same resource is imported.

🔬 Testing

This can be tested by importing connection clients!

📝 Checklist

  • All new/changed/fixed functionality is covered by tests (or N/A)
  • I have added documentation for all new/changed functionality (or N/A)

@WesleyKlop WesleyKlop requested a review from a team as a code owner April 25, 2023 14:46
Copy link
Contributor

@sergiught sergiught left a comment

Choose a reason for hiding this comment

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

Appreciate the contribution @WesleyKlop! We weren't aware of pulumi internal mechanics, however we have other such similar import mechanics across other resources like auth0_organization_connection and auth0_organization_member. If we're going this path would you mind altering the import for those resources as well? So we have consistency? 🙏🏻

@WesleyKlop
Copy link
Contributor Author

Yes ofcourse, will do!

This id is not reproducible and since only 1 connection + client binding
exists the imported id should be unique enough.

This would also not cause repeated imports to differ
Some resources are not identified in the API so we should always
overwrite their id. This is now clarified.

In other cases the SetId call is removed when its key is composite.

When Terraform has ownership (creates) a resource, a comment was not
added.
@WesleyKlop
Copy link
Contributor Author

Appreciate the contribution @WesleyKlop! We weren't aware of pulumi internal mechanics, however we have other such similar import mechanics across other resources like auth0_organization_connection and auth0_organization_member. If we're going this path would you mind altering the import for those resources as well? So we have consistency? 🙏🏻

@sergiught I changed it! :)

Copy link
Contributor

@sergiught sergiught left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the contribution @WesleyKlop 💯 Great work and appreciate going the extra mile on this one 💪🏻

@sergiught sergiught merged commit ce85364 into auth0:main May 2, 2023
@WesleyKlop
Copy link
Contributor Author

Thank you very much! :D

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.

2 participants