openstack/swift
Code Issues Proposed changes

Read-only middleware fixups

Browse Source 
- Expose cluster-wide read-only status in /info
- Use a set instead of list for write_methods
- Provide a more explicit response when returning 405
- Consider self.read_only in account_read_only
- Use return_values instead of callables for more mocks

Change-Id: I45387067a69919706800df3a8ca12ae8f5d16d90
This commit is contained in:
Tim Burke 2018-05-29 12:47:47 -07:00
parent c01c43d982
commit beb014b0b1
2 changed files with 22 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
swift/common/middleware/read_only.py
View File

@ -14,7 +14,8 @@
# limitations under the License. # limitations under the License.
from swift.common.constraints import check_account_format from swift.common.constraints import check_account_format
from swift.common.swob import HTTPMethodNotAllowed, Request from swift.common.swob import HTTPMethodNotAllowed, Request
from swift.common.utils import get_logger, config_true_value from swift.common.utils import get_logger, config_true_value, \
register_swift_info
from swift.proxy.controllers.base import get_info from swift.proxy.controllers.base import get_info
""" """
@ -67,9 +68,9 @@ class ReadOnlyMiddleware(object):
self.app = app self.app = app
self.logger = logger or get_logger(conf, log_route='read_only') self.logger = logger or get_logger(conf, log_route='read_only')
self.read_only = config_true_value(conf.get('read_only')) self.read_only = config_true_value(conf.get('read_only'))
self.write_methods = ['COPY', 'POST', 'PUT'] self.write_methods = {'COPY', 'POST', 'PUT'}
if not config_true_value(conf.get('allow_deletes')): if not config_true_value(conf.get('allow_deletes')):
self.write_methods += ['DELETE'] self.write_methods.add('DELETE')
def __call__(self, env, start_response): def __call__(self, env, start_response):
req = Request(env) req = Request(env)
@ -86,24 +87,23 @@ class ReadOnlyMiddleware(object):
dest_account = req.headers.get('Destination-Account') dest_account = req.headers.get('Destination-Account')
account = check_account_format(req, dest_account) account = check_account_format(req, dest_account)
account_read_only = self.account_read_only(req, account) if self.account_read_only(req, account):
if account_read_only is False: msg = 'Writes are disabled for this account.'
return self.app(env, start_response) return HTTPMethodNotAllowed(body=msg)(env, start_response)
if self.read_only or account_read_only:
return HTTPMethodNotAllowed()(env, start_response)
return self.app(env, start_response) return self.app(env, start_response)
def account_read_only(self, req, account): def account_read_only(self, req, account):
""" """
Returns None if X-Account-Sysmeta-Read-Only is not set. Check whether an account should be read-only.
Returns True or False otherwise.
This considers both the cluster-wide config value as well as the
per-account override in X-Account-Sysmeta-Read-Only.
""" """
info = get_info(self.app, req.environ, account, swift_source='RO') info = get_info(self.app, req.environ, account, swift_source='RO')
read_only = info.get('sysmeta', {}).get('read-only', '') read_only = info.get('sysmeta', {}).get('read-only', '')
if read_only == '': if not read_only:
return None return self.read_only
return config_true_value(read_only) return config_true_value(read_only)
@ -114,6 +114,9 @@ def filter_factory(global_conf, **local_conf):
conf = global_conf.copy() conf = global_conf.copy()
conf.update(local_conf) conf.update(local_conf)
if config_true_value(conf.get('read_only')):
register_swift_info('read_only')
def read_only_filter(app): def read_only_filter(app):
return ReadOnlyMiddleware(app, conf) return ReadOnlyMiddleware(app, conf)

28
test/unit/common/middleware/test_read_only.py
View File

