terraform-plugin-sdk
terraform-plugin-sdk copied to clipboard
Show warnings when fixing names via StateFunc?
This is related to https://github.com/hashicorp/terraform/pull/4020 but also many other PRs which have been mimicking the AWS API behaviour in similar ways.
I understand the convenience for the user having existing resources and I understand that is the motivation for @stack72 and others to solve these issues that way. However I think that Terraform shouldn't be so silent about these fixes.
I think we should be more transparent and at least show warnings if the converted names differ from the original ones.
I'd personally like to use warnings in a way that it would allow us to turn on strict validation in the future (after a few versions, when we're sure enough users have been warned), but I understand the convenience for existing users has probably priority and that others may not like this (correct me if I'm wrong).
@radeksimko I really like this idea - we should definitely be notifying the user that we are changing their variables so that they work with the APIs we are using
I'm not sure I entirely agree with you. For an API that is case-insensitive, all of the different-cased versions of a value are semantically equivalent, and I don't really see the point in drawing attention to byte-wise diffs that don't represent a difference in meaning.
I would agree that we should never cross the line into making a change that results in a difference of meaning/behavior, but as long as we're just mimicking the same normalization that the underlying API was going to do I see this as a benefit (improved UX) rather than a drawback.
Have you encountered a real-world drawback to having these things silently normalized, from an end-user perspective?
(I know it has a very real developer drawback in that we have to keep adding all of these silly StateFunc
s everywhere, but I think we've lowercased enough of these now that we could just have a reusable "make this lowercase" function that just wraps strings.ToLower
to make its argument be interface{}
, so we can write StateFunc: something.ToLowerCase
instead of the same wrapper function each time.)
@radeksimko / @apparentlymart was any discussion given to this outside of this issue? I feel this has sort of dropped off the face of the planet atm :)
Easiest to accomplish this when we support diagnostics in protocol 5
This may also be related to https://github.com/hashicorp/terraform-plugin-sdk/issues/19
#74 is the tracking issue for diagnostics.