alerting icon indicating copy to clipboard operation
alerting copied to clipboard

[BUG] Alerting not honouring PUT method

Open adityaj1107 opened this issue 4 years ago • 4 comments

Issue by thenom Friday Jul 17, 2020 at 15:33 GMT Originally opened as https://github.com/opendistro-for-elasticsearch/alerting/issues/228


Describe the bug Cannot PUT a new monitor with a specified ID for a non-existant monitor:

PUT _opendistro/_alerting/monitors/flibble
{
  "type": "monitor",
  "name": "flibble",
  "enabled": true,
  "schedule": {
    "period": {
      "interval": 10,
      "unit": "MINUTES"
    }
...

Results in:

{
  "Message" : "Monitor with flibble is not found"
} {
  "_index" : ".opendistro-alerting-config",
  "_type" : "_doc",
  "_id" : "flibble",
  "found" : false
}

Is there any reason for this as this means that a search is required before hand to find if it exists in the cluster and then to perform a second call (either POST or PUT) based on the results of the search. I am just in the process of setting up OpenDistro so not sure if this issue appears elsewhere.

https://tools.ietf.org/html/rfc2616#section-9.6

If the Request-URI does not point to an existing resource, and that URI is capable of being defined as a new resource by the requesting user agent, the origin server can create the resource with that URI.

If this method was allowed as stated then this would remove that extra step in automation and you could just overwrite the monitor. I initially thought this might have been to inject the additional fields that appear in the monitor after the initial POST but this cant be the case because you still inject those fields during subsequent PUT operations when providing an existing _id.

Thanks,

adityaj1107 avatar Jun 02 '21 21:06 adityaj1107

Comment by skkosuri-amzn Wednesday Aug 19, 2020 at 16:32 GMT


Thanks for creating this issue. PUT _opendistro/_alerting/monitors/{id} : needs to create new monitor, if the given {id} is not present.

adityaj1107 avatar Jun 02 '21 21:06 adityaj1107

Comment by aditjind Wednesday Jun 02, 2021 at 03:26 GMT


Hi @thenom

According to the latest RFC7231, the PUT method definition states that:

A service that selects a proper URI on behalf of the client, after receiving a state-changing request, SHOULD be implemented using the POST method rather than PUT.

Since the ID for the monitors the plugin creates via POST method has the ID generated randomly (the document ID for the monitor document), we think this change would break the contract.

Let me know your thoughts.

Reference

adityaj1107 avatar Jun 02 '21 21:06 adityaj1107

Wanted to mention that @thenom made a comment in the old repo after this issue was migrated so that their response isn't missed. Please continue any further discussions in this issue.

qreshi avatar Feb 18 '22 15:02 qreshi

Looks like it can be implemented like notifications API https://opensearch.org/docs/latest/observing-your-data/notifications/api/#create-channel-configuration via specify as monitor_id in POST request.

It is important for me because i now do some automantin for monitors management (it's to hard deal with too many monitors via UI). So because we have only one unique identifier i should store it in my configuration and use it for creating monitor in that way i can be sure that i crate or change correct monitor every time i change my configuration. In case if I can't use monitor ID for creating monitor (this is the situation at the moment in version 2.13.0) I must use monitor name as unique identifier. But its wrong and uncomfortable. Because name can be changed via UI and it will lead to creating second monitor by automation, I can't freely change name in my configuration without additional manipulation, because it will also will be lead to create duplicate resource.

ComBin avatar Apr 18 '24 18:04 ComBin