summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFelipe Monteiro <felipe.monteiro@att.com>2018-10-31 10:24:08 -0400
committerFelipe Monteiro <felipe.monteiro@att.com>2018-10-31 15:02:28 -0400
commitb03a4522cbf68c6661406216828b0548ab15850e (patch)
treec54fda77e1eeb99fb6f7be423eba8b20d8bed732
parent265a5bf18fee52c0ae25bf70cea3049114ef3505 (diff)
fix: Use schema instead of metadata.schema for replacement check
Recently added replacement check incorrectly uses metadata.schema and metadata.name to key on the document -- but it should be schema and metadata.name, the combination of which uniquely defines a document. Change-Id: I6cd1679ad41be38cb78d65ce2763e60f7da390d2
Notes
Notes (review): Code-Review+1: Rick Bartra <rb560u@att.com> Code-Review+2: Scott Hussey <sthussey@att.com> Code-Review+1: Vladyslav Drok <vdrok@mirantis.com> Workflow+1: Craig Anderson <craig.anderson@att.com> Code-Review+2: Craig Anderson <craig.anderson@att.com> Code-Review+1: Evgeniy L <eli@mirantis.com> Verified+2: Zuul Submitted-by: Zuul Submitted-at: Thu, 01 Nov 2018 22:04:19 +0000 Reviewed-on: https://review.openstack.org/614523 Project: openstack/airship-deckhand Branch: refs/heads/master
-rw-r--r--deckhand/engine/_replacement.py12
-rw-r--r--deckhand/errors.py41
-rw-r--r--deckhand/tests/functional/gabbits/schema-validation/schema-validation-success.yaml2
-rw-r--r--deckhand/tests/unit/control/test_errors.py4
-rw-r--r--deckhand/tests/unit/control/test_validations_controller.py2
-rw-r--r--deckhand/tests/unit/db/test_documents_negative.py2
-rw-r--r--deckhand/tests/unit/db/test_revisions.py4
-rw-r--r--deckhand/tests/unit/engine/test_document_layering_and_replacement_negative.py12
8 files changed, 38 insertions, 41 deletions
diff --git a/deckhand/engine/_replacement.py b/deckhand/engine/_replacement.py
index a420aad..d9bf3d7 100644
--- a/deckhand/engine/_replacement.py
+++ b/deckhand/engine/_replacement.py
@@ -107,16 +107,14 @@ def check_replacement_is_false_uniqueness(
107 if not document.is_control: 107 if not document.is_control:
108 document_identifier = ( 108 document_identifier = (
109 document['metadata']['name'], 109 document['metadata']['name'],
110 document['metadata']['schema'] 110 document['schema']
111 ) 111 )
112 if document_identifier in non_replacement_documents: 112 if document_identifier in non_replacement_documents:
113 error_message = ( 113 error_message = (
114 'Documents with the same name and schema existing in ' 114 'More than one document with the same name and schema was '
115 'different layers without any of them having ' 115 'found, but none has `replacement: true`. Ensure that only 2 '
116 '`replacement = true` cannot exist as Deckhand will ' 116 'documents exist for each replacement and that one has '
117 'arbitrarily select any of them for processing and none are ' 117 '`replacement: true` and the other `replacement: false`.')
118 'distinguishable from one another because none are a '
119 'parent or child or a replacement document.')
120 raise errors.InvalidDocumentReplacement( 118 raise errors.InvalidDocumentReplacement(
121 schema=document.schema, name=document.name, 119 schema=document.schema, name=document.name,
122 layer=document.layer, reason=error_message) 120 layer=document.layer, reason=error_message)
diff --git a/deckhand/errors.py b/deckhand/errors.py
index 54a30cf..9d31724 100644
--- a/deckhand/errors.py
+++ b/deckhand/errors.py
@@ -160,7 +160,7 @@ class DeckhandException(Exception):
160 a 'msg_fmt' property. That msg_fmt will get printf'd 160 a 'msg_fmt' property. That msg_fmt will get printf'd
161 with the keyword arguments provided to the constructor. 161 with the keyword arguments provided to the constructor.
162 """ 162 """
163 msg_fmt = "An unknown exception occurred." 163 msg_fmt = "An unknown exception occurred"
164 164
165 def __init__(self, message=None, code=500, **kwargs): 165 def __init__(self, message=None, code=500, **kwargs):
166 kwargs.setdefault('code', code) 166 kwargs.setdefault('code', code)
@@ -195,7 +195,7 @@ class InvalidDocumentFormat(DeckhandException):
195 195
196 **Troubleshoot:** 196 **Troubleshoot:**
197 """ 197 """
198 msg_fmt = ("The provided documents failed schema validation.") 198 msg_fmt = ("The provided documents failed schema validation")
199 code = 400 199 code = 400
200 200
201 201
@@ -210,7 +210,7 @@ class InvalidDocumentLayer(DeckhandException):
210 msg_fmt = ("Invalid layer '%(document_layer)s' for document " 210 msg_fmt = ("Invalid layer '%(document_layer)s' for document "
211 "[%(document_schema)s] %(document_name)s was not found in " 211 "[%(document_schema)s] %(document_name)s was not found in "
212 "layerOrder: %(layer_order)s for provided LayeringPolicy: " 212 "layerOrder: %(layer_order)s for provided LayeringPolicy: "
213 "%(layering_policy_name)s.") 213 "%(layering_policy_name)s")
214 code = 400 214 code = 400
215 215
216 216
@@ -234,7 +234,7 @@ class IndeterminateDocumentParent(DeckhandException):
234 **Troubleshoot:** 234 **Troubleshoot:**
235 """ 235 """
236 msg_fmt = ("Too many parent documents found for document [%(schema)s, " 236 msg_fmt = ("Too many parent documents found for document [%(schema)s, "
237 "%(layer)s] %(name)s. Found: %(found)s. Expected: 1.") 237 "%(layer)s] %(name)s. Found: %(found)s. Expected: 1")
238 code = 400 238 code = 400
239 239
240 240
@@ -246,7 +246,7 @@ class SubstitutionDependencyCycle(DeckhandException):
246 * Check that there is no two-way substitution dependency between documents. 246 * Check that there is no two-way substitution dependency between documents.
247 """ 247 """
248 msg_fmt = ('Cannot determine substitution order as a dependency ' 248 msg_fmt = ('Cannot determine substitution order as a dependency '
249 'cycle exists for the following documents: %(cycle)s.') 249 'cycle exists for the following documents: %(cycle)s')
250 code = 400 250 code = 400
251 251
252 252
@@ -266,8 +266,7 @@ class MissingDocumentKey(DeckhandException):
266 msg_fmt = ("Missing action path in %(action)s needed for layering from " 266 msg_fmt = ("Missing action path in %(action)s needed for layering from "
267 "either the data section of the parent [%(parent_schema)s, " 267 "either the data section of the parent [%(parent_schema)s, "
268 "%(parent_layer)s] %(parent_name)s or child [%(child_schema)s, " 268 "%(parent_layer)s] %(parent_name)s or child [%(child_schema)s, "
269 "%(child_layer)s] %(child_name)s " 269 "%(child_layer)s] %(child_name)s document")
270 "document.")
271 code = 400 270 code = 400
272 271
273 272
@@ -283,7 +282,7 @@ class MissingDocumentPattern(DeckhandException):
283 msg_fmt = ("The destination document's `data` section is missing the " 282 msg_fmt = ("The destination document's `data` section is missing the "
284 "pattern %(pattern)s specified under " 283 "pattern %(pattern)s specified under "
285 "`substitutions.dest.pattern` at path %(jsonpath)s, specified " 284 "`substitutions.dest.pattern` at path %(jsonpath)s, specified "
286 "under `substitutions.dest.path`.") 285 "under `substitutions.dest.path`")
287 code = 400 286 code = 400
288 287
289 288
@@ -308,7 +307,7 @@ class UnsupportedActionMethod(DeckhandException):
308 307
309 **Troubleshoot:** 308 **Troubleshoot:**
310 """ 309 """
311 msg_fmt = ("Method in %(actions)s is invalid for document %(document)s.") 310 msg_fmt = ("Method in %(actions)s is invalid for document %(document)s")
312 code = 400 311 code = 400
313 312
314 313
@@ -318,7 +317,7 @@ class RevisionTagBadFormat(DeckhandException):
318 **Troubleshoot:** 317 **Troubleshoot:**
319 """ 318 """
320 msg_fmt = ("The requested tag data %(data)s must either be null or " 319 msg_fmt = ("The requested tag data %(data)s must either be null or "
321 "dictionary.") 320 "dictionary")
322 code = 400 321 code = 400
323 322
324 323
@@ -338,7 +337,7 @@ class SubstitutionSourceDataNotFound(DeckhandException):
338 "%(src_path)s in source document [%(src_schema)s, %(src_layer)s] " 337 "%(src_path)s in source document [%(src_schema)s, %(src_layer)s] "
339 "%(src_name)s which is referenced by destination document " 338 "%(src_name)s which is referenced by destination document "
340 "[%(dest_schema)s, %(dest_layer)s] %(dest_name)s under its " 339 "[%(dest_schema)s, %(dest_layer)s] %(dest_name)s under its "
341 "`metadata.substitutions`.") 340 "`metadata.substitutions`")
342 code = 400 341 code = 400
343 342
344 343
@@ -352,7 +351,7 @@ class EncryptionSourceNotFound(DeckhandException):
352 msg_fmt = ( 351 msg_fmt = (
353 "Required encryption source reference could not be resolved into a " 352 "Required encryption source reference could not be resolved into a "
354 "secret because it was not found among encryption sources. Ref: " 353 "secret because it was not found among encryption sources. Ref: "
355 "%(secret_ref)s. Referenced by: [%(schema)s, %(layer)s] %(name)s.") 354 "%(secret_ref)s. Referenced by: [%(schema)s, %(layer)s] %(name)s")
356 code = 400 # Indicates bad data was passed in, causing a lookup to fail. 355 code = 400 # Indicates bad data was passed in, causing a lookup to fail.
357 356
358 357
@@ -362,7 +361,7 @@ class DocumentNotFound(DeckhandException):
362 **Troubleshoot:** 361 **Troubleshoot:**
363 """ 362 """
364 msg_fmt = ("The requested document using filters: %(filters)s was not " 363 msg_fmt = ("The requested document using filters: %(filters)s was not "
365 "found.") 364 "found")
366 code = 404 365 code = 404
367 366
368 367
@@ -371,7 +370,7 @@ class RevisionNotFound(DeckhandException):
371 370
372 **Troubleshoot:** 371 **Troubleshoot:**
373 """ 372 """
374 msg_fmt = "The requested revision=%(revision_id)s was not found." 373 msg_fmt = "The requested revision=%(revision_id)s was not found"
375 code = 404 374 code = 404
376 375
377 376
@@ -381,7 +380,7 @@ class RevisionTagNotFound(DeckhandException):
381 **Troubleshoot:** 380 **Troubleshoot:**
382 """ 381 """
383 msg_fmt = ("The requested tag '%(tag)s' for revision %(revision)s was " 382 msg_fmt = ("The requested tag '%(tag)s' for revision %(revision)s was "
384 "not found.") 383 "not found")
385 code = 404 384 code = 404
386 385
387 386
@@ -392,7 +391,7 @@ class ValidationNotFound(DeckhandException):
392 """ 391 """
393 msg_fmt = ("The requested validation entry %(entry_id)s was not found " 392 msg_fmt = ("The requested validation entry %(entry_id)s was not found "
394 "for validation name %(validation_name)s and revision ID " 393 "for validation name %(validation_name)s and revision ID "
395 "%(revision_id)s.") 394 "%(revision_id)s")
396 code = 404 395 code = 404
397 396
398 397
@@ -403,7 +402,7 @@ class DuplicateDocumentExists(DeckhandException):
403 **Troubleshoot:** 402 **Troubleshoot:**
404 """ 403 """
405 msg_fmt = ("Document [%(schema)s, %(layer)s] %(name)s already exists in " 404 msg_fmt = ("Document [%(schema)s, %(layer)s] %(name)s already exists in "
406 "bucket: %(bucket)s.") 405 "bucket: %(bucket)s")
407 code = 409 406 code = 409
408 407
409 408
@@ -416,7 +415,7 @@ class SingletonDocumentConflict(DeckhandException):
416 msg_fmt = ("A singleton document [%(schema)s, %(layer)s] %(name)s already " 415 msg_fmt = ("A singleton document [%(schema)s, %(layer)s] %(name)s already "
417 "exists in the system. The new document(s) %(conflict)s cannot " 416 "exists in the system. The new document(s) %(conflict)s cannot "
418 "be created. To create a document with a new name, delete the " 417 "be created. To create a document with a new name, delete the "
419 "current one first.") 418 "current one first")
420 code = 409 419 code = 409
421 420
422 421
@@ -425,7 +424,7 @@ class LayeringPolicyNotFound(DeckhandException):
425 424
426 **Troubleshoot:** 425 **Troubleshoot:**
427 """ 426 """
428 msg_fmt = ("Required LayeringPolicy was not found for layering.") 427 msg_fmt = ("Required LayeringPolicy was not found for layering")
429 code = 409 428 code = 409
430 429
431 430
@@ -440,7 +439,7 @@ class SubstitutionSourceNotFound(DeckhandException):
440 msg_fmt = ( 439 msg_fmt = (
441 "Required substitution source document [%(src_schema)s] %(src_name)s " 440 "Required substitution source document [%(src_schema)s] %(src_name)s "
442 "was not found, yet is referenced by [%(document_schema)s] " 441 "was not found, yet is referenced by [%(document_schema)s] "
443 "%(document_name)s.") 442 "%(document_name)s")
444 code = 409 443 code = 409
445 444
446 445
@@ -449,7 +448,7 @@ class PolicyNotAuthorized(DeckhandException):
449 448
450 **Troubleshoot:** 449 **Troubleshoot:**
451 """ 450 """
452 msg_fmt = "Policy doesn't allow %(action)s to be performed." 451 msg_fmt = "Policy doesn't allow %(action)s to be performed"
453 code = 403 452 code = 403
454 453
455 454
diff --git a/deckhand/tests/functional/gabbits/schema-validation/schema-validation-success.yaml b/deckhand/tests/functional/gabbits/schema-validation/schema-validation-success.yaml
index d984a1c..da032d0 100644
--- a/deckhand/tests/functional/gabbits/schema-validation/schema-validation-success.yaml
+++ b/deckhand/tests/functional/gabbits/schema-validation/schema-validation-success.yaml
@@ -161,7 +161,7 @@ tests:
161 $.[0].details.messageList[0].level: Error 161 $.[0].details.messageList[0].level: Error
162 $.[0].details.messageList[0].name: D002 162 $.[0].details.messageList[0].name: D002
163 $.[0].kind: Status 163 $.[0].kind: Status
164 $.[0].message: The provided documents failed schema validation. 164 $.[0].message: The provided documents failed schema validation
165 $.[0].reason: Validation 165 $.[0].reason: Validation
166 $.[0].status: Failure 166 $.[0].status: Failure
167 167
diff --git a/deckhand/tests/unit/control/test_errors.py b/deckhand/tests/unit/control/test_errors.py
index ed18407..538973a 100644
--- a/deckhand/tests/unit/control/test_errors.py
+++ b/deckhand/tests/unit/control/test_errors.py
@@ -158,7 +158,7 @@ class TestValidationMessageFormatting(test_base.BaseControllerTest):
158 } 158 }
159 ] 159 ]
160 }, 160 },
161 'message': 'The provided documents failed schema validation.', 161 'message': 'The provided documents failed schema validation',
162 'metadata': {} 162 'metadata': {}
163 } 163 }
164 body = yaml.safe_load(resp.text) 164 body = yaml.safe_load(resp.text)
@@ -224,7 +224,7 @@ class TestValidationMessageFormatting(test_base.BaseControllerTest):
224 } 224 }
225 ] 225 ]
226 }, 226 },
227 'message': 'The provided documents failed schema validation.', 227 'message': 'The provided documents failed schema validation',
228 'metadata': {} 228 'metadata': {}
229 } 229 }
230 body = yaml.safe_load(resp.text) 230 body = yaml.safe_load(resp.text)
diff --git a/deckhand/tests/unit/control/test_validations_controller.py b/deckhand/tests/unit/control/test_validations_controller.py
index 05c94d4..15bcb7c 100644
--- a/deckhand/tests/unit/control/test_validations_controller.py
+++ b/deckhand/tests/unit/control/test_validations_controller.py
@@ -348,7 +348,7 @@ class TestValidationsController(BaseValidationsControllerTest):
348 VALIDATION_FAILURE_RESULT) 348 VALIDATION_FAILURE_RESULT)
349 self.assertEqual(201, resp.status_code) 349 self.assertEqual(201, resp.status_code)
350 expected_error = ('The requested validation entry 5 was not found for ' 350 expected_error = ('The requested validation entry 5 was not found for '
351 'validation name %s and revision ID %d.' % ( 351 'validation name %s and revision ID %d' % (
352 validation_name, revision_id)) 352 validation_name, revision_id))
353 353
354 resp = self.app.simulate_get( 354 resp = self.app.simulate_get(
diff --git a/deckhand/tests/unit/db/test_documents_negative.py b/deckhand/tests/unit/db/test_documents_negative.py
index 1633a13..dcd5bb9 100644
--- a/deckhand/tests/unit/db/test_documents_negative.py
+++ b/deckhand/tests/unit/db/test_documents_negative.py
@@ -61,7 +61,7 @@ class TestDocumentsNegative(base.DeckhandWithDBTestCase):
61 61
62 # Verify that the document cannot be created in another bucket. 62 # Verify that the document cannot be created in another bucket.
63 alt_bucket_name = test_utils.rand_name('bucket') 63 alt_bucket_name = test_utils.rand_name('bucket')
64 error_re = r"^Document .* already exists in bucket: %s.$" % bucket_name 64 error_re = r"^Document .* already exists in bucket: %s$" % bucket_name
65 self.assertRaisesRegex( 65 self.assertRaisesRegex(
66 errors.DuplicateDocumentExists, error_re, self.create_documents, 66 errors.DuplicateDocumentExists, error_re, self.create_documents,
67 alt_bucket_name, payload) 67 alt_bucket_name, payload)
diff --git a/deckhand/tests/unit/db/test_revisions.py b/deckhand/tests/unit/db/test_revisions.py
index 34419ba..e26c0f9 100644
--- a/deckhand/tests/unit/db/test_revisions.py
+++ b/deckhand/tests/unit/db/test_revisions.py
@@ -126,7 +126,7 @@ class TestRevisions(base.DeckhandWithDBTestCase):
126 126
127 # Validate that all revisions were deleted. 127 # Validate that all revisions were deleted.
128 for revision_id in all_revision_ids: 128 for revision_id in all_revision_ids:
129 error_re = (r'^The requested revision=%s was not found.$' 129 error_re = (r'^The requested revision=%s was not found$'
130 % revision_id) 130 % revision_id)
131 self.assertRaisesRegex(errors.RevisionNotFound, error_re, 131 self.assertRaisesRegex(errors.RevisionNotFound, error_re,
132 self.show_revision, revision_id) 132 self.show_revision, revision_id)
@@ -135,7 +135,7 @@ class TestRevisions(base.DeckhandWithDBTestCase):
135 for doc in created_documents: 135 for doc in created_documents:
136 filters = {'id': doc['id']} 136 filters = {'id': doc['id']}
137 error_re = (r'^The requested document using filters: %s was not ' 137 error_re = (r'^The requested document using filters: %s was not '
138 'found.$' % filters) 138 'found$' % filters)
139 self.assertRaisesRegex(errors.DocumentNotFound, error_re, 139 self.assertRaisesRegex(errors.DocumentNotFound, error_re,
140 self.show_document, **filters) 140 self.show_document, **filters)
141 141
diff --git a/deckhand/tests/unit/engine/test_document_layering_and_replacement_negative.py b/deckhand/tests/unit/engine/test_document_layering_and_replacement_negative.py
index 7f73a94..2515f0c 100644
--- a/deckhand/tests/unit/engine/test_document_layering_and_replacement_negative.py
+++ b/deckhand/tests/unit/engine/test_document_layering_and_replacement_negative.py
@@ -139,9 +139,9 @@ class TestDocumentLayeringReplacementNegative(
139 self._test_layering, documents) 139 self._test_layering, documents)
140 140
141 def test_replacement_true_with_parent_replacement_true_raises_exc(self): 141 def test_replacement_true_with_parent_replacement_true_raises_exc(self):
142 """Validate that when documents have the same `metadata.name` and 142 """Validate that when documents have the same ``metadata.name`` and
143 `metadata.schema` existing in different layers without any of them 143 ``schema`` existing in different layers without any of them
144 having `replacement = true` raises an exception 144 having ``replacement = true`` raises an exception
145 """ 145 """
146 doc_factory = factories.DocumentFactory(2, [1, 1]) 146 doc_factory = factories.DocumentFactory(2, [1, 1])
147 documents = doc_factory.gen_test({}) 147 documents = doc_factory.gen_test({})
@@ -154,8 +154,8 @@ class TestDocumentLayeringReplacementNegative(
154 document['metadata']['layeringDefinition'].pop( 154 document['metadata']['layeringDefinition'].pop(
155 'parentSelector') 155 'parentSelector')
156 156
157 error_re = (r'Documents with the same name and schema existing in ' 157 error_re = (
158 'different layers without any of them having ' 158 r'More than one document with the same name and schema was found, '
159 '`replacement = true` cannot exist.*') 159 'but none has `replacement: true`.*')
160 self.assertRaisesRegexp(errors.InvalidDocumentReplacement, error_re, 160 self.assertRaisesRegexp(errors.InvalidDocumentReplacement, error_re,
161 self._test_layering, documents) 161 self._test_layering, documents)