-
Notifications
You must be signed in to change notification settings - Fork 12
Implement Webdriver #88
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
e66fd6b
8847aeb
f1dbfe3
5ec93d6
cd9e76c
3d95368
3839aba
4ff5e6d
ad35915
f775c83
0993c5d
7ae353d
e3ce68a
db21b0c
607d8be
39928f6
1f73866
fa8a428
e7fe006
0e0e18b
06a8ada
93ae7e1
5091bca
00a44a3
eb7932a
5a939eb
b149749
7def1f2
75b8b5d
6f4b2d6
25d8c49
97e1d35
fa49b5b
8cd0543
dbc1e40
dd78b54
409c8ca
f672452
1072953
d0bb9ab
6db0adb
c38387d
e7a8c7e
546d044
c4035f2
4b482f9
9e9e37a
e78609b
c14f7f1
8bd4c45
93c6069
a0c563f
fa9709e
61b8a99
0f849fd
ab1b456
3b17973
650c3a0
5569fc2
c8ee3eb
e37e127
6964ef9
9c945d8
c6fae99
2ecaf1d
324a698
d7f3243
73f5e33
4586699
fc9aedc
47fa6f7
114906e
b8e5d52
cf7750c
65986f2
b4923de
f9a2d00
c25f348
e2b7bbe
6c96603
1e7da88
2dd01ea
c0b9766
bd6a94e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,10 +6,21 @@ | |
| namespace Icinga\Module\Pdfexport\Controllers; | ||
|
|
||
| use Icinga\Application\Config; | ||
| use Icinga\Module\Pdfexport\Forms\ChromeBinaryForm; | ||
| use Icinga\Web\Controller; | ||
| use Icinga\Module\Pdfexport\Forms\BackendConfigForm; | ||
| use Icinga\Web\Form\ConfigSectionForm; | ||
| use Icinga\Web\Notification; | ||
| use Icinga\Web\Session; | ||
| use ipl\Html\Attributes; | ||
| use ipl\Html\Contract\Form; | ||
| use ipl\Html\HtmlString; | ||
| use ipl\Html\Table; | ||
| use ipl\Web\Compat\CompatController; | ||
| use Icinga\Web\Widget\Tabs; | ||
| use ipl\Web\Widget\ButtonLink; | ||
| use ipl\Web\Widget\Icon; | ||
| use ipl\Web\Widget\Link; | ||
|
|
||
| class ConfigController extends Controller | ||
| class ConfigController extends CompatController | ||
| { | ||
| public function init() | ||
| { | ||
|
|
@@ -18,14 +29,97 @@ public function init() | |
| parent::init(); | ||
| } | ||
|
|
||
| public function chromeAction() | ||
| public function backendsAction(): void | ||
| { | ||
| $form = (new ChromeBinaryForm()) | ||
| ->setIniConfig(Config::module('pdfexport')); | ||
| $button = new ButtonLink( | ||
| $this->translate('Create a New Backend'), | ||
| 'pdfexport/config/createbackend', | ||
| 'plus', | ||
| ['title' => $this->translate('Create a New Backend')], | ||
| ); | ||
| $button->setBaseTarget('_next'); | ||
| $this->addContent($button); | ||
|
|
||
| $form->handleRequest(); | ||
| $table = new Table(); | ||
| $table->setAttributes(Attributes::create([ | ||
| 'class' => 'table-row-selectable common-table', | ||
| 'data-base-target' => '_next', | ||
| ])); | ||
| $table->add(Table::tr([ | ||
| Table::th($this->translate('Backend')), | ||
| Table::th($this->translate('Priority')), | ||
| ])); | ||
|
|
||
| $this->view->tabs = $this->Module()->getConfigTabs()->activate('chrome'); | ||
| $this->view->form = $form; | ||
| $config = Config::module('pdfexport'); | ||
|
|
||
| $sections = []; | ||
| foreach ($config as $name => $data) { | ||
| $sections[] = [$name, $data, (int) $data->get('priority')]; | ||
| } | ||
|
|
||
| usort($sections, function ($a, $b) { | ||
| return $a[2] <=> $b[2]; | ||
| }); | ||
|
|
||
| foreach ($sections as [$name, $data]) { | ||
| $table->add(Table::tr([ | ||
| Table::td([ | ||
| new Icon('print'), | ||
| new Link($name, 'pdfexport/config/backend?backend=' . $name), | ||
| ]), | ||
| Table::td($data->get('priority')), | ||
| ], [ | ||
| 'class' => 'clickable', | ||
| ])); | ||
| } | ||
|
|
||
| $this->mergeTabs($this->Module()->getConfigTabs()->activate('backends')); | ||
| $this->addContent($table); | ||
| } | ||
|
|
||
| public function backendAction(): void | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Being able to rename a backend would be nice. See Icinga/icinga-sso-web#3
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would leave that up to Icinga/icingaweb2#5480
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't be necessary if you use random IDs as section keys like Icinga/icinga-sso-web#3.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think randomize IDs are necessary here. They make the config file hard to read and probably cause problems with configuration management. The INI section name is usually the name if the resource, so I implemented this exact behaviour in
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Being able to deactivate a backend temporarily for testing would be nice, but I guess changing priorities will do it as well. You decide.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Implemented |
||
| { | ||
| $name = $this->params->shiftRequired('backend'); | ||
| $this->addTitleTab($this->translate(sprintf('Edit %s', $name))); | ||
|
|
||
| $form = (new BackendConfigForm(Config::module('pdfexport'), $name)) | ||
| ->setCsrfCounterMeasureId(Session::getSession()->getId()) | ||
| ->on(Form::ON_SUBMIT, function () { | ||
| Notification::success($this->translate('Updated print backend')); | ||
| $this->redirectNow('__CLOSE__'); | ||
| }) | ||
| ->on(ConfigSectionForm::ON_DELETE, function () { | ||
| Notification::success($this->translate('Print backend deleted')); | ||
| $this->redirectNow('__CLOSE__'); | ||
| }) | ||
| ->on(ConfigSectionForm::ON_RENAME, function () { | ||
| Notification::success($this->translate('Print backend renamed')); | ||
| $this->redirectNow('__CLOSE__'); | ||
| }) | ||
| ->handleRequest($this->getServerRequest()); | ||
|
|
||
| $this->addContent(HtmlString::create($form->render())); | ||
| } | ||
|
|
||
| public function createbackendAction(): void | ||
| { | ||
| $this->addTitleTab($this->translate('Create Print Backend')); | ||
|
|
||
| $form = (new BackendConfigForm(Config::module('pdfexport'), null)) | ||
| ->setCsrfCounterMeasureId(Session::getSession()->getId()) | ||
| ->on(Form::ON_SUBMIT, function () { | ||
| Notification::success($this->translate('Created new print backend')); | ||
| $this->redirectNow('__CLOSE__'); | ||
| }) | ||
| ->handleRequest($this->getServerRequest()); | ||
|
|
||
| $this->addContent($form); | ||
| } | ||
|
|
||
| protected function mergeTabs(Tabs $tabs): void | ||
| { | ||
| foreach ($tabs->getTabs() as $tab) { | ||
| $this->tabs->add($tab->getName(), $tab); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,181 @@ | ||||||
| <?php | ||||||
|
|
||||||
| // SPDX-FileCopyrightText: 2019 Icinga GmbH <https://icinga.com> | ||||||
| // SPDX-License-Identifier: GPL-3.0-or-later | ||||||
|
|
||||||
| namespace Icinga\Module\Pdfexport\Forms; | ||||||
|
|
||||||
| use Exception; | ||||||
| use Icinga\Module\Pdfexport\Backend\Chromedriver; | ||||||
| use Icinga\Module\Pdfexport\Backend\Geckodriver; | ||||||
| use Icinga\Module\Pdfexport\Backend\HeadlessChromeBackend; | ||||||
| use Icinga\Web\Form\ConfigSectionForm; | ||||||
| use ipl\Validator\CallbackValidator; | ||||||
| use ipl\Web\Common\CsrfCounterMeasure; | ||||||
|
|
||||||
| class BackendConfigForm extends ConfigSectionForm | ||||||
| { | ||||||
| use CsrfCounterMeasure; | ||||||
|
|
||||||
| public function assemble(): void | ||||||
| { | ||||||
| $this->addElement('number', 'priority', [ | ||||||
| 'label' => $this->translate('Priority'), | ||||||
| 'required' => true, | ||||||
| 'placeholder' => 100, | ||||||
| 'min' => 0, | ||||||
| 'description' => $this->translate('The priority of the backend. A lower priority will be used first.'), | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This ordering seems counter-intuitive to me at first sight. @flourish86 What do you think?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the same order as menu items internally. To be fair afaik. it is the first time that a user would have to interact with the number directly. |
||||||
| ]); | ||||||
|
|
||||||
| $this->addElement('checkbox', 'enabled', [ | ||||||
| 'label' => $this->translate('Enabled'), | ||||||
| 'value' => true, | ||||||
| 'description' => $this->translate('If the backend is not enabled, it will be ignored for PDF generation.'), | ||||||
| ]); | ||||||
|
|
||||||
| $this->addElement('select', 'type', [ | ||||||
| 'label' => $this->translate('Type'), | ||||||
| 'multiOptions' => [ | ||||||
| '' => sprintf(' - %s - ', t('Please choose')), | ||||||
| 'chrome_webdriver' => t('Chrome WebDriver'), | ||||||
| 'firefox_webdriver' => t('Firefox WebDriver'), | ||||||
| 'remote_chrome' => t('Headless Chrome (Remote)'), | ||||||
| 'local_chrome' => t('Headless Chrome (Local)'), | ||||||
| ], | ||||||
| 'required' => true, | ||||||
| 'class' => 'autosubmit', | ||||||
| 'value' => 'local_chrome', | ||||||
| ]); | ||||||
|
|
||||||
| $type = $this->getValue('type'); | ||||||
|
Al2Klimov marked this conversation as resolved.
|
||||||
|
|
||||||
| switch ($type) { | ||||||
| case 'remote_chrome': | ||||||
| $this->addElement('text', 'host', [ | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So, you conditionally add elements, but no hidden ones? This can lose values on toggling |
||||||
| 'label' => $this->translate('Host'), | ||||||
| 'description' => $this->translate('Host address of the server with the running web browser.'), | ||||||
| 'required' => true, | ||||||
| 'validators' => [ | ||||||
| new CallbackValidator(function ($value, CallbackValidator $validator) { | ||||||
| $port = $this->getValue('port') ?: 9222; | ||||||
|
|
||||||
| try { | ||||||
| $chrome = HeadlessChromeBackend::createRemote($value, $port); | ||||||
| $version = $chrome->getVersion(); | ||||||
| } catch (Exception $e) { | ||||||
| $validator->addMessage($e->getMessage()); | ||||||
| return false; | ||||||
| } | ||||||
|
|
||||||
| if ($version < HeadlessChromeBackend::MIN_SUPPORTED_CHROME_VERSION) { | ||||||
| $validator->addMessage(t( | ||||||
| 'Chrome/Chromium supporting headless mode required' | ||||||
| . ' which is provided since version %s. Version detected: %s', | ||||||
| )); | ||||||
| return false; | ||||||
| } | ||||||
|
|
||||||
| return true; | ||||||
| }), | ||||||
| ], | ||||||
| ]); | ||||||
|
|
||||||
| $this->addElement('number', 'port', [ | ||||||
| 'label' => $this->translate('Port'), | ||||||
| 'description' => $this->translate('Port of the chrome developer tools. (Default: 9222)'), | ||||||
| 'placeholder' => 9222, | ||||||
| 'min' => 1, | ||||||
| 'max' => 65535, | ||||||
| ]); | ||||||
|
|
||||||
| break; | ||||||
|
|
||||||
| case 'local_chrome': | ||||||
| $this->addElement('text', 'binary', [ | ||||||
| 'label' => $this->translate('Binary'), | ||||||
| 'placeholder' => '/usr/bin/google-chrome', | ||||||
|
Al2Klimov marked this conversation as resolved.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (I can't evaluate it myself, as) both browsers run into a Gateway Timeout. |
||||||
| 'description' => $this->translate('Path to the binary of the web browser.'), | ||||||
| 'validators' => [ | ||||||
| new CallbackValidator(function ($value, CallbackValidator $validator) { | ||||||
| if (empty($value)) { | ||||||
| return true; | ||||||
| } | ||||||
|
|
||||||
| try { | ||||||
| $chrome = (HeadlessChromeBackend::createLocal($value)); | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| $version = $chrome->getVersion(); | ||||||
| } catch (Exception $e) { | ||||||
| $validator->addMessage($e->getMessage()); | ||||||
| return false; | ||||||
| } | ||||||
|
|
||||||
| if ($version < HeadlessChromeBackend::MIN_SUPPORTED_CHROME_VERSION) { | ||||||
| $validator->addMessage(t( | ||||||
| 'Chrome/Chromium supporting headless mode required' | ||||||
| . ' which is provided since version %s. Version detected: %s', | ||||||
| )); | ||||||
| } | ||||||
|
|
||||||
| return true; | ||||||
| }), | ||||||
| ], | ||||||
| ]); | ||||||
|
|
||||||
| $this->addElement('checkbox', 'force_temp_storage', [ | ||||||
| 'label' => $this->translate('Use temp storage'), | ||||||
| 'description' => $this->translate( | ||||||
| 'Use temp storage to transfer the html to the local chrome instance.' | ||||||
| ), | ||||||
| 'checkedValue' => '1', | ||||||
| 'uncheckedValue' => '0', | ||||||
| ]); | ||||||
|
|
||||||
| break; | ||||||
|
|
||||||
| case 'firefox_webdriver': | ||||||
| case 'chrome_webdriver': | ||||||
| $this->addElement('text', 'host', [ | ||||||
| 'label' => $this->translate('Host'), | ||||||
| 'description' => $this->translate('Host address of the webdriver server'), | ||||||
| 'required' => true, | ||||||
| 'validators' => [ | ||||||
| new CallbackValidator(function ($value, CallbackValidator $validator) use ($type) { | ||||||
| $port = $this->getValue('port') ?: 4444; | ||||||
|
|
||||||
| try { | ||||||
| $url = "$value:$port"; | ||||||
| $backend = match ($type) { | ||||||
| 'chrome_webdriver' => new Chromedriver($url), | ||||||
| 'firefox_webdriver' => new Geckodriver($url), | ||||||
| default => throw new Exception("Invalid webdriver type $type"), | ||||||
| }; | ||||||
|
|
||||||
| if (! $backend->isSupported()) { | ||||||
| $validator->addMessage( | ||||||
| t('The webdriver server reports that it is unable to generate PDFs'), | ||||||
| ); | ||||||
| return false; | ||||||
| } | ||||||
| } catch (Exception $e) { | ||||||
| $validator->addMessage($e->getMessage()); | ||||||
| return false; | ||||||
| } | ||||||
| return true; | ||||||
| }), | ||||||
| ], | ||||||
| ]); | ||||||
|
|
||||||
| $this->addElement('number', 'port', [ | ||||||
| 'label' => $this->translate('Port'), | ||||||
| 'description' => $this->translate('Port of the webdriver instance. (Default: 4444)'), | ||||||
| 'placeholder' => 4444, | ||||||
| 'min' => 1, | ||||||
| 'max' => 65535, | ||||||
| ]); | ||||||
|
|
||||||
| break; | ||||||
| } | ||||||
|
|
||||||
| $this->addCsrfCounterMeasure(); | ||||||
| } | ||||||
| } | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, /icingaweb2/config/userbackend displays vertical arrows next to multiple user backends and uses the order in the .ini file as prio. So the user doesn't have to manage such numbers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking about adding the drag-and-drop behavior like in kubernetes-web.
I would want that to be part of icingaweb or ipl-web first.