From cd237fbcad92c8496da7b36f5196cf45cf177cfc Mon Sep 17 00:00:00 2001 From: Darragh Bailey Date: Fri, 16 Jun 2017 12:12:31 +0100 Subject: [PATCH] Pass form data as body instead of in the url Switch create node to pass form data via request body, removing the need for special handling in the _build_url method. Consequently remove the string placeholder in the endpoint format string. Remove unnecessary parameters of name and type from inclusion in the json form data item, confirming they are ignored and required to be set in the main form data. Tested against jenkins 2.19.4. Change-Id: I5f8f870a2ee3539505e984abccb0c87c632c1b9a --- jenkins/__init__.py | 12 ++++-------- tests/test_node.py | 19 +++++-------------- 2 files changed, 9 insertions(+), 22 deletions(-) diff --git a/jenkins/__init__.py b/jenkins/__init__.py index 77ffd31..dbb8e1c 100755 --- a/jenkins/__init__.py +++ b/jenkins/__init__.py @@ -110,7 +110,7 @@ BUILD_WITH_PARAMS_JOB = '%(folder_url)sjob/%(short_name)s/buildWithParameters' BUILD_INFO = '%(folder_url)sjob/%(short_name)s/%(number)d/api/json?depth=%(depth)s' BUILD_CONSOLE_OUTPUT = '%(folder_url)sjob/%(short_name)s/%(number)d/consoleText' NODE_LIST = 'computer/api/json' -CREATE_NODE = 'computer/doCreateItem?%s' +CREATE_NODE = 'computer/doCreateItem' DELETE_NODE = 'computer/%(name)s/doDelete' NODE_INFO = 'computer/%(name)s/api/json?depth=%(depth)s' NODE_TYPE = 'hudson.slaves.DumbSlave$DescriptorImpl' @@ -286,10 +286,7 @@ class Jenkins(object): def _build_url(self, format_spec, variables=None): if variables: - if format_spec == CREATE_NODE: - url_path = format_spec % urlencode(variables) - else: - url_path = format_spec % self._get_encoded_params(variables) + url_path = format_spec % self._get_encoded_params(variables) else: url_path = format_spec @@ -1281,13 +1278,11 @@ class Jenkins(object): launcher_params['stapler-class'] = launcher inner_params = { - 'name': name, 'nodeDescription': nodeDescription, 'numExecutors': numExecutors, 'remoteFS': remoteFS, 'labelString': labels, 'mode': mode, - 'type': NODE_TYPE, 'retentionStrategy': { 'stapler-class': 'hudson.slaves.RetentionStrategy$Always' @@ -1303,7 +1298,8 @@ class Jenkins(object): } self.jenkins_open(Request( - self._build_url(CREATE_NODE, params), b'')) + self._build_url(CREATE_NODE, locals()), + urlencode(params).encode('utf-8'))) self.assert_node_exists(name, 'create[%s] failed') diff --git a/tests/test_node.py b/tests/test_node.py index a4e7064..4e67b90 100644 --- a/tests/test_node.py +++ b/tests/test_node.py @@ -223,7 +223,8 @@ class JenkinsCreateNodeTest(JenkinsNodesTestBase): 'host': 'my.jenkins.slave1' } self.j.create_node( - 'slave1', + # Note the use of a URL-encodable character "+" here. + '10.0.0.1+test-node', nodeDescription='my test slave', remoteFS='/home/juser', labels='precise', @@ -231,11 +232,13 @@ class JenkinsCreateNodeTest(JenkinsNodesTestBase): launcher=jenkins.LAUNCHER_SSH, launcher_params=params) - actual = jenkins_mock.call_args_list[1][0][0].get_full_url() + actual = jenkins_mock.call_args_list[1][0][0].data.decode('utf-8') # 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'name=10.0.0.1%2Btest-node', actual) + self.assertIn(u'type=hudson.slaves.DumbSlave%24DescriptorImpl', actual) self.assertIn(u'username%22%3A+%22juser', actual) self.assertIn( u'stapler-class%22%3A+%22hudson.plugins.sshslaves.SSHLauncher', @@ -247,20 +250,8 @@ class JenkinsCreateNodeTest(JenkinsNodesTestBase): 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) - def test_build_url(self): - j = jenkins.Jenkins('https://test.noexist/') - # Note the use of a URL-encodable character "+" here. - variables = {'name': '10.0.0.1+test-node'} - result = j._build_url(jenkins.CREATE_NODE, variables=variables) - expected = 'https://test.noexist/computer/doCreateItem?name=10.0.0.1%2Btest-node' - self.assertEqual(result, expected) - @patch.object(jenkins.Jenkins, 'jenkins_open') def test_already_exists(self, jenkins_mock): jenkins_mock.side_effect = [