terraform test: allow computed/mocked values override during planning#36227
terraform test: allow computed/mocked values override during planning#36227
Conversation
6bfae8e to
aea2a5a
Compare
23ce4ef to
5031c9d
Compare
d724364 to
3cdb84a
Compare
liamcervante
left a comment
There was a problem hiding this comment.
I think the implementation looks good, just not super happy with the chosen name. I'd be interested to know your thoughts!
internal/configs/mock_provider.go
Outdated
| // When this attribute is set to true, the values specified in the override | ||
| // block will be used for computed attributes even when planning. Otherwise, | ||
| // the computed values will be set to unknown, just like in a real plan. | ||
| overrideComputed = "override_computed" |
There was a problem hiding this comment.
We have the command attribute available in the run blocks: https://developer.hashicorp.com/terraform/language/tests#run-blocks
I wonder if like compute_command that maps to a keyword of either plan or apply might be more clear about what is happening here? We can make it default to apply.
I think that makes it a bit more clear about what the attribute is actually doing. What do you think?
There was a problem hiding this comment.
Thanks for the suggestion. I contemplated quite a bit on the right name to communicate what's going on here, and you are right that override_computed seems a bit unclear. However, I have similar thoughts about compute_command, especially the compute part. That may be confusing. What about override_command/override_target/override_target_command = plan/apply? From a user POV, they have defined the override, and want it to be applied. That it is a computed attribute is more subtle.
There was a problem hiding this comment.
I like override_command, I think that could work. Another idea I had was override_during - as an attribute it doesn't make much sense on it's own, but when coupled with apply or plan then it brings a lot of context eg. override_during = plan.
But, override_command is also good I think 👍
694d10b to
dd3f791
Compare
2be1048 to
0cdeaa9
Compare
liamcervante
left a comment
There was a problem hiding this comment.
Sorry, I just noticed a couple of places where you have pointers and I don't think you need them. This doesn't affect the behaviour of anything though, but I think makes every thing look a bit neater.
Also, since we just merged the changes to the CHANGELOG process you should add a CHANGELOG entry in this PR now.
| // of a real plan. This attribute indicates that the computed values | ||
| // should be overridden with the values specified in the override block, | ||
| // even when planning. | ||
| useForPlan *bool |
There was a problem hiding this comment.
sorry, I didn't notice this before - I don't think this needs to be a pointer. You can just make it a normal bool, and public so callers can read it directly.
We don't need to distinguish between "not set" and explicitly set to false right?
| return provider, diags | ||
| } | ||
|
|
||
| func extractOverrideDuring(content *hcl.BodyContent) (*string, hcl.Diagnostics) { |
There was a problem hiding this comment.
I think you could also make this return a string without a pointer -> just return "apply" if the attribute isn't set.
There was a problem hiding this comment.
In this section internal/configs/mock_provider.go#L317-L325, the field needs to be a pointer. This is necessary because, if the override_* block does not explicitly set a value for override_during, the global value should be applied. However, if the override_* block does set a value, it should take precedence over the global value. Using a pointer allows us to differentiate between an explicitly set value and a zero value.
0cdeaa9 to
d29ae92
Compare
| return provider, diags | ||
| } | ||
|
|
||
| func extractOverrideDuring(content *hcl.BodyContent) (*string, hcl.Diagnostics) { |
|
There's a lot of commits here, so I'd make sure to squash and merge this - you might do that be default already but just wanted to make sure. |
|
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. |
Fixes #35851
This PR enables users to use mocked or overridden values when running a plan command. The attribute
override_duringcan be set to plan or apply. This attribute can be set in the blocks foroverride_*, andmock_provider.Target Release
1.11.0
Draft CHANGELOG entry
NEW FEATURES | UPGRADE NOTES | ENHANCEMENTS | BUG FIXES | EXPERIMENTS