Skip to content
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

JENKINS-74820 - forceSandBox - Hide command-launcher drop down from non-administrators #103

Merged
merged 17 commits into from
Nov 12, 2024

Conversation

jgarciacloudbees
Copy link
Contributor

@jgarciacloudbees jgarciacloudbees commented Nov 6, 2024

JENKINS-74820 - forceSandBox - Hide command-launcher drop down from non-administrators


Breaking changes:
This relies on jenkinsci/jenkins#9495 for managing error messages when saving Command-Launcher in non allowed cases (nonAdmin users saving an existing node with Launcher = commandLauncher when forceSandbox is enabled)

In order to do so, we have modified from:

@DataBoundConstructor
    public CommandLauncher(String command) {

To

@DataBoundConstructor
    public CommandLauncher(String command) throws Descriptor.FormException {

So in order to use it we need to modify the implementations using the "constructor CommandLauncher(String command)" to proper handle the exception "Descriptor.FormException"


Related to jenkinsci/script-security-plugin#585

Related to JENKINS-73941 - Option to hide "Use Groovy Sandbox" for users without Administer permission globally in the system.

When creating new nodes in the system based on the Launch Method "Launch agent via execution of command on the controller", the plugin Command-Launcher-Plugin does not support the Sandbox execution, so is automatically creating new scripts to be verified by the admin.

In the context of the forceSandbox changes implemented in JENKINS-73941, this is not allowed, so we should hide this option in this case.

We have implemented a new DescriptorVisibilityFilter to verify this case of use:

  • hudson.slaves.CommandLauncher.DescriptorVisibilityFilterForceSandBox

Tests Implemented:

  • New class hudson.slaves.CommandLauncherForceSandboxTest

This covers all the scenarios:

  • In case a new node is created, the user is non admin and forceSandbox is enabled, CommandLauncher Descriptor is not offered.
  • In any other creation scenario, CommandLauncher Descriptor is offered.
    *When editing a node, if the user is non admin and forceSandbox is enabled, the CommandLauncher Descriptor is not offered, except if the existing node already has this Launcher configured
  • In this case, if the nonAdmin user tries to save the Node definition with the launcher "CommandLauncher" it will fail with Descriptor.FormException. In case the user changes the descripfor to any other one, then he will be able to save it.

Manual tests:

  • When launching a node with CommandLauncher configured and forceSandbox enabled in the system, if the script is not approved, in the launch message, we don't see any reference to admin approval, but just informing the system requires the sandbox enabled.

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

CC: @jglick, @amuniz.

@jgarciacloudbees jgarciacloudbees marked this pull request as ready for review November 11, 2024 13:18
@jgarciacloudbees jgarciacloudbees requested a review from a team as a code owner November 11, 2024 13:18
src/main/java/hudson/slaves/CommandLauncher.java Outdated Show resolved Hide resolved
src/main/java/hudson/slaves/CommandLauncher.java Outdated Show resolved Hide resolved
src/main/java/hudson/slaves/CommandLauncher.java Outdated Show resolved Hide resolved
src/main/java/hudson/slaves/CommandLauncher.java Outdated Show resolved Hide resolved
src/main/java/hudson/slaves/CommandLauncher.java Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
@jgarciacloudbees jgarciacloudbees marked this pull request as draft November 12, 2024 16:26
@jgarciacloudbees jgarciacloudbees marked this pull request as ready for review November 12, 2024 18:14
@jglick jglick merged commit d85919c into jenkinsci:master Nov 12, 2024
17 checks passed
@jgarciacloudbees jgarciacloudbees deleted the BEE-52312 branch November 12, 2024 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants