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.

Sign up for GitHub

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

Jump to bottom

Webhook Custom HTTP Method + Basic Auth Support #4124

Merged
merged 17 commits into from
Jul 24, 2019
Merged

Webhook Custom HTTP Method + Basic Auth Support #4124

merged 17 commits into from
Jul 24, 2019

Conversation

beeankha
Copy link
Contributor

@beeankha beeankha commented Jun 20, 2019 •
edited
Loading

SUMMARY

This PR is for the webhook-specific portion of Issue #79

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME
  • API
  • UI
AWX VERSION
awx 5.0.0
ADDITIONAL INFORMATION

Implementation details discussed here: #79 (comment)

@beeankha beeankha self-assigned this Jun 20, 2019
@softwarefactory-project-zuul

This comment has been minimized.

Sign in to view
@softwarefactory-project-zuul

This comment has been minimized.

Sign in to view
@beeankha beeankha force-pushed the webhook_enhancement branch from f52c935 to e33881c Compare June 24, 2019 15:15
@softwarefactory-project-zuul

This comment has been minimized.

Sign in to view
@softwarefactory-project-zuul

This comment has been minimized.

Sign in to view
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@beeankha beeankha changed the title [WIP] Webhook Custom HTTP Method + Basic Auth Support Webhook Custom HTTP Method + Basic Auth Support Jun 24, 2019
@jakemcdermott jakemcdermott force-pushed the webhook_enhancement branch 2 times, most recently from 5fad177 to d21e78d Compare June 24, 2019 20:18
@softwarefactory-project-zuul

This comment has been minimized.

Sign in to view
@jakemcdermott
Copy link
Contributor

recheck

@softwarefactory-project-zuul

This comment has been minimized.

Sign in to view
@jakemcdermott
Copy link
Contributor

recheck

@softwarefactory-project-zuul

This comment has been minimized.

Sign in to view
@jakemcdermott
Copy link
Contributor

recheck

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@beeankha
Copy link
Contributor Author

I tested an upgrade situation by creating webhook notifications in Tower v3.5.1 then upgrading to 3.6 with the changes contained in this PR.

On the backend, the upgraded webhook notifications populate with the expected blank default values for username/password (which, even though they are blank/None, are still base64 encrypted) and the http_method of POST and they work as expected.

@ryanpetrello
Copy link
Contributor

ryanpetrello commented Jun 25, 2019 •
edited
Loading

On the backend, the upgraded webhook notifications populate with the expected blank default values for username/password (which, even though they are blank/None, are still base64 encrypted) and the http_method of POST and they work as expected.

Want to confirm to make sure I understand - if username and/or password are '' or 'None', we should probably not send an Authorization: Basic ... header at all.

see: https://github.com/ansible/awx/pull/4124/files#r297259836

In other words, if you install 3.5.1 and upgrade to 3.6.0, I'd expect that any notification templates you made prior to upgrade would not send anyAuthorization: Basic ... header post-upgrade (at least, not until you went and set a username and password)

@beeankha beeankha force-pushed the webhook_enhancement branch from 34fa526 to 7580491 Compare July 23, 2019 18:52
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@jakemcdermott jakemcdermott force-pushed the webhook_enhancement branch 4 times, most recently from 87ef672 to bf5c0ff Compare July 23, 2019 23:43
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

Spredzy
Spredzy approved these changes Jul 24, 2019
View reviewed changes
Copy link
Contributor

@Spredzy Spredzy left a comment

Choose a reason for hiding this comment

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

Thank you !

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 128fa89 into ansible:devel Jul 24, 2019
@beeankha beeankha deleted the webhook_enhancement branch July 24, 2019 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryanpetrello ryanpetrello ryanpetrello approved these changes

@jakemcdermott jakemcdermott jakemcdermott left review comments

@elyezer elyezer elyezer approved these changes

@Spredzy Spredzy Spredzy approved these changes

@jlmitch5 jlmitch5 jlmitch5 approved these changes

Assignees

@Spredzy Spredzy

@beeankha beeankha

Labels
component:api component:ui type:enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@beeankha @jakemcdermott @ryanpetrello @rooftopcellist @elyezer @Spredzy @jlmitch5