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

[ILM] Relax POST policy route validation #84203

Merged

Conversation

jloleysens
Copy link
Contributor

Summary

After implementing #83077 I noticed that on instances of Kibana that did not know about the shrink action we are unable to update a policy through UI because of server-side validation. The thinking behind #83077 was that we want to preserve any values users may have configured through the ES API (even if Kibana does not know about it).

It seems that the simplest way to do this for now is to relax our server side policy validation. We can rely on ES to validate the policy configuration instead. The UI has control over fields it know about, the rest we pass back unaltered to ES.

Additionally, by relaxing our server-side validation we have less of a maintenance burden in case ES adds ILM policy features we have/will not support.

@jloleysens jloleysens added Feature:ILM v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more release_note:skip Skip the PR/issue when compiling release notes v7.11.0 labels Nov 24, 2020
@jloleysens jloleysens requested a review from a team as a code owner November 24, 2020 12:27
@elasticmachine
Copy link
Contributor

Pinging @elastic/es-ui (Team:Elasticsearch UI)

Copy link
Contributor

@yuliacech yuliacech left a comment

Choose a reason for hiding this comment

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

Hi @jloleysens , I agree that maintaining the schema is not providing as much benefit as it costs the effort. Our (de)serialization logic with unit tests should be enough to confirm that the object has the correct structure. And ES does the validation on its end as well.
nit: comment on line 26 is not needed anymore.

@jloleysens
Copy link
Contributor Author

Thanks for the review @yuliacech ! I am going to update the comment to capture the decision we made here so I will probably leave the link to the endpoint docs, but just state that we are intentionally not deeply validating the object structure.

@jloleysens jloleysens merged commit 8dfa489 into elastic:master Nov 25, 2020
@jloleysens jloleysens deleted the ilm/relax-server-side-policy-validation branch November 25, 2020 08:28
jloleysens added a commit to jloleysens/kibana that referenced this pull request Nov 25, 2020
* relax policy post route validation

* update comment
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

jloleysens added a commit that referenced this pull request Nov 25, 2020
* relax policy post route validation

* update comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:ILM release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants