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
When using up to date distributions for dib development, pep8
installs using python3. This patch fixes the problem, that
not the complete dib-lint (which is called) is compatible
with python3.
Change-Id: I417d03746edb4d34011b997edf8b5b9662ea6f09
Signed-off-by: Andreas Florath <andreas@florath.net>
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
It has always been a weird thing that dib is a python package, but
is totally driven by the disk-image-create script. It creates this
strange division that is hard to explain.
This moves disk-image-create to a regular python entry-point
Currently, this simply exec()s the original disk-image-create script.
However, we now have a (private) interface between disk-image-create
written in python and the driver shell script. Here's some things we
could do, for example:
* Argument parsing is generally nicer in Python, and then end result
is mostly just setting environment variables to flag different things
in the shell script. I could see us moving the argument-parsing into
diskimage_builder.disk_image_create:main() and just setting things in
os.environ before the exec()).
* I7092e1845942f249175933d67ab121188f3511fd sets IMAGE_ELEMENT_YAML in
disk-image-create by calling-back to element-info. We can just call
element_dependencies.find_all_elements() in here an export is to
os.environ before disk-image-create starts.
* remove need for ramdisk-image-create symlink by just exporting
IS_RAMDISK based on sys.argv[1] value
* you could even unit test some of this :)
Change-Id: I69ca3d26fede0506a6353c077c69f735c8d84d28
Currently we have all our elements and library files in a top-level
directory and install them into
<root>/share/diskimage-builder/[elements|lib] (where root is either /
or the root of a virtualenv).
The problem with this is that editable/development installs (pip -e)
do *not* install data_files. Thus we have no canonical location to
look for elements -- leading to the various odd things we do such as a
whole bunch of guessing at the top of disk-image-create and having a
special test-loader in tests/test_elements.py so we can run python
unit tests on those elements that have it.
data_files is really the wrong thing to use for what are essentially
assets of the program. data_files install works well for things like
config-files, init.d files or dropping documentation files.
By moving the elements under the diskimage_builder package, we always
know where they are relative to where we import from. In fact,
pkg_resources has an api for this which we wrap in the new
diskimage_builder/paths.py helper [1].
We use this helper to find the correct path in the couple of places we
need to find the base-elements dir, and for the paths to import the
library shell functions.
Elements such as svc-map and pkg-map include python unit-tests, which
we do not need tests/test_elements.py to special-case load any more.
They just get found automatically by the normal subunit loader.
I have a follow-on change (I69ca3d26fede0506a6353c077c69f735c8d84d28)
to move disk-image-create to a regular python entry-point.
Unfortunately, this has to move to work with setuptools. You'd think
a symlink under diskimage_builder/[elements|lib] would work, but it
doesn't.
[1] this API handles stuff like getting files out of .zip archive
modules, which we don't do. Essentially for us it's returning
__file__.
Change-Id: I5e3e3c97f385b1a4ff2031a161a55b231895df5b
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>
Move element-info from a wrapper script to a standard entry-point
console_script.
Update the documentation to explain how to run it for development. I
don't think we should support the idea that you can check-out the code
and run ./bin/disk-image-create -- it has dependencies (dib-utils,
etc) and needs to be run from a virtualenv (this is what CI in the
gate does). A follow-up can clean-up some of the path munging stuff
we have for this in disk-image-create.
Change-Id: Ic0c03995667f320a27ac30441279f3e6abb6bca8
Block device handling can be somewhat complex - especially
when taking things like md, lvm or encryption into account.
This patch factors out the creation and deletion of the local
loop image device handling into a python library.
The main propose of this patch is to implement the needed
infrastructure. Based on this, more advanced functions can be added.
Example: (advanced) partitioning, LVM, handling different boot
scenarios (BIOS, UEFI, ...), possibility of handling multiple images
(local loop image, iSCSI, physical hard disk, ...), handling of
different filesystems for different partitions / LVs.
Change-Id: Ib626b36a00f8a5dc3dbde8df3e2619a2438eaaf1
Signed-off-by: Andreas Florath <andreas@florath.net>
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
