salt.modules.file.line documentation improvements

[JensRantil]

See https://docs.saltstack.com/en/latest/ref/modules/all/salt.modules.file.html#salt.modules.file.line.

• Fix the :param. Nothing else in the documentation is using that format.
• I have no idea what "Match the target line for an action by a fragment of a string or regular expression." even means. There is no general description of file.line. I assume match will be required for every call to file.line? How do I decide whether I want to only match a fixed string fragment or regular expression?
• what before and after actually are used for. Are they lines that need to be before or after? How do I choose if they should be regexps or not?
• A lot of the parameters default to None. Which are required? Do they have any default values?
• Use a better example that covers more cases.

[jfindlay]

@JensRantil, thanks for the the report.

[zenion]

Additionally, the CLI Examples: of the documentation is wrong and causes an error if someone attempts to follow it as it contains commas that aren't supposed to be there as seen below:

salt '_' file.line /etc/nsswitch.conf "networks: files dns", after="hosts:._?", mode='ensure'

commas should be removed in all occurrences of this cli example.

[JensRantil]

I don't understand why this doesn't work:

my-state:
  file.line:
   - name: /my/file
   - mode: ensure
   - content: "line-I-want-to-exist"
   - location: end

This is the error I'm getting (incomprehensible):

          ID: my-state
    Function: file.line
        Name: /my/file
      Result: False
     Comment: An exception occurred in this state: Traceback (most recent call last):
                File "/usr/lib/python2.7/dist-packages/salt/state.py", line 1624, in call
                  **cdata['kwargs'])
                File "/usr/lib/python2.7/dist-packages/salt/loader.py", line 1491, in wrapper
                  return f(*args, **kwargs)
                File "/usr/lib/python2.7/dist-packages/salt/states/file.py", line 2560, in line
                  backup=backup, quiet=quiet, indent=indent)
                File "/usr/lib/python2.7/dist-packages/salt/modules/file.py", line 1611, in line
                  raise CommandExecutionError("Wrong conditions? "
              CommandExecutionError: Wrong conditions? Unable to ensure line without knowing where to put it before and/or after.
     Started: 21:30:00.738783
    Duration: 4.958 ms
     Changes:

I'd like Salt to add this line to the end of the file if it doesn't exist. Feels like a common use-case. Is my use case not possible? Please make sure that the documentation explains why this doesn't work.

[JensRantil]

Okay, looks like I should use file.append instead...

[JensRantil]

Another example of where I don't understand how file.line should be used:

/etc/fstab:
  file.line:
    - match: "/dev/xvda.*"
    - mode: Delete

What I want to do: Remove any line starting with /dev/xvda.

Error message I am getting:

          ID: /etc/fstab
    Function: file.line
      Result: False
     Comment: Missing parameter content for state file.line
     Started:
    Duration:
     Changes:

Why would I need to specify content when I'm deleting a line matching a regexp? Can file.line only delete fixed lines? The documentation doesn't state.

[JensRantil]

Also, why are modes specified with capital first letter? Feels weird since nothing else is in capitals Salt.

[brevilo]

Stuck in the same way (if that helps)...

[campbellmc]

On the subject of file.line mode: Delete, aside from having to use contentrather than match...

It seems there is double escape processing for regex strings.
That is, with a single escape:
Rendering SLS 'base:blah' failed: found unknown escape character '.'; line xxx

This means double escaping to obtain a literal period:

\. for a literal period must be \\.

As it's not standard regex escaping, although not uncommon when there is more than one string processor, it should be documented to save people a whole bunch of time.

[JensRantil]

Gah! I wished I'd know that last week! Thanks! That explains why the state didn't match... I eventually resorted to a really ugly workaround involving file.replace...

[Heiko-san]

I agree with the previous posts.
The behavior of file.line is everything else than intiutive.

I think the most common situation that people want to describe with such a resource would be:
"find occurence in file and replace it with my given line, if not present append it to the end of the file (with the OPTION to override end of file with something like after and/or before)"

EDIT: Ignore this post, actually file.replace seems to be doing what I want, I'll give it a try, however the names are somehow confusing.

[aabognah]

I am having trouble forcing file.line to replace occurrences of the match string at the beginning of a line. I am using '^' but it seems to be interpreted literally and the match is not found.

file.line:
  - name: filename
  - mode: replace
  - match: '^match string'
  - content: 'to be replaced with this'

file contents:

match string
some other test. match string again.

outcome:
nothing is replaced when using ^
without ^ all occurrences of 'match string' are replaced

[JensRantil]

@aabognah Have you tried using file.replace instead?

[githubcdr]

I'm having same issues here, the state should not generate an exception but just return what's wrong in a user friendly way. Making sure a simple line exists using Saltstack takes a lot of effort in current situation.

file.append is not a great alternative, it keeps adding lines as the name suggests, not what you want.

The docs are still not updated and suffer from a markup bug I think.

[dr4Ke]

@githubcdr file.append should not append a line more than once. If it does, there is something wrong in your state or a bug in the function. I use it

As for file.line, I consider it as not working as expected, and very badly documented. I use other functions such as file.replace or file.append instead.

[githubcdr]

I agree, I was looking for an easy way to change a keyword using file.line, I failed.

On another note file.append will not help you you want to change the value of FOO;

FOO=BAR

This is something that file.line should make easy, at least try :) file.replace seem to be better at everything compared to file.line.

