search

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.1k stars 5.47k forks source link

file.recurse file_mode: keep doesn't work with salt-ssh #54733

Open nergdron opened 4 years ago

nergdron commented 4 years ago

Description of Issue

When recursively copying a directory to a minion using the file.recurse state, when using the file_mode: keep options, permissions from the source host on files are not actually kept intact, despite documentation saying they will be: https://docs.saltstack.com/en/latest/ref/states/all/salt.states.file.html#salt.states.file.recurse

Setup

/root/some_dir:
  file.recurse:
    - source: salt://{{ slspath }}/some_dir
    - user: root
    - group: root
    - file_mode: keep

Steps to Reproduce Issue

Use salt-ssh to apply the state to a remote system, watch as all files are created with 600 permissions instead of their source permissions.

Versions Report

Salt Version:
           Salt: 2019.2.0

Dependency Versions:
           cffi: 1.12.3
       cherrypy: Not Installed
       dateutil: 2.7.3
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.10.1
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.6.1
   mysql-python: Not Installed
      pycparser: 2.19
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 3.7.3 (default, Aug 20 2019, 17:04:43)
   python-gnupg: Not Installed
         PyYAML: 5.1
          PyZMQ: 18.0.2
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.3.1

System Versions:
           dist: Ubuntu 19.04 disco
         locale: UTF-8
        machine: x86_64
        release: 5.0.0-25-generic
         system: Linux
        version: Ubuntu 19.04 disco
nergdron commented 4 years ago

note that explicitly setting permissions with something like file_mode: 755 works, but then all files have the same permissions. so I'd have to create a ton of individual state entries for each file to get all the permissions right.

xeacott commented 4 years ago

I was just looking into this and the file.recurse state makes it was to calling file.manage in the execution module, which does says this:

    mode
        The permissions to set on this file, e.g. ``644``, ``0775``, or
        ``4664``.

        The default mode for new files and directories corresponds to the
        umask of the salt process. The mode of existing files and directories
        will only be changed if ``mode`` is specified.

        .. note::
            This option is **not** supported on Windows.

        .. versionchanged:: 2016.11.0
            This option can be set to ``keep``, and Salt will keep the mode
            from the Salt fileserver. This is only supported when the
            ``source`` URL begins with ``salt://``, or for files local to the
            minion. Because the ``source`` option cannot be used with any of
            the ``contents`` options, setting the ``mode`` to ``keep`` is also
            incompatible with the ``contents`` options.

        .. note:: keep does not work with salt-ssh.

            As a consequence of how the files are transferred to the minion, and
            the inability to connect back to the master with salt-ssh, salt is
            unable to stat the file as it exists on the fileserver and thus
            cannot mirror the mode on the salt-ssh minion
nergdron commented 4 years ago

ahhh so maybe it needs a documentation note on the file.recursive state page to match that. but also, I assume the files are just getting SCP'd over, so I'm not sure why salt-ssh couldn't stat the files locally and apply those changes remotely, or even just use rsync if available. but that's definitely more into feature request territory.

regardless, it'd be good to have a supported workaround for salt-ssh to do this without having to have like a thousand separate file entries to sync a whole dir.

xeacott commented 4 years ago

At minimum, that information should bleed into the state page, I agree. As far as the implementation moving forward, I'll add a feature request to this until it gets investigated as to exactly why it is this way.

xeacott commented 4 years ago

But there does need to be a separate PR to address the documentation. :)

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

stale[bot] commented 4 years ago

Thank you for updating this issue. It is no longer marked as stale.

petrows commented 3 years ago

Have same issue on salt. Even more, on salt-master i have another issue, test=true does not show changes, but salt.apply does something:

  1. Salt-ssh call on machine: 
    
./bin/apply-ssh pve.pws state.apply                                                                                                            
pve.pws:
----------
...
----------
      ID: pve-backup-code
Function: file.recurse
    Name: /opt/backup
  Result: True
 Comment: Recursively updated /opt/backup
 Started: 20:02:08.571623
Duration: 46.607 ms
 Changes:   
          ----------
          /opt/backup/bin/backup:
              ----------
              mode:
                  0600
          /opt/backup/bin/backup-post:
              ----------
              mode:
                  0600
          /opt/backup/bin/backup-pre:
              ----------
              mode:
                  0600
          /opt/backup/bin/daily-post:
              ----------
              mode:
                  0600
          /opt/backup/bin/daily-pre:
              ----------
              mode:
                  0600
          /opt/backup/bin/git-update:
              ----------
              mode:
                  0600
          /opt/backup/bin/vz-dump:
              ----------
              mode:
                  0600
          /opt/backup/bin/weekly-pre:
              ----------
              mode:
                  0600
          /opt/backup/rsnapshot/daily.conf:
              ----------
              mode:
                  0600

Summary for pve.pws

Succeeded: 103 (changed=5) Failed: 0

Total states run: 103 Total run time: 3.879 s


Same machine on salt-master:

 salt pve.pws state.apply test=true

pve.pws:

Summary for pve.pws

Succeeded: 103 Failed: 0

Total states run: 103 Total run time: 3.538 s

 salt pve.pws state.apply

pve.pws:

      ID: pve-backup-code
Function: file.recurse
    Name: /opt/backup
  Result: True
 Comment: Recursively updated /opt/backup
 Started: 20:08:24.438259
Duration: 181.159 ms
 Changes:
          ----------
          /opt/backup/bin/backup:
              ----------
              mode:
                  0755
          /opt/backup/bin/backup-post:
              ----------
              mode:
                  0755
          /opt/backup/bin/backup-pre:
              ----------
              mode:
                  0755
          /opt/backup/bin/daily-post:
              ----------
              mode:
                  0755
          /opt/backup/bin/daily-pre:
              ----------
              mode:
                  0755
          /opt/backup/bin/git-update:
              ----------
              mode:
                  0755
          /opt/backup/bin/vz-dump:
              ----------
              mode:
                  0755
          /opt/backup/bin/weekly-pre:
              ----------
              mode:
                  0755
          /opt/backup/rsnapshot/daily.conf:
              ----------
              mode:
                  0644

Summary for pve.pws

Succeeded: 103 (changed=1) Failed: 0

Total states run: 103 Total run time: 4.675 s



With `test-true` no changes detected, but call of `state.apply` actually reverts broken permissions from `salt-ssh`