ensure=>'positioned' doesn't work with position=>'before first'

[alexjfisher]

pam { "test":
  ensure    => 'positioned',
  service   => 'password-auth',
  type      => 'account',
  control   => 'sufficient',
  module    => 'pam_foo.so',
  arguments => ['use_first_pass', 'foo'],
  position  => 'before first',
}

won't reorder password-auth when pam_foo.so isn't the first account entry.
Unless the pam entry is the last entry in a section,
https://github.com/hercules-team/augeasproviders_pam/blob/master/lib/puppet/provider/pam/augeas.rb#L74 will find a match even if we're not first in the section.

Specifically, the match will be the 'first' entry that follows our entry, and in_position? will erroneously return true.

[asasfu]

I have a patch for consideration for this and can pop open a PR but should probably get a review and it caused me to notice a bug in the lookup in augeas itself. hercules-team/augeas#317

The patch is as follows:

  def in_position?
    unless resource[:position].nil?
      path, before = self.class.position_path(resource[:position], resource[:type])

      mpath = "#{resource_path}[preceding-sibling::#{path}]"

      augopen do |aug|
        augmatch = aug.match(mpath).empty?
        augmatch = before == 'before' ? augmatch : !augmatch
      end
    end
  end

It removes the use of following-sibling as it didn't seem to work properly for this case and then applies an inverse on the match .empty? if you said you wanted it after.
There may be a more ruby-esque way to do that augmatch = before...?... selector line possibly? but it works.

I tested it with [preceding-sibling::*[type='auth'][1]] 1 through 6 (6 being the last item in auth section) ( position => 'before *[type='auth'][1]' )and it seemed to function exactly as it was supposed to, which it didn't before with the current implementation as stated by @alexjfisher

The bug described before seems to affect the current implementation and the new code pasted above, when using last() because following-sibling and preceding-sibling both don't translate last() and effectively return 1 which is not the item you want. If you put after ...[last()] you're effectively putting after ...[1] no matter how many items are in there and it will always comment that it is out of position but then not change it since it is in the correct spot.

[raphink]

@asasfu can you provide a PR with a unit test fixed by your code please?

[BarnumD]

Any fix for this? I'm still running into it.