[HPR-817] command/validate: add option to eval datasources #12106
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lbajolet-hashicorp
force-pushed
the
validate_add_eval_datasources_opt
branch
from
6dd169c
to
97cb140
Compare
nywilken
reviewed
Nov 11, 2022
command/cli.go
Outdated
| @@ -129,14 +129,16 @@ type FixArgs struct { | |||
|
|
|||
| func (va *ValidateArgs) AddFlagSets(flags *flag.FlagSet) { | |||
| flags.BoolVar(&va.SyntaxOnly, "syntax-only", false, "check syntax only") | |||
| flags.BoolVar(&va.EvaluateDatasources, "evaluate-datasources", false, "evaluate datasources for validation (may incur costs)") | |||
There was a problem hiding this comment.
I'm not sure on the copy but should we indicate that this is for HCL2 templates only?
There was a problem hiding this comment.
I can add this in the message sure, doesn't hurt to be precise
nywilken
approved these changes
Nov 11, 2022
lbajolet-hashicorp
force-pushed
the
validate_add_eval_datasources_opt
branch
from
97cb140
to
6519070
Compare
|
With the latest validation option changes this PR needs to be rebased. We also need to update the documented flags for the validate command. https://developer.hashicorp.com/packer/docs/commands/validate I suggest adding more details to the flag description about possibly incurring cost for some datasource evaluations. |
nywilken
changed the title
lbajolet-hashicorp
force-pushed
the
validate_add_eval_datasources_opt
branch
from
6519070
to
507f54e
Compare
lbajolet-hashicorp
force-pushed
the
validate_add_eval_datasources_opt
branch
from
507f54e
to
c52af4a
Compare
nywilken
changed the title
nywilken
approved these changes
Nov 16, 2022
Comment on lines
62
to
69
| - `-warn-on-undeclared-var` - Warn when the template assigns undeclared | ||
| variables. This is especially relevant for HCL2 templates with .pkrvars.hcl | ||
| dependencies, as defining a value for a variable is not enough on its own for | ||
| Packer to function, there also needs to be some variable definitions in the | ||
| template. However, for some cases, templates may only declare what they locally | ||
| need for building, and reference a global variable file with more variable | ||
| assignations than there are definitions. This would yield a warning for each | ||
| assigned and non-declared variable. This option will enable those warnings. |
There was a problem hiding this comment.
Small copy change. What do you think?
Suggested change
| - `-warn-on-undeclared-var` - Warn when the template assigns undeclared | |
| variables. This is especially relevant for HCL2 templates with .pkrvars.hcl | |
| dependencies, as defining a value for a variable is not enough on its own for | |
| Packer to function, there also needs to be some variable definitions in the | |
| template. However, for some cases, templates may only declare what they locally | |
| need for building, and reference a global variable file with more variable | |
| assignations than there are definitions. This would yield a warning for each | |
| assigned and non-declared variable. This option will enable those warnings. | |
| - `-warn-on-undeclared-var` - Setting this flag will yield a warning for each assignment within a variable definitions file (*.pkrvars.hcl | *.pkrvars.json) that does not have an accompanying variable block. This can occur when using a var-file that contains a large amount of unused variables for a given HCL2 template. For HCL2 template builds defining a value for a variable in a var-file is not enough on its own for Packer to function, as there also needs to be a variable block definition in the template files `pkr.hcl` for the variable. By default `packer build` will not warn when a var-file contains one or more undeclared variables. |
Comment on lines
34
to
41
| - `-evaluate-datasources` - Evaluate the datasources when validating the template. | ||
| This is only valid on HCL2 templates, since JSON templates do not feature | ||
| datasources, this option will be ignored. Note that because datasources may rely | ||
| on external services for fetching data, this will incur some costs at validation | ||
| when the services being contacted are billing for the operation. | ||
|
|
There was a problem hiding this comment.
Slight reword with a call out for the billing warning
Suggested change
| - `-evaluate-datasources` - Evaluate the datasources when validating the template. | |
| This is only valid on HCL2 templates, since JSON templates do not feature | |
| datasources, this option will be ignored. Note that because datasources may rely | |
| on external services for fetching data, this will incur some costs at validation | |
| when the services being contacted are billing for the operation. | |
| - `-evaluate-datasources` - Evaluate all data sources when validating a template. | |
| This is only valid on HCL2 templates, since JSON templates do not feature | |
| data sources. For JSON templates this flag will be ignored. | |
| ~> **Warning:** Data sources may rely on external services for fetching data, which can incur some costs at validation if the services being contacted are billing per operation. |
Comment on lines
48
to
58
| - `-no-warn-on-undeclared-var` - Silence warnings when the template assigns | ||
| undeclared variables. This is especially relevant for HCL2 templates with | ||
| .pkrvars.hcl dependencies, as defining a value for a variable is not enough on | ||
| its own for Packer to function, there also needs to be some variable definitions | ||
| in the template. However, for some cases, templates may only declare what they | ||
| locally need for building, and reference a global variable file with more variable | ||
| assignations than there are definitions. This would yield a warning for each | ||
| assigned and non-declared variable. This option will disable those warnings. | ||
|
|
There was a problem hiding this comment.
Suggested change
| - `-no-warn-on-undeclared-var` - Silence warnings when the template assigns | |
| undeclared variables. This is especially relevant for HCL2 templates with | |
| .pkrvars.hcl dependencies, as defining a value for a variable is not enough on | |
| its own for Packer to function, there also needs to be some variable definitions | |
| in the template. However, for some cases, templates may only declare what they | |
| locally need for building, and reference a global variable file with more variable | |
| assignations than there are definitions. This would yield a warning for each | |
| assigned and non-declared variable. This option will disable those warnings. | |
| - `-no-warn-on-undeclared-var` - Silence warnings when the a variable definition file contains variable assignments for undeclared variables. This can occur when using a var-file that contains a large amount of unused variables for a given HCL2 template. For HCL2 template defining a value for a variable in a var-file is not enough on its own for Packer to function, as there also needs to be a variable block definition in the template files `pkr.hcl` for the variable. By default `packer validate` will warn when a var-file contains one or more undeclared variables. |
When packer validate is invoked, it does not try to evaluate the datasources before attempting to decide if the template is valid. In many cases, this works, but sometimes it will fail as the value is unknown by the validation code. Since the validation code for all the elements of a Packer template is left to be implemented by plugins, we cannot rely on checking for unknown values everywhere, especially since the unknown references are replaced automatically by a value of the right type for the configuration expected. So, in order for such configurations to be validable, we add an extra option to packer validate, that will let users evaluate the datasources from a template.
When the options for bypassing/enabling assigned and undeclared variables were added to Packer, the website documentation for those commands and options was not updated. This commit adds some documentation for those options.
lbajolet-hashicorp
force-pushed
the
validate_add_eval_datasources_opt
branch
from
34de5a7
to
4f9afb6
Compare
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When packer validate is invoked, it does not try to evaluate the datasources before attempting to decide if the template is valid.
In many cases, this works, but sometimes it will fail as the value is unknown by the validation code.
Since the validation code for all the elements of a Packer template is left to be implemented by plugins, we cannot rely on checking for unknown values everywhere, especially since the unknown references are replaced automatically by a value of the right type for the configuration expected.
So, in order for such configurations to be validable, we add an extra option to packer validate, that will let users evaluate the datasources from a template.
Closes #11294