Skip to content

Commit fee3e84

Browse files
committed
Watcher: Fix put watch action (#31524)
If no version is specified when putting a watch, the index API should be used instead of the update API, so that the whole watch gets overwritten instead of being merged with the existing one. Merging only happens when a version is specified, so that credentials can be omitted, which is important for the watcher UI.
1 parent a465429 commit fee3e84

File tree

2 files changed

+104
-9
lines changed

2 files changed

+104
-9
lines changed
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
---
2+
"Test put watch api without version overwrites watch":
3+
- do:
4+
cluster.health:
5+
wait_for_status: yellow
6+
7+
- do:
8+
xpack.watcher.put_watch:
9+
id: "my_watch"
10+
body: >
11+
{
12+
"trigger": {
13+
"schedule": {
14+
"hourly": {
15+
"minute": [ 0, 5 ]
16+
}
17+
}
18+
},
19+
"input": {
20+
"simple": {
21+
"foo": "bar"
22+
}
23+
},
24+
"actions": {
25+
"logging": {
26+
"logging": {
27+
"text": "yaml test"
28+
}
29+
}
30+
}
31+
}
32+
- match: { _id: "my_watch" }
33+
34+
- do:
35+
xpack.watcher.get_watch:
36+
id: "my_watch"
37+
- match: { watch.input.simple.foo: "bar" }
38+
39+
# change the simple input fields, then ensure the old
40+
# field does not exist on get
41+
- do:
42+
xpack.watcher.put_watch:
43+
id: "my_watch"
44+
body: >
45+
{
46+
"trigger": {
47+
"schedule": {
48+
"hourly": {
49+
"minute": [ 0, 5 ]
50+
}
51+
}
52+
},
53+
"input": {
54+
"simple": {
55+
"spam": "eggs"
56+
}
57+
},
58+
"actions": {
59+
"logging": {
60+
"logging": {
61+
"text": "yaml test"
62+
}
63+
}
64+
}
65+
}
66+
- match: { _id: "my_watch" }
67+
68+
- do:
69+
xpack.watcher.get_watch:
70+
id: "my_watch"
71+
- match: { watch.input.simple.spam: "eggs" }
72+
- is_false: watch.input.simple.foo
73+

x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/transport/actions/put/TransportPutWatchAction.java

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77

88
import org.elasticsearch.action.ActionListener;
99
import org.elasticsearch.action.DocWriteResponse;
10+
import org.elasticsearch.action.index.IndexRequest;
11+
import org.elasticsearch.action.index.IndexResponse;
1012
import org.elasticsearch.action.support.ActionFilters;
1113
import org.elasticsearch.action.support.WriteRequest;
1214
import org.elasticsearch.action.update.UpdateRequest;
@@ -97,24 +99,44 @@ protected void masterOperation(PutWatchRequest request, ClusterState state,
9799
try (XContentBuilder builder = jsonBuilder()) {
98100
watch.toXContent(builder, DEFAULT_PARAMS);
99101

100-
UpdateRequest updateRequest = new UpdateRequest(Watch.INDEX, Watch.DOC_TYPE, request.getId());
101-
updateRequest.docAsUpsert(isUpdate == false);
102-
updateRequest.version(request.getVersion());
103-
updateRequest.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE);
104-
updateRequest.doc(builder);
102+
if (isUpdate) {
103+
UpdateRequest updateRequest = new UpdateRequest(Watch.INDEX, Watch.DOC_TYPE, request.getId());
104+
updateRequest.version(request.getVersion());
105+
updateRequest.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE);
106+
updateRequest.doc(builder);
105107

106-
executeAsyncWithOrigin(client.threadPool().getThreadContext(), WATCHER_ORIGIN, updateRequest,
107-
ActionListener.<UpdateResponse>wrap(response -> {
108+
executeAsyncWithOrigin(client.threadPool().getThreadContext(), WATCHER_ORIGIN, updateRequest,
109+
ActionListener.<UpdateResponse>wrap(response -> {
110+
boolean created = response.getResult() == DocWriteResponse.Result.CREATED;
111+
if (shouldBeTriggeredLocally(request, watch)) {
112+
triggerService.add(watch);
113+
}
114+
listener.onResponse(new PutWatchResponse(response.getId(), response.getVersion(), created));
115+
}, listener::onFailure),
116+
client::update);
117+
} else {
118+
IndexRequest indexRequest = new IndexRequest(Watch.INDEX, Watch.DOC_TYPE, request.getId());
119+
indexRequest.source(builder);
120+
indexRequest.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE);
121+
executeAsyncWithOrigin(client.threadPool().getThreadContext(), WATCHER_ORIGIN, indexRequest,
122+
ActionListener.<IndexResponse>wrap(response -> {
108123
boolean created = response.getResult() == DocWriteResponse.Result.CREATED;
109-
if (localExecute(request) == false && watch.status().state().isActive()) {
124+
// if not yet in distributed mode (mixed 5/6 version in cluster), only trigger on the master node
125+
if (shouldBeTriggeredLocally(request, watch)) {
110126
triggerService.add(watch);
111127
}
112128
listener.onResponse(new PutWatchResponse(response.getId(), response.getVersion(), created));
113129
}, listener::onFailure),
114-
client::update);
130+
client::index);
131+
}
115132
}
116133
} catch (Exception e) {
117134
listener.onFailure(e);
118135
}
119136
}
137+
138+
private boolean shouldBeTriggeredLocally(PutWatchRequest request, Watch watch) {
139+
return localExecute(request) == false &&
140+
watch.status().state().isActive();
141+
}
120142
}

0 commit comments

Comments
 (0)