Allow variables to be explicitly typed (and enforced) #3167
Conversation
|
I have the new behavior all wired in and it's looking good. I'll push them tonight after revisiting the tests once more to ensure I didn't miss anything. |
There was a problem hiding this comment.
Pull Request Overview
This PR enables explicit variable typing and enforces type conversion for variable overrides, following HCL's type expression syntax.
- Introduces a new "type" field in variable declarations to support explicit typing.
- Updates the variable resolution flow to apply JSON and CSV conversions based on the specified type.
- Adds helper functions to parse CSV values and to resolve type constraints with fallback handling.
This allows variables to have explicit types, similar to Terraform variables. It uses HCL's `typeexpr` extension for the specification. For conversion of overrides to complex types (when explicit typing is provided), HCL's native JSON-based unmarshalling is used. Typing is independent of any default, but if a default is provided, it will be validated. Similarly, if an override is provided, it will be converted to that type. When typing is not provided, previous behavior is used, namely passing through as a string when no default, converting to primitives if the default was primitive, and failing otherwise (complex types). For complex types, the happy path is lists of primitives, but in theory any complex/composite type can be used provided they are expressed correctly in JSON. In the interest of simplicity and correctness, there are no shortcuts for lists. There *is* a shortcut for strings as users don't provide them for untyped variables and would be unintuitive. Signed-off-by: Roberto Villarreal <[email protected]>
Though CSV is favored for 'simple' lists, a JSON value will be used if it parses without error. This assumes that it is extremely unlikely that something that parses as JSON would be intended to be parsed as CSV, e.g. `["a"` and `"b"]`, as opposed to `a` and `b`. If parsing/conversion fails, it is treated as if it was a CSV. Since the CSV approach required processing of each element, code was refactored to reuse the same logic used for individual non-typed variables. Signed-off-by: Roberto Villarreal <[email protected]>
The primary intent is to make JSON parsing explicitly opt-in rather than using heuristics to determine intent. With some exceptions, given bake variable `VAR`, an environment variable `VAR_JSON` must be used to provide JSON content. The value in `VAR_JSON` will be ignored when: * a bake built-in of that same name exists * a user-provided variable of that same name exists * typing (attribute `type`) is not present The first is unlikely to happen as built-ins will likely start with `BUILDX_BAKE_`, an unlikely prefix for end users. The second may be a real scenario, where users have `VAR_JSON` dedicated to accepting a string with JSON content and decoding via an HCL function. This will continue to work as-is, but can be simplified by removing the variable from their bake file (`VAR_JSON`) and applying typing (to `VAR`). Signed-off-by: Roberto Villarreal <[email protected]>
Co-authored-by: Copilot <[email protected]> Signed-off-by: Roberto Villarreal <[email protected]>
Signed-off-by: Roberto Villarreal <[email protected]>
There was a problem hiding this comment.
Looks good thanks!
We should update the documentation to reflect this new type constraint.
In reference docs https://github.com/docker/buildx/blob/master/docs/bake-reference.md#variable we could have something similar to https://developer.hashicorp.com/terraform/language/values/variables#type-constraints
And in manual docs https://github.com/docker/docs/blob/main/content/manuals/build/bake/variables.md we could have some examples like https://developer.hashicorp.com/terraform/language/expressions/types#types
Does "we" mean me? I can give it a shot. (I didn't know whether you folks had dedicated technical writers and/or if docs followed a separate release process, so figured I'd wait for clarity.) |
|
@rrjjvv You can open up a docs update in follow-up if you can. We will ping the necessary people for review. |
Resolves #3160 and likely replaces #3161
In #3160 (comment) it was suggested that prior to allowing variable lists to be overridden via environment, proper typing capabilities should be implemented (and used). This implements that typing, which results in the actual override code being superior to #3161 in every way, but touches more code.
This allows (but does not require) variables to have explicit types, similar to Terraform variables. It uses HCL's
typeexprextension for the specification. For conversion of overrides to complex types (when explicit typing is provided), HCL's native JSON-based unmarshalling is used.Typing is independent of any default value, but if a default is provided, it will be validated. Similarly, if an override is provided, it will be converted to that type. If typing is not provided, previous behavior is used.
For complex types, the happy path is lists of primitives, but in theory any complex/composite type can be used provided they are expressed correctly in JSON. In the interest of simplicity and correctness, there are no shortcuts for lists (i.e.,
VAR='["foo","bar"]'and notVAR=foo,bar). There is a shortcut for strings as users don't provide them for untyped variables and would be unintuitive (i.e.VAR=foois valid for a string, though not valid JSON).There should be no changes in behavior (for current or future un-typed usage), with one theoretical exception. Today, extraneous attributes (for variables at least) are accepted, but ignored with no warnings. It is theoretical possibly that users may have become reliant on this, e.g.
Anyone using that attribute for their own purposes will encounter breaking behavior with this implementation (unless it happens to be a legitimate type expression). I don't know if that is a real concern.
Also, this PR only contains code changes. I assumed that 1) this change was unlikely to go through on the first submission and may require changes (or, of course, rejected), and 2) you may have dedicated people/processes for updating user docs. Let me know if you want/need me to do anything in this regard.