ansible / galaxy-dev

Ansible Automation Hub
11 stars 13 forks source link

Collections can be imported with a readme that is not in md format #168

Open thedoubl3j opened 4 years ago

thedoubl3j commented 4 years ago

Currently, a collection can be imported to galaxy with a readme file that is in any format.

However, for it render properly, it needs to be in a md file format.

A collection should fail on import if the readme is NOT a .md file.

Example: https://galaxy.ansible.com/sensu/sensu_go Source: https://github.com/sensu/sensu-go-ansible

bcoca commented 4 years ago

i would not restrict other readme files/formats, just require at LEAST one supported one.

thedoubl3j commented 4 years ago

I have no issue with that either, we just have support for .md at the moment and folks are wondering why they can't see their readme. nothing defining what is supported and not supported

chouseknecht commented 4 years ago

Currently, AH throws an exception when galaxy.yml readme key is blank, or when readme file does not exist. However, if it points to readme.txt or readme.rst and that file exists, it does NOT throw an exception.

Upon upload to AH, a Collection Version is NOT automatically certified. The Partner Engineering team reviews it and decides whether or not to mark it as certified. So the Partner Engineering team could decide on a policy to reject Collection Versions artifacts that do not contain a README.md file.

Before making any changes, we'll need requirements from Product Management and probably an update tot he Collection Format specification.

rhenshall commented 4 years ago

While AH only displays README.md I think we should reject it, however it should be possible to include other formats. The point of documentation is that it is displayed, allowing something that can't be displayed as the only option is inefficient. We should reject out of hand for the user experience to be optimal and avoid human error for false approvals.

chouseknecht commented 4 years ago

According to the collection format spec doc (link below), "readme is required and must be .md"

https://docs.google.com/document/d/1MyH6K7z6Zf5XyUBqVhcpXa2G9MyCC10NhpgL-G1KF5A/edit#heading=h.roeil4yx8rzo

Based on that, I would posit that the AH import process should throw an exception when it cannot parse the README, since it expects to find a .md format and that format comports with the spec.

rhenshall commented 4 years ago

That seals it, it's what specs are for.

chouseknecht commented 4 years ago

@awcrosby

Marking as a bug. I know you and @bmclaughlin are busy working on the sandboxing of ansible-test, so if this doesn't get fixed until 4.1, that's probably OK.

awcrosby commented 4 years ago

FYI after this change is made, and an updated galaxy-importer is released onto pypi, then pulp-ansible will need be updated. Since we are temporarily pinned to a pulp-ansible commit, it may be best to wait until 4.1 to deploy this.

chouseknecht commented 4 years ago

Once we resolve #190, then we should reject a collection when we fail to find and/or transform the README into HTML.

Blocked by #190