Commit Graph

39 Commits

Author SHA1 Message Date
Ian Wienand
812ded362d Also check bin/ for tabs
Add bin/ to dib-lint basic script checks

Change-Id: I92849ab608a6c792d790f041dc7614278c274d30
2017-01-13 15:45:56 +11:00
Jenkins
d1ca1b1957 Merge "Catch errors in DIB_INIT_SYSTEM export" 2016-12-14 07:14:10 +00:00
Markos Chandras
e22faa0f77 elements: Drop executable bits from environment files
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
2016-12-01 23:06:56 +00:00
Ian Wienand
ff79dc9793 Catch errors in DIB_INIT_SYSTEM export
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
2016-11-23 23:03:50 +00:00
Ian Wienand
f15550f9fe Special case dib-python in dib-lint
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
2016-11-23 19:58:43 +11:00
Ian Wienand
9e392f56b0 Don't set tracing in environment files
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
2016-10-20 13:58:00 +11:00
Jenkins
9dedca018f Merge "Handle file magic type varying order matching" 2016-06-07 01:00:57 +00:00
Jenkins
5f4cac3303 Merge "Add a best-effort sudo safety check" 2016-05-26 17:31:00 +00:00
Darragh Bailey
716be1d6f0 Handle file magic type varying order matching
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
2016-05-25 17:55:59 +01:00
Ian Wienand
672705831f Add a best-effort sudo safety check
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
2016-05-09 15:41:38 +10:00
Ian Wienand
8b4a5e9919 Split YAML & JSON parsing
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
2016-04-22 11:20:57 +10:00
Ian Wienand
b388b20f99 Add some output to dib-lint
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
2016-04-22 11:20:10 +10:00
Ben Nemec
c3ee0acdd8 Allow skipping the md docs check
Not every project that uses dib elements will necessarily want this
check enabled.

Change-Id: Id4b167ed220dd55852b6587b884fabe7bc8554eb
2016-04-15 10:38:32 -05:00
Ben Nemec
bdf3aab53a Don't stop dib-lint on first flake8 failure
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
2016-04-15 10:29:48 -05:00
Ian Wienand
d8abe72537 Only match #!/bin/bash in scripts
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
2016-01-29 15:49:10 +11:00
Ian Wienand
9189e24f6d dib-lint: ignore blank lines in element ordering
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
2015-10-06 14:02:10 +11:00
Ian Wienand
e678910166 Output failing lines when dib-lint finds wrong indents
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
2015-09-24 21:03:36 +10:00
Pino Toscano
aeab45ad7f dib-lint: validate json/yaml files
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
2015-09-15 12:48:23 +02:00
Antoine Musso
35584efa90 dib-lint: make it work on Mac OS X
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
2015-06-18 11:12:01 +02:00
Ian Wienand
ab05a8b7e2 Skip backups and other non-relevant files for dib-lint
I noticed dib-lint was running on my editor backup files and checking
python files (covered by flake8) and RST files

Change-Id: Ide2e8a4c1aa8d619bc4b0326a2cfc5e63c74475f
2015-05-29 15:39:46 +10:00
Gregory Haynes
cc308464c6 No markdown docs for elements
Sphinx does not support markdown, therefore all our element docs should
not be in this format.

Change-Id: I6fceb5c2c218e94a463f13d6d9050aea485e6c31
2015-04-02 23:55:19 +00:00
Ian Wienand
36b59c001c Standarise tracing for scripts
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
2015-02-12 10:41:32 +11:00
Chris Jones
f4a2eb6ef3 Continue past dependency ordering diffs.
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
2014-12-12 18:36:55 +00:00
Gregory Haynes
fd0708286c Check python with flake8 instead of dib-lint
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
2014-11-21 09:04:52 -08:00
Ryan Brady
6fedbc58f9 Fix indent exclusion
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
2014-11-11 21:06:25 -05:00
Ryan Brady
c1985ea772 Add dib-lint exclusions
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
2014-07-24 13:50:48 -04:00
Ben Nemec
541a4b2270 Add global exclusions to dib-lint
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
2014-07-03 11:50:04 -05:00
Ben Nemec
e824b43cbd Factor out error behavior in dib-lint
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
2014-05-28 11:04:27 -05:00
Jenkins
43827916d5 Merge "dib-lint: ensure file finish with a new line" 2014-05-28 00:41:03 +00:00
Ben Nemec
2830ef4854 Check for set -o pipefail
Like the other set lint checks, this will hopefully catch errors
in element scripts sooner.

Change-Id: Ib1600938f6ffed657ff2950ef54bbcbdb5a0db08
2014-05-23 15:57:27 -05:00
Jenkins
273d364fc4 Merge "Ensure scripts are set -u" 2014-05-22 03:03:28 +00:00
Gonéri Le Bouder
0e516cc8d1 indent: search for !=4 spaces indentation
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
2014-05-10 11:17:39 +02:00
Ben Nemec
e0650572f2 Ensure scripts are set -u
This will hopefully catch bugs where they happen rather than
allowing scripts to continue on and fail later.

Change-Id: Idacd9274415b21db285a198dafff19b1d19a4a68
2014-05-06 15:51:07 -05:00
Gonéri Le Bouder
bd69d32806 dib-lint: check for tab indent in files
With this change, dib-lint ensure elements do not use tab
indentation. The following files are checked:

- executable file
- .md file

Change-Id: I071262ff9f6599548f869f5439ee127f64eeb46f
2014-05-06 13:54:38 +02:00
Gonéri Le Bouder
27e7aaefa6 dib-lint: ensure file finish with a new line
git and the patch command expect file to end with a new line.

Change-Id: I783583b2a5a703007bf206ba56a1caa6fe15973e
2014-05-06 13:44:58 +02:00
Ben Nemec
79ab95b16e Make sure all scripts are set -e
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
2014-05-02 17:18:54 -05:00
Ben Nemec
825b1964bb Allow excluding tests from certain files
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
2014-05-02 17:18:54 -05:00
Ben Nemec
e6e5076698 Enforce alphabetical ordering of element-deps
We've started to require this in reviews, so we should really have
automation in place to catch it right away.

Change-Id: I43fd90647acba400cea11c665fb587856514b0ee
2014-03-26 15:23:18 -05:00
Ben Nemec
f961d745e3 Add dib-lint script
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
2014-03-26 15:23:05 -05:00