Skip to content

Commit 2815a77

Browse files
przemekwitekprzemekwitek
authored
[Transform] Report transforms without config as erroneous. (elastic#81141)
1 parent 7d1c434 commit 2815a77

File tree

17 files changed

+696
-177
lines changed

17 files changed

+696
-177
lines changed

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/action/GetTransformAction.java

Lines changed: 72 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,12 @@
77

88
package org.elasticsearch.xpack.core.transform.action;
99

10+
import org.elasticsearch.Version;
1011
import org.elasticsearch.action.ActionRequestValidationException;
1112
import org.elasticsearch.action.ActionType;
1213
import org.elasticsearch.common.ValidationException;
1314
import org.elasticsearch.common.io.stream.StreamInput;
15+
import org.elasticsearch.common.io.stream.StreamOutput;
1416
import org.elasticsearch.common.io.stream.Writeable;
1517
import org.elasticsearch.common.logging.DeprecationCategory;
1618
import org.elasticsearch.common.logging.DeprecationLogger;
@@ -27,6 +29,7 @@
2729
import java.io.IOException;
2830
import java.util.ArrayList;
2931
import java.util.List;
32+
import java.util.Objects;
3033

3134
import static org.elasticsearch.action.ValidateActions.addValidationError;
3235

@@ -79,31 +82,77 @@ public String getResourceIdField() {
7982
}
8083
}
8184

82-
public static class Response extends AbstractGetResourcesResponse<TransformConfig> implements Writeable, ToXContentObject {
85+
public static class Response extends AbstractGetResourcesResponse<TransformConfig> implements ToXContentObject {
86+
87+
public static class Error implements Writeable, ToXContentObject {
88+
private static final ParseField TYPE = new ParseField("type");
89+
private static final ParseField REASON = new ParseField("reason");
90+
91+
private final String type;
92+
private final String reason;
93+
94+
public Error(String type, String reason) {
95+
this.type = Objects.requireNonNull(type);
96+
this.reason = Objects.requireNonNull(reason);
97+
}
98+
99+
public Error(StreamInput in) throws IOException {
100+
this.type = in.readString();
101+
this.reason = in.readString();
102+
}
103+
104+
@Override
105+
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
106+
builder.startObject();
107+
builder.field(TYPE.getPreferredName(), type);
108+
builder.field(REASON.getPreferredName(), reason);
109+
builder.endObject();
110+
return builder;
111+
}
112+
113+
@Override
114+
public void writeTo(StreamOutput out) throws IOException {
115+
out.writeString(type);
116+
out.writeString(reason);
117+
}
118+
}
83119

84120
public static final String INVALID_TRANSFORMS_DEPRECATION_WARNING = "Found [{}] invalid transforms";
85121
private static final ParseField INVALID_TRANSFORMS = new ParseField("invalid_transforms");
122+
private static final ParseField ERRORS = new ParseField("errors");
86123

87-
public Response(List<TransformConfig> transformConfigs, long count) {
88-
super(new QueryPage<>(transformConfigs, count, TransformField.TRANSFORMS));
89-
}
124+
private final List<Error> errors;
90125

91-
public Response() {
92-
super();
126+
public Response(List<TransformConfig> transformConfigs, long count, List<Error> errors) {
127+
super(new QueryPage<>(transformConfigs, count, TransformField.TRANSFORMS));
128+
this.errors = errors;
93129
}
94130

95131
public Response(StreamInput in) throws IOException {
96132
super(in);
133+
if (in.getVersion().onOrAfter(Version.V_8_1_0)) {
134+
if (in.readBoolean()) {
135+
this.errors = in.readList(Error::new);
136+
} else {
137+
this.errors = null;
138+
}
139+
} else {
140+
this.errors = null;
141+
}
97142
}
98143

99144
public List<TransformConfig> getTransformConfigurations() {
100145
return getResources().results();
101146
}
102147

103-
public long getCount() {
148+
public long getTransformConfigurationCount() {
104149
return getResources().count();
105150
}
106151

152+
public List<Error> getErrors() {
153+
return errors;
154+
}
155+
107156
@Override
108157
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
109158
List<String> invalidTransforms = new ArrayList<>();
@@ -132,11 +181,26 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
132181
invalidTransforms.size()
133182
);
134183
}
135-
184+
if (errors != null) {
185+
builder.field(ERRORS.getPreferredName(), errors);
186+
}
136187
builder.endObject();
137188
return builder;
138189
}
139190

