-
Notifications
You must be signed in to change notification settings - Fork 244
Add ignore_server_additions attribute to prevent drift from API defaults #336
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
Conversation
When an API returns additional fields (like defaults, metadata, or timestamps) that weren't in the original configuration, the provider was detecting these as drift and updating state. This caused a perpetual diff between config and state, leading to unnecessary resource recreation on every apply. This commit adds a new `ignore_server_additions` attribute that, when set to true, ignores fields added by the server that weren't in the user's original configuration. This differs from `ignore_all_server_changes` which ignores ALL changes including modifications to configured fields. Fixes Mastercard#319
|
Nice - this is a great addition, @jor2 This is a good time to discuss the default behavior and document it since I plan to cut a v3 of this provider along with the migration to TFSDKv2 |
|
Hi there! thanks for the consideration @DRuggeri ! I suspect most cases of "server added fields" are for metadata like timestamps - and this would be useful there - but I also worry there are valid cases where this isn't really what you'd want. For example consider a hypothetical API resource with an optional TTL field and omitting this field might have semantics meaning "no expiration". Say you intentionally omit the TTL when you create the resource, but later someone/something sets that field out of band, like in a dashboard UI or there's a bad script or something. I personally would expect that the next time you go to terraform apply, the provider would attempt to revert that change and remove the field. For that reason, my opinion is that it probably shouldn't be default, but I do think it's a good feature! Especially for @jor2's usecase where a PATCH request might return waaay more fields than you sent in. Also @jor2 - this doesn't detract in any way from your hard work or your PR (I think it's a great improvement!) - but I don't see that you've mentioned the |
|
Hi @michaelPotter thanks for the feedback! yeah, the main issue was I would have to go examine the response fields every time I want to use the restapi provider and add all those fields I don't care about to |
|
Awesome, guys, thanks for the dialog. It makes sense to leave it opt-in... and also good to have the option that tracks just your data elements ignoring anything the server tacks on. I've merged this code into the new Terraform Plugin Framework branch and will release it soon! |
|
@DRuggeri looks like this made it into the v3 release candidate. Just wondering if there is any rough ETA yet for v3 GA release? Are we talking weeks or months? Thanks in advance. |
|
Spot on, @ocofaigh - it's in the v3 RC. I planned to let the RC sit and stew for a few weeks before releasing it. We've had some issues crop up, which @jor2 has already helped address. I'd like to get #347 addressed, cut a new RC, and assuming no new reports filter in, release that in about 2 weeks. |
Summary
When REST APIs return additional fields beyond what was configured (such as default values, timestamps, metadata, or computed properties), the provider detects these as drift. This causes:
terraform planterraform applyThis PR adds a new
ignore_server_additionsattribute that, when set totrue, ignores fields added by the server that weren't present in the user's original configuration.How it differs from
ignore_all_server_changesignore_all_server_changesignore_server_additionsThe new attribute is more targeted - it allows you to still detect when the server modifies a field you explicitly configured, while ignoring additional fields the server adds.
Example usage
Changes
ignore_server_additionsattribute to the resource schemagetDeltafunction to accept the new parameterTest plan
Fixes #319