Skip to content

Experiment: type constraints for output values#31728

Closed
apparentlymart wants to merge 4 commits intomainfrom
f-output-value-types
Closed

Experiment: type constraints for output values#31728
apparentlymart wants to merge 4 commits intomainfrom
f-output-value-types

Conversation

@apparentlymart
Copy link
Copy Markdown
Contributor

@apparentlymart apparentlymart commented Sep 1, 2022

As requested in #30579, this PR introduces experimental support for declaring explicit type constraints in output blocks, using the same type constraint syntax we already use for input variables. The experiment keyword is output_type_constraints.

output "example" {
  type  = string
  value = local.example
}

The goal of merging this would be to be able to use subsequent alpha releases (once we update our release process to enable experiments for them) to be used as a vehicle both for evaluating the potential interest in a feature like this and for learning if a new argument inside the output block is the most ideal design for it. It would not be available in non-alpha releases until stabilized, and so merging this PR is not sufficient to close the associated issue.


I already have a few design questions just as a result of implementing the experiment:

  • One of the goals expressed in Ability to set "type" on module output #30579 is to use an output value's type constraint as part of its documentation, so that consumers of the module can clearly see what type of value will be returned.

    However, our type constraint syntax is oriented towards the person providing a value rather than the person consuming it. While I can certainly imagine it being convenient to the author of the module to use optional attributes and default values, it isn't clear to me that this information is useful as documentation for the recipient of a value: the decision of whether to assign a value directly or to rely on automatic insertion during type conversion is an implementation detail of the module, not part of its interface.

    Would it make sense to exclude the ability to specify optional attributes and/or default values from output value type constraints? Or alternatively, could we build something in terraform-config-inspect (our main interface for use-cases such as docs generation) to "simplify" the specified type constraint to strip out all of the optional annotations and just describe the type?

  • When we implemented optional attributes for input variables we were constrained by compatibility to still allow explicitly assigning null to an attribute not declared as optional, even though in all other senses we aim to treat "null" as exactly equivalent to unset.

    Since this is a green field feature, would it make sense to use a stricter interpretation of "optional" here such that we can guarantee to the recipient of the value that an output value which isn't declared as optional will never be null? Although that would serve purely as documentation today, it could potentially benefit us down the road to have a static promise that a particular attribute can never be null as we work to reduce the number of situations where a derived expression is forced to return unknown.

    On the other hand, this would end up being just a special case for object attributes unless we accompanied it with some other features: we have no current syntax to declare that an output value as a whole is guaranteed not to be null, and no syntax to declare that elements of a collection cannot be null. We may just need to accept that our language is not one that is amenable to static analysis of the "nullable" property.

  • Anything we add for output values always invites the question of whether we should also support it for local values, but in this case I think that would just reduce to the existing idea of having e.g. a convert(value, typeexpr) function which can be used in any context.

    I'm still keen on that idea, and it's true that if it were present then it could be used in output values as an alternative to this explicit type argument, so we should consider whether the convert function would be a more general/orthogonal answer to this problem.

    With that said: we still don't have a fully-specified plan for how to deal with making the keywords like string, bool, any valid in a function argument position without a breaking language change, and also an explicit type argument is more practical to extract using syntax-only analysis in terraform-config-inspect and other potential "shallow integrations" that aren't capable of full expression evaluation. I think if we were to adopt convert as the solution to this request then we'd need to at least show that it's viable for terraform-config-inspect to extract a type constraint for use in documentation.

As long as this remains an experiment I don't think answering these questions is a blocker. Indeed, if we move foward I expect that subsequent discussion will yield additional questions for us to consider.

@vercel
Copy link
Copy Markdown

vercel bot commented Sep 1, 2022

Deployment failed with the following error:

Invalid website/vercel.json file provided

@alisdair
Copy link
Copy Markdown
Contributor

alisdair commented Sep 2, 2022

You may want to rebase this now that the PR removing our fork of typeexpr has been merged. Nothing ought to have changed except the import path.

This doesn't do anything yet, but subsequent commits will use it as a gate
for a possible new language feature where module authors can specify a
type constraint for an output value.
Normally when we decode a module that uses experiments we emit a warning
to alert the user that they are operating outside of our usual
compatibility promises. Previously our "valid files" tests would treat
such warnings as failure conditions.

We'll now make an exception that this particular kind of warning is not
counted as non-success, and also explicitly enable the ability to use
language experiments on some of the parsers in other tests that also
evaluate the "valid files" test fixtures so that we can use experiments
there without generating errors.
This uses the same type constraint expression syntax as we use in the
argument of the same name in variable blocks. It doesn't do anything yet,
but in subsequent commits it should become similar in purpose to type
constraints on variables, allowing module authors to specify exactly how
their result types are guaranteed to be constrained and have Terraform
automatically convert the result to match the given constraint.
The ability to set actually set a type constraint on an output value is
currently guarded by the "output_type_constraints" experiment opt-in, so
any non-experiment-using configuration will always have ConstraintType
set to cty.DynamicPseudoType as shown in some of the unit tests updated
here.

Those who opt in to the experiment can specify a type constraint and
defaults using exactly the same syntax as for input variables, and
essentially get the same effect but in reverse: the type constraint and
defaults get applied at the boundary where the value is leaving the
module where it's declared, as opposed to when it's entering into a module
as for input variables.
@apparentlymart
Copy link
Copy Markdown
Contributor Author

Thanks for the note about that, @alisdair! I've now belatedly rebased as you suggested. It still seems to be working against the upstreamed implementation of "typeexpr".

@apparentlymart apparentlymart requested a review from a team November 30, 2022 02:19
@apparentlymart
Copy link
Copy Markdown
Contributor Author

apparentlymart commented Mar 10, 2023

While working on something else I realized a further benefit of having explicit type constraints on output values that we can only realize if it's present in the first release that supports it:

If all of the output values for a given module have explicit type constraints and all of them are exact then it becomes possible to predict the exact type of the object representing all of the output values, which means that if module.whatever is unknown for any reason then the evaluator would be able to return an unknown value of a specific object type, a map of a specific object type, or a list of a specific object type depending whether the module call uses count or for_each.

This is not possible today because output values are always dynamically-typed and so it's never safe to assume the final type of an output value whose type is not already concrete.

It's important to include this in the first release if we ever want to do it because any existing module that works today effectively has type = any under this change but as soon as folks start writing explicit type constraints for output values the data type we use to represent an entire module using those constraints cannot change from object to map or tuple to list without that being a potential breaking change. If we implement it in the first release then specifying type exactly across all output values will serve as the opt-in for this new behavior, and no existing modules will include it and so they cannot possibly be affected by the change.

@flisboac
Copy link
Copy Markdown

flisboac commented Aug 25, 2023

Any news from this? I'm very interested in this functionality. It makes intention much clearer, and avoids the need for excessive (written) documentation.

Is there an ETA/milestone for this?

@apparentlymart
Copy link
Copy Markdown
Contributor Author

Nobody on the Terraform team at HashiCorp is currently working on this idea. For me personally, I'm currently assigned to something else and so won't be able to work on this more in the immediate future.

Since this is just an experiment/prototype this PR will probably never be merged as it is, and instead there'd be another PR that aims to build a shippable version. For that reason I'd suggest voting on #30579 to express interest in this, rather than voting/commenting here. I'm going to lock this conversation for now just to encourage keeping the voting/discussion in the central issue so that others on our team can see it (since only I will get notifications about this PR).

Thanks!

@hashicorp hashicorp locked and limited conversation to collaborators Aug 25, 2023
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