[BUG] file.symlink will not replace/update existing symlink

[onmeac]

Description

file.symlink docs:

If the file already exists and is a symlink pointing to any location other than the specified target, the symlink will be replaced.

However this does not seem to work in version 3006.1

local:
----------
          ID: foo
    Function: file.symlink
        Name: /foo
      Result: False
     Comment: Unable to create new symlink /foo -> /baz: Found existing symlink: /foo
     Started: 16:19:03.417405
    Duration: 6.565 ms
     Changes:   

Summary for local
------------
Succeeded: 0
Failed:    1
------------
Total states run:     1
Total run time:   6.565 ms

Setup
symlink_test.sls:

foo:
  file.symlink:
    - name: /foo
    - target: /baz

• on-prem machine
• VM (Virtualbox, KVM, etc. please specify)
• VM running on a cloud service, please be explicit and add details
• container (Kubernetes, Docker, containerd, etc. please specify)
• or a combination, please be explicit
• jails if it is FreeBSD
• classic packaging
• onedir packaging
• used bootstrap to install

Steps to Reproduce the behavior

• ln -nfs /bar /foo
• salt-call state.apply symlink_test

Expected behavior

----------
          ID: foo
    Function: file.symlink
        Name: /foo
      Result: True
     Comment: Created new symlink /foo -> /baz
     Started: 16:13:06.093500
    Duration: 7.27 ms
     Changes:   
              ----------
              new:
                  /foo

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

Versions Report

salt --versions-report

(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)

Salt Version:
          Salt: 3006.1
 
Python Version:
        Python: 3.10.11 (main, May  5 2023, 02:31:54) [GCC 11.2.0]
 
Dependency Versions:
          cffi: 1.14.6
      cherrypy: unknown
      dateutil: 2.8.1
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 3.1.2
       libgit2: Not Installed
  looseversion: 1.0.2
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     packaging: 22.0
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.9.8
        pygit2: Not Installed
  python-gnupg: 0.4.8
        PyYAML: 5.4.1
         PyZMQ: 23.2.0
        relenv: 0.12.3
         smmap: Not Installed
       timelib: 0.2.4
       Tornado: 4.5.3
           ZMQ: 4.3.4
 
System Versions:
          dist: centos 7.9.2009 Core
        locale: utf-8
       machine: x86_64
       release: 3.10.0-1160.81.1.el7.x86_64
        system: Linux
       version: CentOS Linux 7.9.2009 Core

[onmeac]

Maybe related to changes made by 62768 ?

[Oloremo]

It's due to #62768 and now we need to pass - atomic: true.
Should be the other way around to preserve original functionality. ;-/

[nicholasmhughes]

The change in the default behavior was unintentional, but it seems to mirror the default functionality of the ln command.

# ln -ns /baz /foo
ln: failed to create symbolic link '/foo': File exists

And then adding -f allows it to succeed. Similarly, the file.symlink state fails given the example case above unless force: true is passed as a parameter.

Expanding on the documentation quoted above, the rest of it seems to hint at the intention of requiring the force parameter for override:

    If the file already exists and is a symlink pointing to any location other
    than the specified target, the symlink will be replaced. If an entry with
    the same name exists then the state will return False. If the existing
    entry is desired to be replaced with a symlink pass force: True, if it is
    to be renamed, pass a backupname.

...but the force parameter documentation itself seems to mirror the first sentence of the function doc and draw a distinction between being a symlink and "not a symlink" in requiring the force parameter, which is the default behavior that was being relied upon.

    force
        If the name of the symlink exists and is not a symlink and
        force is set to False, the state will fail. If force is set to
        True, the existing entry in the way of the symlink file
        will be deleted to make room for the symlink, unless
        backupname is set, when it will be renamed

        .. versionchanged:: 3000
            Force will now remove all types of existing file system entries,
            not just files, directories and symlinks.

Just included this for reference in case Future Me was wondering what the heck was going on. I'll look at a patch to restore the default functionality and try to target 3006.2 for a release.