Bug #2141

Augeas resource bug with single and double quotes

Added by Avi Miller almost 3 years ago. Updated over 2 years ago.

Status:Closed Start date:04/07/2009
Priority:Urgent Due date:
Assignee:Bryan Kearney % Done:

0%
Category:-
Target version:0.25.0
Affected Puppet version:0.24.8 Branch:
Keywords:
Votes: 0

Description

In Puppet 0.24.7, the following Augeas resource worked fine:

augeas { "pam_remove_console":
   context => "/files/etc/pam.d/",
   changes => "rm */*[module='pam_console.so']",
   onlyif  => "match */*[module='pam_console.so'] size > 0",
}

I’ve upgraded to Puppet 0.24.8 and I’m now getting the following error:

(//Node[basenode]/pam/Augeas[pam_remove_console]) Failed to retrieve current state of resource: Error sending command 'rm */*[module=' with params ["pam_console.so", "]"]/Command 'rm */*[module=' is not supported

Looks like Puppet is breaking on the ‘ even though I opened with “ — and escaping the ’ doesn’t help.

scanner.rb (2.7 kB) David Lutterkort, 04/08/2009 09:40 pm

History

#1
Updated by James Turnbull almost 3 years ago

  • Status changed from Unreviewed to Accepted
  • Assignee set to Bryan Kearney

#2
Updated by David Lutterkort almost 3 years ago

The attached file is a standalone program that fixes the problems with the current scanner for the changes commands. The parse_commands method should be a drop-in for the current method in the augeas provider.

It would be great to put the tests into some sort of unit tests.

#3
Updated by Bryan Kearney almost 3 years ago

  • Status changed from Accepted to In Topic Branch Pending Review

I have sent a pull reuqest for the following branch:

http://github.com/bkearney/puppet/commits/ticket/0.24.x/2141

Please test this branch and let me know if it works.

#4
Updated by Bryan Kearney almost 3 years ago

Please be aware there are some subtle changes in the type which may cause issues for you. These are:

1) There can be no ‘ or “ characters around path elements. 2) The context is now only prepended to relative paths. So, if a path is specified as /foo/bar the context will not be appended. It will only be appended if it is foo/bar 3) The syntax for array matching in the onlyif is now eq or noteq. It was == before. 4) The get and set commnands used to concatenate all items at the end of a string so "set path to some value” would be interpreted as “set” “path” “to some value”. This is no longer supported. The caller must put ’ or “ around the "to some value” for it to work"

#5
Updated by James Turnbull almost 3 years ago

  • Status changed from In Topic Branch Pending Review to Closed
  • Target version set to 0.25.0

Pushed in commit:9a264218419fdb0164246366de79ac65acee63cc in branch 0.24.x

#6
Updated by James Turnbull over 2 years ago

  • Status changed from Closed to Re-opened

#7
Updated by David Lutterkort over 2 years ago

@Bryan: why did you change the syntax for ‘==’ to ‘eq’ and ‘noteq’ ? Can’t we keep ‘==’ and add ‘!=’ ? I am concerned about breaking people’s manifest with the ‘==’ –> ‘eq’ transition.

#8
Updated by David Lutterkort over 2 years ago

I’ve rebased the patch and added a few additional code cleanups.

The result is in the dev/2141 branch at git://git.et.redhat.com/puppet.git (http://git.et.redhat.com/?p=puppet.git;a=shortlog;h=refs/heads/dev/2141)

I’d be more than happy to condense the patch series into fewer patches if wanted (I kept them very fine grained to make them more reviewable)

Also, let me know if you want me to rebase again if master changes

#9
Updated by Bryan Kearney over 2 years ago

@lutter: There was a conflict in the parsing logic with comparators (== and !=)> I needed a glob after them, the the current parsing logic would not support that.

let me take a look at lutters patches this morning.

#10
Updated by Bryan Kearney over 2 years ago

Just checking.. why was it moved to re-opened? The example works with the submitted code, which is destined for 0.25. Is the goal to provide a patch for 24.8? Let me know.

Lutters code has some clean up stuff in it, but I would rather treat that is seperate items for a future release as opposed to tieing them to this bug.

— bk

#11
Updated by David Lutterkort over 2 years ago

The bug was only reopened to track that the patch for this is in 0.24.x, but not master – I then kinda got carried away with some code cleanup

#12
Updated by David Lutterkort over 2 years ago

I modified my patch series to address two additional issue:

  • Special characters in paths can be escaped by preceding them with a backslash (“commit e6794016”:http://git.et.redhat.com/?p=puppet.git;a=commit;h=e6794016a646d84704e5c39d96993e8fbc146bde)
  • Change the syntax for match comparison back to use ‘==’/‘!=’ instead of ‘eq’/‘noteq’ (“commit cd36949b”:http://git.et.redhat.com/?p=puppet.git;a=commit;h=cd36949bd03c50951fdf775e6b5252cf94a0d618)

Put the result in the branch “dev/2141-2”:http://git.et.redhat.com/?p=puppet.git;a=shortlog;h=refs/heads/dev/2141-2

#13
Updated by David Lutterkort over 2 years ago

  • Status changed from Re-opened to Ready For Checkin

#14
Updated by Bryan Kearney over 2 years ago

+1 for the changes. They all look good. I am having gem errors so I can not run the rpsec tests, but my test scripts worked. Ship it.

#15
Updated by James Turnbull over 2 years ago

  • Status changed from Ready For Checkin to Closed

Pushed in commit:7c5125bb6dec630d2509905e3c202e0dfe59d959 et al in branch master.

Also available in: Atom PDF