Skip to content

Fix validating role params of dependencies that are used by ansible-galaxy #82182

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

Draft
wants to merge 3 commits into
base: devel
Choose a base branch
from

Conversation

s-hertel
Copy link
Contributor

@s-hertel s-hertel commented Nov 9, 2023

SUMMARY

Fixes #82154

Validate normally if these are documented by the role, but don't require documentation since the role may be unaware of these fields used by ansible-galaxy.

ISSUE TYPE
  • Bugfix Pull Request
ADDITIONAL INFORMATION

…alaxy

Validate normally if these are documented by the role, but don't require
documentation since the role may be unaware of these fields used by
ansible-galaxy.
@ansibot ansibot added bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. has_issue labels Nov 9, 2023
@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Nov 9, 2023
@ansibot
Copy link
Contributor

ansibot commented Nov 9, 2023

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/galaxy/role.py:85:12: pointless-exception-statement: Exception statement has no effect

click here for bot help

@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Nov 9, 2023
# Since role runtime (with argument spec) requires all role params be documented,
# add a supplemental spec so this isn't a failure by default for ansible-galaxy options.
# If the role actually documents these options, they'll be validated normally.
internal_galaxy_spec = {k: {} for k in VALID_SPEC_KEYS}
Copy link
Member

@bcoca bcoca Nov 10, 2023

Choose a reason for hiding this comment

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

I would add warning/debug with the skipped keys, for those that are 'overloading' role params and expecting a variable to pass through.

Copy link
Member

Choose a reason for hiding this comment

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

i just remembered, we did add that warning at one point .. .i'm just not sure where/at what point

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel like we'd need to deprecate this dependencies syntax for ansible-galaxy if we want to issue a warning at runtime by default. I could add a debug message in addition to or instead of the vvv message in validate_argument_spec.

@ansibot ansibot removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. ci_verified Changes made in this PR are causing tests to fail. labels Nov 10, 2023
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Nov 22, 2023
@VorobevPavel-dev
Copy link

VorobevPavel-dev commented Dec 8, 2023

@s-hertel @bcoca checked changes with my error.

(valid args) On branch from PR there is no log for argument_spec task (seems like it runs check but with name from previous task): https://pastebin.com/pHw9zMK5

(invalid args) If there is any error, it relates to previous role, not current's argument_spec task: https://pastebin.com/9wTESW6Y

@ansibot ansibot added the stale_pr This PR has not been pushed to for more than one year. label Jan 28, 2025
@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Apr 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug. has_issue needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. stale_pr This PR has not been pushed to for more than one year.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unexpected role argument spec behavior with transitive dependencies
4 participants