These two web auth exceptions produce output that's a little different
than the others, in that they duplicate the error and description
json fields. Use an error field that looks like the other auth
errors (which are unique short strings derived from class names).
This lets clients concatenate the two and produce a reasonable
output.
Change-Id: I1703444c19bfa0a06e11c3c521b7f46b31053d7b
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>
Since python 3.10 ssl.PROTOCOL_TLS has been deprecated. We are expected
to use ssl.PROTOCOL_TLS_CLIENT and ssl.PROTOCOL_TLS_SERVER depending on
how the sockets are to be used. Switch over to these new constants to
avoid the DeprecationWarning.
One thing to note is that PROTOCOL_TLS_CLIENT has default behaviors
around cert verification and hostname checking. Zuul is already
explicitly setting those options the way it wants to and I've left that
alone to avoid trouble if the defaults change later.
Finally, this doesn't fix the occurence of this error that happens
within kazoo. A separate PR has been made upstream to kazoo and this
should be fixed in the next kazoo release.
Change-Id: Ib41640f1d33d60503066464c8c98f865a74f003a
The fingergw (and its associated testing) was not properly managing ssl
sockets. The issue was we were in a context manager for the underlying
tcp socket which will get closed, but that doesn't call close() on the
ssl socket wrapping the tcp socket. Fix this by moving common recv()
code into a function then use the ssl socket in an inner context manager
if we are using ssl.
Both ssl and plain tcp will close() properly and we avoid duplicating
common code.
Change-Id: I1feefbd03a90734cf3c16baa6ed8f52cd8e00d14
The Zuul Ansible callback stream plugin assumed that the ansible loop
var was always called 'item' in the result_dict. You can override this
value (and it is often necessary to do so to avoid collisions) to
something less generic. In those cases we would get errors like:
b'[WARNING]: Failure using method (v2_runner_item_on_ok) in callback plugin'
b'(<ansible.plugins.callback.zuul_stream.CallbackModule object at'
b"0x7fbecc97c910>): 'item'"
And stream output would not include the info typically logged.
Address this by checking if ansible_loop_var is in the results_dict and
using that value for the loop var name instead. We still fall back to
'item' as I'm not sure that ansible_loop_var is always present.
Change-Id: I408e6d4af632f8097d63c04cbcb611d843086f6c
Currently, we are looking up the worker_zone for the log streaming from
the BuildRequest's path in ZooKeeper. This is a problem for unzoned
builds as those builds don't provide a zone information in their path
(zone=None).
Due to this, the log streaming won't use the FingerGateways and instead
always falls back to use the direct connection to the executor. This
works as long as the executor is located in the same region as zuul-web,
but in other cases the log streaming is broken.
To fix this, the executor will now store its zone information in the
worker_info of the BuildRequest when accepting the BuildRequest. In
the streamer_utils library we will use this zone information instead of
the zone from the ZooKeeper path.
Co-Authored-By: Simon Westphahl <simon.westphahl@bmw.de>
Change-Id: I63b148fa29e05157fce032d0f41b909da8a11e87
This adds matrics which report the number of thread workers in use
as well as the number of requests queued at the start of each
request in cherrypy.
It also reports the number of streamers currently running.
These can help us detect and diagnose problems with the web server.
Change-Id: Iadf9479ae84167892ab11ae122f275637c0c6c6f
It's conceivable that someone may want to use a public CA (like
letsencrypt) to provide certs for the finger gateway. In that
case, we should check hostnames. Add an option for that (and
make it the default).
Change-Id: I41f6f6f20ca9a4ecdb562e1760d8509e44b258f3
This combines the client and server certificate options to make
typical deployments simpler. The same certificate will be used by
a fingergw acting as a client or a server.
A new option is added to tell fingergw to use the cert only for
client use; that way a fingergw can act as an unencrypted end-user
gateway while still able to connect to encrypted servers.
The options are renamed to tls_* to match zookeeper; once gearman
is removed, we will have no ssl_* options.
Documentation and a release note for TLS fingergw support is added.
Change-Id: If3e445336de4644a5303f2ecc7c4a27e4320d042
When using fingergw for inter region log streaming it can be desirable
to support ssl encrypted connections with client auth just like we do
with gearman. This will also make it easy to route traffic to the
finger gateway via an openshift route using SNI and pass-through.
Docs and release note added in a subsequent change.
Change-Id: Ia5c739a3fcf229da140c4e2ebbe1a771c63b0489
This uses the component registry rather than gearman to perform
fingergw routing lookups. It also adjusts the logic for routing
to match the latest version of the spec, where unzoned fingergw
process are expected to route to zoned fingergws if they exist
(because the unzoned fingergw might be a public gateway outside
of the zone).
Change-Id: I2f9fed03159db59cc4e496802b9dab05f746e1a2
In some distributed deployments we need to route traffic via single
entry points that need to dispatch the traffic. For this use case make
all components aware of their zone so it is possible to compute if
traffic needs to go via an intermediate finger gateway or not.
Therefore we register the gearman function 'fingergw:info:<zone>' if
the fingergw is zoned. That way the scheduler will be able to route
streams from different zones via finger gateways that are responsible
for their zone.
Change-Id: I655427283205ea02de6f0f271b4aa5092ac05278
We currently read the config in the fingergw app and put the config
into the fingergw object. This gets ugly when adding more config
options so move evaluation of the config file into the FingerGateway
class. This is a preparation for adding ssl support to the
FingerGateway which will need more config options.
Depends-On: https://review.opendev.org/663413
Change-Id: I83f3863586b85f8befd84eb8f6079fa35ee3a8cb
Currently, the ZooKeeper connection is initialized directly in the cmd
classes like zuul.cmd.scheduler or zuul.cmd.merger and then passed to
the server instance.
Although this makes it easy to reuse a single ZooKeeper connection for
multiple components in the tests it's not very realistic.
A better approach would be to initialize the connection directly in the
server classes so that each component has its own connection to
ZooKeeper.
Those classes already get all necessary parameters, so we could get rid
of the additional "zk_client" parameter.
Furthermore it would allow us to use a dedicated ZooKeeper connection
for each component in the tests which is more realistic than sharing a
single connection between all components.
Change-Id: I12260d43be0897321cf47ef0c722ccd74599d43d
This mirrors the configuration in Nodepool for using TLS-enabled
ZooKeeper in tests. We use the ensure-zookeeper role in order
to get a newer ZooKeeper than is supplied in bionic.
Change-Id: I14413fccbc9a6a7a75b6233d667e2a1d2856d894
To make the tests more like running the apps, use fromConfig in the
tests rather than directly creating a client from a string connection
description. Note that since fromConfig requires TLS and the tests
don't use it, a temporary argument has been added to that method to
allow non-tls connections; it's only used within the test suite.
In future changes, we can configure the tests to use TLS connections,
remove the argument, and then move client instantiation into the
server classes themselves (which will remove additional differences
between test and production startup code).
Change-Id: Ic45c0e60c464ed8eeb44cb32bab039403f73ec69
The ZooKeeperClient now provides a fromConfig() method that parses all
necessary configuration values to instantiate a ZooKeeperClient.
Previously, this needed to be done in every component to initialize the
connection to ZooKeeper.
Change-Id: I5fa4ddab5f85c658291f1262ee0392a60086846e
Part of point 5 in https://etherpad.openstack.org/p/zuulv4
Connection is idle for now.
Also update component documentation.
Change-Id: I97a97f61940fab2a555c3651e78fa7a929e8ebfb
Each scheduler in tests needs its own connection registries.
This change only affects tests.
Change-Id: I2ad188cf5a72c46f7486c23ab653ff574123308b
Story: 2007192
Zuul uses the command processor for most components to send commands
to a running service. We should add this to zuul-web as well starting
with the stop command. In a later change we'll add commands for
starting/stopping a repl server.
Change-Id: I1d02ec30341be0890afb332dcf9f32f10a52ead5
When running test cases in the debugger it spits out resource warnings
about non-closed streams [1]. Explicitly closing the streams of the
subprocesses and log streamer fixes this warning. Maybe this even
solves the memory leak we're currently seeing in the executors.
[1] Trace:
/zuul/executor/server.py:1894: ResourceWarning: unclosed file <_io.BufferedReader name=60>
self.proc = None
ResourceWarning: Enable tracemalloc to get the object allocation traceback
/zuul/executor/server.py:124: ResourceWarning: unclosed file <_io.BufferedReader name=11>
stdout=subprocess.PIPE, stderr=subprocess.DEVNULL)
ResourceWarning: Enable tracemalloc to get the object allocation traceback
Change-Id: I65f191dc2e50f9c84f5bf6a3967d768d7ebe6b04
We special case ansible debug tasks with msg parameters to bypass normal
logging. This meant that multiline debug messages were passed through
verbatim and didn't get per line timestamps and log formatting. This is
not consistent with the rest of our logging so we update it to log each
line in a multi line debug message separately.
Change-Id: Iacb5a101b2a2b971f631e9a14caa51899495ea2d
Currently, if a websocket client hangs up, we won't notice until
the next time we try to send data. But ws4py has a callback for
informing us of a hangup sooner. Use that.
Change-Id: Ifbd28161f7fc77cd1dc01883b132366a801b76ef
A recent change to scope sql connections to tenants was incorrect
because it assumed zuul-web would have access to a tenant's layout,
but that is only in the zuul-scheduler's memory. Instead, this change
causes zuul-web to perform an RPC call to the scheduler to ask which
connection to use before performing sql queries.
This slipped through testing because the zuul web test fixture used
the same connection registry object as the scheduler, and therefore
*was* able to access data which normally would not be available to it.
This updates the zuul web test fixture to use a separate connection
registry.
Also, because zuul-web doesn't need any other kinds of connections at
the moment, add a facility to allow it to only load sql connections,
so that it doesn't do anything with the gerrit or github drivers.
Change-Id: Iac6d8c6960f4a31eb2f78511664b704758e41687
The ws4py websocket manager assumes that the received_message
method will return quickly. It maintains a single thread for
handling all websockets, but the updated websocket handler in
zuul-web assumed it would run in a thread for each request.
To correct this, start a new single thread in the ZuulWeb object
which handles ongoing finger streaming duties for all websocket
streaming threads. Whenver a new finger streaming websocket
request is handled, that finger socket is added to a poll list
in the new thread, and any newly received data is forwarded to
its associated websocket.
The test_websocket_streaming unit test is altered to start two
client simultaneously to verify the new behavior.
Change-Id: I505b05b98c2c8fa1b3e875cd5e9266e9929f1367
This reduces duplication and increases consistency.
Also, an extra logfile.close() call is removed in test_streaming
(there is a cleanup handler which closes the file).
Change-Id: I940ba07a48bade03b6b1f3ced9621d156f7755ae
* Aiohttp (and related libraries) have a python support policy
which is causing us problems.
* Cherrypy supports threads which integrates well with the rest
of Zuul.
Change-Id: Ib611df06035890d3e87fc5ad92fdfc7ac441edce
The only rest API endpoint that uses sql queries is
/api/tenant/{tenant}/builds. There's no connection in there, which
means it doesn't make sense for that to be attached to a sql connection
(which is currently the case). Moreover, it doesn't make sense for
*every* tenant's endpoint to be attached to the *same* connection.
In other words, the current situation only allows for a single sql
connection system-wide, even if someone is using different connections
per tenant.
Moving the handler for the endpoint into the sql driver means that it
can dispatch the query to the appropriate connection for a given tenant
(since a tenant is always implied by the REST endpoint).
Moreover, the *rest* of the system actually allows multiple connections
within a single tenant, and we should support that here, but I don't
immediately have a solution of how to handle pagination across queries
that span multiple connections. This is an improvement in that it is
now tenant-scoped, but it's not ideal.
This also removes the (undocumented!) sql_connection_name config file
option.
It also uses the tenant name from the path to constructe the query so
that it always includes the correct tenant (this eliminates the
inadvertant ability for one tenant to query another tenant's builds).
The internal API here isn't great, but it will get cleaned up in the
next patch which converts to cherrypy.
Change-Id: Ie1f19f0b392d4c010ef43dc6220ff1c8667f5a4a
Change ID I032f15eef15a26287d0d912d6ef105cf5beae200 introduced a test
using the same port (9000) being used in another test. This is going
to cause random test flaps. Use random ports.
Change-Id: I209ca0923cfd05038ef9f4bc14523af8cc9f2dd4
The existing structure with the API and HTML interleaved makes it hard
to have apache do html offloading for whitelabeled deploys (like
openstack's) where the api and web are served from the same host.
Similarly, the tenant selector in the url for the html content being bare
and not prefixed by anything makes it harder to pull routing information
javascript-side.
Introduce an 'api' prefix to the REST API calls so that we can apply
rewrite rules differently for things starting with /api than things that
don't. Add the word 'tenant' before each tenant subpath.
Also add a '/t/' to the url for the html, so that we have anchors for
routing regexes but the urls don't get too long and unweildy.
Finally, also add /key as a prefix to the key route for similar reasons.
Change-Id: I0cbbf6f1958e91b5910738da9b4fb4c563d48dd4
As a follow to our previous commit, add both ipv4 / ipv6 unit test
coverage to expose:
[Errno -9] Address family for hostname not supported
exception. For the purpose of testing we can use ::1 and 127.0.0.1 as
ipv6 / ipv4 addresses.
Change-Id: Ib9b1729647657f3735dd25059363b70aa0e2e6e0
Signed-off-by: Paul Belanger <pabelanger@redhat.com>
yarn drives package and dependency management. webpack handles
bundling, minification and transpiling down to browser-acceptable
javascript but allows for more modern javascript like import statements.
There are some really neat things in the webpack dev server. CSS
changes, for instance, get applied immediately without a refresh. Other
things, like the jquery plugin do need a refresh, but it's handled just
on a file changing.
As a followup, we can also consider turning the majority of the status page
into a webpack library that other people can depend on as a mechanism
for direct use. Things like that haven't been touched because allowing
folks to poke at the existing known status page without too many changes
using the tools seems like a good way for people to learn/understand the
stack.
Move things so that the built content gets put
into zuul/web/static so that the built-in static serving from zuul-web
will/can serve the files.
Update MANIFEST.in so that if npm run build:dist is run before the
python setup.py sdist, the built html/javascript content will be
included in the source tarball.
Add a pbr hook so that if yarn is installed, javascript content will be
built before the tarball.
Add a zuul job with a success url that contains a source_url
pointing to the live v3 data.
This adds a framework for verifying that we can serve the web app
urls and their dependencies for all of the various ways we want to
support folks hosting zuul-web.
It includes a very simple reverse proxy server for approximating
what we do in openstack to "white label" the Zuul service -- that
is, hide the multitenancy aspect and present the single tenant
at the site root.
We can run similar tests without the proxy to ensure the default,
multi-tenant view works as well.
Add babel transpiling enabling use of ES6 features
ECMAScript6 has a bunch of nice things, like block scoped variables,
const, template strings and classes. Babel is a javascript transpiler
which webpack can use to allow us to write using modern javascript but
the resulting code to still work on older browsers.
Use the babel-plugin-angularjs-annotate so that angular's dependency
injection doesn't get borked by babel's transpiling things (which causes
variables to otherwise be renamed in a way that causes angular to not
find them)
While we're at it, replace our use of var with let (let is the new
block-scoped version of var) and toss in some use of const and template
strings for good measure.
Add StandardJS eslint config for linting
JavaScript Standard Style is a code style similar to pep8/flake8. It's
being added here not because of the pep8 part, but because the pyflakes
equivalent can catch real errors. This uses the babel-eslint parser
since we're using Babel to transpile already.
This auto-formats the existing code with:
npm run format
Rather than using StandardJS directly through the 'standard' package,
use the standardjs eslint plugin so that we can ignore the camelCase
rule (and any other rule that might emerge in the future)
Many of under_score/camelCase were fixed in a previous version of the patch.
Since the prevailing zuul style is camelCase methods anyway, those fixes
were left. That warning has now been disabled.
Other things, such as == vs. === and ensuring template
strings are in backticks are fixed.
Ignore indentation errors for now - we'll fix them at the end of this
stack and then remove the exclusion.
Add a 'format' npm run target that will run the eslint command with
--fix for ease of fixing reported issues.
Add a 'lint' npm run target and a 'lint' environment that runs with
linting turned to errors. The next patch makes the lint environment more
broadly useful.
When we run lint, also run the BundleAnalyzerPlugin and set the
success-url to the report.
Add an angular controller for status and stream page
Wrap the status and stream page construction with an angular controller
so that all the javascripts can be bundled in a single file.
Building the files locally is wonderful and all, but what we really want
is to make a tarball that has the built code so that it can be deployed.
Put it in the root source dir so that it can be used with the zuul
fetch-javascript-tarball role.
Also, replace the custom npm job with the new build-javascript-content
job which naturally grabs the content we want.
Make a 'main.js' file that imports the other three so that we just have
a single bundle. Then, add a 'vendor' entry in the common webpack file
and use the CommonsChunkPlugin to extract dependencies into their own
bundle. A second CommonsChunkPlugin entry pulls out a little bit of
metadata that would otherwise cause the main and vendor chunks to change
even with no source change. Then add chunkhash into the filename. This
way the files themselves can be aggressively cached.
This all follows recommendations from https://webpack.js.org/guides/caching/https://webpack.js.org/guides/code-splitting/ and
https://webpack.js.org/guides/output-management/
Change-Id: I2e1230783fe57f1bc3b7818460463df1e659936b
Co-Authored-By: Tristan Cacqueray <tdecacqu@redhat.com>
Co-Authored-By: James E. Blair <jeblair@redhat.com>
Now that we have a finger gateway, we no longer need to start the
executor as root so that the finger streamer on the executor can
bind to port 79 (default port for the finger streamer is changed
from 79 to 7900). Remove that requirement.
Change-Id: I6df685044c4ce81fd263043adba832609da100af
This adds the zuul-fingergw app that should be run as root (so that
it can connect to the standard finger port 79), but changes user privs
immediately after binding that port.
Common streaming functions have been moved to streamer_utils.py to
be shared among modules.
Support for CommandSocket has been included.
Change-Id: Ia35492fe951e7b9367eeab0b145d96189d72c364
James E. Blair