Skip to content

[Cache] Add TLS scheme for Redis connection #14728

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

Merged
merged 1 commit into from
Apr 7, 2021
Merged

[Cache] Add TLS scheme for Redis connection #14728

merged 1 commit into from
Apr 7, 2021

Conversation

misaert
Copy link
Contributor

@misaert misaert commented Dec 21, 2020

@OskarStark OskarStark modified the milestones: 5.3, next Jan 8, 2021
nicolas-grekas added a commit to symfony/symfony that referenced this pull request Feb 22, 2021
… (misaert)

This PR was submitted for the 5.x branch but it was merged into the 4.4 branch instead.

Discussion
----------

[Cache] Fix Redis TLS scheme `rediss` for Redis connection

| Q             | A
| ------------- | ---
| Branch?       | 5.x
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        | symfony/symfony-docs#14728

Like #35503 on Symfony Messenger, this will enable TLS support for Redis adapter.

The implementation just prefix the host with `tls://` as described here: https://github.com/phpredis/phpredis#connect-open

I don't know how to test it because I guess I need a TLS Redis in `src/Symfony/Component/Cache/Tests/Adapter/RedisAdapterTest.php`.

Commits
-------

3288897 [Cache] Fix Redis TLS scheme `rediss` for Redis connection
@nicolas-grekas
Copy link
Member

nicolas-grekas commented Feb 22, 2021

Is that correct? Isn't TLS enabled by using the rediss scheme instead?

@misaert
Copy link
Contributor Author

misaert commented Feb 22, 2021

No, it is not correct because the implementation has changed over time. I take care of changing this PR to correspond with the choosen implementation 😉

@misaert misaert changed the title [Cache] Add TLS option for Redis connection [Cache] Add TLS scheme for Redis connection Feb 22, 2021
@misaert
Copy link
Contributor Author

misaert commented Feb 22, 2021

Changed. Do I have to change the destination of the branch to 4.4 one?

@nicolas-grekas
Copy link
Member

Yes please, and rebase on 4.4 before.

@misaert misaert changed the base branch from 5.x to 4.4 February 22, 2021 19:51
@misaert
Copy link
Contributor Author

misaert commented Feb 22, 2021

Done.

@OskarStark OskarStark modified the milestones: next, 4.4 Feb 22, 2021
@OskarStark OskarStark removed the Waiting Code Merge Docs for features pending to be merged label Feb 22, 2021
wouterj added a commit that referenced this pull request Apr 7, 2021
@wouterj wouterj merged commit 5e69cc3 into symfony:4.4 Apr 7, 2021
@wouterj
Copy link
Member

wouterj commented Apr 7, 2021

Hi @misaert! I'm not sure why this PR has been open for quite a while after the code merge. All seems perfect. Thanks for taking care of updating the docs!

wouterj added a commit that referenced this pull request Apr 8, 2021
* 4.4:
  Add troubleshooting for parallel merges to maintainer guide
  Update framework.rst
  JsonResponse content updated
  [#14728] Be explicit about the double 's'
  [Messenger] fix typo
  [Messenger] Routing & Inheritance
  [Cache] Add TLS scheme for Redis connection
wouterj added a commit that referenced this pull request Apr 8, 2021
* 5.2:
  Add troubleshooting for parallel merges to maintainer guide
  Update framework.rst
  JsonResponse content updated
  Fixed table markup
  [Messenger] Add options for PostgreSQL LISTEN/NOTIFY support
  Update data_collector.rst
  [#14728] Be explicit about the double 's'
  [#14700] Minor rewording
  Update login_link.rst
  Added explaination on context in events and initial marking
  [Messenger] fix typo
  [Messenger] Routing & Inheritance
  docs(http-client): fix default retry_failed configuration example
  [Cache] Add TLS scheme for Redis connection
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants