Docker swarm by gdbelvin · Pull Request #578 · google/keytransparency

gdbelvin · 2017-05-11T18:47:25Z

This is a first cut at defining the proper docker swarm that implements Key Transparency correctly.
The complexity of the initial configuration has exceeded what Goreman can support.

Missing from this PR:

Starting the Trillian Admin server
Provisioning an initial log and map

gdbelvin · 2017-05-23T20:17:48Z

Now that google/trillian#607 is in, we can unblock this.

@cesarghali Could you review?
@liamsi Could you run through the instructions in the new README and see how they work for you?

liamsi · 2017-05-31T13:08:31Z

@liamsi Could you run through the instructions in the new README and see how they work for you?

~~Here are my first thoughts/comments/suggestions (will update comment as soon as I have more feedback):~~

Update: All issues below were adressed in gdbelvin#1, gdbelvin#2, and google/trillian#660

~~for consistency we might specify the minimum docker version necessary (similar to etcd which says v3.1.3); the docker-compose.yml file format is of version 3 which requires docker engine 1.13.0+~~ (done)
~~docker-compose isn't installed by default on some platforms; maybe, it should be mentioned as an explicit requirement?~~ (done)
~~docker-compose up -d raises an error due to google/trillian@ba4e1cd:~~ (fixed, see below)

Error message


[...]
WARNING: Image for service trillian-log was built because it did not already exist. To rebuild this image you must use `docker-compose build` or `docker-compose up --build`.
Building trillian-map
ERROR: Cannot locate specified Dockerfile: server/vmap/trillian_map_server/Dockerfile

