New collection: trendmicro.deepsec

[justjais]

trendmicro.deepsec is Trendmicro Deepsecurity Endpoint security Ansible security automation supported collection

Ansible Galaxy: https://galaxy.ansible.com/trendmicro/deepsec
GitHub: https://github.com/ansible-collections/trendmicro.deepsec
Issues Tracker: https://github.com/ansible-collections/trendmicro.deepsec/issues
GitHub Handles:
Is the collection part of Automation Hub: Yes
We meet Collection Requirements: Yes

[justjais]

@tadeboro Can you please include the TM deepsec collection to Ansible package. It does fulfill all the checklist WRT addition of new collections to Ansible package.

[tadeboro]

Thank you for your submission, @justjais.

The items in the checklist can be in one of the following states:

• Items with a checkmark and no attached comment are OK and need no intervention from your side.
• Items with a checkmark that contain a comment will not block the inclusion process, but by following the advice given in the comment, the collection's quality can be improved.
• Items with no checkmark and no extra details are waiting for evaluation.
• Items with no checkmark that contain a comment need your attention.

Feel free to ask any questions and provide feedback. Thanks in advance for your understanding and patience.

For details about the following points, refer to the Collection Requirements.

Every comment should say whether the reviewer expects it to be addressed, or whether it's optional.

Public availability and communication:

• published on Ansible Galaxy Ansible Galaxy with version 1.0.0 or later
• has a policy of releasing, versioning and deprecation announced to contributors and users in some way
  MUST FIX: The collection documentation does not contain this information. See community.sops, dellemc.sonicenterprise, or sensu.sensu_go for example.
• have a Code of Conduct (CoC) compatible with the Ansible Code of Conduct (CoC)
• have a publicly available issue tracker that does not require a paid level of service to create an account or view issues
• have a public SCM repository, releases of the collection are tagged in this repository

Standards and documentation:

• adheres to semantic versioning
• follows licensing rules
• follows the Ansible documentation standards and the style guide
  MUST FIX: https://github.com/ansible-collections/trendmicro.deepsec/blob/main/plugins/modules/deepsec_firewallrules.py#L69 says that any is default but this is not enforced in argspec not properly documented. The same goes for https://github.com/ansible-collections/trendmicro.deepsec/blob/main/plugins/modules/deepsec_firewallrules.py#L110, https://github.com/ansible-collections/trendmicro.deepsec/blob/main/plugins/modules/deepsec_firewallrules.py#L157, https://github.com/ansible-collections/trendmicro.deepsec/blob/main/plugins/modules/deepsec_firewallrules.py#L199
  OPTIONAL: Module parameter descriptions are inconsistent. Some of them are lists of paragraphs while others are just a single paragraph.
  OPTIONAL: Documentation is inconsistent when it comes to addressing parameter names and values. Guidelines recommend using I(param_name) for parameter names and C(value) for parameter values. Modules in the collection currently mostly use 'value' for values and no special markup for parameter names.
• follows development conventions; as well as these other requirements:
  • modules that only gather information are named <something>_info
  • modules that return ansible_facts are named <something>_facts and do not return non-facts
  • other modules must not allow querying information using specific state option values, or similar mechanisms (like state=get or state=query). These features should be moved to <something>_info or <something>_fact modules.
  • check_mode is supported in all _info and _facts modules
• supports Python 2.6 or greater and Python 3.5 or greater. If it does not, read the full guidelines to see if you qualify for an exception and document the unsupported Python versions in the collection README.md and in every module and plugin (or in doc fragments)
  MUST FIX: As far as I can tell from https://dashboard.zuul.ansible.com/t/ansible/project/github.com/ansible-collections/trendmicro.deepsec, this collection is only tested on Python 2.7, 3.6, and 3.8, so there is no guarantee that it works on Python 2.6 and 3.5. This situation must be clarified and the actual support documented in modules.
• only uses the allowed plugin types in the plugins/ directory
• has README.md
• documentation, examples, and return sections use FQCNs for the M(..) format macros when referring to modules
• modules (or plugins) from ansible-core use ansible.builtin. as a FQCN prefix
• documentation and return sections use version_added: containing the collection version for which an option, module or plugin was added
  MUST FIX: https://github.com/ansible-collections/trendmicro.deepsec/blob/main/plugins/httpapi/deepsec.py#L16 contains an invalid version number.
• FQCNs are used for extends_documentation_fragment:, unless you are referring to doc_fragments from ansible-core

Collection management:

• galaxy.yml having tags field set
• collection dependencies must have a lower bound on the version which is at least 1.0.0, and are all part of the ansible package
  MUST FIX: The collection is missing dependency on ansible.netcommon.
• meta/runtime.yml defines the minimal version of Ansible which the collection works with
• has changelog, preferably with changelogs/changelog.yaml

Tests:

• passed ansible-test sanity
• if test/sanity/ignore*.txt exists, it does not contain error codes listed here
• has CI tests up and running against each of the "major versions" of ansible-base/ansible-core that the collection supports
• all CI tests run against every pull request
• all CI tests run against a commit that releases the collection; if they don't pass, the collection won't be released
• all CI tests run regularly (nightly, or at least once per week)
  MUST FIX: As far as I can tell from https://dashboard.zuul.ansible.com/t/ansible/builds?job_name=ansible-test-sanity-docker&project=ansible-collections%2Ftrendmicro.deepsec, tests are not scheduled to run regularly. They are only executed on changes. Can someone that knows how Zuul works pitch in here and verify my findings?

[tadeboro]

I finished my initial review, and the collection is not ready for inclusion in the current state.

Apart from the issues listed in the checklist, I also noticed that the galaxy.yml file does not contain a version number that would be even remotely in sync with what is published on Ansible Galaxy. I guess that this version number is replaced at the publication time, which works in most cases. Still, it does mean that users installing collection directly from the git repository will end up with a totally meaningless version number.

[tadeboro]

I did a second review of the collection and it is still not ready for inclusion. See the MUST FIX comments in the review for things that still need to be fixed.

[justjais]

@tadeboro Thanks very much for reviewing the collection, I've checked your comment, I'll fix it all.

But for the comment where you've suggested having _info/_facts module for facts collecting modules, it's the way new resource modules are designed and which is followed by Ansible supported network collections modules (e.g. ios, nxos, junos). I am currently working on updating the named modules to have all the newly supported states as present in resource modules like merged, replace, overridden, and deleted along with gathered state.

[tadeboro]

But for the comment where you've suggested having _info/_facts module for facts collecting modules, it's the way new resource modules are designed and which is followed by Ansible supported network collections modules (e.g. ios, nxos, junos).

This is something we should probably discuss in one of the community meetings. Right now, the https://github.com/ansible-collections/overview/blob/main/collection_requirements.rst#development-conventions document does not take the network development guidelines into consideration (mostly because most of the people that work on community stuff are not intimately familiar with the state of the art in the networking area). Do you have any links that point to the resource module documentation that would contain developer guidelines?

[tadeboro]

I found https://docs.ansible.com/ansible/latest/network/user_guide/network_resource_modules.html on the documentation site. Is this up to date?

[felixfontein]

👍 to bringing this up for discussion. The inclusion criteria took this rule from the official Ansible development documentation (https://docs.ansible.com/ansible/devel/dev_guide/developing_modules_best_practices.html#scoping-your-module-s); this rule has been part of that for many years.

[justjais]

@tadeboro yes, this is correct and updated

[tadeboro]

I created ansible-community/community-topics#33 so that we do not lose track of this.

[justjais]

@tadeboro as the upcoming release deadline is close (ref: release roadmap), can we please expedite the process to make TM deepsec as part of Ansible supported content list.

[tadeboro]

This is not really up to me. I did try to speed up the decision-making process in early August, but it turned out that the community is not too keen on advocating resources modules as one of the "right" ways of writing Ansible modules. We did again talk about this on September 1st, 2021 and we again came to no concrete conclusion.

One of the problems I see here is the fact that no one that advocates for resource modules came up with a proposal about WHERE resource modules should be allowed/encouraged. All we got is a definition of resource modules that we have nowhere to use right now.

I will add some proposals that surfaced during the meeting discussions to ansible-community/community-topics#33, ask for people's feedback, and schedule the discussion for the next meeting, but there is not much else I can do at this point in time.

[tadeboro]

Today, we discussed this once again in the community meeting and we accepted the resource module pattern as a valid template for developing content that will be distributed in the community package.

[IPvSean]

community is not too keen on advocating resources modules as one of the "right" ways of writing Ansible modules

FWIW I think this is a loaded comment. Who counts as community and who doesn't? Was there a poll/vote or was it just people in a particular IRC chatroom at one given time? What time zones were available? I also don't think community users are the only stakeholder in conversations like this. Does my opinion not count because I was not in a particular IRC chatroom at a given time?

Resource modules are a natural evolution of Ansible to allow both the best parts of imperative and declarative models. They are insanely helpful for network vendors, and help onboard people with more traditional IT backgrounds that might not understand complex templating.

[gundalow]

Hi,
I believe this is referring to The Steering Committee

Was there a poll/vote or was it just people in a particular IRC chatroom at one given time?

This is something we are very aware of, we have
ansible-community/community-topics#38 to track improving decision making and increase the asynchronus discussion.

In fact, this is a topic for Contributor's Summit on 1st October

All important topics are tracked in community-topics and this one is tracked as ansible-community/community-topics#33

We do suggest that people subscribe to community-topics and provide there feedback. We know we (Community Team) need to do a better job at socialising these topics, we will be having them posted in the new Matrix rooms.

[tadeboro]

There are less than two weeks left for collections to get ready for inclusion in Ansible 5 (see roadmap for more details). This candidate still has unresolved issues and is not ready for inclusion right now.