Skip to content

Conversation

@johnwarburton
Copy link

Hi
Please find a PR related to issue #75
Regards

John

Copy link
Member

@rwaffen rwaffen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, maybe add a comment why we don't do set -e in this script.
so that nobody adds this back.

@lbetz do you have an opinion on this or maybe how to do it differently instead of just removing the set command?

@rwaffen rwaffen added the bug Something isn't working label Sep 23, 2025
@shaun-rutherford
Copy link

set -e just causes the script to fail immediately if there is a non-zero return on a command.
Removing it IMO is covering up what is actually broken in the script with the Kube setup.

@shaun-rutherford
Copy link

I think the real issue for the user is that the mount is read-only and so the script can't change permissions anyway.

Is there no way to mount in the auth.conf file as read/write? I'm kinda failing to see how this change would fix the issue (albeit maybe I'm wrong?)

@shaun-rutherford
Copy link

With all that said, we've had to make changes to this file as well in order to make it work on docker swarm as our ssl data is on an NFS share and doing the chown -R causes file locking issues.

I would argue we should actually add commands that only change the ownership of files that actually need to be owned as puppet (and leave everything else owned as root, which would be better security).

Example of what we are doing internally:

find /etc/puppetlabs/puppet/ssl/certificate_requests ! -user puppet -print | while read i; do chown puppet:puppet $i; done
find /etc/puppetlabs/puppet/ssl/certs ! -user puppet -print | while read i; do chown puppet:puppet $i; done
find /etc/puppetlabs/puppet/ssl/private ! -user puppet -print | while read i; do chown puppet:puppet $i; done
find /etc/puppetlabs/puppet/ssl/private_keys ! -user puppet -print | while read i; do chown puppet:puppet $i; done
find /etc/puppetlabs/puppet/ssl/public_keys ! -user puppet -print | while read i; do chown puppet:puppet $i; done
find /etc/puppetlabs/puppet/ssl/crl.pem ! -user puppet -print | while read i; do chown puppet:puppet $i; done
find /opt/puppetlabs/server/data/puppetserver/yaml/facts/*.yaml ! -user puppet -type f -print | while read i; do chown puppet:puppet $i; done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants