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

Rename Whitelist to AllowList in Actions and Alerting #75099

Merged
merged 9 commits into from
Aug 24, 2020
Merged

Rename Whitelist to AllowList in Actions and Alerting #75099

merged 9 commits into from
Aug 24, 2020

Conversation

YulNaumenko
Copy link
Contributor

@YulNaumenko YulNaumenko commented Aug 14, 2020 •
edited
Loading

Renamed whitelistedHosts to allowedHosts
Resolve #71630

@YulNaumenko YulNaumenko added Feature:Alerting v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.10.0 labels Aug 14, 2020
@YulNaumenko YulNaumenko requested review from a team as code owners August 14, 2020 22:28
@YulNaumenko YulNaumenko self-assigned this Aug 14, 2020
@elasticmachine
Copy link
Contributor

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

@YulNaumenko YulNaumenko removed the request for review from a team August 14, 2020 22:32
pmuellr
pmuellr reviewed Aug 17, 2020
View reviewed changes
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.

Going to leave as a comment for now, because it feels like we could improve the names we're using for the config property / utility functions. I can be convinced that these names or fine though - convince me!

x-pack/plugins/actions/README.md Outdated Show resolved Hide resolved
x-pack/plugins/actions/README.md Outdated Show resolved Hide resolved
@YulNaumenko
Merge remote-tracking branch 'upstream/master' into alerting-whitelis…
cc23861 
…t-renaming

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
@YulNaumenko
Fixed due to comments and tests failing
bd25261
@YulNaumenko
Merge remote-tracking branch 'upstream/master' into alerting-whitelis…
ebe2391 
…t-renaming

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
@YulNaumenko
Fixed failing tests
601d174
@YulNaumenko
Merge remote-tracking branch 'upstream/master' into alerting-whitelis…
628186c 
…t-renaming

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
@mikecote mikecote self-requested a review August 21, 2020 17:34
mikecote
mikecote approved these changes Aug 21, 2020
View reviewed changes
Copy link
Contributor

@mikecote mikecote left a comment

Choose a reason for hiding this comment

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

Changes LGTM! Few questions / fixes.

x-pack/plugins/actions/server/actions_config.ts Outdated Show resolved Hide resolved
x-pack/plugins/actions/server/actions_config.ts Outdated Show resolved Hide resolved
x-pack/plugins/actions/server/actions_config.ts Outdated Show resolved Hide resolved
x-pack/plugins/actions/README.md Outdated Show resolved Hide resolved
x-pack/plugins/actions/README.md Outdated Show resolved Hide resolved
x-pack/plugins/actions/README.md Outdated Show resolved Hide resolved
mikecote
mikecote reviewed Aug 21, 2020
View reviewed changes
Copy link
Contributor

@mikecote mikecote left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I think we'll need to also create an issue on cloud to rename this field in their whitelist?

I'm also wondering if we should deprecate the old configuration until 8.0? (a lot of people's Kibana's wont start otherwise)

@YulNaumenko
Merge remote-tracking branch 'upstream/master' into alerting-whitelis…
219fdba 
…t-renaming

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
@YulNaumenko
Fixed due to comments
44f0817
@YulNaumenko YulNaumenko mentioned this pull request Aug 24, 2020
Closed
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

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

@YulNaumenko YulNaumenko merged commit f28a9e6 into elastic:master Aug 24, 2020
@YulNaumenko YulNaumenko mentioned this pull request Aug 25, 2020
Closed
YulNaumenko added a commit to YulNaumenko/kibana that referenced this pull request Aug 25, 2020
@YulNaumenko
Rename Whitelist to AllowList in Actions and Alerting (elastic#75099)
146e3e4 
* Rename Whitelist to AllowList in Actions and Alerting

* revert not related change

* Fixed due to comments and tests failing

* Fixed failing tests

* Fixed due to comments
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Aug 26, 2020
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 75099 or prevent reminders by adding the backport:skip label.

2 similar comments
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 75099 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 75099 or prevent reminders by adding the backport:skip label.

@YulNaumenko YulNaumenko mentioned this pull request Aug 31, 2020
Merged
YulNaumenko added a commit to YulNaumenko/kibana that referenced this pull request Aug 31, 2020
@YulNaumenko
Rename Whitelist to AllowList in Actions and Alerting (elastic#75099)
9559101 
* Rename Whitelist to AllowList in Actions and Alerting

* revert not related change

* Fixed due to comments and tests failing

* Fixed failing tests

* Fixed due to comments
YulNaumenko added a commit that referenced this pull request Aug 31, 2020
@YulNaumenko
Rename Whitelist to AllowList in Actions and Alerting (#75099) (#76333)
a08593b 
* Rename Whitelist to AllowList in Actions and Alerting

* revert not related change

* Fixed due to comments and tests failing

* Fixed failing tests

* Fixed due to comments
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Aug 31, 2020
parkiino pushed a commit to parkiino/kibana that referenced this pull request Sep 1, 2020
@YulNaumenko @parkiino
Rename Whitelist to AllowList in Actions and Alerting (elastic#75099)
1643f99 
* Rename Whitelist to AllowList in Actions and Alerting

* revert not related change

* Fixed due to comments and tests failing

* Fixed failing tests

* Fixed due to comments
@tylersmalley
Copy link
Contributor

tylersmalley commented Sep 8, 2020 •
edited
Loading

@elastic/kibana-alerting-services this appears to be a breaking change which was backported to 7.x without any deprecation period as was suggested by the comment here. Is this correct? If so, we absolutely need to address this.

@mikecote
Copy link
Contributor

mikecote commented Sep 8, 2020

Thanks for raising this @tylersmalley. A follow up PR was created and backported under: #76325 to deprecate the setting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mikecote mikecote mikecote approved these changes

@spong spong spong approved these changes

@pmuellr pmuellr Awaiting requested review from pmuellr

Assignees

@YulNaumenko YulNaumenko

Labels
Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use neutral naming for actions URL whitelist
7 participants
@YulNaumenko @elasticmachine @kibanamachine @tylersmalley @mikecote @pmuellr @spong