Skip to content
This repository was archived by the owner on Sep 20, 2021. It is now read-only.

Added a flags to enable azure services.#155

Merged
CalvinRodo merged 9 commits into
masterfrom
postgres_ssl_connections
Feb 26, 2020
Merged

Added a flags to enable azure services.#155
CalvinRodo merged 9 commits into
masterfrom
postgres_ssl_connections

Conversation

@CalvinRodo

@CalvinRodo CalvinRodo commented Feb 19, 2020

Copy link
Copy Markdown
Member

Please note:

This is a bit janky ideally we would use a special production.js configuration for this, and wouldn't use the default connection in the datastore.js that is mainly meant for local stuff only. This will change soon as we have prod builds working

To Test:

  1. Setup a firewall rule in azure to allow you to connect to an Azure Database for PostgreSQL.
  2. Setup a firewall rule in azure to allow you to connect to an Azure Cache for Redis service.
  3. Set the FEATURE_AZURE_PG_SSL flag to true
  4. Set the FEATURE_REDIS_SSL flag to true
  5. Download the Cert specified here to your local folder https://docs.microsoft.com/en-us/azure/postgresql/concepts-ssl-connection-security
  6. Run the thing
  7. ???
  8. Profit (or you know it doesn't crash on startup.)

Also adds the latest version of Lodash

@CalvinRodo CalvinRodo requested a review from a team as a code owner February 19, 2020 16:33
@jeffmaher jeffmaher temporarily deployed to cppd-medical-postgres-s-k4vfmc February 19, 2020 16:35 Inactive
@CalvinRodo CalvinRodo changed the title Added a flag to enable AppServiceSSL locally Added a flag to enable Azure DB for PG SSL Feb 19, 2020
@jeffmaher jeffmaher temporarily deployed to cppd-medical-postgres-s-k4vfmc February 19, 2020 16:48 Inactive
@jeffmaher jeffmaher temporarily deployed to cppd-medical-postgres-s-k4vfmc February 19, 2020 16:49 Inactive
Comment thread config/datastores.js Outdated
@dsamojlenko

Copy link
Copy Markdown
Member

See #158 for production builds

@jeffmaher jeffmaher temporarily deployed to cppd-medical-postgres-s-k4vfmc February 21, 2020 13:46 Inactive
@jeffmaher jeffmaher temporarily deployed to cppd-medical-postgres-s-k4vfmc February 21, 2020 13:53 Inactive
@lgtm-com

lgtm-com Bot commented Feb 21, 2020

Copy link
Copy Markdown

This pull request introduces 1 alert when merging ebe1272 into 9fa1359 - view on LGTM.com

new alerts:

  • 1 for Syntax error

Getting latest version of lodash
Output environment variables when logging at debug level
@jeffmaher jeffmaher temporarily deployed to cppd-medical-postgres-s-k4vfmc February 21, 2020 16:12 Inactive
var-kyle
var-kyle previously approved these changes Feb 21, 2020

@var-kyle var-kyle left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good.

@jeffmaher jeffmaher temporarily deployed to cppd-medical-postgres-s-k4vfmc February 21, 2020 20:05 Inactive
@CalvinRodo CalvinRodo changed the title Added a flag to enable Azure DB for PG SSL Added a flags to enable azure services. Feb 21, 2020
@CalvinRodo

Copy link
Copy Markdown
Member Author

I'll update the description later.

Comment thread config/bootstrap.js Outdated
Comment thread config/datastores.js
Comment on lines +66 to +69
options: {
logging: 'verbose',
dialectOptions: generateDialectOptions(),
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just noting that we should come back once we are able to run production builds and remove this conditional stuff and move it into config/env/production.js

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe put a @todo comment block above?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Added a todo, but this will likely get resolved in #158

Comment thread config/session.js
Comment on lines +12 to +19
const featureFlags = require('../api/utils/FeatureFlags');
function generateTls() {
if (featureFlags.isEnabled('FEATURE_REDIS_SSL')) {
return { };
}
return undefined;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same comment as above about coming back around to this later. Maybe add a @todo comment block.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same as above, this will get changed in #158

@jeffmaher jeffmaher temporarily deployed to cppd-medical-postgres-s-k4vfmc February 26, 2020 15:15 Inactive
@var-kyle var-kyle requested a review from dsamojlenko February 26, 2020 15:43
@dsamojlenko dsamojlenko temporarily deployed to cppd-medical-postgres-s-k4vfmc February 26, 2020 15:54 Inactive
@CalvinRodo CalvinRodo merged commit 547c5a1 into master Feb 26, 2020
@CalvinRodo CalvinRodo deleted the postgres_ssl_connections branch February 26, 2020 16:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants