Provider - Add the missing suppress.CaseDifference for case-insensitive string properties

[magodo]

For string properties whose ValidateFunc is set to validation.StringInSlice with case-insensitive flag set to true, ensure it comes with the DiffSuppressFunc: suppress.CaseDifference or the alike.

With the help of gogrep (gogrep -x '$_: {$*_, Type: pluginsdk.TypeString, $*_}' -g 'validation.StringInSlice($_, true)' -v 'suppress.CaseDifference' ./...), I've complemented all the properties that are not met.

PS: I assume either StateFunc: state.IgnoreCase or DiffSuppressFunc: uppress.CaseDifference does the trick. So I didn't add up for the StateFunc.

Fixes #15173

[tombuildsstuff]

hey @magodo

Thanks for this PR.

Whilst this is one approach to solve this issue, unfortunately this is actually a slightly different bug insofar as these fields should be case-sensitive instead of case-insensitive (and have incorrectly been marked as true instead of false here). As such would you mind invert this PR, by removing the DiffSuppressFunc and making these case-sensitive instead so that we can fix this and flag these at validation time, which should fix #15173?

Thanks!

[magodo]

@tombuildsstuff I'm not sure whether it is totally fine to make all of them to be case-sensitiv. E.g. some of them are marked as case-insensitive for walking around service issues? Also, it might breaks some users' existing configurations?

[tombuildsstuff]

@magodo Identity is being reworked to make it consistent (see #15187) - so those will be fixed as a part of that, if there's others we should look into normalizing those too fwiw?

[magodo]

@tombuildsstuff Thank you for mentioning the identity issue, that makes a lot of sense. While for the other properties, we are still not sure whether it is safe to make them case sensitive for the same reason. This PR is to complement the in correct case-insensitive usages, about whether to make some of them to be case-sensitive, how about we open another issue/PR for that?

[katbyte]

@magodo - all enums should be case sensitive and only when its absolutely required to work around a bug should we be ignoring case, and there should be a linked issue documenting it somewhere on the rest specs. also in those cases we can just change the casing on read to match what the enums are if the service refuses to respect the enum casing while still enforcing the correct case in terraform.

[magodo]

@katbyte I absolutely agree that we should always allow users to specify enum in case sensitive manner, unless there is a necessary need to work around something. While this PR is not for this purpose, it just make sure thoese already marked as case insensitive (no matter on purpose or not) enums are really acting as case insensitively, i.e. apply with uppercase previously and then change it to lowercase won't cause a diff. This is accomplished by adding the suppression.

In order to identify which case insensitve enums are not necessary is another topic, from my opinion, it should be done case by case, with some decent test to verify. Also, doing that change might break some existing configs users are using. So I'd suggest to make it in another PR(s). WDYT?

[katbyte]

At this point so close to 3.0 i suggest we do the opposite, remove the case insensitive bool on validation with a 3.0 flag and make the move towards can sensitivity. this is a large pr to fix a single issue with storagev2

[magodo]

@katbyte I've refined the PR to modify all the places listed by gogrep -x 'validation.StringInSlice($_, true)' ./internal/... with following changes:

• If the enums has non-alphabetic only (e.g. number only), change the true to false in the validation.StringInSlice()
• Otherwise, change the true to !features.ThreePointOh()
• Change the DiffSuppressFunc to suppress.CaseDifferenceV2Only, which only take effect if !features.ThreePointOh()
• Remove the StateFunc: state.IgnoreCase if exists. As it doesn't contribute a lot when we want case insensitive, and will bring problems if we want case sensitive in v3.0, as it will always lowercase the string when persists in state.

As discussed offline, some of the enums listed in the validation.StringInSlice are hardcoded - rather than using SDK const. For this case, we regard those hardcoded values are of correct casing.

Also, we can expect some of these schemas will be reverted back to be case insensitive in case the backend API has case problems. In that case, we will want a PR to do the revert (either via resetting them to be case insensitive, or do the casing conversion during flatten), together with a Swagger issue to track the issue.

[katbyte]

Thanks @magodo - now just need to fix up the merge conflicts and runt ests to check case

[magodo]

@katbyte I've resolved the conflicts now. Now the output of gogrep -x 'validation.StringInSlice($_, !features.ThreePointOh())' -p 2 -v 'DiffSuppressFunc: suppress.CaseDifferenceV2Only' ./internal/... only shows 7 locations, where each of them is verified.

[katbyte]

Thanks @magodo - LGTM 🏗️

[github-actions]

This functionality has been released in v2.97.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.