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.

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

[Ingest Manager] Don't error if editing the same agent_policy #80403

Merged
merged 2 commits into from
Oct 14, 2020

Conversation

jfsiii
Copy link
Contributor

@jfsiii jfsiii commented Oct 13, 2020

Summary

fixes #80136

Haven't added/updated tests yet but tested the various combinations in the UI and all worked as expected.

Screenshots

Policy listing

Screen Shot 2020-10-14 at 7 05 19 AM

Update existing policy succeeds

Screen Shot 2020-10-14 at 7 05 49 AM

Creating new policy w/existing name fails

Screen Shot 2020-10-14 at 7 06 06 AM

Updating policy to existing name fails

Screen Shot 2020-10-14 at 7 08 05 AM
### Checklist

@jfsiii jfsiii requested a review from a team October 13, 2020 19:34
@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Oct 13, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Team:Ingest Management)

@jfsiii
Copy link
Contributor Author

jfsiii commented Oct 13, 2020

@jen-huang (maybe @EricDavisX ) would you mind checking this out locally and using the UI to check?

Assuming that works for you and the CI passes, I'd like to get merge this ASAP so we can possibly get it in before the BC is cut tomorrow. I can follow up with tests, but since it's critical I'd really like to get something in the BC. Thanks.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

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

Copy link
Contributor

@skh skh left a comment

Choose a reason for hiding this comment

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

I can reproduce the error, and verified that this change fixes it. 👍

@jfsiii jfsiii merged commit 2955584 into elastic:master Oct 14, 2020
jfsiii pushed a commit to jfsiii/kibana that referenced this pull request Oct 14, 2020
## Summary

fixes elastic#80136

Haven't added/updated tests yet but tested the various combinations in the UI and all worked as expected.

<details><summary>Screenshots</summary>
<h3>Policy listing</h3>
<img width="1302" alt="Screen Shot 2020-10-14 at 7 05 19 AM" src="https://user-images.githubusercontent.com/57655/95981056-1c1c6d00-0dec-11eb-9746-5e91043f3d19.png">

<h3>Update existing policy succeeds</h3>
<img width="1308" alt="Screen Shot 2020-10-14 at 7 05 49 AM" src="https://user-images.githubusercontent.com/57655/95981059-1cb50380-0dec-11eb-9e55-7a14b72085f9.png">
<h3>Creating new policy w/existing name fails</h3>
<img width="1308" alt="Screen Shot 2020-10-14 at 7 06 06 AM" src="https://user-images.githubusercontent.com/57655/95981062-1cb50380-0dec-11eb-833b-1ab6636a5c67.png">

<h3>Updating policy to existing name fails</h3>
<img width="1308" alt="Screen Shot 2020-10-14 at 7 08 05 AM" src="https://user-images.githubusercontent.com/57655/95981078-1f175d80-0dec-11eb-91d5-7ffd976da48e.png">

</details>
### Checklist

- [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
jfsiii pushed a commit that referenced this pull request Oct 14, 2020
…0502)

## Summary

fixes #80136

Haven't added/updated tests yet but tested the various combinations in the UI and all worked as expected.

<details><summary>Screenshots</summary>
<h3>Policy listing</h3>
<img width="1302" alt="Screen Shot 2020-10-14 at 7 05 19 AM" src="https://user-images.githubusercontent.com/57655/95981056-1c1c6d00-0dec-11eb-9746-5e91043f3d19.png">

<h3>Update existing policy succeeds</h3>
<img width="1308" alt="Screen Shot 2020-10-14 at 7 05 49 AM" src="https://user-images.githubusercontent.com/57655/95981059-1cb50380-0dec-11eb-9e55-7a14b72085f9.png">
<h3>Creating new policy w/existing name fails</h3>
<img width="1308" alt="Screen Shot 2020-10-14 at 7 06 06 AM" src="https://user-images.githubusercontent.com/57655/95981062-1cb50380-0dec-11eb-833b-1ab6636a5c67.png">

<h3>Updating policy to existing name fails</h3>
<img width="1308" alt="Screen Shot 2020-10-14 at 7 08 05 AM" src="https://user-images.githubusercontent.com/57655/95981078-1f175d80-0dec-11eb-91d5-7ffd976da48e.png">

</details>
### Checklist

- [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
@jen-huang
Copy link
Contributor

This needs a backport to 7.x too 🙂

jfsiii pushed a commit to jfsiii/kibana that referenced this pull request Oct 14, 2020
## Summary

fixes elastic#80136

Haven't added/updated tests yet but tested the various combinations in the UI and all worked as expected.

<details><summary>Screenshots</summary>
<h3>Policy listing</h3>
<img width="1302" alt="Screen Shot 2020-10-14 at 7 05 19 AM" src="https://user-images.githubusercontent.com/57655/95981056-1c1c6d00-0dec-11eb-9746-5e91043f3d19.png">

<h3>Update existing policy succeeds</h3>
<img width="1308" alt="Screen Shot 2020-10-14 at 7 05 49 AM" src="https://user-images.githubusercontent.com/57655/95981059-1cb50380-0dec-11eb-9e55-7a14b72085f9.png">
<h3>Creating new policy w/existing name fails</h3>
<img width="1308" alt="Screen Shot 2020-10-14 at 7 06 06 AM" src="https://user-images.githubusercontent.com/57655/95981062-1cb50380-0dec-11eb-833b-1ab6636a5c67.png">

<h3>Updating policy to existing name fails</h3>
<img width="1308" alt="Screen Shot 2020-10-14 at 7 08 05 AM" src="https://user-images.githubusercontent.com/57655/95981078-1f175d80-0dec-11eb-91d5-7ffd976da48e.png">

</details>
### Checklist

- [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
jfsiii pushed a commit that referenced this pull request Oct 15, 2020
…0589)

## Summary

fixes #80136

Haven't added/updated tests yet but tested the various combinations in the UI and all worked as expected.

<details><summary>Screenshots</summary>
<h3>Policy listing</h3>
<img width="1302" alt="Screen Shot 2020-10-14 at 7 05 19 AM" src="https://user-images.githubusercontent.com/57655/95981056-1c1c6d00-0dec-11eb-9746-5e91043f3d19.png">

<h3>Update existing policy succeeds</h3>
<img width="1308" alt="Screen Shot 2020-10-14 at 7 05 49 AM" src="https://user-images.githubusercontent.com/57655/95981059-1cb50380-0dec-11eb-9e55-7a14b72085f9.png">
<h3>Creating new policy w/existing name fails</h3>
<img width="1308" alt="Screen Shot 2020-10-14 at 7 06 06 AM" src="https://user-images.githubusercontent.com/57655/95981062-1cb50380-0dec-11eb-833b-1ab6636a5c67.png">

<h3>Updating policy to existing name fails</h3>
<img width="1308" alt="Screen Shot 2020-10-14 at 7 08 05 AM" src="https://user-images.githubusercontent.com/57655/95981078-1f175d80-0dec-11eb-91d5-7ffd976da48e.png">

</details>
### Checklist

- [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.10.0 v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error message is displayed on editing the value of policy fields
5 participants