airship/shipyard
Code Issues Proposed changes

[bugfix] Guards for configdocs creation API

Browse Source 
Addresses a situation where a missing header or invalid message body
was accepted as a good request (but empty), resulting in subsequent
failures during a standard creation of configdocs use case.

Change-Id: I7287ec890666c7cbcb1791be13619580a1254e8f
This commit is contained in:
Bryan Strassner 2018-04-30 18:09:32 -05:00
parent d3d3a0ecb7
commit 5484e7e6fe
4 changed files with 113 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
docs/source/API.rst
View File

@ -139,7 +139,7 @@ progress, this POST should be rejected with a 409 error.
Query Parameters
''''''''''''''''
- bufferMode=append|replace\|\ **rejectOnContents**
- buffermode=append|replace\|\ **rejectOnContents**
Indicates how the existing Shipyard Buffer should be handled. By default,
Shipyard will reject the POST if contents already exist in the Shipyard
Buffer.
@ -161,13 +161,21 @@ Responses
- The response headers will include a Location indicating the GET
endpoint to retrieve the configDocs
400 Bad Request
When:
- The request is missing a message body, attempting to create a collection
with no contents.
- The request has no new/changed contents for the collection.
- The request is missing a Content-Length header.
409 Conflict
A condition in the system is blocking this document ingestion
- If a commitconfigdocs POST is in progress.
- If any collections exist in the Shipyard Buffer unless bufferMode=replace
or bufferMode=append.
- If bufferMode=append, and the collection being posted is already in the
- If any collections exist in the Shipyard Buffer unless buffermode=replace
or buffermode=append.
- If buffermode=append, and the collection being posted is already in the
Shipyard Buffer
GET /v1.0/configdocs/{collection_id}

52
src/bin/shipyard_airflow/shipyard_airflow/control/configdocs/configdocs_api.py
View File

@ -22,7 +22,7 @@ from shipyard_airflow.control.configdocs import configdocs_helper
from shipyard_airflow.control.api_lock import (api_lock, ApiLockType)
from shipyard_airflow.control.base import BaseResource
from shipyard_airflow.control.configdocs.configdocs_helper import (
BufferMode, ConfigdocsHelper)
ConfigdocsHelper)
from shipyard_airflow.errors import ApiError
CONF = cfg.CONF
@ -55,13 +55,33 @@ class ConfigDocsResource(BaseResource):
"""
Ingests a collection of documents
"""
document_data = req.stream.read(req.content_length or 0)
content_length = req.content_length or 0
if (content_length == 0):
raise ApiError(
title=('Content-Length is a required header'),
description='Content Length is 0 or not specified',
status=falcon.HTTP_400,
error_list=[{
'message': (
'The Content-Length specified is 0 or not set. Check '
'that a valid payload is included with this request '
'and that your client is properly including a '
'Content-Length header. Note that a newline character '
'in a prior header can trigger subsequent headers to '
'be ignored and trigger this failure.')
}],
retry=False, )
document_data = req.stream.read(content_length)
buffer_mode = req.get_param('buffermode')
helper = ConfigdocsHelper(req.context)
validations = self.post_collection(
helper=helper,
collection_id=collection_id,
document_data=document_data,
buffer_mode_param=req.params.get('buffermode'))
buffer_mode_param=buffer_mode)
resp.location = '/api/v1.0/configdocs/{}'.format(collection_id)
resp.body = self.to_json(validations)
resp.status = falcon.HTTP_201
@ -105,15 +125,28 @@ class ConfigDocsResource(BaseResource):
"""
Ingest the collection after checking preconditions
"""
if buffer_mode_param is None:
buffer_mode = BufferMode.REJECTONCONTENTS
else:
buffer_mode = ConfigdocsHelper.get_buffer_mode(buffer_mode_param)
buffer_mode = ConfigdocsHelper.get_buffer_mode(buffer_mode_param)
if helper.is_buffer_valid_for_bucket(collection_id, buffer_mode):
buffer_revision = helper.add_collection(collection_id,
document_data)
return helper.get_deckhand_validation_status(buffer_revision)
if helper.is_collection_in_buffer(collection_id):
return helper.get_deckhand_validation_status(buffer_revision)
else:
raise ApiError(
title=('Collection {} not added to Shipyard '
'buffer'.format(collection_id)),
description='Collection empty or resulted in no revision',
status=falcon.HTTP_400,
error_list=[{
'message': (
'Empty collections are not supported. After '
'processing, the collection {} added no new '
'revision, and has been rejected as invalid '
'input'.format(collection_id))
}],
retry=False,
)
else:
raise ApiError(
title='Invalid collection specified for buffer',
@ -125,7 +158,8 @@ class ConfigDocsResource(BaseResource):
'Setting a different buffermode may '
'provide the desired functionality')
}],
retry=False, )
retry=False,
)
class CommitConfigDocsResource(BaseResource):

9
src/bin/shipyard_airflow/shipyard_airflow/control/configdocs/configdocs_helper.py
View File

@ -79,9 +79,12 @@ class ConfigdocsHelper(object):
self.revision_dict = None
@staticmethod
def get_buffer_mode(buffermode_string):
"""
Checks the buffer mode for valid values.
def get_buffer_mode(buffermode_string=None):
"""Checks the buffer mode for valid values.
:param buffermode_string: the string matching a valid buffer mode
Returns Buffermode.REJECTONTENTS value if the input is not specified
or is an invalid value.
"""
if buffermode_string:
try:

56
src/bin/shipyard_airflow/tests/unit/control/test_configdocs_api.py
View File

@ -49,9 +49,20 @@ class TestConfigDocsResource():
@patch.object(ConfigDocsResource, 'post_collection', common.str_responder)
@mock.patch.object(ApiLock, 'release')
@mock.patch.object(ApiLock, 'acquire')
def test_on_post(self, mock_acquire, mock_release, api_client):
def test_on_post_empty(self, mock_acquire, mock_release, api_client):
result = api_client.simulate_post(
"/api/v1.0/configdocs/coll1", headers=common.AUTH_HEADERS)
assert result.status_code == 400
assert 'Content-Length specified is 0 or not set' in result.text
@patch.object(ConfigDocsResource, 'post_collection', common.str_responder)
@mock.patch.object(ApiLock, 'release')
@mock.patch.object(ApiLock, 'acquire')
def test_on_post(self, mock_acquire, mock_release, api_client):
headers = {'Content-Length': '1'}
headers.update(common.AUTH_HEADERS)
result = api_client.simulate_post(
"/api/v1.0/configdocs/coll1", headers=headers, body='A')
assert result.status_code == 201
@patch.object(ConfigDocsResource, 'get_collection', common.str_responder)
@ -86,6 +97,8 @@ class TestConfigDocsResource():
mock_method.assert_called_once_with('buffer', 'apples')
@patch.object(ConfigdocsHelper, 'is_collection_in_buffer',
lambda x, y: True)
def test_post_collection(self):
"""
Tests the post collection method of the ConfigdocsResource
@ -106,17 +119,53 @@ class TestConfigDocsResource():
mock_method.assert_called_once_with(collection_id, document_data)
with pytest.raises(ApiError):
@patch.object(ConfigdocsHelper, 'is_collection_in_buffer',
lambda x, y: True)
@patch.object(ConfigdocsHelper, 'is_buffer_valid_for_bucket',
lambda x, y, z: False)
def test_post_collection_not_valid_for_buffer(self):
"""
Tests the post collection method of the ConfigdocsResource
"""
helper = None
collection_id = 'trees'
document_data = 'lots of info'
with pytest.raises(ApiError) as apie:
cdr = ConfigDocsResource()
helper = ConfigdocsHelper(CTX)
# not valid for bucket
helper.is_buffer_valid_for_bucket = lambda a, b: False
helper.get_deckhand_validation_status = (
lambda a: ConfigdocsHelper._format_validations_to_status([], 0)
)
cdr.post_collection(helper=helper,
collection_id=collection_id,
document_data=document_data)
assert apie.value.status == '409 Conflict'
@patch.object(ConfigdocsHelper, 'is_collection_in_buffer',
lambda x, y: False)
@patch.object(ConfigdocsHelper, 'is_buffer_valid_for_bucket',
lambda x, y, z: True)
def test_post_collection_not_added(self):
"""
Tests the post collection method of the ConfigdocsResource
"""
helper = None
collection_id = 'trees'
document_data = 'lots of info'
with patch.object(ConfigdocsHelper, 'add_collection') as mock_method:
cdr = ConfigDocsResource()
helper = ConfigdocsHelper(CTX)
helper.get_deckhand_validation_status = (
lambda a: ConfigdocsHelper._format_validations_to_status([], 0)
)
with pytest.raises(ApiError) as apie:
cdr.post_collection(helper=helper,
collection_id=collection_id,
document_data=document_data)
assert apie.value.status == '400 Bad Request'
mock_method.assert_called_once_with(collection_id, document_data)
class TestCommitConfigDocsResource():
@ -124,7 +173,6 @@ class TestCommitConfigDocsResource():
@mock.patch.object(ApiLock, 'acquire')
def test_on_post(self, mock_acquire, mock_release, api_client):
queries = ["", "force=true", "dryrun=true"]
ccdr = CommitConfigDocsResource()
with patch.object(
CommitConfigDocsResource, 'commit_configdocs', return_value={}
) as mock_method: