Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rate limiting middleware #38

Open
wants to merge 1 commit into
base: stable/yoga-m3
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions custom-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,6 @@ uwsgi-shortmsecs
raven
-e git+https://github.com/sapcc/octavia-f5-provider-driver.git@stable/yoga-m3#egg=octavia-f5-provider-driver
git+https://github.com/sapcc/openstack-watcher-middleware.git#egg=watcher-middleware
git+https://github.com/sapcc/openstack-rate-limit-middleware.git#egg=rate-limit-middleware
git+https://github.com/sapcc/openstack-audit-middleware.git@master#egg=audit-middleware
git+https://github.com/sapcc/openstack-uwsgi-middleware.git@main#egg=uwsgi-middleware
20 changes: 19 additions & 1 deletion octavia/api/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import sys

import rate_limit as ratelimitmiddleware
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be wiser to try the import and check later if the import succeeded. This way the test-requirements.txt don't need the rate limit middleware - there is no tests anyway in octavia.

# Try using custom auditmiddleware
try:
import auditmiddleware as audit_middleware
Expand Down Expand Up @@ -86,16 +87,33 @@ def setup_app(pecan_config=None, debug=False, argv=None):
def _wrap_app(app):
"""Wraps wsgi app with additional middlewares."""

# This needs to be the first middleware (and the last in the chain)
# UWSGI needs to be the first middleware (and the last in the chain)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

has nothing to do with this PR

try:
from uwsgi_middleware import uwsgi
app = uwsgi.Uwsgi(app)
except (EnvironmentError, OSError, ImportError) as e:
LOG.debug("Could not load uwsgi middleware: %s", e)

# Inject Request ID
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

has nothing to do with this PR

app = request_id.RequestId(app)

if CONF.audit.enabled:

# rate limiting - depends on audit middleware, therefore must stand
# before it in order to appear after it in the chain
app = ratelimitmiddleware.OpenStackRateLimitMiddleware(
app,
config_file=CONF.rate_limiting.config_file,
service_type=CONF.rate_limiting.service_type,
rate_limit_by=CONF.rate_limiting.rate_limit_by,
max_sleep_time_seconds=CONF.rate_limiting.max_sleep_time_seconds,
backend_host=CONF.rate_limiting.backend_host,
backend_port=CONF.rate_limiting.backend_port,
backend_max_connections=CONF.rate_limiting.backend_max_connections,
backend_timeout_seconds=CONF.rate_limiting.backend_timeout_seconds,
)
Comment on lines 100 to +114
Copy link
Collaborator

@notandy notandy Oct 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ratelimitmiddleware has nothing to do with - and doesn't require audit_middleware, but watcher_middleware. Therefor, this needs to moved down but above the watcher middleware initialization.


# audit middleware
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nothing to do with this PR

try:
app = audit_middleware.AuditMiddleware(
app,
Expand Down
36 changes: 27 additions & 9 deletions octavia/common/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -822,6 +822,32 @@
'using tagged statsd metrics.')),
]

rate_limiting_opts = [
cfg.StrOpt('config_file', default="/etc/octavia/ratelimit.yaml",
help=_('Path to the rate limiting middleware configuration file')),
cfg.StrOpt('service_type', default="loadbalancer",
help=_('The service type according to CADF specification')),
cfg.StrOpt('rate_limit_by',
help=_('Per default rate limits are applied based on '
'`initiator_project_id`. However, this can also be se to '
'`initiator_host_address` or `target_project_id`')),
cfg.IntOpt('max_sleep_time_seconds', default=20,
help=_('The maximal time a request can be suspended in seconds. '
'Instead of immediately returning a rate limit response, '
'a request can be suspended until the specified maximum '
'duration to fit the configured rate limit. This feature '
'can be disabled by setting the max sleep time to 0 seconds.')),
cfg.StrOpt('backend_host', default="127.0.0.1",
help=_('Redis backend host for rate limiting middleware')),
cfg.PortOpt('backend_port', default=6379,
help=_('Redis backend port for rate limiting middleware')),
cfg.IntOpt('backend_max_connections', default=100,
help=_('Maximum connections for redis connection pool.')),
cfg.IntOpt('backend_timeout_seconds', default=2,
help=_('Timeout for obtaining a connection to the backend. '
'It should be >= 1 second. Skips rate limit on timeout.')),
]

driver_agent_opts = [
cfg.StrOpt('status_socket_path',
default='/var/run/octavia/status.sock',
Expand Down Expand Up @@ -882,15 +908,6 @@
help=_('The name of the service according to CADF')),
cfg.StrOpt('config_file',
help=_('Path to configuration file')),
cfg.StrOpt('statsd_host',
default='127.0.0.1',
help=_('Host of the StatsD backend')),
cfg.StrOpt('statsd_namespace',
default='openstack_watcher',
help=_('Namespace to use for metrics')),
cfg.IntOpt('statsd_port',
default=9125,
help=_('Port of the StatsD backend')),
cfg.BoolOpt('target_project_id_from_path',
default=False,
help=_('Whether to get the target project uid from the path')),
Expand Down Expand Up @@ -928,6 +945,7 @@
cfg.CONF.register_opts(neutron_opts, group='neutron')
cfg.CONF.register_opts(quota_opts, group='quotas')
cfg.CONF.register_opts(audit_opts, group='audit')
cfg.CONF.register_opts(rate_limiting_opts, group='rate_limiting')
cfg.CONF.register_opts(driver_agent_opts, group='driver_agent')

cfg.CONF.register_opts(local.certgen_opts, group='certificates')
Expand Down
3 changes: 3 additions & 0 deletions test-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,6 @@ tempest>=23.0.0 # Apache-2.0
# Required for pep8 - doc8 tests
sphinx>=2.0.0,!=2.1.0 # BSD
bashate>=0.5.1 # Apache-2.0

# Any requirements we need for CCloud
git+https://github.com/sapcc/openstack-rate-limit-middleware.git#egg=rate-limit-middleware
Comment on lines +23 to +24
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unneded if octavia can startup without the module (see earlier comment)

Loading