From d6ee04f9a3e3af07348b143534fe9943e1b0825d Mon Sep 17 00:00:00 2001 From: "Ian H. Pittwood" Date: Wed, 15 May 2019 16:32:35 -0500 Subject: [PATCH] Specify collection for upload command Currently, using the upload command in Pegleg will upload all discovered collections to Shipyard by repo. Uploading multiple of these repos can result in 409 errors during uplift scenarios. This change compiles all documents into a single collection document that can then be uploaded to Shipyard. Requires a collection name to be specified that will be used as the 'collection_id' for uploading to Shipyard. Buffer mode is set by default to 'replace' instead of 'auto'. Change-Id: I546b03fd82873296fff10aba355a50e4b11352d0 --- doc/source/cli/cli.rst | 9 +- pegleg/cli.py | 26 ++++- pegleg/engine/util/shipyard_helper.py | 105 ++++++++---------- .../unit/engine/util/test_shipyard_helper.py | 68 +++++++++--- tests/unit/test_cli.py | 23 +++- 5 files changed, 146 insertions(+), 85 deletions(-) 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):