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

Add support to apply ACL to Junctions #423

Merged
merged 10 commits into from
Nov 2, 2022

Conversation

gillg
Copy link
Contributor

@gillg gillg commented Oct 7, 2022

SUMMARY

Currently applying ACLs on a SymLink or Junction just applies the ACLs to the "link" not to the target.
There was no way to follow links, what seems to be the first intent when you set ACLs

I didn't added a new official variable for now, I let you decide if it's necessary or not.

Copy link
Collaborator

@jborean93 jborean93 left a comment

Choose a reason for hiding this comment

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

It would be great if we could have some tests to cover this scenario and a changelog as per https://docs.ansible.com/ansible/latest/community/development_process.html#changelogs-how-to.

plugins/modules/win_acl.ps1 Outdated Show resolved Hide resolved
plugins/modules/win_acl.ps1 Outdated Show resolved Hide resolved
@jborean93
Copy link
Collaborator

Sorry I led you a tiny bit astray in my recommended example. When writing some tests for this functionality I realised a few things were missing. I've since pushed a commit that:

  • Fixed my incorrect suggestion
  • Added tests
  • Changed the default for follow to false

Unfortunately having follow: true as a default would be a breaking change for this module so I've had to change the default to $false. This keeps things aligned with the win_stat, stat modules as well.

Thanks for working on this one though!

@jborean93 jborean93 merged commit 2d15d14 into ansible-collections:main Nov 2, 2022
@gillg
Copy link
Contributor Author

gillg commented Nov 2, 2022

Thank you a lot @jborean93 I would not be comfortable to write the tests anyway!
And fixing my English in the changelog is always a good idea 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants