Selenium Hub Plugin

[mklaiber]

Repository URL

https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins

New Repository Name

selenium-hub-plugin

Description

Hi there,

I'm working for the small IT and Software developing company it.x informationssysteme gmbh located in Constance, Germany.
We recently developed a plugin which provides the Selenium hub directly on the Jenkins controller. Also, the Jenkins agents can be used as Selenium nodes, which makes the whole testing within the CI process much easier.

We decided to make the plugin public to the open source community that everyone can benefit from it.

We hope to hear from you soon!

Moritz

GitHub users to have commit permission

@mklaiber
@julianboehne
@itx-admin
@gschafra

Jenkins project users to have release permission

mklaiber

Issue tracker

GitHub issues

Automated release via GitHub Actions (recommended)

No

[jenkins-cert-app]

Security audit, information and commands

The security team is auditing all the hosting requests, to ensure a better security by default.

This message informs you that a Jenkins Security Scan was triggered on your repository.
It takes ~10 minutes to complete.

Commands

The bot will parse all comments, and it will check if any line start with a command.

Security team only:

• /audit-ok => the audit is complete, the hosting can continue 🎉.
• /audit-skip => the audit is not necessary, the hosting can continue 🎉.
• /audit-findings => the audit reveals some issues that require corrections ✏️.

Anyone:

• /request-security-scan => the findings from the Jenkins Security Scan were corrected, this command will re-scan your repository 🔍.
• /audit-review => the findings from the audit were corrected, this command will ping the security team to review the findings 👀. It's only applicable when the previous audit required changes.

Only one command can be requested per comment.

(automatically generated message, version: 1.35.6)

[github-actions]

Hello from your friendly Jenkins Hosting Checker

It appears you have some issues with your hosting request. Please see the list below and correct all issues marked Required. Your hosting request will not be approved until these issues are corrected. Issues marked with Warning or Info are just recommendations and will not stall the hosting process.

• ⛔ Required: The parent pom version '5.23' should be at least '5.24' or higher.
• ⛔ Required: The jelly file https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/adaadde006bd47f855bd5fe36ec31b5c62c69998/src/main/resources/selenium/plugin/SeleniumGlobalProperty/index.jelly contains a <script> tag with inline javascript.
• ⛔ Required: The following usernames in 'Jenkins project users to have release permission' need to log into Jira: mklaiber (reports are re-synced hourly, wait to re-check for a bit after logging in)
• ⛔ Required: The following usernames in 'Jenkins project users to have release permission' need to log into Artifactory: mklaiber (reports are re-synced hourly, wait to re-check for a bit after logging in)
• ⛔ Required: The following issues have been identified related to plugin dependency management:
  ** The artifactId bom-2.528.x of the bom is not in sync with the jenkins baseline version ``
• ⛔ Required: The file .github/CODEOWNERS doesn't contain the expected line * @jenkinsci/selenium-hub-plugin-developers
• ⛔ Required: The 'artifactId' from the pom.xml (selenium-plugin) is incorrect, it should be selenium-hub ('New Repository Name' field with "-plugin" removed)
• ⛔ Required: Please define the property jenkins.baseline and use this property in <jenkins.version>${jenkins.baseline}.3</jenkins.version> and the artifactId of the bom.
• ⛔ Required: Please define the property hpi.strictBundledArtifacts and set it to true. This should help prevent accidental library bundling when adding and updating dependencies.See Bundling third-party libraries.
• ⚠️ Warning: The jelly file https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/adaadde006bd47f855bd5fe36ec31b5c62c69998/src/main/resources/selenium/plugin/SeleniumGlobalProperty/index.jelly potentially uses inline javascript attribute onclick
• ℹ️ Info: One or more usages of inline javascript tags, style tags or event handlers have been identified. See https://www.jenkins.io/doc/developer/security/csp/ for more information how to make your jelly files CSP compliant

You can re-trigger a check by editing your hosting request or by commenting /hosting re-check

[jenkins-cert-app]

❌ Jenkins Security Scan failed.
The Security team was notified about this.

[mawinter69]

