puppetlabs / pupperware

Container fun time lives here.
Other
183 stars 67 forks source link

(REPLATS-150) Set service uid/gid labels for cert dirs #252

Closed jpartlow closed 3 years ago

jpartlow commented 3 years ago

The SpecHelpers import preset certs to speed up start up time. To do this they use a trick of composing the containers with --no-start first so their entrypoints don't run but volumes are created, then running a base alpine container with the data dir volume mounted and with the preset certs mounted, and then copying over the certs and setting permissions.

All of that is fine, but the act of mounting the datadir in alpine under /opt (which is owned by root in that container) ends up setting the host volume path's uid to root as well, which then later interferes with the container's service user interacting with the datadir when the actual service container is brought up with the volume mounted.

With this patch, the mount point is instead chowned with the labeled uid/gid, ensuring the volume root continues to be owned by the service user. The service ids match those set in the individual service container repo Dockerfiles.

Iristyle commented 3 years ago

So I think this one is going to require a little coordination, right? i.e. once we merge this, none of the cert preloading works anymore until all the containers are updated / shipped.

One thing we could do temporarily is get https://github.com/puppetlabs/pupperware/pull/253 in, then turn cert preloading off temporarily as you're putting up your PRs. Then once all the containers are updated, we could merge this one in... then turn the preloading back on. Otherwise, I think we're going to end up with a lot of red on PRs

Iristyle commented 3 years ago

One other thought is to change the helper method and instead of defaulting to root:root, inspect the target container metadata and use its uid:gid when it has them, otherwise fall back to root:root.

That just might work... let me look into that tomorrow as a more flexible solution so we don't have to do the PR dance.

jpartlow commented 3 years ago

It would be good to double check, but I don't think this impacts root containers. Locally, the test suite passed when run against either a service user owned container (so, with one of my prs) or a root owned container, and the supporting containers that get pulled in by docker-compose are still root atm. Given that the containers run as root, I don't think it matters whether the test suite modifies the cert dir ownership.

Iristyle commented 3 years ago

Yeah I'm pretty sure you're right - I was probably at the keyboard a bit too long yesterday :(

We can easily test this against pupperware-commercial repo. I'll throw up a PR in a bit. I think it may still be worth investigating automatically determining uid/gid so as not have to specify them like this, but we can always get this PR in sooner as well.

Iristyle commented 3 years ago

From the logs in the pupperware-commercial at https://travis-ci.com/github/puppetlabs/pupperware-commercial/builds/222537843 for test PR https://github.com/puppetlabs/pupperware-commercial/pull/81 (where all the tests passed)

Pre-loading certificates for service pe-bolt-server
Copying existing files through transient container:
  from         : /home/travis/build/puppetlabs/pupperware-commercial/.bundle/gems/ruby/2.6.0/bundler/gems/pupperware-6cd02315e791/gem/lib/pupperware/certs/pe-bolt-server
  to volume    : pupperware-commercial_bolt-server/certs
  with uid:gid : 10009:10009
Unable to find image 'alpine:3.10' locally
3.10: Pulling from library/alpine
bfdacc68c91b: Pulling fs layer
bfdacc68c91b: Verifying Checksum
bfdacc68c91b: Download complete
bfdacc68c91b: Pull complete
Digest: sha256:4929878917a37edf0b7f69594552508a3432fe304335dd92be29bbaa839362ed
Status: Downloaded newer image for alpine:3.10
Pre-loading certificates for service pe-console-services
Copying existing files through transient container:
  from         : /home/travis/build/puppetlabs/pupperware-commercial/.bundle/gems/ruby/2.6.0/bundler/gems/pupperware-6cd02315e791/gem/lib/pupperware/certs/pe-console-services
  to volume    : pupperware-commercial_console-services/certs
  with uid:gid : 10007:10007
Pre-loading certificates for service pe-orchestration-services
Copying existing files through transient container:
  from         : /home/travis/build/puppetlabs/pupperware-commercial/.bundle/gems/ruby/2.6.0/bundler/gems/pupperware-6cd02315e791/gem/lib/pupperware/certs/pe-orchestration-services
  to volume    : pupperware-commercial_orchestration-services/certs
  with uid:gid : 10009:10009
Pre-loading certificates for service postgres
Copying existing files through transient container:
  from         : /home/travis/build/puppetlabs/pupperware-commercial/.bundle/gems/ruby/2.6.0/bundler/gems/pupperware-6cd02315e791/gem/lib/pupperware/certs/postgres
  to volume    : pupperware-commercial_puppetdb-postgres/certs
  with uid:gid : 999:999
Pre-loading certificates for service puppet
Copying existing files through transient container:
  from         : /home/travis/build/puppetlabs/pupperware-commercial/.bundle/gems/ruby/2.6.0/bundler/gems/pupperware-6cd02315e791/gem/lib/pupperware/certs/puppet
  to volume    : pupperware-commercial_puppetserver/
  with uid:gid : 10006:10006
Pre-loading certificates for service puppetdb
Copying existing files through transient container:
  from         : /home/travis/build/puppetlabs/pupperware-commercial/.bundle/gems/ruby/2.6.0/bundler/gems/pupperware-6cd02315e791/gem/lib/pupperware/certs/puppetdb
  to volume    : pupperware-commercial_puppetdb/certs
  with uid:gid : 10008:10008

It would appear that you are correct and this is all working as it should. Thanks for setting me straight @jpartlow! Merging!