From 950ba1c4e94cab32f5163675f74f78dac8ee1ae7 Mon Sep 17 00:00:00 2001 From: Marius Kittler Date: Wed, 10 Jul 2024 18:21:17 +0200 Subject: [PATCH] Prevent live view viewers from making openQA unresponsive * Avoid live view viewers from occupying too many Mojolicious workers by moving the image streaming of the live mode to the live handler daemon * Make sure the frontend code can still connect to the streaming route in a development setup by taking the different port into account and by setting CORS headers * Tested by running the main web UI service allowing only a limited number of connections via `script/openqa prefork -c 1 -w 1`. This causes it to be unresponsive if there is at least one live view tab open. With this change this is no longer a problem and multiple live tabs can be opened and are updated simultaneously. * Tested without and with reverse proxy. --- assets/javascripts/openqa.js | 30 ++++++++++++------- assets/javascripts/running.js | 22 ++++++++------ lib/OpenQA/LiveHandler.pm | 4 +++ .../{WebAPI => Shared}/Controller/Running.pm | 12 ++++++-- t/26-controllerrunning.t | 30 +++++++++---------- templates/webapi/test/live.html.ep | 2 +- 6 files changed, 63 insertions(+), 37 deletions(-) rename lib/OpenQA/{WebAPI => Shared}/Controller/Running.pm (95%) diff --git a/assets/javascripts/openqa.js b/assets/javascripts/openqa.js index 75345727c8c9..1eb79254c433 100644 --- a/assets/javascripts/openqa.js +++ b/assets/javascripts/openqa.js @@ -121,16 +121,9 @@ function reloadPage() { location.reload(); } -// returns an absolute "ws://" URL for the specified URL which might be relative -function makeWsUrlAbsolute(url, servicePortDelta) { - // don't adjust URLs which are already absolute - if (url.indexOf('ws:') === 0) { - return url; - } - - // read port from the page's current URL - var location = window.location; - var port = Number.parseInt(location.port); +function makeUrlPort(servicePortDelta) { + // read port from the location of the current page + let port = Number.parseInt(window.location.port); if (Number.isNaN(port)) { // don't put a port in the URL if there's no explicit port port = ''; @@ -138,7 +131,24 @@ function makeWsUrlAbsolute(url, servicePortDelta) { if (port !== 80 || port !== 443) port += servicePortDelta; port = ':' + port; } + return port; +} + +function makeUrlAbsolute(url, servicePortDelta) { + const location = window.location; + const port = makeUrlPort(servicePortDelta); + return location.protocol + '//' + location.hostname + port + (url.indexOf('/') !== 0 ? '/' : '') + url; +} + +// returns an absolute "ws://" URL for the specified URL which might be relative +function makeWsUrlAbsolute(url, servicePortDelta) { + // don't adjust URLs which are already absolute + if (url.indexOf('ws:') === 0) { + return url; + } + const location = window.location; + const port = makeUrlPort(servicePortDelta); return ( (location.protocol == 'https:' ? 'wss://' : 'ws:/') + location.hostname + diff --git a/assets/javascripts/running.js b/assets/javascripts/running.js index be640cb06c7f..ec173d0de1e6 100644 --- a/assets/javascripts/running.js +++ b/assets/javascripts/running.js @@ -221,7 +221,7 @@ function addDataListener(elem, callback) { // ensure any previously added event source is removed removeDataListener(elem); - // define callback function for response of OpenQA::WebAPI::Controller::Running::streamtext + // define callback function for response of OpenQA::Shared::Controller::Running::streamtext if (!elem.eventCallback) { elem.eventCallback = function (event) { // define max size of the live log @@ -241,7 +241,7 @@ function addDataListener(elem, callback) { var catData = currentData + newData; var newStartIndex = newLength - maxLiveLogLength; - // discard one (probably) partial line (in accordance with OpenQA::WebAPI::Controller::Running::streamtext) + // discard one (probably) partial line (in accordance with OpenQA::Shared::Controller::Running::streamtext) for (; newStartIndex < catData.length && catData[newStartIndex] !== '\n'; ++newStartIndex); firstElement.innerHTML = catData.substr(newStartIndex); @@ -309,13 +309,13 @@ var last_event; // loads a data-url img into a canvas function loadCanvas(canvas, dataURL) { - var context = canvas[0].getContext('2d'); + var context = canvas.getContext('2d'); // load image from data url var scrn = new Image(); scrn.onload = function () { - canvas[0].width = this.width; - canvas[0].height = this.height; + canvas.width = this.width; + canvas.height = this.height; context.clearRect(0, 0, this.width, this.width); context.drawImage(this, 0, 0); }; @@ -324,12 +324,16 @@ function loadCanvas(canvas, dataURL) { function initLivestream() { // setup callback for livestream - var livestream = $('#livestream'); - livestream.eventCallback = function (event) { + const livestream = document.getElementById('livestream'); + const servicePortDelta = Number.parseInt(document.getElementById('developer-panel').dataset.servicePortDelta); + const url = makeUrlAbsolute(livestream.dataset.url, servicePortDelta); + livestream.dataset.url = url; + const elements = $(livestream); + elements.eventCallback = function (event) { loadCanvas(livestream, event.data); last_event = event; }; - liveViewElements.push({log: livestream}); + liveViewElements.push({log: elements}); } function disableLivestream() { @@ -1125,7 +1129,7 @@ function processWsCommand(obj) { case 'error': // handle errors - // ignore connection errors if there's no running module according to OpenQA::WebAPI::Controller::Running::status + // ignore connection errors if there's no running module according to OpenQA::Shared::Controller::Running::status // or the test execution is stopped if ((!testStatus.running || developerMode.stoppingTestExecution) && category === 'cmdsrv-connection') { console.log('ignoring error from ws proxy: ' + what); diff --git a/lib/OpenQA/LiveHandler.pm b/lib/OpenQA/LiveHandler.pm index 4c5ed7fb9b68..9f426a596476 100644 --- a/lib/OpenQA/LiveHandler.pm +++ b/lib/OpenQA/LiveHandler.pm @@ -7,6 +7,7 @@ use Mojo::Base 'Mojolicious', -signatures; use OpenQA::Schema; use OpenQA::Log 'setup_log'; use OpenQA::Setup; +use OpenQA::Utils qw(service_port); has secrets => sub { shift->schema->read_application_secrets }; @@ -56,6 +57,9 @@ sub startup ($self) { $job_r->post('/upload_progress')->name('developer_post_upload_progress') ->to('live_view_handler#post_upload_progress'); + # register route for live streaming of image + $test_r->get('/streaming')->name('streaming')->to('running#streaming'); + OpenQA::Setup::setup_plain_exception_handler($self); } diff --git a/lib/OpenQA/WebAPI/Controller/Running.pm b/lib/OpenQA/Shared/Controller/Running.pm similarity index 95% rename from lib/OpenQA/WebAPI/Controller/Running.pm rename to lib/OpenQA/Shared/Controller/Running.pm index 7b4df550c934..7baa92b13d20 100644 --- a/lib/OpenQA/WebAPI/Controller/Running.pm +++ b/lib/OpenQA/Shared/Controller/Running.pm @@ -1,7 +1,7 @@ # Copyright 2014-2021 SUSE LLC # SPDX-License-Identifier: GPL-2.0-or-later -package OpenQA::WebAPI::Controller::Running; +package OpenQA::Shared::Controller::Running; use Mojo::Base 'Mojolicious::Controller', -signatures; use Mojo::Util 'b64_encode'; @@ -178,8 +178,16 @@ sub streaming ($self) { $self->render_later; Mojo::IOLoop->stream($self->tx->connection)->timeout(900); my $res = $self->res; + my $headers = $res->headers; $res->code(200); - $res->headers->content_type('text/event-stream'); + $headers->content_type('text/event-stream'); + + # set CORS headers required when not using a reverse proxy (so the port differs) + if ($self->is_local_request) { + my $req_origin = $self->req->url->base->clone->port(undef); + $headers->append(Vary => 'Origin'); + $headers->access_control_allow_origin("$req_origin:" . service_port('webui')); + } # setup a function to stop streaming again my $timer_id; diff --git a/t/26-controllerrunning.t b/t/26-controllerrunning.t index c177d75b9fb6..f38771489f7c 100644 --- a/t/26-controllerrunning.t +++ b/t/26-controllerrunning.t @@ -14,7 +14,7 @@ use lib "$FindBin::Bin/lib", "$FindBin::Bin/../external/os-autoinst-common/lib"; use DateTime; use Test::Warnings ':report_warnings'; use OpenQA::Test::TimeLimit '6'; -use OpenQA::WebAPI::Controller::Running; +use OpenQA::Shared::Controller::Running; use OpenQA::Jobs::Constants; use Test::MockModule; use Test::Output qw(combined_is combined_like); @@ -59,7 +59,7 @@ subtest streaming => sub { $id = Mojo::IOLoop->stream($stream); my $log = Mojo::Log->new; my $contapp = Mojolicious->new(log => $log); - my $controller = OpenQA::WebAPI::Controller::Running->new(app => $contapp); + my $controller = OpenQA::Shared::Controller::Running->new(app => $contapp); my $faketx = Mojo::Transaction::Fake->new(fakestream => $id); $log->unsubscribe('message'); $log->on(message => sub { my ($log, $level, @lines) = @_; $log_messages .= join "\n", @lines, '' }); @@ -89,7 +89,7 @@ subtest streaming => sub { # test image streaming $contapp->attr(schema => sub { FakeSchema->new() }); - $controller = OpenQA::WebAPI::Controller::Running->new(app => $contapp); + $controller = OpenQA::Shared::Controller::Running->new(app => $contapp); $faketx = Mojo::Transaction::Fake->new(fakestream => $id); $controller->tx($faketx); monkey_patch 'FakeSchema::Find', find => sub { Job->new }; @@ -122,13 +122,13 @@ subtest init => sub { my $not_found; my $render_specific_not_found; my $render; - monkey_patch 'OpenQA::WebAPI::Controller::Running', not_found => sub { $not_found = 1 }; - monkey_patch 'OpenQA::WebAPI::Controller::Running', + monkey_patch 'OpenQA::Shared::Controller::Running', not_found => sub { $not_found = 1 }; + monkey_patch 'OpenQA::Shared::Controller::Running', render_specific_not_found => sub { $render_specific_not_found = 1 }; - monkey_patch 'OpenQA::WebAPI::Controller::Running', reply => sub { shift }; - monkey_patch 'OpenQA::WebAPI::Controller::Running', render => sub { shift; $render = [@_] }; + monkey_patch 'OpenQA::Shared::Controller::Running', reply => sub { shift }; + monkey_patch 'OpenQA::Shared::Controller::Running', render => sub { shift; $render = [@_] }; - my $c = OpenQA::WebAPI::Controller::Running->new(app => $app); + my $c = OpenQA::Shared::Controller::Running->new(app => $app); $c->param(testid => 'foobar'); # No job could be found @@ -170,15 +170,15 @@ subtest edit => sub { my $not_found; my $render_specific_not_found; my $found; - monkey_patch 'OpenQA::WebAPI::Controller::Running', init => sub { 1 }; + monkey_patch 'OpenQA::Shared::Controller::Running', init => sub { 1 }; monkey_patch 'FakeSchema::Find', find => sub { undef }; - monkey_patch 'OpenQA::WebAPI::Controller::Running', not_found => sub { $not_found = 1 }; - monkey_patch 'OpenQA::WebAPI::Controller::Running', + monkey_patch 'OpenQA::Shared::Controller::Running', not_found => sub { $not_found = 1 }; + monkey_patch 'OpenQA::Shared::Controller::Running', render_specific_not_found => sub { $render_specific_not_found = 1 }; - monkey_patch 'OpenQA::WebAPI::Controller::Running', reply => sub { shift }; + monkey_patch 'OpenQA::Shared::Controller::Running', reply => sub { shift }; # No results - my $c = OpenQA::WebAPI::Controller::Running->new(app => $app); + my $c = OpenQA::Shared::Controller::Running->new(app => $app); $c->param('testid', "foobar"); $c->stash("job", Job->new); $c->edit(); @@ -187,8 +187,8 @@ subtest edit => sub { # Check if we can get the fake results my $details_count; monkey_patch 'FakeSchema::Find', find => sub { Job->new }; - monkey_patch 'OpenQA::WebAPI::Controller::Running', redirect_to => sub { $found = 1; $details_count = $_[5]; }; - $c = OpenQA::WebAPI::Controller::Running->new(app => $app); + monkey_patch 'OpenQA::Shared::Controller::Running', redirect_to => sub { $found = 1; $details_count = $_[5]; }; + $c = OpenQA::Shared::Controller::Running->new(app => $app); $c->param('testid', "foobar"); $c->stash("job", Job->new); $c->edit(); diff --git a/templates/webapi/test/live.html.ep b/templates/webapi/test/live.html.ep index e89f2fff3955..1237ab51de9d 100644 --- a/templates/webapi/test/live.html.ep +++ b/templates/webapi/test/live.html.ep @@ -193,7 +193,7 @@
- +