honeymoose/OpenSearch
1
0
Fork 0
mirror of https://github.com/honeymoose/OpenSearch.git synced 2025-03-25 17:38:44 +00:00
Code Issues Packages Projects Releases Wiki Activity

[ML] Validate initial job settings (elastic/x-pack-elasticsearch#1646)

Browse Source 
* [ML] Validate initial job settings

* Add same job creation checks to the validate endpoint

Original commit: elastic/x-pack-elasticsearch@ab76cf9ea2
This commit is contained in:
David Kyle 2017-06-07 09:34:58 +01:00 committed by GitHub
parent 87edc4bfdd
commit ae299f633e
10 changed files with 160 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
plugin/src/main/java/org/elasticsearch/xpack/ml/action/PutJobAction.java
View File

@ -39,6 +39,7 @@ import org.elasticsearch.xpack.ml.job.config.Job;
import org.elasticsearch.xpack.ml.job.messages.Messages;
import java.io.IOException;
import java.util.List;
import java.util.Objects;
public class PutJobAction extends Action<PutJobAction.Request, PutJobAction.Response, PutJobAction.RequestBuilder> {
@ -72,6 +73,13 @@ public class PutJobAction extends Action<PutJobAction.Request, PutJobAction.Resp
jobBuilder.getId(), jobId));
}
// Some fields cannot be set at create time
List<String> invalidJobCreationSettings = jobBuilder.invalidCreateTimeSettings();
if (invalidJobCreationSettings.isEmpty() == false) {
throw new IllegalArgumentException(Messages.getMessage(Messages.JOB_CONFIG_INVALID_CREATE_SETTINGS,
String.join(",", invalidJobCreationSettings)));
}
return new Request(jobBuilder);
}

21
plugin/src/main/java/org/elasticsearch/xpack/ml/action/ValidateJobConfigAction.java
View File

