Skip to content

Ensure ansible-config properly validates all entries - #84921

Open
pavan-msys wants to merge 2 commits into
ansible:develfrom
pavan-msys:issue-84843
Open

Ensure ansible-config properly validates all entries#84921
pavan-msys wants to merge 2 commits into
ansible:develfrom
pavan-msys:issue-84843

Conversation

@pavan-msys

@pavan-msys pavan-msys commented Apr 3, 2025

Copy link
Copy Markdown
SUMMARY

This PR improves the validation mechanism in ansible-config to correctly handle dynamic 'galaxy server' entries. Previously, configurations missing mandatory fields, such as the 'url' in galaxy server definitions, were not flagged as errors. This fix ensures that such missing fields are properly detected and reported during validation.

Fixes #84843

ISSUE TYPE
  • Bugfix Pull Request

@ansibot ansibot added bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. has_issue labels Apr 3, 2025
Comment thread lib/ansible/cli/config.py Outdated
display.error(f"Found unknown key '{k}' in section '{s}' in '{C.CONFIG_FILE}.")
found = True

# Validate only the required key for galaxy servers

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please create a changelog fragment. See this fragment as an example.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also, write an integration test for this change. Thanks.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is the url required? iirc the server entry itself could be a url/ip

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

also, not the correct approach, the config definitions tell you which field is required or not, hardcoding the fields is not what we want to do.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the other part is checking type, if the fields are defined

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@bcoca

Got it! I'm currently updating the validation logic based on your feedback. I'll push the changes shortly.

Thanks

@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Apr 3, 2025
@s-hertel s-hertel removed the needs_triage Needs a first human triage before being processed. label Apr 3, 2025
@ansibot ansibot added the stale_review Updates were made after the last review and the last review is more than 7 days old. label Apr 9, 2025
@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Apr 10, 2025
@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 Apr 24, 2025
@pavan-msys

Copy link
Copy Markdown
Author

Hi,

Just checking in to see if there's any update on this PR. Happy to make any changes if needed!

@ansibot ansibot added the stale_pr This PR has not been pushed to for more than one year. label Apr 14, 2026
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 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. stale_review Updates were made after the last review and the last review is more than 7 days old.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ansible-config does not correclty validate all entries

5 participants