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

Ensure provided mode is compared on equal grounds with existing mode #14769

Closed
wants to merge 1 commit into from

Conversation

dagwieers
Copy link
Contributor

Issue Type:
  • Bugfix Pull Request

This fixes #14771 for Ansible 2.x

Ansible Version:
ansible 2.0.1.0
  config file = /home/dag/home-made/ansible.testing/ansible.cfg
  configured module search path = Default w/o overrides
Summary:

We noticed that doing template/copy on files with a slightly different octal mode than expected, check-mode would result in a change, while a real run would return ok.

We traced this to the fact that Ansible converts existing permissions using sstat.S_IMODE(), however it doesn't do so with the provided mode. So when we provided '0100755', it compared '0755' with '0100755'. In a real run this would be equal, but in check-mode it would be considered a change.

We noticed that doing template/copy on files with a slightly different octal mode than expected, check-mode would result in a change, while a real run would return ok. We traced this to the fact that Ansible converts existing permissions using sstat.S_IMODE(), however it doesn't do so with the provided mode. So when we provided '00755', it compared '0755' with '00755'. In a real run this would be equal, but in check-mode it would be considered a change.
dagwieers added a commit to dagwieers/ansible that referenced this pull request Mar 3, 2016
…(Python 1.9)

This is a backport of ansible#14769

We noticed that doing template/copy on files with a slightly different octal mode than expected, check-mode would result in a change, while a real run would return ok.

We traced this to the fact that Ansible converts existing permissions using sstat.S_IMODE(), however it doesn't do so with the provided mode. So when we provided '00755', it compared '0755' with '00755'. In a real run this would be equal, but in check-mode it would be considered a change.
@bcoca
Copy link
Member

bcoca commented Mar 3, 2016

I don't think anyone ever tried setting the 'filesystem type' part of the mode before ... I'm not sure if we should 'accept' it and deal with it or error out as it seems like bad input.

@dagwieers
Copy link
Contributor Author

I understand. What I wanted to fix was the difference between check-mode and a real run.

Currently a real run does accept the "raw" file mode and handles it correctly. So I fixed the check-mode to do the same. Again, as you say, it's unlikely a lot of people will experience this.

PS The meaning of 'mode' is in fact the complete file mode, and not just the permission bits. So if we just take it by its name we should be liberal to what we accept... :-)

@bcoca
Copy link
Member

bcoca commented Mar 3, 2016

@dagwieers there are probably a dozen people that would interpret mode that way, outside the other 10 that actually implement this feature inside the filesystem. Most understand mode as the permissions, specially since that is the only things most people have access to change, the other 'mode bits' are just descriptive of what the filesystem object is and not really meant to be changed directly by the user (would be fun changing filetype from device to symlink ....) .

@dagwieers dagwieers mentioned this pull request Mar 4, 2016
@tonk
Copy link
Contributor

tonk commented Mar 4, 2016

I completely missed the link between this pull and issue #14771. Sorry for that. Please, ignore my fix.

@bcoca bcoca added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Mar 4, 2016
@bcoca bcoca added this to the stable-2.0 milestone Mar 4, 2016
@dagwieers dagwieers closed this Mar 4, 2016
@jimi-c jimi-c removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Mar 4, 2016
@ansibot ansibot added bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Mar 5, 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
bug This issue/PR relates to a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using full st_mode results in difference in check-mode run and real run
5 participants