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

Panic thrown if Optional or Required not set to true #31211

Closed
independentid opened this issue Jun 8, 2022 · 7 comments
Closed

Panic thrown if Optional or Required not set to true #31211

independentid opened this issue Jun 8, 2022 · 7 comments
Labels
bug new new issue not yet triaged waiting-response An issue/pull request is waiting for a response from the community

Comments

@independentid
Copy link

independentid commented Jun 8, 2022

Terraform Version

Terraform v1.2.2
on darwin_amd64
+ provider registry.terraform.io/hashicorp/google v4.24.0

Terraform Configuration Files

data "scaffolding_example" "test" {
  configurable_attribute = "example"
}

Debug Output

Expected Behavior

Expecting the modified example data source to work. Modification included an object set with "Required: false" + markdown and type.

Actual Behavior

Panic attack thrown when example_data_source_test.go run

Steps to Reproduce

  1. Instantiate a terraform plugin frameworks template.2.
  2. Modify the example_data_source.go code to add a new attribute test with markdown, type, and required set to false.
  3. Run the test and a panic attack will result4.
  4. Make sure all modifications include either a Required: true or Optional: true and the test will pass.5.

Additional Context

Nothing atypical. Took a while to find the issue.

Note: if you change the name of the provider, the provider_test.go must be updated or none of the tests will pass. This was not obvious in any documentation. Naming is hyper-sensitive to get plugin working. Additional diagnostics would be very helpful.

References

Unknown

@independentid independentid added bug new new issue not yet triaged labels Jun 8, 2022
@jbardin
Copy link
Member

jbardin commented Jun 8, 2022

Hi @independentid,

Can you provide the error output from Terraform so we can see where the panic might originate?

Thanks!

@jbardin jbardin added the waiting-response An issue/pull request is waiting for a response from the community label Jun 8, 2022
@apparentlymart
Copy link
Member

apparentlymart commented Jun 8, 2022

I believe that it's correct to raise an error if an attribute doesn't have at least one of Required, Optional, or Computed set to true, but of course it ought to be a proper error diagnostic and not an outright crash.

I suspect this flew under the radar so far because SDKv2 has its own validation checks for schema which can catch this before Terraform Core ever sees the schema, whereas the new framework exposes the Terraform Core schema primitives more directly and so is perhaps allowing this to pass through and expecting Terraform Core to check it. And it sounds like Terraform Core is checking it, but is not reporting it in a useful way, so I expect the fix will be to make it return a normal error diagnostic rather than to panic.

@independentid
Copy link
Author

independentid commented Jun 8, 2022

