We include the yaml text of the entire configuration object in
many of our configuration error reports. This can be quite large
which is not always helpful to the user, and can cause operational
problems with large numbers of errors of large objects.
To address that, let's only include a few lines. But let's also
try to make those lines useful by centering the actual attribute
that caused the error in our snippet.
To do this, we need to keep the line number of all of our configuration
attribute keys. This is accomplished by subclassing str and adding
a "line" attribute so that it behaves exactly like a string most
of the time, but when we go to assemble a configuration error, we can
check to see if we have a line number, and if so, zoom in on that line.
Example:
Zuul encountered a deprecated syntax while parsing its configuration
in the repo org/common-config on branch master. The problem was:
All regular expressions must conform to RE2 syntax, but an
expression using the deprecated Perl-style syntax has been detected.
Adjust the configuration to conform to RE2 syntax.
The RE2 syntax error is: invalid perl operator: (?!
The problem appears in the "branches" attribute
of the "org/project" project stanza:
...
name: org/project
check:
jobs:
- check-job:
branches: ^(?!invalid).*$
gate:
jobs:
- check-job
post:
...
in "org/common-config/zuul.yaml@master", line 84
This reduction is currently only performed for deprecation warnings but
can be extended to the rest of the configuration errors in subsequent
changes.
Change-Id: I9d9cb286dacee86d54ea854df48d7a4dd37f5f12
We subclass the yaml.SafeDumper class to adjust its behavior with an
override of the ignore_aliases method. It is possible to subclass
the cyaml.CSafeDumper class as well. The "C" part is actually the
Parser and Emitter, not the Dumper/Representer, so our override
is still effective whether we use the C or Python versions.
This can produce a significant performance increase when exchanging
large amounts of data with Ansible.
The C emitter is more aggressive about not using unecessary quotes,
so the ansible dumper test assertions need to change. To add some
extra assurance, that test is also updated to check that the round-trip
load is as expected as well.
Change-Id: I30fd82c0b9472120d010f3f4a65e17fb426b0f7e
User complained about the anchors and aliases in inventory.yaml, which is
intended for huram to read. With anchors and aliases reduced the readbility.
This change is intended to fix this.
Change-Id: I99333f85d7fdda213ddf2c14e5db89825b093b56
We aren't generally using type annotations, and mypy itself is
somewhat flawed, occasionally producing false positives and rarely
catching errors. Stop running it.
Change-Id: I6f24457f7d99ca11ec9228e505e6edec558baf9e
This extends the previous change which freezes job variables to
also supply a copy of the original values to jobs under the
unsafe_vars hierachy.
Change-Id: I6c9f24e2055f78a8136090b04b34afeaf5cd9588
Freze Zuul job variables when starting a build so that jinja
templates can not be used to expose secrets. The values will be
frozen by running a playbook with set_fact, and that playbook
will run without access to secrets. After the playbook
completes, the frozen variables are read from and then removed
from the fact cache. They are then supplied as normal inventory
variables for any trusted playbooks or playbooks with secrets.
The regular un-frozen variables are used for all other untrusted
playbooks.
Extra-vars are now only used to establish precedence among all
Zuul job variables. They are no longer passed to Ansible with
the "-e" command line option, as that level of precedence could
also be used to obtain secrets.
Much of this work is accomplished by "squashing" all of the Zuul
job, host, group, and extra variables into a flat structure for
each host in the inventory. This means that much of the variable
precedence is now handled by Zuul, which then gives Ansible
variables as host vars. The actual inventory files will be much
more verbose now, since each host will have a copy of every "all"
value. But this allows the freezing process to be much simpler.
When writing the inventory for the setup playbook, we now use the
!unsafe YAML tag which is understood by Ansible to indicate that
it should not perform jinja templating on variables. This may
help to avoid any mischief with templated variables since they
have not yet been frozen.
Also, be more strict about what characters are allowed in ansible
variable names. We already checked job variables, but we didn't
verify that secret names/aliases met the ansible variable
requirements. A check is added for that (and a unit test that
relied on the erroneous behavior is updated).
Story: 2008664
Story: 2008682
Change-Id: I04d8b822fda6628e87a4a57dc368f20d84ae5ea9
This reverts commit ddb7259f0d.
This failed earlier because of differing import paths between the
unit tests and actually running the service. We have removed the
metaclass magic which led to that and are now more explicit with
our yaml loading/dumping.
Change-Id: Ifec567846acebe7b9e71c2629edf38f36d271cbf
In preparation for expanded use of the encrypted YAML object in the
executor, make the yaml loading and dumping more explicit.
We currently rely on metaclass magic to cause the encrypted yaml
objects to work. To avoid any future potential import errors and
also to make it easy and clear about which kind of objects should
be supported while loading/dumping yaml, stop using the metaclass
method and instead explicitly create encrypted loader/dumper clasess
and helper methods.
Change-Id: I97a65e3f162ffaa20bb09a09278b00a76e7b1c0c
We want to be able to work with encrypted/pkcs1-oaep objects in yaml
outside of the zuul scheduler. Previously this meant you needed to
import zuul.configloader to trigger the necessary side effects to make
pyyaml recognize this object type. This change moves the object
definition into zuul.lib.yamlutil (which configloader imports) so that
any user of zuul's yamlutil gets encrypted/pkcs1-oaep object handling.
Change-Id: Ife6f164a97f7de2df1bfea3c423ec96027a952d5
Revert "Decrypt project ssh keys in executors"
This reverts commit 77bde6f765.
Revert "Decrypt secrets on the executors"
This reverts commit fbb17e1f35.
Revert "Support serializing encrypted secret objects"
This reverts commit 707570e46b.
We observed an error parsing the encrypted/pkcs1 yaml on the executors
in production on OpenDev. We have not found the cause yet; revert this
until we identify it.
Change-Id: Icc751c54a376e68fd5d9e29dbbb67ed7aa6d67c5
Rather than decrypting secrets on the scheduler and sending them
to the executors unencrypted, now that the private keys are in ZK
and the executors have access to them, we can defer decryption to
the executors. This means that when we move the build requests
from gearman to ZK, we avoid storing decrypted secrets in ZK.
We accomplish this by serializing the entire secret (parts or all
of which may be encrypted or plaintext) to YAML in the scheduler
and deserializing the YAML into a Secret object on the executor.
We do this because we already have support for indicating an
encrypted value via custom YAML tags.
This means that the build request (which is currently transmitted
via gearman and soon to be via ZK) serializes the rest of the job
to JSON. This means we're storing a serialized-to-YAML secret as
a scalar value in a serialized-to-JSON data structure. There's
nothing technically wrong with this, and it is the minimal version
of this change, however it's slightly unusual and may result in
a little extra work. We may want to consider serializing the
entire job request as YAML instead.
Change-Id: I6d94c1d8da8b68e5fb60c27e73039155a02fb485
In memory, we store secrets as deserialized YAML objects, which
can be dicts, lists, scalars, or our own custom EncryptedPKCS1_OAEP
subclass of YamlObject. We need to be able to serialize a
(potentially) encrypted secret in order to transmit it to the
executor via ZK. The easiest and most sensible serialization
mechanism is to reverse our deserialization and use YAML.
This change supports this by indicating that an EncryptedPKCS1_OAEP
object is safe to serialize using the SafeDumper. Any time we
safe_dump to yaml a Python data structure with an EncryptedPKCS1_OAEP
in it, it will be serialized using our custom tag.
This also corrects an unrelated parameter name typo in the
safe_dump method.
Change-Id: I1034946e5261003d9e83119ceee99b89d0dc53a1
mypy depends on typed-ast which relatively recently added python 3.8
support. Unfortunately the mypy we were using requires an older
typed-ast and fails under python 3.8. Fix this by bumping mypy up to a
level where minimal code changes are necessary.
The code changes we made ignore type conflicts due to redefinition on
conditional imports. Comments include link back to a bug describing this
issue and workaround in mypy.
Change-Id: Iec69a27b16a1e09eb6bfbcf8d68deb1ae68d42a3
Zuul recently added zuul.message which needs to be protected against
interpretation by jinja in ansible. This was initially done by marking
it with the !unsafe tag. However this has the disadvantage that the
inventory is no longer parsable by standard yaml parsers without
teaching them the !unsafe tag.
There is a similar simple possibility that doesn't rely on this tag by
base64 encoding the commit message. Ansible has filters for decoding
this so it is still quite easy to deal with base64 encoded vars in
ansible via '{{ zuul.message | b64decode }}'.
Change-Id: I9628e2770dda120b269612e28bb6217036942b8e
YAMLObject classes can self-register themselves in the yaml dumper and
loader via metaclass magic. Use it so we don't execute statements
during import.
Change-Id: I31f097701f863e3f2298028d76054e80dfc2d23e
python3 includes support for optional type annotations which can be used by
static analysis tools to perform type checking. The mypy tool is a
static type checking tool that can also infer type information in many
cases, but which will use explicit type information if it is present.
Add mypy to test-requirements and to the pep8 job so that our pep8 job
can do more analysis work and less with the code style.
To support this, there were a few places in the current codebase that
needed an explicit type hint. For variables/attributes in 3.5 this is done via
comments. There is a conditional import that was confusion that just got
marked with an 'ignore'.
Our ansible action and lookup plugins confuse mypi with the way they
import the ansible base classes. That's ok - they confuse us with that
too. The .pyi files are 'typeshed' files, which are a way that one can
provide static type annotations without putting the information into the
file itself. mypy will always prefer a .pyi file over a .py file (since
the point of them is to be external annotion/interface description) So
in order to get mypy to not barf on the ansible import weirdness, just
add a corresponding empty .pyi file. We could potentially actually put
interface descriptions in them - but I don't think there is very much
value in that.
It should be amusing to at least someone that we have to flake8: noqa
an import from typing that was done to provide a type hint in a comment.
Change-Id: I6c4ac3dcfc6fd990e6c6886749de147ad28389d1
PyYAML doesn't automatically use the much faster and more memory
efficient libyaml bindings, even if the extension is available. So we
provide our own module that exports the pieces needed to use the faster
one, or fall back to the pure python implementation.
Change-Id: I7ee99f5017cb83153ab8fa9bc23548ed639777c1
James E. Blair