Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Adding User support for Detector and DetectorJob #251

Merged
merged 4 commits into from
Oct 15, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,8 @@ dependencies {
compile "org.elasticsearch:elasticsearch:${es_version}"
compileOnly "org.elasticsearch.plugin:elasticsearch-scripting-painless-spi:${versions.elasticsearch}"
compileOnly "com.amazon.opendistroforelasticsearch:opendistro-job-scheduler-spi:1.10.1.1"
// Will be moved to Maven Depedency when https://github.com/opendistro-for-elasticsearch/common-utils repo publishes a release
compile files('libs/common-utils-1.10.1.0.jar')
compile group: 'com.google.guava', name: 'guava', version:'15.0'
compile group: 'org.apache.commons', name: 'commons-math3', version: '3.6.1'
compile group: 'com.google.code.gson', name: 'gson', version: '2.8.5'
Expand All @@ -306,6 +308,7 @@ dependencies {
compile group: 'commons-lang', name: 'commons-lang', version: '2.6'
compile 'software.amazon.randomcutforest:randomcutforest-core:1.0'
compile 'software.amazon.randomcutforest:randomcutforest-serialization-json:1.0'
compile "org.elasticsearch.client:elasticsearch-rest-client:${es_version}"

compile "org.jacoco:org.jacoco.agent:0.8.5"
compile ("org.jacoco:org.jacoco.ant:0.8.5") {
Expand Down
Binary file added libs/common-utils-1.10.1.0.jar
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,8 @@ private void stopAdJob(String detectorId) {
job.getEnabledTime(),
Instant.now(),
Instant.now(),
job.getLockDurationSeconds()
job.getLockDurationSeconds(),
job.getUser()
);
IndexRequest indexRequest = new IndexRequest(AnomalyDetectorJob.ANOMALY_DETECTOR_JOB_INDEX)
.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@

import com.amazon.opendistroforelasticsearch.ad.annotation.Generated;
import com.amazon.opendistroforelasticsearch.ad.util.ParseUtils;
import com.amazon.opendistroforelasticsearch.commons.authuser.User;
import com.google.common.base.Objects;

/**
Expand Down Expand Up @@ -80,6 +81,7 @@ public class AnomalyDetector implements Writeable, ToXContentObject {
private static final String LAST_UPDATE_TIME_FIELD = "last_update_time";
public static final String UI_METADATA_FIELD = "ui_metadata";
public static final String CATEGORY_FIELD = "category_field";
public static final String USER_FIELD = "user";

private final String detectorId;
private final Long version;
Expand All @@ -96,6 +98,7 @@ public class AnomalyDetector implements Writeable, ToXContentObject {
private final Integer schemaVersion;
private final Instant lastUpdateTime;
private final List<String> categoryFields;
private User user;

/**
* Constructor function.
Expand All @@ -114,7 +117,8 @@ public class AnomalyDetector implements Writeable, ToXContentObject {
* @param uiMetadata metadata used by Kibana
* @param schemaVersion anomaly detector index mapping version
* @param lastUpdateTime detector's last update time
* @param categoryFields a list of partition fields
* @param categoryFields a list of partition fields
* @param user user to which detector is associated
*/
public AnomalyDetector(
String detectorId,
Expand All @@ -131,7 +135,8 @@ public AnomalyDetector(
Map<String, Object> uiMetadata,
Integer schemaVersion,
Instant lastUpdateTime,
List<String> categoryFields
List<String> categoryFields,
User user
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we just add User in each detector document. Security plugin will do authorization check for each request based on backend_roles or roles ? Can you share the related code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question, security plugin will only provide the authorization headers.
My next PR will address this. Basically we have to query and filter by roles or backend_roles.
Here is a code pointer in alerting which does that: opendistro-for-elasticsearch/alerting@44abca1#diff-39bec5249e2b91e055cdb30bb3edcd28ee868ab3676e98652547bcce28eed895R86

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, that makes sense.

) {
if (Strings.isBlank(name)) {
throw new IllegalArgumentException("Detector name should be set");
Expand Down Expand Up @@ -166,6 +171,7 @@ public AnomalyDetector(
this.schemaVersion = schemaVersion;
this.lastUpdateTime = lastUpdateTime;
this.categoryFields = categoryFields;
this.user = user;
}

// TODO: remove after complete code merges. Created to not to touch too
Expand All @@ -184,7 +190,8 @@ public AnomalyDetector(
Integer shingleSize,
Map<String, Object> uiMetadata,
Integer schemaVersion,
Instant lastUpdateTime
Instant lastUpdateTime,
User user
) {
this(
detectorId,
Expand All @@ -201,7 +208,8 @@ public AnomalyDetector(
uiMetadata,
schemaVersion,
lastUpdateTime,
null
null,
user
);
}

Expand Down Expand Up @@ -237,6 +245,11 @@ public AnomalyDetector(StreamInput input) throws IOException {
schemaVersion = input.readInt();
lastUpdateTime = input.readInstant();
this.categoryFields = input.readStringList();
if (input.readBoolean()) {
this.user = new User(input);
} else {
user = null;
}
}

public XContentBuilder toXContent(XContentBuilder builder) throws IOException {
Expand All @@ -260,6 +273,12 @@ public void writeTo(StreamOutput output) throws IOException {
output.writeInt(schemaVersion);
output.writeInstant(lastUpdateTime);
output.writeStringCollection(categoryFields);
if (user != null) {
output.writeBoolean(true); // user exists
user.writeTo(output);
} else {
output.writeBoolean(false); // user does not exist
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For old detector, they have no user field, so they will be open to all users which has AD permission ?
For new detector, we will create default user with the creator's user role?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly, for existing detectors where User doesn't exist, the filter will not work and users who have all access to rest api's will be able to see those detectors.
This is to maintain backward compatibility.

}
}

@Override
Expand Down Expand Up @@ -289,6 +308,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
if (categoryFields != null) {
xContentBuilder.field(CATEGORY_FIELD, categoryFields.toArray());
}
if (user != null) {
Copy link
Contributor

@ylwu-amzn ylwu-amzn Oct 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great point sure. Will add.

xContentBuilder.field(USER_FIELD, user);
}
return xContentBuilder.endObject();
}

Expand Down Expand Up @@ -354,6 +376,7 @@ public static AnomalyDetector parse(
int schemaVersion = 0;
Map<String, Object> uiMetadata = null;
Instant lastUpdateTime = null;
User user = null;

List<String> categoryField = null;

Expand Down Expand Up @@ -415,6 +438,9 @@ public static AnomalyDetector parse(
case CATEGORY_FIELD:
categoryField = (List) parser.list();
break;
case USER_FIELD:
user = User.parse(parser);
break;
default:
parser.skipChildren();
break;
Expand All @@ -435,7 +461,8 @@ public static AnomalyDetector parse(
uiMetadata,
schemaVersion,
lastUpdateTime,
categoryField
categoryField,
user
);
}

Expand Down Expand Up @@ -583,4 +610,8 @@ public long getDetectorIntervalInSeconds() {
public Duration getDetectionIntervalDuration() {
return ((IntervalTimeConfiguration) getDetectionInterval()).toDuration();
}

public User getUser() {
return user;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.elasticsearch.common.xcontent.XContentParser;

import com.amazon.opendistroforelasticsearch.ad.util.ParseUtils;
import com.amazon.opendistroforelasticsearch.commons.authuser.User;
import com.amazon.opendistroforelasticsearch.jobscheduler.spi.ScheduledJobParameter;
import com.amazon.opendistroforelasticsearch.jobscheduler.spi.schedule.CronSchedule;
import com.amazon.opendistroforelasticsearch.jobscheduler.spi.schedule.IntervalSchedule;
Expand Down Expand Up @@ -64,6 +65,7 @@ enum ScheduleType {
private static final String IS_ENABLED_FIELD = "enabled";
private static final String ENABLED_TIME_FIELD = "enabled_time";
private static final String DISABLED_TIME_FIELD = "disabled_time";
public static final String USER_FIELD = "user";

private final String name;
private final Schedule schedule;
Expand All @@ -73,6 +75,7 @@ enum ScheduleType {
private final Instant disabledTime;
private final Instant lastUpdateTime;
private final Long lockDurationSeconds;
private final User user;

public AnomalyDetectorJob(
String name,
Expand All @@ -82,7 +85,8 @@ public AnomalyDetectorJob(
Instant enabledTime,
Instant disabledTime,
Instant lastUpdateTime,
Long lockDurationSeconds
Long lockDurationSeconds,
User user
) {
this.name = name;
this.schedule = schedule;
Expand All @@ -92,6 +96,7 @@ public AnomalyDetectorJob(
this.disabledTime = disabledTime;
this.lastUpdateTime = lastUpdateTime;
this.lockDurationSeconds = lockDurationSeconds;
this.user = user;
}

public AnomalyDetectorJob(StreamInput input) throws IOException {
Expand All @@ -107,6 +112,11 @@ public AnomalyDetectorJob(StreamInput input) throws IOException {
disabledTime = input.readInstant();
lastUpdateTime = input.readInstant();
lockDurationSeconds = input.readLong();
if (input.readBoolean()) {
user = new User(input);
} else {
user = null;
}
}

@Override
Expand All @@ -123,6 +133,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
if (disabledTime != null) {
xContentBuilder.field(DISABLED_TIME_FIELD, disabledTime.toEpochMilli());
}
if (user != null) {
xContentBuilder.field(USER_FIELD, user);
}
return xContentBuilder.endObject();
}

Expand All @@ -140,6 +153,12 @@ public void writeTo(StreamOutput output) throws IOException {
output.writeInstant(disabledTime);
output.writeInstant(lastUpdateTime);
output.writeLong(lockDurationSeconds);
if (user != null) {
output.writeBoolean(true); // user exists
user.writeTo(output);
} else {
output.writeBoolean(false); // user does not exist
}
}

public static AnomalyDetectorJob parse(XContentParser parser) throws IOException {
Expand All @@ -152,6 +171,7 @@ public static AnomalyDetectorJob parse(XContentParser parser) throws IOException
Instant disabledTime = null;
Instant lastUpdateTime = null;
Long lockDurationSeconds = DEFAULT_AD_JOB_LOC_DURATION_SECONDS;
User user = null;

ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser::getTokenLocation);
while (parser.nextToken() != XContentParser.Token.END_OBJECT) {
Expand Down Expand Up @@ -183,6 +203,9 @@ public static AnomalyDetectorJob parse(XContentParser parser) throws IOException
case LOCK_DURATION_SECONDS:
lockDurationSeconds = parser.longValue();
break;
case USER_FIELD:
user = User.parse(parser);
break;
default:
parser.skipChildren();
break;
Expand All @@ -196,7 +219,8 @@ public static AnomalyDetectorJob parse(XContentParser parser) throws IOException
enabledTime,
disabledTime,
lastUpdateTime,
lockDurationSeconds
lockDurationSeconds,
user
);
}

Expand Down Expand Up @@ -258,4 +282,8 @@ public Instant getLastUpdateTime() {
public Long getLockDurationSeconds() {
return lockDurationSeconds;
}

public User getUser() {
return user;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,8 @@ private void indexAnomalyDetector(String detectorId) throws IOException {
anomalyDetector.getUiMetadata(),
anomalyDetector.getSchemaVersion(),
Instant.now(),
anomalyDetector.getCategoryField()
anomalyDetector.getCategoryField(),
anomalyDetector.getUser()
);
IndexRequest indexRequest = new IndexRequest(ANOMALY_DETECTORS_INDEX)
.setRefreshPolicy(refreshPolicy)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,8 @@ private void onGetAnomalyDetectorResponse(GetResponse response) throws IOExcepti
Instant.now(),
null,
Instant.now(),
duration.getSeconds()
duration.getSeconds(),
detector.getUser()
);

getAnomalyDetectorJobForWrite(job);
Expand Down Expand Up @@ -218,7 +219,8 @@ private void onGetAnomalyDetectorJobForWrite(GetResponse response, AnomalyDetect
Instant.now(),
currentAdJob.getDisabledTime(),
Instant.now(),
job.getLockDurationSeconds()
job.getLockDurationSeconds(),
job.getUser()
);
indexAnomalyDetectorJob(newJob, null);
}
Expand Down Expand Up @@ -301,7 +303,8 @@ public void stopAnomalyDetectorJob(String detectorId) {
job.getEnabledTime(),
Instant.now(),
Instant.now(),
job.getLockDurationSeconds()
job.getLockDurationSeconds(),
job.getUser()
);
indexAnomalyDetectorJob(
newJob,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,8 @@ private void testRunAdJobWithEndRunExceptionNowAndStopAdJob(boolean jobExists, b
Instant.now().minusSeconds(60),
Instant.now(),
Instant.now(),
60L
60L,
TestHelpers.randomUser()
).toXContent(TestHelpers.builder(), ToXContent.EMPTY_PARAMS)
),
Collections.emptyMap(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ protected AnomalyDetector createAnomalyDetector(AnomalyDetector detector, Boolea
detector.getUiMetadata(),
detector.getSchemaVersion(),
detector.getLastUpdateTime(),
null
null,
detector.getUser()
);
}

Expand Down Expand Up @@ -178,7 +179,8 @@ public ToXContentObject[] getAnomalyDetector(String detectorId, BasicHeader head
detector.getUiMetadata(),
detector.getSchemaVersion(),
detector.getLastUpdateTime(),
null
null,
detector.getUser()
),
detectorJob };
}
Expand Down
Loading