Instead of manually invoking ts.PackerCommand().UsePluginDir(), we
introduce a new PackerCommand() function on TestDirSpec which
fast-tracks this common use-case.
In order for the creation of a temporary directory to install plugins
into to be simpler to understand and use, we change how the directory is
created, cleaned-up, and installs plugins into.
Now, instead of a tuple of a string (path) and a cleanup function, we
return a structure that comprises the test suite, and the temporary
directory, along with methods to handle those steps independently.
When using a PackerCommand, the Run function was made public as a way to
access the contents of an execution.
This was clumsy as it had too many responsabilities, and was not needed
strictly as Assert was performing the executions, as many times as
required.
This could introduce cases in which one run as spent by the caller, then
the remainder were executed through Assert.
Therefore, we change this convention.
Now, run is private to the type, and only through Assert can a command
be executed.
If a test needs access to a command's output, stderr, or error, it can
do so through the Output function, which requires Assert to be called
first.
When a command asserts its output with checkers, by default it will
register errors through a t.Errorf.
While this works, in some cases we would want to stop execution
immediately if a function's Assert fails, as the rest of the test may
depend on the assertion being valid.
In the current state, this means either getting the result of the run to
check if an error was returned (not fully reliable as if the command was
run multiple times, and the last run succeeded, we won't get an error),
or relying on t.IsFailed() (completely reliable).
Instead, we introduce a new function on packerCommand, that lets users
change how Assert behaves, so that if an error was reported, instead of
logging the error and flagging the test as failed, we can use t.Fatalf,
so that the test immedately fails and stops execution.
The lib name for the common components for writing packer_test suites
was not clear, and did not follow the convention established in Packer
core and plugins.
Therefore this commit does two things: first the lib is renamed into
common as to follow this convention, and clearly document which
components are common to all tests.
Also checkers are placed in a subpackage of common, common/check, so
that it is clearer what is meant to be used as checks for a command's
execution status after it's been run, as part of Assert.
The function's name was the same as the plugins test suite runner/init
function, making it impossible to differentiate between the two when
looking at the logs, or when attempting to filter which tests to run
using the suite function's name.
The interface for building a plugin through the test suite was
confusing, as it would build the plugin and return its path, cache the
path for the version built, and return the path regardless if something
was built or not.
While in the current state this is harmless as builds are idempotent,
since the state of the plugin package/module does not change, this will
in the future as we introduce customisation techniques on the plugin's
directory and files, making this double-use potentially dangerous.
Furthermore, the current behaviour is unclear, as the function hides
that caching mechanism, which could come as a surprise for users
attempting to build a plugin for the duration of a test, while the built
plugin is linked to the test suite being run, and not the unit test
being evaluated.
Therefore this commit changes the sequence in which plugins are built
and used. Now the `CompilePlugin` function builds a plugin, and does not
return its path anymore, instead terminating the tests immediately if
they fail.
In normal test usage, a new `GetPluginPath` function is introduced,
which looks-up the path in the suite's cache, failing immediately if
invoked before the plugin is built.
With this change, it is heavily advised to build plugins when
initialising the suite, then in the tests, the GetPluginPath function
should be used to get a plugin's path for interacting with packer
commands.
The compiledPlugins map with a mutex was essentially a glorified
sync.Map, and therefore did not need to be defined as such.
Instead, this commit replaces its uses by a normal sync.Map, and keeps
it in the base test suite.
When testing a packer build or validate, one common use case is to check
which plugins are loaded and used by Packer to run a command.
This is generally done through Grep, but repeating the same pattern can
be redundant, and if the output changes, all those need to be updated.
Therefore, this commit introduces a PluginsUsed gadget, which can be
orchestrated to ensure a plugin is used, or not used, allowing to check
for multiple plugins at once.
As a small reorganisation attempt, this commit moves some file-system
oriented functions from plugin.go into its own file so they are more
logically grouped.
In terms of organisation, we keep functions that interact with plugins
into its own file, therefore the function that build plugin versions
should really be in plugin.go, not in suite.go.
The compiledPlugins map used to be a global variable, which can be
problematic as we move to independent test suites, since those test
suites run in the same process space, the global variable could point to
now deleted plugin versions/paths in separate suites, which would make
tests fail with random errors.
To avoid this, the map is now scoped to the test suite, and a new copy
is created lazily if used by the test suite.
Having only one test suite for the whole of Packer makes it harder to
segregate between test types, and makes for a longer runtime as no tests
run in parallel by default.
This commit splits the packer_test suite into several components in
order to make extension easier.
First we have `lib`: this package embeds the core for running Packer
test suites. This ships facilities to build your own test suite for
Packer core, and exposes convenience methods and structures for building
plugins, packer core, and use it to run a test suite in a temporary
directory.
Then we have two separate test suites: one for plugins, and one for core
itself, the latter of which does not depend on plugins being compiled at
all.
This sets the stage for more specialised test suites in the future, each
of which can run in parallel on different parts of the code.
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.
Lucas Bajolet
Wenfeng Pan
Wilken Rivera
dependabot[bot]
hashicorp-tsccr[bot]andhashicorp-tsccr[bot]
Jeanne Angeles Franco
Devashish
Lucas Bajolet
Jamie Finnigan
Wilken Rivera
graevyandWilken Rivera
Jenna Goldstrich