Shouldn't go run $GOPATH/github.com/google/trillian/cmd/createtree/main.go rather be go run $GOPATH/src/github.com/google/trillian/cmd/createtree/main.go (at least this seams to be the default layout) (resolved)
~~see catch up on trillian modified paths gdbelvin/keytransparency#1 which fixes the last two problems~~ (merged)
~~./scripts/prepare_server.sh "hangs' quite a long time after last user input requested (frontend pub IP); maybe, tell the user what is going on (go build and key-generation)?~~
~~running docker-compose up shows several issues (I'm currently investigating if this is my firewall / docker configuration, or some general problem)~~
~~apparently, on ubuntu one needs to chmod 644 ../trilliantorage/mysql/storage.sql before running docker-compose up db; otherwise permissions might be -rw-r----- 1 and:~~ (we should keep this in mind but no changes needed; maybe later: a trouble shooting doc)

db_1   | /usr/local/bin/docker-entrypoint.sh: running /docker-entrypoint-initdb.d/storage.sql
db_1   | /usr/local/bin/docker-entrypoint.sh: line 180: /docker-entrypoint-initdb.d/storage.sql: Permission denied

in the line below I had to change localhost to the explicit IP assigned by docker to the container (the one that is shown by docker inspect ID under Networks->IPAddress (same problem on a fresh VM, I'll come up with a script that inserts the right IP in there)

go run $GOPATH/src/github.com/google/trillian/cmd/createtree/main.go --admin_server=localhost:8090 --pem_key_path=testdata/log-rpc-server.privkey.pem --pem_key_password="towel" --signature_algorithm=ECDSA --tree_type=LOG

while running prepare_server.sh one has to provide 0.0.0.0 as an IP address, otherwise the SAN extension doesn't get written (by gen_server_keys.sh) and one get's an error message ( x509: cannot validate certificate for 0.0.0.0 because it doesn't contain any IP SANs)

gdbelvin · 2017-05-31T17:40:19Z

Updated README.md to include versions and mention docker-compose.

cesarghali · 2017-05-31T15:56:02Z

README.md

+Set the `LOG_ID` and `MAP_ID` environment variables in `docker-compose.yml` with the output
+of these respective commands.
+
+7. Relaunch and observe


Why do we need to launch twice? Shouldn't we do the provisioning and editing the yml file before launching?
Also, this item should be number 8.

Updated to just launch the trillian components in this step:
docker-compose up trillian-map

cesarghali · 2017-05-31T15:57:20Z

README.md

+7. Relaunch and observe
+- `docker-compose up -d`
+- `docker-compose logs --tail=0 --follow`
+- [https://localhost:8080/v1/users/foo@bar.com](https://localhost:8080/v1/users/foo@bar.com)


Does this actually work? It might for now but I thought we want to have all API calls authenticated and if we do that this might stop working.

The open source repo only has an unauthenticated GET.
Different providers can choose to do different things in their environments.

cesarghali · 2017-05-31T15:59:00Z

README.md

+
+7. Provision a log and a map 
+```sh
+go run $GOPATH/src/github.com/google/trillian/cmd/createtree/main.go --admin_server=localhost:8090 --pem_key_path=testdata/log-rpc-server.privkey.pem --pem_key_password="towel" --signature_algorithm=ECDSA --tree_type=LOG


Where can log-rpc-server.privkey.pem be found? If a user try to run this command will it work? If not we should add a step for creating this private key.

It worked for me (I had to change from localhost to the IP assigned to the container by the docker engine).

cesarghali · 2017-05-31T16:22:43Z

cmd/keytransparency-server/Dockerfile

+ENV DB_USER=test \
+    DB_PASSWORD=zaphod \
+    DB_DATABASE=test \
+    DB_HOST=127.0.0.0:3306


Does this mean the user should install mysql and set the password to zaphod and create a test database? If yes, can we add a step for that, maybe in the readme?

The mysql db image uses the following environment variables to setup and configure the database on initial startup.

environment: MYSQL_PASSWORD: zaphod MYSQL_USER: test MYSQL_DATABASE: test MYSQL_RANDOM_ROOT_PASSWORD: "yes"

cesarghali · 2017-05-31T16:24:08Z

README.md

+```
+
+Set the `LOG_ID` and `MAP_ID` environment variables in `docker-compose.yml` with the output
+of these respective commands.


s/these/the above

cesarghali · 2017-05-31T17:16:39Z

docker-compose.yml

+    ports:
+      - "8080:8080" # json & grpc
+    environment:
+      LOG_ID: 8879615737060686335


Can you please add a comment that LOG_ID and MAP_ID values should be updated, here and below?

liamsi · 2017-06-01T14:42:48Z

README.md


  ```sh
-  goreman start
+docker-compose up trillian-map -d


docker-compose up trillian-map -d fails with ERROR: No such service: -d has to be docker-compose up -d trillian-map instead.

liamsi · 2017-06-01T14:55:43Z

cmd/keytransparency-signer/Dockerfile

+    MAP_URL=""
+ENV LOG_ID=0 \
+    LOG_URL=localhost:8090 \
+    LOG_KEY=genfiles/trillian-log-pubkey.pem


Which of the generated keys is this one? Shouldn't this be genfiles/p256-pubkey.pem?

This key actually comes from the Trillian repo :-
It corresponds to the private key the the Trillian Log uses.

OK, I'll change the default to ../trillian/testdata/log-rpc-server.pubkey.pem.

liamsi · 2017-06-02T10:50:16Z

README.md

+```sh
+go run $GOPATH/src/github.com/google/trillian/cmd/createtree/main.go --admin_server=localhost:8090 --pem_key_path=testdata/log-rpc-server.privkey.pem --pem_key_password="towel" --signature_algorithm=ECDSA --tree_type=LOG
+go run $GOPATH/src/github.com/google/trillian/cmd/createtree/main.go --admin_server=localhost:8090 --pem_key_path=testdata/log-rpc-server.privkey.pem --pem_key_password="towel" --signature_algorithm=ECDSA --tree_type=MAP
+```


Shouldn't this key rather be testdata/map-rpc-server.privkey.pem instead (of log-)?

liamsi · 2017-06-02T11:14:44Z

cmd/keytransparency-signer/Dockerfile

+
+ENTRYPOINT /go/bin/keytransparency-signer \
+	--db="${DB_USER}:${DB_PASSWORD}@tcp(${DB_HOST})/${DB_DATABASE}" \
+	--period="$SIGN_PERIOD" --key="$SIGN_KEY" \


The private signing key --key="$SIGN_KEY" is treated as path to a file flag here and yet the passed string is interpreted to contain the PEM bytes here. Looks like this fails (but independent from this docker pull).

After chatting with @RJPercival (thanks again), I've decided that it's reasonable to add an additional flag for the password of the private-key/PEM-file with an option for a config-file (with the PW contained) like implemented for trillian in google/trillian/pull/620

cesarghali

LGTM.

Given the fact I haven't ran a docker image before, I'm thinking of waiting until this PR becomes stable and try to follow the README instructions as someone with no knowledge of this.

- catch-up with google#654 (where the flag was removed) - needed for google/keytransparency#578

- catch-up with #654 (where the flag was removed) - needed for google/keytransparency#578

codecov-io · 2017-06-09T11:01:12Z

Codecov Report

Merging #578 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #578   +/-   ##
=======================================
  Coverage   59.87%   59.87%           
=======================================
  Files          34       34           
  Lines        2318     2318           
=======================================
  Hits         1388     1388           
  Misses        643      643           
  Partials      287      287

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4a4a462...53803c2. Read the comment docs.

- Add logging and don't crash signer - Update README

gdbelvin force-pushed the docker branch from c31610d to 4a8dd94 Compare May 11, 2017 18:47

gdbelvin added the WIP label May 16, 2017

gdbelvin force-pushed the docker branch 2 times, most recently from 8a2d888 to 81bb595 Compare May 23, 2017 19:33

gdbelvin removed the WIP label May 23, 2017

gdbelvin requested a review from cesarghali May 23, 2017 20:18

gdbelvin mentioned this pull request May 23, 2017

Prometheus #589

Merged

gdbelvin force-pushed the docker branch 3 times, most recently from d29a2be to 2cef673 Compare May 25, 2017 19:14

cesarghali suggested changes May 31, 2017

View reviewed changes

liamsi reviewed Jun 1, 2017

View reviewed changes

liamsi reviewed Jun 2, 2017

View reviewed changes

cesarghali approved these changes Jun 2, 2017

View reviewed changes

liamsi mentioned this pull request Jun 5, 2017

Allow file based configuration #597

Closed

liamsi added a commit to liamsi/trillian that referenced this pull request Jun 7, 2017

remove dump_metrics_interval from Docker files, too

59d2876

- catch-up with google#654 (where the flag was removed) - needed for google/keytransparency#578

liamsi mentioned this pull request Jun 7, 2017

Update Docker files: Remove dump_metrics_interval flag google/trillian#660

Merged

daviddrysdale pushed a commit to google/trillian that referenced this pull request Jun 7, 2017

remove dump_metrics_interval from Docker files, too (#660)

2cc3f34

- catch-up with #654 (where the flag was removed) - needed for google/keytransparency#578

Docker and dockercompose files

53803c2

- Add logging and don't crash signer - Update README

gdbelvin force-pushed the docker branch from d720884 to 53803c2 Compare June 9, 2017 11:16

gdbelvin merged commit b3e9875 into google:master Jun 9, 2017

gdbelvin deleted the docker branch June 9, 2017 11:59

Comments

Conversation

gdbelvin commented May 11, 2017

Uh oh!

gdbelvin commented May 23, 2017

Uh oh!

liamsi commented May 31, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gdbelvin commented May 31, 2017

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!

liamsi Jun 1, 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!

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!

gdbelvin Jun 1, 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!

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!

cesarghali left a comment

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Jun 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

liamsi commented May 31, 2017 •

edited

Loading

liamsi Jun 1, 2017 •

edited

Loading

gdbelvin Jun 1, 2017 •

edited

Loading

codecov-io commented Jun 9, 2017 •

edited

Loading