Merge "Improve ExternalServiceFilter"

This commit is contained in:
Zuul 2022-09-15 16:54:31 +00:00 committed by Gerrit Code Review
commit 9ed6780f11
5 changed files with 398 additions and 95 deletions

View File

@ -21,3 +21,8 @@ class MaxLeaseDurationException(exceptions.NotAuthorized):
msg_fmt = _('Lease duration of %(lease_duration)s seconds must be less '
'than or equal to the maximum lease duration of '
'%(max_duration)s seconds.')
class ExternalServiceFilterException(exceptions.BlazarException):
code = 400
msg_fmt = _('%(message)s')

View File

@ -22,9 +22,12 @@ class BaseFilter(metaclass=abc.ABCMeta):
def __init__(self, conf=None):
self.conf = conf
self.register_opts(conf)
for opt in self.enforcement_opts:
self.conf.register_opt(opt, 'enforcement')
@classmethod
def register_opts(cls, conf):
for opt in cls.enforcement_opts:
conf.register_opt(opt, 'enforcement')
def __getattr__(self, name):
func = getattr(self.conf.enforcement, name)

View File

@ -13,14 +13,16 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import datetime
from datetime import datetime
import json
import requests
from urllib.parse import urljoin
from urllib.parse import urlparse
from blazar.enforcement.exceptions import ExternalServiceFilterException
from blazar.enforcement.filters import base_filter
from blazar import exceptions
from blazar.exceptions import BlazarException
from blazar.i18n import _
from blazar.utils.openstack.keystone import BlazarKeystoneClient
from oslo_config import cfg
from oslo_log import log as logging
@ -28,29 +30,18 @@ from oslo_log import log as logging
LOG = logging.getLogger(__name__)
class DateTimeEncoder(json.JSONEncoder):
class ISODateTimeEncoder(json.JSONEncoder):
def default(self, o):
if isinstance(o, datetime.datetime):
return str(o)
if isinstance(o, datetime):
return o.isoformat()
return json.JSONEncoder.default(self, o)
class ExternalServiceUnsupportedHTTPResponse(exceptions.BlazarException):
code = 400
msg_fmt = _('External service enforcement filter returned a %(status)s '
'HTTP response. Only 204 and 403 responses are supported.')
GENERIC_DENY_MSG = 'External service enforcement filter denied the request.'
class ExternalServiceUnsupportedDeniedResponse(exceptions.BlazarException):
code = 400
msg_fmt = _('External service enforcement filter returned a 403 HTTP '
'response %(response)s without a valid JSON dictionary '
'containing a "message" key.')
class ExternalServiceFilterException(exceptions.NotAuthorized):
code = 400
class ExternalServiceMisconfigured(BlazarException):
msg_fmt = _('%(message)s')
@ -58,21 +49,21 @@ class ExternalServiceFilter(base_filter.BaseFilter):
enforcement_opts = [
cfg.StrOpt(
'external_service_endpoint',
'external_service_base_endpoint',
default=None,
help='The URL of the external service API.'),
cfg.StrOpt(
'external_service_check_create',
'external_service_check_create_endpoint',
default=None,
help='Overwrite check-create endpoint with absolute URL.'),
help='Overrides check-create endpoint with another URL.'),
cfg.StrOpt(
'external_service_check_update',
'external_service_check_update_endpoint',
default=None,
help='Overwrite check-update endpoint with absolute URL.'),
help='Overrides check-update endpoint with another URL.'),
cfg.StrOpt(
'external_service_on_end',
'external_service_on_end_endpoint',
default=None,
help='Overwrite on-end endpoint with absolute URL.'),
help='Overrides on-end endpoint with another URL.'),
cfg.StrOpt(
'external_service_token',
default="",
@ -82,75 +73,99 @@ class ExternalServiceFilter(base_filter.BaseFilter):
def __init__(self, conf=None):
super(ExternalServiceFilter, self).__init__(conf=conf)
def get_headers(self):
self._validate_url(conf.enforcement.external_service_base_endpoint)
self.base_endpoint = conf.enforcement.external_service_base_endpoint
self.check_create_endpoint = self._construct_url(
"check-create",
conf.enforcement.external_service_check_create_endpoint)
self.check_update_endpoint = self._construct_url(
"check-update",
conf.enforcement.external_service_check_update_endpoint)
self.on_end_endpoint = self._construct_url(
"on-end",
conf.enforcement.external_service_on_end_endpoint)
endpoints = (
self.check_create_endpoint,
self.check_update_endpoint,
self.on_end_endpoint,
)
if all(x is None for x in endpoints):
raise ExternalServiceMisconfigured(
message=_("ExternalService has no endpoints set."))
self.token = conf.enforcement.external_service_token
@staticmethod
def _validate_url(url):
if url is None:
return
parsed_url = urlparse(url)
if parsed_url.scheme not in ("http", "https"):
raise ExternalServiceMisconfigured(
message=_("ExternalService URL scheme must be http(s): "
"%s") % url)
if parsed_url.netloc == '':
raise ExternalServiceMisconfigured(
message=_("ExternalService URL must have netloc: "
"%s") % url)
def _construct_url(self, method, replacement_url):
if replacement_url is None:
if self.base_endpoint is None:
return None
return urljoin(self.base_endpoint, method)
self._validate_url(replacement_url)
return replacement_url
def _get_headers(self):
headers = {'Content-Type': 'application/json'}
if self.external_service_token:
headers['X-Auth-Token'] = (self.external_service_token)
else:
client = BlazarKeystoneClient()
headers['X-Auth-Token'] = client.session.get_token()
if self.token:
headers['X-Auth-Token'] = self.token
return headers
def _get_absolute_url(self, path):
url = self.external_service_endpoint
def _post(self, url, body):
body = json.dumps(body, cls=ISODateTimeEncoder)
res = requests.post(url, headers=self._get_headers(), data=body)
if url[-1] == '/':
url += path[1:]
else:
url += path
return url
def post(self, url, body):
body = json.dumps(body, cls=DateTimeEncoder)
req = requests.post(url, headers=self.get_headers(), data=body)
if req.status_code == 204:
if res.status_code == 204:
return True
elif req.status_code == 403:
elif res.status_code == 403:
try:
message = req.json()['message']
message = res.json()['message']
except (requests.JSONDecodeError, KeyError):
raise ExternalServiceUnsupportedDeniedResponse(
response=req.content)
raise ExternalServiceFilterException(message=message)
# NOTE(yoctozepto): It is more secure not to send the actual
# response to the end user as it may leak something.
# Instead, we log it for debugging.
LOG.debug("The External Service API returned a malformed "
"response (403): %s", res.content)
message = GENERIC_DENY_MSG
else:
raise ExternalServiceUnsupportedHTTPResponse(
status=req.status_code)
# NOTE(yoctozepto): It is more secure not to send the actual
# response to the end user as it may leak something.
# Instead, we log it for debugging.
LOG.debug("The External Service API returned a malformed "
"response (%d): %s", res.status_code, res.content)
message = GENERIC_DENY_MSG
raise ExternalServiceFilterException(message=message)
def check_create(self, context, lease_values):
body = dict(context=context, lease=lease_values)
if self.external_service_check_create:
self.post(self.external_service_check_create, body)
return
if self.external_service_endpoint:
path = '/check-create'
self.post(self._get_absolute_url(path), body)
return
if self.check_create_endpoint:
self._post(self.check_create_endpoint, dict(
context=context, lease=lease_values))
def check_update(self, context, current_lease_values, new_lease_values):
body = dict(context=context, current_lease=current_lease_values,
lease=new_lease_values)
if self.external_service_check_update:
self.post(self.external_service_check_update, body)
return
if self.external_service_endpoint:
path = '/check-update'
self.post(self._get_absolute_url(path), body)
return
if self.check_update_endpoint:
self._post(self.check_update_endpoint, dict(
context=context, current_lease=current_lease_values,
lease=new_lease_values))
def on_end(self, context, lease_values):
body = dict(context=context, lease=lease_values)
if self.external_service_on_end:
self.post(self.external_service_on_end, body)
return
if self.external_service_endpoint:
path = '/on-end'
self.post(self._get_absolute_url(path), body)
return
if self.on_end_endpoint:
self._post(self.on_end_endpoint, dict(
context=context, lease=lease_values))

View File

@ -0,0 +1,252 @@
# Copyright (c) 2022 Radosław Piliszek <radoslaw.piliszek@gmail.com>
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
# implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import datetime
import json
from unittest import mock
from blazar.enforcement.exceptions import ExternalServiceFilterException
from blazar.enforcement.filters import external_service_filter
from blazar.tests import TestCase
from oslo_config.cfg import CONF
class FakeResponse204():
status_code = 204
class FakeResponse403Empty():
status_code = 403
content = "irrelevant_but_logged"
@staticmethod
def json():
return {}
class FakeJSONDecodeError(Exception):
pass
class FakeResponse403InvalidJSON():
status_code = 403
content = "NOT_JSON"
@staticmethod
def json():
raise FakeJSONDecodeError()
class FakeResponse403WithMessage():
status_code = 403
content = "irrelevant"
@staticmethod
def json():
return {"message": "Hello!"}
class FakeResponse500():
status_code = 500
content = "ALL_YOUR_BUGS_BELONG_TO_US"
class ISODateTimeEncoderTestCase(TestCase):
def test_json_date(self):
dt = datetime.datetime(2022, 9, 8, 13, 31, 44, 12345)
obj = {"datetime": dt}
x = json.dumps(obj, cls=external_service_filter.ISODateTimeEncoder)
self.assertEqual('{"datetime": "2022-09-08T13:31:44.012345"}', x)
def test_json_with_tz(self):
tz = datetime.timezone(datetime.timedelta(hours=2))
dt = datetime.datetime(2022, 9, 8, 13, 31, 44, 12345, tz)
obj = {"datetime": dt}
x = json.dumps(obj, cls=external_service_filter.ISODateTimeEncoder)
self.assertEqual('{"datetime": "2022-09-08T13:31:44.012345+02:00"}', x)
class ConfiguringExternalServiceFilterTestCase(TestCase):
def setUp(self):
super().setUp()
external_service_filter.ExternalServiceFilter.register_opts(CONF)
def test_basic_misconfiguration(self):
self.assertRaises(external_service_filter.ExternalServiceMisconfigured,
external_service_filter.ExternalServiceFilter, CONF)
def test_bad_url(self):
CONF.set_override(
'external_service_base_endpoint', 'this_url_cOuLDnOtBeWoRsE',
group='enforcement')
self.addCleanup(CONF.clear_override, 'external_service_base_endpoint',
group='enforcement')
self.assertRaises(external_service_filter.ExternalServiceMisconfigured,
external_service_filter.ExternalServiceFilter, CONF)
def test_check_create_endpoint_is_enough(self):
CONF.set_override(
'external_service_check_create_endpoint', 'http://localhost',
group='enforcement')
self.addCleanup(CONF.clear_override,
'external_service_check_create_endpoint',
group='enforcement')
external_service_filter.ExternalServiceFilter(CONF)
def test_check_updaye_endpoint_is_enough(self):
CONF.set_override(
'external_service_check_update_endpoint', 'http://localhost',
group='enforcement')
self.addCleanup(CONF.clear_override,
'external_service_check_update_endpoint',
group='enforcement')
external_service_filter.ExternalServiceFilter(CONF)
def test_on_end_endpoint_is_enough(self):
CONF.set_override(
'external_service_on_end_endpoint', 'http://localhost',
group='enforcement')
self.addCleanup(CONF.clear_override,
'external_service_on_end_endpoint',
group='enforcement')
external_service_filter.ExternalServiceFilter(CONF)
class ExternalServiceFilterTestCase(TestCase):
def setUp(self):
super().setUp()
external_service_filter.ExternalServiceFilter.register_opts(CONF)
CONF.set_override(
'external_service_base_endpoint', 'http://localhost',
group='enforcement')
self.addCleanup(CONF.clear_override, 'external_service_base_endpoint',
group='enforcement')
self.filter = external_service_filter.ExternalServiceFilter(CONF)
self.ctx = {
"is_context": True
}
self.lease = {
"is_lease": True
}
self.old_lease = {
"is_old_lease": True
}
@mock.patch("requests.post")
def test_check_create_allowed(self, post_mock):
post_mock.return_value = FakeResponse204()
self.filter.check_create(self.ctx, self.lease)
post_mock.assert_called_with(
"http://localhost/check-create",
headers={'Content-Type': 'application/json'},
data='{"context": {"is_context": true}, '
'"lease": {"is_lease": true}}')
@mock.patch("requests.post")
def test_check_create_denied(self, post_mock):
post_mock.return_value = FakeResponse403WithMessage()
self.assertRaises(ExternalServiceFilterException,
self.filter.check_create,
self.ctx, self.lease)
post_mock.assert_called_with(
"http://localhost/check-create",
headers={'Content-Type': 'application/json'},
data='{"context": {"is_context": true}, '
'"lease": {"is_lease": true}}')
@mock.patch("requests.post")
def test_check_create_failed(self, post_mock):
post_mock.return_value = FakeResponse403Empty()
self.assertRaises(ExternalServiceFilterException,
self.filter.check_create,
self.ctx, self.lease)
post_mock.assert_called_with(
"http://localhost/check-create",
headers={'Content-Type': 'application/json'},
data='{"context": {"is_context": true}, '
'"lease": {"is_lease": true}}')
@mock.patch("requests.post")
def test_check_update_allowed(self, post_mock):
post_mock.return_value = FakeResponse204()
self.filter.check_update(self.ctx, self.old_lease, self.lease)
post_mock.assert_called_with(
"http://localhost/check-update",
headers={'Content-Type': 'application/json'},
data='{"context": {"is_context": true}, '
'"current_lease": {"is_old_lease": true}, '
'"lease": {"is_lease": true}}')
@mock.patch("requests.post")
def test_check_update_denied(self, post_mock):
post_mock.return_value = FakeResponse403WithMessage()
self.assertRaises(ExternalServiceFilterException,
self.filter.check_update,
self.ctx, self.old_lease, self.lease)
post_mock.assert_called_with(
"http://localhost/check-update",
headers={'Content-Type': 'application/json'},
data='{"context": {"is_context": true}, '
'"current_lease": {"is_old_lease": true}, '
'"lease": {"is_lease": true}}')
@mock.patch("requests.post")
@mock.patch("requests.JSONDecodeError", FakeJSONDecodeError)
def test_check_update_failed(self, post_mock):
post_mock.return_value = FakeResponse403InvalidJSON()
self.assertRaises(ExternalServiceFilterException,
self.filter.check_update,
self.ctx, self.old_lease, self.lease)
post_mock.assert_called_with(
"http://localhost/check-update",
headers={'Content-Type': 'application/json'},
data='{"context": {"is_context": true}, '
'"current_lease": {"is_old_lease": true}, '
'"lease": {"is_lease": true}}')
@mock.patch("requests.post")
def test_on_end_success(self, post_mock):
post_mock.return_value = FakeResponse204()
self.filter.on_end(self.ctx, self.lease)
post_mock.assert_called_with(
"http://localhost/on-end",
headers={'Content-Type': 'application/json'},
data='{"context": {"is_context": true}, '
'"lease": {"is_lease": true}}')
@mock.patch("requests.post")
def test_on_end_failure(self, post_mock):
post_mock.return_value = FakeResponse500()
self.assertRaises(ExternalServiceFilterException,
self.filter.on_end,
self.ctx, self.lease)
post_mock.assert_called_with(
"http://localhost/on-end",
headers={'Content-Type': 'application/json'},
data='{"context": {"is_context": true}, '
'"lease": {"is_lease": true}}')

View File

@ -37,6 +37,11 @@ as follows:
..
Do note that filter config options follow filter names - the prefix is always
the snake case of the filter name (``MaxLeaseDurationFilter`` becomes
``max_lease_duration``; in this case it is special that there is nothing
beyond the prefix but there is also ``max_lease_duration_exempt_project_ids``).
MaxLeaseDurationFilter
----------------------
@ -55,17 +60,32 @@ ExternalServiceFilter
---------------------
This filter delegates the decision for each API to an external HTTP service.
The service must use token-based authentication and implement the following
endpoints for POST method:
The service must use token-based authentication, accepting (or ignoring)
the static token sent by Blazar in the ``X-Auth-Token`` header.
The following endpoints should be implemented:
* ``POST /v1/check-create``
* ``POST /v1/check-update``
* ``POST /v1/on-end``
* ``POST /check-create``
* ``POST /check-update``
* ``POST /on-end``
The exact URLs can be overridden and not all have to be used (although
we imagine a proper implementation requires at least both checks unless
lease updates are disabled in the first place).
The external service should return ``204 No Content`` if the parameters meet
defined criteria and ``403 Forbidden`` if not.
defined criteria and ``403 Forbidden`` if not. The service may send a JSON
body response with the ``403 Forbidden`` reply, including the rejection
reasoning in the field named ``message`` as in:
Example format of data the external service will receive in a request body:
.. sourcecode:: json
{
"message": "You shall not pass!"
}
An example of data the external service will receive in a request body (do note
all dates and times are encoded as strings following the ISO8601 standard that
is expected in JSON to represent dates and times):
* Request example:
@ -79,8 +99,8 @@ Example format of data the external service will receive in a request body:
"region_name": "RegionOne"
},
"current_lease": {
"start_date": "2020-05-13 00:00",
"end_time": "2020-05-14 23:59",
"start_date": "2020-05-13T00:00:00.012345+02:00",
"end_time": "2020-05-14T23:59:00.012345+02:00",
"reservations": [
{
"resource_type": "physical:host",
@ -101,8 +121,8 @@ Example format of data the external service will receive in a request body:
]
},
"lease": {
"start_date": "2020-05-13 00:00",
"end_time": "2020-05-14 23:59",
"start_date": "2020-05-13T00:00:00.012345+02:00",
"end_time": "2020-05-14T23:59:00.012345+02:00",
"reservations": [
{
"resource_type": "physical:host",
@ -130,3 +150,11 @@ Example format of data the external service will receive in a request body:
]
}
}
The ``current_lease`` field is present only in ``check-update`` requests and
describes the existing lease. In both checks the ``lease`` field describes
the new lease. In ``on-end``, the ``lease`` field describes the lease that
has just ended.
There is no guarantee on the delivery of the ``on-end`` event and it should be
considered an optimisation rather than a reliable mechanism.