In a couple places in the codebase we didn't check the errors from
functions we execute.
In some cases this is harmless (or at least ignorable), but others may
need to log what went wrong, so for all the reported occurrences we
either ignore explicitly or handle the error with a log.
The logOutput function had a bunch of nested checks in the code for the
function, as well as one ineffasign pointed out by golangci-lint.
This commit changes how special cases are processed, and changes a bit
how log redirection to a file is handled.
Prior to this commit, only when PACKER_LOG and PACKER_LOG_FILE are set
does Packer redirect logs to the specified path, which is a bit
superfluous as setting PACKER_LOG_FILE should be enough on its own to
redirect logs and enable verbose logs.
Now, when neither is set, no logOutput is set, if PACKER_LOG_FILE is set
(regardless of PACKER_LOG), we redirect to the specified file, and
finally if PACKER_LOG is set, we redirect to stderr.
Some of the variables we create are flagged by our linters as
ineffective assignments, which makes sense as those are generally fed by
code below, so we don't need to use the declaration/assignation syntax
(:=) but instead can fall back to using var with a type to get the zero
value of the declared entity.
Test files have some calls that don't necessarily get checked since they
have a very low probability to fail, and in a testing environment it's
acceptable to not have bulletproof code.
Therefore, we add a rule to not run that check on any file that ends
with `_test.go` (Go test convention), so the linter isn't as noisy as it
used to be.
Since the enumer implementation we used hadn't been updated for 5+
years, this didn't work with recent linux/go versions, and enumer
crashed while attempting to parse/analyse the source files.
There's another alternative on Github, forked from the one we used,
which seems more maintained now, and does produce the expected files in
Packer.
Before change
```
~> govulncheck ./...
=== Symbol Results ===
Vulnerability #1: GO-2024-2947
Leak of sensitive information to log files in
github.com/hashicorp/go-retryablehttp
More info: https://pkg.go.dev/vuln/GO-2024-2947
Module: github.com/hashicorp/go-retryablehttp
Found in: github.com/hashicorp/[email protected]
Fixed in: github.com/hashicorp/[email protected]
Example traces found:
#1: hcl2template/function/vault.go:30:30: function.init calls template.Vault, which eventually calls retryablehttp.Client.Do
Your code is affected by 1 vulnerability from 1 module.
```
After Change
```
~> govulncheck ./...
No vulnerabilities found.
```
* doc fix for unattended debian/ubuntu installer linkrot
the links for chef-maintained preseeds/autoinstallers for debian/ubuntu both rotted. i'm just getting started with packer, so i hope this edit is accurate.
* Remove webarchive links for legacy JSON
When possible point users to HCL2 templates for getting started examples.
* As per the style guide aim for inclusive language
---------
Co-authored-by: Wilken Rivera <[email protected]>
* Update UpsertBucket to first call GetBucket, this will allow bucket level role based authentication, as CreateBucket uses project level auth
* Fix one incorrect test failure message
* Document use of TMPDIR required for remote plugin installation
* Update website/content/docs/configure.mdx
Co-authored-by: Lucas Bajolet <[email protected]>
---------
Co-authored-by: Lucas Bajolet <[email protected]>
When a command is run, it is the expectation that no test should make
Packer panic. If it did, something is wrong and Packer should be fixed
so it doesn't panic anymore in that situation.
The way we did the check before was adding a PanicCheck after the
command ran, so we could make sure of that during `Assert`.
However, since we introduced the possibility to have multiple runs,
having this addition as part of the run loop meant that the PanicCheck
would be run as many times as there were runs.
While this worked, this implied that we'd do the same check multiple
times on a single command output, which is not optimal.
Instead, this commit moves the check to within the `Run` function, this
way for each run of the command we do the check once, and then we can
assert the results of the command on what output it produced.
Previously duplicate detection for local variables happened during
`Initialise`, through a call to `checkForDuplicateLocalDefinition`.
This works in a majority of cases, but for commands like `console`, this
was not detected as the return diagnostics for `Initialise` are ignored.
That check can be done as early as during parsing however, as the names
of blocks are not dynamic in the slightest (no interpolation possible),
so we move that detection logic into `Parse`, so that the behaviour is
coherent between all commands.
The logic for evaluating local variables used to rely on their
definition order, with some edge cases. Typically `locals` blocks define
multiple local variables, which don't necessarily appear in the same
order at evaluation as within the template, leading to inconsistent
behaviour, as the order in which those are added to the list of local
variables is non-deterministic.
To avoid this problem, we change how local variables are evaluated, and
we're adopting a workflow similar to datasources, where the local
variables first build a list of direct dependencies. Then when
evaluation happens, we evaluate all the dependencies recursively for
each local variable, which takes care of this issue.
As with Datasources, we add a cap to the recursion: 10. I.e. if the
evaluation of a single variable provokes an infinite recursion, we stop
at that point and return an error to the user, telling them to fix their
template.
GetVarsByType is a function that gets a list of Traversals from a hcl
Block.
This approach works when what we are visiting is indeed one, however
when we can get an immediate list of Traversals, but want to filter them
based on their roots, we have to reimplement parts of that function.
Therefore, we split this function in two, GetVarsByType still keeps its
current behaviour, but the filtering step is exposed as another function
now: FilterTraversalsByType, so we can reuse it elsewhere.
The SkipNoAcc function on PackerTestSuite allows to mark a test run as
not to be run every time we run `make test`, but only when PACKER_ACC=1
is set in the environment.
This allows us to skip executing tests that are either long-running, or
that depend on external dependencies (typically Github), which have a
higher potential to fail on a normal run of Packer tests.
When a test fails to exert its assertions on the command-line output, a
test fails, but we don't necessarily can troubleshoot what happened,
especially when this happens in a CI environment.
Therefore, for convenience, we add the faculty for packerCommand.Assert
to automatically dump a command's output (both stdout and stderr) if a
test fails.
Some commands need to have an input in order to work.
For those, we add the capability for the packerCommand struct to have
their stdin defined from a string, which is then fed to the command
being executed.
When a Packer command is created for testing the tool, we generally run
it once, then the command is essentially nooping.
This change allows us to run Packer multiple times with the same
parameters, and make sure all runs conform to a specific list of checks.
This allows us to more reliably test non-deterministic behaviours.
Updates the provisioners documentation for a better reader experience by including a list of the built-in provisioners and a link to the community supported provisioners that are all withing the navigation tree.
Ref: #12898
Signed-off-by: Ryan Johnson <[email protected]>
Adds an example of a string template being used.
The example demonstrates how a template sequence can be used to embed the value of a variable into a string that can be used as script content.
Ref: #12651
Signed-off-by: Ryan Johnson <[email protected]>
Adds documentation for `ssh_keypair_name`, `ssh_agent_auth`, `temporary_key_pair_name`, and `ssh_private_key_file`.
The note is updated noting that not all builders support these options.
Ref: #10722
Signed-off-by: Ryan Johnson <[email protected]>
Remotely installing plugins with a pre-release as part of the constraint
is unsupported by Packer, and should error if that happens.
This test makes sure that this gets treated as an error if that's the
case, even before attempting to connect to the source.
'%d' gets output as-is in the temporary workdir we create. This is
unnecessary and could even be problematic in some cases, so we scrub it
from the MkdirTemp call.
Since legacy config files may declare single plugin components, we need
to warn that they're not supported anymore.
This is in process of being PR'd into main, but to ensure the config
works as intended and we do get the error, we add some tests for that.
These changes include a series of test cases for validating packer init
using the force and upgrade flag. Include in this test is a test case
for validating the plugin installation error when init encounters a
plugin whose reported version does not match the version within the
plugin name.
```
--- PASS: Test_PackerCoreSuite (18.66s)
--- PASS: Test_PackerCoreSuite/TestPackerInitForce (4.91s)
--- PASS: Test_PackerCoreSuite/TestPackerInitForce/installs_any_missing_plugins (2.76s)
--- PASS: Test_PackerCoreSuite/TestPackerInitForce/reinstalls_plugins_matching_version_constraints (2.14s)
--- PASS: Test_PackerCoreSuite/TestPackerInitUpgrade (3.70s)
--- PASS: Test_PackerCoreSuite/TestPackerInitUpgrade/upgrades_a_plugin_to_the_latest_matching_version_constraints (2.02s)
--- PASS: Test_PackerCoreSuite/TestPackerInitWithMixedVersions (1.96s)
--- PASS: Test_PackerCoreSuite/TestPackerInitWithMixedVersions/skips_the_plugin_installation_with_mixed_versions_before_exiting_with_an_error (1.96s)
--- PASS: Test_PackerCoreSuite/TestPackerInitWithNonGithubSource (1.22s)
--- PASS: Test_PackerCoreSuite/TestPackerInitWithNonGithubSource/try_installing_from_a_non-github_source,_should_fail (0.07s)
--- PASS: Test_PackerCoreSuite/TestPackerInitWithNonGithubSource/manually_install_plugin_to_the_expected_source (0.59s)
--- PASS: Test_PackerCoreSuite/TestPackerInitWithNonGithubSource/re-run_packer_init_on_same_template,_should_succeed_silently (0.55s)
```
To make sure we do scrub the metadata in the plugin name when installing
it from a local binary, we add a test that does that installation with
both alternatives: 1.0.0-dev and 1.0.0-dev+metadata, which should result
in only one alternative being installed (the last one that succeeded).
The installation with a metadata part in the version for a plugin had
one test that relied on the plugin directories being populated with
packer plugins install --path.
This could change in the future, while the command should remain
functional, so we explicitely call it in the test instead of through the
function that creates/populates a temp plugin dir.
When building a pipeline to count the number of lines returned by
Packer, it can be a bit cumbersome to have to chain the calls to
MkPipeCheck to do that check, so we add one convenience function for the
simplest case: counting the number of lines on stdout, without any kind
of filtering.
When manually installing a plugin to the plugin directory, we compute a
SHA256SUM file from the plugin binary, and install it alongside it so we
can test the loading process for Packer.
In the introduction of the function, we added a check that if we were
running on Windows, we'd remove the extension of the sumfile's name
before writing it.
This is actually not necessary (and breaks the loading logic) as Packer
looks for the name of the plugin with extension, followed by
_SHA256SUM in order to compare the effective digest of the file to the
one written to this file.
Since this prevents the tests that use this function from succeeding in
a Windows environment, we remove this extra step.
Windows relies on the `TMP` (or alternatives) being set in the
environment in order to be able to create temporary directories and
files.
If this is not set, the `os.TempDir` function defaults on the windows
installation root directory (typically C:\Windows), leading to
permission errors when running Packer in the context of a test, as we're
installing plugins in a temporary directory.
To avoid this problem, we get the current setting from the test's
invocation environment, and forward it to the subcommand we execute for
our tests.
Since on Windows extensions are mandatory in order to have something
executable, we compile Packer with a `.exe` suffix during tests, so we
can use the executable afterwards to run tests with.
Devashish
Lucas Bajolet
Lucas Bajolet
Jamie Finnigan
Wilken Rivera
dependabot[bot]
hashicorp-tsccr[bot]
Wilken Rivera
graevyandWilken Rivera
Jenna Goldstrich
Ryan Johnson
Ryan Johnson