Skip to content
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

nil pointer exception when testing provider-defined functions #928

Closed
SarahFrench opened this issue Feb 22, 2024 · 2 comments · Fixed by #940
Closed

nil pointer exception when testing provider-defined functions #928

SarahFrench opened this issue Feb 22, 2024 · 2 comments · Fixed by #940
Labels
documentation Improvements or additions to documentation
Milestone

Comments

@SarahFrench
Copy link
Member

When testing provider-defined functions in a way that requires preparing a function.RunResponse input to the function under test, like this:

I get a nil pointer exception like this:

panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x40 pc=0x100474da0]

goroutine 8 [running]:
testing.tRunner.func1.2({0x103963300, 0x105921220})
        /Users/sarahfrench/sdk/go1.20.12/src/testing/testing.go:1526 +0x1c8
testing.tRunner.func1()
        /Users/sarahfrench/sdk/go1.20.12/src/testing/testing.go:1529 +0x384
panic({0x103963300, 0x105921220})
        /Users/sarahfrench/sdk/go1.20.12/src/runtime/panic.go:884 +0x204
github.com/hashicorp/terraform-plugin-framework/function.(*ResultData).Set(0x14000c0df48, {0x103f76c68, 0x14000190018}, {0x103719700, 0x140005cf4d0})
        /Users/sarahfrench/go/pkg/mod/github.com/hashicorp/terraform-plugin-framework@v1.5.0/function/result_data.go:38 +0x40
github.com/hashicorp/terraform-provider-google/google/functions.ProjectFromIdFunction.Run({}, {0x103f76c68, 0x14000190018}, {{{0x140005cf320?, 0x105961680?, 0x14000bb3f18?}}}, 0x14000c0df30)
        /Users/sarahfrench/go/src/github.com/hashicorp/terraform-provider-google/google/functions/project_from_id.go:61 +0x1ec
github.com/hashicorp/terraform-provider-google/google/functions.TestFunctionRun_project_from_id.func1(0x1400105f860)
        /Users/sarahfrench/go/src/github.com/hashicorp/terraform-provider-google/google/functions/project_from_id_internal_test.go:81 +0x6c
testing.tRunner(0x1400105f860, 0x1400108c730)
        /Users/sarahfrench/sdk/go1.20.12/src/testing/testing.go:1576 +0x10c
created by testing.(*T).Run
        /Users/sarahfrench/sdk/go1.20.12/src/testing/testing.go:1629 +0x368

and I can address the problem by updating my test to include this:

got := function.RunResponse{
	Result: function.NewResultData(basetypes.StringValue{}), // Required to avoid nil pointer error
}
@bflad
Copy link
Member

bflad commented Feb 22, 2024

Hi @SarahFrench 👋 Thank you for reporting this and sorry you ran into trouble here.

It looks like that documentation is specifying the function.RunResponse.Result for the test cases but not the got variable. If you are interested in submitting a documentation fix for that variable declaration to include the Result, we would be happy to accept it.

Cheers!

@bflad bflad added the documentation Improvements or additions to documentation label Feb 22, 2024
@bflad bflad added this to the v1.6.1 milestone Feb 28, 2024
SarahFrench added a commit to SarahFrench/terraform-plugin-framework that referenced this issue Feb 29, 2024
austinvalle added a commit that referenced this issue Mar 1, 2024
…r 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>
austinvalle added a commit that referenced this issue Mar 1, 2024
…r 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>
austinvalle added a commit that referenced this issue Mar 1, 2024
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>
austinvalle added a commit that referenced this issue Mar 20, 2024
* 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>
austinvalle added a commit that referenced this issue Mar 20, 2024
* 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>
bendbennett added a commit that referenced this issue Mar 21, 2024
* 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>
Copy link

github-actions bot commented Apr 1, 2024

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

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Improvements or additions to documentation
Projects
None yet
2 participants