[boltronics]

I usually use file.replace, but it only seems to work with regular expressions. If I want to look for a URL which is pulled from a pillar source and is not in regex format, it's probably not going to work so well due to slashes and other characters that have special meanings in regex land.

file.line looks like a solution here, but it clearly has a lot of problems. I'd be happy with file.replace if it only supported fixed strings.

[lorengordon]

@boltronics if file.replace is not working with basic strings, that should be reported as a bug.

[boltronics]

@lorengordon You mean a feature request? There's nothing to indicate it was meant to support fixed strings.

[lorengordon]

@boltronics no, a bug. strings definitely work. if you are finding they are not, it is either a bug, or a usage error.

C:\salt\srv\states\base> cat foo.sls
foo:
  file.replace:
    - name: C:\salt\srv\states\base\foo.txt
    - pattern: foo
    - repl: '#foo'
C:\salt\srv\states\base> cat foo.txt
foo
bar
baz
C:\salt\srv\states\base> C:\salt\salt-call.bat state.sls foo
local:
----------
          ID: foo
    Function: file.replace
        Name: C:\salt\srv\states\base\foo.txt
      Result: True
     Comment: Changes were made
     Started: 21:53:51.975000
    Duration: 36.0 ms
     Changes:
              ----------
              diff:
                  ---
                  +++
                  @@ -1,3 +1,3 @@
                  -foo
                  +#foo
                   bar
                   baz

Summary for local
------------
Succeeded: 1 (changed=1)
Failed:    0
------------
Total states run:     1
Total run time:  36.000 ms
C:\salt\srv\states\base> cat .\foo.txt
#foo
bar
baz

[boltronics]

@lorengordon But that's just because your string example happens to be regex-compatible. ie. it's no different to running:

$ echo 'foobar' | sed 's/foo/#foo/'
#foobar
$

Try this instead:

$ cat urls.sls 
foo:
  file.replace:
    - name: /tmp/urls.txt
    - pattern: https://encrypted.google.com/search?hl=en&q=Test%20search%20argument
    - repl: 'https://duckduckgo.com/?q=Test+search+argument&t=ffsb&ia=web'
