From c1d90e189715fdc233ebb5db2def54bcdba6e616 Mon Sep 17 00:00:00 2001 From: Chris Church Date: Thu, 26 Feb 2015 15:46:55 -0500 Subject: [PATCH] Keep git URLs in SCP format for project updates. Fixes https://trello.com/c/xUL2FZyu --- awx/main/tasks.py | 8 ++++-- awx/main/tests/projects.py | 59 ++++++++++++++++++++++++++------------ awx/main/utils.py | 42 ++++++++++++++++----------- 3 files changed, 72 insertions(+), 37 deletions(-) diff --git a/awx/main/tasks.py b/awx/main/tasks.py index 5e3393f417..0c78151973 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -791,14 +791,16 @@ class RunProjectUpdate(BaseTask): scm_password = False if scm_url_parts.scheme != 'svn+ssh': scm_username = False - elif scm_url_parts.scheme == 'ssh': + elif scm_url_parts.scheme.endswith('ssh'): scm_password = False scm_url = update_scm_url(scm_type, scm_url, scm_username, - scm_password) + scm_password, scp_format=True) + else: + scm_url = update_scm_url(scm_type, scm_url, scp_format=True) # When using Ansible >= 1.5, pass the extra accept_hostkey parameter to # the git module. - if scm_type == 'git' and scm_url_parts.scheme == 'ssh': + if scm_type == 'git' and scm_url_parts.scheme.endswith('ssh'): try: if Version(kwargs['ansible_version']) >= Version('1.5'): extra_vars['scm_accept_hostkey'] = 'true' diff --git a/awx/main/tests/projects.py b/awx/main/tests/projects.py index 20dae18704..1c97c28fe3 100644 --- a/awx/main/tests/projects.py +++ b/awx/main/tests/projects.py @@ -816,9 +816,15 @@ class ProjectUpdatesTest(BaseTransactionTest): # - rsync://host.xz/path/to/repo.git/ ('git', 'rsync://host.xz/path/to/repo.git/', ValueError, ValueError, ValueError), # - [user@]host.xz:path/to/repo.git/ (SCP style) - ('git', 'host.xz:path/to/repo.git/', 'ssh://host.xz/path/to/repo.git/', 'ssh://testuser@host.xz/path/to/repo.git/', 'ssh://testuser:testpass@host.xz/path/to/repo.git/'), - ('git', 'user@host.xz:path/to/repo.git/', 'ssh://user@host.xz/path/to/repo.git/', 'ssh://testuser@host.xz/path/to/repo.git/', 'ssh://testuser:testpass@host.xz/path/to/repo.git/'), - ('git', 'user:pass@host.xz:path/to/repo.git/', 'ssh://user:pass@host.xz/path/to/repo.git/', 'ssh://testuser:pass@host.xz/path/to/repo.git/', 'ssh://testuser:testpass@host.xz/path/to/repo.git/'), + ('git', 'host.xz:path/to/repo.git/', 'git+ssh://host.xz/path/to/repo.git/', 'git+ssh://testuser@host.xz/path/to/repo.git/', 'git+ssh://testuser:testpass@host.xz/path/to/repo.git/'), + ('git', 'user@host.xz:path/to/repo.git/', 'git+ssh://user@host.xz/path/to/repo.git/', 'git+ssh://testuser@host.xz/path/to/repo.git/', 'git+ssh://testuser:testpass@host.xz/path/to/repo.git/'), + ('git', 'user:pass@host.xz:path/to/repo.git/', 'git+ssh://user:pass@host.xz/path/to/repo.git/', 'git+ssh://testuser:pass@host.xz/path/to/repo.git/', 'git+ssh://testuser:testpass@host.xz/path/to/repo.git/'), + ('git', 'host.xz:~/path/to/repo.git/', 'git+ssh://host.xz/~/path/to/repo.git/', 'git+ssh://testuser@host.xz/~/path/to/repo.git/', 'git+ssh://testuser:testpass@host.xz/~/path/to/repo.git/'), + ('git', 'user@host.xz:~/path/to/repo.git/', 'git+ssh://user@host.xz/~/path/to/repo.git/', 'git+ssh://testuser@host.xz/~/path/to/repo.git/', 'git+ssh://testuser:testpass@host.xz/~/path/to/repo.git/'), + ('git', 'user:pass@host.xz:~/path/to/repo.git/', 'git+ssh://user:pass@host.xz/~/path/to/repo.git/', 'git+ssh://testuser:pass@host.xz/~/path/to/repo.git/', 'git+ssh://testuser:testpass@host.xz/~/path/to/repo.git/'), + ('git', 'host.xz:/path/to/repo.git/', 'git+ssh://host.xz//path/to/repo.git/', 'git+ssh://testuser@host.xz//path/to/repo.git/', 'git+ssh://testuser:testpass@host.xz//path/to/repo.git/'), + ('git', 'user@host.xz:/path/to/repo.git/', 'git+ssh://user@host.xz//path/to/repo.git/', 'git+ssh://testuser@host.xz//path/to/repo.git/', 'git+ssh://testuser:testpass@host.xz//path/to/repo.git/'), + ('git', 'user:pass@host.xz:/path/to/repo.git/', 'git+ssh://user:pass@host.xz//path/to/repo.git/', 'git+ssh://testuser:pass@host.xz//path/to/repo.git/', 'git+ssh://testuser:testpass@host.xz//path/to/repo.git/'), # - /path/to/repo.git/ (local file) ('git', '/path/to/repo.git', ValueError, ValueError, ValueError), ('git', 'path/to/repo.git', ValueError, ValueError, ValueError), @@ -829,7 +835,7 @@ class ProjectUpdatesTest(BaseTransactionTest): ('git', 'ssh:github.com:ansible/ansible-examples.git', ValueError, ValueError, ValueError), ('git', 'ssh://github.com:ansible/ansible-examples.git', ValueError, ValueError, ValueError), # Special case for github URLs: - ('git', 'git@github.com:ansible/ansible-examples.git', 'ssh://git@github.com/ansible/ansible-examples.git', ValueError, ValueError), + ('git', 'git@github.com:ansible/ansible-examples.git', 'git+ssh://git@github.com/ansible/ansible-examples.git', ValueError, ValueError), ('git', 'bob@github.com:ansible/ansible-examples.git', ValueError, ValueError, ValueError), # Special case for bitbucket URLs: ('git', 'ssh://git@bitbucket.org/foo/bar.git', None, ValueError, ValueError), @@ -926,39 +932,56 @@ class ProjectUpdatesTest(BaseTransactionTest): ('svn', 'svn+ssh://user@host.xz:1022/path/to/repo', None, 'svn+ssh://testuser@host.xz:1022/path/to/repo', 'svn+ssh://testuser:testpass@host.xz:1022/path/to/repo'), ('svn', 'svn+ssh://user:pass@host.xz/path/to/repo/', None, 'svn+ssh://testuser:pass@host.xz/path/to/repo/', 'svn+ssh://testuser:testpass@host.xz/path/to/repo/'), ('svn', 'svn+ssh://user:pass@host.xz:1022/path/to/repo', None, 'svn+ssh://testuser:pass@host.xz:1022/path/to/repo', 'svn+ssh://testuser:testpass@host.xz:1022/path/to/repo'), - - # FIXME: Add some invalid URLs. ] + # Some invalid URLs. + for scm_type in ('git', 'svn', 'hg'): + urls_to_test.append((scm_type, 'host', ValueError, ValueError, ValueError)) + urls_to_test.append((scm_type, '/path', ValueError, ValueError, ValueError)) + urls_to_test.append((scm_type, 'mailto:joe@example.com', ValueError, ValueError, ValueError)) + urls_to_test.append((scm_type, 'telnet://host.xz/path/to/repo', ValueError, ValueError, ValueError)) + def is_exception(e): - return bool(isinstance(e, Exception) or - (isinstance(e, type) and issubclass(e, Exception))) + return bool(isinstance(e, Exception) or (isinstance(e, type) and issubclass(e, Exception))) + for url_opts in urls_to_test: scm_type, url, new_url, new_url_u, new_url_up = url_opts - #print scm_type, url new_url = new_url or url new_url_u = new_url_u or url new_url_up = new_url_up or url + + # Check existing URL as-is. if is_exception(new_url): self.assertRaises(new_url, update_scm_url, scm_type, url) else: updated_url = update_scm_url(scm_type, url) self.assertEqual(new_url, updated_url) + if updated_url.startswith('git+ssh://'): + new_url2 = new_url.replace('git+ssh://', '', 1).replace('/', ':', 1) + updated_url2 = update_scm_url(scm_type, url, scp_format=True) + self.assertEqual(new_url2, updated_url2) + + # Check URL with username replaced. if is_exception(new_url_u): - self.assertRaises(new_url_u, update_scm_url, scm_type, - url, username='testuser') + self.assertRaises(new_url_u, update_scm_url, scm_type, url, username='testuser') else: - updated_url = update_scm_url(scm_type, url, - username='testuser') + updated_url = update_scm_url(scm_type, url, username='testuser') self.assertEqual(new_url_u, updated_url) + if updated_url.startswith('git+ssh://'): + new_url2 = new_url_u.replace('git+ssh://', '', 1).replace('/', ':', 1) + updated_url2 = update_scm_url(scm_type, url, username='testuser', scp_format=True) + self.assertEqual(new_url2, updated_url2) + + # Check URL with username and password replaced. if is_exception(new_url_up): - self.assertRaises(new_url_up, update_scm_url, scm_type, - url, username='testuser', password='testpass') + self.assertRaises(new_url_up, update_scm_url, scm_type, url, username='testuser', password='testpass') else: - updated_url = update_scm_url(scm_type, url, - username='testuser', - password='testpass') + updated_url = update_scm_url(scm_type, url, username='testuser', password='testpass') self.assertEqual(new_url_up, updated_url) + if updated_url.startswith('git+ssh://'): + new_url2 = new_url_up.replace('git+ssh://', '', 1).replace('/', ':', 1) + updated_url2 = update_scm_url(scm_type, url, username='testuser', password='testpass', scp_format=True) + self.assertEqual(new_url2, updated_url2) def is_public_key_in_authorized_keys(self): auth_keys = set() diff --git a/awx/main/utils.py b/awx/main/utils.py index 7fc42acad5..16c5bf2557 100644 --- a/awx/main/utils.py +++ b/awx/main/utils.py @@ -163,7 +163,7 @@ def decrypt_field(instance, field_name): def update_scm_url(scm_type, url, username=True, password=True, - check_special_cases=True): + check_special_cases=True, scp_format=False): ''' Update the given SCM URL to add/replace/remove the username/password. When username/password is True, preserve existing username/password, when @@ -183,20 +183,28 @@ def update_scm_url(scm_type, url, username=True, password=True, parts.port except ValueError: raise ValueError('Invalid %s URL' % scm_type) - #print parts + if parts.scheme == 'git+ssh' and not scp_format: + raise ValueError('Unsupported %s URL' % scm_type) + if '://' not in url: # Handle SCP-style URLs for git (e.g. [user@]host.xz:path/to/repo.git/). - if scm_type == 'git' and '@' in url: - userpass, hostpath = url.split('@', 1) - hostpath = '/'.join(hostpath.split(':', 1)) - modified_url = '@'.join([userpass, hostpath]) - parts = urlparse.urlsplit('ssh://%s' % modified_url) - elif scm_type == 'git' and ':' in url: - if url.count(':') > 1: + if scm_type == 'git' and ':' in url: + if '@' in url: + userpass, hostpath = url.split('@', 1) + else: + userpass, hostpath = '', url + if hostpath.count(':') > 1: raise ValueError('Invalid %s URL' % scm_type) - - modified_url = '/'.join(url.split(':', 1)) - parts = urlparse.urlsplit('ssh://%s' % modified_url) + host, path = hostpath.split(':', 1) + #if not path.startswith('/') and not path.startswith('~/'): + # path = '~/%s' % path + #if path.startswith('/'): + # path = path.lstrip('/') + hostpath = '/'.join([host, path]) + modified_url = '@'.join(filter(None, [userpass, hostpath])) + # git+ssh scheme identifies URLs that should be converted back to + # SCP style before passed to git module. + parts = urlparse.urlsplit('git+ssh://%s' % modified_url) # Handle local paths specified without file scheme (e.g. /path/to/foo). # Only supported by git and hg. (not currently allowed) elif scm_type in ('git', 'hg'): @@ -206,10 +214,10 @@ def update_scm_url(scm_type, url, username=True, password=True, parts = urlparse.urlsplit('file://%s' % url) else: raise ValueError('Invalid %s URL' % scm_type) - #print parts + # Validate that scheme is valid for given scm_type. scm_type_schemes = { - 'git': ('ssh', 'git', 'http', 'https', 'ftp', 'ftps'), + 'git': ('ssh', 'git', 'git+ssh', 'http', 'https', 'ftp', 'ftps'), 'hg': ('http', 'https', 'ssh'), 'svn': ('http', 'https', 'svn', 'svn+ssh'), } @@ -235,9 +243,9 @@ def update_scm_url(scm_type, url, username=True, password=True, # Special handling for github/bitbucket SSH URLs. if check_special_cases: special_git_hosts = ('github.com', 'bitbucket.org', 'altssh.bitbucket.org') - if scm_type == 'git' and parts.scheme == 'ssh' and parts.hostname in special_git_hosts and netloc_username != 'git': + if scm_type == 'git' and parts.scheme.endswith('ssh') and parts.hostname in special_git_hosts and netloc_username != 'git': raise ValueError('Username must be "git" for SSH access to %s.' % parts.hostname) - if scm_type == 'git' and parts.scheme == 'ssh' and parts.hostname in special_git_hosts and netloc_password: + if scm_type == 'git' and parts.scheme.endswith('ssh') and parts.hostname in special_git_hosts and netloc_password: #raise ValueError('Password not allowed for SSH access to %s.' % parts.hostname) netloc_password = '' special_hg_hosts = ('bitbucket.org', 'altssh.bitbucket.org') @@ -256,6 +264,8 @@ def update_scm_url(scm_type, url, username=True, password=True, netloc = u':'.join([netloc, unicode(parts.port)]) new_url = urlparse.urlunsplit([parts.scheme, netloc, parts.path, parts.query, parts.fragment]) + if scp_format and parts.scheme == 'git+ssh': + new_url = new_url.replace('git+ssh://', '', 1).replace('/', ':', 1) return new_url