@ -32,8 +32,7 @@ def start_response(*args):
read_methods = 'GET HEAD'.split() read_methods = 'GET HEAD'.split()
write_methods = 'COPY DELETE POST PUT'.split() write_methods = 'COPY DELETE POST PUT'.split()
ro_resp = ['<html><h1>Method Not Allowed</h1><p>The method is not ' + ro_resp = ['Writes are disabled for this account.']
'allowed for this resource.</p></html>']
class TestReadOnly(unittest.TestCase): class TestReadOnly(unittest.TestCase):
@ -81,11 +80,8 @@ class TestReadOnly(unittest.TestCase):
ro = read_only.filter_factory(conf)(FakeApp()) ro = read_only.filter_factory(conf)(FakeApp())
ro.logger = FakeLogger() ro.logger = FakeLogger()
def get_fake_read_only(*args, **kwargs):
return {'sysmeta': {'read-only': 'true'}}
with mock.patch('swift.common.middleware.read_only.get_info', with mock.patch('swift.common.middleware.read_only.get_info',
get_fake_read_only): return_value={'sysmeta': {'read-only': 'true'}}):
for method in read_methods: for method in read_methods:
req = Request.blank('/v/a') req = Request.blank('/v/a')
req.method = method req.method = method
@ -104,11 +100,8 @@ class TestReadOnly(unittest.TestCase):
ro = read_only.filter_factory(conf)(FakeApp()) ro = read_only.filter_factory(conf)(FakeApp())
ro.logger = FakeLogger() ro.logger = FakeLogger()
def get_fake_read_only(*args, **kwargs):
return {'sysmeta': {'read-only': 'false'}}
with mock.patch('swift.common.middleware.read_only.get_info', with mock.patch('swift.common.middleware.read_only.get_info',
get_fake_read_only): return_value={'sysmeta': {'read-only': 'false'}}):
for method in read_methods + write_methods: for method in read_methods + write_methods:
req = Request.blank('/v/a') req = Request.blank('/v/a')
req.method = method req.method = method
@ -123,11 +116,8 @@ class TestReadOnly(unittest.TestCase):
ro = read_only.filter_factory(conf)(FakeApp()) ro = read_only.filter_factory(conf)(FakeApp())
ro.logger = FakeLogger() ro.logger = FakeLogger()
def get_fake_read_only(*args, **kwargs):
return {'sysmeta': {'read-only': 'false'}}
with mock.patch('swift.common.middleware.read_only.get_info', with mock.patch('swift.common.middleware.read_only.get_info',
get_fake_read_only): return_value={'sysmeta': {'read-only': 'false'}}):
for method in read_methods + write_methods: for method in read_methods + write_methods:
req = Request.blank('/v/a') req = Request.blank('/v/a')
req.method = method req.method = method
@ -158,11 +148,8 @@ class TestReadOnly(unittest.TestCase):
ro = read_only.filter_factory(conf)(FakeApp()) ro = read_only.filter_factory(conf)(FakeApp())
ro.logger = FakeLogger() ro.logger = FakeLogger()
def get_fake_read_only(*args, **kwargs):
return {'sysmeta': {'read-only': 'on'}}
with mock.patch('swift.common.middleware.read_only.get_info', with mock.patch('swift.common.middleware.read_only.get_info',
get_fake_read_only): return_value={'sysmeta': {'read-only': 'on'}}):
req = Request.blank('/v/a') req = Request.blank('/v/a')
req.method = "DELETE" req.method = "DELETE"
resp = ro(req.environ, start_response) resp = ro(req.environ, start_response)
@ -235,13 +222,10 @@ class TestReadOnly(unittest.TestCase):
ro = read_only.filter_factory(conf)(FakeApp()) ro = read_only.filter_factory(conf)(FakeApp())
ro.logger = FakeLogger() ro.logger = FakeLogger()
def fake_account_read_only(*args, **kwargs):
return 'true'
with mock.patch( with mock.patch(
'swift.common.middleware.read_only.ReadOnlyMiddleware.' + 'swift.common.middleware.read_only.ReadOnlyMiddleware.' +
'account_read_only', 'account_read_only',
fake_account_read_only): return_value='true'):
headers = {'Destination-Account': 'b'} headers = {'Destination-Account': 'b'}
req = Request.blank('/v/a', headers=headers) req = Request.blank('/v/a', headers=headers)
req.method = "COPY" req.method = "COPY"