Skip to content
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

Webhook_credential field validation updates #14843 #14844

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

Conversation

kk-at-redhat
Copy link

@kk-at-redhat kk-at-redhat commented Feb 3, 2024

SUMMARY

Allow webhook_credential in ansible.controller.job_template to accept empty value and remove s=any existing selections in that field.

related #14843

ISSUE TYPE
  • Bug, Docs Fix or other nominal change
COMPONENT NAME
  • Collection

@github-actions github-actions bot added component:awx_collection issues related to the collection for controlling AWX community labels Feb 3, 2024
@@ -575,8 +575,11 @@ def main():
new_fields['project'] = project_data['id']
else:
new_fields['project'] = module.resolve_name_to_id('projects', project)
if webhook_credential is not None:

if webhook_credential is not None and webhook_credential != '':
Copy link
Member

Choose a reason for hiding this comment

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

What happens if the user leaves off webhook credential? This seems like this would set it to None.

We have no set pattern for this but you have sparked @jbradberry interest.

Copy link
Contributor

Choose a reason for hiding this comment

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

@kk-at-redhat I believe correct logic needs to distinguish the case "user didn't include this key-value" and "user explicitly set it to something empty/false", which only using module.params.get(...) with the default None fallback doesn't get us. I'm thinking something like:

if 'webhook_credential' in module.params:
    if webhook_credential:
        new_fields['webhook_credential'] = module.resolve_name_to_id('credentials', webhook_credential)
    else:
        new_fields['webhook_credential'] = None

Copy link
Contributor

Choose a reason for hiding this comment

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

Similar logic could be used for #14841

@dmzoneill dmzoneill changed the title related #14843 Webhook_credential field validation updates #14843 Feb 13, 2024
@dmzoneill dmzoneill marked this pull request as draft February 14, 2024 14:52
@jbradberry jbradberry marked this pull request as ready for review May 31, 2024 17:22
@jbradberry jbradberry force-pushed the webhook_credential_fix branch from 433ed05 to 436204b Compare May 31, 2024 17:23
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community component:awx_collection issues related to the collection for controlling AWX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants