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

configs: Experimental support for optional object type attributes #26540

Merged
merged 1 commit into from
Oct 12, 2020

Conversation

apparentlymart
Copy link
Contributor

@apparentlymart apparentlymart commented Oct 9, 2020

This builds on an experimental feature in the underlying cty library which allows marking specific attribtues of an object type constraint as optional, which in turn modifies how the cty conversion package handles missing attributes in a source value: it will silently substitute a null value of the appropriate type rather than returning an error. This is similar to the ability to silently convert a bool true into the string "true": rather than returning an error, cty will just silently insert a suitably-typed null value as the value of each optional attribute that isn't in the input object.

This new language feature aims to address the following two common situations:

  • If a module author needs to extend an object type constraint with a new attribute in order to support a new feature, making that attribute optional allows the change to be backward-compatible with previous releases. In today's Terraform language, avoiding a breaking change would require adding an entirely new variable alongside the old one.
  • When a module is creating an abstraction that has some optional features, it may work well to use the presence of an optional attribute as the opt-in for such a feature. In today's Terraform language, authors must either use a separate variable to represent activating an optional feature or module callers must explicitly write attribute = null to conform to the object type constraint while leaving an attribute unset.

In order to implement the experiment this commit temporarily forks the HCL typeexpr extension package into a local internal/typeexpr package, where I've extended the type constraint syntax to allow annotating object type attributes as being optional using the HCL function call syntax. If the experiment is successful -- both at the Terraform layer and in the underlying cty library -- I expect to send these modifications to upstream HCL so that other HCL-based languages can potentially benefit from this new capability. If the experiment is unsuccessful then we'll just delete the local fork and switch back to the upstream typeexpr package. Either way, the internal/typeexpr package should not survive into the 0.15 release.

Because it's experimental, the optional attribute modifier is allowed only with an explicit opt-in to the module_variable_optional_attrs experiment. The opt-in is required only for a module that is declaring an optional attribute in an object type constraint; callers of such a module are not required to opt-in directly themselves, although calling into a module with the experiment activated will still generate the expected warnings about the configuration using an experimental feature.

My intent is for this to be available as an experiment throughout the v0.14 release line to gather feedback and then, if successful, to stabilize something similar in the forthcoming v0.15 release after adjusting for feedback.

This builds on an experimental feature in the underlying cty library which
allows marking specific attribtues of an object type constraint as
optional, which in turn modifies how the cty conversion package handles
missing attributes in a source value: it will silently substitute a null
value of the appropriate type rather than returning an error.

In order to implement the experiment this commit temporarily forks the
HCL typeexpr extension package into a local internal/typeexpr package,
where I've extended the type constraint syntax to allow annotating object
type attributes as being optional using the HCL function call syntax.
If the experiment is successful -- both at the Terraform layer and in
the underlying cty library -- we'll likely send these modifications to
upstream HCL so that other HCL-based languages can potentially benefit
from this new capability.

Because it's experimental, the optional attribute modifier is allowed only
with an explicit opt-in to the module_variable_optional_attrs experiment.
@codecov
Copy link

codecov bot commented Oct 9, 2020

Codecov Report

Merging #26540 into master will decrease coverage by 0.00%.
The diff coverage is 66.66%.

Impacted Files Coverage Δ
configs/named_values.go 67.88% <ø> (ø)
configs/experiments.go 72.72% <66.66%> (-2.69%) ⬇️
internal/providercache/dir.go 65.30% <0.00%> (-6.13%) ⬇️
terraform/node_resource_plan.go 95.32% <0.00%> (-1.87%) ⬇️
states/statefile/version4.go 57.96% <0.00%> (-0.24%) ⬇️

Copy link
Contributor

@mildwonkey mildwonkey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😍 This is delightful, and my only "concern" is that it's going to be hugely popular and so we'll be in trouble if we end up removing it in 0.15 - not that I'm very concerned that will happen!

@apparentlymart
Copy link
Contributor Author

@mildwonkey that is, of course, a hazard with all experiments. 😄

With that said, the situation that would cause this to not be stablized in 0.15 is if there is feedback that shows the design is completely wrong and that returning to the drawing board is required, in which case it'll inform a second round of design rather than causing the underlying use-cases to be dropped altogether.

@ghost
Copy link

ghost commented Nov 12, 2020

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.

@ghost ghost locked as resolved and limited conversation to collaborators Nov 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants