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.

Sign up for GitHub

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

Jump to bottom

look for role/collection deps; report errors #107

Merged
merged 3 commits into from
Oct 27, 2021
Merged

look for role/collection deps; report errors #107

merged 3 commits into from
Oct 27, 2021

Conversation

richm
Copy link
Contributor

@richm richm commented Oct 26, 2021

No description provided.

@richm richm requested review from nhosoi and spetrosi October 26, 2021 23:41
@richm
Copy link
Contributor Author

richm commented Oct 26, 2021

I plan on adding this to tox-lsr and github actions

@nhosoi
Copy link
Contributor

nhosoi commented Oct 27, 2021

I installed the latest rhel-system-roles-#.#.#-1.el9.noarch.rpm and ran report-modules-plugins.py, then it failed as follows:

$ python report-modules-plugins.py /usr/share/ansible/collections/ansible_collections/
Traceback (most recent call last):
  File "/home/nhosoi/linux-system-roles/auto-maintenance/report-modules-plugins.py", line 1041, in <module>
    sys.exit(main())
  File "/home/nhosoi/linux-system-roles/auto-maintenance/report-modules-plugins.py", line 966, in main
    process_path(pth, ctx)
  File "/home/nhosoi/linux-system-roles/auto-maintenance/report-modules-plugins.py", line 533, in process_path
    process_collections(pathname, ctx)
  File "/home/nhosoi/linux-system-roles/auto-maintenance/report-modules-plugins.py", line 526, in process_collections
    process_collection(collection_path, ctx)
  File "/home/nhosoi/linux-system-roles/auto-maintenance/report-modules-plugins.py", line 511, in process_collection
    get_collection_plugins(pathname, ctx)
  File "/home/nhosoi/linux-system-roles/auto-maintenance/report-modules-plugins.py", line 501, in get_collection_plugins
    ctx.add_local_plugins(plugin_dir, dirname)
  File "/home/nhosoi/linux-system-roles/auto-maintenance/report-modules-plugins.py", line 728, in add_local_plugins
    self._load_plugins_from_file(plugin_subdir, plugin_file, plugins)
  File "/home/nhosoi/linux-system-roles/auto-maintenance/report-modules-plugins.py", line 704, in _load_plugins_from_file
    exec(code_str, missingglobals, missinglocals)
  File "<string>", line 1, in <module>
  File "<string>", line 9, in FilterModule
NameError: name 'cidr_merge' is not defined

The undefined cidr_merge is in the vendored plugin ipaddr.py from vpn. I wonder we should not run this tool against the vendored role/collection?

@richm
Copy link
Contributor Author

richm commented Oct 27, 2021

I installed the latest rhel-system-roles-#.#.#-1.el9.noarch.rpm and ran report-modules-plugins.py, then it failed as follows:

$ python report-modules-plugins.py /usr/share/ansible/collections/ansible_collections/
Traceback (most recent call last):
  File "/home/nhosoi/linux-system-roles/auto-maintenance/report-modules-plugins.py", line 1041, in <module>
    sys.exit(main())
  File "/home/nhosoi/linux-system-roles/auto-maintenance/report-modules-plugins.py", line 966, in main
    process_path(pth, ctx)
  File "/home/nhosoi/linux-system-roles/auto-maintenance/report-modules-plugins.py", line 533, in process_path
    process_collections(pathname, ctx)
  File "/home/nhosoi/linux-system-roles/auto-maintenance/report-modules-plugins.py", line 526, in process_collections
    process_collection(collection_path, ctx)
  File "/home/nhosoi/linux-system-roles/auto-maintenance/report-modules-plugins.py", line 511, in process_collection
    get_collection_plugins(pathname, ctx)
  File "/home/nhosoi/linux-system-roles/auto-maintenance/report-modules-plugins.py", line 501, in get_collection_plugins
    ctx.add_local_plugins(plugin_dir, dirname)
  File "/home/nhosoi/linux-system-roles/auto-maintenance/report-modules-plugins.py", line 728, in add_local_plugins
    self._load_plugins_from_file(plugin_subdir, plugin_file, plugins)
  File "/home/nhosoi/linux-system-roles/auto-maintenance/report-modules-plugins.py", line 704, in _load_plugins_from_file
    exec(code_str, missingglobals, missinglocals)
  File "<string>", line 1, in <module>
  File "<string>", line 9, in FilterModule
NameError: name 'cidr_merge' is not defined

The undefined cidr_merge is in the vendored plugin ipaddr.py from vpn. I wonder we should not run this tool against the vendored role/collection?

hmm - should not get a stack trace - looks like a bug - should work fine against a vendored collection

@richm
Copy link
Contributor Author

richm commented Oct 27, 2021

Looks like there is a bug in getting filter plugins. filter plugins are not defined the way module and other plugins are defined - that is, the name of the plugin is not the name of the file (e.g. plugins/modules/foo.py defines a module named foo). Instead, filter plugins are all defined in a single file like this:

somefile.py
...
class FilterModule(object):

    filter_map = {
        # IP addresses and networks
        "cidr_merge": cidr_merge,
        "ipaddr": ipaddr,
        "ipmath": ipmath,
...

    def filters(self):
        return self.filter_map

the names of the filter plugins are the keys of filter_map. The problem with the scanner code is - how to get these values? One way is to use some regex search in the file - but that seems pretty fragile - and in some cases, the filter names could be generated i.e. not hard coded strings like the above. And - it is the filters method that is ultimately responsible for returning the list and it can change the values returned. So I took the approach of parsing the definition using the python exec function. Then, I could override the globals() and locals() so that when I got a reference to an undefined symbol like cidr_merge I could return None, because I don't care about the function, only the name of the plugin. However, it seems that this isn't working - for some reason, my special globals/locals dict is not being referenced for accesses inside the class definition - this is similar to https://stackoverflow.com/questions/2904274/globals-and-locals-in-python-exec - and I cannot figure out a way to intercept variable lookups inside the class definition.

@richm
better handling for filter, test plugin files
58817c1 
Improve parsing of filter and test plugin files.  When we get
a NameError because of an undefined symbol, define that symbol with
a value of `True` and try again, until the `filters` or `tests` method
returns successfully.

Also fixed handling for several other types of files.
@richm richm force-pushed the report-plugins-deps branch from 21938fb to 58817c1 Compare October 27, 2021 19:10
@richm
Copy link
Contributor Author

richm commented Oct 27, 2021

@nhosoi with the latest commit I am able to run the script against the collection from the rpm with the vendored plugins:

# after using rpm2cpio to extract the collection . . .
python ~/linux-system-roles/auto-maintenance/report-modules-plugins.py --details ~/Downloads/usr/share/ansible/collections/ansible_collections
The following ansible.builtin modules are used at runtime:
assert blockinfile command copy debug fail fetch file find getent include_role include_tasks include_vars known_hosts lineinfile meta package package_facts ping reboot service service_facts set_fact setup shell slurp stat systemd tempfile template uri user wait_for_connection
The following ansible.builtin filters are used at runtime:
b64decode basename bool combinations comment extract flatten from_json intersect mandatory min password_hash quote regex_replace regex_search splitext ternary to_json to_nice_json type_debug union unique
The following ansible.builtin tests are used at runtime:
changed failed file match search subset version
The following ansible.builtin lookups are used at runtime:
lookup
The following jinja2 filters are used at runtime:
count d default first format int join length list map reject rejectattr replace select selectattr trim truncate
The following jinja2 tests are used at runtime:
defined iterable mapping none number sameas sequence string undefined

The following additional plugins are used at runtime:


Found 0 errors

@richm
report usage of plugins defined locally
0b3f51b 
for example, on the system roles collection:
```
The following local modules are used at runtime:
blivet certificate_request kernel_settings mount nbde_client_clevis nbde_server_tang network_connections seboolean sefcontext selinux selinux_modules_facts selogin seport timesync_provider
The following local filters are used at runtime:
ipaddr
```
These include the plugins defined by the role, and the vendored plugins.
@richm
Copy link
Contributor Author

richm commented Oct 27, 2021

ok - please review

nhosoi
nhosoi approved these changes Oct 27, 2021
View reviewed changes
Copy link
Contributor

@nhosoi nhosoi left a comment

Choose a reason for hiding this comment

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

Thank you, @richm. It works against the collection including local filters now.
lgtm

@richm richm merged commit 5eb5d61 into linux-system-roles:master Oct 27, 2021
@richm richm deleted the report-plugins-deps branch October 27, 2021 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nhosoi nhosoi nhosoi approved these changes

@spetrosi spetrosi Awaiting requested review from spetrosi

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@richm @nhosoi