Salt: file.line mode=insert not working

Created on 10 Jan 2017  路  13Comments  路  Source: saltstack/salt

Description of Issue/Question

state file.line with mode=insert doesn't seem to work for this use-case. I just get No changes needed to be made

I can use mode=ensure, but that ends up only allowing me to manage a single line and subsequent file.line states with mode=ensure replace the previous line that was managed.

I attempted escaping the various "special" characters in the match strings for insert_line_one and insert_line_six but that didn't help.

If I switch after to '.*' for insert_line_one and insert_line_six then the lines will be inserted but not where I wanted them.

I think it has something to do with a check that is trying to match a line in the similar location to where I'm attempting to insert <testing type="some">onetwothree</testing> and returning a false positive which leads to it skipping the insert. If I remove <testing type="that">threefourfive</testing> from between <one> and </one> lines then insert_line_one will work, but insert_line_six will still say No changes needed to be made because now its getting another false positive match and skipping the insert.

insert_line_simple and insert_line_simple_again show that it has no problem inserting a line without the xml attribute type="xxxx" in the content or match

I'm at a loss to submit a PR for this, it's preventing me from using file.line, so I've resorted to a hacky workaround with file.replace as it's less hastle than attempting a fix.

Any clues @isbm ?

Setup

/root/testfile:

<test>
  <one>
    <testing type="that">threefourfive</testing>
  </one>
</test>

Steps to Reproduce Issue

sls:

insert_line_one:
  file.line:
    - name   : /root/testfile
    - content: <testing type="some">onetwothree</testing>
    - mode   : insert
    - after  : <one>
    - match  : '<testing type="some">.*</testing>'
    - indent : True

insert_line_six:
  file.line:
    - name   : /root/testfile
    - content: <testing type="thing">sixseveneight</testing>
    - mode   : insert
    - after  : <one>
    - match  : '<testing type="thing">.*</testing>'
    - indent : True

insert_line_simple:
  file.line:
    - name   : /root/testfile
    - content: <testing>simple</testing>
    - mode   : insert
    - before : <one>
    - match  : '<testing>.*</testing>'
    - indent : True

insert_line_simple_again:
  file.line:
    - name   : /root/testfile
    - content: <testing>simple</testing>
    - mode   : insert
    - after  : <one>
    - match  : '<testing>.*</testing>'
    - indent : True

Expected result

<test>
  <testing>simple</testing>
  <one>
  <testing>simple again</testing>
    <testing type="thing">sixseveneight</testing>
    <testing type="some">onetwothree</testing>
    <testing type="that">threefourfive</testing>
  </one>
</test>

Actual result

<test>
  <testing>simple</testing>
  <one>
  <testing>simple again</testing>
    <testing type="that">threefourfive</testing>
  </one>
</test>

local:
----------
          ID: insert_line_one
    Function: file.line
        Name: /root/testfile
      Result: True
     Comment: No changes needed to be made
     Started: 16:58:29.081017
    Duration: 4.982 ms
     Changes:
----------
          ID: insert_line_six
    Function: file.line
        Name: /root/testfile
      Result: True
     Comment: No changes needed to be made
     Started: 16:58:29.086139
    Duration: 0.722 ms
     Changes:
----------
          ID: insert_line_simple
    Function: file.line
        Name: /root/testfile
      Result: True
     Comment: Changes were made
     Started: 16:58:29.086999
    Duration: 1.157 ms
     Changes:
              ----------
              diff:
                  ---
                  +++
                  @@ -1,4 +1,5 @@
                   <test>+  <testing>simple</testing>   <one>     <testing type="that">threefourfive</testing>   </one>
----------
          ID: insert_line_simple_again
    Function: file.line
        Name: /root/testfile
      Result: True
     Comment: Changes were made
     Started: 16:58:29.088301
    Duration: 0.87 ms
     Changes:
              ----------
              diff:
                  ---
                  +++
                  @@ -1,6 +1,7 @@
                   <test>   <testing>simple</testing>   <one>+  <testing>simple</testing>     <testing type="that">threefourfive</testing>   </one> </test>

Summary for local
------------
Succeeded: 4 (changed=2)
Failed:    0
------------
Total states run:     4
Total run time:   7.731 ms



md5-b24903365653ade67c148d700dc26c64



Salt Version:
           Salt: 2016.11.1

Dependency Versions:
           cffi: 1.9.1
       cherrypy: Not Installed
       dateutil: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.7.2
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.8
   mysql-python: Not Installed
      pycparser: 2.17
       pycrypto: 2.6.1
         pygit2: Not Installed
         Python: 2.7.5 (default, Nov  6 2016, 00:28:07)
   python-gnupg: Not Installed
         PyYAML: 3.10
          PyZMQ: 15.3.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.1.4

System Versions:
           dist: centos 7.3.1611 Core
        machine: x86_64
        release: 3.10.0-514.2.2.el7.x86_64
         system: Linux
        version: CentOS Linux 7.3.1611 Core
Bug P4 Platform State Module severity-medium stale
Source
picture mlalpho
👍1

Most helpful comment

A workaround is using file.replace

create /tmp/test.txt:
  file.managed:
    - name: /tmp/test.txt
    - contents: |
        pr foo
        pr bar

insert a to /tmp/test.txt:
  file.replace:
    - name: /tmp/test.txt
    - pattern: 'pr foo'
    - repl: '\g<0>\npr test'
    - require:
      - file: create /tmp/test.txt
picture doitian on 16 Jan 2017
👍2

All 13 comments

Yeah, I wrote that few years ago with no unit tests. My bad.

isbm picture isbm on 10 Jan 2017

Any help would be appreciated if you've got time to look deeper. Cheers :beers:

mlalpho picture mlalpho on 10 Jan 2017

Looks like i'm able to replicate this behavior.

Here is a docker container for anyone that wants to quickly replicate this issue:

  1. docker run -it -v /home/ch3ll/git/salt/:/testing/ ch3ll/issues:38670
  2. salt-call --local state.sls test -ldebug
  3. cat /root/testfile

We will need to get this fixed up thanks

Ch3LL picture Ch3LL on 12 Jan 2017

I met the same issue. The cause is that file.line does not change the line if it finds the original line before/after the matched line has the common prefix of your content.

# modules/file.py line 1722
            elif after and not before:
                _assert_occurrence(body, after, 'after')
                out = []
                lines = body.split(os.linesep)
                for idx in range(len(lines)):
                    _line = lines[idx]
                    out.append(_line)
                    cnd = _get_line_indent(_line, content, indent)
                    if _line.find(after) > -1:
                        # No dupes or append, if "after" is the last line
                        if (idx < len(lines) and _starts_till(lines[idx + 1], cnd) < 0) or idx + 1 == len(lines):
                            out.append(cnd)
                body = os.linesep.join(out)

The problem is the function _starts_till. I'm not sure whether it is a feature, it is obscure and undocumented, I did some test and here is the result:

print(_starts_till('one', 'one')) #=> 0
print(_starts_till('one two', 'one two2')) #=> 0
print(_starts_till('one two three', 'one two2 three3')) #=> 1
print(_starts_till('one two three four', 'one two2 three3 four4')) #=> 2

A minimum test case to reproduce the issue:

create /tmp/test.txt:
  file.managed:
    - name: /tmp/test.txt
    - contents: |
        pr foo
        pr bar

insert a to /tmp/test.txt:
  file.line:
    - name: /tmp/test.txt
    - mode: insert
    - after: 'pr foo'
    - content: 'pr test'
    - require:
      - file: create /tmp/test.txt

The line "pr test" is not inserted since it is considered the same with "pr bar"

doitian picture doitian on 16 Jan 2017

A workaround is using file.replace

create /tmp/test.txt:
  file.managed:
    - name: /tmp/test.txt
    - contents: |
        pr foo
        pr bar

insert a to /tmp/test.txt:
  file.replace:
    - name: /tmp/test.txt
    - pattern: 'pr foo'
    - repl: '\g<0>\npr test'
    - require:
      - file: create /tmp/test.txt
doitian picture doitian on 16 Jan 2017
👍2

The workaround from @doitian is very useful. One clarification: if you want it to work just like mode=insert, make sure that pattern matches the entire line before the content that you want to insert. \g<0> inserts the text that was matched, so if pattern matches part of a line, only the partial match will be inserted before the new content. Here is an example of inserting a new line in a configuration file after a commented-out example:

    - pattern: '#mynetworks = hash.*'
    - repl: '\g<0>\nmynetworks = {{ pillar['networks'] }}'
craigafinch picture craigafinch on 28 Oct 2017
👍1

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.

stale[bot] picture stale[bot] on 20 Feb 2019

Hi,

Is this still a known issue or am I doing something wrong? It _seems_ as though if the start of "content" matches for more than 1 character then the entire line is seen to have matched and nothing happens.

"content" matching first character only: (works as expected)

cat /home/alex/file
ABC

cat test.sls
allow-access-1.1.1.1:
  file.line:
  - name: /home/alex/file
  - mode: ensure
  - after: ABC
  - content: a 1.1.1.1;

