Merge pull request #2545 from Vlatombe/JENKINS-38187

[FIX JENKINS-38187] Fix setting JNLP port through system property (cherry picked from commit 3f5fe7f)
jenkinsci · Nov 3, 2016 · 5d58ce9 · 5d58ce9
1 parent 63c2036
commit 5d58ce9
Show file tree

Hide file tree

Showing 8 changed files with 130 additions and 6 deletions.
diff --git a/core/src/main/java/hudson/security/GlobalSecurityConfiguration.java b/core/src/main/java/hudson/security/GlobalSecurityConfiguration.java
@@ -49,6 +49,9 @@
 import net.sf.json.JSONObject;
 
 import org.jenkinsci.Symbol;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.DoNotUse;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
 
@@ -72,6 +75,15 @@ public int getSlaveAgentPort() {
         return Jenkins.getInstance().getSlaveAgentPort();
     }
 
+    /**
+     * @since TODO
+     * @return true if the slave agent port is enforced on this instance.
+     */
+    @Restricted(DoNotUse.class) // only for index.groovy
+    public boolean isSlaveAgentPortEnforced() {
+        return Jenkins.getInstance().isSlaveAgentPortEnforced();
+    }
+
     public Set<String> getAgentProtocols() {
         return Jenkins.getInstance().getAgentProtocols();
     }

diff --git a/core/src/main/java/jenkins/install/SetupWizard.java b/core/src/main/java/jenkins/install/SetupWizard.java
@@ -20,6 +20,7 @@
 import javax.servlet.http.HttpServletRequestWrapper;
 import javax.servlet.http.HttpServletResponse;
 
+import jenkins.util.SystemProperties;
 import org.acegisecurity.Authentication;
 import org.acegisecurity.context.SecurityContextHolder;
 import org.acegisecurity.providers.UsernamePasswordAuthenticationToken;
