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

Proposal to refine docker deployment #1302

Merged
merged 2 commits into from Jun 20, 2019

Conversation

Projects
None yet
3 participants
@DazWilkin
Copy link
Collaborator

commented Jun 19, 2019

Triggered by my challenges using KeyTransparency's (KT's) docker-compose (#1300)

This is a proposal to streamline the use of docker-compose and the container images.

It will not (yet) work as-is.

0. CI|CD Key Transparency Server often unavailable

The CI|CD deployment rarely works and the KT server (35.202.56.9:443) is generally inaccessible. This makes the need for a user-deployable mechanism more important. The solution for docker-compose exists and is the easiest solution.

1. Go Modules

The use of Go Modules (appears to) cause problems with the docker-compose builds. The Dockerfiles were being referenced from github.com/google and this directory contains no go.mod files. This resulted in issues for the docker build commands. Splitting KT and Trillian (see #2) allows the build context to be more appropriately set to github.com/google/keytransparency and this does contain a go.mod and the issue goes away.

2. Disconnect KT and Trillian

Currently KT's docker-compose will rebuild Trillian images as-needed. I think this is too strong a dependency and that -- ideally -- KT should reference "golden" images for the Trillian components through a repository, e.g. us.gcr.io/trillian/log-server.

3. Switch to Distroless

Trillian's images are built using Distroless. Recommend switching KT's. This reduces the resulting image sizes and -- as a consequence -- reduces the scope of vulnerabilities.

There is one downside to this. The distroless/base image does not include curl. The docker-compose healthchecks use curl to check endpoints. These no longer work with distroless. An alternative is to add a simple Golang healthcheck that can be included in the Dockerfile and referenced from the Dockerfile and|or docker-compose.

4. Sequencer configuration issue

The sequencer service configuration did not explicitly --tls-key and --tls-cert to point to the genfiles mapped to /kt/ in the container.

@DazWilkin DazWilkin requested a review from google/keytransparency as a code owner Jun 19, 2019

@googlebot googlebot added the cla: yes label Jun 19, 2019

build:
context: ../trillian
dockerfile: examples/deployment/docker/log_server/Dockerfile
image: us.gcr.io/trillian/log-server:latest

This comment has been minimized.

Copy link
@gdbelvin

gdbelvin Jun 19, 2019

Collaborator

I both love and dislike :latest

It both fetches the "latest" image - great. But it also prevents kubectl apply from updating the image when "latest" change. I don't know what the best practices are around this, but "latest" has given me a lot of heartburn keeping the CI environment up-to-date.

This comment has been minimized.

Copy link
@DazWilkin

DazWilkin Jun 19, 2019

Author Collaborator

Yes, I don't recommend :latest either. Apologies... It was a shortcut to not having a better way to grab the latest release of Trillian's images.

@gdbelvin

This comment has been minimized.

Copy link
Collaborator

commented Jun 19, 2019

@DazWilkin Thank you very much for giving the Docker files some much needed love.

  1. Agree - I'd welcome input on a better way to make it a more reliable playground, but I think working Docker files are a must regardless.

  2. I'm not sure I fully follow, but I trust your judgement here
    a. Trillian recently turned on Go Modules, so now both KT and Trillian are Go Module enabled.
    b. Updating the build context is the right thing to do. I'm not sure why the KT build files had such a weird context.

  3. Relying on Trillian golden builds sounds like absolutely the right thing to do. It would be nice if there was some command we could run to get a particular environment to pick up the latest Trillian images.

  4. Distroless looks amazing. You pulled off some impressive Dockerfile tricks there. Love the small sizes.

  5. Thanks for fixing the sequencer configuration issue.
    At some point I'd love to remove secrets from the docker images entirely. #1191 was an effort in that direction but I got side tracked.

@DazWilkin

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 19, 2019

@gdbelvin in issue #1300 I was receiving build errors. The error results from the build's context not having a go.mod. This was github.com/google in KT's Dockerfiles. By disconnecting from Trillian (and not building both repo's images at once), the context can become github.com/google/keytransparency, this does contain a go.mod and so the problem went away.

I suspect the break was either Modules being added to Trillian|KT, or KT's Dockerfiles being changed to root from github.com/google rather than github.com/google/keytransparency.

@gdbelvin gdbelvin merged commit adc8c83 into google:master Jun 20, 2019

3 checks passed

GolangCI No issues found!
Details
Travis CI - Pull Request Build Passed
Details
cla/google All necessary CLAs are signed

gdbelvin added a commit to gdbelvin/keytransparency that referenced this pull request Jul 18, 2019

Merge branch 'master' into genkeys
* master: (106 commits)
  Remove unused logVerifier (google#1324)
  Verify Revisions in StreamRevisions (google#1323)
  Pair verifier functions (google#1322)
  Split VerifyRevision into Verify{LogRoot,MapRevision (google#1318)
  Make Previous hash check optional (google#1307)
  Remove VerifySignedMapRoot from VerifierInterface (google#1320)
  Remove trailing whitespace (google#1321)
  Encapsulate Client Verifier State in test vectors (google#1316)
  Pass along err message (google#1314)
  Remove unnessesary func() (google#1319)
  New test vector transcript format (google#1315)
  Track map revision inside mutation (google#1310)
  Move verifier to its own package (google#1312)
  go generate ./... (google#1306)
  Fix proto copying in revisions and paginator tests. (google#1309)
  Fix proto copying in server_test. (google#1308)
  go mod tidy (google#1305)
  Use new TrillianMapWrite API (google#1304)
  Configurable maximum queue depth for metric reporting. (google#1303)
  Proposal to refine docker deployment (google#1302)
  ...

gdbelvin added a commit to gdbelvin/keytransparency that referenced this pull request Jul 18, 2019

Merge branch 'master' into test
* master: (95 commits)
  Remove unused logVerifier (google#1324)
  Verify Revisions in StreamRevisions (google#1323)
  Pair verifier functions (google#1322)
  Split VerifyRevision into Verify{LogRoot,MapRevision (google#1318)
  Make Previous hash check optional (google#1307)
  Remove VerifySignedMapRoot from VerifierInterface (google#1320)
  Remove trailing whitespace (google#1321)
  Encapsulate Client Verifier State in test vectors (google#1316)
  Pass along err message (google#1314)
  Remove unnessesary func() (google#1319)
  New test vector transcript format (google#1315)
  Track map revision inside mutation (google#1310)
  Move verifier to its own package (google#1312)
  go generate ./... (google#1306)
  Fix proto copying in revisions and paginator tests. (google#1309)
  Fix proto copying in server_test. (google#1308)
  go mod tidy (google#1305)
  Use new TrillianMapWrite API (google#1304)
  Configurable maximum queue depth for metric reporting. (google#1303)
  Proposal to refine docker deployment (google#1302)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.