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

Inconsistent Optional w/ Default behavior #1185

Closed
tylerFowler opened this issue Apr 3, 2023 · 3 comments · Fixed by #1188
Closed

Inconsistent Optional w/ Default behavior #1185

tylerFowler opened this issue Apr 3, 2023 · 3 comments · Fixed by #1188
Labels
documentation Improvements or additions to documentation

Comments

@tylerFowler
Copy link

SDK version

v2.24.1

...

Relevant provider source code

schema.Schema{
  Type: schema.TypeString,
  Default:      "Some default",
  Optional:     true,
  Computed:     true,
}

Debug Output

resource my_resource: some_attribute: Default must be nil if computed

Expected Behavior

Per the SDK documentation:

        // Optional indicates whether the practitioner can choose to not enter
	// a value in the configuration for this attribute. Optional cannot be used
	// with Required.
	//
	// If also using Default or DefaultFunc, Computed should also be enabled,
	// otherwise Terraform can output warning logs or "inconsistent result
	// after apply" errors.
	Optional [bool](https://pkg.go.dev/builtin#bool)

Actual Behavior

This recommendation of supplying "Computed" when both "optional" and with a "default" seems to produce an error suggesting that "Computed" attributes must never have a default.

@tylerFowler tylerFowler added the bug Something isn't working label Apr 3, 2023
@bflad bflad added documentation Improvements or additions to documentation and removed bug Something isn't working labels Apr 10, 2023
@bflad
Copy link
Member

bflad commented Apr 10, 2023

Hi @tylerFowler 👋 Thank you for raising this and sorry you ran into this confusing contradiction in the Go documentation.

This additional documentation was added as part of #912 along with a bunch of other schema documentation changes. I'm guessing I was optimistically trying to document what Terraform normally expects from providers in this case, but didn't explicitly verify that it was actually possible in this SDK. This SDK sets a special flag when communicating with Terraform that enables some demotes some Terraform data consistency errors to warning logs. Newer SDKs, such as terraform-plugin-framework, intentionally do not set this special flag so provider developers are always aware of schema definition or data handling logic issues.

Rather than potentially removing the schema implementation validation logic for this:

if v.Computed && v.Default != nil {
	return fmt.Errorf("%s: Default must be nil if computed", k)
}

And have other potential unexpected behaviors introduced for provider developers if they do try to set Computed: true and Default: ..., the safer option here will unfortunately be removing the invalid documentation from the Optional field to match the current implementation.

bflad added a commit that referenced this issue Apr 10, 2023
… Go documentation

Reference: #1185

While the intention of the Go documentation matches Terraform's expectations, other parts of the SDK explicitly validate against the schema definition it was recommending. This documentation change is safer than trying to reconcile all the potential behavior changes should this particular validation be removed and a developer attempts to set `Computed`.
bflad added a commit that referenced this issue Apr 10, 2023
… Go documentation (#1188)

Reference: #1185

While the intention of the Go documentation matches Terraform's expectations, other parts of the SDK explicitly validate against the schema definition it was recommending. This documentation change is safer than trying to reconcile all the potential behavior changes should this particular validation be removed and a developer attempts to set `Computed`.
@tylerFowler
Copy link
Author

This is totally fine, just wanted to ensure that the version of the logic is in fact the correct one. My changeset that led me to this was one in response to seeing messages in Terraform logs like "provider xxxx unexpectedly produced new value for attribute Y ..."

@github-actions
Copy link

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 May 11, 2023
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