Automate Local and AWS Deployments #667

witoff · 2017-06-08T06:36:15Z

Summary

Per #666 (yikes!), we'd like to better document deployment configurations for trillian. This builds on pull #631 to provide simple, few-line local and cloud deployments using docker-compose and terraform.

Notes

This removes the MySQL strict clause in storage.sql's which is incompatible with aurora.
This merges Dockerfiles into /examples/deployments/ directory. Shipping these opinionated Dockerfiles next to source code felt like a stretch.
This pulls wait-for-it in at docker build time. Is this an appropriate way to use a dependency with our license?

AlCutter · 2017-06-12T11:45:18Z

examples/deployment/README.md

+terraform apply aws/
+
+# Set Variables
+HOST=...


How about DB_HOST?

merged this with automated launch userdata within terraform (more automation!) and changed to DB_HOST.

AlCutter · 2017-06-12T12:33:26Z

examples/deployment/aws/terraform.tf

+
+resource "aws_security_group" "trillian" {
+  name        = "trillian"
+  description = "Allow MySQL from Single IP"


This seems to allow from a single CIDR block :)

AlCutter · 2017-06-12T12:57:54Z

examples/deployment/scripts/download-wait-for-it.sh

+}
+
+download
+echo "ffe253ce564df22adbcf9c799e251ca0  wait-for-it.sh" > file.md5


You could do shas256sum --check <( echo "... wait-for-it.sh" ) and avoid creating the file.md5

doh, thanks. fixed!

AlCutter · 2017-06-13T16:57:47Z

BTW - we generally rebase topic branches onto latest master rather than merge master into the topic branch

witoff · 2017-06-16T05:45:36Z

rebased :)

RJPercival

Sorry for the wait, since I wasn't marked as a reviewer and @AlCutter had already approved it, I'd managed to overlook this.

RJPercival · 2017-06-23T21:46:35Z

examples/deployment/README.md

