Remove rename change, block apply when changing name

car-roll · Aug 16, 2023 · 108bc22 · 108bc22
1 parent dbf61fd
commit 108bc22
Show file tree

Hide file tree

Showing 5 changed files with 12 additions and 104 deletions.
diff --git a/core/src/main/java/hudson/slaves/Cloud.java b/core/src/main/java/hudson/slaves/Cloud.java
@@ -311,34 +311,32 @@ public HttpResponse doDoDelete() throws IOException {
     }
 
     /**
-     * Accepts the update to the node name.
+     * Accepts the update to the node configuration.
      */
     @POST
-    public HttpResponse doRename(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException, Descriptor.FormException {
+    public HttpResponse doConfigSubmit(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException, Descriptor.FormException {
         checkPermission(Jenkins.ADMINISTER);
 
-        if (FormApply.isApply(req)) {
-            throw new Descriptor.FormException(jenkins.agents.Messages.Cloud_CannotApplyRename(), "name");
-        }
-
         Jenkins j = Jenkins.get();
         Cloud cloud = j.getCloud(this.name);
         if (cloud == null) {
             throw new ServletException("No such cloud " + this.name);
         }
         Cloud result = cloud.reconfigure(req, req.getSubmittedForm());
         String proposedName = result.name;
-        if (!proposedName.equals(this.name)
-                && j.getCloud(proposedName) != null) {
-            throw new Descriptor.FormException(jenkins.agents.Messages.CloudSet_CloudAlreadyExists(proposedName), "name");
+        if (!proposedName.equals(this.name)) {
+            if (FormApply.isApply(req)) {
+                throw new Descriptor.FormException(jenkins.agents.Messages.Cloud_CannotApplyRename(), "name");
+            } else if (j.getCloud(proposedName) != null) {
+                throw new Descriptor.FormException(jenkins.agents.Messages.CloudSet_CloudAlreadyExists(proposedName), "name");
+            }
         }
         j.clouds.replace(this, result);
         j.save();
-
         String reqPath = req.getOriginalRequestURI();
         String[] uriTokens = reqPath.replaceFirst("^/", "").split("/");
-        if (uriTokens.length < 3 || !"rename".equals(uriTokens[uriTokens.length - 1])) {
-            // We should never be here, expecting URI format jenkins/cloud/name/rename
+        if (uriTokens.length < 3 || !"configSubmit".equals(uriTokens[uriTokens.length - 1])) {
+            // We should never be here, expecting URI format jenkins/cloud/name/config
             throw new ServletException("Expected cloud rename URI: " + reqPath);
         }
         String cloudId = uriTokens[uriTokens.length - 2];
@@ -347,36 +345,10 @@ public HttpResponse doRename(StaplerRequest req, StaplerResponse rsp) throws IOE
             if (!proposedName.equals(this.name)) {
                 // name changed
                 cloudId = proposedName;
-            } else {
-                cloudId = this.name;
             }
         }
-
-        // take the user to the renamed cloud top page.
-        return FormApply.success("../" + cloudId);
-    }
-
-    /**
-     * Accepts the update to the node configuration. Node name is not allowed to be changed.
-     */
-    @POST
-    public HttpResponse doConfigSubmit(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException, Descriptor.FormException {
-        checkPermission(Jenkins.ADMINISTER);
-
-        Jenkins j = Jenkins.get();
-        Cloud cloud = j.getCloud(this.name);
-        if (cloud == null) {
-            throw new ServletException("No such cloud " + this.name);
-        }
-        Cloud result = cloud.reconfigure(req, req.getSubmittedForm());
-        String proposedName = result.name;
-        if (!proposedName.equals(this.name)) {
-            throw new Descriptor.FormException(jenkins.agents.Messages.Cloud_DoNotRename(), "name");
-        }
-        j.clouds.replace(this, result);
-        j.save();
         // take the user back to the cloud top page.
-        return FormApply.success(".");
+        return FormApply.success("../" + cloudId + '/');
     }
 
     public Cloud reconfigure(@NonNull final StaplerRequest req, JSONObject form) throws Descriptor.FormException {

diff --git a/core/src/main/resources/hudson/slaves/Cloud/rename.jelly b/core/src/main/resources/hudson/slaves/Cloud/rename.jelly
diff --git a/core/src/main/resources/hudson/slaves/Cloud/sidepanel.jelly b/core/src/main/resources/hudson/slaves/Cloud/sidepanel.jelly
@@ -30,9 +30,6 @@ THE SOFTWARE.
             <l:task contextMenu="false" href="." icon="symbol-computer" title="${%Status}"/>
             <l:task href="configure" icon="symbol-settings"
                     title="${app.hasPermission(app.ADMINISTER) ? '%Configure' : '%View Configuration'}"/>
-            <l:task href="rename" icon="symbol-changes"
-                    permission="${app.ADMINISTER}"
-                    title="${%Rename}"/>
             <l:delete permission="${app.ADMINISTER}" title="${%Delete Cloud}" message="${%delete.cloud(it.displayName)}"/>
             <t:actions />
         </l:tasks>

diff --git a/core/src/main/resources/jenkins/agents/Messages.properties b/core/src/main/resources/jenkins/agents/Messages.properties
@@ -4,5 +4,4 @@ CloudSet.SpecifyCloudToCopy=Specify which cloud to copy
 CloudSet.NoSuchCloud=No such cloud: {0}
 CloudsLink.DisplayName=Clouds
 CloudsLink.Description=Add, remove, and configure cloud instances to provision agents on-demand.
-Cloud.DoNotRename=Rename cloud using the "Rename" page.
 Cloud.CannotApplyRename=Cannot "apply" cloud rename, use "save".
diff --git a/test/src/test/java/hudson/slaves/CloudTest.java b/test/src/test/java/hudson/slaves/CloudTest.java
@@ -6,7 +6,6 @@
 import static org.hamcrest.Matchers.instanceOf;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 
 import edu.umd.cs.findbugs.annotations.NonNull;
@@ -23,7 +22,6 @@
 import jenkins.model.Jenkins;
 import jenkins.model.TransientActionFactory;
 import org.acegisecurity.acls.sid.Sid;
-import org.htmlunit.FailingHttpStatusCodeException;
 import org.htmlunit.html.HtmlForm;
 import org.htmlunit.html.HtmlPage;
 import org.htmlunit.html.HtmlTextInput;
@@ -96,26 +94,14 @@ public void cloudNameIsEncodedInGetUrl() {
     public void changeCloudName() throws Exception {
         ACloud aCloud = new ACloud("a", "0");
         j.jenkins.clouds.add(aCloud);
-        HtmlForm form = j.createWebClient().goTo(aCloud.getUrl() + "rename").getFormByName("config");
+        HtmlForm form = j.createWebClient().goTo(aCloud.getUrl() + "configure").getFormByName("config");
         HtmlTextInput input = form.getInputByName("_.name");
         input.setText("b");
         j.submit(form);
         ACloud actual = j.jenkins.clouds.get(ACloud.class);
         assertEquals("b", actual.getDisplayName());
     }
 
-    @Test
-    @Issue("JENKINS-71737")
-    public void changeCloudNameinConfig() throws Exception {
-        ACloud aCloud = new ACloud("a", "0");
-        j.jenkins.clouds.add(aCloud);
-        HtmlForm form = j.createWebClient().goTo(aCloud.getUrl() + "configure").getFormByName("config");
-        HtmlTextInput input = form.getInputByName("_.name");
-        input.setText("b");
-        Exception ex = assertThrows(FailingHttpStatusCodeException.class, () -> j.submit(form));
-        assertTrue(ex.getMessage().contains("Bad Request"));
-    }
-
     public static final class ACloud extends AbstractCloudImpl {
 
         @DataBoundConstructor