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

rhaido: added FreeBSD/FreeNAS support; tested against FreeNAS 11 ZFS … #51747

Open
wants to merge 4 commits into
base: devel
from

Conversation

Projects
None yet
3 participants
@rhaido
Copy link
Contributor

rhaido commented Feb 5, 2019

SUMMARY

This pull request adds:

  • support of FreeBSD/FreeNAS ACLs including NFSv4 ZFS ACLs (production usage of this feature, verified)
  • support of recursive ACLs (starting from FreeBSD 12)
  • added new parameter/cross-platform feature: 'reset' ACLs. It can be used in the following scenarios:
    a. srip all ACLs - tested for both Linux and FreeBSD/FreeNAS ZFS ACLs
    b. reset ACLs before applying new ones - i.e. in case you want to start from scratch in any case.
  • documentation was updated and examples were added

All additions/features were tested on Linux (Debian 9 Stretch) and on FreeNAS/FreeBSD 11.1

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

acl

Mike Grozak
rhaido: added FreeBSD/FreeNAS support; tested against FreeNAS 11 ZFS …
…acls; recursive functionality verified on FreeBSD 12; added 'reset' ACL feature, which is used to either completely reset ACLs or reset ACLS before an application of new ones
@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Feb 5, 2019

The test ansible-test sanity --test pylint [explain] failed with 4 errors:

lib/ansible/modules/files/acl.py:211:69: bad-whitespace Exactly one space required after comma         if get_platform().lower() == 'freebsd' and etype in ('owner@','group@','everyone@'):                                                                      ^
lib/ansible/modules/files/acl.py:211:78: bad-whitespace Exactly one space required after comma         if get_platform().lower() == 'freebsd' and etype in ('owner@','group@','everyone@'):                                                                               ^
lib/ansible/modules/files/acl.py:224:21: bad-whitespace Exactly one space required after comma     if mode in ('set','rm'):                      ^
lib/ansible/modules/files/acl.py:225:44: bad-whitespace Exactly one space required after comma         cmd = [module.get_bin_path('setfacl',True)]                                             ^

The test ansible-test sanity --test ansible-doc --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/files/acl.py:0:0: missing documentation (or could not parse documentation): expected string or buffer

The test ansible-test sanity --test ansible-doc --python 2.7 [explain] failed with 1 error:

lib/ansible/modules/files/acl.py:0:0: missing documentation (or could not parse documentation): expected string or buffer

The test ansible-test sanity --test ansible-doc --python 3.5 [explain] failed with 1 error:

lib/ansible/modules/files/acl.py:0:0: missing documentation (or could not parse documentation): expected string or bytes-like object

The test ansible-test sanity --test ansible-doc --python 3.6 [explain] failed with 1 error:

lib/ansible/modules/files/acl.py:0:0: missing documentation (or could not parse documentation): expected string or bytes-like object

The test ansible-test sanity --test ansible-doc --python 3.7 [explain] failed with 1 error:

lib/ansible/modules/files/acl.py:0:0: missing documentation (or could not parse documentation): expected string or bytes-like object

The test ansible-test sanity --test pep8 [explain] failed with 4 errors:

lib/ansible/modules/files/acl.py:211:70: E231 missing whitespace after ','
lib/ansible/modules/files/acl.py:211:79: E231 missing whitespace after ','
lib/ansible/modules/files/acl.py:224:22: E231 missing whitespace after ','
lib/ansible/modules/files/acl.py:225:45: E231 missing whitespace after ','

The test ansible-test sanity --test validate-modules [explain] failed with 2 errors:

lib/ansible/modules/files/acl.py:0:0: E305 DOCUMENTATION.options.recursive.description.1: expected str @ data['options']['recursive']['description'][1]. Got {'FreeBSD': 'starting from FreeBSD 12'}
lib/ansible/modules/files/acl.py:0:0: E326 Value for "choices" from the argument_spec (['other', 'user', 'group', 'mask', 'owner@', 'group@', 'everyone@']) for "etype" does not match the documentation (['group', 'mask', 'other', 'user'])

click here for bot help

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Feb 5, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Feb 5, 2019

The test ansible-test sanity --test ansible-doc --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/files/acl.py:0:0: missing documentation (or could not parse documentation): expected string or buffer

The test ansible-test sanity --test ansible-doc --python 2.7 [explain] failed with 1 error:

lib/ansible/modules/files/acl.py:0:0: missing documentation (or could not parse documentation): expected string or buffer

The test ansible-test sanity --test ansible-doc --python 3.5 [explain] failed with 1 error:

lib/ansible/modules/files/acl.py:0:0: missing documentation (or could not parse documentation): expected string or bytes-like object

The test ansible-test sanity --test ansible-doc --python 3.6 [explain] failed with 1 error:

lib/ansible/modules/files/acl.py:0:0: missing documentation (or could not parse documentation): expected string or bytes-like object

The test ansible-test sanity --test ansible-doc --python 3.7 [explain] failed with 1 error:

lib/ansible/modules/files/acl.py:0:0: missing documentation (or could not parse documentation): expected string or bytes-like object

The test ansible-test sanity --test validate-modules [explain] failed with 2 errors:

lib/ansible/modules/files/acl.py:0:0: E305 DOCUMENTATION.options.recursive.description.1: expected str @ data['options']['recursive']['description'][1]. Got {'FreeBSD': 'starting from FreeBSD 12'}
lib/ansible/modules/files/acl.py:0:0: E309 version_added for new option (reset) should be 2.8. Currently 0.0

click here for bot help

@ansibot ansibot removed the ci_verified label Feb 5, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Feb 5, 2019

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/files/acl.py:0:0: E309 version_added for new option (reset) should be 2.8. Currently 0.0

click here for bot help

@ansibot ansibot added the ci_verified label Feb 5, 2019

@rhaido

This comment has been minimized.

Copy link
Contributor Author

rhaido commented Feb 7, 2019

Colleagues, just out of interest - how long it will take to review the proposed change?
Thanks!
M.

@s-hertel s-hertel removed the needs_triage label Feb 12, 2019

@s-hertel

This comment has been minimized.

Copy link
Contributor

s-hertel commented Feb 12, 2019

It may be good to split this up into smaller pull requests for each option and add tests. Currently we don't have FreeBSD 12 support for CI. Here's an issue tracking that #48781.

@rhaido

This comment has been minimized.

Copy link
Contributor Author

rhaido commented Feb 13, 2019

FreeBSD 12 is only necessary for recursive ACL setup. If I just put back an original line of code, which restricts back recursive option to Linux platforms, will it help to get through the review?

Thanks in advance!

@ansibot ansibot added the stale_ci label Feb 21, 2019

@ansibot ansibot added the files label Mar 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.