-
Notifications
You must be signed in to change notification settings - Fork 654
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.
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
Fix role name handling in prerun.py #1490
Conversation
According to https://galaxy.ansible.com/docs/contributing/creating_role.html#role-metadata: "role_name is not used at all if the role is installed using its Git URL. Instead, the name of the repo is used." So, make _install_galaxy_role() respect this to compute the role name. To avoid importing ansible galaxy files, the SCM list is hardcoded to git or hg. Signed-off-by: Arnaud Patard <apatard@hupstream.com>
_install_galaxy_role() is taking great pain to get and check the fqrn, even handling/using the shortname when the 'role-name' check is in skip_list but the code is not using the fqrn value to create the symlink in .cache/roles. Correct this by using the computed fqrn name for the link name. Signed-off-by: Arnaud Patard <apatard@hupstream.com>
Change the ansible-role-mysql git clone path to match the role fqrn as it's what's used with galaxy and what's currently done by the role ci. (ref: https://github.com/geerlingguy/ansible-role-mysql/blob/86ffba892e1553b7e6f042ec007eef25daa8f5db/.github/workflows/ci.yml#L21) Signed-off-by: Arnaud Patard <apatard@hupstream.com>
@geerlingguy Please review this change because it breaks one of your roles. That is because you added, for good reasons, As you can see, with this change we get an error on molecule playbooks as they are unable to find the full qualified role name. Obviously that removing the role-name from the skip list would allow the linter to pass. Still, I do really want your opinion on this matter as I want to minimize the chance of alienating more users. If you are not happy about it, do you have any proposed behaviors in mind? |
If the author field in meta/main.yml looks like "John Doe" (at least 2 words separated by a space), act as if there was no namespace. It's not perfect but it's not possible to match author<->galaxy account and there will still be a warning/error issued about it, since the fqrn won't match namespace.role_name. Signed-off-by: Arnaud Patard <apatard@hupstream.com>
According to : https://galaxy.ansible.com/docs/contributing/creating_role.html#role-metadata: In the past, Galaxy would apply a regex expression to the GitHub repository name and automatically remove 'ansible-' and 'ansible-role-'. For example, if your repository name was 'ansible-role-apache', the role name would translate to 'apache', So update the regexp to handle ansible-role- and ansible- Signed-off-by: Arnaud Patard <apatard@hupstream.com>
If the role-name rule is in skip_list: - if role_name is set in meta/main.yml, assume that the author wants to use this name, even if it's still not up to standards and set the link name to the computed fqrn, - if the role_name is not set, just use the current directory name. Note: don't use 'role_name' variable here since some part of the name has been stripped from the current directory name. Signed-off-by: Arnaud Patard <apatard@hupstream.com>
Flake8 is complaining that the function is now too complex, so split it into several ones. Signed-off-by: Arnaud Patard <apatard@hupstream.com>
Instead of using None for value not found and sometime return an empty string make both functions always return a string and make it empty is something went wrong. Signed-off-by: Arnaud Patard <apatard@hupstream.com>
@ssbarnea - TBH, for now I have disabled ansible-lint on all my roles because it was just filling up my inbox every day with another 20+ 'CI failure' notifications and there was an awful lot of work I had to do to each role just to get it to pass lint again (trying to configure ansible-lint so it would skip cache directory—I think that's since been fixed), adding in a list of dependencies in yet another location (the 'mock role' list or something), etc. In the past, I could just install ansible-lint and it would work, and lint my role. Today it is a lot harder since 5.x came out. I am still using ansible-lint on my playbook repositories, where it seems like very little was changed, but for my Galaxy roles I'm not sure when I'll get the time to work on re-adding configuration for all the roles to run with the linter again. |
The cache thing should be fixed iirc. For the dependency location thing, it's a different issue and if I find time I'll probably look at that too as I got bitten by it too, but in a different PR. No idea what @ssbarnea will think of that.
With this PR, I hope that for roles without dependencies, things should just work again, well except for roles with wrong fqrn and in this case, either fixing or |
As it may lead to breakages, drop the check on .git or .hg presence to get role name. Signed-off-by: Arnaud Patard <apatard@hupstream.com>
This PR tries to address issues with the link made in
.cache/roles
for current role name.The first patch tries to make
_install_galaxy_role()
behave in same way asansible-galaxy
andthe second one is fixing the link name to use the fqrn computed earlier in the function. This will
provide a way to allow using the role short name by putting the 'role-name' check in
skip_list
.