Skip to content
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.

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

Add a placeholder for POSTGRES_PASSWORD #13929

Closed
wants to merge 1 commit into from
Closed

Add a placeholder for POSTGRES_PASSWORD #13929

wants to merge 1 commit into from

Conversation

progval
Copy link
Contributor

@progval progval commented Jun 1, 2020

It's a required environment variable, the postgresql container
crashes without it.

It's a required environment variable, the postgresql container
crashes without it.
@ClearlyClaire
Copy link
Contributor

Note that it's because of a breaking change in the postgres docker image: docker-library/postgres#658
It's only used at setup, so already-running images are fine.

We could revert to the old behavior, but this may not be advisable, especially if we end up merging #13941 (but docker-compose setups seem to not work without that…)

@SuperSandro2000
Copy link
Contributor

We should encourage people to use credentials for their databases.

@ClearlyClaire
Copy link
Contributor

We should encourage people to use credentials for their databases.

I mean, kinda, but people who did install using docker-compose in the past most probably have a password-less postgres anyway. We don't set a password either for Redis. And in the case of docker-compose this isn't an issue, thanks to the services being on their own network.

Requiring people to set a password for postgres is an extra step that, in this particular configuration, does not provide any extra security.

@SuperSandro2000
Copy link
Contributor

I am afraid to tell you that in the default docker daemon configuration this is an issue cause icc (inter container communication) is enabled which allows everything to talk to everything cause networks and port publishs are ignored.

@ClearlyClaire
Copy link
Contributor

I am afraid to tell you that in the default docker daemon configuration this is an issue cause icc (inter container communication) is enabled which allows everything to talk to everything cause networks and port publishs are ignored.

Ah, indeed. wtf

@shleeable
Copy link
Contributor

shleeable commented Jun 3, 2020

Bootstrap the container before your boot the compose services.

# this is untested.
$ docker run -d --name bootstrap-postgres \
    -e POSTGRES_USER=mastodon \
    -e POSTGRES_PASSWORD=ThePasswordYouWannaUseHere \
    -e POSTGRES_DB=mastodonDB \
    -v ./postgres:/var/lib/postgresql/data postgres:9.6-alpine

@ClearlyClaire
Copy link
Contributor

I mean, the current PR seems fine, with corresponding documentation to change that password.

@shleeable
Copy link
Contributor

hey, if you're going to hardcode the password. Can you add the username and db values as well?

    environment:
      POSTGRES_USER: "mastodon"
      POSTGRES_PASSWORD: "ThePasswordYouWannaUseHere"
      POSTGRES_DB: "mastodonDB"

@ClearlyClaire
Copy link
Contributor

ClearlyClaire commented Jun 3, 2020

Why? The setup tasks etc. default to db and postgres, not mastodonDB and mastodon

EDIT: postgres and postgres, sorry

@shleeable
Copy link
Contributor

shleeable commented Jun 3, 2020

db is the hostname, not the database/schema name.
the default values are postgres for username and database.

If you wanted to keep the current defaults from https://github.com/tootsuite/mastodon/blob/9b7e3b4774d47c184aa759364d41f40e0cdfa210/lib/tasks/mastodon.rake#L43-64

    environment:
      POSTGRES_USER: "mastodon"
      POSTGRES_PASSWORD: "ThePasswordYouWannaUseHere"
      POSTGRES_DB: "mastodon_production"

then you could remove those conditions on using_docker

edit: infact, I'd like to request that change.

@ClearlyClaire
Copy link
Contributor

If you wanted to keep the current defaults from https://github.com/tootsuite/mastodon/blob/9b7e3b4774d47c184aa759364d41f40e0cdfa210/lib/tasks/mastodon.rake#L43-64

This uses postgres and postgres as default when using docker. Sure, I guess we could change that. My only worry is that it could cause confusion for existing installs.

@shleeable
Copy link
Contributor

shleeable commented Jun 3, 2020

  1. thinking on it. I dislike the idea of hardcoding the password in the docker-compose.
    why add the password in plaintext in another location? plus because that value is only used once and isn't required after the container is bootstrapped. and I'll delete it every time and get merge conflicts every update.

I vote you don't merge this.

  1. Would you all be happy If I just write some documentation and record the installation process using asciinema? end to end?

@ClearlyClaire
Copy link
Contributor

yeah properly documenting it seems like a good plan, it'd just have been good if less documentation and user steps were needed

@shleeable
Copy link
Contributor

@ThibG almost completed draft mastodon/documentation#778

@shleeable
Copy link
Contributor

@progval please close this. I'll keep the docker documentation updated soon

@progval progval closed this Jun 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants