This error is used in some places, but not all. Correct that to
improve config error structured data.
Change-Id: Ice4fbee679ff8e7ab05042452bbd4f45ca8f1122
This error exception class went unused, likely due to complications
from circular imports.
To resolve this, move all of the configuration error exceptions
into the exceptions.py file so they can be imported in both
model.py and configloader.py.
Change-Id: I19b0f078f4d215a2e14c2c7ed893ab225d1e1084
The validate-tenants scheduler subcommand is supposed to perform
complete tenant validation, and in doing so, it interacts with zk.
It is supposed to isolate itself from the production data, but
it appears to accidentally use the same unparsed config cache
as the production system. This is mostly okay, but if the loading
paths are different, it could lead to writing cache errors into
the production file cache.
The error is caused because the ConfigLoader creates an internal
reference to the unparsed config cache and therefore ignores the
temporary/isolated unparsed config cache created by the scheduler.
To correct this, we will always pass the unparsed config cache
into the configloader.
Change-Id: I40bdbef4b767e19e99f58cbb3aa690bcb840fcd7
The fix in I473ba605decb136cd527308a63f16a5e548697fb did not fully solve
the problem with the new Github default merge modes.
In case the branch cache already contains the new merge modes, but the
tenant project config (TPC) still only supports the old subset of modes,
dynamic layout creation will fail saying that the new default merge mode
is not supported.
To fix this we will supply a new parameter with valid merge modes from
the TPC when getting the project default branch instead of getting the
project merge modes directly from the branch cache. Based on the list of
valid modes the driver can then select the best default merge mode.
This change also updates the model API upgrade test v17 -> v18 to cover
this case.
Change-Id: Ibc5645d4725f0ec31cb7ab18d4500452d866166a
This refactors the the config error handling based on nested
context managers that add increasing amounts of information
about error locations. In other words, when we start processing
a Job object, we will:
with ParseContext.errorContext(info about job):
do some stuff
with ParseContext.errorContext(info about job attr):
do some stuff regarding the job attribute
with ParseContext.errorContext(next job attr):
do stuff with a different attr
We store a stack of error contexts on the parse context, and at any
point we can access the accumulator for the most recent one with
ParseContext.accumulator in order to add a warning or error. If we
have an attribute line number, we'll use it, otherwise we'll just
use the object-level information.
We also collapse the exception hnadlers into a single context
manager which catches exceptions and adds them to the accumulator.
This lets us decide when to catch an exception and skip to the next
phase of processing separately from where we narrow our focus to
a new object or attribute. These two actions often happen together,
but not always.
This should serve to simplify the configloader code and make it
easier to have consistent error handling within.
Change-Id: I180f9b271acd4b62039003fa8b38900f9863bad8
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
When making a websocket request, browsers do not send the
"Authorization" header. Therefore if a Zuul tenant is run in
a configuration where authz is required for read-only access,
the websocket-based log streaming will always fail.
To correct this, we will remove the http request authz check
from the console-stream endpoint, and add an optional token
parameter to the websocket message payload. The JS web app
will be responsible for sending the auth token in the payload,
and the web server will validate it if it is required for the
tenant. Thanks to Andrei Dmitriev for this suggestion.
Since we essentially have two different authz code paths in
zuul-web now, in order to share as much code as possible, the
authz sequence is refactored in such a way that the final authz
check can be deferred. First we create an AuthContext at the
start of the request which stores tenant and header information,
then the actual validation is performed in a separate step where
the token can optionally be provided.
In the http code path, we create the AuthContext and validate
immediately, using the Authorization header, and we do all of that
in the cherrypy tool at the start of the request.
In the websocket code path, we create the AuthContext as the
websocket handler is being created by the cherrypy request handler,
then we perform validation after receiving a message on the
websocket. We use the token supplied from the request.
Error handling is adjusted so in the http code path, exceptions
that return appropriate http errors are raised, but in the
websocket path, these are caught and translated into websocket
close calls.
A related issue is that we perform no validation that the
streaming build log being requested belongs to the tenant via
which the request is being sent. This was unecessary before
read-only access was an option, but now that it is, we should
check that a streaming build request arrives via the correct
tenant URL. This change adjusts that as well.
During testing, it was noted that the tenant configuration syntax
allows admin-rules and access-rules to use the scalar-or-list
pattern, however some parts of the code assumed only lists. The
configloader is updated to use scalar-or-list for both of those
values.
Change-Id: Ifd4c21bb1fe962bf23acb5b4f10b3bbaba61e63a
Co-Authored-By: Andrei Dmitriev <andrei.dmitriev@nokia.com>
The always-dynamic-branches option specifies a regex such that
branches that match it are ignored for Zuul configuration purposes,
unless a change is proposed, at which point the zuul.yaml config
is read from the branch in the same way as if a change was made
to the file.
Because creading and deleting dynamic branches do not cause
reconfigurations, the list of project branches stored on a tenant
may not be updated after a dynamic branch is created. This list
is used to decide from what branches to try to load config files.
Together, all of this means that if you create an always-dynamic-branch
and propose a change to it shortly afterwords, Zuul is likely to
ignore the change since it won't know to load configuration from
its branch.
To correct this, we extend the list of branches from which Zuul
knows to read configuration with the branch of the item under test
and any items ahead of it in the queue (but only if these branches
match the dynamic config regex so that we don't include an excluded
branch).
Also add a log entry to indicate when we are loading dynamic
configuration from a file.
Change-Id: Ibd15ce4a154311cdb523c5603f4ad17f761d1078
This change adds a warning (viewable in the web UI) when multiple
configurations are found for a project-branch combination.
Change-Id: I3defb2b1e6d7b59c450361c11d709802fe193373
To protect Zuul servers from accidental DoS attacks in case someone,
say, uploads a 1k change tree to gerrit, add an option to limit the
dependency processing in the Gerrit driver and in Zuul itself (since
those are the two places where we recursively process deps).
Change-Id: I568bd80bbc75284a8e63c2e414c5ac940fc1429a
This allows users to set a maximum value for the active window
in the event they have a project that has long stretches of
passing tests but they still don't want to commit too many resources
in case of a failure.
We should all be so lucky.
Change-Id: I52b5f3a9e7262b88fb16afc4520b35854e8df184
This adds a configuration warning (viewable in the web UI) for any
regular expressions found in in-repo configuration that can not
be compiled and executed with RE2.
Change-Id: I092b47e9b43e9548cafdcb65d5d21712fc6cc3af
The re2 library does not support negative lookahead expressions.
Expressions such as "(?!stable/)", "(?!master)", and "(?!refs/)" are
very useful branch specifiers with likely many instances in the wild.
We need to provide a migration path for these.
This updates the configuration options which currently accepts Python
regular expressions to additionally accept a nested dictionary which
allows specifying that the regex should be negated. In the future,
other options (global, multiline, etc) could be added.
A very few options are currently already compiled with re2. These are
left alone for now, but once the transition to re2 is complete, they
can be upgraded to use this syntax as well.
Change-Id: I509c9821993e1886cef1708ddee6d62d1a160bb0
The current "layout_lock" in the scheduler is really an "abide" lock.
We lock it every time we change something in the abide (including
tenant layouts). The name is inherited from pre-multi-tenant Zuul.
This can cause some less-than-optimal behavior when we need to wait to
acquire the "layout_lock" for a tenant reconfiguration event in one
thread while another thread holds the same lock because it is
reloading the configuration for a different tenant. Ideally we should
be able to have finer-grained tenant-level locking instead, allowing
for less time waiting to reconfigure.
The following sections describe the layout lock use prior to this
commit and how this commit adjusts the code to make it safe for
finer-grained locking.
1) Tenant iteration
The layout lock is used in some places (notably some cleanup methods)
to avoid having the tenant list change during the method. However,
the configloader already performs an atomic replacement of the tenant
list making it safe for iteration. This change adds a lock around
updates to the tenant list to prevent corruption if two threads update
it at the same time.
The semaphore cleanup method indirectly references the abide and
layout for use in global and local semaphores. This is just for path
construction, and the semaphores exist apart from the abide and layout
configurations and so should not be affected by either changing while
the cleanup method is running.
The node request cleanup method could end up running with an outdated
layout objects, including pipelines, however it should not be a
problem if these orphaned objects end up refreshing data from ZK right
before they are removed.
In these cases, we can simply remove the layout lock.
2) Protecting the unparsed project branch cache
The config cache cleanup method uses the unparsed project branch cache
(that is, the in-memory cache of the contents of zuul config files) to
determine what the active projects are.
Within the configloader, the cache is updated and then also used while
loading tenant configuration. The layout lock would have made sure
all of these actions were mutually exclusive. In order to remove the
layout lock here, we need to make sure the Abide's
unparsed_project_branch_cache is safe for concurrent updates.
The unparsed_project_branch_cache attribute is a dictionary that
conains references to UnparsedBranchCache objects. Previously, the
configloader would delete each UnparsedBranchCache object from the
dictionary, reinitialize it, then incrementially add to it.
This current process has a benign flaw. The branch cache is cleared,
and then loaded with data based on the tenant project config (TPC)
currently being processed. Because the cache is loaded based on data
from the TPC, it is really only valid for one tenant at a time despite
our intention that it be valid for the entire abide. However, since
we do check whether it is valid for a given TPC, and then clear and
reload it if it is not, there is no error in data, merely an
incomplete utilization of the cache.
In order to make the cache safe for use by different tenants at the
same time, we address this problem (and effectively make it so that it
is also *effective* for different tenants, even at different times).
The cache is updated to store the ltime for each entry in the cache,
and also to store null entries (with ltimes) for files and paths that
have been checked but are not present in the project-cache. This
means that at any given time we can determine whether the cache is
valid for a given TPC, and support multiple TPCs (i.e., multiple
tenants).
It's okay for the cache to be updated simultaneously by two tenants
since we don't allow the cache contents to go backwards in ltime. The
cache will either have the data with at least the ltime required, or
if not, that particular tenant load will spawn cat jobs and update it.
3) Protecting Tenant Project Configs (TPCs)
The loadTPC method on the ConfigLoader would similarly clear the TPCs
for a tenant, then add them back. This could be problematic for any
other thread which might be referencing or iterating over TPCs. To
correct this, we take a similar approach of atomic replacement.
Because there are two flavors of TPCs (config and untrusted) and they
are stored in two separate dictionaries, in order to atomically update
a complete tenant at once, the storage hierarchy is restructured as
"tenant -> {config/untrusted} -> project" rather than
"{config/untrusted} -> tenant -> project". A new class named
TenantTPCRegistry holds both flavors of TPCs for a given tenant, and
it is this object that is atomically replaced.
Now that these issues are dealt with, we can implement a tenant-level
thread lock that is used simply to ensure that two threads don't
update the configuration for the same tenant at the same time.
The scheduler's unparsed abide is updated in two places: upon full
reconfiguration, or when another scheduler has performed a full
reconfiguration and updated the copy in ZK. To prevent these two
methods from performing the same update simultaneously, we add an
"unparsed_abide_lock" and mutually exclude them.
Change-Id: Ifba261b206db85611c16bab6157f8d1f4349535d
This supplies a per-project default value for Zuul's default-branch
based on what the default branch is set to in GitHub. This means
that if users omit the default-branch setting on a Zuul project
stanza, Zuul will automatically use the correct value.
If the value in GitHub is changed, an event is emitted which allows
us to automatically reconfigure the tenant.
This could be expanded to other drivers that support an indication
of which branch is default.
Change-Id: I660376ecb3f382785d3bf96459384cfafef200c9
This allows users to trigger the new early failure detection by
matching regexes in the streaming job output.
For example, if a unit test job outputs something sufficiently
unique on failure, one could write a regex that matches that and
triggers the early failure detection before the playbook completes.
For hour-long unit test jobs, this could save a considerable amount
of time.
Note that this adds the google-re2 library to the Ansible venvs. It
has manylinux wheels available, so is easy to install with
zuul-manage-ansible. In Zuul itself, we use the fb-re2 library which
requires compilation and is therefore more difficult to use with
zuul-manage-ansible. Presumably using fb-re2 to validate the syntax
and then later actually using google-re2 to run the regexes is
sufficient. We may want to switch Zuul to use google-re2 later for
consistency.
Change-Id: Ifc9454767385de4c96e6da6d6f41bcb936aa24cd
Combining stdout/stderr in the result can lead to problems when e.g.
the stdout of a task is used as an input for another task.
This is also different from the normal Ansible behavior and can be
surprising and hard to debug for users.
The new behavior is configurable and off by default to retain backward
compatibility.
Change-Id: Icaced970650913f9632a8db75a5970a38d3a6bc4
Co-Authored-By: James E. Blair <jim@acmegating.com>
This adds the ability for the configuration loader to accumulate
warning-level configuration errors. Additionally, the
driver-specific triggers can do the same.
An initial pair of warnings is added for two recently deprecated
attributes on the gerrit trigger.
Change-Id: I61f856b3aea3e0ad3147c52c7aa724f40734d1f5
We pass around a single LoadingErrors object in quite a bit of the
config parser, and upcoming changes will pass it around even more.
Most of those instances also have a ParseContext as well, and the
two objects have the same lifecycle and scope, so let's attach the
LoadingErrors to the ParseContext and simplify things.
Change-Id: I57d0edc3c69eace9ba12c2975278a1b9312f9d67
Since project default branches were fixed in
Iddd5ef906c2bdbcc472615ffe25033c4ca961df7 the workaround in the
project detail view of the dashboard to distinguish actual project
configs from project-template configs will no longer work. Extend
the config model to incorporate a flag which indicates whether it
comes from a template or an actual project directly, and use that in
the dashboard rather than a (now broken) proxy value.
Change-Id: I2958a9bb01ca5261604773de742b6b652fca4cae
It can be confusing to know what merge modes are considered valid for a
project when Zuul tells you: "Merge mode foo is not supported by project
bar." Update this error message to include the list of merge modes that
Zuul considers to be valid for the project to reduce confusion.
Change-Id: If0d5cf1b06a93f3da507fba43077a5a9e327ca5e
This is the first in a series of changes to improve the usability
of the web view of config errors. The end goal is to be able to
display them in a more structured manner. A secondary goal is to
eventually add warnings (eg, deprecation warnings) which is
really only feasible if we have structured presentation of
errors.
This change does the following:
* Adds severity and error names to existing configuration errors
* And makes them available via the config-errors API endpoint
* Reduces the call sites for the error accumulator
(LoadingErrors.addError)
* Unifies the calling convention for the accumulator
(we stop passing in Exception objects)
Change-Id: Ia17dd3e7ad8cdfa8a07bb03b871078415d0c145e
So far we've used "master" as default branch when we encounter a project
configuration that doesn't have a default branch set. The problem here
is, that later project stanzas that do define a default branch won't
have any effect.
To fix this, we will only fall back on using "master" in the project
metadata if none of the project definitions declares a default branch.
Change-Id: Iddd5ef906c2bdbcc472615ffe25033c4ca961df7
Options such as always-dynamic-branches or exclude-unprotected-branches
can cause Zuul to toggle between using or not using implied branch matchers
as projects move between having one or more than one branch.
Let operators avoid that by specifying the intended behavior if necessary.
Change-Id: Ib8efa224fc396220ae85896845be4a908ac1008d
So far we've aborted all cat jobs when any of the cat jobs failed.
However, since we ignore those exceptions anyways unless we are
validating the tenant configuration, we should continue processing the
rest of the job results that haven't failed.
If an early cat job failed we might otherwise not load configuration
from repositories and branches even if those cat jobs were successful.
Change-Id: I34f2a23641de9138b1e887df86ae2602ca190277
PyYAML is efficient with YAML anchors and will only construct one
Python object for a YAML mapping that is used multiple times via
anchors.
We copy job variables (a mapping) straight from the YAML to an
attribute of the Job object, then we freeze the Job object which
converts all of its dicts to mappingproxy objects. This mutates
the contents of the vars dict, and if that is used on another
job via an anchor, we will end up with mappingproxy objects in
what we think is a "raw yaml" configuration dict, and we will not
be able to serialize it in case of errors.
To address this, perform a deep copy of every configuration yaml
blob before parsing it into an object.
Change-Id: Idaa6ff78b1ac5a108fb9f43700cf8e66192c43ce
It seems we never added serialization for ProjectContext objects. This
issue showed up after I3ed9a98dfc1ed63ac11025eb792c61c9a6414384 since we
use the ProjectContext for reporting errors related to the merge mode.
Since the ProjectContext and SourceContext are almost identical we can
use a SourceContext object with irrelevant fields set to None in places
where we currently use the ProjectContext.
ERROR zuul.Pipeline.tenant.check: [e: ...] Error in dynamic layout
Traceback (most recent call last):
File "/opt/zuul/lib/python3.10/site-packages/zuul/manager/__init__.py", line 1136, in _loadDynamicLayout
item.setConfigErrors(relevant_errors)
File "/opt/zuul/lib/python3.10/site-packages/zuul/model.py", line 5549, in setConfigErrors
self.current_build_set.setConfigErrors(errors)
File "/opt/zuul/lib/python3.10/site-packages/zuul/model.py", line 4129, in setConfigErrors
el = ConfigurationErrorList.new(self._active_context,
File "/opt/zuul/lib/python3.10/site-packages/zuul/zk/zkobject.py", line 224, in new
data = obj._trySerialize(context)
File "/opt/zuul/lib/python3.10/site-packages/zuul/zk/zkobject.py", line 249, in _trySerialize
return self.serialize(context)
File "/opt/zuul/lib/python3.10/site-packages/zuul/model.py", line 289, in serialize
"errors": [e.serialize() for e in self.errors],
File "/opt/zuul/lib/python3.10/site-packages/zuul/model.py", line 289, in <listcomp>
"errors": [e.serialize() for e in self.errors],
File "/opt/zuul/lib/python3.10/site-packages/zuul/model.py", line 254, in serialize
"key": self.key.serialize()
File "/opt/zuul/lib/python3.10/site-packages/zuul/model.py", line 211, in serialize
"context": self.context and self.context.serialize(),
AttributeError: 'ProjectContext' object has no attribute 'serialize'
Change-Id: I1b48abcd593685c332f3f218e2ed05cbcebfb244
This fixes a race condition when two schedulers request files for the
same repository at roughly the same time.
The files cache ltime is generated before creating the cat job. We also
cannot guarantee the order in which the job results are processed.
Since we include the files cache ltime in the min. ltimes as part
of the layout state we need to make sure that the timestamp of the cache
doesn't go backward.
This should fix the following exception we occassionally see in
zuul-web:
WARNING zuul.ConfigLoader: Zuul encountered an error while accessing the repo org/project.
The error was:
Configuration files missing from cache. Check Zuul scheduler logs for more information.
Change-Id: I9fe2ed0b7a95dd12bb978273673e905d06bee72d
We allow job.ansible-version to be a str, int, or float and coerce
it to a string. We should do the same for
tenant.default-ansible-version.
Change-Id: I1a104b84d578f4932597893f62cd0cd06f031b4a
Independent pipelines ignore requirements for non-live changes
because they are not actually executed. However, a user might
configure an independent pipeline that requires code review and
expect a positive code-review pipeline requirement to be enforced.
To ignore it risks executing unreviewed code via dependencies.
To correct this, we now enforce pipeline requirements in independent
pipelines in the same way as dependent ones.
This also adds a new "allow-other-connections" pipeline configuration
option which permits users to specify exhaustive pipeline requirements.
Change-Id: I6c006f9e63a888f83494e575455395bd534b955f
Story: 2010515
This updates the branch cache (and associated connection mixin)
to include information about supported project merge modes. With
this, if a project on github has the "squash" merge mode disabled
and a Zuul user attempts to configure Zuul to use the "squash"
mode, then Zuul will report a configuration syntax error.
This change adds implementation support only to the github driver.
Other drivers may add support in the future.
For all other drivers, the branch cache mixin simply returns a value
indicating that all merge modes are supported, so there will be no
behavior change.
This is also the upgrade strategy: the branch cache uses a
defaultdict that reports all merge modes supported for any project
when it first loads the cache from ZK after an upgrade.
Change-Id: I3ed9a98dfc1ed63ac11025eb792c61c9a6414384
This adds the ability to specify that the Zuul executor should
acquire a semaphore before running an individual playbook. This
is useful for long running jobs which need exclusive access to
a resources for only a small amount of time.
Change-Id: I90f5e0f570ef6c4b0986b0143318a78ddc27bbde
This allows configuration of read-only access rules, and corresponding
documentation. It wraps every API method in an auth check (other than
info endpoints).
It exposes information in the info endpoints that the web UI can use
to decide whether it should send authentication information for all
requests. A later change will update the web UI to use that.
Change-Id: I3985c3d0b9f831fd004b2bb010ab621c00486e05
In order to allow for authenticated read-only access to zuul-web,
we need to be able to control the authz of the API root. Currently,
we can only specify auth info for tenants. But if we want to control
access to the tenant list itself, we need to be able to specify auth
rules.
To that end, add a new "api-root" tenant configuration object which,
like tenants themselves, will allow attaching authz rules to it.
We don't have any admin-level API endpoints at the root, so this change
does not add "admin-rules" to the api-root object, but if we do develop
those in the future, it could be added.
A later change will add "access-rules" to the api-root in order to
allow configuration of authenticated read-only access.
This change does add an "authentication-realm" to the api-root object
since that already exists for tenants and it will make sense to have
that in the future as well. Currently the /info endpoint uses the
system default authentication realm, but this will override it if
set.
In general, the approach here is that the "api-root" object should
mirror the "tenant" object for all attributes that make sense.
Change-Id: I4efc6fbd64f266e7a10e101db3350837adce371f
GitHub supports a "rebase" merge mode where it will rebase the PR
onto the target branch and fast-forward the target branch to the
result of the rebase.
Add support for this process to the merger so that it can prepare
an effective simulated repo, and map the merge-mode to the merge
operation in the reporter so that gating behavior matches.
This change also makes a few tweaks to the merger to improve
consistency (including renaming a variable ref->base), and corrects
some typos in the similar squash merge test methods.
Change-Id: I9db1d163bafda38204360648bb6781800d2a09b4
James E. Blair