• I would propose to use 2.504.3 as the baseline, unless you have some good reason that you need the latest LTS
• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/adaadde006bd47f855bd5fe36ec31b5c62c69998/pom.xml#L57 The dependency to strcutsplugin is not needed
• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/adaadde006bd47f855bd5fe36ec31b5c62c69998/pom.xml#L106 Please remove the build section. It is not needed. Especially hard coding the version of the maven-hpi-plugin is bad. That version is set via the parent pom and that ensures you get a newer version when updating the parent pom.
• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/adaadde006bd47f855bd5fe36ec31b5c62c69998/pom.xml#L85,L99 Those repos are not needed
• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/adaadde006bd47f855bd5fe36ec31b5c62c69998/src/main/java/selenium/plugin/PropertyDashboardLink.java#L25 That link has a bad effekt as it will inject the link to the manage page for every user. But as a non admin clicking this link will just show me an empty page. As all the things there are anyway only accessible to persons with admin permissions, I think there is no need for this class. Admins can navigate there via the Manage Jenkins link. ( https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/adaadde006bd47f855bd5fe36ec31b5c62c69998/src/main/java/selenium/plugin/PropertyDashboardLink.java#L27 would be a problem anyway as a ManagementLink is already a singleton in Jenkins. So this would define a second instance).
• In general I would recommend to lower the permission to Jenkins.MANAGE. A permission that is not enabled by default currently but allows to grant users the permission to manage parts of Jenkins and I think this fits in this category very well.
• I suggest to make SeleniumGlobalProperty implement Describable<SeleniumGlobalProperty>. That way you can avoid that you have to set the select option yourself in the jelly. Instead you can just use f:select (see Select in Design Library). It's a bit tricky to get this working, most important you need to include

<j:set var="instance" value="${it}"/>
<j:set var="descriptor" value="${it.descriptor}"/>

in the jelly so Jenkins is able to read the existing value and finds the descriptor and move the doFill... to the descriptor class

• SeleniumGlobalProperty could define a Category so it is not under Uncategorized on the Manage Jenkins page with

    @NonNull
    @Override
    public Category getCategory() {
        return Category.TOOLS;
    }

• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/adaadde006bd47f855bd5fe36ec31b5c62c69998/src/main/java/selenium/plugin/SeleniumAgentPropertyLink.java#L29 That is not very reliable to detect if the node is the controller. Just do if (node instanceof Jenkins)
• Generally do not use hard coded colors. The tables are unreadable in dark theme due to this. Use the colors classes from Design Library. Then you can be sure that they fit for all themes.

• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/adaadde006bd47f855bd5fe36ec31b5c62c69998/src/main/resources/selenium/plugin/SeleniumGlobalProperty/index.jelly#L123 Would be good to use modern design for tables by following the Design Library guidelines. Now it looks a bit old fashioned.

• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/adaadde006bd47f855bd5fe36ec31b5c62c69998/src/main/resources/selenium/plugin/SeleniumGlobalProperty/index.jelly#L199 -> Banner

• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/main/src/main/webapp/48x48/selenium.png In dark theme the logo is barely visible. You could use the selenium-icon-solid from https://plugins.jenkins.io/oss-symbols-api/ with symbol-selenium-icon-solid plugin-oss-symbols-api in the getIconFileName methods

[mklaiber]

Hi @mawinter69,

Thank you very much for the very useful suggestions. I have now implemented your suggestions.

I had no idea about the Jenkins design library. I have made numerous improvements to the user interface and style changes to adapt as closely as possible to the design language. Even though UI is not my area of expertise, I hope that this look is more appropriate and ensures better clarity.

If you have more input, please let me know!

[mklaiber]

/hosting re-check

[github-actions]

Hello from your friendly Jenkins Hosting Checker

It appears you have some issues with your hosting request. Please see the list below and correct all issues marked Required. Your hosting request will not be approved until these issues are corrected. Issues marked with Warning or Info are just recommendations and will not stall the hosting process.

• ⛔ Required: The parent pom version '5.24' should be at least '5.28' or higher.
• ⛔ Required: The jelly file https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/adaadde006bd47f855bd5fe36ec31b5c62c69998/src/main/resources/selenium/plugin/SeleniumGlobalProperty/index.jelly contains a <script> tag with inline javascript.
• ⛔ Required: The following issues have been identified related to plugin dependency management:
  ** The bom version 5681.v79d2ddf61465 of bom-${jenkins.baseline}.x should be updated to the latest version 5701.va_b_018a_a_6b_0d3
