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

[Feature] - Add TLS unauthorized support to Redis bull connections #3113

Closed

Conversation

pa-sundae
Copy link

@pa-sundae pa-sundae commented Apr 9, 2022

As Redis5 is at end of life and Redis6 introduced TLS: Today there is no support for utilizing the Redis bull connections when scaling n8n to utilize a TLS connection. Hosting Providers like Heroku have Redis5 at end of life, and are enforcing TLS with Redis6.

This code commit is specific to add a new environment variable to utilize the rejectUnauthorized key inside the RedisOptions of TLS. I wasn't sure how/what the standard maybe to pass certificates themselves, so I did not make that code change.

This for now should allow those using n8n on Heroku to utilize redis6 once they remove support for Redis5 at the end of the month.

To the maintainers -- The same changes should probably be made to any n8n flow utilized by redis to also have such an option; additionally, what would be the best way to test this? The contributing markdown file doesn't address these type of core change requests.

@CLAassistant
Copy link

CLAassistant commented Apr 9, 2022

CLA assistant check
All committers have signed the CLA.

@pa-sundae
Copy link
Author

@ivov @BHesseldieck

@Joffcom Joffcom added community Authored by a community member core Enhancement outside /nodes-base and /editor-ui labels Apr 9, 2022
@pa-sundae
Copy link
Author

Any specifics that need to occur to move this forward @Joffcom ?

@Joffcom
Copy link
Member

Joffcom commented May 18, 2022

Hey @pa-sundae,

I shall take a look at this in the morning, the internal team I have been working in have been focused on node improvements / bugs and have not really looked at any core changes.

To test an internal change it would be a case of setting up an environment that uses the change with the latest n8n version to set a baseline and make sure it is all working (or not if it is around handling certificate trusts) then try the change to make sure it is still working.

Looking at the change quickly it looks ok, I am wondering if it needs all those lines or if a ternary would do the job but it does the same thing.

@Joffcom
Copy link
Member

Joffcom commented May 19, 2022

Hey @pa-sundae,

The good news is internally we think the change makes sense and we would want to support the latest verison of Redis for the queue mode where possible. The downside is the chap that knows a lot about our queue setup and would likely review the change is off at the moment so it will be a bit longer until we have had a proper chance to look at it.

@pa-sundae
Copy link
Author

Thank you for the update.

@MrCobos
Copy link

MrCobos commented Dec 15, 2022

Any progress on this? I am interested in using queue mode on AWS but redis AUTH can only be implemented with in-transit encryption (TLS).

I have seen this related feature request, which provides more code to implement the auth system with your certificates, thus complementing this pull request.

@al-bglhk
Copy link

Checking if there's action on this please? Modern redis servers forces TLS, and also makes sense.

@Joffcom
Copy link
Member

Joffcom commented Mar 25, 2023

I will double check on Monday.

@al-bglhk
Copy link

I will double check on Monday.

Any update please? Or anything I can help?

I have created a PR if it helps the progress.
#5784

@Joffcom
Copy link
Member

Joffcom commented Mar 29, 2023

Hey @al-bglhk,

No updates yet, there are a few PRs for the Redis options in queue mode so it could be that we handle all of them at the same time.

@Joffcom
Copy link
Member

Joffcom commented Jul 25, 2023

This is being tracked internally as ENG-85

@Joffcom Joffcom added the in linear Issue or PR has been created in Linear for internal review label Jul 25, 2023
@leandrosilvaferreira
Copy link

Any updates about using redis with tls connections ? AWS Elasticache Redis now require TLS connections :(

image

@Joffcom
Copy link
Member

Joffcom commented Jan 24, 2024

Hey @leandrosilvaferreira,

It looks like we added support for TLS on Redis outside of this with QUEUE_BULL_REDIS_TLS, Our implementation does mean that you will need to have valid certificates in use but I suspect that will ok if it is redis from a larger provider.

@pa-sundae I know you made this PR a while ago but it looks like we missed this when adding support recently as part of a larger project so for now I am going to close this PR, If we decide to add support for invalid certificates though I will try and makes sure we use this as a base.

@Joffcom Joffcom closed this Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Authored by a community member core Enhancement outside /nodes-base and /editor-ui in linear Issue or PR has been created in Linear for internal review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants