file.uncomment does not work as expected and documented

[rhertzog]

I have a file /etc/apache2/conf.d/charset which contains:

#AddDefaultCharset UTF-8

I have setup a state to uncomment this:

/etc/apache2/conf.d/charset:
  file.uncomment:
    - regex: AddDefaultCharset UTF-8
    - watch_in:
      - service: apache2
    - require:
      - pkg: apache2

But when run, the state returns back that the file already contains the uncommented line while this is clearly not the case:

    State: - file
    Name:      /etc/apache2/conf.d/charset
    Function:  uncomment
        Result:    True
        Comment:   Pattern already uncommented
        Changes:   

The code only checks that the provided regex matches apparently which doesn't make sense. It should use a more elaborated regex that ensures that the first (non-white) character is not a comment marker.

Then I thought that maybe we were supposed to include a leading ^ that is supposed to get stripped (as documented) but then it's not stripped and the regex fails to find a line to uncomment...

Something is really fishy in the whole logic of this function.

[thatch45]

comment and uncomment have been problem children, we will of course work on this, but I am going to suggest using file.managed

[s0undt3ch]

@rhertzog could you please provide the output of salt --versions-report?

[rhertzog]

@s0undt3ch sure:

$ salt --versions-report
           Salt: 0.10.5
         Python: 2.7.3rc2 (default, Apr 22 2012, 22:30:17)
         Jinja2: 2.6
       M2Crypto: 0.21.1
 msgpack-python: 0.1.10
   msgpack-pure: not installed
       pycrypto: 2.6
         PyYAML: 3.10
          PyZMQ: 2.2.0

[inthecloud247]

Also experiencing a similar issue:

$ salt --versions-report
           Salt: 0.12.1
         Python: 2.7.3 (default, Aug  1 2012, 05:14:39)
         Jinja2: 2.6
       M2Crypto: 0.21.1
 msgpack-python: 0.1.10
   msgpack-pure: not installed
       pycrypto: 2.4.1
         PyYAML: 3.10
          PyZMQ: 2.2.0.1

here's the state I created that doesn't work correctly:

cron-logging:
  file.uncomment:
    - name: /etc/rsyslog.d/50-default.conf
    - regex: 'cron\.\*'
    - char: '#'

the beginning of my /etc/rsyslog.d/50-default.conf file:

#  Default rules for rsyslog.
#
#                       For more information see rsyslog.conf(5) and /etc/rsyslog.conf

#
# First some standard log files.  Log by facility.
#
auth,authpriv.*                 /var/log/auth.log
*.*;auth,authpriv.none          -/var/log/syslog
#cron.*                                /var/log/cron.log
#daemon.*                       -/var/log/daemon.log
kern.*                          -/var/log/kern.log
#lpr.*                          -/var/log/lpr.log
mail.*                          -/var/log/mail.log
#user.*                         -/var/log/user.log

result:

    State: - file
    Name:      /etc/rsyslog.d/50-default.conf
    Function:  uncomment
        Result:    True
        Comment:   Pattern already uncommented
        Changes:  

But when I switch to use file.comment, it adds another '#' to the correct line. So it does seem to be identifying the correct line here. It's just not removing the comment marker properly.

[basepi]

This should be fixed by #4015. Can anyone verify? (once it gets merged, of course)

[inthecloud247]

hm are there any functionality tests for the module that could also help verify if it's working as expected?

[basepi]

Yes, there are state tests in place for it. Plus I did my own engineering tests.

[inthecloud247]

cool :-)

On Wed, Mar 13, 2013 at 9:25 AM, Colton Myers notifications@github.comwrote:

Yes, there are state tests in place for it. Plus I did my own engineering
tests.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/2875#issuecomment-14851811
.