ansible / validated-content-discussion

A place for review of validated content candidates and general discussion around validated content.
GNU General Public License v3.0
2 stars 4 forks source link

candidate: F5 Event Driven Security #29

Open VDI-Tech-Guy opened 6 months ago

VDI-Tech-Guy commented 6 months ago

Candidate Name

Event Driven Security with F5

Link to Github Repo

https://github.com/f5devcentral/f5-bd-ansible-eda

Review Due By

April 17, 2024

CHECKLIST

Business Checks:

Content Viability

Content Duplication

Technical Checks:

Content Conformity

Content Compliance

Content Testing/Validation

Emily Bock recommended me to following this path please reach out to her for questions

alisonlhart commented 6 months ago

Hello @VDI-Tech-Guy! I've started the preliminary review for this collection. This will be presented to Red Hat's Automation Community of Practice as a validated content candidate. That group will have 2 weeks from presentation (initially presented on April 3, 2024), to review the collection and add feedback. This will have a final sign-off date of April 17, 2024.

There are a number of changes that should be made before this review starts. A few are detailed below, from the checklist: - isn’t just a demo One of our requirements is that a validated content collection is not just a demo. The language in the README indicates it's a demo and not meant for production usage. If the language can be modified to indicate that these are templates meant for customer use and not just for demonstration, that will meet this requirement.

- is packaged as a content collection - see the Collection Requirements Checklist The content submitted is missing collection structural elements like a galaxy.yml file and meta/runtime.yml file. It will also need a changelog file.

- supported plugin/module used in a validated role is noted as a dependency There are a number of collections used in the playbooks that aren't noted as dependencies. These will need to be listed the galaxy.yml file's "dependencies" list.

There are a number of other missing requirements detailed in the checklist. Please go through it and ask any questions as needed.

Here is an example validated content collection for helpful reference: https://github.com/redhat-cop/network.backup

alisonlhart commented 5 months ago

@VDI-Tech-Guy Hello! Have you had a chance to look at the review feedback above?

VDI-Tech-Guy commented 3 months ago

Hello @alisonlhart - I have done some updates to this to reflect the asks, however due to this being Event Driven Ansible related code mixed with Playbooks im uncertain of a example that be provided to make this a collection, and am uncertain if that is even possible in this usecase as EDA as far as i know is not made into collections (rulebooks) etc. Could you look at the provided changes and chat with Emily Bock about this as im uncertain what else needs to be modified for EDA + AAP Validations.

alisonlhart commented 3 months ago

@VDI-Tech-Guy Here's some documentation on EDA generally, which links out to more detail of EDA collection structure requirements. The overall structure is the same as a collection without EDA content, but adds EDA content under the extensions/eda/ dir.

Here's an example of a collection that contains EDA content for reference: https://github.com/ansible/event-driven-ansible

Rulebooks should be contained under <collection_root>/extensions/eda/rulebooks/, while playbooks can remain under the standard structure of <collection_root>/playbooks/.

Let me know if you have any other questions or need additional examples!

VDI-Tech-Guy commented 1 month ago

@alisonlhart I have updated the repo with fixes and the help of Chyna to resolve some of the problems in the code, she said it now builds correctly and i have gone through and updated Lint checks, there will be 1 error that cannot be resolved

yaml[line-length]: Line too long (396 > 160 characters). in playbooks/bigip-classic/block-ips.yaml. this is due to the code thats being injected is XML Formatted and has to be a specific way for this to work correctly.

chynasan commented 4 days ago

Hello! I mentioned this in our chat, but there are a couple small fixes I wanted to mention that we'd like to see based on a quick review of the Candidate checklist from the original issue posting:

First, the README is missing information about how to install/use this content, as well as the required Python version. The README should also include a link to a copy of the license to make it easier for users to access once the collection is published to AutomationHub. You can see an example of a README from one of our internally-developed collections here if you'd like.

Additionally, there should be a CODEOWNERS file in the repo root outlining at least two maintainers by GitHub username. Since these are mostly documentation-related, I expect they should be pretty easy to fix.