Skip to content

Conversation

@peterschretlen
Copy link
Contributor

Summary

Make the user and password secrets in the webhook action optional.
Resolves #55359

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@peterschretlen peterschretlen added v7.7.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes labels Feb 5, 2020

after(() => esArchiver.unload('empty_kibana'));

it('webhook can be executed without username and password', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test is in the spaces_only functional test config because it does not use security, so the webhook simulator can be used without auth.

expect(result.status).to.eql('error');
expect(result.message).to.match(/error calling webhook, invalid response/);
expect(result.serviceMessage).to.eql('[400] Bad Request');
expect(result.message).to.match(/error calling webhook, retry later/);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The expected result here was incorrect due to a typo in the webhook simulator when the body is failure

@mikecote mikecote added Feature:Alerting Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// labels Feb 5, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

Copy link
Member

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

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

LGTM, but I think we need to change the config.schema definition to use schema.nullable(), made a few other comments.

@peterschretlen
Copy link
Contributor Author

@elasticmachine merge upstream

@peterschretlen
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine elasticmachine requested a review from a team as a code owner February 7, 2020 16:37
Copy link
Member

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

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

LGTM

@peterschretlen
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@YulNaumenko YulNaumenko left a comment

Choose a reason for hiding this comment

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

LGTM!

@peterschretlen
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@peterschretlen peterschretlen merged commit bb7e152 into elastic:master Feb 10, 2020
peterschretlen pushed a commit to peterschretlen/kibana that referenced this pull request Feb 10, 2020
gmmorris added a commit to gmmorris/kibana that referenced this pull request Feb 11, 2020
* master: (34 commits)
  [Index management] Server-side NP ready (elastic#56829)
  Webhook action - make user and password secrets optional (elastic#56823)
  [DOCS] Removes reference to IRC (elastic#57245)
  [Monitoring] NP migration: Local angular module (elastic#51823)
  [SIEM] Adds ECS link to help menu (elastic#57104)
  Ensure http interceptors are shares across lifecycle methods (elastic#57150)
  [Remote clusters] Migrate server code out of legacy (elastic#56781)
  fixes render bug in alert list (elastic#57152)
  siem 7.6 updates (elastic#57169)
  Make the update alert API key API work when AAD is out of sync (elastic#56640)
  fix(NA): MaxListenersExceededWarning on getLoggerStream (elastic#57133)
  [Metrics UI] Setup commonly used time ranges in timepicker (elastic#56701)
  [Maps] set filter.meta.key to geoFieldName so query passes filterMatchesIndex when ignoreFilterIfFieldNotInIndex is true (elastic#56692)
  Create plugin mock for event log plugin (elastic#57048)
  fix ts error on master (elastic#57236)
  Don't create API key for disabled alerts when calling create API (elastic#57041)
  Fix enable and disable API to still work when AAD is out of sync (elastic#56634)
  [DOCS] Canvas embed objects (elastic#57156)
  Delete autocomplete namespace (elastic#57187)
  Security - Inject logout url (elastic#57201)
  ...
@mikecote mikecote added release_note:fix and removed release_note:skip Skip the PR/issue when compiling release notes labels Apr 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Alerting release_note:fix Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// v7.7.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Webhook action requires basic authentication credentials

6 participants