[BUG] TOML serializer error when merging a file which contains datetime value
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 15.7k
- Forks
- 5.6k
- Avg merge
- 2d 44m
- Merged PRs (30d)
- 80
Description
Description
salt.states.file.serializer for toml triggers an exception if the file (using merge_if_exists: True) contains a datetime value (eg. token_obtained_at = 2022-08-24T16:32:01Z) such as used by gitlab runner configs /etc/gitlab-runner/config.toml.
If the file contains no datetime as values, everything works as expected.
If the file contains any datetime as values, the following error is returned.
----------
ID: glrunner-config-file-file-managed
Function: file.serialize
Result: False
Comment: An exception occurred in this state: Traceback (most recent call last):
File "/usr/lib/python3/dist-packages/salt/state.py", line 2179, in call
ret = self.states[cdata["full"]](
File "/usr/lib/python3/dist-packages/salt/loader/lazy.py", line 149, in __call__
return self.loader.run(run_func, *args, **kwargs)
File "/usr/lib/python3/dist-packages/salt/loader/lazy.py", line 1201, in run
return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
File "/usr/lib/python3/dist-packages/salt/loader/lazy.py", line 1216, in _run_as
return _func_or_method(*args, **kwargs)
File "/usr/lib/python3/dist-packages/salt/loader/lazy.py", line 1249, in wrapper
return f(*args, **kwargs)
File "/usr/lib/python3/dist-packages/salt/states/file.py", line 7763, in serialize
merged_data = salt.utils.dictupdate.merge_recurse(
File "/usr/lib/python3/dist-packages/salt/utils/dictupdate.py", line 78, in merge_recurse
copied = copy.deepcopy(obj_a)
File "/usr/lib/python3.9/copy.py", line 146, in deepcopy
y = copier(x, memo)
File "/usr/lib/python3.9/copy.py", line 230, in _deepcopy_dict
y[deepcopy(key, memo)] = deepcopy(value, memo)
File "/usr/lib/python3.9/copy.py", line 172, in deepcopy
y = _reconstruct(x, memo, *rv)
File "/usr/lib/python3.9/copy.py", line 264, in _reconstruct
y = func(*args)
File "/usr/lib/python3.9/copy.py", line 263, in <genexpr>
args = (deepcopy(arg, memo) for arg in args)
File "/usr/lib/python3.9/copy.py", line 172, in deepcopy
y = _reconstruct(x, memo, *rv)
File "/usr/lib/python3.9/copy.py", line 264, in _reconstruct
y = func(*args)
TypeError: __init__() missing 1 required positional argument: 'toml_offset'
Started: 17:09:39.283899
Duration: 11.334 ms
Changes:
----------
Setup
Master and minions are deployed in local Proxmox VMs running Debian 10/11.
Writing a state to manage part of gitlab-runner config.toml while allowing gitlab-runner --register to still make changes to the config file.
Using salt.states.file.serializer with serializer: toml.
Currently just testing with some values in defaults.yaml. This part contains no timestamps
- defaults.yaml
values:
configs:
concurrent: 1
check_interval: 5
session_server:
session_timeout: 1800
listen_address: "[::]:8093"
advertise_address: "{{ salt['grains.get']('id')}}:8093"
- state
glrunner-config-file-file-managed:
file.serialize:
- dataset: {{ glrunner.configs }}
- name: /etc/gitlab-runner/config.toml
- serializer: toml
- mode: '0600'
- user: root
- group: root
- makedirs: True
- show_changes: True
- merge_if_exists: True
- existing file at /etc/gitlab-runner/config.toml on the minion
concurrent = 1
check_interval = 0
[session_server]
listen_address = "[::]:8093"
advertise_address = "myrunnerhostname.example.com:8093"
session_timeout = 1800
[[runners]] <-- This section and down is not managed by saltstack, but is added by `gitlab-runner register`
name = "myrunnerhostname"
url = "https://mygitlab.example.com/"
id = 6
token = "SECRETTOKEN"
token_obtained_at = 2022-08-24T16:32:01Z <--- the offending lines. No error if I test without these two lines.
token_expires_at = 0001-01-01T00:00:00Z <---/
executor = "shell"
[runners.custom_build_dir]
[runners.cache]
[runners.cache.s3]
[runners.cache.gcs]
[runners.cache.azure]
Steps to Reproduce the behavior
Attempt to use file.serialize state with merge_if_exist: True set when existing declarations in the file contain datetime values.
Expected behavior
Expect /etc/gitlab-runner/config.toml file to be updated with the supplied dataset while keeping the rest of the file intact.
In this case, the only change should be changing check_interval = 0 to check_interval = 5
Versions Report
salt --versions-report
(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)Salt Version:
Salt: 3004.2
Dependency Versions:
cffi: Not Installed
cherrypy: unknown
dateutil: 2.7.3
docker-py: Not Installed
gitdb: 2.0.5
gitpython: 2.1.11
Jinja2: 2.10
libgit2: 0.27.7
M2Crypto: Not Installed
Mako: Not Installed
msgpack: 0.5.6
msgpack-pure: Not Installed
mysql-python: Not Installed
pycparser: 2.19
pycrypto: 2.6.1
pycryptodome: 3.6.1
pygit2: 0.27.4
Python: 3.7.3 (default, Jan 22 2021, 20:04:44)
python-gnupg: Not Installed
PyYAML: 5.4.1
PyZMQ: 17.1.2
smmap: 2.0.5
timelib: Not Installed
Tornado: 4.5.3
ZMQ: 4.3.1
System Versions:
dist: debian 10 buster
locale: utf-8
machine: x86_64
release: 4.19.0-21-amd64
system: Linux
version: Debian GNU/Linux 10 buster
Additional context
I suspect this may be similar to an issue I found in another project while researching this - https://github.com/uiri/toml/issues/308 - which suggests it may be an issue with using deepcopy, which does appear in the error message included above.
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 serialize and trace the merge_if_exists path into salt/utils/dictupdate.py, where the traceback shows deepcopy failing on TOML datetime values. Reproduce with the supplied config.toml structure and datetime fields, then verify that check_interval changes while the existing runner configuration remains intact.
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
- 48/100