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
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
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
When dib-lint complains about wrong indents, it doesn't give you any
indication where the problem is. This repeats the grep on failure,
outputting the line and line-number.
As a bonus, skip *.orig files from merges
Change-Id: Ifbbdf854ea19191f66e9823468dbc0afc2f93e1f
Look for files .yaml and pkg-map configurations, and try to load them
either as json or yaml. This way, invalid ones can be detected before
they are committed unnoticed.
Also, exclude .yaml files from being searched while checking bash and
python scripts.
Change-Id: I2478837cfe66929ae1b0d7dd96e049773a35e11c
Pass an explicit template to mktemp since that is required on Mac OS X.
Hardcoded to /tmp since I did not want to bother validating TMPDIR which
is user supplied.
Explicit define a repeating group which causes a grep error:
repetition-operator operand invalid
Change-Id: Ib23cc15db7f7dfdd611280884e46342562d545df
I noticed dib-lint was running on my editor backup files and checking
python files (covered by flake8) and RST files
Change-Id: Ide2e8a4c1aa8d619bc4b0326a2cfc5e63c74475f
There is a wide variety of tracing options through the various shell
scripts. Some use "set -eux", others explicity set xtrace and others
do nothing. There is a "-x" option to bin/disk-image-create but it
doesn't flow down to the many scripts it calls.
This adds a global integer variable set by disk-image-create
DIB_DEBUG_TRACE. All scripts have a stanza added to detect this and
turn on tracing. Any other tracing methods are rolled into this. So
the standard header is
---
if [ "${DIB_DEBUG_TRACE:-0}" -gt 0 ]; then
set -x
fi
set -eu
set -o pipefail
---
Multiple -x options can be specified to dib-create-image, which
increases the value of DIB_DEBUG_TRACE. If script authors feel their
script should only trace at higher levels, they should modify the
"-gt" value. If they feel it should trace by default, they can modify
the default value also.
Changes to pachset 16 : scripts which currently trace themselves by
default have retained this behaviour with DIB_DEBUG_TRACE defaulting
to "1". This was done by running [1] on patch set 15. See the thread
beginning at [2]
dib-lint is also updated to look for the variable being matched.
[1] https://gist.github.com/ianw/71bbda9e6acc74ccd0fd
[2] http://lists.openstack.org/pipermail/openstack-dev/2014-November/051575.html
Change-Id: I6c5a962260741dcf6f89da9a33b96372a719b7b0
We now guard against diff returning non-zero (which it does when it
discovers changes). This allows dib-lint runs to continue past the point
of displaying a dependency ordering nit.
Change-Id: Ib7801597400d6d87146181935922c8832baf5aaa
We check python files with dib-lint rather than flake8 which have
conflicting opinions. This means weve been (forcibly) writing non pep8
python.
Also fixing pep8 issues so tests pass.
Change-Id: Idc9db40334f6e15738a7802c06697270df68741c
The current dib-lint script has an exclusion for indent but it is
erroneously used as a label around the tab indention check. This
patch renames the tab indention exclusion and adds the indent
exclusion where it belongs.
Change-Id: I701595e7a08f4764a45c6701d67cbb931c3d47f3
This patch adds exclusions for the indentation and newline
rules in dib-lint. The indentation exclusion is needed for
files using heredoc and wanting to preserve indentation.
The newline exclusion is added here for consisently treating
all dib-lint rules the same.
Change-Id: Ic8a2164eba3cf99e55473846e3a5fe663a3c1095
This will be useful for adopting dib-lint in other projects, in
particular tripleo-image-elements. It allows some dib-lint rules
to be used even if the project can't pass all of the checks. The
failing checks can then be fixed one at a time and removed from the
exclusions list.
For consistency, this change reads the new exclusions from tox.ini
in the same way as flake8. To use the exclusion mechanism, the
the following section can be added to tox.ini:
[dib-lint]
ignore = sete setu setpipefail
Change-Id: I6d8024e7613890e935ceb5e50d0d507bd554d8dd
Currently when dib-lint finds a problem it does something like:
echo "ERROR: Problem found"
rc=1
This is repetitive and error-prone since it's easy to forget to set
rc to actually fail the check. This change makes those two steps
a single function call.
Change-Id: I40b5bf39348a69add1f955c49f310e3bda21be0e
With this change, dib-lint raises an error if when finding an element
using space indentation that is not multiple of 4.
Co-Authored-By: Jon-Paul Sullivan <jonpaul.sullivan@hp.com>
Change-Id: I470e1fdfc38a3f3c7ba5644c5103f2a9ef073005
This will hopefully catch bugs where they happen rather than
allowing scripts to continue on and fail later.
Change-Id: Idacd9274415b21db285a198dafff19b1d19a4a68
With this change, dib-lint ensure elements do not use tab
indentation. The following files are checked:
- executable file
- .md file
Change-Id: I071262ff9f6599548f869f5439ee127f64eeb46f
If scripts are not set -e then errors can be ignored, causing more
confusing failures later.
Also adds an exclusion comment to the ramdisk init script since we
don't want that to exit on failure.
Change-Id: Idf43993bd10b1ef16c1d3b0d9df8d0ad94c46458
There are certain scripts (such as the ramdisk init script) that
need to ignore linting rules for one reason or another. This adds
support for exclusions via a comment in the file like:
"# dib-lint: disable=executable sete"
There should be no ", but I need those to prevent git from treating
that as a comment. This syntax is similar to the pylint exclusion
mechanism.
Exclusion support is added to the executable check, but not
the alphabetical ordering one because I can't imagine a reason we
would need to disable that, and I don't know that comments are
supported in those files anyway.
Change-Id: I9ecfb47269841dc75a005855455ac26ad2cbc642
We've started to require this in reviews, so we should really have
automation in place to catch it right away.
Change-Id: I43fd90647acba400cea11c665fb587856514b0ee
This will provide a place to put checks that catch common errors
in elements. To start, this just checks that files starting with
a shebang are chmod +x so they can actually be run.
Change-Id: I4116a8f38f7bdfc5866764354c459fad8ca18e92