allow-access-2.2.2.2:
  file.line:
  - name: /home/alex/file
  - mode: ensure
  - after: ABC
  - content: a 2.2.2.2;



md5-1e9be9b170bcacca91354c8bed42d9c8



sudo salt-call --local state.apply
local:
----------
          ID: allow-access-1.1.1.1
    Function: file.line
        Name: /home/alex/file
      Result: True
     Comment: Changes were made
     Started: 04:48:24.017955
    Duration: 7.287 ms
     Changes:
              ----------
              diff:
                  ---
                  +++
                  @@ -1 +1,2 @@
                   ABC
                  +a 1.1.1.1;
----------
          ID: allow-access-2.2.2.2
    Function: file.line
        Name: /home/alex/file
      Result: True
     Comment: Changes were made
     Started: 04:48:24.025399
    Duration: 1.187 ms
     Changes:
              ----------
              diff:
                  ---
                  +++
                  @@ -1,2 +1,3 @@
                   ABC
                  +a 2.2.2.2;
                   a 1.1.1.1;

Summary for local
------------
Succeeded: 2 (changed=2)
Failed:    0
------------
Total states run:     2
Total run time:   8.474 ms



md5-1e9be9b170bcacca91354c8bed42d9c8



cat /home/alex/file
ABC
a 2.2.2.2;
a 1.1.1.1;



md5-e37f0f70b99da3ca2becd76f3f554968



cat test.sls
allow-access-1.1.1.1:
  file.line:
  - name: /home/alex/file
  - mode: ensure
  - after: ABC
  - content: aa 1.1.1.1;

allow-access-2.2.2.2:
  file.line:
  - name: /home/alex/file
  - mode: ensure
  - after: ABC
  - content: aa 2.2.2.2;



md5-1e9be9b170bcacca91354c8bed42d9c8



sudo salt-call --local state.apply
local:
----------
          ID: allow-access-1.1.1.1
    Function: file.line
        Name: /home/alex/file
      Result: True
     Comment: Changes were made
     Started: 06:23:44.059359
    Duration: 6.691 ms
     Changes:
              ----------
              diff:
                  ---
                  +++
                  @@ -1 +1,2 @@
                   ABC
                  +aa 1.1.1.1;
----------
          ID: allow-access-2.2.2.2
    Function: file.line
        Name: /home/alex/file
      Result: True
     Comment: No changes needed to be made
     Started: 06:23:44.066178
    Duration: 0.785 ms
     Changes:

Summary for local
------------
Succeeded: 2 (changed=1)
Failed:    0
------------
Total states run:     2
Total run time:   7.476 ms



md5-1e9be9b170bcacca91354c8bed42d9c8



cat /home/alex/file
ABC
aa 1.1.1.1;



md5-1e9be9b170bcacca91354c8bed42d9c8



salt-call --version
salt-call 2019.2.0 (Fluorine)

Happy to provide more information if what I'm seeing can't be easily reproduced.

aMozejko1 picture aMozejko1 on 26 Jun 2019

I think this new PR https://github.com/saltstack/salt/pull/52333 might help in this use case

Ch3LL picture Ch3LL on 2 Jul 2019

It seems to only match the first word instead of the whole line. And file.replace work-around is not a clean solution because you have to stop it changing the line every time Salt is run. I added a grep on the file to make this stop.

insert a to /tmp/test.txt:
  file.replace:
    - name: /tmp/test.txt
    - pattern: 'pr foo'
    - repl: '\g<0>\npr test'
    - unless:
      - grep "pr test" /tmp/test.txt
    - require:
      - file: create /tmp/test.txt
jsaathof picture jsaathof on 15 Nov 2019

bump @waynew i know you were originally working on that PR which has now been moved to https://github.com/saltstack/salt/pull/53888

can you respond to @jsaathof 's concern "It seems to only match the first word instead of the whole line." surrounding the PR.

Ch3LL picture Ch3LL on 12 Dec 2019

Hi,
it seems that this issue still persists in Salt 3000? Any updates on this?

Best regards
margau

margau picture margau on 6 Mar 2020

bump @waynew see above ^

@margau looks like the PR was not merged yet but is labeled for sodium, but looks like it might not solve @jsaathof 's use case so pinging @waynew to get his comments here

Ch3LL picture Ch3LL on 9 Mar 2020
Was this page helpful?
0 / 5 - 0 ratings

Related issues

seanacais picture seanacais  路  3Comments
Oloremo picture Oloremo  路  3Comments
twangboy picture twangboy  路  3Comments
sagetherage picture sagetherage  路  3Comments
icycle77 picture icycle77  路  3Comments