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 incorrect case of tagOwners and autoApprovers #46

Merged
merged 1 commit into from
Aug 31, 2023

Conversation

mangini
Copy link
Contributor

@mangini mangini commented Apr 21, 2023

tagOwners and autoApprovers are incorrectly serializing all lowercase, which is causing an inconsistency on the terraform provider: the terraform plan always show a diff between what is previously deployed (camelcase 'tagOwners' and 'autoApprovers') and the new plan (all lowercase 'tagowners' and 'autoapprovers'), regardless of whether those attributes were changed or not.

Copy link
Contributor

@knyar knyar 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!

I've addressed spurious diffs in Terraform plan in tailscale/terraform-provider-tailscale#274, but we should use canonical field names in the client anyway.

tagOwners and autoApprovers are incorrectly serializing all lowercase, which is causing an inconsistency on the terraform provider: the terraform plan always show a diff between what is previously deployed (camelcase 'tagOwners' and 'autoApprovers') and the new plan (all lowercase 'tagowners' and 'autoapprovers'), regardless of whether those attributes were changed or not.
@knyar knyar merged commit eedc358 into tailscale:main Aug 31, 2023
1 check passed
@erezrokah
Copy link

Hi 👋 Thank you for this fix 🚀 I think it should be considered as a breaking change as the output of serializing the struct to JSON is different now. Not sure there's anything to be done but maybe something to consider in future changes

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.

3 participants