-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
base: devel
Are you sure you want to change the base?
Conversation
@@ -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 != '': |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
433ed05
to
436204b
Compare
|
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
COMPONENT NAME