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

Fix alerts unable to create / update when the name has trailing whitepace(s) #76079

Merged
merged 2 commits into from
Aug 27, 2020
Merged

Fix alerts unable to create / update when the name has trailing whitepace(s) #76079

merged 2 commits into from
Aug 27, 2020

Conversation

mikecote
Copy link
Contributor

@mikecote mikecote commented Aug 27, 2020 •
edited
Loading

Resolves #76035

In this PR, I'm fixing an issue where alerts fail to be created or updated when you have trailing whitepsace(s) due to Elasticsearch's grant API key API not accepting leading and trailing whitepsace(s) in the API key's name.

@mikecote mikecote added release_note:fix Feature:Alerting v8.0.0 Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.10.0 v7.9.1 labels Aug 27, 2020
@mikecote mikecote self-assigned this Aug 27, 2020
FrankHassanabad
FrankHassanabad approved these changes Aug 27, 2020
View reviewed changes
Copy link
Contributor

@FrankHassanabad FrankHassanabad left a comment

Choose a reason for hiding this comment

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

Although still draft, this will work for us on security solutions! Thanks for the fast fix

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

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

@mikecote mikecote marked this pull request as ready for review August 27, 2020 17:23
@mikecote mikecote requested a review from a team as a code owner August 27, 2020 17:23
@elasticmachine
Copy link
Contributor

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

pmuellr
pmuellr approved these changes Aug 27, 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.

LGTM

I don't know exactly how these api key names get used, but curious if there's some chance we'll have a migration and/or AAD issue. I'm guessing the alerting framework isn't sensitive to the "name" change, in fact I don't even think we could set the name until recently, we generally just rely on the key id. And it's only used to make the api key descriptive in the api key UI. So , I'd guess we don't have any migration concerns ...

@mikecote
Copy link
Contributor Author

@pmuellr

I don't know exactly how these api key names get used, but curious if there's some chance we'll have a migration and/or AAD issue.

We should be ok on this part as the alert SO remains the same. The name in the API key in Elasticsearch will now be trimmed when creating new keys. This would mean alerts can still have whitespace leading or trailing in the name (not breaking AAD / needing a migration).

I'm guessing the alerting framework isn't sensitive to the "name" change, in fact I don't even think we could set the name until recently, we generally just rely on the key id. And it's only used to make the api key descriptive in the api key UI. So , I'd guess we don't have any migration concerns ...

The issue got introduced when the grant API key API added support for name: #71623 just recently. Previously when using the grant API, we never passed a name (only the create API key API a while ago).

@mikecote mikecote merged commit 50193ea into elastic:master Aug 27, 2020
@mikecote mikecote mentioned this pull request Aug 27, 2020
Merged
mikecote added a commit to mikecote/kibana that referenced this pull request Aug 27, 2020
@mikecote
Fix alerts unable to create / update when the name has trailing white…
6a68721 
…pace(s) (elastic#76079)

* Trim alert name in API key name

* Add API integration tests
@mikecote mikecote mentioned this pull request Aug 27, 2020
Merged
mikecote added a commit that referenced this pull request Aug 27, 2020
@mikecote
Fix alerts unable to create / update when the name has trailing white…
57d8509 
…pace(s) (#76079) (#76170)

* Trim alert name in API key name

* Add API integration tests
jloleysens added a commit to jloleysens/kibana that referenced this pull request Aug 31, 2020
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into ilm/data-tier…
174a82d 
…s-for-710

* 'master' of github.com:elastic/kibana: (43 commits)
  [APM] Chart units don't update when toggling the chart legends (elastic#74931)
  [ILM] Add support for frozen phase in UI (elastic#75968)
  Hides advanced json for count metric (elastic#74636)
  add client-side feature usage API (elastic#75486)
  [Maps] add drilldown support map embeddable (elastic#75598)
  [Enterprise Search] Request handler refactors/enhancements + update existing routes to use shared handler (elastic#76106)
  [Resolver] model `location.search` in redux (elastic#76140)
  [APM] Prevent imports of public in server code (elastic#75979)
  fix eslint issue
  skip flaky suite (elastic#76223)
  [APM] Transaction duration anomaly alerting integration (elastic#75719)
  [Transforms] Avoid using "Are you sure" (elastic#75932)
  [Security Solution][Exceptions] - Fix bug of alerts not updating after closure from exceptions modal (elastic#76145)
  [plugin-helpers] improve 3rd party KP plugin support (elastic#75019)
  [docs/getting-started] link to yarn v1 specifically (elastic#76169)
  [Security_Solution][Resolver] Resolver loading and error state (elastic#75600)
  Fixes App Search documentation links (elastic#76133)
  Fix alerts unable to create / update when the name has trailing whitepace(s) (elastic#76079)
  [Resolver] Fix useSelector usage (elastic#76129)
  [Enterprise Search] Migrate util and components from ent-search (elastic#76051)
  ...

# Conflicts:
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/data_tier_allocation/node_allocation.tsx
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/edit_policy.tsx
#	x-pack/plugins/index_lifecycle_management/public/application/services/policies/types.ts
#	x-pack/plugins/index_lifecycle_management/public/application/services/policies/warm_phase.ts
mikecote added a commit that referenced this pull request Aug 31, 2020
@mikecote @elasticmachine
[7.9] Fix alerts unable to create / update when the name has trailing…
1898b38 
… whitepace(s) (#76079) (#76172)

* Merge with master

* Fix some references

* Fix test failures

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@mikecote mikecote added v7.9.2 and removed v7.9.1 labels Aug 31, 2020
@tatdatpham
Copy link

When this commit apply to Kibana PROD?
I got same issue, so now i cant use alerts component. Have another way to quick fix ? Thank so much!

@mikecote
Copy link
Contributor Author

mikecote commented Sep 1, 2020

@tatdatpham you should be able to work around this by removing any trailing spaces in the alert's name. The fix will be part of the 7.9.1 or 7.9.2 patch release.

@mikecote mikecote added the v7.9.2 label Sep 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pmuellr pmuellr pmuellr approved these changes

@FrankHassanabad FrankHassanabad FrankHassanabad approved these changes

Assignees

@mikecote mikecote

Labels
Feature:Alerting release_note:fix Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.9.2 v7.10.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Alerts and security solutions rules with spaces have validation issues
6 participants
@mikecote @kibanamachine @elasticmachine @tatdatpham @pmuellr @FrankHassanabad