Feature: ability to turn off SSL required

[jpmcb]

Type of feature

🍕 Feature

Current behavior

Currently, this microservice only can make connections to postgres over ssl:

https://github.com/open-sauced/pizza/blob/65f6e1859bda344c8ae16af61130781e332ee846/pkg/database/handler.go#L28

Suggested solution

We should consider having a flag / config that disables ssl being required in order to connect. Ideally, this would flip to some "auto" mode that can connect to any postgres.

This should also log a warning if this setting has been turned on so administrators can be assured the configuration that's being run.

Additional context

No response

Code of Conduct

• [X] I agree to follow this project's Code of Conduct

Contributing Docs

• [X] I agree to follow this project's Contribution Docs

[AvineshTripathi]

Do you think instead using the flag using the prefer mode of ssl a good option (more info https://www.postgresql.org/docs/8.4/libpq-connect.html#LIBPQ-CONNECT-SSLMODE) if not then if the flag is set to true then maybe setting sslmode to disable is good

[k1nho]

prefer is a nice choice, but I think is not supported on lib/pq at least last time I tried. I think using the flag and setting sslmode=%s to receive either disable when the flag is set or require should be the solution. This is also a workaround for dev environment to avoid going over the ssl cert setup.

[jpmcb]

This is also a workaround for dev environment to avoid going over the ssl cert setup.

Exactly. This isn't really a product requirement as much as it is a "nice to have" to lower the bar abit so people don't have to setup a postgres cluster with SSL. I could also see the argument that there shouldn't be the ability to turn off SSL (since it makes the security story abit worse here). But generally, OpenSauced wouldn't ship a database without SSL turned on so this microservice would fail to start for our use cases if we tried deploying it in that way.

[mtfoley]

@jpmcb regarding lowering the bar a bit, I started some work on my fork for spinning up Postgres, migrations, and the application via Docker compose. Branch is still pretty rough, but it works. Let me know if I ought to create an issue for this to discuss further.

[jpmcb]

A docker compose file would be useful for contributors although it wouldn't be the full picture (since the database is really a database from the API which we have some steps for setting up: https://github.com/open-sauced/api#%EF%B8%8F-setting-up-a-postgresql-database-locally).

It could be very cool to have a docker compose that captures more of the stack to give a full local development experience.

[mtfoley]

Got it. I created the separate issue and linked a draft PR for it. Regarding the change for this issue, it'd be easy enough to create a map that we could use to validate against reasonable values (e.g. "require", "disable", etc) and warn if we don't see a value specified, or if it's invalid.

e.g.

validSSLModes := map[string]bool { 
  "require": true,
  "disable": true,
  // ...
}
sslMode, sslModeSet := os.LookupEnv("SSLMODE")
if !sslModeSet {
  // log warning about variable being unset
  sslMode = "require"
} else if _, valid := validSSLMode[sslMode]; !valid {
   // log warning about variable being invalid
  sslMode = "require"
}

// work with sslMode from here