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

Set sslmode to prefer for SSH connections #3685

Merged
merged 5 commits into from
Jun 28, 2023

Conversation

SteveDMurphy
Copy link
Contributor

@SteveDMurphy SteveDMurphy commented Jun 26, 2023

Closes #3680

Description Of Changes

  • default connection arguments for Redshift via SSH to only prefer SSL

Code Changes

  • Specify sslmode=prefer for SSH connections by default

Steps to Confirm

  • Tested in with a customer implementation, thinking about alternatvies for testing still

Pre-Merge Checklist

@cypress
Copy link

cypress bot commented Jun 26, 2023

Passing run #2919 ↗︎

0 4 0 0 Flakiness 0
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.

Details:

Merge e22ef7c into 243f6e2...
Project: fides Commit: 207cce9a5a ℹ️
Status: Passed Duration: 00:44 💡
Started: Jun 27, 2023 11:42 AM Ended: Jun 27, 2023 11:43 AM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@codecov
Copy link

codecov bot commented Jun 26, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.01 ⚠️

Comparison is base (243f6e2) 87.10% compared to head (e22ef7c) 87.09%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3685      +/-   ##
==========================================
- Coverage   87.10%   87.09%   -0.01%     
==========================================
  Files         310      310              
  Lines       19031    19033       +2     
  Branches     2437     2437              
==========================================
  Hits        16577    16577              
- Misses       2026     2028       +2     
  Partials      428      428              
Impacted Files Coverage Δ
src/fides/api/service/connectors/sql_connector.py 65.56% <0.00%> (-0.49%) ⬇️

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

@SteveDMurphy SteveDMurphy self-assigned this Jun 27, 2023
@SteveDMurphy SteveDMurphy added the run unsafe ci checks Runs fides-related CI checks that require sensitive credentials label Jun 27, 2023
@SteveDMurphy SteveDMurphy marked this pull request as ready for review June 27, 2023 14:15
Copy link
Contributor

@pattisdr pattisdr left a comment

Choose a reason for hiding this comment

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

The issue and the solution make sense to me!

Copy link
Contributor

@adamsachs adamsachs left a comment

Choose a reason for hiding this comment

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

looks good! changes are well-scoped to specifically apply only to RedshiftConnector and if CONFIG.security.bastion_server_ssh_private_key is set, so minimal impact for other use cases 👍

@SteveDMurphy SteveDMurphy merged commit 0337c66 into main Jun 28, 2023
@SteveDMurphy SteveDMurphy deleted the SteveDMurphy-3680-ssl-prefer branch June 28, 2023 11:03
SteveDMurphy added a commit that referenced this pull request Jul 1, 2023
@SteveDMurphy SteveDMurphy mentioned this pull request Jul 5, 2023
37 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run unsafe ci checks Runs fides-related CI checks that require sensitive credentials
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Additional connector param required for Redshift
3 participants