@@ -117,9 +118,9 @@ public class SetupWizard extends PageDecorator {
                     authStrategy.setAllowAnonymousRead(false);
                     jenkins.setAuthorizationStrategy(authStrategy);
 
-                    // Shut down all the ports we can by default:
-                    jenkins.setSlaveAgentPort(-1); // -1 to disable
-    
+                    // Disable jnlp by default, but honor system properties
+                    jenkins.setSlaveAgentPort(SystemProperties.getInteger(Jenkins.class.getName()+".slaveAgentPort",-1));
+
                     // require a crumb issuer
                     jenkins.setCrumbIssuer(new DefaultCrumbIssuer(false));
 

diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java
@@ -30,6 +30,7 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
+import com.google.inject.Inject;
 import com.google.inject.Injector;
 import com.thoughtworks.xstream.XStream;
 import hudson.BulkChange;
@@ -596,7 +597,16 @@ protected void onModified() throws IOException {
      * TCP agent port.
      * 0 for random, -1 to disable.
      */
-    private int slaveAgentPort = SystemProperties.getInteger(Jenkins.class.getName()+".slaveAgentPort",0);
+    private int slaveAgentPort = getSlaveAgentPortInitialValue(0);
+
+    private static int getSlaveAgentPortInitialValue(int def) {
+        return SystemProperties.getInteger(Jenkins.class.getName()+".slaveAgentPort", def);
+    }
+
+    /**
+     * If -Djenkins.model.Jenkins.slaveAgentPort is defined, enforce it on every start instead of only the first one.
+     */
+    private static final boolean SLAVE_AGENT_PORT_ENFORCE = SystemProperties.getBoolean(Jenkins.class.getName()+".slaveAgentPortEnforce", false);
 
     /**
      * The TCP agent protocols that are explicitly disabled (we store the disabled ones so that newer protocols
@@ -982,6 +992,9 @@ private Object readResolve() {
         if (jdks == null) {
             jdks = new ArrayList<>();
         }
+        if (SLAVE_AGENT_PORT_ENFORCE) {
+            slaveAgentPort = getSlaveAgentPortInitialValue(slaveAgentPort);
+        }
         return this;
     }
 
@@ -1091,11 +1104,26 @@ public int getSlaveAgentPort() {
         return slaveAgentPort;
     }
 
+    /**
+     * @since TODO
+     */
+    public boolean isSlaveAgentPortEnforced() {
+        return Jenkins.SLAVE_AGENT_PORT_ENFORCE;
+    }
+
     /**
      * @param port
      *      0 to indicate random available TCP port. -1 to disable this service.
      */
     public void setSlaveAgentPort(int port) throws IOException {
+        if (SLAVE_AGENT_PORT_ENFORCE) {
+            LOGGER.log(Level.WARNING, "setSlaveAgentPort({0}) call ignored because system property {1} is true", new String[] { Integer.toString(port), Jenkins.class.getName()+".slaveAgentPortEnforce" });
+        } else {
+            forceSetSlaveAgentPort(port);
+        }
+    }
+
+    private void forceSetSlaveAgentPort(int port) throws IOException {
         this.slaveAgentPort = port;
         launchTcpSlaveAgentListener();
     }
@@ -1206,6 +1234,38 @@ private void launchTcpSlaveAgentListener() throws IOException {
         }
     }
 
+    @Extension
+    @Restricted(NoExternalUse.class)
+    public static class EnforceSlaveAgentPortAdministrativeMonitor extends AdministrativeMonitor {
+        @Inject
+        Jenkins j;
+
+        @Override
+        public String getDisplayName() {
+            return jenkins.model.Messages.EnforceSlaveAgentPortAdministrativeMonitor_displayName();
+        }
+
+        public String getSystemPropertyName() {
+            return Jenkins.class.getName() + ".slaveAgentPort";
+        }
+
+        public int getExpectedPort() {
+            int slaveAgentPort = j.slaveAgentPort;
+            return Jenkins.getSlaveAgentPortInitialValue(slaveAgentPort);
+        }
+
+        public void doAct(StaplerRequest req, StaplerResponse rsp) throws IOException {
+            j.forceSetSlaveAgentPort(getExpectedPort());
+            rsp.sendRedirect2(req.getContextPath() + "/manage");
+        }
+
+        @Override
+        public boolean isActivated() {
+            int slaveAgentPort = Jenkins.getInstance().slaveAgentPort;
+            return SLAVE_AGENT_PORT_ENFORCE && slaveAgentPort != Jenkins.getSlaveAgentPortInitialValue(slaveAgentPort);
+        }
+    }
+
     public void setNodeName(String name) {
         throw new UnsupportedOperationException(); // not allowed
     }

diff --git a/core/src/main/resources/hudson/security/GlobalSecurityConfiguration/index.groovy b/core/src/main/resources/hudson/security/GlobalSecurityConfiguration/index.groovy
@@ -26,8 +26,18 @@ l.layout(norefresh:true, permission:app.ADMINISTER, title:my.displayName, csscla
             set("descriptor", my.descriptor);
 
             f.optionalBlock( field:"useSecurity", title:_("Enable security"), checked:app.useSecurity) {
-                f.entry (title:_("TCP port for JNLP agents"), field:"slaveAgentPort") {
-                    f.serverTcpPort()
+                f.entry(title: _("TCP port for JNLP agents"), field: "slaveAgentPort") {
+                    if (my.slaveAgentPortEnforced) {
+                        if (my.slaveAgentPort == -1) {
+                            text(_("slaveAgentPortEnforcedDisabled"))
+                        } else if (my.slaveAgentPort == 0) {
+                            text(_("slaveAgentPortEnforcedRandom"))
+                        } else {
+                            text(_("slaveAgentPortEnforced", my.slaveAgentPort))
+                        }
+                    } else {
+                        f.serverTcpPort()
+                    }
                 }
                 f.advanced(title: _("Agent protocols"), align:"left") {
                     f.entry(title: _("Agent protocols")) {

diff --git a/core/src/main/resources/hudson/security/GlobalSecurityConfiguration/index.properties b/core/src/main/resources/hudson/security/GlobalSecurityConfiguration/index.properties
@@ -0,0 +1,3 @@
+slaveAgentPortEnforced=enforced to {0,number,#} on startup through system property.
+slaveAgentPortEnforcedRandom=enforced to random port on startup through system property.
+slaveAgentPortEnforcedDisabled=disabled on startup through system property.
diff --git a/.../resources/jenkins/model/Jenkins/EnforceSlaveAgentPortAdministrativeMonitor/message.jelly b/.../resources/jenkins/model/Jenkins/EnforceSlaveAgentPortAdministrativeMonitor/message.jelly
@@ -0,0 +1,34 @@
+<!--
+The MIT License
+
+Copyright (c) 2016, CloudBees, Inc.
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in
+all copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+THE SOFTWARE.
+-->
+<?jelly escape-by-default='true'?>
+<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form" xmlns:i="jelly:fmt">
+    <div class="warning">
+        ${%description(it.systemPropertyName, it.expectedPort)}
+        <form method="post" action="${rootURL}/${it.url}/act" name="${it.id}">
+            <div style="float:right">
+                <f:submit name="reset" value="${%reset(it.expectedPort)}"/>
+            </div>
+        </form>
+    </div>
+</j:jelly>
diff --git a/...urces/jenkins/model/Jenkins/EnforceSlaveAgentPortAdministrativeMonitor/message.properties b/...urces/jenkins/model/Jenkins/EnforceSlaveAgentPortAdministrativeMonitor/message.properties
@@ -0,0 +1,2 @@
+description=JNLP Agent Port has been changed but was specified through system property {0} on startup. Its value will be reset to {1,number,#} on restart.
+reset=Reset to {0,number,#}
diff --git a/core/src/main/resources/jenkins/model/Messages.properties b/core/src/main/resources/jenkins/model/Messages.properties
@@ -68,3 +68,5 @@ ParameterizedJobMixIn.build_now=Build Now
 BlockedBecauseOfBuildInProgress.shortDescription=Build #{0} is already in progress{1}
 BlockedBecauseOfBuildInProgress.ETA=\ (ETA:{0})
 BuildDiscarderProperty.displayName=Discard old builds
+
+EnforceSlaveAgentPortAdministrativeMonitor.displayName=Enforce JNLP Slave Agent Port