Skip to content
This repository has been archived by the owner on Jan 24, 2023. It is now read-only.

Add env var support for ssl mode #4442

Merged

Conversation

benwalk
Copy link
Contributor

@benwalk benwalk commented Jul 9, 2020

Description

SSL mode for the database connection string was not configurable. This change allows configuration of this via env var DB_SSL_MODE.

Motivation and Context

Enabling encrypted connections to backend databases should be allowed.

This issue is filed upstream at #4434.

How Has This Been Tested?

The existing tests validate TLS connection strings work.

The value of the supplied mode is already validated here.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Docs update
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have followed the guidelines in CONTRIBUTING.md, including the required formatting of the commit message

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jul 9, 2020

CLA Check
The committers are authorized under a signed CLA.

@codecov-commenter
Copy link

codecov-commenter commented Jul 9, 2020

Codecov Report

Merging #4442 into master will increase coverage by 0.59%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4442      +/-   ##
==========================================
+ Coverage   58.91%   59.51%   +0.59%     
==========================================
  Files         891      981      +90     
  Lines       29085    32691    +3606     
  Branches     4201     4673     +472     
==========================================
+ Hits        17136    19455    +2319     
- Misses      11949    13236    +1287     

@richard-cox richard-cox added the community Community Raised Issue label Jul 10, 2020
@richard-cox
Copy link
Contributor

Hi @benwalk , thanks for raising this PR. It's frustrating that the SSL mode information isn't automatically provided by the credentials section that the service provides, however we did spot in some cases it's included as a param in the url. Is that the case for the db's you're connecting to?

@benwalk
Copy link
Contributor Author

benwalk commented Jul 10, 2020

@richard-cox Unfortunately, it's not. We could push towards expanding/augmenting the credentials section the service provides, but our other apps that use this service also accept config for setting the SSL mode whereas Stratos does not.

@richard-cox
Copy link
Contributor

@benwalk Ok, thanks. We're aiming to get this in for our 4.0 release (RC1 end of this week). We're just fixing an issue where PRs aren't running correctly from forks (E2E tests fail to run but report success - #4453). Could you sign the CLA via the link in the communitybridge-easyclas comment?

@richard-cox
Copy link
Contributor

@benwalk Ok, gates successfully running and passing again. If you've signed the CLA you may need to close and re-open the PR for communitybridge-easycla to update

@benwalk
Copy link
Contributor Author

benwalk commented Jul 21, 2020

Thanks @richard-cox; I am working with my org on EasyCLA and hope to have it signed soon.

@richard-cox
Copy link
Contributor

@benwalk Any joy with that EasyCLA? We'd love to get this into 4.0 RC1 which hopefully will happen tomorrow. We could recreate your change in a new PR, however you'd need to state you absolve all rights to the code in this PR (IANAL but I think that should work ok)?

@benwalk benwalk closed this Jul 24, 2020
@benwalk benwalk reopened this Jul 24, 2020
@benwalk
Copy link
Contributor Author

benwalk commented Jul 24, 2020

Despite my org signing the CLA, and me being authorized for the Cloud Foundry Foundation project, I am unable to sign as a contributor. I've filed a support ticket with Community Bridge to see if they can help me out.

@richard-cox
Copy link
Contributor

Strange, could you try rebasing from master? That might be enough to retrigger it

@benwalk
Copy link
Contributor Author

benwalk commented Jul 27, 2020

@richard-cox It's been signed! It seems there was a caching issue.

@richard-cox richard-cox merged commit 956a895 into cloudfoundry:master Jul 27, 2020
@richard-cox
Copy link
Contributor

Magic, thanks for your patience. We need to do an RC2, so this will make it into 4.0.0

@benwalk benwalk deleted the INFRA-601/ssl-config-env branch July 27, 2020 19:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants