Unable to handle symlinks in GitFS repositories

eliasp commented 9 years ago

Salt:

               Salt: 2014.7.1
             Python: 2.7.6 (default, Mar 22 2014, 22:59:56)
             Jinja2: 2.7.2
           M2Crypto: 0.21.1
     msgpack-python: 0.3.0
       msgpack-pure: Not Installed
           pycrypto: 2.6.1
            libnacl: Not Installed
             PyYAML: 3.10
              ioflo: Not Installed
              PyZMQ: 14.0.1
               RAET: Not Installed
                ZMQ: 4.0.4
               Mako: 0.9.1

OS: Ubuntu 14.04.2 LTS (amd64)

GitFS backend: python-git-0.3.2~RC1-3

Using symlinks seems to break Salt in several places.

What I'm trying to do:

Setup a GitFS repository salt-formulas which contains multiple git subtree clones (originally wanted to use submodules, see also #13664) of public formula repos. The reasons for not simply adding the repositories of the public formulas to the SaltMaster's fileserver config are:

adding/removing a salt-formula would require changes of the master config and therefore a restart of the master
Untested code from public repositories would be used directly for our critical infrastructure. By proxy-ing them through our own repo as subtree modules we can control which upstream version is used and test it in a separate branch/environment before.

This way, I wanted to have a directory structure like this:

├── samba -> samba-formula/samba
└── samba-formula
    ├── LICENSE
    ├── pillar.example
    ├── README.rst
    └── samba
        ├── client.sls
        ├── config.sls
        ├── files
        │   └── smb.conf
        ├── init.sls
        └── map.jinja

As the formulas internally often use absolute references such as include: samba.config, having samba-formula in the root doesn't work. Besides that, being able to include the formula in own states by simply including samba instead of samba-formula/samba would also be a plus. So either the content of samba-formula needs to be moved to the repository's root (which would then break the repository and make further updates impossible) or a symlink samba pointing to samba-formula/samba needs to be created.

This would have made it possible to have a repository of repositories which could be comfortably managed and kept in sync with their upstreams.

Using a symlink to not break the repositories leads to several issues:

Jinja: Jinja is unable to render anything as it can't find the file referenced through a symlink:

    Rendering SLS 'dev-feature-linux_domain_membership :samba-formula/samba.config' failed: Jinja error: samba/map.jinja
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/salt/utils/templates.py", line 286, in render_jinja_tmpl
    output = template.render(**unicode_context)
  File "/usr/lib/python2.7/site-packages/jinja2/environment.py", line 969, in render
    return self.environment.handle_exception(exc_info, True)
  File "/usr/lib/python2.7/site-packages/jinja2/environment.py", line 742, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "<template>", line 1, in top-level template code
  File "/usr/lib/python2.7/site-packages/salt/utils/jinja.py", line 132, in get_source
    raise TemplateNotFound(template)
TemplateNotFound: samba/map.jinja

; line 1

---
{% from "samba/map.jinja" import samba with context %}    <======================

include:
  - samba

samba_config:
[...]

SLS: Referencing those formulas in states fails like this:

include:
    - samba

… fails like this:

    Specified SLS ntp in saltenv dev-feature-linux_domain_membership is not available on the salt master

So it seems like at some point symlinks from GitFS sources aren't handled properly at all and end up invisible/unusable for consumers such as the renderer or state module.

rallytime commented 9 years ago

@eliasp I am fairly certain that this has been fixed in #18400, but I think that fix only made it into 2014.7.2 and not 2014.7.1. Would you mind giving this a try on 2014.7.2 and see if you're still running into this problem?

ping @terminalmage

terminalmage commented 9 years ago

2014.7.2 had a gitfs regression with the repo locking stuff. It should work OK, but there might be some instability there if you do a manual salt-run fileserver.update while an update is in progress.

eliasp commented 9 years ago

Thanks for the heads up. 2014.7.2 isn't available on Ubuntu yet, but I manually deployed salt/fileserver/gitfs.py (and required other files) from current 2014.7 to my master, wiped the cache, restarted the master and ran salt-run fileserver.update.

The result is unfortunately still the same.

terminalmage commented 9 years ago

OK, this was fixed in the 2014.7 branch but appears to have since regressed. I'm looking at it now.

terminalmage commented 9 years ago

Actually, the issue that I fixed was with serving symlinks, this is something different.

vielmetti commented 9 years ago

I'm following along here as @eliasp seems to have a reasonable design for how this should work (that's better than the approach I tried in #22084).

eliasp commented 9 years ago

Some updates on this one… eliasp/salt@f67e2fe makes it a bit more clear where the problem originates from. Jinja's template renderer shows now clearly the absolute path of the file it was trying to open.

In this case /var/cache/salt/minion/files/base/fail2ban/map.jinja, while one would expect the result of the resolved symlink (through os.readlink()) to be /var/cache/salt/minion/files/base/fail2ban-formula/fail2ban/map.jinja instead.

For comparison: /var/cache/salt/minion/files/base/fail2ban/map.jinja - provided to the Jinja template renderer /var/cache/salt/minion/files/base/fail2ban-formula/fail2ban/map.jinja - expected full/resolved path

Digging deeper it looks like the GitFS cache of the fileserver already "flattens" the symlink-based directory structure:

/var/cache/salt/master/gitfs/refs/base/fail2ban
/var/cache/salt/master/gitfs/refs/base/fail2ban-formula
└── fail2ban
    └── init.sls

fail2ban which one would expect to be a symlink to fail2ban-formula/fail2ban is just an empty directory.

So this looks to me like it's definitely a problem with GitFS.

eliasp commented 8 years ago

Finally tracked the problem down…: salt/fileclient.py: (list_states())

 454             for path in self.file_list(saltenv):
 455                 if salt.utils.is_windows():
 456                     path = path.replace('\\', '/')
 457                 if path.endswith('.sls'):
 458                     # is an sls module!
 459                     if path.endswith('{0}init.sls'.format('/')):
 460                         states.append(path.replace('/', '.')[:-9])
 461                     else:
 462                         states.append(path.replace('/', '.')[:-4])

As this uses file_list() to acquire a list of files and then filters out non-SLS files to create the list of states, it'll end up without e.g. fail2ban which points to fail2ban-formula/fail2ban - containing init.sls and thereby should be finally resolving fail2ban to fail2ban-formula.fail2ban.

I don't have a PR ready for this yet - @terminalmage do you have a sane approach to resolving this?

terminalmage commented 8 years ago

I can't think of the solution off the top of my head but I am working on some fileserver stuff this week and will try to figure something out.

I just want to make sure we're on the same page first, though. Are you saying that fail2ban/init.sls doesn't show up in the output from salt-run fileserver.file_list? If so, I might have an idea of why. For the default fileserver backend we use os.walk() to find all the files in the configured file_roots directories. GitFS has no direct parallel to this, and thus I don't think we follow symlinks when they go into subdirectories. This sort of logic must be specifically be implemented for each gitfs provider backend, and it's something I can probably do while I'm poking around in the gitfs code this week working on an unrelated issue.

eliasp commented 8 years ago

Are you saying that fail2ban/init.sls doesn't show up in the output from salt-run fileserver.file_list?

@terminalmage That's correct and the reason for this happening is, that fail2ban is listed, as a symlink (which is technically a file), but it doesn't result in being added as a state as this would require:

the symlink to be resolved
the absolute target of the symlink (in the worst case, it might be a symlink to another symlink) needs to be checked whether it's a foo.sls or foo/init.sls and if so, added as state by the name of the original symlink.

Although I don't have the case with fail2ban around here anymore at the moment, I can provide the output for epel-formula for which I have the exactly same layout in my repository here:

# salt-run fileserver.file_list | grep epel
- epel
- epel-formula/LICENSE
- epel-formula/README.rst
- epel-formula/epel/init.sls
- epel-formula/pillar.example

It looks like this in the repository:

# ls -ltr *epel*
lrwxrwxrwx 1 elias elias   17 Jul 23  2015 epel -> epel-formula/epel

epel-formula:
total 16
-rw-r--r-- 1 elias elias  593 Jan 16 00:58 LICENSE
-rw-r--r-- 1 elias elias  606 Jun 22 12:53 README.rst
-rw-r--r-- 1 elias elias  948 Jun 22 13:00 pillar.example
drwxr-xr-x 2 elias elias 4096 Jun 22 13:00 epel

So I can't reference it as epel but only as epel-formula.epel.

terminalmage commented 8 years ago

OK, then I believe I have a handle on this. Essentially, in gitfs when a symlink points to a file we follow the symlink and return the file to which it points, but when the symlink points to a directory we do not follow it. This used to not be done for any of the backends, it was added at one point for the default (roots) backend via an option called fileserver_followsymlinks but never expanded to the other backends.

Once I implement this for gitfs, then the symlinked directory will contain all of the files/dirs underneath the directory to which it points. So, your example would look like this:

# salt-run fileserver.file_list | grep epel
- epel/LICENSE
- epel/README.rst
- epel/epel/init.sls
- epel/pillar.example
- epel-formula/LICENSE
- epel-formula/README.rst
- epel-formula/epel/init.sls
- epel-formula/pillar.example

the epel symlink should not be showing up in the file list, probably at all. Out of curiosity, what happens when you run the following command? (replace myminion with any minion ID)

salt myminion state.single file.managed /tmp/epel source=salt://epel

stale[bot] commented 6 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.

TheLocehiliosan commented 5 years ago

This still seems to be an issue. @terminalmage - is this a change that is still planned? I think this feature is pretty critical. It is very important to adding multiple 3rd party formulas into a single repo and still be able to maintain the subtree with upstream changes.

TheLocehiliosan commented 5 years ago

@terminalmage - One other comment on this for now. I started digging into the code, and it looks like salt DOES follow the links. For paths which are found to be symlinks, the blob referenced by the corresponding path is used.

I think the major problem is when symlinks are pointing to directories. Off hand, I think Git does not manage directories at all, just data within paths (for example, you are not able to simply manage an empty directory in Git). So there is probably no blob data for the path. I did confirm I could use a symlink that pointed to a file instead of a directory, and in that case the data was available to the minion.

terminalmage commented 5 years ago

This would be nice to implement, but I have many more critical projects to work on at the moment. I can reopen this.

stale[bot] commented 5 years ago

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

sagetherage commented 3 years ago

The core team will not be able to get to this in the Aluminium release - the community can open a PR against it, and I will attempt to get this into Silicone

jheiselman commented 2 years ago

I have another use case to add to this. We use an automated CA to create certificates for things like web servers, Tomcat servers, etc. We keep those in a secured NFS location and then symlink that into our root fs so that the certificates can be sent out via Salt FS.

Our current project has us trying to use salt-master and salt-api from within Docker. I have mounted the NFS share on the host and bind mounted it into the container. All of that works as expected. But now, we are seeing the same issue were the symlink in our GitFS is being flattened and turned into an ordinary file, so we can not reference the proper location now in our formulas/states.

twangboy commented 1 year ago

This is still an issue on 3006.2

saltstack / salt

Unable to handle symlinks in GitFS repositories #21413