191+
@Override
192+
public void writeTo(StreamOutput out) throws IOException {
193+
super.writeTo(out);
194+
if (out.getVersion().onOrAfter(Version.V_8_1_0)) {
195+
if (errors != null) {
196+
out.writeBoolean(true);
197+
out.writeList(errors);
198+
} else {
199+
out.writeBoolean(false);
200+
}
201+
}
202+
}
203+
140204
@Override
141205
protected Reader<TransformConfig> getReader() {
142206
return TransformConfig::new;

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/transform/action/GetTransformActionResponseTests.java

Lines changed: 80 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -19,52 +19,107 @@
1919
import org.elasticsearch.xpack.core.watcher.watch.Payload.XContent;
2020

2121
import java.io.IOException;
22-
import java.util.ArrayList;
2322
import java.util.List;
2423
import java.util.Map;
2524

25+
import static org.hamcrest.Matchers.equalTo;
26+
import static org.hamcrest.Matchers.is;
27+
import static org.hamcrest.Matchers.nullValue;
28+
2629
public class GetTransformActionResponseTests extends AbstractWireSerializingTransformTestCase<Response> {
2730

2831
public static Response randomTransformResponse() {
29-
List<TransformConfig> configs = new ArrayList<>();
30-
int totalConfigs = randomInt(10);
31-
for (int i = 0; i < totalConfigs; ++i) {
32-
configs.add(TransformConfigTests.randomTransformConfig());
33-
}
32+
List<TransformConfig> configs = randomList(0, 10, () -> TransformConfigTests.randomTransformConfig());
33+
List<Response.Error> errors = randomBoolean() ? randomList(1, 5, () -> randomError()) : null;
34+
return new Response(configs, randomNonNegativeLong(), errors);
35+
}
3436

35-
return new Response(configs, randomNonNegativeLong());
37+
private static Response.Error randomError() {
38+
return new Response.Error(randomAlphaOfLengthBetween(1, 10), randomAlphaOfLengthBetween(1, 20));
3639
}
3740

3841
public void testInvalidTransforms() throws IOException {
39-
List<TransformConfig> transforms = new ArrayList<>();
40-
41-
transforms.add(TransformConfigTests.randomTransformConfig());
42-
transforms.add(TransformConfigTests.randomInvalidTransformConfig());
43-
transforms.add(TransformConfigTests.randomTransformConfig());
44-
transforms.add(TransformConfigTests.randomInvalidTransformConfig());
42+
List<TransformConfig> transforms = List.of(
43+
TransformConfigTests.randomTransformConfig("valid-transform-1"),
44+
TransformConfigTests.randomInvalidTransformConfig("invalid-transform-1"),
45+
TransformConfigTests.randomTransformConfig("valid-transform-2"),
46+
TransformConfigTests.randomInvalidTransformConfig("invalid-transform-2")
47+
);
4548

46-
Response r = new Response(transforms, transforms.size());
49+
Response r = new Response(transforms, transforms.size(), null);
4750
XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values()));
4851
r.toXContent(builder, XContent.EMPTY_PARAMS);
4952
Map<String, Object> responseAsMap = createParser(builder).map();
5053
assertEquals(2, XContentMapValues.extractValue("invalid_transforms.count", responseAsMap));
51-
List<String> expectedInvalidTransforms = new ArrayList<>();
52-
expectedInvalidTransforms.add(transforms.get(1).getId());
53-
expectedInvalidTransforms.add(transforms.get(3).getId());
54+
List<String> expectedInvalidTransforms = List.of("invalid-transform-1", "invalid-transform-2");
5455
assertEquals(expectedInvalidTransforms, XContentMapValues.extractValue("invalid_transforms.transforms", responseAsMap));
5556
assertWarnings(LoggerMessageFormat.format(Response.INVALID_TRANSFORMS_DEPRECATION_WARNING, 2));
5657
}
5758

59+
public void testErrors() throws IOException {
60+
List<Response.Error> errors = List.of(
61+
new Response.Error("error-type-1", "error-message-1"),
62+
new Response.Error("error-type-2", "error-message-2"),
63+
new Response.Error("error-type-3", "error-message-3")
64+
);
65+
66+
Response r = new Response(List.of(), 0, errors);
67+
XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values()));
68+
r.toXContent(builder, XContent.EMPTY_PARAMS);
69+
Map<String, Object> responseAsMap = createParser(builder).map();
70+
assertThat(XContentMapValues.extractValue("invalid_transforms", responseAsMap), is(nullValue()));
71+
assertThat(
72+
XContentMapValues.extractValue("errors", responseAsMap),
73+
is(
74+
equalTo(
75+
List.of(
76+
Map.of("type", "error-type-1", "reason", "error-message-1"),
77+
Map.of("type", "error-type-2", "reason", "error-message-2"),
78+
Map.of("type", "error-type-3", "reason", "error-message-3")
79+
)
80+
)
81+
)
82+
);
83+
ensureNoWarnings();
84+
}
85+
86+
public void testBothInvalidConfigsAndErrors() throws IOException {
87+
List<TransformConfig> transforms = List.of(TransformConfigTests.randomInvalidTransformConfig("invalid-transform-7"));
88+
List<Response.Error> errors = List.of(
89+
new Response.Error("error-type-1", "error-message-1"),
90+
new Response.Error("error-type-2", "error-message-2"),
91+
new Response.Error("error-type-3", "error-message-3")
92+
);
93+
94+
Response r = new Response(transforms, transforms.size(), errors);
95+
XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values()));
96+
r.toXContent(builder, XContent.EMPTY_PARAMS);
97+
Map<String, Object> responseAsMap = createParser(builder).map();
98+
assertThat(XContentMapValues.extractValue("invalid_transforms.count", responseAsMap), is(equalTo(1)));
99+
assertThat(
100+
XContentMapValues.extractValue("invalid_transforms.transforms", responseAsMap),
101+
is(equalTo(List.of("invalid-transform-7")))
102+
);
103+
assertThat(
104+
XContentMapValues.extractValue("errors", responseAsMap),
105+
is(
106+
equalTo(
107+
List.of(
108+
Map.of("type", "error-type-1", "reason", "error-message-1"),
109+
Map.of("type", "error-type-2", "reason", "error-message-2"),
110+
Map.of("type", "error-type-3", "reason", "error-message-3")
111+
)
112+
)
113+
)
114+
);
115+
assertWarnings(LoggerMessageFormat.format(Response.INVALID_TRANSFORMS_DEPRECATION_WARNING, 1));
116+
}
117+
58118
@SuppressWarnings("unchecked")
59119
public void testNoHeaderInResponse() throws IOException {
60-
List<TransformConfig> transforms = new ArrayList<>();
61-
int totalConfigs = randomInt(10);
62-
63-
for (int i = 0; i < totalConfigs; ++i) {
64-
transforms.add(TransformConfigTests.randomTransformConfig());
65-
}
120+
List<TransformConfig> transforms = randomList(0, 10, () -> TransformConfigTests.randomTransformConfig());
66121

67-
Response r = new Response(transforms, transforms.size());
122+
Response r = new Response(transforms, transforms.size(), null);
68123
XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values()));
69124
r.toXContent(builder, XContent.EMPTY_PARAMS);
70125
Map<String, Object> responseAsMap = createParser(builder).map();
@@ -79,7 +134,7 @@ public void testNoHeaderInResponse() throws IOException {
79134
for (int i = 0; i < transforms.size(); ++i) {
80135
assertArrayEquals(
81136
transforms.get(i).getSource().getIndex(),
82-
((ArrayList<String>) XContentMapValues.extractValue("source.index", transformsResponse.get(i))).toArray(new String[0])
137+
((List<String>) XContentMapValues.extractValue("source.index", transformsResponse.get(i))).toArray(new String[0])
83138
);
84139
assertEquals(null, XContentMapValues.extractValue("headers", transformsResponse.get(i)));
85140
}

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/transform/transforms/TransformConfigTests.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ public static TransformConfig randomTransformConfig(String id, Version version,
141141
);
142142
}
143143

144-
public static TransformConfig randomInvalidTransformConfig() {
144+
public static TransformConfig randomInvalidTransformConfig(String id) {
145145
if (randomBoolean()) {
146146
PivotConfig pivotConfig;
147147
LatestConfig latestConfig;
@@ -154,7 +154,7 @@ public static TransformConfig randomInvalidTransformConfig() {
154154
}
155155

156156
return new TransformConfig(
157-
randomAlphaOfLengthBetween(1, 10),
157+
id,
158158
randomInvalidSourceConfig(),
159159
randomDestConfig(),
160160
null,
@@ -171,7 +171,7 @@ public static TransformConfig randomInvalidTransformConfig() {
171171
);
172172
} // else
173173
return new TransformConfig(
174-
randomAlphaOfLengthBetween(1, 10),
174+
id,
175175
randomSourceConfig(),
176176
randomDestConfig(),
177177
null,

‎x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/TransformDeprecationChecker.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ private void recursiveGetTransformsAndCollectDeprecations(
6363
for (TransformConfig config : getTransformResponse.getTransformConfigurations()) {
6464
issues.addAll(config.checkForDeprecations(components.xContentRegistry()));
6565
}
66-
if (getTransformResponse.getCount() >= (page.getFrom() + page.getSize())) {
66+
if (getTransformResponse.getTransformConfigurationCount() >= (page.getFrom() + page.getSize())) {
6767
PageParams nextPage = new PageParams(page.getFrom() + page.getSize(), PageParams.DEFAULT_SIZE);
6868
recursiveGetTransformsAndCollectDeprecations(components, issues, nextPage, listener);
6969
} else {

0 commit comments

Comments
 (0)