$ cat /tmp/urls.txt
https://saltstack.com/
https://encrypted.google.com/search?hl=en&q=Test%20search%20argument
https://www.gnu.org/
$ diff <(tail -n 2 /tmp/urls.txt | head -n 1) <(tail -n 2 urls.sls | head -n 1 | sed 's/.*:\ //')
$ echo $?
0
$ sudo salt-call state.sls urls
[INFO    ] Loading fresh modules for state activity
[INFO    ] Fetching file from saltenv 'base', ** done ** 'urls.sls'
[INFO    ] Running state [/tmp/urls.txt] at time 14:19:14.277698
[INFO    ] Executing state file.replace for [/tmp/urls.txt]
[INFO    ] No changes needed to be made
[INFO    ] Completed state [/tmp/urls.txt] at time 14:19:14.282500 duration_in_ms=4.802
local:
----------
          ID: foo
    Function: file.replace
        Name: /tmp/urls.txt
      Result: True
     Comment: No changes needed to be made
     Started: 14:19:14.277698
    Duration: 4.802 ms
     Changes:   

Summary for local
------------
Succeeded: 1
Failed:    0
------------
Total states run:     1
Total run time:   4.802 ms
$ cat /tmp/urls.txt
https://saltstack.com/
https://encrypted.google.com/search?hl=en&q=Test%20search%20argument
https://www.gnu.org/
$ 

This makes perfect sense, since the URL is not a regex but is being treated as one as per the docs. But we can do this instead:

$ cat urls.sls 
foo:
  file.replace:
    - name: /tmp/urls.txt
    - pattern: https:\/\/encrypted\.google\.com\/search\?hl=en&q=Test%20search%20argument
    - repl: 'https://duckduckgo.com/?q=Test+search+argument&t=ffsb&ia=web'
$ sudo salt-call state.sls urls
[INFO    ] Loading fresh modules for state activity
[INFO    ] Fetching file from saltenv 'base', ** done ** 'urls.sls'
[INFO    ] Running state [/tmp/urls.txt] at time 14:20:32.908637
[INFO    ] Executing state file.replace for [/tmp/urls.txt]
[INFO    ] File changed:
--- 
+++ 
@@ -1,3 +1,3 @@
 https://saltstack.com/
-https://encrypted.google.com/search?hl=en&q=Test%20search%20argument
+https://duckduckgo.com/?q=Test+search+argument&t=ffsb&ia=web
 https://www.gnu.org/

[INFO    ] Completed state [/tmp/urls.txt] at time 14:20:32.915406 duration_in_ms=6.769
local:
----------
          ID: foo
    Function: file.replace
        Name: /tmp/urls.txt
      Result: True
     Comment: Changes were made
     Started: 14:20:32.908637
    Duration: 6.769 ms
     Changes:   
              ----------
              diff:
                  --- 
                  +++ 
                  @@ -1,3 +1,3 @@
                   https://saltstack.com/
                  -https://encrypted.google.com/search?hl=en&q=Test%20search%20argument
                  +https://duckduckgo.com/?q=Test+search+argument&t=ffsb&ia=web
                   https://www.gnu.org/

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

From the file.replace docs, pattern is passed to re.search, which clearly says it operates on a "regular expression pattern". This does not appear to be something that can be changed via one of the optional flags either.

Hence the only option right now is to use file.line for such operations, and that is not a bug.

[JensRantil]

@boltronics I suggest you file a separate feature request for that. It feels like this really is somewhat off-topic for this issue.

[boltronics]

@JensRantil I don't see how. I was pointing out a valid use case where file.line is the only obvious solution (if you read the docs), hence the importance of getting the bugs mentioned here ironed out.

Edit: But I'll open a separate feature request anyway, since it seems there is a lot of misunderstanding around this issue.

[jf]

Indeed, the documentation is unclear. How does one supply an actual string, vs a regular expression? I have the same problem as well, where characters like '^', and '$' are interpreted as literal characters and will only match if those corresponding characters are literally in the file.

[jf]

I just looked through the source for file.line, and it seems that _there is no way to specify an exact match with '^', and '$'_ (re.search() is called in _regex_to_static(), but without any flags, so i.e. no re.MULTILINE). Given that this is the case, unless we're sure about the format of the file, we could very well end up with more matches than we care for. I'm hence wondering about the utility for file.line, as opposed to simply using file.replace.... Anybody any idea?

[shallot]

With regard to "Match ... by a fragment of a string or regular expression", I'm guessing this means what it usually means with grep - substring match will be done by default, but if you specify regex syntax, it will be used

Having said that, the distinction between the content and match parameters here is fairly unclear, and the relationships between parameters are not spelled out.

It should be spelled out that you need match only if you're doing a 'delete' or a 'replace'.
Likewise, if you're doing an 'ensure' or an 'insert', you do not need a match but you do need content.
And when you're doing an 'ensure', you need one or both of before and after.

[shallot]

It should also be documented that for 'ensure' to work with both before and after, the latter two have to be immediately next to that line.

So for example if you want to add a line in this kind of a block:

foo {
  bar
  baz
}

...you can use after: 'foo {' and before: 'bar' but you can NOT use before: 'baz'

Note also how https://github.com/saltstack/salt/issues/40821 makes this difficult

[craigafinch]

Here is a use case for file.line. I am currently using file.replace, but that's not ideal, as you will hopefully see by the end of my example.

Here is a snippet of a basic CentOS repository file:

[base]
name=CentOS-$releasever - Base
mirrorlist=http://mirrorlist.centos.org/?release=$releasever&arch=$basearch&repo=os&infra=$infra
#baseurl=http://mirror.centos.org/centos/$releasever/os/$basearch/
gpgcheck=1
gpgkey=file:///etc/pki/rpm-gpg/RPM-GPG-KEY-CentOS-7

#released updates 
[updates]
name=CentOS-$releasever - Updates
mirrorlist=http://mirrorlist.centos.org/?release=$releasever&arch=$basearch&repo=updates&infra=$infra
#baseurl=http://mirror.centos.org/centos/$releasever/updates/$basearch/
gpgcheck=1
gpgkey=file:///etc/pki/rpm-gpg/RPM-GPG-KEY-CentOS-7

...

I want to insert the line priority=1 after every gpgkey line. Right now, I do this:

  file.replace:
    - name: /etc/yum.repos.d/CentOS-Base.repo
    - pattern: gpgkey=file:///etc/pki/rpm-gpg/RPM-GPG-KEY-CentOS-7\n\n
    - repl: |
        gpgkey=file:///etc/pki/rpm-gpg/RPM-GPG-KEY-CentOS-7
        priority=1\n

It would be more obvious what I'm trying to do (insert a line as opposed to replace it) if I could get file.line working.

[gdm85]

I spent quite a bit of time trying to have a line ensured to be in a file. After frustration, I came up with a shell-based replacement:

have-potatoes:
  cmd.run:
    - unless: grep -qF "$LINE" /var/lib/shopping.list
    - name: echo "$LINE" >> /var/lib/shopping.list
    - env:
       - LINE: 'potatoes 45kg'

I hope in future it will be easier to reproduce this functionality in Salt (notice that since -F mode of grep does not anchor, this is far from a perfect replacement of a line-based check).

[dr4Ke]

@gdm85 This simple need should be addressed by the file.append state. Did you look at it? The removal of a line is more tricky, though.
I use file.blockreplace where I can to manage lines in files. The file.line is clearly not production ready.

[gdm85]

@dr4Ke I did. My use case is not about appending though, as lines can be in any position in the file. Does file.append check if the line is present somewhere else before the end of the file? It does not, as far as I remember.

[dr4Ke]

If I understand correctly what you mean: yes, it does. I use this state for exactly this kind of things. The line must exists anywhere in the file. If it doesn't, it's appended to it.

The documentation is not clear about that though. It says "Ensure that some text appears at the end of a file" and next "text will not be appended if it already exists in the file". A better description could be:

Ensure that some text appears in a file, by appending it if not already present.

What do you think?

[stale[bot]]

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.