diff --git a/doc/source/cli/cli.rst b/doc/source/cli/cli.rst index b70b7d31..5c3ace53 100644 --- a/doc/source/cli/cli.rst +++ b/doc/source/cli/cli.rst @@ -427,13 +427,18 @@ collection does not already exist in the Shipyard buffer. replace: Clear the Shipyard Buffer before adding the specified collection. -auto: Let Pegleg determine the appropriate buffer mode to use. +**--collection** (Required, Default=). + +Specifies the name of the compiled collection of documents that will be +uploaded to Shipyard. Usage: :: - ./pegleg.sh site upload --context-marker= --buffer= + ./pegleg.sh site upload --context-marker= \ + --buffer-mode= \ + --collection= Site Secrets Group ------------------ diff --git a/pegleg/cli.py b/pegleg/cli.py index face17ef..e6e5fbca 100644 --- a/pegleg/cli.py +++ b/pegleg/cli.py @@ -346,6 +346,13 @@ def lint_site(*, fail_on_missing_sub_src, exclude_lint, warn_lint, site_name): warn_lint=warn_lint) +def collection_default_callback(ctx, param, value): + LOG.debug('Evaluating %s: %s', param.name, value) + if not value: + return ctx.params['site_name'] + return value + + @site.command('upload', help='Upload documents to Shipyard') # Keystone authentication parameters @click.option('--os-project-domain-name', @@ -374,20 +381,26 @@ def lint_site(*, fail_on_missing_sub_src, exclude_lint, warn_lint, site_name): '--buffer-mode', 'buffer_mode', required=False, - default='auto', + default='replace', show_default=True, + type=click.Choice(['append', 'replace']), help='Set the buffer mode when uploading documents. Supported buffer ' 'modes include append, replace, auto.\n' 'append: Add the collection to the Shipyard Buffer, only if that ' 'collection does not already exist in the Shipyard buffer.\n' 'replace: Clear the Shipyard Buffer before adding the specified ' - 'collection.\n' - 'auto: Let Pegleg determine the appropriate buffer mode to use.') + 'collection.\n') +@click.option( + '--collection', + 'collection', + help='Specifies the name to use for the uploaded collection. ' + 'Defaults to the specified `site_name`.', + callback=collection_default_callback) @SITE_REPOSITORY_ARGUMENT @click.pass_context -def upload(ctx, *, os_project_domain_name, - os_user_domain_name, os_project_name, os_username, - os_password, os_auth_url, context_marker, site_name, buffer_mode): +def upload(ctx, *, os_project_domain_name, os_user_domain_name, + os_project_name, os_username, os_password, os_auth_url, + context_marker, site_name, buffer_mode, collection): if not ctx.obj: ctx.obj = {} @@ -406,6 +419,7 @@ def upload(ctx, *, os_project_domain_name, } ctx.obj['context_marker'] = str(context_marker) ctx.obj['site_name'] = site_name + ctx.obj['collection'] = collection click.echo(ShipyardHelper(ctx, buffer_mode).upload_documents()) diff --git a/pegleg/engine/util/shipyard_helper.py b/pegleg/engine/util/shipyard_helper.py index f7537956..e27109e4 100644 --- a/pegleg/engine/util/shipyard_helper.py +++ b/pegleg/engine/util/shipyard_helper.py @@ -52,7 +52,7 @@ class ShipyardHelper(object): 4. Formats response from Shipyard api_client """ - def __init__(self, context, buffer_mode='auto'): + def __init__(self, context, buffer_mode='replace'): """ Initializes params to be used by Shipyard @@ -72,80 +72,69 @@ class ShipyardHelper(object): self.auth_vars, self.context_marker) self.api_client = ShipyardClient(self.client_context) self.buffer_mode = buffer_mode + self.collection = self.ctx.obj.get('collection', self.site_name) def upload_documents(self): - """Uploads documents to Shipyard """ + """Uploads documents to Shipyard""" collected_documents = files.collect_files_by_repo(self.site_name) - LOG.info("Uploading %d collection(s) ", len(collected_documents)) + collection_data = [] + LOG.info("Processing %d collection(s)", len(collected_documents)) for idx, document in enumerate(collected_documents): - # Append flag is not required for the first - # collection being uploaded to Shipyard. It - # is needed for subsequent collections. - if self.buffer_mode == 'auto': - if idx == 0: - buffer_mode = None - else: - buffer_mode = 'append' - elif self.buffer_mode == 'append' or self.buffer_mode == 'replace': - buffer_mode = self.buffer_mode - else: - raise exceptions.InvalidBufferModeException() - # Decrypt the documents if encrypted pegleg_secret_mgmt = PeglegSecretManagement( docs=collected_documents[document]) decrypted_documents = pegleg_secret_mgmt.get_decrypted_secrets() - data = yaml.safe_dump_all(decrypted_documents) + collection_data.extend(decrypted_documents) + collection_as_yaml = yaml.dump_all(collection_data, + Dumper=yaml.SafeDumper) - try: - self.validate_auth_vars() - # Get current buffer status. - response = self.api_client.get_configdocs_status() - buff_stat = response.json() - # If buffer is empty then proceed with existing buffer value - # else pass the 'replace' flag. - for stat in range(len(buff_stat)): - if (buff_stat[stat]['new_status'] != 'unmodified' and - buffer_mode != 'append'): - buffer_mode = 'replace' - resp_text = self.api_client.post_configdocs( - collection_id=document, - buffer_mode=buffer_mode, - document_data=data - ) + # Append flag is not required for the first + # collection being uploaded to Shipyard. It + # is needed for subsequent collections. + if self.buffer_mode in ['append', 'replace']: + buffer_mode = self.buffer_mode + else: + raise exceptions.InvalidBufferModeException() - except AuthValuesError as ave: - resp_text = "Error: {}".format(ave.diagnostic) - raise DocumentUploadError(resp_text) - except Exception as ex: - resp_text = ( - "Error: Unable to invoke action due to: {}" - .format(str(ex))) - LOG.debug(resp_text, exc_info=True) - raise DocumentUploadError(resp_text) + try: + self.validate_auth_vars() + resp_text = self.api_client.post_configdocs( + collection_id=self.collection, + buffer_mode=buffer_mode, + document_data=collection_as_yaml + ) - # FIXME: Standardize status_code in Deckhand to avoid this - # workaround. - code = 0 - if hasattr(resp_text, 'status_code'): - code = resp_text.status_code - elif hasattr(resp_text, 'code'): - code = resp_text.code - if code >= 400: - if hasattr(resp_text, 'content'): - raise DocumentUploadError(resp_text.content) - else: - raise DocumentUploadError(resp_text) + except AuthValuesError as ave: + resp_text = "Error: {}".format(ave.diagnostic) + raise DocumentUploadError(resp_text) + except Exception as ex: + resp_text = ( + "Error: Unable to invoke action due to: {}" + .format(str(ex))) + LOG.debug(resp_text, exc_info=True) + raise DocumentUploadError(resp_text) + + # FIXME: Standardize status_code in Deckhand to avoid this + # workaround. + code = 0 + if hasattr(resp_text, 'status_code'): + code = resp_text.status_code + elif hasattr(resp_text, 'code'): + code = resp_text.code + if code >= 400: + if hasattr(resp_text, 'content'): + raise DocumentUploadError(resp_text.content) else: - output = self.formatted_response_handler(resp_text) - LOG.info("Uploaded document in buffer %s ", output) + raise DocumentUploadError(resp_text) + else: + output = self.formatted_response_handler(resp_text) + LOG.info("Uploaded document in buffer %s ", output) # Commit in the last iteration of the loop when all the documents # have been pushed to Shipyard buffer. - if idx == len(collected_documents) - 1: - return self.commit_documents() + return self.commit_documents() def commit_documents(self): """Commit Shipyard buffer documents """ diff --git a/tests/unit/engine/util/test_shipyard_helper.py b/tests/unit/engine/util/test_shipyard_helper.py index 72dc75bd..28f79507 100644 --- a/tests/unit/engine/util/test_shipyard_helper.py +++ b/tests/unit/engine/util/test_shipyard_helper.py @@ -14,12 +14,9 @@ import os -import json import mock import pytest - -from tests.unit import test_utils -from mock import ANY +import yaml from pegleg.engine import util from pegleg.engine.util.pegleg_secret_management import ENV_PASSPHRASE @@ -28,14 +25,35 @@ from pegleg.engine.util.shipyard_helper import ShipyardHelper from pegleg.engine.util.shipyard_helper import ShipyardClient # Dummy data to be used as collected documents -DATA = {'test-repo': +DATA = { + 'test-repo': [{'schema': 'pegleg/SiteDefinition/v1', - 'metadata': {'schema': 'metadata/Document/v1', - 'layeringDefinition': {'abstract': False, - 'layer': 'site'}, - 'name': 'site-name', - 'storagePolicy': 'cleartext'}, - 'data': {'site_type': 'foundry'}}]} + 'metadata': {'schema': 'metadata/Document/v1', + 'layeringDefinition': {'abstract': False, + 'layer': 'site'}, + 'name': 'site-name', + 'storagePolicy': 'cleartext'}, + 'data': {'site_type': 'foundry'}}]} + +MULTI_REPO_DATA = { + 'repo1': + [{'schema': 'pegleg/SiteDefinition/v1', + 'metadata': {'schema': 'metadata/Document/v1', + 'layeringDefinition': {'abstract': False, + 'layer': 'site'}, + 'name': 'site-name', + 'storagePolicy': 'cleartext'}, + 'data': {'site_type': 'foundry'}}], + 'repo2': + [{'schema': 'pegleg/SiteDefinition/v1', + 'metadata': {'schema': 'metadata/Document/v1', + 'layeringDefinition': {'abstract': False, + 'layer': 'site'}, + 'name': 'site-name', + 'storagePolicy': 'cleartext'}, + 'data': {'site_type': 'foundry'}}] + +} @pytest.fixture(autouse=True) @@ -51,6 +69,7 @@ class context(): class FakeResponse(): code = 404 + def _get_context(): ctx = context() ctx.obj = {} @@ -67,8 +86,10 @@ def _get_context(): } ctx.obj['context_marker'] = '88888888-4444-4444-4444-121212121212' ctx.obj['site_name'] = 'test-site' + ctx.obj['collection'] = 'test-site' return ctx + def _get_bad_context(): ctx = context() ctx.obj = {} @@ -85,9 +106,18 @@ def _get_bad_context(): } ctx.obj['context_marker'] = '88888888-4444-4444-4444-121212121212' ctx.obj['site_name'] = 'test-site' + ctx.obj['collection'] = None return ctx +def _get_data_as_collection(data): + collection = [] + for repo, documents in data.items(): + for document in documents: + collection.append(document) + return yaml.dump_all(collection, Dumper=yaml.SafeDumper) + + def test_shipyard_helper_init_(): """ Tests ShipyardHelper init method """ # Scenario: @@ -102,8 +132,9 @@ def test_shipyard_helper_init_(): assert shipyard_helper.site_name == context.obj['site_name'] assert isinstance(shipyard_helper.api_client, ShipyardClient) + @mock.patch('pegleg.engine.util.files.collect_files_by_repo', autospec=True, - return_value=DATA) + return_value=MULTI_REPO_DATA) @mock.patch.object(ShipyardHelper, 'formatted_response_handler', autospec=True, return_value=None) @mock.patch.dict(os.environ, { @@ -119,19 +150,22 @@ def test_upload_documents(*args): # 3) Check documents uploaded to Shipyard with correct parameters context = _get_context() - shipyard_helper = ShipyardHelper(context) with mock.patch('pegleg.engine.util.shipyard_helper.ShipyardClient', autospec=True) as mock_shipyard: mock_api_client = mock_shipyard.return_value mock_api_client.post_configdocs.return_value = 'Success' - result = ShipyardHelper(context).upload_documents() + result = ShipyardHelper(context, 'replace').upload_documents() # Validate Shipyard call to post configdocs was invoked with correct # collection name and buffer mode. - mock_api_client.post_configdocs.assert_called_with('test-repo', - None, ANY) + expected_data = _get_data_as_collection(MULTI_REPO_DATA) + mock_api_client.post_configdocs.assert_called_with( + collection_id='test-site', + buffer_mode='replace', + document_data=expected_data) mock_api_client.post_configdocs.assert_called_once() + @mock.patch('pegleg.engine.util.files.collect_files_by_repo', autospec=True, return_value=DATA) @mock.patch.object(ShipyardHelper, 'formatted_response_handler', @@ -158,6 +192,7 @@ def test_upload_documents_fail(*args): with pytest.raises(util.shipyard_helper.DocumentUploadError): ShipyardHelper(context).upload_documents() + @mock.patch('pegleg.engine.util.files.collect_files_by_repo', autospec=True, return_value=DATA) @mock.patch.object(ShipyardHelper, 'formatted_response_handler', @@ -175,6 +210,7 @@ def test_fail_auth(*args): with pytest.raises(util.shipyard_helper.AuthValuesError): ShipyardHelper(context).validate_auth_vars() + @mock.patch.object(ShipyardHelper, 'formatted_response_handler', autospec=True, return_value=None) def test_commit_documents(*args): diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index 0d3bffdb..f2401209 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -13,10 +13,8 @@ # limitations under the License. import os -import shutil from click.testing import CliRunner -from mock import ANY import mock import pytest import yaml @@ -375,6 +373,8 @@ class TestSiteCliActions(BaseCLIActionTest): repo_path = self.treasuremap_path self._validate_render_site_action(repo_path) + ### Upload tests ### + def test_upload_documents_shipyard_using_local_repo_path(self): """Validates ShipyardHelper is called with correct arguments.""" # Scenario: @@ -387,11 +387,28 @@ class TestSiteCliActions(BaseCLIActionTest): with mock.patch('pegleg.cli.ShipyardHelper') as mock_obj: result = self.runner.invoke(cli.site, ['-r', repo_path, 'upload', - self.site_name]) + self.site_name, '--collection', + 'collection']) assert result.exit_code == 0 mock_obj.assert_called_once() + def test_upload_collection_callback_default_to_site_name(self): + """Validates that collection will default to the given site_name""" + # Scenario: + # + # 1) Mock out ShipyardHelper + # 2) Check that ShipyardHelper was called with collection set to + # site_name + repo_path = self.treasuremap_path + + with mock.patch('pegleg.cli.ShipyardHelper') as mock_obj: + result = self.runner.invoke(cli.site, + ['-r', repo_path, 'upload', + self.site_name]) + assert result.exit_code == 0 + mock_obj.assert_called_once() + class TestGenerateActions(BaseCLIActionTest): def test_generate_passphrase(self):