Better certificate generation by chregu · Pull Request #1 · liip/pontsun

chregu · 2019-08-14T08:08:34Z

Initial problem: The certificate generation doesn't work on OS X, since openssl behaves differently (it errors out)

Solution: Use a docker image to generate to certificates on all platforms. Also to make the environment consistent.

Problem 2: If you want to have an additional domain name other than the default one for your project (let's say, api.rokka.test instead of rokka-api.pontsun.test), there was no easy way.

Solution 2: Add alternate DNS entries to the cert. The script now reads all the existing and adds new one before recreating it.

More stuff in this MR:

Moved certificates from certificates/ to etc/certificates to be able to have more config options in that directory (it's configurable via the PONTSUN_DIR_ETC variable)
Automatically installs the generated root certificate to the system trust store on OS X or if smallstep-cli is installed (https://smallstep.com/docs/cli/), if it doesn't exist already (needs password)

The helper image (liip/pontsun-helper:latest) for openssl cert generation isn't on hub.docker.com yet (I need to get the credentials before I can upload it), you have to generate it first locally with build/build.sh

OdyX

In general, not fan of the idea to need a container for something as trivial as generating a cert, but why not.

I'd make sure no bashisms are in use, and rely on /bin/sh (with set -e) everywhere.

OdyX · 2019-08-14T08:12:05Z


 # Local
 .env
+


No whitespace; add comment please

added a comment, left the whitespace

OdyX · 2019-08-14T08:13:31Z

@@ -0,0 +1,5 @@
+#!/usr/bin/env bash
+DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"


There's really nothing less ugly?

DIR=$(realpath $(dirname $(dirname $0)))

?

Nope, unfortunately not, see:

https://www.ostricher.com/2014/10/the-right-way-to-get-the-directory-of-a-bash-script/
https://stackoverflow.com/questions/59895/get-the-source-directory-of-a-bash-script-from-within-the-script-itself

It's even worse (or not really possible at all) in Posix sh

https://stackoverflow.com/questions/29832037/how-to-get-script-directory-in-posix-sh#29834779

OdyX · 2019-08-14T08:14:05Z

@@ -0,0 +1,5 @@
+FROM debian:stable-slim


Please specify the Debian release; buster-slim; but thanks for using Debian!

OdyX · 2019-08-14T08:15:06Z

@@ -0,0 +1,13 @@
+#!/usr/bin/env bash


Would be best to avoid bashisms and use standard /bin/sh, no?

Also set -e all these please.

added set -e

OdyX · 2019-08-14T08:15:51Z

@@ -0,0 +1,59 @@
+#!/bin/bash


Not coherent with the /usr/bin/env calls elsewhere.

replaced with #!/usr/bin/env bash for now (see chat discussion)

Aerzas

We're missing a documentation or two to make it run smoothly for newcomers and we could maybe improve some structure but it's working well, thank you.

Aerzas · 2019-08-19T06:20:59Z

-    chmod 600 $PROJECT_NAME.key $PROJECT_NAME.pem
-fi
+docker run --rm -v $DIR/../:/generate/ -v $PONTSUN_DIR_ETC/certificates/:/certs/ -it liip/pontsun-helper:latest /generate/scripts/helper/docker-generate-certificates.sh $1
+if [[ "$OSTYPE" == "darwin"* ]]; then


If we start to support explicitly Mac, shouldn't we support other platforms also ?

Sure. Windows support is needed/wanted, too. But I need the help of our windows users here and something that can be added later.

We supported mac before anyway already (the source of this scripts and the docs/ at least ;))

smallstep CLI has a command to install certificates into truststores for windows, linux and macOS.
Respective code is at smallstep/truststore:

macOS

linux

windows

It's now included and used, if installed (didn't check on windows, but that's something to be added later)

It = smallstep CLI

Aerzas · 2019-08-19T06:26:42Z

+
+# Load env file
+set -a
+test -f $(dirname $0)/../../containers/.env && source $(dirname $0)/../../containers/.env


It is not specified in the README.md that we need to copy the .env file and without it it's not working by default. We should maybe warn the user if we don't find some env variables and stop the script with an explicit message.

It does that now. Plus I added some section to the README

Aerzas · 2019-08-19T06:35:00Z

-    cat $PROJECT_NAME.crt $PROJECT_NAME.key > $PROJECT_NAME.pem
-    chmod 600 $PROJECT_NAME.key $PROJECT_NAME.pem
-fi
+docker run --rm -v $DIR/../:/generate/ -v $PONTSUN_DIR_ETC/certificates/:/certs/ -it liip/pontsun-helper:latest /generate/scripts/helper/docker-generate-certificates.sh $1


By default the liip/pontsun-helper image doesn't exist, we should maybe integrate the build process of the image here or push this image to a repo so users only need to pull it here.

I'm still in the process of recovering our "liip" docker hub account, so that we can push it there (and then yes, why not add it into CI/CD)and people don't have to do anything.

Aerzas · 2019-08-19T06:47:10Z

@@ -0,0 +1,45 @@
+#!/usr/bin/env bash


For a single command that also exists as a script maybe this binary is not needed. Is there other commands that are planned to be added later ?

Yes, there will be more up and down for example (see #5 which is WIP for now)

One reason to do this was also, that we have to possibility to maybe move away from a shell script here and do it in go or whatever other language.

mweibel · 2019-12-04T15:31:15Z

what's the status here?
currently the documentation still refers to use USER_ID=$(id -u) docker-compose -f docker-compose.certificates.yml up for the certificates but that file doesn't exist.

// Edit: actually it does exist, I just didn't see it (and needs a cd containers). Will amend docs.

chregu force-pushed the certificate-generation-improvements branch from 01a2878 to c43800f Compare August 14, 2019 08:08

OdyX reviewed Aug 14, 2019

View reviewed changes

chregu changed the title ~~Better certificate generation and adding an "global" pontsun script~~ Better certificate generation and adding a "global" pontsun script Aug 14, 2019

chregu mentioned this pull request Aug 14, 2019

WIP: Add up and down command/scripts and make portainer optional #5

Closed

Aerzas reviewed Aug 19, 2019

View reviewed changes

Comment thread README.md Outdated

Aerzas suggested changes Aug 19, 2019

View reviewed changes

chregu force-pushed the certificate-generation-improvements branch 2 times, most recently from e9fefaa to 02ddea4 Compare August 20, 2019 09:46

Better certificate generation

cac4c2b

chregu force-pushed the certificate-generation-improvements branch from 02ddea4 to cac4c2b Compare August 20, 2019 09:47

chregu changed the title ~~Better certificate generation and adding a "global" pontsun script~~ Better certificate generation Aug 20, 2019

chregu mentioned this pull request Oct 22, 2019

Generate certificates via a container #16

Merged

		@@ -0,0 +1,5 @@
		#!/usr/bin/env bash
		DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"

Uh oh!

Conversation

chregu commented Aug 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

OdyX 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!

Uh oh!

Aerzas left a comment

Choose a reason for hiding this comment

Uh oh!

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!

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!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mweibel commented Dec 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

chregu commented Aug 14, 2019 •

edited

Loading

mweibel commented Dec 4, 2019 •

edited

Loading