puppetlabs / puppetserver-helm-chart

The Helm Chart for Puppet Server
Apache License 2.0
51 stars 56 forks source link

chown: changing ownership of '/etc/puppetlabs/puppetserver/': Operation not permitted #160

Closed antwacky closed 1 year ago

antwacky commented 1 year ago

Describe the Bug

The perms-and-dirs fails with:

chown: changing ownership of '/etc/puppetlabs/puppetserver/': Operation not permitted

On lots of directories.

As does the puppetserver container, e.g.

chown: changing ownership of '/etc/puppetlabs/puppetserver/conf.d/webserver.conf': Operation not permitted

Expected Behavior

The containers should start.

Steps to Reproduce

Deploy the helm chart with the following values (using longhorn as the PVC storage class):

puppetserver:
  puppeturl: git@gitlab.com/test/puppet.git
r10k:
  code:
    viaSsh:
      credentials:
        existingSecret: puppet-gitlab-ssh
storage:
  storageClass: longhorn
postgresql:
  global:
    storageClass: longhorn

Environment

Additional Context

Nada

antwacky commented 1 year ago

Seems to be changing the user that's the problem, if I change the puppet server perms-and-dirs args as below (using root as the user instead of puppet):

        - mkdir -p /etc/puppetlabs/puppet/eyaml/keys; mkdir -p /etc/puppetlabs/code/environments;
          mkdir -p /etc/puppetlabs/puppet/manifests; mkdir -p /etc/puppetlabs/code/r10k_cache;
          chown -R root:puppet /etc/puppetlabs; cp /tmp/puppet/configmap/r10k_code_entrypoint.sh
          /etc/puppetlabs/puppet/r10k_code_entrypoint.sh; cp /tmp/puppet/configmap/r10k_code_cronjob.sh
          /etc/puppetlabs/puppet/r10k_code_cronjob.sh; cp /tmp/puppet/configmap/r10k_code.yaml
          /etc/puppetlabs/puppet/r10k_code.yaml; chown puppet:puppet /etc/puppetlabs/puppet/r10k_code_entrypoint.sh
          /etc/puppetlabs/puppet/r10k_code_cronjob.sh /etc/puppetlabs/puppet/r10k_code.yaml;
          chmod +x /etc/puppetlabs/puppet/r10k_code_entrypoint.sh /etc/puppetlabs/puppet/r10k_code_cronjob.sh;
          cp /tmp/puppet/configmap/site.pp /etc/puppetlabs/puppet/manifests/site.pp;
          chown root:puppet /etc/puppetlabs/puppet/manifests/site.pp; mkdir -p /opt/puppetlabs/server/data/puppetserver/dropsonde/bin/;
          touch /opt/puppetlabs/server/data/puppetserver/dropsonde/bin/dropsonde;
          chown root:puppet -R /opt/puppetlabs/server/data/puppetserver/;

It get's slightly further, although there's similar logic within the entry point scripts of other containers preventing it from starting with similar errors.

davidphay commented 1 year ago

Hey @pagey101 - thanks for the bug report!

please feel free to send our way a PR - I'll be happy to review and merge it, afterwards.

Just 1 advice, don't replace directly the permission, add a new param in values.yaml like global.chown: puppet:puppet then you can override it in your own values.yaml and it does not affect everybody.

chwehrli commented 1 year ago

I had the same issue when installing 7.4.0 (on Azure AKS running k8s 1.24.9). What helped was to rename/overwrite capabilities from 'CAP_FOO' to just 'FOO' - here's from my values file:

values:
  puppetserver:
    securityContext:
      capabilities:
        add:
          - CHOWN
          - SETUID
          - SETGID
          - DAC_OVERRIDE
          - AUDIT_WRITE
          - FOWNER
  puppetdb:
    securityContext:
      capabilities:
        add:
          - FOWNER
          - CHOWN
          - SETUID
          - SETGID
          - DAC_OVERRIDE

Unfortunately this doesn't work for perms-and-dirs init-container as there capabilities cannot be overwritten by values file. If I change it manually then whole puppet stack comes up.

antwacky commented 1 year ago

Fantastic thank you, I removed the capabilities.drop: [all] to test and this is the issue.

I might submit a PR to allow overriding of the capabilities later.

davidphay commented 1 year ago

can you try to add this ?

values:
  puppetserver:
    securityContext:
      capabilities:
        add:
        - CAP_CHOWN
        - CAP_SETUID
        - CAP_SETGID
        - CAP_DAC_OVERRIDE
        - CAP_AUDIT_WRITE
        - CAP_FOWNER
        - CHOWN
        - SETUID
        - SETGID
        - DAC_OVERRIDE
        - AUDIT_WRITE
        - FOWNER
  puppetdb:
    securityContext:
      capabilities:
        add:
          - CAP_FOWNER
          - CAP_CHOWN
          - CAP_SETUID
          - CAP_SETGID
          - CAP_DAC_OVERRIDE
          - FOWNER
          - CHOWN
          - SETUID
          - SETGID
          - DAC_OVERRIDE

If it work we can add it by default

antwacky commented 1 year ago

I've added the suggested values, and it works to an extent.

As @chwehrli mentioned, the capabilities on the perms-and-dirs container are not parameterised. I've fixed this with a kustomisation patch:

patches:

  - patch: |-
      - op: replace
        path: /spec/template/spec/initContainers/0/securityContext/capabilities
        value:
          add:
            - CHOWN
            - SETUID
            - SETGID
            - DAC_OVERRIDE
            - AUDIT_WRITE
            - FOWNER
    target:
      kind: Deployment
      name: puppetserver-puppetserver-master
davidphay commented 1 year ago

I'll add the fix in the next release, thanks for the help !

davidphay commented 1 year ago

@pagey101 can you test the latest release please and tell me if the issue is fixed ? https://github.com/puppetlabs/puppetserver-helm-chart/pull/162

davidphay commented 1 year ago

@chwehrli can you test it too please

davidphay commented 1 year ago

I close the issue, feel free to open it again if it not work as expected