From 302eb5dad6439842e076fead5bd76f20f615e6a8 Mon Sep 17 00:00:00 2001
From: Dong Ma <winterma.dong@gmail.com>
Date: Tue, 8 Dec 2015 17:49:38 +0800
Subject: [PATCH] Fixed create_node() Error

Fixed create_node() raises HTTP Error 400 Bad Request

This issue caused by:
    self.jenkins_open(Request(
        self._build_url(CREATE_NODE, params), b''))
The "params" dictionary of options isn't parsed correctly
during _build_url funciton that Jenkins is expecting

Fixed it use urlencode to parse the params and construct
the URL

Change-Id: I85fa43bc0b4ff1530133b862734e94aede9b44a5
Closes-Bug: #1500898
---
 doc/source/examples.rst | 15 ++++++++++++++
 jenkins/__init__.py     |  5 ++++-
 tests/test_node.py      | 45 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/doc/source/examples.rst b/doc/source/examples.rst
index d20dbb8..812343e 100644
--- a/doc/source/examples.rst
+++ b/doc/source/examples.rst
@@ -104,6 +104,21 @@ This is an example showing how to add, configure, enable and delete Jenkins node
     server.disable_node('slave1')
     server.enable_node('slave1')
 
+    # create node with parameters
+    params = {
+        'port': '22',
+        'username': 'juser',
+        'credentialsId': '10f3a3c8-be35-327e-b60b-a3e5edb0e45f',
+        'host': 'my.jenkins.slave1'
+    }
+    server.create_node(
+        'slave1',
+        nodeDescription='my test slave',
+        remoteFS='/home/juser',
+        labels='precise',
+        exclusive=True,
+        launcher=jenkins.LAUNCHER_SSH,
+        launcher_params=params)
 
 Example 6: Working with Jenkins Build Queue
 -------------------------------------------
diff --git a/jenkins/__init__.py b/jenkins/__init__.py
index 28fa714..34cae71 100644
--- a/jenkins/__init__.py
+++ b/jenkins/__init__.py
@@ -240,7 +240,10 @@ class Jenkins(object):
     def _build_url(self, format_spec, variables=None):
 
         if variables:
-            url_path = format_spec % self._get_encoded_params(variables)
+            if format_spec == CREATE_NODE:
+                url_path = format_spec % urlencode(self._get_encoded_params(variables))
+            else:
+                url_path = format_spec % self._get_encoded_params(variables)
         else:
             url_path = format_spec
 
diff --git a/tests/test_node.py b/tests/test_node.py
index 9688af0..4d28241 100644
--- a/tests/test_node.py
+++ b/tests/test_node.py
@@ -208,6 +208,51 @@ class JenkinsCreateNodeTest(JenkinsNodesTestBase):
         self.assertTrue(self.j.node_exists('test node'))
         self._check_requests(jenkins_mock.call_args_list)
 
+    @patch.object(jenkins.Jenkins, 'jenkins_open')
+    def test_urlencode(self, jenkins_mock):
+        jenkins_mock.side_effect = [
+            None,
+            None,
+            json.dumps(self.node_info),
+            json.dumps(self.node_info),
+        ]
+        params = {
+            'port': '22',
+            'username': 'juser',
+            'credentialsId': '10f3a3c8-be35-327e-b60b-a3e5edb0e45f',
+            'host': 'my.jenkins.slave1'
+        }
+        self.j.create_node(
+            'slave1',
+            nodeDescription='my test slave',
+            remoteFS='/home/juser',
+            labels='precise',
+            exclusive=True,
+            launcher=jenkins.LAUNCHER_SSH,
+            launcher_params=params)
+
+        actual = jenkins_mock.call_args_list[1][0][0].get_full_url()
+        # As python dicts do not guarantee order so the parameters get
+        # re-ordered when it gets processed by _get_encoded_params(),
+        # verify sections of the URL with self.assertIn() instead of
+        # the entire URL
+        self.assertIn(u'username%22%3A+%22juser', actual)
+        self.assertIn(
+            u'stapler-class%22%3A+%22hudson.plugins.sshslaves.SSHLauncher',
+            actual)
+        self.assertIn(u'host%22%3A+%22my.jenkins.slave1', actual)
+        self.assertIn(
+            u'credentialsId%22%3A+%2210f3a3c8-be35-327e-b60b-a3e5edb0e45f',
+            actual)
+        self.assertIn(u'port%22%3A+%2222', actual)
+        self.assertIn(u'remoteFS%22%3A+%22%2Fhome%2Fjuser', actual)
+        self.assertIn(u'labelString%22%3A+%22precise', actual)
+        self.assertIn(u'name%22%3A+%22slave1', actual)
+        self.assertIn(
+            u'type%22%3A+%22hudson.slaves.DumbSlave%24DescriptorImpl',
+            actual)
+        self._check_requests(jenkins_mock.call_args_list)
+
     @patch.object(jenkins.Jenkins, 'jenkins_open')
     def test_already_exists(self, jenkins_mock):
         jenkins_mock.side_effect = [