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

Allow configuring a custom CA with redis tls #3451

Merged
merged 5 commits into from
Jun 7, 2023

Conversation

ewdurbin
Copy link
Contributor

@ewdurbin ewdurbin commented Jun 4, 2023

Similar to #3450, allows users with custom or third-party Certificate Authorities (Like AWS Elasticache) to fully validate TLS.

Code Changes

  • Adds a ssl_ca_certs configuration to the redis cache for setting a path to a CA certificate to validate TLS against.

Steps to Confirm

  • Configure a redis instance with a self-signed certificate or private CA signed certificate and note that TLS validates when using ssl_cert_reqs="required".

Pre-Merge Checklist

@ThomasLaPiana ThomasLaPiana self-assigned this Jun 5, 2023
@codecov
Copy link

codecov bot commented Jun 6, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01 🎉

Comparison is base (d638d66) 87.12% compared to head (6810af1) 87.13%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3451      +/-   ##
==========================================
+ Coverage   87.12%   87.13%   +0.01%     
==========================================
  Files         312      312              
  Lines       18757    18761       +4     
  Branches     2389     2390       +1     
==========================================
+ Hits        16342    16348       +6     
+ Misses       1991     1989       -2     
  Partials      424      424              
Impacted Files Coverage Δ
src/fides/api/util/cache.py 93.80% <ø> (ø)
src/fides/core/config/redis_settings.py 93.33% <100.00%> (+10.40%) ⬆️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@ThomasLaPiana ThomasLaPiana left a comment

Choose a reason for hiding this comment

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

LGTM! Great change, waiting for CI checks to pass before merging

@ThomasLaPiana
Copy link
Contributor

remaining failures are intermittent

@ThomasLaPiana ThomasLaPiana merged commit 60e8a51 into ethyca:main Jun 7, 2023
@ewdurbin
Copy link
Contributor Author

ewdurbin commented Jun 7, 2023

remaining failures are intermittent

Known intermittent or introduced by my changeset 🙈

@ewdurbin ewdurbin deleted the redis_ca_certs_option branch June 7, 2023 03:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants