Feature request: HEAD /_ilm/policy/{name}

[ruflin]

In elastic/kibana#52632 we check if an ILM policy already exist to make sure we don't overwrite it. Our initial implementation to do this check was:

GET /_ilm/policy/{name}

The problem with the above is that it throws an exception on the Elasticsearch side which I don't like as I only want to check if it is there. The current solution I was pointed to, to work around this is using the following:

GET /_ilm/policy/?filter_path={name}

What I would prefer to see is heaving a HEAD request for this as we have in other API's:

HEAD /_ilm/policy/{name}

[elasticmachine]

Pinging @elastic/es-core-features (:Core/Features/ILM+SLM)

[BogdanSukonnov]

can I take this issue?

[dakrone]

Certainly, let me know if you have any questions.

…

On Sat, Jan 11, 2020 at 4:22 AM Bogdan Sukonnov ***@***.***> wrote: can I take this issue?

[BogdanSukonnov]

@dakrone , @ruflin I don't understand what would be the response for new policy, and no exception part. Are you expect just 404 response? Can you, please give me an example of expected behavior in another API's?

[dakrone]

I would expect a 200 response if the policy exists, a 404 if it does not (with no body for either).

[BogdanSukonnov]

@dakrone thanks, that I can do. What I am struggling to figure out is how to response with 404 without exception. Only way I find to response with 404 is to call listener.onFailure(new ResourceNotFoundException(... and this will be an exception in logs, which we here try to avoid. So I need to call listener.onResponse(... that will return 404 somehow. I'm stuck here, can you give me some advice?

[dakrone]

@BogdanSukonnov sorry, I misunderstood what you were asking. I tested this locally, and all that is actually needed is to add the line:

controller.registerHandler(RestRequest.Method.HEAD, "/_ilm/policy/{name}", this);

And that's it. ES will automatically handle the response elision for you. So it'd just be this line and then a test for the functionality.

[BogdanSukonnov]

@dakrone it's great! Thanks! I'll try. Can't believe this is so simple :)

[BogdanSukonnov]

@dakrone OK, it's not so simple. It's maybe easy to get 404 response like always, when endpoint is not properly registered. But as soon as I implemented all necessary classes to receive 200 response for existing policy, the only way to answer with 404 for non-existing policy is to throw a ResourceNotFoundException exception. That lead to dirty logs, and it is not a desirable result in terms of this issue.
I found another way to solve this problem, without exception. It involves consuming parameters from RestRequest only if the policy is present, so when the policy name not found, ES does not consume request parameters and treat request as bad. But the response for non-existing policy will be 400 Bad Request. Is this OK?

[BogdanSukonnov]

No, unfortunately, it's not working, too. Looks that I can't solve this issue.

[DaveCTurner]

I think a ResourceNotFoundException is appropriate, and I think the logging you are seeing is only at DEBUG level so that's ok. I opened #51198 to discuss the possibility of not emitting DEBUG-level logging by default like this.

[BogdanSukonnov]

@DaveCTurner thanks, but this behavior is already implemented for Get method. Is there any point to do basically the same just to have another HTTP method?

[DaveCTurner]

That question is best directed back at @ruflin. Nicolas, HEAD /_ilm/policy/{name} would throw the same exception as GET /_ilm/policy/{name}. Is your issue simply that this exception is logged, or do you need HEAD to work here? The exception logging is deliberate (today) but we are discussing changing it in #51198.

Additionally, checking whether the policy exists and then creating it is racy: someone else might create the policy in between those two calls. Do you need the ability to atomically PUT a policy only if it doesn't exist?

[ruflin]

What I expected is the same behaviour as with the index API. When I use HEAD /foo for the index foo and it does not exist, nothing shows up in the logs which is what I expected. If in my code I want to check for 5 ILM policies to exist, the users gets a lot of information logged if they don't exist which I think might be confusing. GET /_ilm/policy/?filter_path={name} does what I expect.

I'm not really concerned here about the race condition part.

[DaveCTurner]

Ok, thanks for the clarification. TBH it's a bit strange that HEAD /foo doesn't log anything if the index isn't found since it does throw an exception internally, but this is how it is today. Since the fundamental problem here is just the excessive logging, I think that #51198 will fix this.

[DaveCTurner]

Addressed by #51459.