file.comment uses unanchored multiline search #54260

OrangeDog · 2019-08-20T09:09:29Z

Description of Issue

When checking for unanchor_regex, salt uses a multiline search. This may cause incorrect results if e.g. . or \s are used at the start of the pattern.

    # Make sure the pattern appears in the file before continuing
    if commented or not __salt__['file.search'](name, regex, multiline=True):
        if __salt__['file.search'](name, unanchor_regex, multiline=True):
            ret['comment'] = 'Pattern already commented'
            ret['result'] = True
            return ret
        else:
            return _error(ret, '{0}: Pattern not found'.format(unanchor_regex))

Setup

State:

/etc/config_file:
  file.comment:
    - regex: '^\s*line to comment'

Target file:

## Section header ##
line to comment

Steps to Reproduce Issue

Running the above state incorrectly returns "Pattern already commented".

Versions Report

Salt Version:
           Salt: 2018.3.4

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.6.1
      docker-py: Not Installed
          gitdb: 2.0.3
      gitpython: 2.1.8
          ioflo: Not Installed
         Jinja2: 2.10
        libgit2: 0.26.0
        libnacl: Not Installed
       M2Crypto: 0.32.0
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: 0.26.2
         Python: 3.6.8 (default, Jan 14 2019, 11:02:34)
   python-gnupg: 0.4.1
         PyYAML: 3.12
          PyZMQ: 16.0.2
           RAET: Not Installed
          smmap: 2.0.3
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.2.5

System Versions:
           dist: Ubuntu 18.04 bionic
         locale: ISO-8859-1
        machine: x86_64
        release: 4.15.0-58-generic
         system: Linux
        version: Ubuntu 18.04 bionic

The text was updated successfully, but these errors were encountered:

frogunder · 2019-08-20T17:38:39Z

@OrangeDog Thank you for reporting this issue.

I am seeing the same behavior.

Thanks.

stale · 2020-01-08T01:05:31Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

OrangeDog · 2020-01-08T10:38:35Z

This (high severity) issue still exists.

stale · 2020-01-08T10:38:37Z

Thank you for updating this issue. It is no longer marked as stale.

DmitryKuzmenko · 2020-02-13T17:57:40Z

Fixed by #51988 and released with Salt 3000

OrangeDog · 2020-02-14T11:19:49Z

@DmitryKuzmenko it doesn't look fixed to me. Same behaviour as before.

$ salt-call --local test.version
local:
    3000
$ salt-call --local state.low '{
    "__id__": "id",
    "state": "file",
    "fun":"comment",
    "name": "/tmp/test_file",
    "regex": "^\\s*line to comment"
}'
local:
    ----------
    __id__:
        id
    __run_num__:
        0
    __sls__:
        None
    changes:
        ----------
    comment:
        Pattern already commented
    duration:
        13.164
    name:
        /tmp/test_file
    result:
        True
    start_time:
        11:15:40.017098

OrangeDog · 2020-02-14T11:26:40Z

~~@DmitryKuzmenko that PR was never released. Check the project board.~~

My mistake. The board search doesn't work for 51988, it has to be #51988.

DmitryKuzmenko · 2020-02-17T11:19:07Z

I'm sorry if I'm wrong but this works for me and v3000 contains this patch

$ git tag --contains 4ada8e734687402d03111eba951280f4b54152fe
v3000
v3000.0rc1
v3000.0rc2
v3000_docs

OrangeDog · 2020-02-17T11:20:31Z

@DmitryKuzmenko are you saying you can no longer reproduce the issue, or just that the commit was released?
If the latter, then I guess it didn't fix this issue.

DmitryKuzmenko · 2020-02-17T12:52:49Z

Hm... It looks a bit confusing just because it looks fixed on my side...

$ cat /tmp/test_file                      
## Section header ##
line to comment
$ ./scripts/salt-call --local state.low '{
    "__id__": "id",
    "state": "file",
    "fun":"comment",  
    "name": "/tmp/test_file",
    "regex": "^\\s*line to comment"
}'
local:
    ----------
    __id__:
        id
    __run_num__:
        0
    __sls__:
        None
    changes:
        ----------
        diff:
            --- 
            +++ 
            @@ -1,2 +1,2 @@
             ## Section header ##
            -line to comment
            +#line to comment
    comment:
        Commented lines successfully
    duration:
        8.939
    name:
        /tmp/test_file
    result:
        True
    start_time:
        15:51:23.393362
