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

split up docker compose file #146

Merged
merged 11 commits into from
Sep 17, 2018
Merged

split up docker compose file #146

merged 11 commits into from
Sep 17, 2018

Conversation

rstorey
Copy link
Member

@rstorey rstorey commented Sep 12, 2018

Set the passwords in your .env file and create the sentry secret key as documented in the README.

Run these commands to test:

make firstup
make down
make allup
make clean
make devup

The make clean command should invoke the make down command.

The firstup command does not work on Mac unless you remove the increase-elk step (this has to be done manually anyway according to the instructions in https://www.elastic.co/guide/en/elasticsearch/reference/current/docker.html The allup command can be used on Macs although it doesn't create the sentry user.

Another thing to try is to do make devup without deploying sentry and prometheus. In this default docker stack, ELK is deployed because of the connection to the app container, so if you're on a Mac you have to increase your ELK resources (see link above).

There's also some deployment code for cloudformation in here. The only tie between the cloudformation code and the docker changes is that the POSTGRES_HOST is now an environment variable.

@rstorey rstorey requested a review from acdha September 12, 2018 17:57
@rstorey rstorey self-assigned this Sep 12, 2018
@acdha
Copy link
Member

acdha commented Sep 13, 2018

👍 on splitting this up since Sentry and ELK are separate services and will only be deployed this way in some local development scenarios.

I was a bit surprised to see the CloudFormation templates in here as well – not sure it's worth the time but it felt like a second stage.

@@ -42,10 +46,10 @@ Resources:
StorageType: gp2
BackupRetentionPeriod: '7'
MasterUsername: concordia
MasterUserPassword: MyPassword
MasterUserPassword: !Ref DbPassword
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't store the password here where it could potentially be read by someone with CloudFormation access. Fortunately, there are several options for this, and I'd recommend (SSM secure parameters) which I used for the shared Sentry deployment.

"MasterUserPassword": "{{resolve:ssm-secure:/devops/sentry/DB/MasterUserPassword:1}}",

Storing that requires two steps:

  1. Setup a KMS key with an alias like concordia-production: https://console.aws.amazon.com/iam/home?#/encryptionKeys/us-east-1/
  2. Generate and store the password: aws ssm put-parameter --type SecureString --key-id "alias/concordia-production" --name "/concordia/production/DB/MasterUserPassword" --value $(pwgen -s 30 1)

There's one catch to this: currently, ECS does not have a supported mechanism to retrieve SSM parameters. That means that the app would need to either use boto to retrieve them or have its Docker entrypoint retrieve it before starting, perhaps something like this:

DB_PASSWORD="$(aws ssm get-parameter --output=text --name /concordia/production/DB/MasterUserPassword --with-decryption --query "Parameter.Value")"
export DB_PASSWORD

@@ -31,6 +31,11 @@ Parameters:
Type: String
Default: /

PostgresHost:
Copy link
Member

Choose a reason for hiding this comment

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

If we add the postgres host to the outputs, we could use Fn::ImportValue to retrieve it in this stack

@rstorey rstorey force-pushed the rstorey-update-docker-compose branch from 83db3b4 to 2c32057 Compare September 13, 2018 16:31
@rstorey rstorey force-pushed the rstorey-update-docker-compose branch from 2c32057 to 5df806e Compare September 13, 2018 16:33
@rstorey
Copy link
Member Author

rstorey commented Sep 13, 2018

I moved the cloud formation stuff to a different branch / different PR. I will implement the ideas in your review comments in that branch.

@rstorey rstorey mentioned this pull request Sep 13, 2018
@rstorey rstorey merged commit c8e3671 into master Sep 17, 2018
@rstorey rstorey deleted the rstorey-update-docker-compose branch September 18, 2018 15:08
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.

2 participants