search

kelseyhightower / confd

Manage local application configuration files using templates and data from etcd or consul
MIT License
8.35k stars 1.41k forks source link

confd failing to read conf.d and templates folders if they happen to be symbolic links #741

Open eduardisimo203 opened 6 years ago

eduardisimo203 commented 6 years ago

We believe that the flaw is here https://github.com/kelseyhightower/confd/blob/93c8904baa404add44565be4e42716eb391f83f9/util/util.go#L89. From what we understand a mode.IsDir() for a symbolically linked folder would not return true

okushchenko commented 6 years ago

@eduardisimo203 yes, that's definitely a bug. Thanks for reporting it! Feel free to open a PR with a fix.

eduardisimo203 commented 6 years ago

After further investigation it seems the issue is NOT with mode.IsDir() call, but instead with the way recursiveLookup is performed. It calls filepath.Walk(...) https://github.com/kelseyhightower/confd/blob/master/util/util.go#L112 which, according to documentation, does not follow symbolic links (https://golang.org/pkg/path/filepath/#Walk)

I found an issue (that got closed) https://github.com/golang/go/issues/17451 that shows why Go decided not to follow symbolic links.

So, unless you decide to use a different method to recurse to find files/folders, at the least it would be a good idea to note this in the release notes as a known issue.

Thanks

abtreece commented 6 years ago

Figured out how to duplicate the bug. Going to tinker with a fix unless you are already working it @eduardisimo203

eduardisimo203 commented 6 years ago

@abtreece please go ahead.

okushchenko commented 6 years ago

@eduardisimo203 let's keep this issue open until it's either fixed or documented properly.

abtreece commented 6 years ago

I haven't had much luck on this. Reading the discussions on filepath.Walk leads me to believe that function will never support symlinks. I believe a solution is possible, but would likely add significant complexity which probably isn't worth it in order to allow the config dirs to be symlinked.

I will update the docs to indicate that symlinks are not supported if that is acceptable alternative.

hubo1016 commented 5 years ago

I think maybe we can follow the link if the root directory (/etc/confd, /etc/confd/conf.d, /etc/confd/templates) are symbol links, then deny any sub folder symbol links, that should be easy to implement ( check symbol link - follow symbol link - walk). This should solve 80% of the problem.

lame commented 5 years ago

I ran into this symlink issue in my Kubernetes deployment using volumeMounts, my work around ended up being to copy the files from the volumeMount directory to another (non-volume) directory, which allowed go's file walk to discover them.

jsok commented 5 years ago

This is a real limitation especially in a Kubernetes environment. It makes a lot of sense to be able to define a confd config in a ConfigMap and mount it into the pod. Kubernetes does this using symlinks so it's out of your control in that case.

Copying the file from the volumeMount to another location (as suggested by @rkk09c) means you cannot update the ConfigMap later and have confd detect template changes automatically.