Merge pull request #2454 from ryanpetrello/more-celery-cleanup

allow users to specify BROKER_URL with passwords that contain : and @

Reviewed-by: https://github.com/softwarefactory-project-zuul[bot]

awx/conf/settings.py

@@ -2,11 +2,13 @@
 from collections import namedtuple
 import contextlib
 import logging
+import re
 import sys
 import threading
 import time
 import StringIO
 import traceback
+import urllib
 
 import six
 
@@ -60,6 +62,15 @@ SETTING_CACHE_DEFAULTS = True
 __all__ = ['SettingsWrapper', 'get_settings_to_cache', 'SETTING_CACHE_NOTSET']
 
 
+def normalize_broker_url(value):
+    parts = value.rsplit('@', 1)
+    match = re.search('(amqp://[^:]+:)(.*)', parts[0])
+    if match:
+        prefix, password = match.group(1), match.group(2)
+        parts[0] = prefix + urllib.quote(password)
+    return '@'.join(parts)
+
+
 @contextlib.contextmanager
 def _ctit_db_wrapper(trans_safe=False):
     '''
@@ -444,7 +455,16 @@ class SettingsWrapper(UserSettingsHolder):
                 value = self._get_local(name)
         if value is not empty:
             return value
-        return self._get_default(name)
+        value = self._get_default(name)
+        # sometimes users specify RabbitMQ passwords that contain
+        # unescaped : and @ characters that confused urlparse, e.g.,
+        # amqp://guest:a@ns:ibl3#@localhost:5672//
+        #
+        # detect these scenarios, and automatically escape the user's
+        # password so it just works
+        if name == 'BROKER_URL':
+            value = normalize_broker_url(value)
+        return value
 
     def _set_local(self, name, value):
         field = self.registry.get_setting_field(name)

awx/main/tests/functional/api/test_settings.py

@@ -7,6 +7,7 @@ import pytest
 import os
 
 from django.conf import settings
+from kombu.utils.url import parse_url
 
 # Mock
 import mock
@@ -358,3 +359,25 @@ def test_isolated_key_flag_readonly(get, patch, delete, admin):
 
     delete(url, user=admin)
     assert settings.AWX_ISOLATED_KEY_GENERATION is True
+
+
+@pytest.mark.django_db
+def test_default_broker_url():
+    url = parse_url(settings.BROKER_URL)
+    assert url['transport'] == 'amqp'
+    assert url['hostname'] == 'rabbitmq'
+    assert url['userid'] == 'guest'
+    assert url['password'] == 'guest'
+    assert url['virtual_host'] == '/'
+
+
+@pytest.mark.django_db
+def test_broker_url_with_special_characters():
+    settings.BROKER_URL = 'amqp://guest:a@ns:ibl3#@rabbitmq:5672//'
+    url = parse_url(settings.BROKER_URL)
+    assert url['transport'] == 'amqp'
+    assert url['hostname'] == 'rabbitmq'
+    assert url['port'] == 5672
+    assert url['userid'] == 'guest'
+    assert url['password'] == 'a@ns:ibl3#'
+    assert url['virtual_host'] == '/'