• ⛔ Required: The file .github/CODEOWNERS doesn't contain the expected line * @jenkinsci/selenium-hub-plugin-developers
• ⚠️ Warning: The jelly file https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/adaadde006bd47f855bd5fe36ec31b5c62c69998/src/main/resources/selenium/plugin/SeleniumGlobalProperty/index.jelly potentially uses inline javascript attribute onclick
• ℹ️ Info: One or more usages of inline javascript tags, style tags or event handlers have been identified. See https://www.jenkins.io/doc/developer/security/csp/ for more information how to make your jelly files CSP compliant

You can re-trigger a check by editing your hosting request or by commenting /hosting re-check

[mklaiber]

/hosting re-check

[mawinter69]

/hosting re-check

[github-actions]

Hello from your friendly Jenkins Hosting Checker

It appears you have some issues with your hosting request. Please see the list below and correct all issues marked Required. Your hosting request will not be approved until these issues are corrected. Issues marked with Warning or Info are just recommendations and will not stall the hosting process.

• ⚠️ Warning: The jelly file https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/ff8435a1371a7732df2477b27a7f1adcc0f57f93/src/main/resources/selenium/plugin/SeleniumGlobalProperty/index.jelly potentially uses inline javascript attribute onclick
• ⚠️ Warning: The jelly file https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/ff8435a1371a7732df2477b27a7f1adcc0f57f93/src/main/resources/selenium/plugin/SeleniumGlobalProperty/index.jelly potentially uses inline javascript attribute onchange
• ℹ️ Info: One or more usages of inline javascript tags, style tags or event handlers have been identified. See https://www.jenkins.io/doc/developer/security/csp/ for more information how to make your jelly files CSP compliant

You can re-trigger a check by editing your hosting request or by commenting /hosting re-check

[mawinter69]

• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/ff8435a1371a7732df2477b27a7f1adcc0f57f93/src/main/java/selenium/plugin/SeleniumAgentAction.java#L108 change this to symbol-selenium-icon-solid plugin-oss-symbols-api, right now the image is not found.
• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/ff8435a1371a7732df2477b27a7f1adcc0f57f93/src/main/java/selenium/plugin/SeleniumGlobalProperty.java#L190 same here
• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/ff8435a1371a7732df2477b27a7f1adcc0f57f93/src/main/java/selenium/plugin/SeleniumGlobalProperty.java#L112 The doSave is not working. Needs to work with StaplerRequest to get the formdata and from there read the seleniumVersion
• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/ff8435a1371a7732df2477b27a7f1adcc0f57f93/src/main/java/selenium/plugin/SeleniumAgentPropertyLink.java#L29 See above. That is not a reliable way to decide if on the Controller or not.
• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/ff8435a1371a7732df2477b27a7f1adcc0f57f93/src/main/java/selenium/plugin/InboundAgentWatcher.java#L71 Here you also check unreliably if a computer is the controller
• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/ff8435a1371a7732df2477b27a7f1adcc0f57f93/src/main/java/selenium/plugin/SeleniumAgentAction.java#L56 That implementation is problematic. The method is called once on startup. It then sets up timer at a fixed rate for all existing agents. But what when agents are added later in time? And when an agent is deleted that timer would continue to run and save files. Maybe you can just have a single timer that then loops over all agents. Also instead of using an Initializer you could use https://javadoc.jenkins.io/hudson/model/PeriodicWork.html (or the asnyc variant, there is also https://javadoc.jenkins.io/hudson/model/AperiodicWork.html, though It think you need a fixed schedule)
• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/ff8435a1371a7732df2477b27a7f1adcc0f57f93/src/main/java/selenium/plugin/SeleniumGlobalProperty.java#L155 Could also use PeriodicWork
• You should also consider that when you have configured clouds, agents come and go frequently
• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/ff8435a1371a7732df2477b27a7f1adcc0f57f93/README.md?plain=1#L11 I think the section Development & Local Testing belongs to a separate file. The README is what users will see on plugins.jenkins.io and should focus on what the plugin is doing and how to configure and use it
• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/ff8435a1371a7732df2477b27a7f1adcc0f57f93/src/main/resources/selenium/plugin/SeleniumGlobalProperty/index.jelly#L24 The form has no submit buttons. Clicking on Start Hub opens a url in a new window but is not starting the hub

