saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Get access to the Salt software package repository here:
https://repo.saltproject.io/
Apache License 2.0
14.12k stars 5.47k forks source link

[FEATURE REQUEST] file.keyvalue should be able to manage file permissions and ownership #66812

Open signe opened 1 month ago

signe commented 1 month ago

file.keyvalue currently provides the create_if_missing flag, but controlling the created (or existing) file's ownership or permissions is impossible.

As mentioned in the original bug, #63545, file.serialize is another option to control keyvalue files and it does allow for owner/perm controls, but it is more suited to managing an entire file and not specific values in the file. serializer.keyvalue doesn't support the same options that file.keyvalue does (e.g. uncomment, append..., prepend..., etc.), and it is also unable to handle any data that doesn't match the <key><separator><value> formatting such as line comments.

So, you can't have a situation where your expected output is:

Create /tmp/foo if not exists Ensure the owner is user/group/644 Prepend value "foo=bar" or uncomment if it's already present Retain the rest of the file unchanged

file.keyvalue will correctly handle the file contents, uncommenting lines if necessary, pre/appending, ignoring line comments, etc., but the ownership will be 🤷‍♀️

php_inis_myconf:
  file.keyvalue:
    - name: {{ php_conf_name }}
    - separator: '='
    - uncomment: '; #'
    - key_values:
        mykey1: myval1
        mykey2: myval2
    - create_if_missing: True
    - prepend_if_not_found: True

file.serialize will create and manage the file with the correct ownership, but the file contents will not be handled correctly. Specifically, pre-existing line comments will cause the deserializer to barf and fail to modify the file at all:

php_inis_myconf:
  file.serialize:
    - name: {{ php_conf_name }}
    - user: root
    - group: root
    - mode: 0x0644
    - merge_if_exists: True
    - serializer: keyvalue
    - serializer_opts:
        - separator: '='
    - dataset:
          mykey1: myval1
          mykey2: myval2

With an existing file of:

; Line comment
mykey3=myval3

Results in errors like:

          ID: php_inis_myconf
    Function: file.serialize
        Name: /etc/php/8.3/mods-available/myconf.ini
      Result: False
     Comment: Failed to deserialize existing data: not enough values to unpack (expected 2, got 1)
     Started: 13:29:34.978887
    Duration: 0.826 ms
     Changes:

Having to add a separate file.managed state to accomplish this is clunky at best, because the file.managed and file.keyvalue can't exist in the same state without errors:

php_inis_managed_myconf:
  file.managed:
    - name: {{ php_conf_name }}
    - user: root
    - group: root
    - mode: 644
    - replace: False
    - require_in:
        file: php_inis_myconf

Attempting this

php_inis_myconf:
  file.managed:
    {# removed for brevity #}

  file.keyvalue:
    {# removed for brevity #}

Will result in errors:

ID 'php_inis_myconf' in SLS 'myconf' contains multiple state declarations of the same type
welcome[bot] commented 1 month ago

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey. Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar. If you have additional questions, email us at saltproject@vmware.com. We’re glad you’ve joined our community and look forward to doing awesome things with you!