Skip to content

Commit

Permalink
[ML] Skip remote clusters when performing up front privileges validat…
Browse files Browse the repository at this point in the history
…ion (#91895)
  • Loading branch information
przemekwitek authored Nov 24, 2022
1 parent 9e04540 commit 23eafaa
Show file tree
Hide file tree
Showing 7 changed files with 363 additions and 8 deletions.
6 changes: 6 additions & 0 deletions docs/changelog/91895.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 91895
summary: Skip remote clusters when performing up front privileges validation for datafeeds
area: Machine Learning
type: bug
issues:
- 87832
63 changes: 63 additions & 0 deletions x-pack/plugin/ml/qa/multi-cluster-tests-with-security/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import org.elasticsearch.gradle.internal.test.RestIntegTestTask
import org.elasticsearch.gradle.Version
import org.elasticsearch.gradle.VersionProperties

apply plugin: 'elasticsearch.internal-testclusters'
apply plugin: 'elasticsearch.standalone-rest-test'
apply plugin: 'elasticsearch.rest-resources'

dependencies {
testImplementation project(':x-pack:qa')
}

Version ccsCompatVersion = new Version(VersionProperties.getElasticsearchVersion().getMajor(), VersionProperties.getElasticsearchVersion().getMinor() - 1, 0)

restResources {
restApi {
include '_common', 'bulk', 'indices', 'cluster', 'search', 'security', 'ml'
}
}

def remoteCluster = testClusters.register('remote-cluster') {
testDistribution = 'DEFAULT'
versions = [ccsCompatVersion.toString(), project.version]
numberOfNodes = 2
setting 'node.roles', '[data,ingest,master]'
setting 'xpack.security.enabled', 'true'
setting 'xpack.watcher.enabled', 'false'
setting 'xpack.license.self_generated.type', 'trial'

user username: "test_user", password: "x-pack-test-password"
}

testClusters.register('mixed-cluster') {
testDistribution = 'DEFAULT'
numberOfNodes = 2
setting 'node.roles', '[data,ingest,master]'
setting 'xpack.security.enabled', 'true'
setting 'xpack.watcher.enabled', 'false'
setting 'xpack.license.self_generated.type', 'trial'
setting 'cluster.remote.my_remote_cluster.seeds', {
remoteCluster.get().getAllTransportPortURI().collect { "\"$it\"" }.toString()
}
setting 'cluster.remote.connections_per_cluster', "1"

user username: "test_user", password: "x-pack-test-password"
}

tasks.register('remote-cluster', RestIntegTestTask) {
mustRunAfter("precommit")
systemProperty 'tests.rest.suite', 'remote_cluster'
}

tasks.register('mixed-cluster', RestIntegTestTask) {
dependsOn 'remote-cluster'
useCluster remoteCluster
systemProperty 'tests.rest.suite', 'multi_cluster'
}

tasks.register("integTest") {
dependsOn 'mixed-cluster'
}

tasks.named("check").configure { dependsOn("integTest") }
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

package org.elasticsearch.multi_cluster;

import com.carrotsearch.randomizedtesting.annotations.Name;
import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
import com.carrotsearch.randomizedtesting.annotations.TimeoutSuite;

import org.apache.lucene.tests.util.TimeUnits;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.test.rest.yaml.ClientYamlTestCandidate;
import org.elasticsearch.test.rest.yaml.ESClientYamlSuiteTestCase;

@TimeoutSuite(millis = 5 * TimeUnits.MINUTE) // to account for slow as hell VMs
public class MultiClusterYamlTestSuiteIT extends ESClientYamlSuiteTestCase {

private static final String USER = "test_user";
private static final String PASS = "x-pack-test-password";

@Override
protected boolean preserveIndicesUponCompletion() {
return true;
}

public MultiClusterYamlTestSuiteIT(@Name("yaml") ClientYamlTestCandidate testCandidate) {
super(testCandidate);
}

@ParametersFactory
public static Iterable<Object[]> parameters() throws Exception {
return createParameters();
}

@Override
protected Settings restClientSettings() {
String token = basicAuthHeaderValue(USER, new SecureString(PASS.toCharArray()));
return Settings.builder().put(super.restClientSettings()).put(ThreadContext.PREFIX + ".Authorization", token).build();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
---
setup:
- skip:
features: headers

- do:
cluster.health:
wait_for_status: yellow

- do:
security.put_user:
username: "joe"
body: >
{
"password": "joe-password",
"roles" : [ "x_cluster_role" ]
}
- do:
security.put_role:
name: "x_cluster_role"
body: >
{
"cluster": ["manage_ml"],
"indices": [
{
"names": ["test_index"],
"privileges": ["read", "view_index_metadata"]
}
]
}
---
teardown:
- do:
security.delete_user:
username: "joe"
ignore: 404

---
"Test ML job with local indices only":
- do:
catch: /Cannot create datafeed \[job-with-local-indices\] because user joe lacks permissions on the indices/
headers: { Authorization: "Basic am9lOmpvZS1wYXNzd29yZA==" } # This is joe
ml.put_job:
job_id: job-with-local-indices
body: >
{
"description":"Analysis of response time by airline",
"analysis_config" : {
"detectors" :[{"function":"count"}]
},
"analysis_limits" : {
"model_memory_limit": "20mb"
},
"data_description" : {
"format":"xcontent"
},
"datafeed_config": {
"indexes":["test_index", "test_index-2"]
}
}
---
"Test ML job with local and remote index":
- do:
headers: { Authorization: "Basic am9lOmpvZS1wYXNzd29yZA==" } # This is joe
ml.put_job:
job_id: job-with-local-and-remote-index
body: >
{
"description":"Analysis of response time by airline",
"analysis_config" : {
"detectors" :[{"function":"count"}]
},
"analysis_limits" : {
"model_memory_limit": "20mb"
},
"data_description" : {
"format":"xcontent"
},
"datafeed_config": {
"indexes":["test_index", "my_remote_cluster:remote_test_index"]
}
}
- match: { job_id: "job-with-local-and-remote-index" }
- is_true: datafeed_config
- match: { datafeed_config.job_id: "job-with-local-and-remote-index" }
- match: { datafeed_config.datafeed_id: "job-with-local-and-remote-index" }
- is_true: datafeed_config.authorization.roles
- is_true: create_time

---
"Test ML job with remote index only":
- do:
headers: { Authorization: "Basic am9lOmpvZS1wYXNzd29yZA==" } # This is joe
ml.put_job:
job_id: job-with-remote-index-only
body: >
{
"description":"Analysis of response time by airline",
"analysis_config" : {
"detectors" :[{"function":"count"}]
},
"analysis_limits" : {
"model_memory_limit": "20mb"
},
"data_description" : {
"format":"xcontent"
},
"datafeed_config": {
"indexes":["my_remote_cluster:remote_test_index"]
}
}
- match: { job_id: "job-with-remote-index-only" }
- is_true: datafeed_config
- match: { datafeed_config.job_id: "job-with-remote-index-only" }
- match: { datafeed_config.datafeed_id: "job-with-remote-index-only" }
- is_true: datafeed_config.authorization.roles
- is_true: create_time
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
---
setup:
- skip:
features: headers

- do:
cluster.health:
wait_for_status: yellow
- do:
security.put_user:
username: "joe"
body: >
{
"password": "joe-password",
"roles" : [ "x_cluster_role" ]
}
- do:
security.put_role:
name: "x_cluster_role"
body: >
{
"cluster": [],
"indices": [
{
"names": ["remote_test_index"],
"privileges": ["read", "view_index_metadata"]
}
]
}
---
teardown:
- do:
security.delete_user:
username: "joe"
ignore: 404

---
"Index data on the remote cluster":
- do:
indices.create:
index: remote_test_index
body:
settings:
index:
number_of_shards: 3
number_of_replicas: 0
aliases:
test_alias: {}
mappings:
properties:
time:
type: date
user:
type: keyword
stars:
type: integer
coolness:
type: integer

- do:
bulk:
refresh: true
body:
- '{"index": {"_index": "remote_test_index"}}'
- '{"user": "a", "stars": 1, "date" : "2018-10-29T12:12:12.123456789Z"}'
- '{"index": {"_index": "remote_test_index"}}'
- '{"user": "a", "stars": 4, "date" : "2018-10-29T12:14:12.123456789Z"}'
- '{"index": {"_index": "remote_test_index"}}'
- '{"user": "a", "stars": 5, "date" : "2018-10-29T12:16:12.123456789Z"}'
- '{"index": {"_index": "remote_test_index"}}'
- '{"user": "b", "stars": 2, "date" : "2018-10-29T12:17:12.123456789Z"}'
- '{"index": {"_index": "remote_test_index"}}'
- '{"user": "b", "stars": 3, "date" : "2018-10-29T12:22:12.123456789Z"}'
- '{"index": {"_index": "remote_test_index"}}'
- '{"user": "a", "stars": 5, "date" : "2018-10-29T12:23:12.123456789Z"}'
- '{"index": {"_index": "remote_test_index"}}'
- '{"user": "b", "stars": 1, "date" : "2018-10-29T12:32:12.123456789Z"}'
- '{"index": {"_index": "remote_test_index"}}'
- '{"user": "a", "stars": 3, "date" : "2018-10-29T12:34:12.123456789Z"}'
- '{"index": {"_index": "remote_test_index"}}'
- '{"user": "c", "stars": 4, "date" : "2018-10-29T12:35:12.123456789Z"}'
- do:
headers: { Authorization: "Basic am9lOmpvZS1wYXNzd29yZA==" } # This is joe
search:
rest_total_hits_as_int: true
index: remote_test_index
body:
aggs:
user:
terms:
field: user

- match: { _shards.total: 3 }
- match: { hits.total: 9 }
- length: { aggregations.user.buckets: 3 }
- match: { aggregations.user.buckets.0.key: "a" }
- match: { aggregations.user.buckets.0.doc_count: 5 }
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.license.License;
import org.elasticsearch.license.LicenseUtils;
import org.elasticsearch.license.RemoteClusterLicenseChecker;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.tasks.Task;
import org.elasticsearch.threadpool.ThreadPool;
Expand Down Expand Up @@ -54,10 +55,12 @@

import java.io.IOException;
import java.time.Instant;
import java.util.Arrays;
import java.util.Map;
import java.util.Objects;
import java.util.function.Supplier;

import static java.util.function.Predicate.not;
import static org.elasticsearch.xpack.ml.utils.SecondaryAuthorizationUtils.useSecondaryAuthIfAvailable;

public class TransportPutDataFrameAnalyticsAction extends TransportMasterNodeAction<
Expand Down Expand Up @@ -160,10 +163,12 @@ private void putValidatedConfig(
if (securityContext != null) {
useSecondaryAuthIfAvailable(securityContext, () -> {
final String username = securityContext.getUser().principal();
RoleDescriptor.IndicesPrivileges sourceIndexPrivileges = RoleDescriptor.IndicesPrivileges.builder()
.indices(preparedForPutConfig.getSource().getIndex())
.privileges("read")
.build();
// DFA doesn't support CCS, but if it did it would need this filter, so it's safest to have the filter
// in place even though it's a no-op.
// TODO: Remove this filter once https://github.com/elastic/elasticsearch/issues/67798 is fixed.
final String[] sourceIndices = Arrays.stream(preparedForPutConfig.getSource().getIndex())
.filter(not(RemoteClusterLicenseChecker::isRemoteIndex))
.toArray(String[]::new);
RoleDescriptor.IndicesPrivileges destIndexPrivileges = RoleDescriptor.IndicesPrivileges.builder()
.indices(preparedForPutConfig.getDest().getIndex())
.privileges("read", "index", "create_index")
Expand All @@ -173,7 +178,17 @@ private void putValidatedConfig(
privRequest.applicationPrivileges(new RoleDescriptor.ApplicationResourcePrivileges[0]);
privRequest.username(username);
privRequest.clusterPrivileges(Strings.EMPTY_ARRAY);
privRequest.indexPrivileges(sourceIndexPrivileges, destIndexPrivileges);
RoleDescriptor.IndicesPrivileges[] indicesPrivileges;
if (sourceIndices.length > 0) {
RoleDescriptor.IndicesPrivileges sourceIndexPrivileges = RoleDescriptor.IndicesPrivileges.builder()
.indices(sourceIndices)
.privileges("read")
.build();
indicesPrivileges = new RoleDescriptor.IndicesPrivileges[] { sourceIndexPrivileges, destIndexPrivileges };
} else {
indicesPrivileges = new RoleDescriptor.IndicesPrivileges[] { destIndexPrivileges };
}
privRequest.indexPrivileges(indicesPrivileges);

ActionListener<HasPrivilegesResponse> privResponseListener = ActionListener.wrap(
r -> handlePrivsResponse(username, preparedForPutConfig, r, masterNodeTimeout, listener),
Expand Down
Loading

0 comments on commit 23eafaa

Please sign in to comment.