$ cat /tmp/test_file
## Section header ##
#line to comment
$ ./scripts/salt-call --local state.low '{
    "__id__": "id",
    "state": "file",
    "fun":"uncomment",
    "name": "/tmp/test_file",
    "regex": "^\\s*line to comment"
}'
local:
    ----------
    __id__:
        id
    __run_num__:
        0
    __sls__:
        None
    changes:
        ----------
        diff:
            --- 
            +++ 
            @@ -1,2 +1,2 @@
             ## Section header ##
            -#line to comment
            +line to comment
    comment:
        Uncommented lines successfully
    duration:
        7.494
    name:
        /tmp/test_file
    result:
        True
    start_time:
        15:51:35.454219
$ cat /tmp/test_file
## Section header ##
line to comment
$ ./scripts/salt-call --local test.version
local:
    3000

OrangeDog · 2020-02-17T13:00:37Z

That is odd. Full versions report for where I tested it:

Salt Version:
           Salt: 3000

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
         Jinja2: 2.11.1
        libgit2: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.6.2
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 3.6.9 (default, Nov  7 2019, 10:44:02)
   python-gnupg: Not Installed
         PyYAML: 5.3
          PyZMQ: 18.1.1
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.3.2

System Versions:
           dist: Ubuntu 18.04 bionic
         locale: UTF-8
        machine: x86_64
        release: 4.4.0-18362-Microsoft
         system: Linux
        version: Ubuntu 18.04 bionic

DmitryKuzmenko · 2020-02-18T11:52:23Z

@OrangeDog to consolidate our environments do you have an ability to try to reproduce this on a clean salt environment with a simple minion config like

master: master_ip
id: minion_id

with this sls file:

$ cat /srv/salt/base/issue_54260.sls 
edit_file:
  file.managed:
    - name: /tmp/test
    - contents: |
        ### Section header ###
        line to comment

comment_file:
  file.comment:
    - name: /tmp/test
    - regex: "^\\s*line to comment"

uncomment_file:
  file.uncomment:
    - name: /tmp/test
    - regex: "^\\s*line to comment"

My results are the following:

$ ./scripts/salt -l critical alpha state.apply issue_54260
alpha:
----------
          ID: edit_file
    Function: file.managed
        Name: /tmp/test
      Result: True
     Comment: File /tmp/test is in the correct state
     Started: 14:51:19.403274
    Duration: 66.092 ms
     Changes:   
----------
          ID: comment_file
    Function: file.comment
        Name: /tmp/test
      Result: True
     Comment: Commented lines successfully
     Started: 14:51:19.469601
    Duration: 7.115 ms
     Changes:   
              ----------
              diff:
                  --- 
                  +++ 
                  @@ -1,2 +1,2 @@
                   ### Section header ###
                  -line to comment
                  +#line to comment
----------
          ID: uncomment_file
    Function: file.uncomment
        Name: /tmp/test
      Result: True
     Comment: Uncommented lines successfully
     Started: 14:51:19.476900
    Duration: 3.723 ms
     Changes:   
              ----------
              diff:
                  --- 
                  +++ 
                  @@ -1,2 +1,2 @@
                   ### Section header ###
                  -#line to comment
                  +line to comment

Summary for alpha
------------
Succeeded: 3 (changed=2)
Failed:    0
------------
Total states run:     3
Total run time:  76.930 ms

OrangeDog · 2020-02-18T12:13:20Z

@DmitryKuzmenko
This was a clean salt environment. I pip install salt into a fresh virtualenv.
No config, no states, no master. Hence --local state.low.

OrangeDog · 2020-02-18T12:19:14Z

@frogunder you reproduced before. What do you see with v3000?

frogunder · 2020-02-24T19:08:22Z

Seems to work correctly for me on 3000.

I pip3 installed salt.

root@ip-172-31-20-13:~# cat /srv/salt/test.sls 
/tmp/test_file:
  file.comment:
    - regex: '^\s*line to comment'
root@ip-172-31-20-13:~# cat /tmp/test_file
## Section header ##
line to comment
root@ip-172-31-20-13:~# salt-call --local test.version
local:
    3000
root@ip-172-31-20-13:~# salt-call --local state.low '{
    "__id__": "id",
    "state": "file",
    "fun":"comment",
    "name": "/tmp/test_file",
    "regex": "^\\s*line to comment"
}'
local:
    ----------
    __id__:
        id
    __run_num__:
        0
    __sls__:
        None
    changes:
        ----------
        diff:
            --- 
            +++ 
            @@ -1,2 +1,2 @@
             ## Section header ##
            -line to comment
            +#line to comment
    comment:
        Commented lines successfully
    duration:
        11.362
    name:
        /tmp/test_file
    result:
        True
    start_time:
        19:06:47.180403
root@ip-172-31-20-13:~# cat /tmp/test_file
## Section header ##
#line to comment
root@ip-172-31-20-13:~# salt --versions
Salt Version:
           Salt: 3000
 
Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
         Jinja2: 2.10
        libgit2: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.6.2
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 3.6.9 (default, Nov  7 2019, 10:44:02)
   python-gnupg: Not Installed
         PyYAML: 3.12
          PyZMQ: 18.1.1
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.3.2
 
System Versions:
           dist: Ubuntu 18.04 bionic
         locale: UTF-8
        machine: x86_64
        release: 4.15.0-1057-aws
         system: Linux
        version: Ubuntu 18.04 bionic
 
root@ip-172-31-20-13:~#

waynew · 2020-02-26T17:05:49Z

Might be a good idea to create a Dockerfile to reproduce this (or, show that it's been fixed)?

OrangeDog · 2020-02-27T13:08:20Z

OK, now I cannot reproduce, and I've not changed anything from what I was doing in #54260 (comment)

So I agree it is fixed, and I must've made some kind of mistake. Apologies.

OrangeDog changed the title ~~file.comment uses multiline search~~ file.comment uses unacnhored multiline search Aug 20, 2019

OrangeDog changed the title ~~file.comment uses unacnhored multiline search~~ file.comment uses unanchored multiline search Aug 20, 2019

frogunder added this to the Approved milestone Aug 20, 2019

frogunder added Bug broken, incorrect, or confusing behavior severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around State-Module triage labels Aug 20, 2019

OrangeDog mentioned this issue Oct 23, 2019

salt.states.file.line match regex support broken #48645

Closed

stale bot added the stale label Jan 8, 2020

stale bot removed the stale label Jan 8, 2020

waynew added Confirmed Salt engineer has confirmed bug/feature - often including a MCVE and removed triage labels Jan 8, 2020

OrangeDog mentioned this issue Feb 11, 2020

Strange behavior between file.comment and file.uncomment #56107

Open

DmitryKuzmenko closed this as completed Feb 13, 2020

Ch3LL reopened this Feb 14, 2020

OrangeDog closed this as completed Feb 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

file.comment uses unanchored multiline search #54260

file.comment uses unanchored multiline search #54260

OrangeDog commented Aug 20, 2019

frogunder commented Aug 20, 2019

stale bot commented Jan 8, 2020

OrangeDog commented Jan 8, 2020

stale bot commented Jan 8, 2020

DmitryKuzmenko commented Feb 13, 2020

OrangeDog commented Feb 14, 2020

OrangeDog commented Feb 14, 2020 •

edited

Loading

DmitryKuzmenko commented Feb 17, 2020

OrangeDog commented Feb 17, 2020 •

edited

Loading

DmitryKuzmenko commented Feb 17, 2020

OrangeDog commented Feb 17, 2020

DmitryKuzmenko commented Feb 18, 2020

OrangeDog commented Feb 18, 2020 •

edited

Loading

OrangeDog commented Feb 18, 2020

frogunder commented Feb 24, 2020

waynew commented Feb 26, 2020

OrangeDog commented Feb 27, 2020

file.comment uses unanchored multiline search #54260

file.comment uses unanchored multiline search #54260

Comments

OrangeDog commented Aug 20, 2019

Description of Issue

Setup

Steps to Reproduce Issue

Versions Report

frogunder commented Aug 20, 2019

stale bot commented Jan 8, 2020

OrangeDog commented Jan 8, 2020

stale bot commented Jan 8, 2020

DmitryKuzmenko commented Feb 13, 2020

OrangeDog commented Feb 14, 2020

OrangeDog commented Feb 14, 2020 • edited Loading

DmitryKuzmenko commented Feb 17, 2020

OrangeDog commented Feb 17, 2020 • edited Loading

DmitryKuzmenko commented Feb 17, 2020

OrangeDog commented Feb 17, 2020

DmitryKuzmenko commented Feb 18, 2020

OrangeDog commented Feb 18, 2020 • edited Loading

OrangeDog commented Feb 18, 2020

frogunder commented Feb 24, 2020

waynew commented Feb 26, 2020

OrangeDog commented Feb 27, 2020

OrangeDog commented Feb 14, 2020 •

edited

Loading

OrangeDog commented Feb 17, 2020 •

edited

Loading

OrangeDog commented Feb 18, 2020 •

edited

Loading