Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Executable and non-executable inventory files should not be conflated #10068

Closed
dazwin opened this issue Jan 22, 2015 · 20 comments
Closed

Executable and non-executable inventory files should not be conflated #10068

dazwin opened this issue Jan 22, 2015 · 20 comments
Labels
feature This issue/PR relates to a feature request.

Comments

@dazwin
Copy link

dazwin commented Jan 22, 2015

I ran into the following error when running ansible-playbook -i [myinventory], using Vagrant on a Windows host:

ERROR: The file [myinventory] is marked as executable, but failed to execute correctly. If this is not supposed to be an executable script, correct this with `chmod -x [myinventory]`

The problem is that vboxfs (as does many other filesystem options) will set the execute permission on all files if mounted on a filesystem that isn't Unix-like such as FAT or NTFS.

My workaround is ugly - in Vagrant, I am mounting my source files a second time so that I can refer to a version of my inventory file without +x, such as:
config.vm.synced_folder ".", "/vagrant-nox", :mount_options => ["fmode=666"]

I consider the decision to execute an inventory file as a script based solely on its execute permission a bad idea. This is a non-obvious problem and a common use case - here as some examples of other users coming across the same problem:

It also allows a malicious user to execute a script with elevated privileges - all they'd need is write permission on a (non-script) inventory file.

A possible solution is to use a separate command line option if an executable inventory should be used, although this does not provide backwards compatibility.

@bcoca bcoca added P4 labels Jan 26, 2015
@mindware
Copy link

I also ran into this very same issue. I agree with @dazwin and think alternatives should be taken into consideration.

@danthedeckie
Copy link
Contributor

I've also been bitten by this, as we keep our production ansible directory on a (SMB) network share, which can then be mounted by any of our sys-admins. Currently I've had to create a stupid work-around script which copies the inventory (and host_vars and group_vars) out, chmod -x's them, and then uses that version. Very annoying.

@danthedeckie
Copy link
Contributor

@majidaldo
Copy link

+1

@georgegeka
Copy link

I think it's a great idea , and the one that should be very easy to implement

@majidaldo
Copy link

heck as a first rough approach, ansible should know that .sh .exe .py .rb ...etc are executable and others are not.

@bcoca
Copy link
Member

bcoca commented May 18, 2015

@majidaldo eh ... no, executable status on linux/unix is represented by the executable bit on a file, NOT by extension, that is a windows thing.

@majidaldo
Copy link

@bcoca i know but i'm implying that ansible could have a mode where it recognizes executables based on file extension.=> backward compat and makes people interested in this issue happy.

btw, i'm actually experimenting with ansible in cygwin. if i have to goof around too much with it a VM is with a shared folder is the only solution.

@bcoca
Copy link
Member

bcoca commented May 18, 2015

no, mapping by extension is not a solution, we are not going to keep an extension registry in Ansible.

Aside from fixing the filesytem to report correctly on the executable bit, the other possible solution here is trying all the plugins in order and only failing at the end.

@brunitto
Copy link

+1 here

We have to use Windows notebooks under a strict company policy and our way to use ansible was to create a playbooks repository under Git and work as a Vagrant VM. I think that it is a very specific problem and it takes time to analyse and figure out how to merge this into a well defined architecture like Ansible.

Our workaround was to copy the main inventory file (within our repository at /vagrant) to /etc/ansible/hosts, using git pull hook. The limitation is that you can work with just one inventory file, but a well defined group counter attack this.

A believe that the @bcoca got the drive to solve this, using a plugin model, where if a executable file fails to execute it is parsed as inventory file.

@ramondelafuente
Copy link
Contributor

+1, a fallback to parsing the file would have saved a lot of time here.

@naxhh
Copy link

naxhh commented Jun 10, 2015

As @dazwin said we had this issue in phansible. In fact a lot of users have complained about this. And is very confusing for them.

@cheesegrits
Copy link

Can this issue get some love?

@jschank
Copy link

jschank commented Aug 12, 2015

I want to vote up this issue as well.
Perhaps inventory files with .ini, .txt or no extension should ignore the executable permission and be "static" and any dynamic inventories should be required to have an extension.

@apelliciari
Copy link

+1, it's annoying

@bcoca
Copy link
Member

bcoca commented Sep 21, 2015

this is fixed in the merged #11643

@sergeytsivin
Copy link

+1, have the same issue. An additional option that disables dynamic inventory would solve the problem.

@sergeytsivin
Copy link

Sorry, I had this issue with ansible 1.9.4, it seems that the issue is fixed in 2.x series, I tested it with 2.0.0.2 and 2.0.1.0 and static inventory with executable bits set worked fine.

@brunitto
Copy link

brunitto commented Mar 9, 2016

I tested this with 2.x and worked gracefully.

@travisdetert
Copy link

What is this even trying to avoid? What is the intent here? (Honest question? )

@ansibot ansibot added feature This issue/PR relates to a feature request. and removed feature_idea labels Mar 2, 2018
@ansible ansible locked and limited conversation to collaborators Apr 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature This issue/PR relates to a feature request.
Projects
None yet
Development

Successfully merging a pull request may close this issue.