811 Commits (a6f76aa0affc940eb9ecb86c72948ca2768874ba)

Author	SHA1	Message	Date
Stuart Auld	7d1f594f54	helper/resource: don't panic if a requested module does not exist	8 years ago
Stuart Auld	df9446f490	helper/resource: allow tests to opt out of temporary file cleanup ... This is rarely needed, but sometimes tests need to create temporary files as part of their operation. This should be used sparingly, since it prevents the pro-active cleanup of the temporary working directory.	8 years ago
Radek Simko	7af1c2b3a4	helper/schema: Prevent crash on removal of computed field in CustomizeDiff	8 years ago
Stefan Schmidt	ee6c8a777a	s/repalce/replace	8 years ago
Gauthier Wallet	1b3f5fcbfb	helper/structure: More cases for NormalizeJsonString tests ... In terraform-providers/terraform-provider-aws#2935, we have been cleaning code duplication by benefiting from the "NormalizeJsonString" present in the "structure" helper. It appears that tests in the AWS provider are covering more use-cases, which are added in this work.	8 years ago
James Bardin	d91dc1a13e	unused variable ... vet can now catch these in closures	8 years ago
James Bardin	4b49a323c3	go fmt ... slight change to go fmt coming in 0.10	8 years ago
James Bardin	0df8da59f7	add FIXMEs ... This new codepath with the getDiff "customzed" return value, along with the associated test need to be removed as soon as we can support unset fields from the config, so we don't continue to carry this broken behavior forward any longer than needed.	8 years ago
Chris Marchesi	cb5ce1d35e	helper/schema: Extend diffChange and bubble up customized values ... This extends the internal diffChange method so that ResourceDiff's implementation of it can report back whether or not the value came from a customized diff. This is an effort to work to preserve the pre-ResourceDiff behaviour that ignores the diff for computed keys when the old value was populated but the new value wasn't - this behaviour is actually being depended on by users that are using it to exploit using zero values in modules. This should allow both scenarios to co-exist by shifting the NewComputed exemption over to exempting values that come from diff customization.	8 years ago
James Bardin	643ef4334f	revert the change that broke the test case ... This reverts one of the changes from 6a4f7b0, which broke empty strings being seen as unset for computed values. This breaks a number of other tests, and is only an intermediate change for evaluating other solutions.	8 years ago
James Bardin	7a8a443994	add failing test case ... This case should be expected to fail with the current diff algorithm, but the existing behavior was widely relied upon so we need to roll this back until there is a representable nil value.	8 years ago
Martin Atkins	c647b22d97	helper/customdiff: Helper functions for CustomizeDiff ... The CustomizeDiff functionality in helper/schema is powerful, but directly writing single CustomizeDiff functions can obscure the intent when a number of different, orthogonal diff-customization behaviors are required. This new library provides some building blocks that aim to allow a more declarative form of CustomizeDiff implementation, by composing a number of smaller operations. For example: &schema.Resource{ // ... CustomizeDiff: customdiff.All( customdiff.ValidateChange("size", func (old, new, meta interface{}) error { // If we are increasing "size" then the new value must be // a multiple of the old value. if new.(int) <= old.(int) { return nil } if (new.(int) % old.(int)) != 0 { return fmt.Errorf("new size value must be an integer multiple of old value %d", old.(int)) } return nil }), customdiff.ForceNewIfChange("size", func (old, new, meta interface{}) bool { // "size" can only increase in-place, so we must create a new resource // if it is decreased. return new.(int) < old.(int) }), customdiff.ComputedIf("version_id", func (d *schema.ResourceDiff, meta interface{}) bool { // Any change to "content" causes a new "version_id" to be allocated. return d.HasChange("content") }), ), } The goal is to allow the various separate operations to be quickly seen and to ensure that each of them runs independently of the others. These functions all create closures on the call parameters, so the result is still just a normal CustomizeDiffFunc and so the helpers in this package can be combined with hand-written functions as needed. As we get more experience writing CustomizeDiff functions we may wish to expand the repertoire of functions here in future; this initial set attempts to cover some common cases we've seen so far. We may also investigate some helper functions that are entirely declarative and so don't take callback functions at all, but want to learn what the relevant use-cases are before going in too deep here.	8 years ago
Stuart Auld	1d1984771b	helper/resource: test check helpers for resources in non-root modules	8 years ago
Chris Marchesi	85119304b3	Merge pull request #16895 from hashicorp/b-resource-test-expecterror ... helper/resource: Fail tests with no error that have ExpectError set	8 years ago
Chris Marchesi	3f8dad30c9	helper/resource: Fail tests with no error that have ExpectError set ... Looks like while we were checking errors correctly when ExpectError was set, we weren't checking for the *absence* of an error, which is should be checked as well (no error is still not the error we are looking for). Added a few more tests for ExpectError as well.	8 years ago
Rob Campbell	5daeee5f6d	Update various files for new version of "stringer" ... The latest version of stringer now uses strconv instead of fmt.	8 years ago
Chris Marchesi	8e57c4361c	Merge pull request #16774 from hashicorp/f-validation-stringmatch ... helper/validation: Add StringMatch	9 years ago
Chris Marchesi	5da62a44f4	helper/validation: Fix comment on StringMatch ... To match the current implementation.	9 years ago
Martin Atkins	ba0514106a	return tfdiags.Diagnostics from validation methods ... Validation is the best time to return detailed diagnostics to the user since we're much more likely to have source location information, etc than we are in later operations. This change doesn't actually add any detail to the messages yet, but it changes the interface so that we can gradually introduce more detailed diagnostics over time. While here there are some minor adjustments to some of the messages to improve their consistency with terminology we use elsewhere.	9 years ago
Chris Marchesi	e33aa9dc2e	helper/validation: Add StringMatch ... StringMatch returns a validation function that can be used to match a string against a regular expression. This can be used for simple substring validations or more complex validation scenarios. Optionally, an error message can be returned so that the user is returned a better error message other than that their field did not match a regular expression that they might not be able to understand.	9 years ago
Radek Simko	2974d63e75	Merge pull request #16588 from hashicorp/f-panic-on-invalid-rd-set ... helper/schema: Opt-in panic on invalid ResourceData.Set	9 years ago
Nándor István Krácser	e5cc8af7f3	helper/resource: fix ungrammatical doc comment in StateChangeConf	9 years ago
Radek Simko	e93d64b18c	helper/schema: Opt-in panic on invalid ResourceData.Set	9 years ago
Chris Marchesi	3fa11e456b	helper/schema: Clear existing map/set/list contents before overwriting ... There are situations where one may need to write to a set, list, or map more than once per single TF operation (apply/refresh/etc). In these cases, further writes using Set (example: d.Set("some_set", newSet)) currently create unstable results in the set writer (the name of the writer layer that holds the data set by these calls) because old keys are not being cleared out first. This bug is most visible when using sets. Example: First write to set writes elements that have been hashed at 10 and 20, and the second write writes elements that have been hashed at 30 and 40. While the set length has been correctly set at 2, since a set is basically a map (as is the entire map writer) and map results are non-deterministic, reads to this set will now deliver unstable results in a random but predictable fashion as the map results are delivered to the caller non-deterministic - sometimes you may correctly get 30 and 40, but sometimes you may get 10 and 20, or even 10 and 30, etc. This problem propagates to state which is even more damaging as unstable results are set to state where they become part of the permanent data set going forward. The problem also applies to lists and maps. This is probably more of an issue with maps as a map can contain any key/value combination and hence there is no predictable pattern where keys would be overwritten with default or zero values. This is contrary to complex lists, which has this problem as well, but since lists are deterministic and the length of a list properly gets updated during the overwrite, the problem is masked by the fact that a read will only read to the boundary of the list, skipping any bad data that may still be available due past the list boundary. This update clears the child contents of any set, list, or map before beginning a new write to address this issue. Tests are included for all three data types.	9 years ago
Chris Marchesi	0c0ae3ca7c	helper/schema: CustomizeDiff allowed on writable resources only ... This keeps CustomizeDiff from being defined on data sources, where it would be useless. We just catch this in InternalValidate like the rest of the CRUD functions that are not used in data sources.	9 years ago
Chris Marchesi	2c541e8c97	helper/schema: Add behaviour detail to various custom diff comments ... Added some more detailed comments to CustomizeDiff's comments. The new comments detail how CustomizeDiff will be called in the event of different scenarios like creating a new resource, diffing an existing one, diffing an existing resource that has a change that requires a new resource, and destroy/tainted resources. Also added similar detail to ForceNew in ResourceDiff. This should help mitigate any confusion that may come up when using CustomizeDiff, especially in the ForceNew scenario when the second run happens with no state.	9 years ago
Chris Marchesi	f5bdbc0911	helper/schema: Simplify setDiff, remove exported SetDiff mentions ... setDiff does not make use of its new parameter anymore, so it has been removed. Also, as there is no more SetDiff (exported) function, mentions of that have been removed from comments too.	9 years ago
Chris Marchesi	3ac0cdf0c0	helper/schema: Better ResourceDiff schema key validation ... This fixes nil pointer issues that could come up if an invalid key was referenced (ie: not one in the schema). Also ships a helper validation function to streamline things.	9 years ago
Chris Marchesi	529d7e6dae	helper/schema: Review -> CustomizeDiff ... Restoring the naming of this field in the resource back to CustomizeDiff, as this is generally more descriptive of the process that's happening, despite the lengthy name.	9 years ago
Chris Marchesi	09e2109ff8	helper/schema: Resouce.Diff no longer ResourceProvider API compatible ... The old comments said that this interface was API compatible with terraform.ResourceProvider's Diff method - with the addition of passing down meta to it, this is no longer the case. Not too sure if this is really a big deal - schema.Resource never fully implemented terraform.ResourceProvider, from what I can see, and the path from Provdier.Diff to Resource.Diff is still pretty clear. Just wanted to remove an outdated comment.	9 years ago
Chris Marchesi	9625830980	helper/schema: Move computedKeys init to init function ... This simplifies the new value initialization and future-proofs it against more complex initialization functionality.	9 years ago
Chris Marchesi	f0aafe4d67	helper/schema: Restore new value for complex set test	9 years ago
Chris Marchesi	3444549d60	helper/schema: Move computed key reader logic to childAddrOf ... This will make this check safer and will remove the risk of it passing on keys with a similar prefix.	9 years ago
Chris Marchesi	36aa63b338	helper/schema: Guard against out of range on childAddrOf ... This could panic if we sent a parent that was actually longer than the child (should almost never come up, but the guard will make it safe anyway). Also fixed a slice style lint warning in UpdatedKeys.	9 years ago
Chris Marchesi	931b0e1441	helper/schema: Remove exported SetDiff method ... The consensus is that it's a generally better idea to move setting the functionality of old values completely to the refresh/read process, hence it's moot to have this function around anymore. This also means we don't need the old value reader/writer anymore, which simplifies things.	9 years ago
Chris Marchesi	ad98471559	helper/schema: Correct some comments ... Removed some outdated comments for SetNew, and normalized the computed key note for SetNew, SetNewComputed, and SetDiff.	9 years ago
Chris Marchesi	8a7c9a6f02	helper/schema: frist -> first	9 years ago
Chris Marchesi	7d5f9ed6b1	helper/schema: NewComputed values should be nil ... When working on this initially, I think I thought that since NewComputed values in the diff were empty strings, that it was using the zero value. After review, it doesn't seem like this is the case - so I have adjusted NewComputed to pass nil values. There is also a guard now that keeps the new value writer from accepting computed fields with non-nil values.	9 years ago
Chris Marchesi	6f422d8c44	helper/schema: Remove unused log line ... Meant to remove this before finalizing the PR. :P	9 years ago
Chris Marchesi	c6647a3bb7	helper/schema: CustomizeDiff -> Review ... To keep with the current convention of most other schema.Resource functional fields being fairly short, CustomizeDiff has been changed to "Review". It would be "Diff", however it is already used by existing functions in schema.Provider and schema.Resource.	9 years ago
Chris Marchesi	f7e42728b6	helper/schema: Remove unused Destroy check for CustomizeDiff ... Both Destroy and DestroyDeposed are not propagated down the diff stack, meaning that there is no way we can tell at this point if an instance is being destroyed or deposed, so this check would never be used. In this regard, Destroy never runs a diff down the stack at all, and a deposition check is not run until *after* the provider's diff function is called. To answer this question and close it off, we could either determine if a resource is deposed earlier, and propagate that down, or treat deposed resources like full destroy nodes, and not diff them at all (but rather making a diff with the only thing in it being DestroyDeposed flagged).	9 years ago
Chris Marchesi	fa1fc2ca8e	helper/schema: CustomizeDiff invocation test ... Just one more test to check to make sure that CustomizeDiff is called on resource level.	9 years ago
Chris Marchesi	ee769188d7	helper/schema: More CustomizeDiff test cases ... Added a few more test cases for CustomizeDiff, caught another error in the process. I think this is ready for review now, and possibly some real-world testing of the waters by way of porting some resources that would benefit from the feature.	9 years ago
Chris Marchesi	8af9610b87	helper/schema: Hook CustomizeDiffFunc into diff logic ... It's alive! CustomizeDiff logic now has been inserted into the diff process. The test_resource_with_custom_diff resource provides some basic testing and a reference implementation. There should now be plenty of test coverage for this feature via the tests added for ResourceDiff, and the basic test added to the schemaMap.Diff test, and the test resource, but more can be added to test any specific case that comes up otherwise.	9 years ago
Chris Marchesi	8126ee8401	helper/schema: Fix supplied config to tests	9 years ago
Chris Marchesi	196d7e63fe	helper/schema: Drop ClearAll from ResourceDiff ... As the interface has been specced out for ResourceDiff, the only diff operation that can function on a non-computed key is ForceNew, and that is only if a diff exists for that key. As such, allowing the user to clear keys that cannot be operated on afterwards does not really make much sense. Will re-add this function if it is determined it's needed after all on review.	9 years ago
Chris Marchesi	22220fd0f7	helper/schema: Final set of ResourceDiff tests, bug fixes ... Final set of coverage for ResourceDiff and bug fixes to correct issues the test cases brought up. Will be dropping ClearAll in next commit, as with how this interface is intended to be used, it does not really make much sense.	9 years ago
Chris Marchesi	a5fc664ea6	helper/schema: Add schemaMap.DeepCopy ... This provides a deep copy of a schemaMap, which will be needed for the diff customization process as ResourceDiff will be able to flag fields as ForceNew - we don't want to affect the source schema.	9 years ago
Chris Marchesi	6a4f7b0dce	helper/schema: Don't ignore diffs for certain NewComputed keys ... In diffString, removed values are marked if the old value is non-nil and the new value is nil, regardless of if the new value is marked as a computed value. With the new diff override behaviour, this can lead to issues where a nil attribute may get inserted into the diff if the new value has been marked as computed (as the value will be marked as missing, but computed). This propagates into finalizeDiff in some respects because even if NewComputed is manually set in the diff that gets passed in, it is ignored, and the schema becomes the only source of truth. Since our new diff behaviour is mainly designed to be supported on computed keys only, it stands to reason that we there will be a time where we want to set a diff as NewComputed on a key, and see that change in the diff. These two small changes makes that happen. No regressions in tests have been observed via this change, so it seems safe and non-invasive.	9 years ago
Chris Marchesi	64cc4084d2	helper/schema: ResourceDiff tests, bug fixes ... The beginning of tests for SetNew. Noticed that removed values are not logged for computed keys in a diff - not too sure if that is going to be a problem (possibly not as GetChange in ResourceData should still reference state for the old value). I came on this most notably in the "TestSetNew/complex-ish_set_diff" test, for reference. More tests pending for other functions for coverage of other functions and test cases as defined in #8769.	9 years ago