-
Notifications
You must be signed in to change notification settings - Fork 92
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
internal/fwserver - Fix DataSource/Resource Type Not Found
errors when GetProviderSchema
is not called
#852
Conversation
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.
Just a couple of minor comments.
@@ -44,15 +42,6 @@ func (s *Server) GetMetadata(ctx context.Context, req *GetMetadataRequest, resp | |||
resp.Resources = []ResourceMetadata{} | |||
resp.ServerCapabilities = s.ServerCapabilities() | |||
|
|||
metadataReq := provider.MetadataRequest{} |
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.
Should we call s.ProviderTypeName()
to set providerTypeName
in order to preserve original behaviour?
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 also wondered this, it felt weird to make the call here to populate a cache used somewhere else. If we do want to preserve this, we could maybe change the naming up:
s.SetProviderTypeName()
But I went for the "lazy-load" approach 😆
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.
Just a clarification, the overall behavior of GetMetadata
+ GetProviderSchema
is still preserved as there already exists a test for this:
"datasources-provider-type-name": { |
The implementation detail has just been moved closer to where it's needed for the other RPCs to consume as well 👍🏻
An alternative approach could be to set s.ProviderTypeName()
at the top of every RPC
@@ -31,15 +29,6 @@ type GetProviderSchemaResponse struct { | |||
func (s *Server) GetProviderSchema(ctx context.Context, req *GetProviderSchemaRequest, resp *GetProviderSchemaResponse) { | |||
resp.ServerCapabilities = s.ServerCapabilities() | |||
|
|||
metadataReq := provider.MetadataRequest{} |
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.
Should we call s.ProviderTypeName()
to set providerTypeName
in order to preserve original behaviour?
DataSource/Resource Type Not Found
errors in Terraform 1.6DataSource/Resource Type Not Found
errors in Terraform 1.6
DataSource/Resource Type Not Found
errors in Terraform 1.6DataSource/Resource Type Not Found
errors when GetProviderSchema
is not called
I'm not sure the best way to add a CI test for this functionality (due to the way this bug is exposed), any suggestions for improving the test coverage of this is appreciated. I'm just been running manual tests with a sandbox provider comparing Before $ go list -m github.com/hashicorp/terraform-plugin-framework/...
github.com/hashicorp/terraform-plugin-framework v1.4.0
$ terraform1.6 -chdir=examples/resources apply -auto-approve
╷
│ Warning: Provider development overrides are in effect
│
│ The following provider development overrides are set in the CLI configuration:
│ - austinvalle/sandbox in /Users/austin.valle/go/bin
│
│ The behavior may therefore not match any released version of the provider and applying changes may
│ cause the state to become incompatible with published releases.
╵
╷
│ Error: Resource Type Not Found
│
│ with examplecloud_thing.this,
│ on resource.tf line 8, in resource "examplecloud_thing" "this":
│ 8: resource "examplecloud_thing" "this" {
│
│ No resource type named "examplecloud_thing" was found in the provider.
╵
╷
│ Error: Data Source Type Not Found
│
│ with data.examplecloud_thing.this,
│ on resource.tf line 12, in data "examplecloud_thing" "this":
│ 12: data "examplecloud_thing" "this" {
│
│ No data source type named "examplecloud_thing" was found in the provider.
╵ After $ go get -u github.com/hashicorp/terraform-plugin-framework@8a471749f9833cc8e7bb0355d436a23782e1c9a4
go: upgraded github.com/hashicorp/terraform-plugin-framework v1.4.0 => v1.4.1-0.20231006203353-8a471749f983
$ go mod tidy
$ go install .
$ terraform1.6 -chdir=examples/resources apply -auto-approve
╷
│ Warning: Provider development overrides are in effect
│
│ The following provider development overrides are set in the CLI configuration:
│ - austinvalle/sandbox in /Users/austin.valle/go/bin
│
│ The behavior may therefore not match any released version of the provider and applying changes may
│ cause the state to become incompatible with published releases.
╵
data.examplecloud_thing.this: Reading...
examplecloud_thing.this: Refreshing state...
data.examplecloud_thing.this: Read complete after 0s
No changes. Your infrastructure matches the configuration.
Terraform has compared your real infrastructure against your configuration and found no differences, so
no changes are needed.
Apply complete! Resources: 0 added, 0 changed, 0 destroyed. |
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 🚀
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. |
Closes #853
Terraform 1.6 no longer calls
GetProviderSchema
before any of the other RPCs, which currently populatess.providerTypeName
. Without this populated, data sources + resources are added to their respective caches without the provider prefix, i.e._thing
instead of the correctexamplecloud_thing
, resulting in a confusingNot Found
error.Notes
Unfortunately, this can't be recreated easily in CI because
terraform-plugin-testing
manages the plugin processes similar to debug mode, so the plugin keeps the provider running, whereas terraform core does not. Therefore, during a test run,s.providerTypeName
happens to be populated due to a priorGetProviderSchema
call populating it.See: https://developer.hashicorp.com/terraform/plugin/debugging#debugging-caveats