Avoid dangerous file parsing and object serialization libraries.
yaml.load is the obvious function to use but it is dangerous[1]
Because yaml.load return Python object may be dangerous if you
receive a YAML document from an untrusted source such as the
Internet. The function yaml.safe_load limits this ability to
simple Python objects like integers or lists.
In addition, Bandit flags yaml.load() as security risk so replace
all occurrences with yaml.safe_load(). Thus I replace yaml.load()
with yaml.safe_load()
[1]https://security.openstack.org/guidelines/dg_avoid-dangerous-input-parsing-libraries.html
Change-Id: I84640973fd9f45a69d2b21f6d594cd5bf10660a6
Closes-Bug: #1634265
The squashfs format brings a couple of advantages over the other
formats. Image is often an order of magnitude smaller and it can
be used natively, either as an initrd, either with loop mount.
Change-Id: If72940b0c4dafb2504c52dd0429a8eb3f8305751
We now support tgz (tar.gz) as an output format.
Change-Id: Iadec92f2f96c3f904f28bd49f87ffc7d48ef7bd7
Signed-off-by: Paul Belanger <pabelanger@redhat.com>
mkfs's arguments are
mkfs [options] [-t type] [fs-options] device [size]
So it seems our MKFS_OPTS are really supposed to be fs-options, rather
than options to mkfs itself.
Why didn't we notice? It's quite a trap -- mkfs.ext2 has a "-t"
option, so when we're calling
$ mkfs -i 4096 ... -t ext4 ...
We actually just fall-back to the default from the mkfs wrapper which
is mkfs.ext2 which works! But when you make that, say, xfs, we're not
calling the right wrapper at all.
Also update documentation
Closes-Bug: #1648287
Change-Id: I3ea5807088ab361bd9c235c07fb1553fbaf9178b
Files in $element/environment.d are meant to be sourced, so drop
the executable bit. Moreover, drop the executable bit from a couple
of other scripts that are either meant to be sourced or simply because
they are configuration files.
Change-Id: I7f724dd9d409f4a835a136f12f48a84aa9acc41e
When you source a file that just does
export FOO=$(bar)
you miss any invalid return codes from "bar" (even under -e) because
bash returns the value of the "export", which is 0
On centos-minimal, we stopped bringing in systemd early and this was
causing dib-init-system to not know what init was available. Since it
did not fail correctly, it lead to confusing errors much later in the
build when service files were not copied correctly. See also
I24ce648485c3d6f3c27ab8f87a638516b3727017
A dib-lint check is added. One minor fixup is in 00-set-apt-sources
(this one is less likely to cause problems). I have run dib-lint over
project-config elements and none use this pattern.
Change-Id: I076c08190d40c315ad6a6d96a3823e9fc52630be
It seems that on Xenial, it does not take much to confuse "file" and
it's mime guessing such that it thinks some files are not python.
"package-installs-v2" is a good example, since it has an interpreter
"dib-python" that "file" doesn't know about, and no extension. While
looking at this, I've added emacs vars here so it opens in python
mode.
Change-Id: I01994b08c5ad8987925f1eec4062f5b6ee72eb8f
Because environment files are sourced into the current environment,
they shouldn't be setting global settings like tracing else they
affect every preceeding import. This is quite confusing when only
half your imports are traced in the logs, because it was either turned
on, or off, by a preceeding environment import.
There is a corresponding dib-run-parts change in
I29f7df1514aeb988222d1094e8269eddb485c2a0 that will greatly increase
debugability for environment files by deliberately logging what files
are sourced and consistently turning on tracing around their import.
This isn't strictly necessary (since dib-run-parts with the prior
change will just turn tracing off after import anyway) but it's a
decent cleanup for consistency. A bare-minimum dib-lint check is
added. Documentation is updated.
Change-Id: I10f68be0642835a04af7e5a2bc101502f61e5357
In shade, we use both md5 and sha256 checksums to help validate the
integrity of an image. Rather then having nodepool do this each time
for every time, have diskimage-builder create these files when we
build the image.
We've added a flag (disabled by default) to toggle this functionality.
Change-Id: I5815ba69b7d477f1e91dc8ec0c69c86168770964
Signed-off-by: Paul Belanger <pabelanger@redhat.com>
Storing the du output in a variable seemed convenient, but I didn't
realise just how big it could get especially with things like infra
images -- there's something like 100MiB of text being stored in a bash
variable here.
Convert this to work with a temporary file
Change-Id: I6a6d22c2142e0f199490c39cca8c94769e4b0232
Since the ironic-agent element builds the ramdisk and extracts the
kernel itself, there's no need to actually generate an image at the
end of the process. Previously the unnecessary image was being
deleted, but this wastes a bunch of time compressing and converting
the image. It's better to just not create the image at all.
This change adds a noop element called no-final-image that
disk-image-create looks for in the element list and, if found, will
cause it to skip the final image generation. This is more flexible
than the previous ironic-agent-specific method that would have
required changes to disk-image-create for every element that wanted
to behave similarly.
Note that this cannot be done using an environment variable, because
element environments.d entries do not propagate out to
disk-image-create. It also doesn't make sense as a user option
because it should be set by the element author, not the user.
Change-Id: I168feb18f0d578b3babbe4784d3ef75e755e1ebd
Under some systems this leads to an error if the oder of parameters
does not comply exactly with the way it is specified.
Change-Id: Ie1ff871dfffecaf95e7ac467b18543561aaa0ceb
The ironic-agent element doesn't care about the final qcow2/raw/
whatever image the disk-image-create command normally creates, so
previously it was deleting it at the end of the process. This is
a pretty significant waste of time when building those images, and
instead we can just skip creating the image when building
ironic-agent.
Change-Id: If48f575e795a823c777891f193ebf8bd943aa296
Allow file test to return all possible mime-types and ensure anything
that matches the python file type is parsed by flake8 instead of
relying on the first match returned.
Closes-Bug: #1585688
Change-Id: Iba31f1853537fe9234ab6f83d66f13dc1c578abb
For something fairly simple, I went back-and-forward with this a bit.
Firstly, I realise calling readlink constantly sucks. Due to the way
we call dib and source various files, you end up with the source-file
from "caller" being usually a very ugly path including levels of "../"
indirection. Cleaning this up to something canonical is the only sane
way to present it.
Because we evaluate _ps4() from a sub-shell in the PS4 string, there's
no way for it to do something like build a global in-memory cache in
an associative array or similar. It could write out a temp file or
some other side-band method, but the overheads of managing this don't
seem any different to just calling readlink. If anyone can think of a
bash-hack around this that doesn't involve a fork() I'm interested.
We could potentially strip some of the leading paths in the assumption
you know what they are; but it gets complex when things are split
across /usr/bin & /usr/lib and external elements, etc. I thought
about arbitrarily shortening it (e.g. just take last 20 characters)
which gives you enough of an idea of the file, but looks a bit ugly.
Or we could just leave the file-name out all together and assume the
function name is unique enough; this also seemed a bit ugly.
Obviously it's a matter of taste in the output. It is certainly
wider, but it also adds a lot of information. It also makes it fairly
clear where there are things we can make less verbose,
e.g. I1e39822f218dc0322e2490a770f3dc867a55802c disables tracing in
run-parts which is just noise. There's a few other frequently used
loops that we could disable tracing for by default to benefit
signal:noise.
tl;dr : take a look at the logs. I think it is a step in the right
direction of making the logs more usable for debugging.
Change-Id: I8054a3050415fcb527baeb7012bf133e5c864bf3
We have some test cases which attempt to build docker images, therefore
we need docker.
Fix a few bugs that showed up when we run docker tests - we need to
docker rm with sudo and docker images don't always have a /tmp so check
before unmounting it.
Change-Id: I147d0ef3f2ea83f35bac568214573a6bde0b1967
As motivation for this; we have had two breakouts of dib in recent
memory. One was a failure to unmount through symlinks in the core
code (I335316019ef948758392b03e91f9869102a472b9) and the other was
removing host keys on the build-system
(Ib01d71ff9415a0ae04d963f6e380aab9ac2260ce).
For the most part, dib runs unprivileged. Bits of the core code are
hopefully well tested (modulo bugs like the first one!). We give free
reign inside the chroot (although there is still some potential there
for adverse external affects via bind mounts). Where we could be a
bit safer (and could have prevented at least the second of these
breakouts) is with some better checking that the "sudo" calls
*outside* the chroot at least looked sane.
This adds a basic check that we're using chroot or image paths when
calling sudo in those parts of elements that run *outside* the chroot.
Various files are updated to accomodate this check; mostly by just
ignoring it for existing code (I have not audited these calls).
Nobody is pretending this type of checking makes dib magically safe,
or removes the issues with it needing to do things as root during the
build. But this can help find egregious errors like the key removal.
Change-Id: I161a5aea1d29dcdc7236f70d372c53246ec73749
It turns out that invalid JSON can be valid YAML ... thus if you mess
up a pkg-map file that still works as a YAML file dib-lint will let it
pass, but when pkg-map later tries to open it as a JSON file, it
fails.
Parse each type separately to catch these problems.
Change-Id: Ib3985e7d1599ed6bf3b7a73b786a53177b71fae0
It's hard to tell if dib-lint is working as it outputs nothing. Add
some minimal output strings at some key points.
Change-Id: Id11cc9ecb8d5215d6fc8d8ef3584bfeeba53ff13
It's better to report all of the failures in one shot, so we should
make sure a flake8 failure doesn't immediately end the dib-lint
run, and instead just sets the error flag like the other checks.
Change-Id: Ib13fc71bb12a6565888bdd89f33fc6ada89f8d8c
This was not well tested. Build the argument into a variable which
can be eval()ed to produce the final output.
Add the flag so we test this during functional tests. Add "-x" to dib
invocations so we can more easily debug failures.
Change-Id: Ifdc82627c520379b4124ccb9a4c2fe806c52c75c
We don't want the output of "du" run on the image spammed into the
logs with "set -x". Swizzle it off around the sensitive commands.
Change-Id: I687e77275f9a49e7934211835aba8610e88cdca6
If you check logs like [1] it's literally thousands of lines of the
same thing over-and-over as the git caching happens. It is basically
all just noise unless you're debugging it specifically. Up this to
tracing level 2 ("-x -x") to see it. Add a note in the help about
multiple flags, which has always been intended but not documented.
Image builds should continue to run with single "-x", but we could
probably greatly increase signal:noise ratio in the logs with a little
more judicial use of this to turn down some of the very noisy &
repetitive parts.
[1] anything in http://nodepool.openstack.org/
Change-Id: I91c5e55814ba9840769357261d203f4850e2eba6
This cuts the image size down alot, esspecially if there were lots of
small file deletes.
The fstrim utility is in the util-linux package and should be on
most all systems. fstrim also works with XFS, ext4, btrfs, etc
prodiving the kernel is new enough.
A reduction of 25% or more in size is common.
Change-Id: I269b4416be450369616f9b8e030f84c30e329804
In the common case of not specifying a size, we are already running
"du" over the image to figure out how big it is. Leverage that by
saving it's output and displaying a pruned list of big files when
requested.
We add a flag to show a summarised option (files >10MiB) and another
to show full output, should you wish that level of detail.
"Invocation" documentation is updated (and formatted a little better
while we're here).
Change-Id: I255800790a62fed1c82fcd311f1cc29c9867766d
Being able to discover DIB's version from the command itself is
convenient. This patch adds a --version option to the disk-image-create
command, failing gracefully if diskimage-builder is not installed.
This adds an explicit dependency on pbr to the requirements since this
is required to run diskimage_builder/version.py outside of a test
environment.
This patch consciously chooses to only provide the long-form option
and no '-v' to allow for the future possibility that a '-v' might
indicate '--verbose' in the future.
Change-Id: I9fc084774d6c7a39a944b07680b3eb8be8e34f9c
This checks the profile, if it has hardened in it's name it needs xattr support
unfortunately xattr support cannot yet be relied on everywhere, so it needs to
be disabled for hardened profile builds to correctly pax-mark.
Change-Id: I7fb855249a9e6c9b6497ab5061b4ea3c014f5081
Closes-Bug: 1537177
Our dib-lint checking is only considering scripts with #!/bin/bash.
While there's nothing really wrong with some other shebang line like
"#!/usr/bin/env bash" let's keep things consistent.
We can use the same regex match to reduce a few forks in the main
checking.
Also a minor cleanup to the file matching
Change-Id: I609721b2671e704ea26075dad7e5b39a8b858f6b
This patch fixes the calculation of the resultant image size
when building an image with diskimage-builder on ext4 a
filesystem.
Prior to this, using the '--image-size 2' (2GB) setting would
generate an image that would not boot under a 2GB nova flavor.
Change-Id: I7a753bdef84c6300ccea73ae4a92bf330dcd77cb
Closes-Bug: #1513622
Patch adds support for using decimal values for $DIB_IMAGE_SIZE.
This allows for creating images that are <1G in size.
Change-Id: I945644a8e77fecfb0b83efa282dc00bb29514e0b
Closes-Bug: #1366909
Some of the elements-deps in the project-config repo have a blank line
at the end, which throws out the ordering. Strip blank lines, like
comment lines, before processing.
As an additional help, show a side-by-side diff of what is provided
versus what is expected when showing an error about sorting.
Change-Id: I007851ee01d6853ad992ce4437331e8bd79bbfce