+Both build and example deployment files are stored within this repo.  For any of the below deployment methods, start here:
+
+```shell
+git clone git@github.com:google/trillian.git


HTTPS remote URLs are recommended over SSH.

RJPercival · 2017-06-26T19:41:42Z

examples/deployment/README.md

+export DB_PASSWORD="$(openssl rand -hex 16)"
+
+# Bring up services defined in this compose file.  This includes:
+# - local MySQL databse


s/databse/database/

RJPercival · 2017-06-26T19:42:07Z

examples/deployment/README.md

+
+# Bring up services defined in this compose file.  This includes:
+# - local MySQL databse
+# - container to seed the database


s/seed/initialize/

RJPercival · 2017-06-26T19:42:45Z

examples/deployment/README.md

+
+**Run With Docker Compose**
+
+For simple deployments running in a container is an easy way to get up and running with a local database.  To use Docker to run and interact with Trillian, start here:


s/For simple deployments/For simple deployments,/

RJPercival · 2017-06-26T19:43:27Z

examples/deployment/README.md

+
+For simple deployments running in a container is an easy way to get up and running with a local database.  To use Docker to run and interact with Trillian, start here:
+
+Set a random password and bring up the services defined in the provided compose file.  This includes a local mysql database database, a one-shot container to create the schema and the trillian server:


s/mysql database database/MySQL database/

RJPercival · 2017-06-26T19:57:00Z

examples/deployment/docker/db_client/Dockerfile

+ADD . /go/src/github.com/google/trillian
+WORKDIR /go/src/github.com/google/trillian
+
+RUN go get -v ./server/trillian_log_server


Why is trillian_log_server needed in this container?

removed this line but going to keep the source code included so /scripts/*.sh can be run

RJPercival · 2017-06-26T19:59:46Z

scripts/resetdb.sh

+  [ -z ${DB_NAME+x} ] && DB_NAME="test"
+  # format reused supplied environment variables
+  FLAGS=""
+  [ -z ${DB_PASSWORD+x} ] || FLAGS="${FLAGS} -p$DB_PASSWORD"


Assumes that $DB_PASSWORD does not contain spaces.

fixed with: -p'${DB_PASSWORD}'

RJPercival · 2017-06-26T20:00:53Z

scripts/resetdb.sh

+    case "$1" in
+      --force) FORCE=true ;;
+      --verbose) VERBOSE=true ;;
+      *) FLAGS="${FLAGS} $1"


I think this will remove quotation marks around arguments.

Consider making $FLAGS an array.

added a verbose logging statement that will print args to clarify what's being run. even as an array i'd still have to reconstruct with proper flags to pass to mysql. if you think this is a blocker we can revert back to passing all flags via $@ to mysql and only accepting other params via envars.

Suggested fix: https://github.com/RJPercival/trillian/commits/witoff/aws

This will properly handle quoted arguments. I made another couple of small improvements while I was poking around in that file, which you can find just after that commit.

RJPercival · 2017-06-26T20:03:37Z

scripts/resetdb.sh

+  if [ -z ${REPLY+x} ] || [[ $REPLY =~ ^[Yy]$ ]]
+  then
+      echo "Resetting DB..."
+      mysql -u $DB_USER $FLAGS -e "DROP DATABASE IF EXISTS ${DB_NAME};"


Some flags, e.g. --defaults-extra-file, will only work if they are the first flag passed to mysql. This flag ordering makes passing that flag impossible.

reordered to: mysql -u $FLAGS $DB_USER and preserved user supplied order in collect_vars fn.

RJPercival · 2017-06-26T20:06:55Z

scripts/resetdb.sh

+      echo "Resetting DB..."
+      mysql -u $DB_USER $FLAGS -e "DROP DATABASE IF EXISTS ${DB_NAME};"
+      mysql -u $DB_USER $FLAGS -e "CREATE DATABASE ${DB_NAME};"
+      mysql -u $DB_USER $FLAGS -e "GRANT ALL ON ${DB_NAME}.* TO '${DB_NAME}'@'localhost' IDENTIFIED BY 'zaphod';"


I'm still not sure how this is supposed to work. Granting access for the user to connect only from localhost shouldn't work if the Trillian server/signer is running on a different machine, as will likely be the case on cloud infrastructure. It may work if $DB_NAME == "test", because some MySQL installations come with default privileges that grant unrestricted access to that database for all users.

i'm also not entirely clear on this. this change mirrors what's currently in the master branch and just makes DB_NAME a variable. after this is merged i'd like work on better integration testing here which may prove this unnecessary.

With the database name hardcoded as "test", it's going to get the special treatment that MySQL gives that table by default. By replacing this with a variable, that's no longer true unless $DB_NAME == "test". Therefore, I think replacing 'localhost' with '%' on this line is warranted. Access to the database server should be restricted to certain IP ranges, so it won't allow brute force attacks on this user by just anyone.

RJPercival · 2017-07-03T17:43:01Z

I think you need to rebase - looks like master was broken due to a change in a dependency when you last rebased.

daviddrysdale · 2017-07-03T17:54:29Z

Yeah, Prometheus changed underneath us upstream -- pull #724 should have fixed it though.

DB operations should be flexibile to run against many environments Dockerize core components remove deprecated comment

simplify cloud and docker deployments

witoff · 2017-07-06T04:18:23Z

k, rebased and tests passing now.

RJPercival · 2017-07-07T17:21:08Z

scripts/resetdb.sh

+      mysql -u $FLAGS $DB_USER -e "DROP DATABASE IF EXISTS ${DB_NAME};"
+      mysql -u $FLAGS $DB_USER -e "CREATE DATABASE ${DB_NAME};"
+      mysql -u $FLAGS $DB_USER -e "GRANT ALL ON ${DB_NAME}.* TO '${DB_NAME}'@'localhost' IDENTIFIED BY 'zaphod';"
+      mysql -u $FLAGS $DB_USER -D ${DB_NAME} < ${TRILLIAN_PATH}/storage/mysql/storage.sql


I don't think you meant to separate "-u" and "$DB_USER". FYI, I've fixed this in the branch I referenced above, as a side-effect of adding "-u $DB_USER" to $FLAGS in collect_vars().

RJPercival · 2017-07-10T21:58:00Z

These should be my final comments; it's looking good to merge bar those.

witoff · 2017-07-24T06:32:03Z

edits are merged (thank you!) and waiting on #756 to fix broken golang/mocks updates before we can merge.

gdbelvin · 2017-07-24T14:44:29Z

Hi @witoff, thanks for this PR. Rather than moving the opinionated Dockerfiles to the example directory, would you mind keeping them in-place and making them less opinionated? Having some kind of starting Dockerfile would help other projects adopt Trillian faster.

If you feel that the opinionated Dockerfiles have value, please use the from Docker command to start with the less opinionated Dockerfiles, and add the opinionated bits in the example directory like you've suggested.

Thanks!
-Gary

witoff · 2017-07-24T20:14:55Z

@gdbelvin updated PR with a proposal on how these could work. my thought process is that main.go exposes a set of flags, default Dockerfiles define what's needed to build/run the binary and it's up to the user to configure from there, likely via docker-compose, docker run etc.

Specifying flags/healthcheck/envars in the Dockerfile makes assumptions about one's infrastructure that could reasonably be incorrect which would force me to write a separate Dockerfile anyways.

I've also kept the /examples/... Dockerfiles separate, since inheriting from the /server/... Dockerfiles would force a 2-phase build (or more scripts) which feels like it would work against the goal here of making it easy to reason about and run one example deployment.

wdyt? if you're okay with this approach does this break any other uses of these dockerfiles that i can help update?

gdbelvin · 2017-07-25T15:08:09Z

This is great. I now remember why I had the flags in the original docker files, and that is because go flags doesn't pick up environment variables, so I basically duplicated the flags with an additional (confusing) set of defaults. These non-opinionated Docker files would be great if we had a way for go flags to pick up passed-in environment variables.

@RJPercival @Martin2112 Do we have a plan for passing flags via environment variables?

RJPercival · 2017-07-25T21:53:08Z

No, we had no plans to implement that. It's easy to do, probably something like this in cmd/flags.go:

func ParseFlagsFromEnv() {
  flag.VisitAll(func (f *Flag) {
    // Might want to modify the flag name before using it as the envar name, e.g. uppercase and/or prefix with "TRILLIAN_".
    if env, ok := os.LookupEnv(f.Name); ok {
      f.Value.Set(env)
    }
  })
}

The only question is how much value there is in supporting this. Presumably this is to support people following the 12 Factor App guidelines, which lays out the advantages. The main cost is that it adds a non-obvious, third way of controlling the behaviour of Trillian's binaries.

Martin2112 · 2017-07-26T08:19:54Z

We've deliberately kept flags isolated and e.g. the logserver main.go is just over 100 lines of code. Anybody who needs this should easily be able create an alternate main() that gets the configuration from somewhere else. Martin

…

On 25 July 2017 at 22:53, Rob Percival ***@***.***> wrote: No, we had no plans to implement that. It's easy to do, probably something like this in cmd/flags.go: func ParseFlagsFromEnv() { flag.VisitAll(func (f *Flag) { // Might want to modify the flag name before using it as the envar name, e.g. uppercase and/or prefix with "TRILLIAN_". if env, ok := os.LookupEnv(f.Name); ok { f.Value.Set(env) } }) } The only question is how much value there is in supporting this. Presumably this is to support people following the 12 Factor App <https://12factor.net/config> guidelines, which lays out the advantages. The main cost is that it adds a non-obvious, third way of controlling the behaviour of Trillian's binaries. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#667 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AMv2Twhx_AWZp7ffc9wSFwH8QBS0DXhMks5sRmPFgaJpZM4Nzok9> .

gdbelvin

I think we can use the command option in docker-compose to add commandline flags now that we're using ENTRYPOINT

LGTM.

googlebot added the cla: yes label Jun 8, 2017

witoff changed the title ~~Document Local and AWS Deployments~~ Automate Local and AWS Deployments Jun 12, 2017

AlCutter reviewed Jun 12, 2017

View reviewed changes

witoff force-pushed the witoff/aws branch from ec9bf10 to 9f9cb71 Compare June 13, 2017 01:46

AlCutter approved these changes Jun 13, 2017

View reviewed changes

witoff force-pushed the witoff/aws branch from 42b8d9c to d6b7947 Compare June 16, 2017 05:40

witoff mentioned this pull request Jun 16, 2017

Add Flexible Flexible ResetDB and Docker Support #631

Closed

RJPercival suggested changes Jun 26, 2017

View reviewed changes

RJPercival assigned witoff Jun 26, 2017

witoff force-pushed the witoff/aws branch 2 times, most recently from 9b39cce to 47bf243 Compare June 30, 2017 23:01

witoff and others added 4 commits July 5, 2017 10:12

Should have a dockerized DB Client with sourcecode and mysql DB ops

56a3301

DB operations should be flexibile to run against many environments Dockerize core components remove deprecated comment

common deployment configurations should be captured in examples

1203e6e

simplify cloud and docker deployments

remove cloud incompatible sql configurations

02e5a64

cleanup pull request before merge

3e6bbe5

witoff force-pushed the witoff/aws branch from 47bf243 to 3e6bbe5 Compare July 5, 2017 17:13

RJPercival reviewed Jul 7, 2017

View reviewed changes

Rob Percival added 3 commits July 7, 2017 18:22

resetdb.sh: handle quoted arguments correctly

8ec0889

resetdb.sh: add to

3418c5f

resetdb.sh: improve formatting of output

115c4da

witoff force-pushed the witoff/aws branch from 3e6bbe5 to daf31fe Compare July 10, 2017 18:57

witoff force-pushed the witoff/aws branch 2 times, most recently from 67f235b to 115c4da Compare July 24, 2017 06:01

RJPercival approved these changes Jul 24, 2017

View reviewed changes

RJPercival requested a review from gdbelvin July 24, 2017 15:01

witoff force-pushed the witoff/aws branch from 4ce56df to a412ba2 Compare July 24, 2017 20:04

add non-opinionated inline with main.go files

87dba74

witoff force-pushed the witoff/aws branch from a412ba2 to 87dba74 Compare July 24, 2017 20:06

gdbelvin approved these changes Jul 26, 2017

View reviewed changes

liamsi mentioned this pull request Jul 26, 2017

Update Dockerfiles google/keytransparency#712

Closed

gdbelvin merged commit 9179d40 into google:master Aug 1, 2017


		Run With Docker Compose

		For simple deployments running in a container is an easy way to get up and running with a local database. To use Docker to run and interact with Trillian, start here:


		For simple deployments running in a container is an easy way to get up and running with a local database. To use Docker to run and interact with Trillian, start here:

		Set a random password and bring up the services defined in the provided compose file. This includes a local mysql database database, a one-shot container to create the schema and the trillian server:

Automate Local and AWS Deployments #667

Automate Local and AWS Deployments #667

Uh oh!

Conversation

witoff commented Jun 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlCutter commented Jun 13, 2017

Uh oh!

witoff commented Jun 16, 2017

Uh oh!

RJPercival left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RJPercival Jul 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RJPercival commented Jul 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daviddrysdale commented Jul 3, 2017

Uh oh!

witoff commented Jul 6, 2017

witoff commented Jun 8, 2017 •

edited

Loading

RJPercival Jul 10, 2017 •

edited

Loading

RJPercival commented Jul 3, 2017 •

edited

Loading

gdbelvin commented Jul 24, 2017 •

edited

Loading

witoff commented Jul 24, 2017 •

edited

Loading