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
Unable to create SSL broker/backend connection to redis #5371
Comments
Shouldn't it be The linked documentation |
@michael-k thanks for pointing out that the dictionary parameter names for If I take a clean install of celery 4.3.0rc2 and then create a app = Celery('app', broker=broker_url, backend=broker_url,
broker_use_ssl = {
'ssl_keyfile': key_file, 'ssl_certfile': cert_file,
'ssl_ca_certs': ca_file,
'ssl_cert_reqs': ssl.CERT_REQUIRED
},
redis_backend_use_ssl = {
'ssl_keyfile': key_file, 'ssl_certfile': cert_file,
'ssl_ca_certs': ca_file,
'ssl_cert_reqs': ssl.CERT_REQUIRED
}) and then try and initiate the connection to the redis server, I still get the error: ValueError: A rediss:// URL must have parameter ssl_cert_reqs be CERT_REQUIRED, CERT_OPTIONAL, or CERT_NONE There's also an error from kombu (kombu/utils/objects.py:42) that appears prior to the other error: KeyError: 'backend' but I'm assuming this is probably because the backend object hasn't been instantiated due to the certificate parameter error. I'm investigating and will aim to provide some further detail on this. |
I realised that if I switch to using purely URL based configuration for the SSL options (as shown in the box just above the redis_backend_use_ssl details in the docs), then the connection works. It looks as though the I've added a suggested modification to In the redis backend code, where the I guess an alternative option would be to remove support for the |
@jcohen02 at least for the Redis backend, you should be passing a string, instead of the Would you mind trying this: app = Celery('app', broker=broker_url, backend=broker_url,
broker_use_ssl = {
'ssl_keyfile': key_file, 'ssl_certfile': cert_file,
'ssl_ca_certs': ca_file,
'ssl_cert_reqs': 'CERT_REQUIRED'
},
redis_backend_use_ssl = {
'ssl_keyfile': key_file, 'ssl_certfile': cert_file,
'ssl_ca_certs': ca_file,
'ssl_cert_reqs': 'CERT_REQUIRED'
}) Let us know if this works! |
@georgepsarakis, thanks for the suggestion. Testing this against ValueError: A rediss:// URL must have parameter ssl_cert_reqs be CERT_REQUIRED, CERT_OPTIONAL, or CERT_NONE The only thing that works for me against query_string_params = {'ssl_cert_reqs':'CERT_REQUIRED', 'ssl_keyfile':key_file,
'ssl_certfile':cert_file, 'ssl_ca_certs':ca_file }
broker_url = ('rediss://localhost:6380/0?%s' %
urllib.parse.urlencode(query_string_params))
app = Celery('app', broker=broker_url, backend=broker_url) In reference to passing a string instead of the If you have access to an SSL-enabled Redis deployment to test on, the issue I'm seeing should be reproducible with the following code: from celery import Celery
key_file = '/tmp/keyfile.key'
cert_file = '/tmp/certfile.crt'
ca_file = '/tmp/CAtmp.pem'
broker_url = 'rediss://localhost:6380'
app = Celery('app', broker=broker_url, backend=broker_url,
broker_use_ssl = {
'ssl_keyfile': key_file, 'ssl_certfile': cert_file,
'ssl_ca_certs': ca_file,
'ssl_cert_reqs': 'CERT_REQUIRED'
},
redis_backend_use_ssl = {
'ssl_keyfile': key_file, 'ssl_certfile': cert_file,
'ssl_ca_certs': ca_file,
'ssl_cert_reqs': 'CERT_REQUIRED'
})
app.connection().connect()
app.send_task('test') |
Hmm, this might sound weird, but try celery/t/unit/backends/test_redis.py Lines 273 to 299 in eef03a3
|
Thanks @lithammer, I've given this a go. With the current ConnectionError: Error while reading from socket: (54, 'Connection reset by peer') I guess this suggests it's trying to make a standard connection to an SSL endpoint and the host is disconnecting? However, with my modified code in jcohen02/celery@45cf3e6, it doesn't seem to make a difference whether the URL scheme is specified as I'm still of the impression that the Thanks for the help with this and hope this info is useful. |
Wonderful. This needs to be resolved before 4.3 GA. |
I think that what is happening is that we only support SSL configuration from the URI and I haven't checked though. |
@xirdneh @jcohen02 the following patch seems to fix the issue: diff --git a/celery/backends/redis.py b/celery/backends/redis.py
index 9954498..fe55bae 100644
--- a/celery/backends/redis.py
+++ b/celery/backends/redis.py
@@ -234,11 +234,13 @@ class RedisBackend(BaseKeyValueStoreBackend, AsyncBackendMixin):
connparams['connection_class'] = redis.SSLConnection
# The following parameters, if present in the URL, are encoded. We
# must add the decoded values to connparams.
- for ssl_setting in ['ssl_ca_certs', 'ssl_certfile', 'ssl_keyfile']:
+ for ssl_setting in ['ssl_ca_certs', 'ssl_certfile',
+ 'ssl_keyfile', 'ssl_cert_reqs']:
ssl_val = query.pop(ssl_setting, None)
if ssl_val:
connparams[ssl_setting] = unquote(ssl_val)
- ssl_cert_reqs = query.pop('ssl_cert_reqs', 'MISSING')
+
+ ssl_cert_reqs = connparams.pop('ssl_cert_reqs', 'MISSING')
if ssl_cert_reqs == 'CERT_REQUIRED':
connparams['ssl_cert_reqs'] = CERT_REQUIRED
elif ssl_cert_reqs == 'CERT_OPTIONAL': The problem is that we relied on the connection URL query parameters, instead of the more broad dictionary Additionally, configuration must also be passed through app.conf.redis_backend_use_ssl = {
'ssl_keyfile': key_file, 'ssl_certfile': cert_file,
'ssl_ca_certs': ca_file,
'ssl_cert_reqs': 'CERT_REQUIRED'
} |
Thanks @georgepsarakis. I included support in my modification so that either a string or the I suppose this adds complexity and maybe the approach of sticking with a string value for |
@jcohen02 although having a unified type between URL query parameters and dictionary configuration could be more clear (and less complicated for some users), I think your modification is quite valid. |
Can anyone issue a PR and extend the tests to cover these cases? |
@thedrow I'm happy to do that. |
- Add redis SSL parameters to config object - Refactored checking of ssl_cert_reqs param so that it is carried out regardless of the url param being provided.
@jcohen02 Let me know when the PR is ready. |
- Add redis SSL parameters to config object - Refactored checking of ssl_cert_reqs param so that it is carried out regardless of the url param being provided.
* Fix handling of SSL configuration parameters not provided via URL. (issue #5371) * Fix and updated tests for issue #5371. - Add redis SSL parameters to config object - Refactored checking of ssl_cert_reqs param so that it is carried out regardless of the url param being provided. * Resolved code styling issues * Addressing code review comments for #5371 * Added skip unless module redis to socket url test - resolves Win build error. * Mods to ssl_cert_reqs parameter handling. * Modified equlity check to use 'is' as per review comment in #5395.
Must I explicitly provide the cert and key files if I am using rediss (redis + ssl)? I have run a test and was able to set and get keys without any extra configuration using PyPi's redis [2019-09-04 22:04:42,950: WARNING/MainProcess] : Still getting this issue though Thanks in advance @georgepsarakis @jcohen02 |
@BeOleg yes these options are mandatory. Please check the configuration documentation. |
Hello, Has anyone done this? I need some guidance on how to setup and reference the key, cert, and ca_file. key_file = '/tmp/keyfile.key' |
Hi @jgarzautexas, how are you creating your connection to redis? Are you using redis for both the broker and the backend? I have to say that I've not worked with this for a while but as a test, if I simply create a connection to an SSL redis endpoint without specifying any SSL parameters, I can reproduce the error you're seeing, e.g.
However, if I add the various SSL configuration options, as follows, I get a successful SSL connection to redis:
Hope this helps. It looks like you may just be missing the |
Thanks @jcohen02. For my staging app my REDIS_URL is If I configure with Appreciate your help. |
No problem @jgarzautexas, unfortunately I'm not familiar with deployment on Heroku so I'm really not sure how you would handle the keys - I think I see the issue but not sure how you would handle this. |
Please move this discussion elsewhere. The mailing list is a good place to have these discussions. |
Did you ever figure this out? I'm running into the same issues on heroku. |
@AustinStehling, I haven't found a solution to this myself. I've moved this discussion to the mailing list, if you'd like to follow-up there with some further information on the issue, hopefully someone on the list can provide assistance. |
If anyone's still trying to sort this one out, I manage to solve it. I've got a Django project with a
|
* Fix handling of SSL configuration parameters not provided via URL. (issue celery#5371) * Fix and updated tests for issue celery#5371. - Add redis SSL parameters to config object - Refactored checking of ssl_cert_reqs param so that it is carried out regardless of the url param being provided. * Resolved code styling issues * Addressing code review comments for celery#5371 * Added skip unless module redis to socket url test - resolves Win build error. * Mods to ssl_cert_reqs parameter handling. * Modified equlity check to use 'is' as per review comment in celery#5395.
Hi can anyone help me on what changes i need to make in my code to get a connection between celery and redis. I am trying to deploy on heroku but keep getting the SSL certification error as above. I am using heroku redis 6. Below is the code i am using. I am not sure how to incorporate the 'r' variable wiith the celery inputs. Any help would be highly appreciated. Thankyou
|
Add the certs to your celery instance. That got it working for me.
|
I'm having some issues making an SSL/TLS connection to redis for the Celery broker and backend.
I'm using:
Celery: 4.3.0rc2 (master)
Python: 3.6
OS: Mac OS X.
Related dependency versions: kombu (4.4.0), redis-py (3.2.0)
Celery works fine using a non-SSL connection to redis. My redis/SSL setup uses redis behind stunnel and I can successfully connect to and use this deployment via py-redis directly and via other libraries.
It looks like this is related to #3830, however that issue seemed to have been resolved by adding the
broker_use_ssl
option and adding this option doesn't appear to be having any effect here.To setup SSL, I'm adding the
broker_use_ssl
attribute to configuration parameters as described in the docs. I'm also adding the same set of parameters for theredis_backend_use_ssl
option. I notice there is some information here about adding the parameters in the query string but I've opted to stick with providing the dict of parameters.When I attempt to make and use the connection as shown above, I get an error:
It looks like the
rediss
URL scheme is being picked up but the other parameters are not. Apologies if I'm missing something in the documentation or elsewhere.Some investigation suggests that the
broker_use_ssl
andredis_backend_use_ssl
options are not being carried through to the connection settings and, indeed, looking at the __init__ function for the Celery object, I can't see that these are being used anywhere. Stepping through the code, they don't seem to be present by the time theconnection
function is reached andssl
in the input parameters to theconnection
function is set toNone
.As a test, I've tried making a couple of additions to the Celery
__init__
function to add thebroker_use_ssl
andredis_backed_use_ssl
to the configuration:This adds the provided options to the Celery
conf
. It flags up another issue which is that the SSL parameters used for the backend connection use different names to those used for the broker connection - the documentation says the values are the same asbroker_use_ssl
(although the correct values are shown in the example of using query string parameters). I assume this is one of the things being handled under issue #4812.A further point that I'm unclear about is that in the
_connection
function, there is a call to get thebroker_use_ssl
parametersssl=self.either('broker_use_ssl', ssl)
and this returnsNone
. However, if I manually create an app object and then calleither
, it returns thebroker_use_ssl
parameters correctly (parameters as per example above):So, I'm unclear if I'm missing something and taking completely the wrong approach here or whether there are some issues with the SSL implementation in 4.3.0rc2.
Thanks.
The text was updated successfully, but these errors were encountered: