Make -u required in CLI when required by repo
This patch detects when a repository URL requires username substitution and raises an exception when no username was specified. Change-Id: Ia60982ecddd957cff8709118b3eb8a905258dd06
This commit is contained in:
Lev Morgan
Stacey Fletcher
parent
652c3abefd
commit
b8733ea7ec
|
|
@ -195,7 +195,7 @@ The SSH public key to use when cloning remote authenticated repositories.
|
|||
|
||||
Required for cloning repositories via SSH protocol.
|
||||
|
||||
**-u / --repo-username** (Optional, SSH only).
|
||||
**-u / --repo-username** (Optional, unless required by repo URL).
|
||||
|
||||
The SSH username to use when cloning remote authenticated repositories
|
||||
specified in the site-definition file. Any occurrences of ``REPO_USERNAME``
|
||||
|
|
@ -203,6 +203,8 @@ in an entry under the ``repositories`` field in a given
|
|||
:file:`site-definition.yaml` will be replaced with this value.
|
||||
|
||||
Required for cloning repositories via SSH protocol.
|
||||
This argument will generate an exception if no repo URL
|
||||
uses ``REPO_USERNAME``.
|
||||
|
||||
Examples
|
||||
^^^^^^^^
|
||||
|
|
|
|||
|
|
@ -81,7 +81,8 @@ REPOSITORY_USERNAME_OPTION = click.option(
|
|||
help='The SSH username to use when cloning remote authenticated '
|
||||
'repositories specified in the site-definition file. Any '
|
||||
'occurrences of REPO_USERNAME will be replaced with this '
|
||||
'value.')
|
||||
'value.\n'
|
||||
'Use only if REPO_USERNAME appears in a repo URL.')
|
||||
|
||||
REPOSITORY_CLONE_PATH_OPTION = click.option(
|
||||
'-p',
|
||||
|
|
|
|||
|
|
@ -67,6 +67,11 @@ class GitInvalidRepoException(PeglegBaseException):
|
|||
message = 'The repository path or URL is invalid: %(repo_path)s'
|
||||
|
||||
|
||||
class GitMissingUserException(PeglegBaseException):
|
||||
"""Exception raised when a username is required, but not provided."""
|
||||
message = 'Repo URL %(url)s reuqires a username, but none was provided.'
|
||||
|
||||
|
||||
#
|
||||
# PKI EXCEPTIONS
|
||||
#
|
||||
|
|
|
|||
|
|
@ -327,6 +327,10 @@ def _format_url_with_repo_username(repo_url_or_path):
|
|||
else:
|
||||
repo_url_or_path = repo_url_or_path.replace(
|
||||
'REPO_USERNAME', repo_user)
|
||||
|
||||
elif "REPO_USERNAME" in repo_url_or_path:
|
||||
raise exceptions.GitMissingUserException(url=repo_url_or_path)
|
||||
|
||||
return repo_url_or_path
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -12,10 +12,8 @@
|
|||
# See the License for the specific language governing permissions and
|
||||
# limitations under the License.
|
||||
|
||||
import copy
|
||||
import mock
|
||||
import os
|
||||
import yaml
|
||||
|
||||
import click
|
||||
import pytest
|
||||
|
|
@ -25,6 +23,8 @@ from pegleg.engine import exceptions
|
|||
from pegleg.engine import repository
|
||||
from pegleg.engine import util
|
||||
|
||||
REPO_USERNAME="test_username"
|
||||
|
||||
TEST_REPOSITORIES = {
|
||||
'repositories': {
|
||||
'global': {
|
||||
|
|
@ -40,6 +40,23 @@ TEST_REPOSITORIES = {
|
|||
}
|
||||
}
|
||||
|
||||
FORMATTED_REPOSITORIES = {
|
||||
'repositories': {
|
||||
'global': {
|
||||
'revision': '843d1a50106e1f17f3f722e2ef1634ae442fe68f',
|
||||
'url': 'ssh://{}@gerrit:29418/aic-clcp-manifests.git'.format(
|
||||
REPO_USERNAME)
|
||||
},
|
||||
'secrets': {
|
||||
'revision':
|
||||
'master',
|
||||
'url': ('ssh://{}@gerrit:29418/aic-clcp-security-'
|
||||
'manifests.git'.format(REPO_USERNAME))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
config.set_repo_username(REPO_USERNAME)
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def clean_temp_folders():
|
||||
|
|
@ -129,7 +146,7 @@ def _test_process_repositories(site_repo=None,
|
|||
]
|
||||
mock_calls.extend([
|
||||
mock.call(r['url'], ref=r['revision'], auth_key=None)
|
||||
for r in TEST_REPOSITORIES['repositories'].values()
|
||||
for r in FORMATTED_REPOSITORIES['repositories'].values()
|
||||
])
|
||||
m_clone_repo.assert_has_calls(mock_calls)
|
||||
elif repo_username:
|
||||
|
|
@ -140,7 +157,7 @@ def _test_process_repositories(site_repo=None,
|
|||
r['url'].replace('REPO_USERNAME', repo_username),
|
||||
ref=r['revision'],
|
||||
auth_key=None)
|
||||
for r in TEST_REPOSITORIES['repositories'].values()
|
||||
for r in FORMATTED_REPOSITORIES['repositories'].values()
|
||||
])
|
||||
elif repo_overrides:
|
||||
# This is computed from: len(cloned extra repos) +
|
||||
|
|
@ -148,7 +165,7 @@ def _test_process_repositories(site_repo=None,
|
|||
expected_call_count = len(TEST_REPOSITORIES['repositories']) + 1
|
||||
assert (expected_call_count == m_clone_repo.call_count)
|
||||
|
||||
for x, r in TEST_REPOSITORIES['repositories'].items():
|
||||
for x, r in FORMATTED_REPOSITORIES['repositories'].items():
|
||||
if x in expected_repo_overrides:
|
||||
repo_url = expected_repo_overrides[x]['url']
|
||||
ref = expected_repo_overrides[x]['revision']
|
||||
|
|
@ -161,7 +178,7 @@ def _test_process_repositories(site_repo=None,
|
|||
else:
|
||||
m_clone_repo.assert_has_calls([
|
||||
mock.call(r['url'], ref=r['revision'], auth_key=None)
|
||||
for r in TEST_REPOSITORIES['repositories'].values()
|
||||
for r in FORMATTED_REPOSITORIES['repositories'].values()
|
||||
])
|
||||
|
||||
if site_repo:
|
||||
|
|
@ -195,19 +212,13 @@ def test_process_repositories():
|
|||
|
||||
def test_process_repositories_with_site_repo_url():
|
||||
"""Test process_repository when site repo is a remote URL."""
|
||||
# With REPO_USERNAME.
|
||||
site_repo = (
|
||||
'ssh://REPO_USERNAME@gerrit:29418/aic-clcp-site-manifests.git@333')
|
||||
_test_process_repositories(
|
||||
site_repo=site_repo,
|
||||
expected_repo_url=(
|
||||
"ssh://REPO_USERNAME@gerrit:29418/aic-clcp-site-manifests"),
|
||||
expected_repo_revision="333")
|
||||
# Without REPO_USERNAME.
|
||||
site_repo = 'ssh://gerrit:29418/aic-clcp-site-manifests.git@333'
|
||||
site_repo = 'ssh://{}:29418/aic-clcp-site-manifests.git@333'.format(
|
||||
REPO_USERNAME)
|
||||
_test_process_repositories(
|
||||
site_repo=site_repo,
|
||||
expected_repo_url="ssh://gerrit:29418/aic-clcp-site-manifests",
|
||||
expected_repo_url="ssh://{}:29418/aic-clcp-site-manifests".format(
|
||||
REPO_USERNAME),
|
||||
expected_repo_revision="333")
|
||||
|
||||
|
||||
|
|
@ -257,7 +268,8 @@ def test_process_repositories_with_repo_overrides_remote_urls():
|
|||
}
|
||||
expected_repo_overrides = {
|
||||
'global': {
|
||||
'url': 'ssh://REPO_USERNAME@gerrit:29418/aic-clcp-manifests',
|
||||
'url': 'ssh://{}@gerrit:29418/aic-clcp-manifests'.format(
|
||||
REPO_USERNAME),
|
||||
'revision': '12345'
|
||||
},
|
||||
}
|
||||
|
|
@ -515,3 +527,26 @@ def test_process_site_repository(_):
|
|||
_do_test(
|
||||
'ssh://foo@opendev.org/airship/treasuremap:12345',
|
||||
expected='ssh://foo@opendev.org/airship/treasuremap:12345')
|
||||
|
||||
|
||||
def test_format_url_with_repo_username():
|
||||
TEST_URL = 'ssh://REPO_USERNAME@gerrit:29418/airship/pegleg'
|
||||
|
||||
with mock.patch.object(
|
||||
config,
|
||||
'get_repo_username',
|
||||
autospec=True,
|
||||
return_value=REPO_USERNAME):
|
||||
res = repository._format_url_with_repo_username(TEST_URL)
|
||||
assert res == 'ssh://{}@gerrit:29418/airship/pegleg'.format(
|
||||
REPO_USERNAME)
|
||||
|
||||
with mock.patch.object(
|
||||
config,
|
||||
'get_repo_username',
|
||||
autospec=True,
|
||||
return_value=''):
|
||||
pytest.raises(
|
||||
exceptions.GitMissingUserException,
|
||||
repository._format_url_with_repo_username,
|
||||
TEST_URL)
|
||||
|
|
|
|||
Loading…
Reference in New Issue