fixed ssl mode

[kikkupico]

fixes #82

Description

With the existing preview apps actions, it's not possible to use Postgres in SSL mode even if the user provides a connection string ending with ?sslmode=require. This happens because of a peculiar behaviour of the node-postgres client that we use. Paraphrasing the node-postgres documentation ( https://node-postgres.com/features/ssl ) -

If you plan to use a combination of a database connection string from the environment and SSL settings in the config object directly, then you must avoid including any of sslcert, sslkey, sslrootcert, or sslmode in the connection string. If any of these options are used then the ssl object is replaced and any additional options provided there will be lost.

So, when a user provides a connection URI ending with ?sslmode=require , unauthorised clients are automatically rejected. Our Postgres client used in the GitHub Actions is unauthorised and hence is rejected. Thus the preview apps flow ends at the DB connection stage with an error similar to the error below

FATAL: no pg_hba.conf entry for host "172.17.0.1", user "ram", database "postgres"

To fix this issue, we explored a few alternatives -

• Always use the option rejectUnauthorized: false in the node-pg client's connection function. This breaks the ability to use DBs without SSL as providing this parameter to the connect function will enable SSL by default.
• Ask the user to provide additional parameters in the preview apps YAML to configure Postgres connection. Eg: SSL_VERIFY=false. This is a non-standard approach and will likely confuse the users who are used to providing ssl mode info in the connection URI.
• Allow the user to provide connection URIs ending with ?sslmode=require but parse the connection URI and set the pgclient's connection options differently. If sslmode=require is found, set the option rejectUnauthorized=false always. Otherwise, don't enable SSL. This appeared to the best option and has been implemented in this PR.

Limitations

1. By using this fix, it is possible to connect to Heroku Postgres. But still we end up with a different error because Heroku does not allow users to create/drop DBs in the free plan. From the documentation ( https://devcenter.heroku.com/articles/connecting-to-heroku-postgres-databases-from-outside-of-heroku ) it is not clear whether the user will be able to create/drop DBs using a paid plan. Changing the action itself to not use drop/re-create DBs is out of the scope of this PR and will be handled later if required.

2. We have to confirm if the error reported here ( https://hasurahq.slack.com/archives/C01KZ6JPPCM/p1654034722372949?thread_ts=1654001877.882759&cid=C01KZ6JPPCM ) is somehow caused by allowing the user to set ?sslmode=require in the connection URI.

Tests

Connection to Heroku from node was tested using this sample script -

const { Client } = require('pg');

const client = new Client({
  connectionString: process.env.PG_URL,
  ssl: {
    rejectUnauthorized: false
  }
});

client.connect();

client.query('SELECT table_schema,table_name FROM information_schema.tables;', (err, res) => {
  if (err) throw err;
  for (let row of res.rows) {
    console.log(JSON.stringify(row));
  }
  client.end();
});

It is observed that removing the ssl option in fact leads to the original error we observed with Heroku PG.

The entire preview apps flow was tested using a sample repo. The following tests were done -

• For Heroku PG (with SSL) - https://github.com/kikkupico/samp-home-food-dev-2/actions/runs/2313574122 (connected through SSL but failed on DB creation as expected.
• Other PG (without SSL) - https://github.com/kikkupico/samp-home-food-dev-2/runs/6406768203?check_suite_focus=true (success, as expected)

[shraddhaag]

Functionality LGTM. Tested the following cases:

1. Non Heroku Postgres server with no SSL issue: ✅ succeeded as expected (run here)
2. Heroku Postgres server with SSL param not given in the connection URI: ❌ failed as expected (run here) error message can be improved here.
3. Heroku Postgres with SSL param in the connection URI: ✅ failed at DB creation step as expected.

We should properly update the documentation to reflect this change as well as update the previews apps YAML generator which this check in the input incase folks are giving the connection URI directly instead of through an ENV var/secrets.

I'd also like to test this with a paid heroku account to see the db creation go through as well. @wawhal could you please help with appropriate accesss here?

Edit: I tried the same workflow with Heroku postgres on paid plan Hobby Basic and getting the same error. Not sure if Heroku allows creating dbs. (run here)

[meetzaveri]

@shraddhaag @kikkupico

Quoting one of the lines from the last comment

Edit: I tried the same workflow with Heroku postgres on paid plan Hobby Basic and getting the same error. Not sure if Heroku allows creating dbs.

On this particular scenario, I have experienced similar issue with Heroku specifically. But when I tried creating DB cluster on DigitalOcean and used it instead of Heroku, there was no such issue that I faced on heroku( like having problem creating ephemeral database(s) )