[mawinter69]

Mandatory changes:

• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/ff8435a1371a7732df2477b27a7f1adcc0f57f93/src/main/java/selenium/plugin/SeleniumAgentAction.java#L164 German text
• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/ff8435a1371a7732df2477b27a7f1adcc0f57f93/src/main/java/selenium/plugin/InboundAgentWatcher.java#L36 Please do not log on INFO level. That only spams the global Jenkins log
• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/ff8435a1371a7732df2477b27a7f1adcc0f57f93/src/main/java/selenium/plugin/SeleniumLifecycleHandler.java#L48 Use a logger and attach the exception. e.printStacktrace() writes to stderr that should generally be avoided
• clicking on the toggle switch of the built-in agent brings up the message Selenium action not found for agent . The reason is that Jenkins.get().getComputer("") will not select the controller. So you need special handling for that case.

Not mandatory for hosting:

• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/ff8435a1371a7732df2477b27a7f1adcc0f57f93/src/main/java/selenium/plugin/SeleniumAgentAction.java#L153 Why not store the config inside the agents home directory at <JENKINS_HOME>/nodes/<agent-name>/selenium-config.xml?
• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/ff8435a1371a7732df2477b27a7f1adcc0f57f93/src/main/java/selenium/plugin/InboundAgentWatcher.java#L37 This could pass the Computer to the so that the called method is not looping over all computers.
• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/ff8435a1371a7732df2477b27a7f1adcc0f57f93/src/main/java/selenium/plugin/InboundAgentWatcher.java#L35 Have you considere using preOnline? Probably makes not a big difference but technically starting before the agent will accept tasks seems to be the better approach.

Some improvements where I think seeing this later would improve the UX for users:

• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/main/src/main/resources/selenium/plugin/SeleniumGlobalProperty/index.jelly when you want more compact tables you can add the class jenkins-table--medium instead of putting padding everywhere
• Instead of having so many inline styles it is better to use a css file where you define the styling. (use <st:adjunct to load the js and css at the same time). Though with applying the jenkins-table--medium you can remove a lot of inline style already.
• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/ff8435a1371a7732df2477b27a7f1adcc0f57f93/src/main/resources/selenium/plugin/SeleniumGlobalProperty/index.jelly#L78 This can be greatly simplified by using <f:toggleSwitch .... You anyway need to remove the onchange attribute as this is inline javascript and not allowed with strict CSP rules

<f:toggleSwitch id="agent-selenium-toggle-${loop.index}" name="enabled" class="selenium-agent-toggle"
        title="${it.hasSeleniumServer(agent)?'%SeleniumGlobalProperty.jelly.active':'%SeleniumGlobalProperty.jelly.inactive'}"
        checked="${it.hasSeleniumServer(agent)}"/>

Plus points for not submitting the form but using fetch in javascript and do a post request to the endpoint to avoid the page reload.

• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/ff8435a1371a7732df2477b27a7f1adcc0f57f93/src/main/resources/selenium/plugin/SeleniumGlobalProperty/index.jelly#L3 use type="one-column". That looks better when there is no side-panel
• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/ff8435a1371a7732df2477b27a7f1adcc0f57f93/src/main/resources/selenium/plugin/SeleniumGlobalProperty/index.jelly#L256 I think the logs could look like the console output logs.

<pre class="console-output" style="display: flex; gap: 4px; flex-direction: column;">

And then remove all styling from the inner divs

• Not sure if nesting l:card is a good idea That adds quite some padding/margin at the bottom. Also the tables inside the l:card look not so nice as the table background is the same as the card background (at least with dark theme, note that in newer Jenkins versions a newer dark theme is used that might look better, haven't tested this) making the table header not stand out. Maybe just put a border around the things
• https://github.com/itx-informationssysteme/Selenium-Plugin_Jenkins/blob/ff8435a1371a7732df2477b27a7f1adcc0f57f93/src/main/resources/selenium/plugin/SeleniumGlobalProperty/index.jelly#L204 That could use f:advanced then you don't need any javascript to show the details. Just adds a line next to the opened area, but that could be easily removed with some css