-
Notifications
You must be signed in to change notification settings - Fork 94
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
function: Replace usage of diagnostics with function errors during execution of provider-defined functions #925
Conversation
…tfprotov5/6.FunctionError
… a provider-defined function returns FunctionErrors
…gnostics documentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some upfront thoughts! Please reach out with questions/concerns.
fwerror/function_error.go
Outdated
// | ||
// To add argument position information to an existing function error, | ||
// see the WithFunctionArgument function. | ||
type FunctionError interface { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given how bespoke these will be compared to other provider development functionality, it may be good to consider:
- Consolidating this into the existing
function
package as it just happens to be a very awkward situation these error types are being introduced, so (famous last words) other error types would not be created in preference of diagnostics, or if they were, they would hopefully be unrelated to functions. - Whether
FunctionError
(function.Error
if above happens) should be simplified to astruct
type instead of an interface. I'm not sure whether we want the extensibility/complexity that is offered by thediag
package here. It might be interesting to peek in source available provider code whether developers are leveraging the extensibility of diagnostics via those interface(s) or just implementing the existing types/methods. Implementing Go 1.13+ error wrapping capabilities might be good enough, where diagnostics do have separate capabilities beyond pure error handling. 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've gone ahead and refactored fwerror.FunctionErrors
to function.FuncError
.
These changes have removed the fwerror.FunctionError
, and fwerror.FunctionErrorWithFunctionArgument
interfaces, as well as removing the fwerror.FunctionErrors
type. In their place is a single function.FuncError
struct type.
A function.ConcatFuncErrors
helper function has been created which concatenates the function.FuncError
text fields of two or more instances of function.FuncError
. We could, if we decide to go this route, make the concatenation a method of FuncError
itself. I thought concatenation might represent the simplest implementation that would allow handling of multiple instances of function.FuncError
, given that errors.Join
returns the error
interface, requiring that we either alter the return type from function.FuncError
to error
and use type assertion where necessary, or implement our own version of Join
, specifically for function.FuncError
. In the latter case, unless we re-introduce an interface, I believe we'll need to add a errs []*FuncError
field to function.FuncError
struct type, which seems a little odd to me.
Interested to hear your thoughts on the function.FuncError
implementation, and the associated helper functions.
The usages of github.com/hashicorp/terraform-plugin-framework/diag
that I found all appeared to be using the pre-defined diagnostics rather than new implementations of the available interfaces:
- https://github.com/DataDog/terraform-provider-datadog/blob/46998f7edae69f6ecd01d522fb7c7fb92e7f401c/datadog/internal/utils/utils.go
- https://github.com/elastic/terraform-provider-elasticstack/blob/4d6e2cbc8d6c09eb3c926ed0be864a89017134e7/internal/clients/config/kibana.go
- https://github.com/lxc/terraform-provider-incus/blob/7dba95ee15d906db2489b96ef18343141c0c4a15/internal/common/incus_import.go
- https://github.com/linode/terraform-provider-linode/blob/b4fb82bcd8e20947be488777aef22092315d85f3/linode/firewall/framework_models.go
- https://github.com/dnsimple/terraform-provider-dnsimple/blob/bc71acb3b1d1ced3521814dd52632f411d828b73/internal/framework/resources/zone_resource.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I read this after I left my comment lol. TLDR: I like concat
😄 . Also by avoiding the language of Join
we also avoid confusion if developers think they could also Unwrap
a function error. Which with this implementation we cannot
function/run.go
Outdated
Diagnostics diag.Diagnostics | ||
// Errors contains errors related to running the function. | ||
// A nil error indicates success, with no errors generated. | ||
Errors fwerror.FunctionErrors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be safer for the framework to expose this abstraction more inline with the protocol (e.g. a single function error) and then potentially offer opinionated helper(s) that may change over time for joining multiple errors into one? Is errors.Join()
or our own version of that acceptable enough? We have tried to avoid the framework from forcing opinionated behaviors on provider developer responses where possible in the past.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned above, we now have a single function.FuncError
struct type, with function.ConcatFuncErrors
and function.FuncErrorFromDiags
helper functions. The function.ConcatFuncErrors
is our current version of errors.Join()
.
## How Errors Affect State | ||
|
||
**Returning function errors does not stop the state from being updated**. | ||
Terraform will still persist the returned state even when function errors | ||
are returned with it. This is to allow Terraform to persist the values that have | ||
already been modified when a resource modification requires multiple API | ||
requests or an API request fails after an earlier one succeeded. | ||
|
||
When returning function errors, we recommend resetting the state in the | ||
response to the prior state available in the configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we double check this? I believe that typically function calls and their errors happen will so early in the operation graph that this would not necessarily be true (except maybe if there are unknown values being passed as arguments).
I'm wondering if its better to just omit this whole section, since "state" is not a function concept and this was mostly relating to managed resources, or whether we need to get into those function error handling implementation details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's ok, I'm going to hold off making any further changes to the documentation until we converge on the implementation of FunctionError
/FuncError
. But as an aside, I checked this, and you're quite right, returning a diagnostic will result in no alteration to the state file. Returning a function error currently has no effect, presumably because it is not yet being handled within core, and requires hashicorp/terraform#34603 to be merged. Although, I believe I could build Terraform with that branch and test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated the docs to remove this section, as suggested.
the response function errors can help ensure that any response will include the | ||
expected function errors. | ||
|
||
### Creating Diagnostics |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
### Creating Diagnostics | |
### Creating Errors |
### Creating Diagnostics | ||
|
||
When working with logic outside the framework, such as interacting with the | ||
vendor or `net/http` library to make the actual calls to manage infrastructure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's avoid net/http
here since we want to discourage those types of functions. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've removed the reference to net/http
.
|
||
require ( | ||
github.com/google/go-cmp v0.6.0 | ||
github.com/hashicorp/terraform-plugin-go v0.21.0 | ||
github.com/hashicorp/terraform-plugin-go v0.22.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Running go get github.com/hashicorp/terraform-plugin-go
results in the addition of the following entry to go.mod
:
toolchain go1.21.6
Which would result in an analogous CI failure to that seen on terraform-plugin-mux
Given that our CI runs both Go 1.20
and 1.21
, I removed this directive, but perhaps this should be handled differently?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh interesting. It sounds like what you've done is basically the solution: https://go.dev/doc/toolchain
The same requirement applies in reverse when downgrading: if the go.mod starts at go 1.22.1 and toolchain go1.24rc1, then go get toolchain@go1.22.9 will update only the toolchain line, but go get toolchain@go1.21.3 will downgrade the go line to go 1.21.3 as well. The effect will be to leave just go 1.21.3 with no toolchain line.
The special form toolchain@none means to remove any toolchain line, as in go get toolchain@none or go get go@1.25.0 toolchain@none.
Sounds like we could also potentially just run a go get toolchain@none
before we run our CI, but you've essentially done that..... Will a go mod tidy
bring it back?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Running go mod tidy
will not re-instate the toolchain directive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some questions, but overall this implementation looks great!
I skipped the doc section because it looks like they still need to be updated and I didn't want to add too much noise (apologies if I already did 😆 )
diag/diagnostic.go
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment placement not really relevant, but I wanted a thread 😆
Open question: Do we think it'd be valuable to note in the diag
package doc that this package does not contain function specific error handling? Currently it's kind of generic about the error handling it can do:
terraform-plugin-framework/diag/doc.go
Lines 4 to 8 in 3c7a391
// Package diag implements diagnostic functionality, which is a practitioner | |
// feedback mechanism for providers. It is designed for display in Terraform | |
// user interfaces, rather than logging based feedback, which is generally | |
// saved to a file for later inspection and troubleshooting. | |
package diag |
Maybe we could redirect them to the function package / specific error struct (since this is the only exception currently)
Same question for the package doc for function
, should we note that it also contains error handling specific to function implementations:
terraform-plugin-framework/function/doc.go
Lines 4 to 18 in 3c7a391
// Package function contains all interfaces, request types, and response | |
// types for a Terraform Provider function implementation. | |
// | |
// In Terraform, a function is a concept which enables provider developers | |
// to offer practitioners a pure function call in their configuration. Functions | |
// are defined by a function name, such as "parse_xyz", a definition | |
// representing the ordered list of parameters with associated data types and | |
// a result data type, and the function logic. | |
// | |
// The main starting point for implementations in this package is the | |
// [Function] type which represents an instance of a function that has its own | |
// argument data when called. The [Function] implementations are referenced by a | |
// [provider.Provider] type Functions method, which enables the function for | |
// practitioner and testing usage. | |
package function |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a note into doc.go
for both diag
and function
packages highlighting the need to use FuncError
to notify practitioners of issues arising during execution of provider-defined functions.
function/arguments_data.go
Outdated
) | ||
|
||
return diags | ||
text := "Invalid Argument Data Usage: When attempting to fetch argument data during the function call, the provider code incorrectly attempted to read argument data. " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
super nit 🦸🏻 - maybe a variable name like errMsg
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
function/arguments_data.go
Outdated
) | ||
|
||
return diags | ||
text := "Invalid Argument Data Usage: When attempting to fetch argument data during the function call, the provider code incorrectly attempted to read argument data. " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
super nit 🦸🏻 - Should we attempt to format the old "summary" with /n/n
at the end? Or should we avoid attempting to format our error messages too much?
text := "Invalid Argument Data Usage:\n\nWhen attempting to fetch argument data during the function call, the provider code incorrectly attempted to read argument data. " +
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to go with consensus on this one. Right now FuncErrorFromDiags
is formatting as follows:
NewFuncError(fmt.Sprintf("%s: %s", d.Summary(), d.Detail()))
So, I'll leave the formatting as it is for right now, but if we decide to update to use \n\n
between the "summary" and "detail" I'll update it in all locations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine leaving it as-is. Feels like avoiding formatting with /n/n
is probably best 👍🏻
// ConcatFuncErrors returns a new function error with the text from all supplied | ||
// function errors concatenated together. If any of the function errors have a | ||
// function argument, the first one encountered will be used. | ||
func ConcatFuncErrors(funcErrs ...*FuncError) *FuncError { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the usage of concat
here instead of something more Go-like, like join
, since they are distinctly different and I think this describes what is happening well 👍🏻
function/func_error.go
Outdated
// FuncErrorFromDiags iterates over the given diagnostics and returns a new function error | ||
// with the text from all error diagnostics concatenated together. If any of the error diagnostics | ||
// have a function argument, the first one encountered will be used. | ||
func FuncErrorFromDiags(ctx context.Context, diags diag.Diagnostics) *FuncError { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to describe the behavior of warning diagnostics here (that they will log and not be represented in the resulting function error)
Also I'm not sure where else we need it, but it might be good to mention some of the automatic "downgrading" of diagnostics somewhere (in-case developers have existing custom types for a parameter that return warning diags, for example)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've expaned the Go docs for FuncErrorFromDiags
to highlight how warning diagnostics will be processed. I've also added a section to the website docs.
function/func_error.go
Outdated
ok := errors.As(other, &funcErr) | ||
|
||
if !ok { | ||
return false | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
curious ❓: Is it possible for this to ever be false
? This may have been left over from when the implementation was using interfaces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oooopsie. Removed.
function/run.go
Outdated
// Error contains errors related to running the function. | ||
// A nil error indicates success, with no errors generated. | ||
Error *FuncError |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a shout here that ConcatFuncErrors
can be utilized to simplify smushing together multiple errors? 🔨
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have added a comment.
|
||
require ( | ||
github.com/google/go-cmp v0.6.0 | ||
github.com/hashicorp/terraform-plugin-go v0.21.0 | ||
github.com/hashicorp/terraform-plugin-go v0.22.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh interesting. It sounds like what you've done is basically the solution: https://go.dev/doc/toolchain
The same requirement applies in reverse when downgrading: if the go.mod starts at go 1.22.1 and toolchain go1.24rc1, then go get toolchain@go1.22.9 will update only the toolchain line, but go get toolchain@go1.21.3 will downgrade the go line to go 1.21.3 as well. The effect will be to leave just go 1.21.3 with no toolchain line.
The special form toolchain@none means to remove any toolchain line, as in go get toolchain@none or go get go@1.25.0 toolchain@none.
Sounds like we could also potentially just run a go get toolchain@none
before we run our CI, but you've essentially done that..... Will a go mod tidy
bring it back?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! 🚀
function/func_error.go
Outdated
|
||
// FuncError is an error type specifically for function errors. | ||
type FuncError struct { | ||
Text string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies, I had commented about the variable name of text
while forgetting the underlying struct field was named Text
😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries. Have left it as errMsg
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I know its trivial in this case, but can we please ensure all exported types and fields have Go documentation? For example, the website documentation for these fields seems great.
### FunctionArgument | ||
|
||
`FunctionArgument` identifies the specific function argument position that caused the | ||
error. Only errors that pertain to a function argument will include this information. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Maybe add a note that it is an int64
type and that the argument position is zero-based
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to include your suggestion.
When creating function errors that affect an entire data source, provider, or | ||
resource, and where a `function.FunctionError` is already available such as within |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When creating function errors that affect an entire data source, provider, or
resource
I didn't quite understand what this sentence is referring to, can you expand on this or maybe clarify?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've stripped that out.
func (f *ExampleFunction) Run(ctx context.Context, req function.RunRequest, resp *function.RunResponse) { | ||
// ... prior logic ... | ||
|
||
val, err := // operation that may return an error | ||
|
||
if err != nil { | ||
resp.Error = ConcatFuncErrors(resp.Error, function.NewFuncError("Error performing operation: " + err.Error())) | ||
return | ||
} | ||
|
||
// ... further logic ... | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Go formatting 🙂 (does our website auto-fix the formatting? I wouldn't be surprised if it did)
Same formatting is a little off for some of the other examples (at least in GitHub)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, the old tabs/spaces fun and games. Fixed throughout the docs.
Co-authored-by: Austin Valle <austinvalle@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
function/func_error.go
Outdated
|
||
// FuncError is an error type specifically for function errors. | ||
type FuncError struct { | ||
Text string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I know its trivial in this case, but can we please ensure all exported types and fields have Go documentation? For example, the website documentation for these fields seems great.
function/result_data.go
Outdated
@@ -22,7 +21,7 @@ type ResultData struct { | |||
} | |||
|
|||
// Equal returns true if the value is equivalent. | |||
func (d ResultData) Equal(o ResultData) bool { | |||
func (d *ResultData) Equal(o ResultData) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Was there a reason for this change? It now means that this method needs to potentially account for d == nil
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change was just to make all methods on the ResultData
struct on a pointer receiver. Have reverted.
function/result_data.go
Outdated
} | ||
|
||
// Value returns the saved value. | ||
func (d ResultData) Value() attr.Value { | ||
func (d *ResultData) Value() attr.Value { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Same here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change was just to make all methods on the ResultData
struct on a pointer receiver. Have reverted.
╷ | ||
│ Error: Message | ||
│ | ||
│ on example.tf line #: | ||
│ #: source configuration line | ||
╵ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI the error diagnostic for a regular function error appears to be:
Error: Error in function call
on example.tf line #:
#: source configuration line
Call to function "{FUNCTION}" failed: {TEXT}.
And for an argument error:
Error: Invalid function argument
on example.tf line #:
#: source configuration line
Invalid value for "{PARAMETER_NAME}" parameter: {TEXT}.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Have updated the docs with an example of both.
…ecution of provider-defined functions (#925) * Replacing function.RunResponse diagnostics with error * Adding custom FunctionError * Adding custom FunctionErrors * Removing unneeded equateErrors gocmp option * Switching to using convenience functions for adding errors to FunctionErrors * Add copyright headers * Refactor to use Error() method on function errors when converting to tfprotov5/6.FunctionError * Adding documentation and testing for fwerrors types * Formatting errors during conversion to tfprotov<5|6>.FunctionError * Removing argument error and argument warning diagnostics * Renaming field name for FunctionErrors from Error to Errors * Modifying documentation to reflect that executing the Run() method of a provider-defined function returns FunctionErrors * Remove references to AddArgumentError and AddArgumentWarning from diagnostics documentation * Removing fwerror package and moving FunctionError to function package * Refactoring to replace FunctionErrors slice with single FunctionError * Bumping terraform-plugin-go to v0.22.0 * Removing unneeded DiagnosticWithFunctionArgument interface and implementation * Altering function signature of ConcatFuncErrors * Removing HasError method * Updating docs * Updates following code review * Adding changelog entries * Fix naming * Update website/docs/plugin/framework/functions/errors.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Formatting * Updates following code review --------- Co-authored-by: Austin Valle <austinvalle@gmail.com>
* initial implementation of dynamic type and value * fix doc string * add dynamic defaults * add plan modifier for dynamic * add dynamic attribute, validator, and plan modifiers * add provider and data source attributes and tests * update comment * add resource schema attribute test * add provider metaschema * add hook into attribute validation * add hook into plan modification for resources * add hook into dynamic default logic * add hooks for dynamic semantic equality * add function param and return definitions * add function tests * modify ToTerraformValue to always return DPT * switch back * make reflection update * update dep * use safe equal * return attr.Type * add todos and docs based on findings * random doc cleanup * doc fix * fix for data path * switch to tuples and add reflection support * update docs * add names to variadic param names * update doc note * update docs * update docs * add doc note * add changelog * delete metaschema and fix broken tests * updated some docs * switch changelog to breaking change * update to tuple index * update comment wording * update doc * add new maintainer note * remove top part of comment * add logic to look at value type for dynamic schemas * add data value tests * add dynamic get at path tests * add dynamic get tests * add path exist tests * add get provider schema tests * add comment to weird obj fix * make changes to support reflection (get) * function: Switch the representation of variadic arguments to `types.Tuple` (#923) * switch to tuples and add reflection support * update docs * add names to variadic param names * update doc note * update docs * update docs * add doc note * add changelog * switch changelog to breaking change * update to tuple index * update comment wording * update doc * add new maintainer note * remove top part of comment * fixed the odd spacing in the comment :) * add dynamic interface * reflection test * add list type update * add list value updates * small refactoring * quick check * revert list type and value * add detailed comments about the lack of dynamic element type support * add some object tests for good measure * add tuple tests * add validation logic for list attribute only * add block and attribute validation for list * function: Replace usage of diagnostics with function errors during execution of provider-defined functions (#925) * Replacing function.RunResponse diagnostics with error * Adding custom FunctionError * Adding custom FunctionErrors * Removing unneeded equateErrors gocmp option * Switching to using convenience functions for adding errors to FunctionErrors * Add copyright headers * Refactor to use Error() method on function errors when converting to tfprotov5/6.FunctionError * Adding documentation and testing for fwerrors types * Formatting errors during conversion to tfprotov<5|6>.FunctionError * Removing argument error and argument warning diagnostics * Renaming field name for FunctionErrors from Error to Errors * Modifying documentation to reflect that executing the Run() method of a provider-defined function returns FunctionErrors * Remove references to AddArgumentError and AddArgumentWarning from diagnostics documentation * Removing fwerror package and moving FunctionError to function package * Refactoring to replace FunctionErrors slice with single FunctionError * Bumping terraform-plugin-go to v0.22.0 * Removing unneeded DiagnosticWithFunctionArgument interface and implementation * Altering function signature of ConcatFuncErrors * Removing HasError method * Updating docs * Updates following code review * Adding changelog entries * Fix naming * Update website/docs/plugin/framework/functions/errors.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Formatting * Updates following code review --------- Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update changelog * diag: remove incorrect code (#935) * update plugin-go dependency, fix errors and switch equal method * function: Add validation for parameter name conflicts and update defaulting logic (#936) * add validation and refactor defaulting logic * add changelogs * test fix * Update website/docs/plugin/framework/functions/documentation.mdx Co-authored-by: Brian Flad <bflad417@gmail.com> * refactor the logic, tests and docs for defaulting * Update website/docs/plugin/framework/functions/documentation.mdx Co-authored-by: Benjamin Bennett <ben.bennett@hashicorp.com> --------- Co-authored-by: Brian Flad <bflad417@gmail.com> Co-authored-by: Benjamin Bennett <ben.bennett@hashicorp.com> * resource/schema: Ensure invalid attribute default value errors are raised (#933) Reference: #590 Reference: #930 Previously the logic handling attribute `Default` values would silently ignore any type errors, which would lead to confusing planning data behaviors. This updates the logic to raise those error properly and adds covering unit testing. These error messages are using the underlying `tftypes` type system errors which is currently a pragmatic compromise throughout various parts of the framework logic that bridges between both type systems to save additional type assertion logic and potential bugs relating to those conversions. In the future if the internal `tftypes` handling and exported fields are replaced with the framework type system types, this logic would instead return error messaging based on the framework type system errors. This also will enhance the schema validation logic to check any `Default` response value and compare its type to the schema, which will raise framework type system errors during the `GetProviderSchema` RPC, or during schema unit testing if provider developers have implemented that additional testing. * Update provider functions testing docs to help users avoid nil pointer error (#940) * Fix capitalisation * Update example test to mitigate nil pointer error See #928 * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> --------- Co-authored-by: Austin Valle <austinvalle@gmail.com> Co-authored-by: Brian Flad <bflad417@gmail.com> * Reinstate go toolchain and add changelog for Go version bump to 1.21 (#937) * Reinstate go toolchain and add changelog for Go version bump to 1.21 * Adding changelog * Updating changelog * Squashed commit of the following: commit e7415b7 Author: Benjamin Bennett <ben.bennett@hashicorp.com> Date: Fri Mar 1 16:16:39 2024 +0000 Reinstate go toolchain and add changelog for Go version bump to 1.21 (#937) * Reinstate go toolchain and add changelog for Go version bump to 1.21 * Adding changelog * Updating changelog commit 1597a95 Author: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Fri Mar 1 16:12:22 2024 +0000 Update provider functions testing docs to help users avoid nil pointer error (#940) * Fix capitalisation * Update example test to mitigate nil pointer error See #928 * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> --------- Co-authored-by: Austin Valle <austinvalle@gmail.com> Co-authored-by: Brian Flad <bflad417@gmail.com> commit bd22b58 Author: Brian Flad <bflad417@gmail.com> Date: Fri Mar 1 07:20:55 2024 -0500 resource/schema: Ensure invalid attribute default value errors are raised (#933) Reference: #590 Reference: #930 Previously the logic handling attribute `Default` values would silently ignore any type errors, which would lead to confusing planning data behaviors. This updates the logic to raise those error properly and adds covering unit testing. These error messages are using the underlying `tftypes` type system errors which is currently a pragmatic compromise throughout various parts of the framework logic that bridges between both type systems to save additional type assertion logic and potential bugs relating to those conversions. In the future if the internal `tftypes` handling and exported fields are replaced with the framework type system types, this logic would instead return error messaging based on the framework type system errors. This also will enhance the schema validation logic to check any `Default` response value and compare its type to the schema, which will raise framework type system errors during the `GetProviderSchema` RPC, or during schema unit testing if provider developers have implemented that additional testing. commit f03ca33 Author: Austin Valle <austinvalle@gmail.com> Date: Thu Feb 29 14:23:29 2024 -0500 function: Add validation for parameter name conflicts and update defaulting logic (#936) * add validation and refactor defaulting logic * add changelogs * test fix * Update website/docs/plugin/framework/functions/documentation.mdx Co-authored-by: Brian Flad <bflad417@gmail.com> * refactor the logic, tests and docs for defaulting * Update website/docs/plugin/framework/functions/documentation.mdx Co-authored-by: Benjamin Bennett <ben.bennett@hashicorp.com> --------- Co-authored-by: Brian Flad <bflad417@gmail.com> Co-authored-by: Benjamin Bennett <ben.bennett@hashicorp.com> * fix dynamic param * incorrect merge conflict fix :) * update comments from feedback * refactor validation logic * license headers * implement remaining resource and datasource validation * implement provider schema validation * update error msg and add object validator * add validation to object attributes * update existing attributes to use new bool return * add validation to function parameters and definitions * refactor to only have one exported function * add tuple tests for completeness * create new fwtype package * add parameter name to validate implementation definition * various PR fixes * add more docs * fix docs from default param change * update comment * add more to reflection case + update comment * comment wording * remove todos and add package docs * add changelogs * Fix the use-case where dynamic value type is known, but the value itself is still null/unknown * check for dynamic underlying value for null/unknown detection * removed the unneccessary interface and removed half-implemented reflection support * doc formatting * update static default naming * add happy path tests for datasource + provider validate implementations * update definition validation messaging with variadic * add doc explicitly saying that dynamic types aren't supported * add docs mentioning required dynamic type to functions * prevent nil panics in `types.Dynamic` implementation w/ tests * proposal for `NewDynamicValue` * add recommendations in doc string * update block msg * update all doc strings and error messages to make recommendations * move the function validate interfaces to internal package * add maintainer note about parameter name * prevent attribute paths from stepping into dynamic types + new tests * add helper methods for checking underlying value unknown/null * add tests and comments about edge case with empty values * move maintainer note * Add dynamic type support documentation and considerations * first pass at dynamic documentation * Apply suggestions from code review Co-authored-by: Brian Flad <bflad417@gmail.com> * update literal representation in attribute page * drop callouts and link to collection type page * Update website/docs/plugin/framework/handling-data/attributes/dynamic.mdx Co-authored-by: Brian Flad <bflad417@gmail.com> * Apply suggestions from code review Co-authored-by: Brian Flad <bflad417@gmail.com> * Update website/docs/plugin/framework/functions/parameters/dynamic.mdx Co-authored-by: Brian Flad <bflad417@gmail.com> * Update website/docs/plugin/framework/handling-data/dynamic-data.mdx Co-authored-by: Brian Flad <bflad417@gmail.com> * include parameter name in example * adjust the path documentation to reflect new changes * rework the first paragraph of the data page * add callout for null underlying element values * add underlying value null/unknown information * Update website/docs/plugin/framework/handling-data/types/dynamic.mdx Co-authored-by: Brian Flad <bflad417@gmail.com> * Update website/docs/plugin/framework/handling-data/dynamic-data.mdx Co-authored-by: Brian Flad <bflad417@gmail.com> * add float64 and number notes --------- Co-authored-by: Brian Flad <bflad417@gmail.com> --------- Co-authored-by: Benjamin Bennett <ben.bennett@hashicorp.com> Co-authored-by: hc-github-team-tf-provider-devex <github-team-tf-provider-devex@hashicorp.com> Co-authored-by: John Behm <jxsl13@googlemail.com> Co-authored-by: Brian Flad <bflad417@gmail.com> Co-authored-by: Sarah French <15078782+SarahFrench@users.noreply.github.com>
* initial implementation of dynamic type and value * fix doc string * add dynamic defaults * add plan modifier for dynamic * add dynamic attribute, validator, and plan modifiers * add provider and data source attributes and tests * update comment * add resource schema attribute test * add provider metaschema * add hook into attribute validation * add hook into plan modification for resources * add hook into dynamic default logic * add hooks for dynamic semantic equality * add function param and return definitions * add function tests * modify ToTerraformValue to always return DPT * switch back * make reflection update * update dep * use safe equal * return attr.Type * add todos and docs based on findings * random doc cleanup * doc fix * fix for data path * switch to tuples and add reflection support * update docs * add names to variadic param names * update doc note * update docs * update docs * add doc note * add changelog * delete metaschema and fix broken tests * updated some docs * switch changelog to breaking change * update to tuple index * update comment wording * update doc * add new maintainer note * remove top part of comment * add logic to look at value type for dynamic schemas * add data value tests * add dynamic get at path tests * add dynamic get tests * add path exist tests * add get provider schema tests * add comment to weird obj fix * make changes to support reflection (get) * function: Switch the representation of variadic arguments to `types.Tuple` (#923) * switch to tuples and add reflection support * update docs * add names to variadic param names * update doc note * update docs * update docs * add doc note * add changelog * switch changelog to breaking change * update to tuple index * update comment wording * update doc * add new maintainer note * remove top part of comment * fixed the odd spacing in the comment :) * add dynamic interface * reflection test * add list type update * add list value updates * small refactoring * quick check * revert list type and value * add detailed comments about the lack of dynamic element type support * add some object tests for good measure * add tuple tests * add validation logic for list attribute only * add block and attribute validation for list * function: Replace usage of diagnostics with function errors during execution of provider-defined functions (#925) * Replacing function.RunResponse diagnostics with error * Adding custom FunctionError * Adding custom FunctionErrors * Removing unneeded equateErrors gocmp option * Switching to using convenience functions for adding errors to FunctionErrors * Add copyright headers * Refactor to use Error() method on function errors when converting to tfprotov5/6.FunctionError * Adding documentation and testing for fwerrors types * Formatting errors during conversion to tfprotov<5|6>.FunctionError * Removing argument error and argument warning diagnostics * Renaming field name for FunctionErrors from Error to Errors * Modifying documentation to reflect that executing the Run() method of a provider-defined function returns FunctionErrors * Remove references to AddArgumentError and AddArgumentWarning from diagnostics documentation * Removing fwerror package and moving FunctionError to function package * Refactoring to replace FunctionErrors slice with single FunctionError * Bumping terraform-plugin-go to v0.22.0 * Removing unneeded DiagnosticWithFunctionArgument interface and implementation * Altering function signature of ConcatFuncErrors * Removing HasError method * Updating docs * Updates following code review * Adding changelog entries * Fix naming * Update website/docs/plugin/framework/functions/errors.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Formatting * Updates following code review --------- Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update changelog * diag: remove incorrect code (#935) * update plugin-go dependency, fix errors and switch equal method * function: Add validation for parameter name conflicts and update defaulting logic (#936) * add validation and refactor defaulting logic * add changelogs * test fix * Update website/docs/plugin/framework/functions/documentation.mdx Co-authored-by: Brian Flad <bflad417@gmail.com> * refactor the logic, tests and docs for defaulting * Update website/docs/plugin/framework/functions/documentation.mdx Co-authored-by: Benjamin Bennett <ben.bennett@hashicorp.com> --------- Co-authored-by: Brian Flad <bflad417@gmail.com> Co-authored-by: Benjamin Bennett <ben.bennett@hashicorp.com> * resource/schema: Ensure invalid attribute default value errors are raised (#933) Reference: #590 Reference: #930 Previously the logic handling attribute `Default` values would silently ignore any type errors, which would lead to confusing planning data behaviors. This updates the logic to raise those error properly and adds covering unit testing. These error messages are using the underlying `tftypes` type system errors which is currently a pragmatic compromise throughout various parts of the framework logic that bridges between both type systems to save additional type assertion logic and potential bugs relating to those conversions. In the future if the internal `tftypes` handling and exported fields are replaced with the framework type system types, this logic would instead return error messaging based on the framework type system errors. This also will enhance the schema validation logic to check any `Default` response value and compare its type to the schema, which will raise framework type system errors during the `GetProviderSchema` RPC, or during schema unit testing if provider developers have implemented that additional testing. * Update provider functions testing docs to help users avoid nil pointer error (#940) * Fix capitalisation * Update example test to mitigate nil pointer error See #928 * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> --------- Co-authored-by: Austin Valle <austinvalle@gmail.com> Co-authored-by: Brian Flad <bflad417@gmail.com> * Reinstate go toolchain and add changelog for Go version bump to 1.21 (#937) * Reinstate go toolchain and add changelog for Go version bump to 1.21 * Adding changelog * Updating changelog * Squashed commit of the following: commit e7415b7 Author: Benjamin Bennett <ben.bennett@hashicorp.com> Date: Fri Mar 1 16:16:39 2024 +0000 Reinstate go toolchain and add changelog for Go version bump to 1.21 (#937) * Reinstate go toolchain and add changelog for Go version bump to 1.21 * Adding changelog * Updating changelog commit 1597a95 Author: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Fri Mar 1 16:12:22 2024 +0000 Update provider functions testing docs to help users avoid nil pointer error (#940) * Fix capitalisation * Update example test to mitigate nil pointer error See #928 * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> --------- Co-authored-by: Austin Valle <austinvalle@gmail.com> Co-authored-by: Brian Flad <bflad417@gmail.com> commit bd22b58 Author: Brian Flad <bflad417@gmail.com> Date: Fri Mar 1 07:20:55 2024 -0500 resource/schema: Ensure invalid attribute default value errors are raised (#933) Reference: #590 Reference: #930 Previously the logic handling attribute `Default` values would silently ignore any type errors, which would lead to confusing planning data behaviors. This updates the logic to raise those error properly and adds covering unit testing. These error messages are using the underlying `tftypes` type system errors which is currently a pragmatic compromise throughout various parts of the framework logic that bridges between both type systems to save additional type assertion logic and potential bugs relating to those conversions. In the future if the internal `tftypes` handling and exported fields are replaced with the framework type system types, this logic would instead return error messaging based on the framework type system errors. This also will enhance the schema validation logic to check any `Default` response value and compare its type to the schema, which will raise framework type system errors during the `GetProviderSchema` RPC, or during schema unit testing if provider developers have implemented that additional testing. commit f03ca33 Author: Austin Valle <austinvalle@gmail.com> Date: Thu Feb 29 14:23:29 2024 -0500 function: Add validation for parameter name conflicts and update defaulting logic (#936) * add validation and refactor defaulting logic * add changelogs * test fix * Update website/docs/plugin/framework/functions/documentation.mdx Co-authored-by: Brian Flad <bflad417@gmail.com> * refactor the logic, tests and docs for defaulting * Update website/docs/plugin/framework/functions/documentation.mdx Co-authored-by: Benjamin Bennett <ben.bennett@hashicorp.com> --------- Co-authored-by: Brian Flad <bflad417@gmail.com> Co-authored-by: Benjamin Bennett <ben.bennett@hashicorp.com> * fix dynamic param * incorrect merge conflict fix :) * update comments from feedback * refactor validation logic * license headers * implement remaining resource and datasource validation * implement provider schema validation * update error msg and add object validator * add validation to object attributes * update existing attributes to use new bool return * add validation to function parameters and definitions * refactor to only have one exported function * add tuple tests for completeness * create new fwtype package * add parameter name to validate implementation definition * various PR fixes * add more docs * fix docs from default param change * update comment * add more to reflection case + update comment * comment wording * remove todos and add package docs * add changelogs * Fix the use-case where dynamic value type is known, but the value itself is still null/unknown * check for dynamic underlying value for null/unknown detection * removed the unneccessary interface and removed half-implemented reflection support * doc formatting * update static default naming * add happy path tests for datasource + provider validate implementations * update definition validation messaging with variadic * add doc explicitly saying that dynamic types aren't supported * add docs mentioning required dynamic type to functions * prevent nil panics in `types.Dynamic` implementation w/ tests * proposal for `NewDynamicValue` * add recommendations in doc string * update block msg * update all doc strings and error messages to make recommendations * move the function validate interfaces to internal package * add maintainer note about parameter name * prevent attribute paths from stepping into dynamic types + new tests * add helper methods for checking underlying value unknown/null * add tests and comments about edge case with empty values * move maintainer note * Add dynamic type support documentation and considerations * first pass at dynamic documentation * Apply suggestions from code review Co-authored-by: Brian Flad <bflad417@gmail.com> * update literal representation in attribute page * drop callouts and link to collection type page * Update website/docs/plugin/framework/handling-data/attributes/dynamic.mdx Co-authored-by: Brian Flad <bflad417@gmail.com> * Apply suggestions from code review Co-authored-by: Brian Flad <bflad417@gmail.com> * Update website/docs/plugin/framework/functions/parameters/dynamic.mdx Co-authored-by: Brian Flad <bflad417@gmail.com> * Update website/docs/plugin/framework/handling-data/dynamic-data.mdx Co-authored-by: Brian Flad <bflad417@gmail.com> * include parameter name in example * adjust the path documentation to reflect new changes * rework the first paragraph of the data page * add callout for null underlying element values * add underlying value null/unknown information * Update website/docs/plugin/framework/handling-data/types/dynamic.mdx Co-authored-by: Brian Flad <bflad417@gmail.com> * Update website/docs/plugin/framework/handling-data/dynamic-data.mdx Co-authored-by: Brian Flad <bflad417@gmail.com> * add float64 and number notes --------- Co-authored-by: Brian Flad <bflad417@gmail.com> --------- Co-authored-by: Benjamin Bennett <ben.bennett@hashicorp.com> Co-authored-by: hc-github-team-tf-provider-devex <github-team-tf-provider-devex@hashicorp.com> Co-authored-by: John Behm <jxsl13@googlemail.com> Co-authored-by: Brian Flad <bflad417@gmail.com> Co-authored-by: Sarah French <15078782+SarahFrench@users.noreply.github.com>
* Enforce provider-defined function parameter naming * Passing function name to ValidateImplementation to provide additional context in diagnostics * Add nil check for Error() method on function error * Adding changelog entry * Update .changes/unreleased/BREAKING CHANGES-20240320-095152.yaml Co-authored-by: Brian Flad <bflad417@gmail.com> * Switching to request-response pattern for function definition validation * Adding change log entry for removal of DefaultParameterNamePrefix and DefaultVariadicParameterName constants * Using parameter names throughout examples, and adding illustration of potential runtime errors * all: Add dynamic type, attribute, and function support (#931) * initial implementation of dynamic type and value * fix doc string * add dynamic defaults * add plan modifier for dynamic * add dynamic attribute, validator, and plan modifiers * add provider and data source attributes and tests * update comment * add resource schema attribute test * add provider metaschema * add hook into attribute validation * add hook into plan modification for resources * add hook into dynamic default logic * add hooks for dynamic semantic equality * add function param and return definitions * add function tests * modify ToTerraformValue to always return DPT * switch back * make reflection update * update dep * use safe equal * return attr.Type * add todos and docs based on findings * random doc cleanup * doc fix * fix for data path * switch to tuples and add reflection support * update docs * add names to variadic param names * update doc note * update docs * update docs * add doc note * add changelog * delete metaschema and fix broken tests * updated some docs * switch changelog to breaking change * update to tuple index * update comment wording * update doc * add new maintainer note * remove top part of comment * add logic to look at value type for dynamic schemas * add data value tests * add dynamic get at path tests * add dynamic get tests * add path exist tests * add get provider schema tests * add comment to weird obj fix * make changes to support reflection (get) * function: Switch the representation of variadic arguments to `types.Tuple` (#923) * switch to tuples and add reflection support * update docs * add names to variadic param names * update doc note * update docs * update docs * add doc note * add changelog * switch changelog to breaking change * update to tuple index * update comment wording * update doc * add new maintainer note * remove top part of comment * fixed the odd spacing in the comment :) * add dynamic interface * reflection test * add list type update * add list value updates * small refactoring * quick check * revert list type and value * add detailed comments about the lack of dynamic element type support * add some object tests for good measure * add tuple tests * add validation logic for list attribute only * add block and attribute validation for list * function: Replace usage of diagnostics with function errors during execution of provider-defined functions (#925) * Replacing function.RunResponse diagnostics with error * Adding custom FunctionError * Adding custom FunctionErrors * Removing unneeded equateErrors gocmp option * Switching to using convenience functions for adding errors to FunctionErrors * Add copyright headers * Refactor to use Error() method on function errors when converting to tfprotov5/6.FunctionError * Adding documentation and testing for fwerrors types * Formatting errors during conversion to tfprotov<5|6>.FunctionError * Removing argument error and argument warning diagnostics * Renaming field name for FunctionErrors from Error to Errors * Modifying documentation to reflect that executing the Run() method of a provider-defined function returns FunctionErrors * Remove references to AddArgumentError and AddArgumentWarning from diagnostics documentation * Removing fwerror package and moving FunctionError to function package * Refactoring to replace FunctionErrors slice with single FunctionError * Bumping terraform-plugin-go to v0.22.0 * Removing unneeded DiagnosticWithFunctionArgument interface and implementation * Altering function signature of ConcatFuncErrors * Removing HasError method * Updating docs * Updates following code review * Adding changelog entries * Fix naming * Update website/docs/plugin/framework/functions/errors.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Formatting * Updates following code review --------- Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update changelog * diag: remove incorrect code (#935) * update plugin-go dependency, fix errors and switch equal method * function: Add validation for parameter name conflicts and update defaulting logic (#936) * add validation and refactor defaulting logic * add changelogs * test fix * Update website/docs/plugin/framework/functions/documentation.mdx Co-authored-by: Brian Flad <bflad417@gmail.com> * refactor the logic, tests and docs for defaulting * Update website/docs/plugin/framework/functions/documentation.mdx Co-authored-by: Benjamin Bennett <ben.bennett@hashicorp.com> --------- Co-authored-by: Brian Flad <bflad417@gmail.com> Co-authored-by: Benjamin Bennett <ben.bennett@hashicorp.com> * resource/schema: Ensure invalid attribute default value errors are raised (#933) Reference: #590 Reference: #930 Previously the logic handling attribute `Default` values would silently ignore any type errors, which would lead to confusing planning data behaviors. This updates the logic to raise those error properly and adds covering unit testing. These error messages are using the underlying `tftypes` type system errors which is currently a pragmatic compromise throughout various parts of the framework logic that bridges between both type systems to save additional type assertion logic and potential bugs relating to those conversions. In the future if the internal `tftypes` handling and exported fields are replaced with the framework type system types, this logic would instead return error messaging based on the framework type system errors. This also will enhance the schema validation logic to check any `Default` response value and compare its type to the schema, which will raise framework type system errors during the `GetProviderSchema` RPC, or during schema unit testing if provider developers have implemented that additional testing. * Update provider functions testing docs to help users avoid nil pointer error (#940) * Fix capitalisation * Update example test to mitigate nil pointer error See #928 * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> --------- Co-authored-by: Austin Valle <austinvalle@gmail.com> Co-authored-by: Brian Flad <bflad417@gmail.com> * Reinstate go toolchain and add changelog for Go version bump to 1.21 (#937) * Reinstate go toolchain and add changelog for Go version bump to 1.21 * Adding changelog * Updating changelog * Squashed commit of the following: commit e7415b7 Author: Benjamin Bennett <ben.bennett@hashicorp.com> Date: Fri Mar 1 16:16:39 2024 +0000 Reinstate go toolchain and add changelog for Go version bump to 1.21 (#937) * Reinstate go toolchain and add changelog for Go version bump to 1.21 * Adding changelog * Updating changelog commit 1597a95 Author: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Fri Mar 1 16:12:22 2024 +0000 Update provider functions testing docs to help users avoid nil pointer error (#940) * Fix capitalisation * Update example test to mitigate nil pointer error See #928 * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> * Update website/docs/plugin/framework/functions/testing.mdx Co-authored-by: Austin Valle <austinvalle@gmail.com> --------- Co-authored-by: Austin Valle <austinvalle@gmail.com> Co-authored-by: Brian Flad <bflad417@gmail.com> commit bd22b58 Author: Brian Flad <bflad417@gmail.com> Date: Fri Mar 1 07:20:55 2024 -0500 resource/schema: Ensure invalid attribute default value errors are raised (#933) Reference: #590 Reference: #930 Previously the logic handling attribute `Default` values would silently ignore any type errors, which would lead to confusing planning data behaviors. This updates the logic to raise those error properly and adds covering unit testing. These error messages are using the underlying `tftypes` type system errors which is currently a pragmatic compromise throughout various parts of the framework logic that bridges between both type systems to save additional type assertion logic and potential bugs relating to those conversions. In the future if the internal `tftypes` handling and exported fields are replaced with the framework type system types, this logic would instead return error messaging based on the framework type system errors. This also will enhance the schema validation logic to check any `Default` response value and compare its type to the schema, which will raise framework type system errors during the `GetProviderSchema` RPC, or during schema unit testing if provider developers have implemented that additional testing. commit f03ca33 Author: Austin Valle <austinvalle@gmail.com> Date: Thu Feb 29 14:23:29 2024 -0500 function: Add validation for parameter name conflicts and update defaulting logic (#936) * add validation and refactor defaulting logic * add changelogs * test fix * Update website/docs/plugin/framework/functions/documentation.mdx Co-authored-by: Brian Flad <bflad417@gmail.com> * refactor the logic, tests and docs for defaulting * Update website/docs/plugin/framework/functions/documentation.mdx Co-authored-by: Benjamin Bennett <ben.bennett@hashicorp.com> --------- Co-authored-by: Brian Flad <bflad417@gmail.com> Co-authored-by: Benjamin Bennett <ben.bennett@hashicorp.com> * fix dynamic param * incorrect merge conflict fix :) * update comments from feedback * refactor validation logic * license headers * implement remaining resource and datasource validation * implement provider schema validation * update error msg and add object validator * add validation to object attributes * update existing attributes to use new bool return * add validation to function parameters and definitions * refactor to only have one exported function * add tuple tests for completeness * create new fwtype package * add parameter name to validate implementation definition * various PR fixes * add more docs * fix docs from default param change * update comment * add more to reflection case + update comment * comment wording * remove todos and add package docs * add changelogs * Fix the use-case where dynamic value type is known, but the value itself is still null/unknown * check for dynamic underlying value for null/unknown detection * removed the unneccessary interface and removed half-implemented reflection support * doc formatting * update static default naming * add happy path tests for datasource + provider validate implementations * update definition validation messaging with variadic * add doc explicitly saying that dynamic types aren't supported * add docs mentioning required dynamic type to functions * prevent nil panics in `types.Dynamic` implementation w/ tests * proposal for `NewDynamicValue` * add recommendations in doc string * update block msg * update all doc strings and error messages to make recommendations * move the function validate interfaces to internal package * add maintainer note about parameter name * prevent attribute paths from stepping into dynamic types + new tests * add helper methods for checking underlying value unknown/null * add tests and comments about edge case with empty values * move maintainer note * Add dynamic type support documentation and considerations * first pass at dynamic documentation * Apply suggestions from code review Co-authored-by: Brian Flad <bflad417@gmail.com> * update literal representation in attribute page * drop callouts and link to collection type page * Update website/docs/plugin/framework/handling-data/attributes/dynamic.mdx Co-authored-by: Brian Flad <bflad417@gmail.com> * Apply suggestions from code review Co-authored-by: Brian Flad <bflad417@gmail.com> * Update website/docs/plugin/framework/functions/parameters/dynamic.mdx Co-authored-by: Brian Flad <bflad417@gmail.com> * Update website/docs/plugin/framework/handling-data/dynamic-data.mdx Co-authored-by: Brian Flad <bflad417@gmail.com> * include parameter name in example * adjust the path documentation to reflect new changes * rework the first paragraph of the data page * add callout for null underlying element values * add underlying value null/unknown information * Update website/docs/plugin/framework/handling-data/types/dynamic.mdx Co-authored-by: Brian Flad <bflad417@gmail.com> * Update website/docs/plugin/framework/handling-data/dynamic-data.mdx Co-authored-by: Brian Flad <bflad417@gmail.com> * add float64 and number notes --------- Co-authored-by: Brian Flad <bflad417@gmail.com> --------- Co-authored-by: Benjamin Bennett <ben.bennett@hashicorp.com> Co-authored-by: hc-github-team-tf-provider-devex <github-team-tf-provider-devex@hashicorp.com> Co-authored-by: John Behm <jxsl13@googlemail.com> Co-authored-by: Brian Flad <bflad417@gmail.com> Co-authored-by: Sarah French <15078782+SarahFrench@users.noreply.github.com> * delete duplicate tests and add name params * remove duplicate from merge conflict * Apply suggestions from code review Co-authored-by: Brian Flad <bflad417@gmail.com> --------- Co-authored-by: Brian Flad <bflad417@gmail.com> Co-authored-by: Austin Valle <austinvalle@gmail.com> Co-authored-by: hc-github-team-tf-provider-devex <github-team-tf-provider-devex@hashicorp.com> Co-authored-by: John Behm <jxsl13@googlemail.com> Co-authored-by: Sarah French <15078782+SarahFrench@users.noreply.github.com>
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. |
Reference: https://github.com/hashicorp/terraform-providers-devex-internal/issues/175
This PR contains changes for replacing the returning of diagnostics with function errors during the execution of the
CallFunction
RPC and the associatedRun
method defined within provider-defined functions.A new error type,
function.FuncError
has been introduced.This will require that provider developers alter the implementation of any previously implemented provider-defined functions as the response type that is returned by the
Run
method has been altered.Currently, the structure of the
RunResponse
type is:The changes contained within this PR have restructured the
RunResponse
in the following way:This will require that provider-defined function implementations that currently interact with diagnostics will need to be altered to interact with a function error.
Context
The motivation behind the changes in this PR arise from a desire to keep the data that flows between Terraform core and providers during the
CallFunction
RPC aligned with the native cty interface that is used by Terraform core. There is an associated PR on Terraform core for making equivalent changes on the core side of the protocol.