Skip to content

Feature: add owner and group filter to builtin.find#76768

Open
phoehnel wants to merge 23 commits into
ansible:develfrom
phoehnel:feature/add-permission-filter
Open

Feature: add owner and group filter to builtin.find#76768
phoehnel wants to merge 23 commits into
ansible:develfrom
phoehnel:feature/add-permission-filter

Conversation

@phoehnel
Copy link
Copy Markdown
Contributor

@phoehnel phoehnel commented Jan 15, 2022

SUMMARY

Adds new parameters to builtin.find to be able to filter results by owner and group.
See examples below for clarification.

The filters are built in a similar way to the existing age and size filter.
They work on the existing data after the file list is obtained.
The filters work for directories, files and links.

Fixes #76586

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

ansible.builtin.find

ADDITIONAL INFORMATION
How to Use
- name: find all elements owned by user and group example
    find: 
        paths: /opt/myapp
        user: example
        group: example

- name: find all elements owned by one of the user in a given list
  find:
    paths: /etc
    users:
      - user1
      - user2

@ansibot

This comment was marked as outdated.

@ansibot ansibot added affects_2.13 feature This issue/PR relates to a feature request. module This issue/PR relates to a module. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. needs_triage Needs a first human triage before being processed. new_contributor This PR is the first contribution by a new community member. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Jan 15, 2022
@phoehnel phoehnel changed the title Feature/add permission filter Feature: add permission filter to builtin.find Jan 15, 2022
@ansibot

This comment was marked as outdated.

@ansibot ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Jan 15, 2022
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jan 15, 2022
@ansibot

This comment was marked as outdated.

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Jan 15, 2022
@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Aug 22, 2023
@ansibot ansibot added needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. and removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Oct 24, 2023
@webknjaz

This comment was marked as resolved.

@azure-pipelines

This comment was marked as resolved.

@webknjaz
Copy link
Copy Markdown
Member

This needs a rebase.

@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Oct 26, 2023
@ansibot ansibot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html and removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. labels Oct 26, 2023
@ansibot ansibot removed the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Feb 5, 2024
@phoehnel
Copy link
Copy Markdown
Contributor Author

phoehnel commented Feb 5, 2024

@webknjaz rebase is done! 👍🏻

@ansibot ansibot removed ci_verified Changes made in this PR are causing tests to fail. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Feb 5, 2024
@sivel
Copy link
Copy Markdown
Member

sivel commented Feb 5, 2024

Outside of the rebase, the permissions functionality will need removed, as it has already been implemented in ca08261 via a slightly different mechanism.

Note, I've not performed any review of this PR.

@phoehnel phoehnel changed the title Feature: add permission, owner and group filter to builtin.find Feature: add owner and group filter to builtin.find Feb 5, 2024
@phoehnel phoehnel requested a review from bcoca February 5, 2024 15:37
@phoehnel
Copy link
Copy Markdown
Contributor Author

phoehnel commented Feb 5, 2024

Outside of the rebase, the permissions functionality will need removed, as it has already been implemented in ca08261 via a slightly different mechanism.

ah fair! didn't saw that. Removed the function now

@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 Feb 19, 2024
@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Jun 18, 2024
@ansibot ansibot added the stale_pr This PR has not been pushed to for more than one year. label Feb 11, 2025
Copy link
Copy Markdown
Contributor

@s-hertel s-hertel left a comment

Choose a reason for hiding this comment

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

This change would need a minor_changes changelog fragment and test cases added to https://github.com/ansible/ansible/blob/devel/test/integration/targets/find/ ensure it keeps working.

Comment on lines +114 to +122
version_added: "2.17"
default: []
groups:
description:
- If set, only elements owned by a group in the list are returned
type: list
aliases: [ group ]
elements: str
version_added: "2.17"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The version_added for the new options should be "2.21" now.

Comment on lines +481 to +491
if (users is None or (iter(users) and len(users) < 1)) and \
(groups is None or (iter(groups) and len(groups) < 1)):
return True
if (iter(users) and len(users) > 0) or (iter(groups) and len(groups) > 0):
stinfo = statinfo(st)
if iter(users) and len(users) > 0:
if stinfo['pw_name'] not in users:
return False
if iter(groups) and len(groups) > 0:
if stinfo['gr_name'] not in groups:
return False
Copy link
Copy Markdown
Contributor

@s-hertel s-hertel Dec 15, 2025

Choose a reason for hiding this comment

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

Calling statinfo for every path more than once feels inefficient. Can we convert the list of users and groups to a list of ints in main() for quick comparison? Then this could be:

Suggested change
if (users is None or (iter(users) and len(users) < 1)) and \
(groups is None or (iter(groups) and len(groups) < 1)):
return True
if (iter(users) and len(users) > 0) or (iter(groups) and len(groups) > 0):
stinfo = statinfo(st)
if iter(users) and len(users) > 0:
if stinfo['pw_name'] not in users:
return False
if iter(groups) and len(groups) > 0:
if stinfo['gr_name'] not in groups:
return False
if users and st.st_uid not in users:
return False
if groups and st.st_gid not in groups:
return False

Comment on lines +505 to +506
users=dict(type='list', default=[], aliases=['user'], elements='str'),
groups=dict(type='list', default=[], aliases=['group'], elements='str'),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

find has -uid/-gid flags too. We could support either with something like:

    uids = []
    for user in params['users']:
        try:
            uid = int(user)
        except ValueError:
            try:
                uid = pwd.getpwnam(user).pw_uid
            except KeyError:
                module.fail_json(msg=f'Failed to look up user {user}')
        uids.append(uid)

    gids = []
    for group in params['groups']:
        try:
            gid = int(group)
        except ValueError:
            try:
                gid = grp.getgrnam(group).gr_gid
            except KeyError:
                module.fail_json(msg=f'Failed to look up group {group}')
        gids.append(gid)

Comment on lines +108 to +115
users:
description:
- If set, only elements owned by a user in the list are returned
type: list
aliases: [ user ]
elements: str
version_added: "2.17"
default: []
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
users:
description:
- If set, only elements owned by a user in the list are returned
type: list
aliases: [ user ]
elements: str
version_added: "2.17"
default: []
users:
description:
- If set, only elements owned by a user in the list are returned.
type: list
elements: str
version_added: "2.21"

Comment on lines +116 to +123
groups:
description:
- If set, only elements owned by a group in the list are returned
type: list
aliases: [ group ]
elements: str
version_added: "2.17"
default: []
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
groups:
description:
- If set, only elements owned by a group in the list are returned
type: list
aliases: [ group ]
elements: str
version_added: "2.17"
default: []
groups:
description:
- If set, only elements owned by a group in the list are returned.
type: list
elements: str
version_added: "2.21"

- name: find all elements owned by user and group root:shadow
find:
paths: /etc
user: root
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would keep the plural form to maintain uniformity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

affects_2.13 feature This issue/PR relates to a feature request. has_issue module This issue/PR relates to a module. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html P3 Priority 3 - Approved, No Time Limitation stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. stale_pr This PR has not been pushed to for more than one year. stale_review Updates were made after the last review and the last review is more than 7 days old. 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.

add executable option to ansible.builtin.find

9 participants