From 02d4af2b1667c7d7cd38dfea2b54a228ae86273e Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Wed, 17 Jan 2018 10:32:39 -0500 Subject: [PATCH] Allow same tag to be created for multiple revisions This PS fixes a unique constraint for revision tags, allowing a tag to be associated with only one revision. It also adds an additional try/catch block when attempting to update an existing revision (although it is mostly superfluous). Closes 14 Change-Id: I83cfacb25f71a33641054c5a26e5b9a1c9907658 --- deckhand/control/revision_tags.py | 2 +- deckhand/db/sqlalchemy/api.py | 11 ++++++--- deckhand/db/sqlalchemy/models.py | 6 ++--- .../control/test_revision_tags_controller.py | 23 +++++++++++++++++-- 4 files changed, 32 insertions(+), 10 deletions(-) diff --git a/deckhand/control/revision_tags.py b/deckhand/control/revision_tags.py index 39925323..ea6e54ff 100644 --- a/deckhand/control/revision_tags.py +++ b/deckhand/control/revision_tags.py @@ -44,7 +44,7 @@ class RevisionTagsResource(api_base.BaseResource): try: resp_tag = db_api.revision_tag_create(revision_id, tag, tag_data) - except errors.RevisionNotFound as e: + except (errors.RevisionNotFound, errors.RevisionTagNotFound) as e: raise falcon.HTTPNotFound(description=e.format_message()) except errors.RevisionTagBadFormat as e: raise falcon.HTTPBadRequest(description=e.format_message()) diff --git a/deckhand/db/sqlalchemy/api.py b/deckhand/db/sqlalchemy/api.py index 55b92ad3..0d51d464 100644 --- a/deckhand/db/sqlalchemy/api.py +++ b/deckhand/db/sqlalchemy/api.py @@ -840,9 +840,14 @@ def revision_tag_create(revision_id, tag, data=None, session=None): resp = tag_model.to_dict() except db_exception.DBDuplicateEntry: # Update the revision tag if it already exists. - tag_to_update = session.query(models.RevisionTag)\ - .filter_by(tag=tag, revision_id=revision_id)\ - .one() + LOG.debug('Tag %s already exists for revision_id %s. Attempting to ' + 'update the entry.', tag, revision_id) + try: + tag_to_update = session.query(models.RevisionTag)\ + .filter_by(tag=tag, revision_id=revision_id)\ + .one() + except sa_orm.exc.NoResultFound: + raise errors.RevisionTagNotFound(tag=tag, revision=revision_id) tag_to_update.update({'data': data}) tag_to_update.save(session=session) resp = tag_to_update.to_dict() diff --git a/deckhand/db/sqlalchemy/models.py b/deckhand/db/sqlalchemy/models.py index 00d7c172..574953cb 100644 --- a/deckhand/db/sqlalchemy/models.py +++ b/deckhand/db/sqlalchemy/models.py @@ -110,17 +110,15 @@ class Revision(BASE, DeckhandBase): class RevisionTag(BASE, DeckhandBase): - UNIQUE_CONSTRAINTS = ('tag', 'revision_id') __tablename__ = 'revision_tags' - tag = Column(String(64), primary_key=True, nullable=False) + id = Column(Integer, primary_key=True) + tag = Column(String(64), nullable=False) data = Column(oslo_types.JsonEncodedDict(), nullable=True, default={}) revision_id = Column( Integer, ForeignKey('revisions.id', ondelete='CASCADE'), nullable=False) - UniqueConstraint(*UNIQUE_CONSTRAINTS) - class Document(BASE, DeckhandBase): UNIQUE_CONSTRAINTS = ('schema', 'name', 'revision_id') diff --git a/deckhand/tests/unit/control/test_revision_tags_controller.py b/deckhand/tests/unit/control/test_revision_tags_controller.py index 49af3192..252dad0b 100644 --- a/deckhand/tests/unit/control/test_revision_tags_controller.py +++ b/deckhand/tests/unit/control/test_revision_tags_controller.py @@ -24,8 +24,10 @@ class TestRevisionTagsController(test_base.BaseControllerTest): super(TestRevisionTagsController, self).setUp() rules = {'deckhand:create_cleartext_documents': '@'} self.policy.set_rules(rules) + self.revision_id = self._create_revision() - # Create a revision to tag. + def _create_revision(self): + # Create a revision with any document (doesn't matter). secrets_factory = factories.DocumentSecretFactory() payload = [secrets_factory.gen_test('Certificate', 'cleartext')] resp = self.app.simulate_put( @@ -33,8 +35,9 @@ class TestRevisionTagsController(test_base.BaseControllerTest): headers={'Content-Type': 'application/x-yaml'}, body=yaml.safe_dump_all(payload)) self.assertEqual(200, resp.status_code) - self.revision_id = list(yaml.safe_load_all(resp.text))[0]['status'][ + revision_id = list(yaml.safe_load_all(resp.text))[0]['status'][ 'revision'] + return revision_id def test_delete_tag(self): rules = {'deckhand:create_tag': '@', @@ -57,6 +60,22 @@ class TestRevisionTagsController(test_base.BaseControllerTest): headers={'Content-Type': 'application/x-yaml'}) self.assertEqual(404, resp.status_code) + def test_apply_same_tag_to_different_revisions(self): + rules = {'deckhand:create_cleartext_documents': '@', + 'deckhand:create_tag': '@'} + self.policy.set_rules(rules) + + # self.revision_id already created in setUp. + alt_revision_id = self._create_revision() + tag = 'same_tag' + + # Apply the same tag to both revisions. + for revision_id in (self.revision_id, alt_revision_id): + resp = self.app.simulate_post( + '/api/v1.0/revisions/%s/tags/%s' % (revision_id, tag), + headers={'Content-Type': 'application/x-yaml'}) + self.assertEqual(201, resp.status_code) + class TestRevisionTagsControllerNegativeRBAC(test_base.BaseControllerTest): """Test suite for validating negative RBAC scenarios for revision tags