[BUG] `file.serialize` does not merge lists even if `merge_if_exists: True` is set
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 15.7k
- Forks
- 5.6k
- Avg merge
- 2d 44m
- Merged PRs (30d)
- 80
Description
Description
I am using file.serialize to update the contents of a JSON file. Since that file may already exist, I have set merge_if_exists: True in order to merge the file contents with the dataset given in the state. Now if both the file and dataset have a list, Salt does not merge the lists but instead overwrites it with version from dataset.
Setup
(Please provide relevant configs and/or SLS files (be sure to remove sensitive info. There is no general set-up of Salt.)
Please be as specific as possible and give set-up details.
- 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
(Include debug logs if possible and relevant)
Create following 3 files:
9d2538af0da3:/ # cat /srv/salt/top.sls
base:
'*':
- bar
9d2538af0da3:/ # cat /srv/salt/bar.sls
write_test_file:
file.serialize:
- name: /test.json
- dataset:
eric: bacon
john:
- eggs
- spam
- serializer: json
- merge_if_exists: True
- show_changes: True
9d2538af0da3:/ # cat /test.json
{
"terry": [
"spam",
"bacon"
],
"john": [
"beans"
]
}
Then apply the state or run test-mode:
9d2538af0da3:/ # salt-call --local state.apply bar test=True
local:
----------
ID: write_test_file
Function: file.serialize
Name: /test.json
Result: None
Comment: Dataset will be serialized and stored into /test.json
Started: 15:51:31.413810
Duration: 17.361 ms
Changes:
----------
diff:
----------
eric:
----------
new:
bacon
old:
<_null_>
john:
----------
new:
- eggs
- spam
old:
- beans
Summary for local
------------
Succeeded: 1 (unchanged=1, changed=1)
Failed: 0
------------
Total states run: 1
Total run time: 17.361 ms
Expected behavior
The lists from the file and from dataset should be merged, like this:
9d2538af0da3:/ # salt-call --local state.apply bar test=True
local:
----------
ID: write_test_file
Function: file.serialize
Name: /test.json
Result: None
Comment: Dataset will be serialized and stored into /test.json
Started: 15:53:02.186886
Duration: 17.415 ms
Changes:
----------
diff:
----------
eric:
----------
new:
bacon
old:
<_null_>
john:
----------
new:
- beans
- eggs
- spam
old:
- beans
Summary for local
------------
Succeeded: 1 (unchanged=1, changed=1)
Failed: 0
------------
Total states run: 1
Total run time: 17.415 ms
Versions Report
salt --versions-report
(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)9d2538af0da3:/ # salt-call --versions-report
Salt Version:
Salt: 3006.0
Python Version:
Python: 3.11.9 (main, Apr 08 2024, 06:18:15) [GCC]
Dependency Versions:
cffi: 1.17.0
cherrypy: Not Installed
contextvars: 2.4
dateutil: Not Installed
docker-py: Not Installed
gitdb: Not Installed
gitpython: Not Installed
Jinja2: 3.1.4
libgit2: Not Installed
looseversion: 1.3.0
M2Crypto: 0.42.0
Mako: Not Installed
msgpack: 1.1.0
msgpack-pure: Not Installed
mysql-python: Not Installed
packaging: 24.1
pycparser: 2.22
pycrypto: Not Installed
pycryptodome: Not Installed
pygit2: Not Installed
python-gnupg: Not Installed
PyYAML: 6.0.1
PyZMQ: 25.1.2
relenv: Not Installed
smmap: Not Installed
timelib: Not Installed
Tornado: 4.5.3
ZMQ: 4.3.5
System Versions:
dist: opensuse-tumbleweed 20240917 n/a
locale: utf-8
machine: x86_64
release: 6.10.10-arch1-1
system: Linux
version: openSUSE Tumbleweed 20240917 n/a
Additional context
The issue seems to be a missing third parameter merge_lists=True in this function call:
merged_data = salt.utils.dictupdate.merge_recurse(
existing_data, dataset
)
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in salt/states/file.py at the file.serialize merge_recurse call linked in the issue, then trace how merge_if_exists handles existing JSON data. Add regression coverage for lists under file.serialize and verify that the existing and dataset list values are merged as shown in the expected output.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- infrastructure
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 52/100