Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix self.instances #13

Closed
wants to merge 6 commits into from
Closed

Fix self.instances #13

wants to merge 6 commits into from

Conversation

raphink
Copy link
Member

@raphink raphink commented Jan 21, 2016

No description provided.

@ndelic0
Copy link

ndelic0 commented Jan 21, 2016

While trying to query pam/su module keep getting absent status. It is the same for all pam resources.

# puppet resource pam 'Pam su/auth'
pam { 'Pam su/auth':
  ensure => 'absent',
}

cat /etc/pam.d/su
#%PAM-1.0
auth    sufficient      pam_rootok.so
auth    sufficient      pam_wheel.so
auth    required        pam_deny.so
account sufficient      pam_succeed_if.so uid = 0 use_uid quiet
account include         system-auth
password include       system-auth
session include         system-auth
session optional        pam_xauth.so

@crayfishx
Copy link

@raphink Any update to this one? This would be a really useful feature as it would allow the possibility of adding some purging capabilities but first the instances needs to return something sensible. Currently the exists? method on the instances returns false, I've been attempting to fix this today but ended up in a world of pain tracing this through all the augeas specific stuff - ping me privately if I can help with this.

@trevor-vaughan
Copy link
Contributor

@crayfishx Honestly, I would deliberately cripple purging from PAM unless you set a parameter. That's about the scariest thing I can think of.

resources = []
aug.match("$target/*[label()!='#comment']").each do |spath|
aug.match("/files/etc/pam.d/*//*[label()!='#comment']").each do |spath|
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want to deconflict between symlinks here or just list everything?

@crayfishx
Copy link

@trevor-vaughan I wasn't meaning native purging using the resources type, that would be scary, but once instances works then there is scope for writing a pam_service type that is able to purge non managed entries within the scope of that service only by comparing the properties of the existing entries to whats in the catalog. (this is a similar approach to the purge_rich_rules attribute of the firewalld_zone type in https://github.com/crayfishx/puppet-firewalld#firewalld-zones)

I could of course write this separately but it would make more sense to use the instances method of the pam provider.

The use case we have for this is an installer that places pam.d/ entries in a service, but we want those to be different. At the moment both the Puppet added rules and installer added rules all go into the mix, I want a way to purge a specific pam service of unmanaged entries.

@trevor-vaughan
Copy link
Contributor

@crayfishx So, is purging guaranteed to happen last? The problem that I always had with puppetlabs-firewall is that a failure somewhere in the catalog could leave your system inaccessible. It's one of the reasons that I keep using the simp-iptables module. There are some things that just need to be atomic.

@crayfishx
Copy link

@trevor-vaughan probably getting a bit off topic for this ticket :-) but this isn't a case that's been reported yet though it's worth looking into. If you can simulate a scenario feel free to raise a ticket over there -->

@trevor-vaughan
Copy link
Contributor

@crayfishx Fair enough. I did about 2 years ago. Sorry for the noise everyone!

@raphink
Copy link
Member Author

raphink commented Oct 17, 2018

Better late than never, let's finish this…

@coveralls
Copy link

coveralls commented Oct 17, 2018

Coverage Status

Coverage increased (+0.5%) to 87.919% when pulling 759db0b on raphink:instances into ed1e6ff on hercules-team:master.

@raphink
Copy link
Member Author

raphink commented Oct 17, 2018

@crayfishx is that good for you?

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants