From fde70e9218a23a3a6206a3008eaae30917980531 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Wed, 26 Sep 2018 02:31:41 +0100 Subject: [PATCH] fix: Allow -r flag to work with remote repository URLs This patch set fixes an issue where currently -r flag won't work with remote repository URLs. site_repository.process_repositories will blow up with an error (for example): Error: https://github.com/openstack/airship-treasuremap/site/ airship-seaworthy/site-definition.yaml not found. Pegleg must be run from the root of a configuration repository. It is apparent that the URL should not be treated as a local path which is what is happening behind the scenes with this bug. Also, CLI unit tests are added to validate the intended behavior. They will be expanded on in future patch sets. Change-Id: I618465841f1e455c8f00f046b3c5d22348b99396 --- pegleg/engine/repository.py | 18 ++-- tests/unit/engine/test_site_repository.py | 15 +-- tests/unit/test_cli.py | 106 ++++++++++++++++++++++ 3 files changed, 126 insertions(+), 13 deletions(-) create mode 100644 tests/unit/test_cli.py diff --git a/pegleg/engine/repository.py b/pegleg/engine/repository.py index 43697a8a..25af8ac5 100644 --- a/pegleg/engine/repository.py +++ b/pegleg/engine/repository.py @@ -131,16 +131,21 @@ def process_site_repository(update_config=False): repo_path_or_url, repo_revision = _extract_repo_url_and_revision( site_repo_or_path) - temp_site_repo = _copy_to_temp_folder(repo_path_or_url, "site") - _process_site_repository(temp_site_repo, repo_revision) + + if os.path.exists(repo_path_or_url): + temp_site_repo = _copy_to_temp_folder(repo_path_or_url, "site") + else: + temp_site_repo = repo_path_or_url + + new_repo_path = _process_site_repository(temp_site_repo, repo_revision) if update_config: # Overwrite the site repo in the config because further processing will # fail if they contain revision info in their paths. - LOG.debug("Updating site_repo=%s in config", temp_site_repo) - config.set_site_repo(temp_site_repo) + LOG.debug("Updating site_repo=%s in config", new_repo_path) + config.set_site_repo(new_repo_path) - return temp_site_repo + return new_repo_path def _process_site_repository(repo_url_or_path, repo_revision): @@ -167,7 +172,8 @@ def _process_site_repository(repo_url_or_path, repo_revision): LOG.info("Processing repository %s with url=%s, repo_key=%s, " "repo_username=%s, revision=%s", repo_alias, repo_url_or_path, repo_key, repo_user, repo_revision) - _handle_repository(repo_url_or_path, ref=repo_revision, auth_key=repo_key) + return _handle_repository( + repo_url_or_path, ref=repo_revision, auth_key=repo_key) def _get_and_validate_site_repositories(site_name, site_data): diff --git a/tests/unit/engine/test_site_repository.py b/tests/unit/engine/test_site_repository.py index 5155cad0..f20c6007 100644 --- a/tests/unit/engine/test_site_repository.py +++ b/tests/unit/engine/test_site_repository.py @@ -410,14 +410,15 @@ def test_process_site_repository(): def _do_test(site_repo): expected = site_repo.rsplit('@', 1)[0] - with mock.patch.object( - config, 'get_site_repo', autospec=True, - return_value=site_repo) as mock_get_site_repo: + config.set_site_repo(site_repo) - with mock.patch.object( - repository, '_handle_repository', autospec=True): - result = repository.process_site_repository() - assert expected == result + with mock.patch.object( + repository, + '_handle_repository', + autospec=True, + return_value=expected): + result = repository.process_site_repository() + assert expected == result # Ensure that the reference is always pruned. _do_test('http://github.com/openstack/treasuremap@master') diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py new file mode 100644 index 00000000..3499983d --- /dev/null +++ b/tests/unit/test_cli.py @@ -0,0 +1,106 @@ +# Copyright 2018 AT&T Intellectual Property. All other rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os +import requests +import shutil +import tempfile + +from click.testing import CliRunner +import pytest + +from pegleg import cli +from pegleg.engine.util import git + + +def is_connected(): + """Verifies whether network connectivity is up. + + :returns: True if connected else False. + """ + try: + r = requests.get("http://www.github.com/", proxies={}) + return r.ok + except requests.exceptions.RequestException: + return False + + +@pytest.mark.skipif( + not is_connected(), reason='git clone requires network connectivity.') +class TestSiteCliActions(object): + @classmethod + def setup_class(cls): + cls.runner = CliRunner() + + # Pin so we know that airship-seaworthy is a valid site. + cls.site_name = "airship-seaworthy" + cls.repo_rev = '6b183e148b9bb7ba6f75c98dd13451088255c60b' + cls.repo_name = "airship-treasuremap" + repo_url = "https://github.com/openstack/%s.git" % cls.repo_name + cls.treasuremap_path = git.git_handler(repo_url, ref=cls.repo_rev) + + @classmethod + def teardown_class(cls): + # Cleanup temporary Git repos. + root_tempdir = tempfile.gettempdir() + for tempdir in os.listdir(root_tempdir): + path = os.path.join(root_tempdir, tempdir) + if git.is_repository(path): + shutil.rmtree(path, ignore_errors=True) + + def test_collect_using_remote_repo_url(self): + """Validates collect action using a remote URL.""" + # Scenario: + # + # 1) Create temporary save location + # 2) Collect into save location (should clone repo automatically) + # 3) Check that expected file name is there + + save_location = tempfile.mkdtemp() + repo_url = 'https://github.com/openstack/%s@%s' % (self.repo_name, + self.repo_rev) + result = self.runner.invoke( + cli.site, + ['-r', repo_url, 'collect', self.site_name, '-s', save_location]) + + collected_files = os.listdir(save_location) + + assert result.exit_code == 0 + assert len(collected_files) == 1 + # Validates that site manifests collected from cloned repositories + # are written out to sensibly named files like airship-treasuremap.yaml + assert collected_files[0].endswith("%s.yaml" % self.repo_name) + + def test_collect_using_local_path(self): + """Validates collect action using a path to a local repo.""" + # Scenario: + # + # 1) Create temporary save location + # 2) Collect into save location (should skip clone repo) + # 3) Check that expected file name is there + + save_location = tempfile.mkdtemp() + repo_path = self.treasuremap_path + + result = self.runner.invoke( + cli.site, + ['-r', repo_path, 'collect', self.site_name, '-s', save_location]) + + collected_files = os.listdir(save_location) + + assert result.exit_code == 0 + assert len(collected_files) == 1 + # Validates that site manifests collected from cloned repositories + # are written out to sensibly named files like airship-treasuremap.yaml + assert collected_files[0].endswith("%s.yaml" % self.repo_name)