@ -19,15 +19,15 @@ import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TransportService;
import org.elasticsearch.xpack.ml.job.config.Job;
import org.elasticsearch.xpack.ml.job.messages.Messages;
import java.io.IOException;
import java.util.Date;
import java.util.List;
import java.util.Objects;
public class ValidateJobConfigAction
@ -58,7 +58,7 @@ extends Action<ValidateJobConfigAction.Request, ValidateJobConfigAction.Response
}
public static class Request extends ActionRequest implements ToXContent {
public static class Request extends ActionRequest {
private Job job;
@ -68,7 +68,14 @@ extends Action<ValidateJobConfigAction.Request, ValidateJobConfigAction.Response
// be valid unless an invalid job ID is specified in the JSON to be validated
job.setId(job.getId() != null ? job.getId() : "ok");
return new Request(job.getCreateTime() == null ? job.build(new Date()) : job.build());
// Some fields cannot be set at create time
List<String> invalidJobCreationSettings = job.invalidCreateTimeSettings();
if (invalidJobCreationSettings.isEmpty() == false) {
throw new IllegalArgumentException(Messages.getMessage(Messages.JOB_CONFIG_INVALID_CREATE_SETTINGS,
String.join(",", invalidJobCreationSettings)));
}
return new Request(job.build(new Date()));
}
Request() {
@ -100,12 +107,6 @@ extends Action<ValidateJobConfigAction.Request, ValidateJobConfigAction.Response
job = new Job(in);
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
job.toXContent(builder, params);
return builder;
}
@Override
public int hashCode() {
return Objects.hash(job);

32
plugin/src/main/java/org/elasticsearch/xpack/ml/job/config/Job.java
View File

@ -751,6 +751,29 @@ public class Job extends AbstractDiffable<Job> implements Writeable, ToXContent
return this;
}
/**
* Return the list of fields that have been set and are invalid to
* be set when the job is created e.g. model snapshot Id should not
* be set at job creation.
* @return List of fields set fields that should not be.
*/
public List<String> invalidCreateTimeSettings() {
List<String> invalidCreateValues = new ArrayList<>();
if (modelSnapshotId != null) {
invalidCreateValues.add(MODEL_SNAPSHOT_ID.getPreferredName());
}
if (lastDataTime != null) {
invalidCreateValues.add(LAST_DATA_TIME.getPreferredName());
}
if (finishedTime != null) {
invalidCreateValues.add(FINISHED_TIME.getPreferredName());
}
if (createTime != null) {
invalidCreateValues.add(CREATE_TIME.getPreferredName());
}
return invalidCreateValues;
}
@Override
public void writeTo(StreamOutput out) throws IOException {
out.writeOptionalString(id);
@ -953,7 +976,7 @@ public class Job extends AbstractDiffable<Job> implements Writeable, ToXContent
validateInputFields();
// Creation time is NOT required in user input, hence validated only on build
Date createTime = ExceptionsHelper.requireNonNull(this.createTime, CREATE_TIME.getPreferredName());
ExceptionsHelper.requireNonNull(createTime, CREATE_TIME.getPreferredName());
if (Strings.isNullOrEmpty(resultsIndexName)) {
resultsIndexName = AnomalyDetectorsIndex.RESULTS_INDEX_DEFAULT;
@ -967,10 +990,9 @@ public class Job extends AbstractDiffable<Job> implements Writeable, ToXContent
return new Job(
id, jobType, jobVersion, description, createTime, finishedTime, lastDataTime,
analysisConfig, analysisLimits,
dataDescription, modelPlotConfig, renormalizationWindowDays, backgroundPersistInterval,
modelSnapshotRetentionDays, resultsRetentionDays, customSettings, modelSnapshotId,
resultsIndexName, deleted);
analysisConfig, analysisLimits, dataDescription, modelPlotConfig, renormalizationWindowDays,
backgroundPersistInterval, modelSnapshotRetentionDays, resultsRetentionDays, customSettings,
modelSnapshotId, resultsIndexName, deleted);
}
private void checkValidBackgroundPersistInterval() {

2
plugin/src/main/java/org/elasticsearch/xpack/ml/job/messages/Messages.java
View File

@ -100,6 +100,8 @@ public final class Messages {
public static final String JOB_CONFIG_FUNCTION_REQUIRES_OVERFIELD = "over_field_name must be set when the ''{0}'' function is used";
public static final String JOB_CONFIG_ID_ALREADY_TAKEN = "The job cannot be created with the Id ''{0}''. The Id is already used.";
public static final String JOB_CONFIG_ID_TOO_LONG = "The job id cannot contain more than {0,number,integer} characters.";
public static final String JOB_CONFIG_INVALID_CREATE_SETTINGS =
"The job is configured with fields [{0}] that are illegal to set at job creation";
public static final String JOB_CONFIG_INVALID_FIELDNAME_CHARS =
"Invalid field name ''{0}''. Field names including over, by and partition fields cannot contain any of these characters: {1}";
public static final String JOB_CONFIG_INVALID_TIMEFORMAT = "Invalid Time format string ''{0}''";

17
plugin/src/test/java/org/elasticsearch/xpack/ml/action/PutJobActionRequestTests.java
View File

@ -5,11 +5,16 @@
*/
package org.elasticsearch.xpack.ml.action;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.xpack.ml.action.PutJobAction.Request;
import org.elasticsearch.xpack.ml.job.config.Job;
import org.elasticsearch.xpack.ml.support.AbstractStreamableXContentTestCase;
import java.io.IOException;
import java.util.Date;
import static org.elasticsearch.xpack.ml.job.config.JobTests.buildJobBuilder;
@ -18,11 +23,10 @@ import static org.elasticsearch.xpack.ml.job.config.JobTests.randomValidJobId;
public class PutJobActionRequestTests extends AbstractStreamableXContentTestCase<Request> {
private final String jobId = randomValidJobId();
private final Date date = new Date();
@Override
protected Request createTestInstance() {
Job.Builder jobConfiguration = buildJobBuilder(jobId, date);
Job.Builder jobConfiguration = buildJobBuilder(jobId, null);
return new Request(jobConfiguration);
}
@ -36,4 +40,13 @@ public class PutJobActionRequestTests extends AbstractStreamableXContentTestCase
return Request.parseRequest(jobId, parser);
}
public void testParseRequest_InvalidCreateSetting() throws IOException {
Job.Builder jobConfiguration = buildJobBuilder(jobId, null);
jobConfiguration.setLastDataTime(new Date());
XContentBuilder xContentBuilder = toXContent(jobConfiguration, XContentType.JSON);
XContentParser parser = XContentFactory.xContent(XContentType.JSON)
.createParser(NamedXContentRegistry.EMPTY, xContentBuilder.bytes());
expectThrows(IllegalArgumentException.class, () -> Request.parseRequest(jobId, parser));
}
}

33
plugin/src/test/java/org/elasticsearch/xpack/ml/action/ValidateJobConfigActionRequestTests.java
View File

@ -5,16 +5,27 @@
*/
package org.elasticsearch.xpack.ml.action;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.test.AbstractStreamableTestCase;
import org.elasticsearch.xpack.ml.action.ValidateJobConfigAction.Request;
import org.elasticsearch.xpack.ml.job.config.JobTests;
import org.elasticsearch.xpack.ml.support.AbstractStreamableXContentTestCase;
import org.elasticsearch.xpack.ml.job.config.Job;
public class ValidateJobConfigActionRequestTests extends AbstractStreamableXContentTestCase<ValidateJobConfigAction.Request> {
import java.io.IOException;
import java.util.Date;
import static org.elasticsearch.xpack.ml.job.config.JobTests.buildJobBuilder;
import static org.elasticsearch.xpack.ml.job.config.JobTests.randomValidJobId;
public class ValidateJobConfigActionRequestTests extends AbstractStreamableTestCase<Request> {
@Override
protected Request createTestInstance() {
return new Request(JobTests.createRandomizedJob());
return new Request(buildJobBuilder(randomValidJobId(), new Date()).build());
}
@Override
@ -22,8 +33,16 @@ public class ValidateJobConfigActionRequestTests extends AbstractStreamableXCont
return new Request();
}
@Override
protected Request parseInstance(XContentParser parser) {
return Request.parseRequest(parser);
public void testParseRequest_InvalidCreateSetting() throws IOException {
String jobId = randomValidJobId();
Job.Builder jobConfiguration = buildJobBuilder(jobId, null);
jobConfiguration.setLastDataTime(new Date());
XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON);
XContentBuilder xContentBuilder = jobConfiguration.toXContent(builder, ToXContent.EMPTY_PARAMS);
XContentParser parser = XContentFactory.xContent(XContentType.JSON)
.createParser(NamedXContentRegistry.EMPTY, xContentBuilder.bytes());
expectThrows(IllegalArgumentException.class, () -> Request.parseRequest(parser));
}
}

20
plugin/src/test/java/org/elasticsearch/xpack/ml/job/config/JobTests.java
View File

@ -23,8 +23,10 @@ import java.util.Arrays;
import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.equalTo;
@ -405,6 +407,24 @@ public class JobTests extends AbstractSerializingTestCase<Job> {
assertThat(Job.getCompatibleJobTypes(Version.V_5_5_0).size(), equalTo(1));
}
public void testInvalidCreateTimeSettings() {
Job.Builder builder = new Job.Builder("invalid-settings");
builder.setModelSnapshotId("snapshot-foo");
assertEquals(Collections.singletonList(Job.MODEL_SNAPSHOT_ID.getPreferredName()), builder.invalidCreateTimeSettings());
builder.setCreateTime(new Date());
builder.setFinishedTime(new Date());
builder.setLastDataTime(new Date());
Set<String> expected = new HashSet();
expected.add(Job.CREATE_TIME.getPreferredName());
expected.add(Job.FINISHED_TIME.getPreferredName());
expected.add(Job.LAST_DATA_TIME.getPreferredName());
expected.add(Job.MODEL_SNAPSHOT_ID.getPreferredName());
assertEquals(expected, new HashSet<>(builder.invalidCreateTimeSettings()));
}
public static Job.Builder buildJobBuilder(String id, Date date) {
Job.Builder builder = new Job.Builder(id);
builder.setCreateTime(date);

18
plugin/src/test/resources/rest-api-spec/test/ml/jobs_crud.yml
View File

@ -662,3 +662,21 @@
"data_description" : {
}
}
---
"Test cannot create job with model snapshot id set":
- do:
catch: /illegal_argument_exception/
xpack.ml.put_job:
job_id: has-model-snapshot-id
body: >
{
"model_snapshot_id": "wont-create-with-this-setting",
"analysis_config" : {
"bucket_span": "1h",
"detectors" :[{"function":"metric","field_name":"responsetime","by_field_name":"airline"}]
},
"data_description" : {
}
}

31
plugin/src/test/resources/rest-api-spec/test/ml/validate.yml
View File

@ -75,3 +75,34 @@
"time_format": "yyyy-MM-dd HH:mm:ssX"
}
}
---
"Test job config is invalid because model snapshot id set":
- do:
catch: /illegal_argument_exception/
xpack.ml.validate:
body: >
{
"model_snapshot_id": "wont-create-with-this-setting",
"analysis_config" : {
"bucket_span": "1h",
"detectors" :[{"function":"metric","field_name":"responsetime","by_field_name":"airline"}]
},
"data_description" : {
}
}
- do:
catch: /The job is configured with fields \[model_snapshot_id\] that are illegal to set at job creation/
xpack.ml.validate:
body: >
{
"model_snapshot_id": "wont-create-with-this-setting",
"analysis_config" : {
"bucket_span": "1h",
"detectors" :[{"function":"metric","field_name":"responsetime","by_field_name":"airline"}]
},
"data_description" : {
}
}

2
qa/smoke-test-ml-with-security/build.gradle
View File

@ -35,6 +35,7 @@ integTestRunner {
'ml/jobs_crud/Test cannot create job with existing result document',
'ml/jobs_crud/Test get job API with non existing job id',
'ml/jobs_crud/Test put job with inconsistent body/param ids',
'ml/jobs_crud/Test cannot create job with model snapshot id set',
'ml/jobs_get/Test get job given missing job_id',
'ml/jobs_get_result_buckets/Test mutually-exclusive params',
'ml/jobs_get_result_buckets/Test mutually-exclusive params via body',
@ -53,6 +54,7 @@ integTestRunner {
'ml/update_model_snapshot/Test without description',
'ml/validate/Test invalid job config',
'ml/validate/Test job config that is invalid only because of the job ID',
'ml/validate/Test job config is invalid because model snapshot id set',
'ml/validate_detector/Test invalid detector'
].join(',')
}