salt: Recursive directory management doesn't preserve permissions

OS: Centos 6.3

Example usage:

/opt/elasticsearch:
    file.recurse:
        - source: salt://dist/elasticsearch/0.19.10
        - user: elasticsearch
        - group: elasticsearch
        - include_empty: True
        - require:
            - user: elasticsearch

salt://dist/elasticsearch/0.19.10 just contains an unpacked elasticsearch distribution on the master.

On the minion, everything is set to 644 (files) / 755 (dirs) meaning that scripts are no longer executable, and have to be fixed afterwards.

About this issue

Original URL
State: closed
Created 12 years ago
Comments: 40 (30 by maintainers)

Commits related to this issue

Start on adding file metadata #2707 — committed to saltstack/salt by thatch45 9 years ago
Merge pull request #29369 from thatch45/fs_meta Start on adding file metadata #2707 — committed to saltstack/salt by deleted user 9 years ago

Most upvoted comments

I just wanted to bump this. After some debugging, I found this. Perhaps a note could be added to the documentation saying “Warning: This command will not preserve file permissions.” Personally, I think file.recurse is useless without preserving permissions. Maybe the right answer is to just remove it until it’s implemented correctly, so that people know to either manage files independently, or just copy over a tarball and extract it.

JohnHazen on May 11, 2015

Besides generic problem of filesystem metadata propagation from master to minion, there is another aspect if we look at the problem’s parts:

permissions = ownership + mode

Depending on the implementation, there are opportunities to make it simpler.

I’ve already mentioned it in a duplicate there, but it belongs here in the main discussion.

Ownership ~ “impossible”

There are specific Implementation difficulties which only apply to user/group ownership because master and minion have two “completely unrelated” uid/gid - user/group databases (i.e. differences in /etc/passwd files, or LDAP, or …).

Let’s emphasize: it may be completely impossible to solve ownership problems properly.

Salt has nothing to do with it (unless user/group DB is also managed by the same Salt master). Just think about where ownership information may potentially come from and how inconsistent can it be.

Mode ~ feasible

Setting only mode is straightforward: set r/w/x mode based on the source regardless of the source’s ownership.

And even this is above what is required in most cases…

Where did we see preserving permissions from the source?

What are other worldwide examples with this feature?

Many SCMs (i.e. Git, Subversion, …) support file modes specifically for cases when scripts should be readily executable after checking them out.
Archivers like tar or zip do exactly the same. And this is why scripts run out of the box (out of the tar ball).
…

I can bet the majority of use cases are narrowed down to execution mode on scripts only.

Please comment below if you really want this feature for anything else than making scripts executable.

The (other) security-related permission are better to be treated explicitly via states. The catch here is that read and write modes are closely related to ownership and, therefore, directly associated with “impossible” to implement propagation of ownership. Executable mode, on the other hand, is functionality-related permission (as security it is foolproof only, easily breakable).

In other words, are you sure read and write modes are sane enough to propagate from source (potentially with the different mapping of uid/gid)?

Conclusion

In short, we are likely to need “executable mode from source” only.

Is Windows still a problem in such case? Maybe someone else can comment on this - I’m not sure.

Next step

(Depending on implementation) Maybe it makes sense to split this feature request?

For example:

(simplest, most frequent): “executable mode from source”
(doable, less frequent): “any mode from source”
(hardest): “ownership from source”

DISCLAIMER: Maybe it was obvious to everyone already. Maybe generic solution covers all cases anyway. Then I’m happy to be simply ignored.

uvsmtid on Jun 3, 2016

No problem. This was overdue and I’m glad to have it implemented.

terminalmage on Jul 27, 2016

Working on this right now. I’ve expanded the os.stat() output to the other fileserver backends, and intend to work on the fileclient and file.managed/file.recurse state modifications over the next few days. With luck I’ll have a pull request opened this week.

terminalmage on Jul 12, 2016

@uvsmtid The scope of this is intended to be mode only, for the very reasons you’ve expressed.

In order to implement this, the mode needs to be transmitted to the minion along with the payload(s) containing the file. @thatch45 has implemented this part already, from what he has said, though I haven’t looked at the code to see for which fileserver backends this has been done.

The other parts of this include:

Having the minion read this mode information and use it in states to enforce the files’ modes
Detecting mode-only changes to files

For instance, when a fileserver request is made right now, the minion will pass the checksum of the file in the request, and if the checksum of the file on the fileserver matches, the master skips re-transmitting it to avoid unneceessary retransmission of files. The mode must be added to the fileserver requests, and the master’s response must be able to distinguish changes to the contents of the file from changes to the mode of the file, so that a change to just the mode does not result in retransmission.

terminalmage on Jun 3, 2016

Do we have a fix or a workaround for this when using file.recurse ?

wongster80 on Aug 18, 2014