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

Relative ansible cfg lookup #17115

Closed
wants to merge 2 commits into from
Closed

Relative ansible cfg lookup #17115

wants to merge 2 commits into from

Conversation

akamensky
Copy link
Contributor

ISSUE TYPE
COMPONENT NAME

constants.py / locating config

ANSIBLE VERSION
ansible 2.2.0 (devel 30268f6bd0) last updated 2016/08/16 09:26:05 (GMT -400)
  lib/ansible/modules/core: (detached HEAD 45c1ae0ac1) last updated 2016/08/16 09:27:13 (GMT -400)
  lib/ansible/modules/extras: (detached HEAD a6b34973a8) last updated 2016/08/16 09:27:30 (GMT -400)
  config file =
  configured module search path = Default w/o overrides
SUMMARY

Fixes #11175 allowing to traverse from playbook file location towards the root in order to find ansible.cfg

path_list.append(path0)

# Get config file location from CWD
# FIXME: Needs deprecation? See: https://github.com/ansible/ansible/issues/11175#issuecomment-109386699
Copy link
Contributor

@abadger abadger Oct 31, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the conversation further down the issue thread, I think this is staying.


for path in [path0, path1, path2, path3]:
for path in path_list:
if path is not None and os.path.exists(path):
try:
p.read(path)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading the linked feature report, I think this the spec we're trying to implement: #11175 (comment)

So we're missing a warning here if the directory is world writable. I think that looks something like:

import stat
[...]
cfg_directory = os.path.dirname(path)
cfg_directory_stat = os.stat(cfg_directory)
if cfg_directory_stat[stat.ST_MODE] & stat.S_IWOTH:
    display.warning('Reading ansible config file from a world readable directory (%s).  This can be a security risk.  If this is the intended location, please change permissions on the directory.' % cfg_directory)

@abadger
Copy link
Contributor

abadger commented Oct 31, 2016

Hi @akamensky sorry it's taken a while to look at this feature request. Right now is the prime time to get new features into Ansible (for the 2.3.x release). I've commented on this and if you're interested in fixing it up I'll get it in. If not, let me know and I'll try to get it the rest of the way.

If you update and I don't appear to have seen it, please feel free to get in touch with me on IRC. I'm abadger1999 in #ansible-devel on irc.freenode.net.

@abadger
Copy link
Contributor

abadger commented Oct 31, 2016

I've taken a quick look at the cli code... I think I agree with you about there being a need for a large reworking of the code to make that a clean change. I think it could be done uncleanly with just a few things changed but it would be fragile. Future devs would have to know that use of constants and parsing args were being done in a particular order and a particular style out of necessity but that wouldn't be obvious from just reading the code. What's being worked on here seems like it is good enough for now and we can work on other fixes if there's a need later.

@ansibot ansibot added affects_2.3 This issue/PR affects Ansible v2.3 needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Dec 13, 2016
@ansibot ansibot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html and removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Jan 2, 2017
@ssbarnea
Copy link
Member

Do we have any updates on this? I would file really useful if ansible would detect the location of the ansible.cfg by looking for the parent folders.

This would allow people to keep playbooks inside subfolders of their repos without having to be extremely careful from which CWD they are running a playbook.

@bcoca
Copy link
Member

bcoca commented Mar 12, 2017

ansible/proposals#35 will provide this 'feature' in a saner way

@lassizci
Copy link

lassizci commented Apr 6, 2017

This would allow people to keep playbooks inside subfolders of their repos without having to be extremely careful from which CWD they are running a playbook.

That and having e.g forks and other settings that are rather local and not project or repo specific but which used to get overwritten by repo config.

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Apr 13, 2017
@ansibot ansibot added the support:core This issue/PR relates to code supported by the Ansible Engineering Team. label Jun 29, 2017
@gundalow gundalow changed the title Issue 11175 relative ansible cfg lookup Relative ansible cfg lookup Aug 3, 2017
@calfonso
Copy link
Contributor

calfonso commented Aug 3, 2017

Hi!

Thanks very much for your submission to Ansible. It sincerely means a lot to us that you've taken time to contribute.

Unfortunately, we're not sure if we want this feature in the program, and I don't want this to seem confrontational. Our reasons for this are:

We are closing this in favor of a future implementation, noted in the comment regarding proposal 35.

However, we're absolutely always up for discussion. Since this is a really busy project, we don't always see comments on closed tickets, but want to encourage open dialog. You can stop by the development list, and we'd be glad to talk about it - and we might even be persuaded otherwise!

https://groups.google.com/forum/#!forum/ansible-devel
In the future, sometimes starting a discussion on the development list prior to implementing a feature can make getting things included a little easier, but it's not always necessary.

Thank you once again for this and your interest in Ansible!

@ansibot ansibot added the feature This issue/PR relates to a feature request. label Mar 4, 2018
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.3 This issue/PR affects Ansible v2.3 c:constants feature This issue/PR relates to a feature request. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ansible.cfg lookup paths for ansible-playbook
7 participants