diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml new file mode 100644 index 0000000..880bfe1 --- /dev/null +++ b/.github/workflows/tests.yaml @@ -0,0 +1,38 @@ + # This workflow will install Python dependencies, run tests and lint with a variety of Python versions +# For more information see: https://help.github.com/actions/language-and-framework-guides/using-python-with-github-actions + +name: Functional Tests + +on: + push: + branches: [ master ] + pull_request: + branches: [ master ] + +env: + VIRTUALENV_PIP: "20.2.3" + +jobs: + build: + + #runs-on: ubuntu-latest + runs-on: ubuntu-20.04 + strategy: + fail-fast: false + matrix: + python: [3.8.14] + tox-env: [pep8, functional] + env: + TOXENV: ${{ matrix.tox-env }} + steps: + - uses: actions/checkout@v2 + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v2 + with: + python-version: ${{ matrix.python }} + - name: Install Tox and any other packages + run: | + pip install tox + - name: Running Tox + run: tox + diff --git a/rate_limit/__init__.py b/rate_limit/__init__.py index 8eed2dc..c0998a4 100644 --- a/rate_limit/__init__.py +++ b/rate_limit/__init__.py @@ -12,7 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. -from .rate_limit import OpenStackRateLimitMiddleware +from .ratelimit import OpenStackRateLimitMiddleware def main(global_config, **settings): diff --git a/rate_limit/common.py b/rate_limit/common.py index 2ba5eaf..35d863f 100644 --- a/rate_limit/common.py +++ b/rate_limit/common.py @@ -33,9 +33,8 @@ class Constants(object): - """ - Common constants used in various places. - """ + """Common constants used in various places.""" + ratelimit_response = 'ratelimit_response' blacklist_response = 'blacklist_response' max_sleep_time_seconds = 'max_sleep_time_seconds' diff --git a/rate_limit/log.py b/rate_limit/log.py index 7fa288b..c5ecb78 100644 --- a/rate_limit/log.py +++ b/rate_limit/log.py @@ -21,10 +21,8 @@ class Logger(object): - """ - Logger that attempts to log and ignores any error. + """Logger that attempts to log and ignores any error.""" - """ def __init__(self, name, product_name='rate_limit'): self.__logger = logging.getLogger(name) try: diff --git a/rate_limit/provider.py b/rate_limit/provider.py index 46e9d2c..e7f499b 100644 --- a/rate_limit/provider.py +++ b/rate_limit/provider.py @@ -120,7 +120,6 @@ def _get_wildcard_ratelimits(self, ratelimits, target_type_uri): :param target_type_uri: the target type URI of the request :return: target type uri ratelimits if exists """ - ttu_ratelimits = [] pattern_list = [ lr_key for lr_key in ratelimits @@ -134,14 +133,13 @@ def _get_wildcard_ratelimits(self, ratelimits, target_type_uri): def _match(self, uri, pattern_list): """ - Check if a URI matches to one of the patterns + Check if a URI matches to one of the patterns. :param uri: URI to check if it matches to one of the patterns :param pattern_list : patterns to match against the URI :return: True if path matches a pattern of the list and pattern as key for self.local_ratelimits. """ - for pattern in pattern_list: if uri.startswith(pattern[:-1]): return True, pattern diff --git a/rate_limit/rate_limit.py b/rate_limit/ratelimit.py similarity index 98% rename from rate_limit/rate_limit.py rename to rate_limit/ratelimit.py index 8d057a6..1cbf7df 100644 --- a/rate_limit/rate_limit.py +++ b/rate_limit/ratelimit.py @@ -16,13 +16,13 @@ from datadog.dogstatsd import DogStatsd -from . import backend as rate_limit_backend -from . import common -from . import errors -from . import provider -from . import response -from . import units -from . import log +from rate_limit import backend as rate_limit_backend +from rate_limit import common +from rate_limit import errors +from rate_limit import provider +from rate_limit import response +from rate_limit import units +from rate_limit import log class OpenStackRateLimitMiddleware(object): @@ -146,7 +146,7 @@ def __init__(self, app, **conf): self.logger.info("OpenStack Rate Limit Middleware ready for requests.") def _setup_response(self): - """Setup configurable RateLimitExceededResponse and BlacklistResponse.""" + """Set up configurable RateLimitExceededResponse and BlacklistResponse.""" # Default responses. ratelimit_response = response.RateLimitExceededResponse() blacklist_response = response.BlacklistResponse() @@ -185,7 +185,7 @@ def _setup_response(self): self.blacklist_response = blacklist_response def __setup_limes_ratelimit_provider(self): - """Setup Limes as provider for rate limits. If not successful fallback to configuration file.""" + """Set up Limes as provider for rate limits. If not successful fallback to configuration file.""" try: limes_ratelimit_provider = provider.LimesRateLimitProvider( service_type=self.service_type, diff --git a/rate_limit/tests/fake.py b/rate_limit/tests/fake.py index 4ec5370..ac92dc5 100644 --- a/rate_limit/tests/fake.py +++ b/rate_limit/tests/fake.py @@ -36,7 +36,7 @@ def incr(self, key, delta=1, time=0): def decr(self, key, delta=1, time=0): return self.incr(key, delta=-delta, time=time) - def delete(self,key): + def delete(self, key): try: del self.store[key] except KeyError: diff --git a/rate_limit/tests/test_actiongroups.py b/rate_limit/tests/test_actiongroups.py index 0b1d788..a4fa90c 100644 --- a/rate_limit/tests/test_actiongroups.py +++ b/rate_limit/tests/test_actiongroups.py @@ -15,7 +15,7 @@ import unittest import os -from rate_limit.rate_limit import OpenStackRateLimitMiddleware +from rate_limit.ratelimit import OpenStackRateLimitMiddleware from . import fake @@ -42,19 +42,18 @@ def test_groups(self): self.assertIsNotNone( rl_groups, "expected rate limit groups to be '{0}' but got '{1}'".format( -""" -groups: - write: - - update - - delete - - update/* - - delete/os-* - - read: - - read - - read/list - - read/*/list -""", + """ + groups: + write: + - update + - delete + - update/* + - delete/os-* + read: + - read + - read/list + - read/*/list + """, rl_groups ) ) @@ -95,7 +94,7 @@ def test_mapping(self): }, { 'action': 'read/rules/list', - 'expected':'read/rules/list', + 'expected': 'read/rules/list', }, ] diff --git a/rate_limit/tests/test_limesratelimitprovider.py b/rate_limit/tests/test_limesratelimitprovider.py index 4d9abb1..52cfd1a 100644 --- a/rate_limit/tests/test_limesratelimitprovider.py +++ b/rate_limit/tests/test_limesratelimitprovider.py @@ -2,7 +2,7 @@ import os import json -from rate_limit.rate_limit import OpenStackRateLimitMiddleware, provider +from rate_limit import OpenStackRateLimitMiddleware, provider from . import fake WORKDIR = os.path.dirname(os.path.realpath(__file__)) diff --git a/rate_limit/tests/test_middleware.py b/rate_limit/tests/test_middleware.py index 09c33f6..af84dfa 100644 --- a/rate_limit/tests/test_middleware.py +++ b/rate_limit/tests/test_middleware.py @@ -13,10 +13,11 @@ # under the License. import os -import time import unittest -from rate_limit.rate_limit import OpenStackRateLimitMiddleware +from unittest.mock import patch, DEFAULT + +from rate_limit.ratelimit import OpenStackRateLimitMiddleware from rate_limit.response import BlacklistResponse from rate_limit.response import RateLimitExceededResponse from . import fake @@ -138,102 +139,94 @@ def test_get_rate_limit(self): "rate limit for '{0} {1}' should be '{2}' but got '{3}'".format(action, target_type_uri, expected_ratelimit, rate_limit) ) - def test_is_ratelimited_swift_local_container_update(self): + @patch.multiple('pyredis.Pool', evalsha=DEFAULT, script_exists=DEFAULT) + def test_is_ratelimited_swift_local_container_update(self, evalsha, script_exists): scope = '123456' action = 'update' target_type_uri = 'account/container' + retry_after = 58 + remaining = 0 + + script_exists.return_value = ['True'] + evalsha.return_value = [remaining, retry_after] + # The current configuration as per /fixtures/swift.yaml allows 2r/m for target type URI account/container and action update. # Thus the first 2 requests should not be rate limited but the 3rd one. - expected = [ - # 1st requests not rate limited. - None, - # 2nd request also not rate limited. - None, - # 3rd request should be a rate limit response - RateLimitExceededResponse( - status='498 Rate Limited', - body='Rate Limit Exceeded', - headerlist=[('X-Retry-After', 58), ('X-RateLimit-Retry-After', 58), - ('X-RateLimit-Limit', '2r/m'), ('X-RateLimit-Remaining', 0)] - ) - ] + expected = RateLimitExceededResponse( + status='498 Rate Limited', + body='Rate Limit Exceeded', + headerlist=[('X-Retry-After', 58), + ('X-RateLimit-Retry-After', retry_after), + ('X-RateLimit-Limit', '2r/m'), + ('X-RateLimit-Remaining', remaining)] + ) - for i in range(len(expected)): - result = self.app._rate_limit(scope=scope, action=action, target_type_uri=target_type_uri) - time.sleep(1) - is_equal, msg = response_equal(expected[i], result) - self.assertTrue(is_equal, "test #{0} failed: {1}".format(i, msg)) + result = self.app._rate_limit(scope=scope, action=action, target_type_uri=target_type_uri) + is_equal, msg = response_equal(expected, result) + self.assertTrue(is_equal, "test failed: {0}".format(msg)) - def test_is_ratelimited_swift_local_wildcard_update(self): + @patch.multiple('pyredis.Pool', evalsha=DEFAULT, script_exists=DEFAULT) + def test_is_ratelimited_swift_local_wildcard_update(self, evalsha, script_exists): scope = '123456' action = 'update' target_type_uri = 'account/container/foo_object' + retry_after = 58 + remaining = 0 + + script_exists.return_value = ['True'] + evalsha.return_value = [remaining, retry_after] + # The current configuration as per /fixtures/swift.yaml allows 2r/m # for target type URI account/container and action update # which is targeted by wildcard pattern account/* # Thus the first 2 requests should not be rate limited but the 3rd one. - expected = [ - # 1st requests not rate limited. - None, - # 2nd request also not rate limited. - None, - # 3rd request should be a rate limit response - RateLimitExceededResponse( - status='498 Rate Limited', - body='Rate Limit Exceeded', - headerlist=[ - ('X-Retry-After', 58), - ('X-RateLimit-Retry-After', 58), - ('X-RateLimit-Limit', '2r/m'), - ('X-RateLimit-Remaining', 0), - ] - ) - ] + expected = RateLimitExceededResponse( + status='498 Rate Limited', + body='Rate Limit Exceeded', + headerlist=[ + ('X-Retry-After', retry_after), + ('X-RateLimit-Retry-After', retry_after), + ('X-RateLimit-Limit', '2r/m'), + ('X-RateLimit-Remaining', remaining), + ] + ) - for i in range(len(expected)): - result = self.app._rate_limit(scope=scope, action=action, target_type_uri=target_type_uri) - time.sleep(1) - is_equal, msg = response_equal(expected[i], result) - self.assertTrue(is_equal, "test #{0} failed: {1}".format(i, msg)) + result = self.app._rate_limit(scope=scope, action=action, target_type_uri=target_type_uri) + is_equal, msg = response_equal(expected, result) + self.assertTrue(is_equal, "test failed: {0}".format(msg)) - def test_is_ratelimited_swift_local_after_wildcard_update(self): + @patch.multiple('pyredis.Pool', evalsha=DEFAULT, script_exists=DEFAULT) + def test_is_ratelimited_swift_local_after_wildcard_update(self, evalsha, script_exists): scope = '123456' action = 'update' target_type_uri = 'account/container/foo_object/something/else' + retry_after = 58 + remaining = 0 + + script_exists.return_value = ['True'] + evalsha.return_value = [remaining, retry_after] + # The current configuration as per /fixtures/swift.yaml allows 4r/m # for target type URI account/container/foo_object/something/else # and action update which is goes after wildcard pattern account/* # Thus the first 4 requests should not be rate limited but the 5rd one. - expected = [ - # 1st requests not rate limited. - None, - # 2nd request also not rate limited. - None, - # 3rd request also not rate limited. - None, - # 4th request also not rate limited. - None, - # 5th request should be a rate limit response - RateLimitExceededResponse( - status='498 Rate Limited', - body='Rate Limit Exceeded', - headerlist=[ - ('X-Retry-After', 58), - ('X-RateLimit-Retry-After', 58), - ('X-RateLimit-Limit', '2r/m'), - ('X-RateLimit-Remaining', 0), - ] - ) - ] + expected = RateLimitExceededResponse( + status='498 Rate Limited', + body='Rate Limit Exceeded', + headerlist=[ + ('X-Retry-After', 58), + ('X-RateLimit-Retry-After', retry_after), + ('X-RateLimit-Limit', '4r/m'), + ('X-RateLimit-Remaining', remaining), + ] + ) - for i in range(len(expected)): - result = self.app._rate_limit(scope=scope, action=action, target_type_uri=target_type_uri) - time.sleep(1) - is_equal, msg = response_equal(expected[i], result) - self.assertTrue(is_equal, "test #{0} failed: {1}".format(i, msg)) + result = self.app._rate_limit(scope=scope, action=action, target_type_uri=target_type_uri) + is_equal, msg = response_equal(expected, result) + self.assertTrue(is_equal, "test failed: {0}".format(msg)) def response_equal(expected, got): @@ -248,14 +241,14 @@ def response_equal(expected, got): return False, "expected status '{0}' but got '{1}'".format(expected.status, got.status) if expected.has_body and expected.body != got.body: - return False, "expected body '{0}' but got '{1}'".format(expected.body, got.body) + return False, "expected body '{0}' but got '{1}'".format(expected.body, got.body) if not expected.has_body and expected.json_body != got.json_body: - return False, "expected json body '{0}' but got '{1}'".format(expected.json_body, got.json_body) + return False, "expected json body '{0}' but got '{1}'".format(expected.json_body, got.json_body) return True, "items are equal" - if type(expected) != type(got): + if type(expected) is not type(got): return False, "expected type {0} but got type {1}".format(type(expected), type(got)) # Compare arguments if neither RateLimitResponse nor BlacklistResponse. diff --git a/rate_limit/tests/test_parse_config.py b/rate_limit/tests/test_parse_config.py index 0fa5a41..e7d9e1a 100644 --- a/rate_limit/tests/test_parse_config.py +++ b/rate_limit/tests/test_parse_config.py @@ -72,16 +72,16 @@ def test_load_swift_config(self): def test_parse_and_convert_to_per_seconds(self): stimuli = [ { - 'in': '5r/s', + 'in': '5r/s', 'expected': 5 }, { 'in': '1r/m', - 'expected': round(1/60.0, 4) + 'expected': round(1 / 60.0, 4) }, { 'in': '10r/d', - 'expected': round(1/8640.0,4) + 'expected': round(1 / 8640.0, 4) }, { 'in': '0.5r/s', diff --git a/rate_limit/tests/test_response.py b/rate_limit/tests/test_response.py index 86d4a4a..f31b2ba 100644 --- a/rate_limit/tests/test_response.py +++ b/rate_limit/tests/test_response.py @@ -1,6 +1,5 @@ import json import os -import six import unittest @@ -23,7 +22,8 @@ def test_default_ratelimitexceededresponse_json(self): self.assertEqual( ratelimit_response.content_type, common.Constants.content_type_json, - "expected response content type to be equal. want '{0}' but got '{1}'".format(common.Constants.content_type_json, ratelimit_response.content_type) + "expected response content type to be equal. want '{0}' but got '{1}'".format( + common.Constants.content_type_json, ratelimit_response.content_type) ) actual_body = sorted(json.loads(ratelimit_response.json_body)) @@ -39,7 +39,8 @@ def test_default_ratelimitexceededresponse_json(self): def test_custom_ratelimitexceededresponse_html(self): conf = common.load_config(SWIFTCONFIGPATH) - status, status_code, headers, body, json_body = response.response_parameters_from_config(conf.get(common.Constants.ratelimit_response)) + status, status_code, headers, body, json_body = response.response_parameters_from_config( + conf.get(common.Constants.ratelimit_response)) ratelimit_response = response.RateLimitExceededResponse( status=status, @@ -105,16 +106,19 @@ def test_default_blacklistresponse(self): .format(common.Constants.content_type_json, blacklist_response.content_type) ) - expected_json_body = json.dumps({"error": {"status": "497 Blacklisted", "message": "You have been blacklisted"}}, sort_keys=True) + expected_json_body = json.dumps( + {"error": {"status": "497 Blacklisted", "message": "You have been blacklisted"}}, sort_keys=True) self.assertEqual( blacklist_response.json_body, expected_json_body, - "expected blacklist response body to be '{0}' but got '{1}'".format(expected_json_body, blacklist_response.json_body) + "expected blacklist response body to be '{0}' but got '{1}'".format(expected_json_body, + blacklist_response.json_body) ) def test_custom_blacklistresponse_json(self): conf = common.load_config(SWIFTCONFIGPATH) - status, status_code, headers, body, json_body = response.response_parameters_from_config(conf.get(common.Constants.blacklist_response)) + status, status_code, headers, body, json_body = response.response_parameters_from_config( + conf.get(common.Constants.blacklist_response)) blacklist_response = response.BlacklistResponse( status=status, @@ -144,7 +148,11 @@ def test_custom_blacklistresponse_json(self): .format(common.Constants.content_type_json, blacklist_response.content_type) ) - expected_json_body = json.dumps({"error": {"status": "497 Blacklisted", "message": "You have been blacklisted. Please contact and administrator."}}, sort_keys=True) + expected_json_body = json.dumps( + {"error": {"status": "497 Blacklisted", + "message": "You have been blacklisted. Please contact and administrator."}}, + sort_keys=True) + self.assertEqual( blacklist_response.json_body, expected_json_body, diff --git a/rate_limit/tests/test_units.py b/rate_limit/tests/test_units.py index 06a9b88..7067cfe 100644 --- a/rate_limit/tests/test_units.py +++ b/rate_limit/tests/test_units.py @@ -35,7 +35,7 @@ def test_parse_sliding_window_rate_limit(self): }, { 'input': '100r/d', - 'expected': (100.0, 24*3600.0) + 'expected': (100.0, 24 * 3600.0) }, { 'input': '5r/2m', diff --git a/rate_limit/utils.py b/rate_limit/utils.py index c231af2..76f62db 100644 --- a/rate_limit/utils.py +++ b/rate_limit/utils.py @@ -11,7 +11,7 @@ def str_if_bytes(value): # NOTE: This function was copied from # https://github.com/andymccurdy/redis-py/blob/master/redis/client.py#L114 def parse_info(response): - "Parse the result of Redis's INFO command into a Python dict" + """Parse the result of Redis's INFO command into a Python dict.""" info = {} response = str_if_bytes(response) diff --git a/test-requirements.txt b/test-requirements.txt index df4b868..d608750 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,4 +1,4 @@ -flake8-docstrings==0.2.1.post1 # MIT +flake8-docstrings==1.7.0 # MIT fixtures>=3.0.0 # Apache-2.0/BSD mock>=2.0.0 # BSD @@ -8,3 +8,4 @@ testrepository>=0.0.18 # Apache-2.0/BSD testtools>=1.4.0 # MIT os-testr>=0.8.0 # Apache-2.0 WebTest>=2.0 # MIT +stestr>=1.0.0 # Apache-2.0 diff --git a/tox.ini b/tox.ini index 7b63590..facac6f 100644 --- a/tox.ini +++ b/tox.ini @@ -2,11 +2,14 @@ minversion = 2.0 # avoid sdist skipsdist = True -envlist = py35,py36,py37,pep8 +envlist = py37,py38,pep8 [testenv] usedevelop = True -install_command = {toxinidir}/tools/tox_install.sh {env:UPPER_CONSTRAINTS_FILE:https://raw.githubusercontent.com/sapcc/requirements/stable/rocky-m3/upper-constraints.txt} {opts} {packages} +deps = + -c{env:TOX_CONSTRAINTS_FILE:https://raw.githubusercontent.com/sapcc/requirements/stable/zed-m3/upper-constraints.txt} + -r{toxinidir}/requirements.txt + -r{toxinidir}/test-requirements.txt setenv = VIRTUAL_ENV={envdir} BRANCH_NAME=master CLIENT_NAME=rate-limit @@ -15,20 +18,25 @@ setenv = VIRTUAL_ENV={envdir} OS_TEST_TIMEOUT=60 TESTS_DIR=./rate_limit/tests/ -deps = -r{toxinidir}/requirements.txt - -r{toxinidir}/test-requirements.txt +[testenv:functional] +setenv = + {[testenv]setenv} + OS_TEST_PATH=./rate_limit/tests commands = stestr run {posargs} [testenv:pep8] +allowlist_externals = flake8 +deps = + {[testenv]deps} commands = flake8 [testenv:venv] commands = {posargs} [flake8] -ignore = D100,D101,D102,D103,D104,D203,D205,W605 +ignore = D100,D101,D102,D103,D104,D107,D203,D205,W605 show-source = True -exclude = .venv,.tox,dist,doc,*egg,build,test* +exclude = .git,__pycache__,docs/source/conf.py,old,build,dist,*.egg,.tox,venv,.venv max-line-length = 140 [testenv:docs]