Here is the relevant crash info...

    hexa_policy_data_source_test.go:10: Step 1/1 error: Error running pre-apply refresh: exit status 11
        2022-06-08T11:40:14.303-0700 [TRACE] unmanaged.provider.stdio: waiting for stdio data
        
        !!!!!!!!!!!!!!!!!!!!!!!!!!! TERRAFORM CRASH !!!!!!!!!!!!!!!!!!!!!!!!!!!!
        
        Terraform crashed! This is always indicative of a bug within Terraform.
        Please report the crash with Terraform[1] so that we can fix this.
        
        When reporting bugs, please include your terraform version, the stack trace
        shown below, and any additional information which may help replicate the issue.
        
        [1]: https://github.com/hashicorp/terraform/issues
        
        !!!!!!!!!!!!!!!!!!!!!!!!!!! TERRAFORM CRASH !!!!!!!!!!!!!!!!!!!!!!!!!!!!
        
        runtime error: invalid memory address or nil pointer dereference
        goroutine 52 [running]:
        runtime/debug.Stack()
        	runtime/debug/stack.go:24 +0x65
        runtime/debug.PrintStack()
        	runtime/debug/stack.go:16 +0x19
        github.com/hashicorp/terraform/internal/logging.PanicHandler()
        	github.com/hashicorp/terraform/internal/logging/panic.go:55 +0x153
        panic({0x2f18c80, 0x4bb79e0})
        	runtime/panic.go:838 +0x207
        github.com/hashicorp/terraform/internal/plugin6/convert.ProtoToConfigSchema(0x0)
        	github.com/hashicorp/terraform/internal/plugin6/convert/schema.go:110 +0x52
        github.com/hashicorp/terraform/internal/plugin6/convert.ProtoToProviderSchema(...)
        	github.com/hashicorp/terraform/internal/plugin6/convert/schema.go:98
        github.com/hashicorp/terraform/internal/plugin6.(*GRPCProvider).GetProviderSchema(0xc000217780)
        	github.com/hashicorp/terraform/internal/plugin6/grpc_provider.go:164 +0x739
        github.com/hashicorp/terraform/internal/terraform.(*contextPlugins).ProviderSchema(0xc0007e73b0, {{0xc0000541f0, 0x4}, {0x32c0c30, 0x9}, {0x32e2659, 0x15}})
        	github.com/hashicorp/terraform/internal/terraform/context_plugins.go:97 +0x3a3
        github.com/hashicorp/terraform/internal/terraform.(*contextPlugins).ResourceTypeSchema(0xc0001a8a78?, {{0xc0000541f0, 0x4}, {0x32c0c30, 0x9}, {0x32e2659, 0x15}}, 0x44, {0xc0000541f0, 0xb})
        	github.com/hashicorp/terraform/internal/terraform/context_plugins.go:172 +0x59
        github.com/hashicorp/terraform/internal/terraform.(*AttachSchemaTransformer).Transform(0xc000853d20, 0xc0007d4b40)
        	github.com/hashicorp/terraform/internal/terraform/transform_attach_schema.go:65 +0x3ea
        github.com/hashicorp/terraform/internal/terraform.(*BasicGraphBuilder).Build(0xc0003a7850, {0x0, 0x0, 0x0})
        	github.com/hashicorp/terraform/internal/terraform/graph_builder.go:40 +0x29e
        github.com/hashicorp/terraform/internal/terraform.(*PlanGraphBuilder).Build(0x3330859?, {0x0, 0x0, 0x0})
        	github.com/hashicorp/terraform/internal/terraform/graph_builder_plan.go:74 +0x77
        github.com/hashicorp/terraform/internal/terraform.(*Context).Validate(0xc000216c80, 0xc0007e00e0)
        	github.com/hashicorp/terraform/internal/terraform/context_validate.go:63 +0x416
        github.com/hashicorp/terraform/internal/backend/local.(*Local).localRun(0xc0000e2500, 0xc0007b7e60)
        	github.com/hashicorp/terraform/internal/backend/local/backend_local.go:123 +0xab9
        github.com/hashicorp/terraform/internal/backend/local.(*Local).opRefresh(0xc0000e2500, {0x386a838, 0xc0007d4880}, {0x386a838, 0xc0007d48c0}, 0xc0007b7e60, 0xc0007d4840)
        	github.com/hashicorp/terraform/internal/backend/local/backend_refresh.go:48 +0x106
        github.com/hashicorp/terraform/internal/backend/local.(*Local).Operation.func1()
        	github.com/hashicorp/terraform/internal/backend/local/backend.go:323 +0xc3
        created by github.com/hashicorp/terraform/internal/backend/local.(*Local).Operation
        	github.com/hashicorp/terraform/internal/backend/local/backend.go:316 +0x43b
--- FAIL: TestAccHexaPolicyDataSource (0.45s)

FAIL

@independentid
Copy link
Author

I should comment as a newbie, I find it difficult to understand the difference between "Required" and "Optional" and how it impacts Terraform logic in particular. It seems like you just have to have something set to True (including "Computed".

@jbardin
Copy link
Member

jbardin commented Jun 8, 2022

Thanks @independentid! This should be taken care of by #31184, which will show the framework diagnostics when something is missing in the schema.

@jbardin jbardin closed this as completed Jun 8, 2022
@apparentlymart
Copy link
Member

apparentlymart commented Jun 8, 2022

@independentid, I can certainly understand that these three flags can be quite confusing at first. I'm actually not sure where in the new framework docs they are documented. There are some notes about them in the SDKv2 documentation and this is one aspect which both SDKs largely have in common because it's reflecting some assumptions that come from the provider protocol itself, regardless of SDK.

It looks like the Schemas section of the framework documentation covers similar information in a different structure.

In case it's helpful, here's a table I sometimes use to summarize the different valid combinations and what they mean, although this does omit some details you can find in those docs I linked above:

Required Optional Computed Meaning
Must be specified in the configuration, and provider must use the exact value given.
May be specified or omitted in the configuration. If specified, the provider must use the exact value given. If omitted, the value is always null.
May not be specified in the configuration. The provider alone decides the value during plan or apply.
May be specified or omitted in the configuration. If specified, the provider must use the exact value given. If omitted, the provider decides the value during plan or apply.

The above are the only valid combinations for these three flags in the current protocol.

If you run into other questions while you are working on this, you can ask about them in the Plugin SDK community forum, which covers both the old SDKv2 and the new framework.

@github-actions
Copy link

github-actions bot commented Jul 9, 2022

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 Jul 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug new new issue not yet triaged waiting-response An issue/pull request is waiting for a response from the community
Projects
None yet
Development

No branches or pull requests

3 participants