voxpupuli / puppet-augeasproviders_pam

Augeas-based PAM type and provider for Puppet
Other
5 stars 21 forks source link

optional => true results in reconfig each time. #27

Open traylenator opened 4 years ago

traylenator commented 4 years ago

Versions:

The following configuration:

pam{'Add pam_afs_session to ssh stack':
  ensure    => 'present',
  service   => 'sshd',
  type      => 'session',
  optional  => true,
  control   => 'optional',
  module    => 'pam_afs_session.so',       
  position  => 'before module postlogin',
  arguments => ['always_aklog', 'debug'],
}

adds a line which looks correct to me:

-session optional pam_afs_session.so always_aklog debug

In particular the - prefix is added for the optional => true.

Subsequent runs of puppet result in:

Notice: /Stage[main]/Afs::Config/Pam[Add pam_afs_session to ssh stack]/optional: optional changed 'false' to 'true' (corrective)

the line itself is unchanged , augtool output.

/files/etc/pam.d/sshd/14
/files/etc/pam.d/sshd/14/optional
/files/etc/pam.d/sshd/14/type = "session"
/files/etc/pam.d/sshd/14/control = "optional"
/files/etc/pam.d/sshd/14/module = "pam_afs_session.so"
/files/etc/pam.d/sshd/14/argument[1] = "always_aklog"
/files/etc/pam.d/sshd/14/argument[2] = "debug"
KoenDierckx commented 3 years ago

We have the same issue Anyone know what causes this ?

KoenDierckx commented 3 years ago

I think the main cause is inside augeas.rb, inside the getter for optional, on line 132 When the /optional path is not there, the match is empty, so it will return true, meaning this is optional, while it should return false, this is not optional (and the other way around) So adding a NOT before transforming into a string and symbol seems to be needed ` define_aug_method(:optional) do |aug, resource|

aug.match("$resource/optional").empty?.to_s.to_sym

(!aug.match("$resource/optional").empty?).to_s.to_sym

end `

BUT, after fixing the getter, you might run into a bug with the assignment function define_aug_method!(:optional=) do |aug, resource, value| if resource[:optional] == :true if aug.match("$resource/optional").empty? aug.clear("$resource/optional") end else aug.rm("$resource/optional") end end

It works going from optional => true to optional=> false So the else case, that does the aug.rm($resource/optional) is ok

But it fails when you try to go from optional => false to optional=> true Normally the aug.clear("$resource/optional") should create a new path with an empty value, but it fails for me

` Debug: Puppet::Type::Pam::ProviderAugeas: Save failure details: /augeas/files/etc/pam.d/test/error/path = /files/etc/pam.d/test/files/etc/pam.d/test/1 /augeas/files/etc/pam.d/test/error/lens = /opt/puppetlabs/puppet/share/augeas/lenses/dist/pam.aug:63.21-.51: /augeas/files/etc/pam.d/test/error/message = Failed to match tree under /files/etc/pam.d/test/1

 { "type" = "account" }
 { "control" = "required" }
 { "module" = "test.so" }
 { "optional" }

with pattern

{ /optional/ }?
{ /type/ = /auth|session|account|password/ }
{ /control/ = /\\[[^]\n#]*\\]|[A-Za-z]+/ }
{ /module/ = /([^\t\n #\\]|\\\\.)+/ }
{ /argument/ = /\\[[^]\n#]+\\]|[^\t\n #[\\][^\t\n #\\]*/ }*
({ /#comment/ = /[^\t\n\r ].*[^\t\n\r ]|[^\t\n\r ]/ }
  | ())

`

perhaps someone more experienced with augeas and this module can investigate this further ?? @raphink @bastelfreak ?