theforeman / puppet-foreman_proxy

Puppet module for Foreman Smart Proxy
GNU General Public License v3.0
43 stars 130 forks source link

Update Salt Master configuration template #839

Open bastian-src opened 4 months ago

bastian-src commented 4 months ago

→ This is more of an pro-active change. I don't quite know where exactly we run into an issue with cherrypy-API permissions. So, I need a second opinion on this.

→ This is motivated by the following change:

Since 3006, Salt installs with an explicit user that runs the Salt Master daemon in order to limit the daemon permissions. If that users differs from the user that we execute the salt ... commands with (when triggering Salt from Smart Proxy), we get some permission issues. More specifically, sudo -u foreman-proxy salt '*' test.ping runs into the following error:

[ERROR   ] Unable to connect to the salt master publisher at /var/run/salt/master
The salt master could not be contacted. Is master running?

When executing salt '*' test.ping, that comand wants to connect to the Salt Master whose PID is stored in /var/run/salt/master/*. But, with the 3006-explicit-user update, they limit access to the /var/run/salt folder to the same user which runs the Salt Master daemon.

Therefore, I see two options:

A) Make the Salt Master daemon run with the same user that we execute the salt ... commands with (currently implemented in this PR) B) Take care that the foreman-proxy user has the necessary permissions/adapt the folder permissions that the foreman-proxy user can execute salt ... even if the daemon is started by a different user.

maximiliankolb commented 2 months ago

Thanks for the ping. Making sure that the docs work after we've merged this is definitely on my plate!

@sbernhard Is there any use case to configure the interfaces? Or are these two simply required to make Salt+Foreman work?

docs: https://docs.saltproject.io/en/master/topics/netapi/netapi-enable-clients.html#enabling-netapi-client-interfaces

maximiliankolb commented 1 week ago

@sbernhard Friendly reminder.

bastian-src commented 12 hours ago

Coming back to this, thanks @ekohl for the review and feedback!

I totally agree, we should re-think the user management and I think since Salt switches to a non-root user we can take this as reason to update ours too.

However, for this PR, I'd just update the master configuration. I'm currently testing which default config we should apply for netapi to guarantee that the salt-api works for Foreman.