saltstack / saltstack/salt

[BUG] `file.symlink` with Existing Directory fails with `atomic=True` or `force=True`

Open
#65,976 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug needs-triage
Dominant language
Python
Stars
15.7k
Forks
5.6k
Avg merge
2d 44m
Merged PRs (30d)
80

Description

Description
When using the file.symlink module function with an existing directory (i.e. not a symlink but a real directory on the disk) it fails with [Errno 21] Is a directory if atomic=True or force=True.

The same thing doesn't happen with an existing file - it also didn't fail with force=True before the atomic option was added.

Setup
Sample SLS:

/tmp/newdir/newfile:
  file.managed:
    - makedirs: true
    - contents: |
        hello world

/tmp/targetdir:
  file.directory: []

/tmp/newdir:
  file.symlink:
    - target: /tmp/targetdir
    - force: true # or atomic: true
  • on-prem machine

Steps to Reproduce the behavior
Apply the sample state with 3006.x

Expected behavior
The expected behaviour (as it was in 3005.1) here would be to wipe /tmp/newdir (including the tmp/newdir/newfile that was created in the first state) and replace it with a symlink to /tmp/targetdir.

I realise its an anti-pattern to create a file/directory and then subsequently wipe it, but that's how it works for files (i.e. if I create a file, and then symlink to it with force=True, it destroys the original file and creates a symlink).

Versions Report
Ran for both 3005.1 (old behaviour where force=True destroyed the directory) and 3006.3 (new version where it doesn't work for either force=True or atomic=True for directories).

```yaml salt@old-master: salt --versions Salt Version: Salt: 3005.1

Dependency Versions:
cffi: 1.15.1
cherrypy: unknown
dateutil: 2.8.2
docker-py: Not Installed
gitdb: Not Installed
gitpython: Not Installed
Jinja2: 3.1.2
libgit2: Not Installed
M2Crypto: 0.38.0
Mako: Not Installed
msgpack: 1.0.3
msgpack-pure: Not Installed
mysql-python: Not Installed
pycparser: 2.21
pycrypto: Not Installed
pycryptodome: 3.11.0
pygit2: Not Installed
Python: 3.11.2 (main, Mar 13 2023, 12:18:29) [GCC 12.2.0]
python-gnupg: 0.4.9
PyYAML: 6.0
PyZMQ: 24.0.1
smmap: Not Installed
timelib: Not Installed
Tornado: 4.5.3
ZMQ: 4.3.4

System Versions:
dist: debian 12 bookworm
locale: utf-8
machine: x86_64
release: 6.4.16-linuxkit
system: Linux
version: Debian GNU/Linux 12 bookworm

salt@old-minion$ salt-call --versions
Salt Version:
Salt: 3005.1

Dependency Versions:
cffi: 1.15.1
cherrypy: Not Installed
dateutil: 2.8.2
docker-py: Not Installed
gitdb: Not Installed
gitpython: Not Installed
Jinja2: 3.1.2
libgit2: Not Installed
M2Crypto: 0.38.0
Mako: Not Installed
msgpack: 1.0.3
msgpack-pure: Not Installed
mysql-python: Not Installed
pycparser: 2.21
pycrypto: Not Installed
pycryptodome: 3.11.0
pygit2: Not Installed
Python: 3.11.2 (main, Mar 13 2023, 12:18:29) [GCC 12.2.0]
python-gnupg: Not Installed
PyYAML: 6.0
PyZMQ: 24.0.1
smmap: Not Installed
timelib: Not Installed
Tornado: 4.5.3
ZMQ: 4.3.4

System Versions:
dist: debian 12 bookworm
locale: utf-8
machine: aarch64
release: 6.4.16-linuxkit
system: Linux
version: Debian GNU/Linux 12 bookworm

salt@new-master: salt --versions
Salt Version:
Salt: 3006.3

Python Version:
Python: 3.11.2 (main, Mar 13 2023, 12:18:29) [GCC 12.2.0]

Dependency Versions:
cffi: 1.15.1
cherrypy: Not Installed
dateutil: 2.8.2
docker-py: Not Installed
gitdb: Not Installed
gitpython: Not Installed
Jinja2: 3.1.2
libgit2: Not Installed
looseversion: 1.0.3
M2Crypto: 0.38.0
Mako: Not Installed
msgpack: 1.0.3
msgpack-pure: Not Installed
mysql-python: Not Installed
packaging: 23.0
pycparser: 2.21
pycrypto: Not Installed
pycryptodome: 3.11.0
pygit2: Not Installed
python-gnupg: 0.4.9
PyYAML: 6.0
PyZMQ: 24.0.1
relenv: Not Installed
smmap: Not Installed
timelib: Not Installed
Tornado: 4.5.3
ZMQ: 4.3.4

System Versions:
dist: debian 12.0 bookworm
locale: utf-8
machine: x86_64
release: 6.4.16-linuxkit
system: Linux
version: Debian GNU/Linux 12.0 bookworm

salt@new-minion$ salt-call --versions
Salt Version:
Salt: 3006.3

Python Version:
Python: 3.11.2 (main, Mar 13 2023, 12:18:29) [GCC 12.2.0]

Dependency Versions:
cffi: 1.15.1
cherrypy: Not Installed
dateutil: 2.8.2
docker-py: Not Installed
gitdb: Not Installed
gitpython: Not Installed
Jinja2: 3.1.2
libgit2: Not Installed
looseversion: 1.0.3
M2Crypto: 0.38.0
Mako: Not Installed
msgpack: 1.0.3
msgpack-pure: Not Installed
mysql-python: Not Installed
packaging: 23.0
pycparser: 2.21
pycrypto: Not Installed
pycryptodome: 3.11.0
pygit2: Not Installed
python-gnupg: Not Installed
PyYAML: 6.0
PyZMQ: 24.0.1
relenv: Not Installed
smmap: Not Installed
timelib: Not Installed
Tornado: 4.5.3
ZMQ: 4.3.4

System Versions:
dist: debian 2023.10.3 bookworm
locale: utf-8
machine: aarch64
release: 6.4.16-linuxkit
system: Linux
version: Debian GNU/Linux 2023.10.3 bookworm

</details>

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reproducing the sample SLS on Salt 3006.3 and comparing its behavior with 3005.1. Trace the file.symlink module function's handling of an existing directory when force or atomic is enabled, then add coverage for the reported case. Done means the existing directory is replaced by the requested symlink without the Errno 21 failure.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
devops, infrastructure
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.