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

my implementation of a file acl module #1

Closed
wants to merge 1 commit into from
Closed

my implementation of a file acl module #1

wants to merge 1 commit into from

Conversation

b6d
Copy link

@b6d b6d commented Feb 11, 2014

Hello,

a while ago, I intended to propose a few changes to Ansible's ACL-module. I based these on the version in release 1.4.4 since I had troubles using the devel-branch at the time.

Since then, Ansible's acl-module has changed quite a bit, and my changes are not compatible any more. However, maybe my version of the module can still provide a little input - this is why I send this request. (this is neither intended to exist alongside the current acl module, nor to replace it)

PS: Unfortunately, I wrongly sent an unfinished version of this pull request to ansible/ansible instead of bcoca/ansible about an hour ago. This was not intended, and I apologize for any confusion.

A few comments regarding the module

  • Main goal was to allow multiple entries instead of a single one while retaining compatibility to the (then) current form of the module. There are two reasons behind this: First, default ACL entries can (afaik) only be removed, if all three of them are removed. Second, I often have loops over several directories we want to set ACLs to, and I wanted to avoid an inner loop for setting all entries of a single directory.
  • Another thing I wanted was to be able to specify a complete ACL and have the module set this ACL exactly - in other words, entries not specified should not be in the ACL after the module has run. This can be done by supplying remove_unspecified=yes
  • The module relies heavily on the GNU implementation of setfacl (the one that came packaged with ubuntu 12.04). I suspect you use a different one, since my setfacl does not understand the parameter "-h" as yours seems to (follow symlinks).
  • Always anticipating changes correctly for check_mode=True seems not as straightforward as I had hoped, especially when supplying multiple ACL entries. After I failed replicating setfacl's logic regarding this in a simple, easily understandable way, I decided to let setfacl do the checking (setfacl --test) and evaluate it's results. This seems to work nicely, at least with my version of setfacl.
  • I tried to have internal representations of ACL entries (Keys and Permissions). An ACL in the module is then a dict of Keys mapping to Permissions.
  • I tried to parse ACL entries via regex, it should hopefully interpret them the same way as setfacl.
  • A parameter to purge the complete ACL was added (state=empty)
  • The module is not well-tested at the moment

I hope that one or the other fragment of my acl-implementation can be useful in the further development of Ansible's ACL module

regards,

Bernhard

this is neither intended to exist alongside the current acl module, nor to
replace it, but to provide input which (I hope) can contribute the current acl
module's development
@bcoca
Copy link
Owner

bcoca commented Feb 11, 2014

hi, I appreciate the feedback and contribution.

#1) I avoided doing multiple entries in a single invocation as it complicated the code a lot, I left it up to the user and with_items:

#2) The updated module does something very close to this, it just always uses the long form of an entry.

#3) Not really, relies mostly on FreeBSD implementation (which is what I was using at the time), since the command is not in Posix standards it might vary (I've tested on Gentoo, Ubuntu and Debian also). The -h was a typo, I need to remove it. It was a placeholder for when I was still contemplating the recursive function.

#4) yep, this one is not as easy as one would think, the last version does a much better job now. I doubt I'll be changing as --test is not available on all setfacl binaries I have used.

#5) this is what the posix1e module already does, but I was asked not to use it to reduce dependencies and in the end it is simpler code.

#6) I do string matching on the first character, the result is the same.

#7) we already have that with the using the ansible convention state=absent

#8) that is what production is for! kidding aside, I have a few minor tests I haven't pushed to the main repo yet, they aren't perfect but it is a start.

Just to point out, I'm not an ansible maintainer, just the module author, discussion or ideas on how to enhance ansible might be better on the ansible-devel mailing list (google group). At least there you'll get more opinions than just mine (also that is where the guys with commit privileges to ansible proper will be).

@b6d
Copy link
Author

b6d commented Feb 11, 2014

Thanks for your quick reply,

I'm looking forward to the acl-module in Ansible 1.5

@bcoca
Copy link
Owner

bcoca commented Feb 11, 2014

We might be able to implement a 'clear all and insert this' with:

-b Remove all ACL entries except for the three required entries

just might be a bit complicated with 'changed' status

@bcoca
Copy link
Owner

bcoca commented Feb 12, 2014

closing this as I moved the conversation to ansible-devel

@bcoca bcoca closed this Feb 12, 2014
bcoca pushed a commit that referenced this pull request Mar 2, 2014
added octal representation of mode and made md5 checksumming optional
bcoca added a commit that referenced this pull request Aug 18, 2015
… own modules

falls back to current service module and requries service mgr facts and new service modules
bcoca pushed a commit that referenced this pull request Aug 30, 2015
bcoca pushed a commit that referenced this pull request Sep 26, 2015
More tests, better handling of list arguments to _split_pattern
bcoca pushed a commit that referenced this pull request Oct 3, 2015
Now we have the following ways to set additional arguments:

1. [ssh_connection]ssh_args in ansible.cfg: global setting, prepended to
   every command line for ssh/scp/sftp. Overrides default ControlPersist
   settings.
2. ansible_ssh_common_args inventory variable. Appended to every command
   line for ssh/scp/sftp. Used in addition to ssh_args, if set above, or
   the default settings.
3. ansible_{sftp,scp,ssh}_extra_args inventory variables. Appended to
   every command line for the relevant binary only. Used in addition to
   #1 and #2, if set above, or the default settings.
3. Using the --ssh-common-args or --{sftp,scp,ssh}-extra-args command
   line options (which are overriden by #2 and #3 above).

This preserves backwards compatibility (for ssh_args in ansible.cfg),
but also permits global settings (e.g. ProxyCommand via _common_args) or
ssh-specific options (e.g. -R via ssh_extra_args).

Fixes ansible#12576
bcoca pushed a commit that referenced this pull request Oct 15, 2015
bcoca pushed a commit that referenced this pull request Nov 12, 2015
bcoca pushed a commit that referenced this pull request Jan 27, 2016
bcoca pushed a commit that referenced this pull request Mar 9, 2016
This implements solution #1 in the proposal ansible#14860.

It only shows the diff if the task induced a change, which means that if the changed_when control overrides the task, not diff will be produced.
See ansible#14860 for a rationale and the use-case.
bcoca pushed a commit that referenced this pull request Jun 6, 2016
bcoca pushed a commit that referenced this pull request Sep 2, 2016
Merge to latest upstream
bcoca pushed a commit that referenced this pull request Oct 13, 2016
bcoca pushed a commit that referenced this pull request Feb 17, 2017
* Add a vault 'encrypt_string' command.

The command will encrypt the string on the command
line and print out the yaml block that can be included
in a playbook.

To be prompted for a string to encrypt:
   
   ansible-vault encrypt_string --prompt

To specify a string on the command line:

   ansible-vault encrypt_string "some string to encrypt"

To read a string from stdin to encrypt:

   echo  "the plaintext to encrypt" | ansible-vault encrypt_string

If a --name or --stdin-name is provided, the output will include that name in yaml key value format: 

   $ ansible-vault encrypt_string "42" --name "the_answer"
    the_answer: !vault-encrypted |
          $ANSIBLE_VAULT;1.1;AES256
          <vault cipher text here>

plaintext provided via prompt, cli, and/or stdin can be mixed:

      $ ansible-vault encrypt_string "42" --name "the_answer" --prompt
      Vault password: 
      Variable name (enter for no name): some_variable
      String to encrypt: microfiber
      # The encrypted version of variable ("some_variable", the string #1 from the interactive prompt).
     some_variable: !vault-encrypted |
              $ANSIBLE_VAULT;1.1;AES256
              < vault cipher text here>
      # The encrypted version of variable ("the_answer", the string #2 from the command line args).
      the_answer: !vault-encrypted |
             $ANSIBLE_VAULT;1.1;AES256
             < vault cipher text here>

Encryption successful
* add stdin and prompting to vault 'encrypt_string'
* add a --name to encrypt_string to optional specify a var name
* prompt for a var name to use with --prompt
* add a --stdin-name for the var name for value read from stdin
bcoca pushed a commit that referenced this pull request Oct 24, 2018
* Add Support of healthcheck in docker_container module

Fixes ansible#33622
Now container can be started with healthcheck enabled

Signed-off-by: Akshay Gaikwad <akgaikwad001@gmail.com>

* Extend docker_container healthcheck (#1)

* Allowing to disable healthcheck.

* Added test for healthcheck.

* Make sure correct types are used.

* Healthcheck needs to be explicitly disabled with test: ['NONE'].

* pep8 fixes

Signed-off-by: Akshay Gaikwad <akgaikwad001@gmail.com>

* Fix bug if healthcheck interval is 1 day or more

`timedelta` object has days too and seconds are up to one day.
Therefore use `total_seconds()` to convert time into seconds.

Signed-off-by: Akshay Gaikwad <akgaikwad001@gmail.com>

* Add test for healthcheck when healthcheck is not specified

This is to avoid the situation when healthcheck is not specified and
treat this as healthcheck is changed or removed.

Signed-off-by: Akshay Gaikwad <akgaikwad001@gmail.com>

* Convert string syntax for healthcheck test to CMD-SHELL

Also add another test case to check idempotency when healthcheck test
is specified as string

Signed-off-by: Akshay Gaikwad <akgaikwad001@gmail.com>

* Playbook fails if minimun docker version is not satisfy for healthcheck

This is to make more consistent with other non-supported options.

Signed-off-by: Akshay Gaikwad <akgaikwad001@gmail.com>
bcoca pushed a commit that referenced this pull request Feb 18, 2019
* postgresql_ext: cascade extension creating/deleting (#1)

* postgresql_ext: cascade extension creating/deleting

* Address code review feedback
bcoca pushed a commit that referenced this pull request Feb 18, 2019
* Enable 'changed' var with ufw check mode

* Fix from comment of the PR + Unit Test

* Fix on ufw module after the second review

- delete rules change works in check mode
- simplify execute def & use it on every call process
- improved regexp
- rename vars defaults to current_default_values

* Add ignore error to execute() and use it in get_current_rules()

* Update after third code review (introduce change in changed status)

* Adjust tests and fix some problems (#1)

* 'active' also appears in 'inactive'.

* 'reject' is also a valid option here.

* For example for reloaded, changed will be set back to False here.

* Improve and adjust tests.

* Fix after merging integration test

* handle "disabled" on default routed

* Add /var/lib/ufw/.. rules files

* add unit test

* Fix pep8 formatting error

* Separate ipv6 and ipv4 rules process from checkmode

* fix non-ascii error on ci

* Some change after review

* Add unit test with sub network mask

* rename is_match function by is_starting

* add changelog fragment
bcoca pushed a commit that referenced this pull request Mar 8, 2019
…nsible#49191) (ansible#53445)

* Fix for "AttributeError: 'module' object has no attribute 'cursors'" (ansible#49191) (#1)

* Fix for "AttributeError: 'module' object has no attribute 'cursors'" (ansible#49191)

* Adding changelog fragment for issue ansible#49191 and the following PR.

* Update lib/ansible/module_utils/mysql.py

Co-Authored-By: timorunge <timorunge@users.noreply.github.com>
bcoca pushed a commit that referenced this pull request Apr 2, 2019
* Add win_hosts module

added win_hosts module for easier manipulation of hosts entries in "%windir%\system32\drivers\etc\hosts" for windows systems

* Update win_hosts.py

* Add alias support to win_hosts module (#1)

* win_hosts supports aliases

added support for adding / removing aliases from a host entry, rather than adding a new entry

added ability for win_hosts to detect aliases:
`192.168.1.1 alias1 alias2 alias3`
```
win_hosts:
  host_name: alias2
  ip_address: 192.168.1.1
```
will result in `192.168.1.1 alias1 alias3`

also includes `replace` and `add` as options for `ip_action` (`replace` is default)

for example:
```
192.168.1.1 my_reused_alias
192.168.1.2 my_reused_alias
```
with
```
win_hosts:
  host_name: my_reused_alias
  ip_address: 192.168.1.3
  ip_action: add
```
the result will be
```
192.168.1.1 my_reused_alias
192.168.1.2 my_reused_alias
```
but with `ip_action=replace` the result would be
```
192.168.1.3 my_reused_alias
```

* fixed metadata version and version added

* fix line endings

* upload fixed line endings

try to upload the file with the fixed line endings

* aliases and canonical names are separate entities. added IPv4 and IPv6 validation

* only makes changes if "check_mode" is false

* improved behavior for duplicate aliases/entries.

* adding tests

* missing aliases file

* fix trailing whitespace and uses explicit paths

* Tweak tests to copy and restore original hosts file
bcoca pushed a commit that referenced this pull request Apr 11, 2019
…nsible#49191) (ansible#53445)

* Fix for "AttributeError: 'module' object has no attribute 'cursors'" (ansible#49191) (#1)

* Fix for "AttributeError: 'module' object has no attribute 'cursors'" (ansible#49191)

* Adding changelog fragment for issue ansible#49191 and the following PR.

* Update lib/ansible/module_utils/mysql.py

Co-Authored-By: timorunge <timorunge@users.noreply.github.com>
(cherry picked from commit b45b599)
bcoca pushed a commit that referenced this pull request Jan 13, 2020
ansible#65228)

* adding encoding dump/import support for the mysql_db module, with updated documentation, and full test suite

* fixing lint issue test #3

* fixing lint issue test #1

* fixing lint issue test #1 second time

* Improving Test to be re-entrant

* improving test to not fail on centos/6

* Update test/integration/targets/mysql_db/tasks/encoding_dump_import.yml

Comminting suggestion

Co-Authored-By: Benjamin MALYNOVYTCH <bmalynovytch@users.noreply.github.com>

* Update test/integration/targets/mysql_db/tasks/encoding_dump_import.yml

comminting suggestion

Co-Authored-By: Benjamin MALYNOVYTCH <bmalynovytch@users.noreply.github.com>

* adding comment

Adding comment to explain test strategy

* Update test/integration/targets/mysql_db/tasks/encoding_dump_import.yml

Co-Authored-By: Benjamin MALYNOVYTCH <bmalynovytch@users.noreply.github.com>

* Update test/integration/targets/mysql_db/tasks/encoding_dump_import.yml

Co-Authored-By: Benjamin MALYNOVYTCH <bmalynovytch@users.noreply.github.com>

* Update test/integration/targets/mysql_db/tasks/encoding_dump_import.yml

accepted

Co-Authored-By: Andrey Klychkov <aaklychkov@mail.ru>

* Update test/integration/targets/mysql_db/tasks/encoding_dump_import.yml

Co-Authored-By: Andrey Klychkov <aaklychkov@mail.ru>

* Update encoding_dump_import.yml

* Fixing typoo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants