Skip to content

services/horizon: Fix docker-compose Postgres' auth #2301

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Feb 21, 2020
Merged

services/horizon: Fix docker-compose Postgres' auth #2301

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
28f60cd
services/horizon: Fix docker-compose Postgres auth
2opremio Feb 20, 2020
a64b258
Merge branch 'master' into fix-compose-pg-pwd
leighmcculloch Feb 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions services/horizon/docker/docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
version: '3'
services:
core-postgres:
image: postgres:9.6-alpine
image: postgres:9.6.17-alpine
Copy link
Member

Choose a reason for hiding this comment

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

I think we should leave this change out and continue to use the latest 9.6 patch version. It's valuable for us to continue to use the latest security patches and to not need to update this manually on each patch release. Normally a patch release won't include breaking changes and this seems like an exception, so the pain of a new version breaking use is unlikely to be frequent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's valuable for us to continue to use the latest security patches and to not need to update this manually on each patch release

As far as I understand this is only used for local development, so I am not sure security is more important than stability but, happy to make the change if you want.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, but nobody wants their developer environment exploitable.

Copy link
Member

@leighmcculloch leighmcculloch Feb 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

It's worth noting that this is only pinning to a version of Postgres, not to a version of the Docker image. The version number is the version of Postgres, and the Docker images can change independent of that with any change to the official Dockerfiles.

Specifying the patch version number wouldn't have prevented this breakage from occurring because the breaking change was a change to the Docker image (docker-library/postgres@42ce743), and not Postgres. See this PR for more information that is adding a warning to their docs about this as a result of this break: docker-library/postgres#689.

If we really want to ensure 100% stability this would need to pin to the image digest, but I think this is unnecessary as breaking changes are rare.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Your point doesn't hold, because tags with the patch number are not rebuilt, and thus can be considered immutable for all they are worth
.

Copy link
Member

Choose a reason for hiding this comment

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

Oh nice, TIL. Thanks!

restart: on-failure
environment:
- POSTGRES_PASSWORD=mysecretpassword
Expand Down Expand Up @@ -30,9 +30,10 @@ services:
network_mode: '${NETWORK_MODE:-bridge}'

horizon-postgres:
image: postgres:9.6-alpine
image: postgres:9.6.17-alpine
restart: on-failure
environment:
- POSTGRES_HOST_AUTH_METHOD=trust
- POSTGRES_DB=horizon
ports:
- "5432:5432"
Expand Down