(maint) Handle new CA path

[justinstoller]

No description provided.

[justinstoller]

This lets the docker entry-point understand the difference between the new and old CA dirs. I believe by exposing the CADIR as an env var it will also be configurable(??) I think in addition to this pupperware should be updated to have a puppet-certs volume label and the docs should be updated to call out the Puppet 7 change and importance of mounting place to keep your certs.

However, this is my first change to the repo and I don't use it much. I'm not sure the best way to test this change or it in concert with any commensurate pupperware changes. Hoping to get some advice from @Iristyle on that.

[mwaggett]

I know we recently talked about how you don't need the --config flag to run this, but I noticed that we do pass the --config flag for the setup command. Is the setup command just being super explicit, or are we doing some custom thing that means we should pass it here too?

[justinstoller]

The setup command is, afaict, being super explicit. However, that branch also writes to that config file so it seemed fine to leave it explicit. We don't touch the config as part of the migration and it seemed fine to leave it implicit there. But I'm happy either removing the setup's --config flag usage or adding it here.

[justinstoller]

Love to get a review from @Iristyle

[Iristyle]

Do we expect users to change this here?

If not, we've been trying to move ENV vars out of the Dockerfile because it signals something that can / should be touched by end users. Instead, better to just embed it in the script where it's used (in this case maybe do a puppet config print to retrieve it?)

Puppetserver is the one container that's a bit out of date with the "best practices" we've been trying to put in everywhere -- partially because its the first container, and partially b/c there are a lot of practices to unwind that people may be relying on b/c we have a lot of active users.

[Iristyle]

I don't think the file test will ever pass if the directory isn't there, so do we need the -d?

[Iristyle]

Also, I think these variables have slightly confusing names since they're named opposite of what I would expect. Maybe do something like this to flip the exit code:

new_ca_exists=$([ -f "$CADIR/ca_crt.pem" ] && echo 1)
old_ca_exists=$([ -f "$SSLDIR/ca/ca_crt.pem" ] && echo 1)

So that then you can use the the -n (not null) and -z (null) tests in shell code like:

# no existing CA
if [ -z "$new_ca_exists" ] && [ -z "$old_ca_exists" ]; then
# legacy CA upgrade
elif [ -z "$new_ca_exists" ] && [ -n "$old_ca_exists" ]; then
fi

But it also wouldn't be terrible to just inline the tests either and avoid the named variables b/c at least -f and -d are easier to remember shell tests than -n and -z:

# no existing CA
if [ ! -f "$CADIR/ca_crt.pem" ] && [ ! -f "$SSLDIR/ca/ca_crt.pem" ]; then
# legacy CA upgrade
elif [ ! -f "$CADIR/ca_crt.pem" ] && [ -f "$SSLDIR/ca/ca_crt.pem" ]; then
fi

[justinstoller]

Sorry for the premature squash! I adopted all of the recommendations that came from review.

This PR will require this Pupperware PR first: puppetlabs/pupperware#272

One can test this by pulling down Pupperware and applying the above PR.

• Export the PUPPETSERVER_IMAGE as something from 6.x. I used: export PUPPETSERVER_IMAGE=puppet/puppetserver:6.14.1.
• Export PUPPET_DNS_ALT_NAMES= your host name.
• Then spin up a Pupperware cluster using docker-compose.
• Have a local agent check into the Puppetware cluster (from the puppet source repo): bundle exec puppet agent -t --server <your host name> (you may need to delete ~/.puppetlabs for this to work).
• Validate that your agent's cert is in ~/.local/share/docker/volumes/compose-services_puppetserver-config/_data/ssl/ca/signed/ and that ~/.local/share/docker/volumes/compose-services_puppetserver-config2/_data/ca does not exist.
• Leave your cluster up, and build this PR and give it an unambiguous version.
• Set the PUPPETSERVER_IMAGE variable to the new image you've created and run docker-compose up -d. This should trigger the new image to migrate the CA.
• Check your local agent back into the server container and validate that it does not re-request a cert.
• Validate that ~/.local/share/docker/volumes/compose-services_puppetserver-config/_data/ssl/ca/ is now a symlink and that ~/.local/share/docker/volumes/compose-services_puppetserver-config2/_data/ca contains the contents that were in the old ca dir.