From 6fab4680a2b8d1dd31cb2efa5711310cec881441 Mon Sep 17 00:00:00 2001 From: jaymode Date: Fri, 25 Mar 2016 07:24:26 -0400 Subject: [PATCH 1/4] security: roles store poller should only update existing entries Original commit: elastic/x-pack-elasticsearch@6573f4d6896caff284af23f52172d3810db4485f --- .../shield/authz/esnative/ESNativeRolesStore.java | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/elasticsearch/x-pack/shield/src/main/java/org/elasticsearch/shield/authz/esnative/ESNativeRolesStore.java b/elasticsearch/x-pack/shield/src/main/java/org/elasticsearch/shield/authz/esnative/ESNativeRolesStore.java index 4a024198bc3..9aa527fa4e5 100644 --- a/elasticsearch/x-pack/shield/src/main/java/org/elasticsearch/shield/authz/esnative/ESNativeRolesStore.java +++ b/elasticsearch/x-pack/shield/src/main/java/org/elasticsearch/shield/authz/esnative/ESNativeRolesStore.java @@ -514,23 +514,12 @@ public class ESNativeRolesStore extends AbstractComponent implements RolesStore, for (SearchHit hit : response.getHits().getHits()) { final String roleName = hit.getId(); final long version = hit.version(); - final boolean existed = existingRoles.remove(roleName); + existingRoles.remove(roleName); // we use the locking mechanisms provided by the map/cache to help protect against concurrent operations // that will leave the cache in a bad state - roleCache.compute(roleName, new BiFunction() { + roleCache.computeIfPresent(roleName, new BiFunction() { @Override public RoleAndVersion apply(String roleName, RoleAndVersion existing) { - if (existing == null) { - if (existed) { - // the cache doesn't have this role anymore, it got cleared by something else, do nothing. - return null; - } else { - // it is new, we can cache it - RoleDescriptor rd = transformRole(hit.getId(), hit.getSourceRef()); - return new RoleAndVersion(rd, version); - } - } - if (version > existing.getVersion()) { RoleDescriptor rd = transformRole(hit.getId(), hit.getSourceRef()); if (rd != null) { From a3807b078da8b3c4bedd16d3af699c944be39455 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Fri, 25 Mar 2016 15:40:06 +0100 Subject: [PATCH 2/4] Monitoring: Add more thread pool stats This commit adds stats for generic/get/management/watcher thread pools. Related to elastic/elasticsearch#1750 Original commit: elastic/x-pack-elasticsearch@8b001b50c6476c6122505297bca1360a0239c90a --- .../resolver/node/NodeStatsResolver.java | 22 +++++++++++++++++-- .../resolver/node/NodeStatsResolverTests.java | 10 +++++++-- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/elasticsearch/x-pack/marvel/src/main/java/org/elasticsearch/marvel/agent/resolver/node/NodeStatsResolver.java b/elasticsearch/x-pack/marvel/src/main/java/org/elasticsearch/marvel/agent/resolver/node/NodeStatsResolver.java index 685b7bee03a..ca88b019f74 100644 --- a/elasticsearch/x-pack/marvel/src/main/java/org/elasticsearch/marvel/agent/resolver/node/NodeStatsResolver.java +++ b/elasticsearch/x-pack/marvel/src/main/java/org/elasticsearch/marvel/agent/resolver/node/NodeStatsResolver.java @@ -56,9 +56,27 @@ public class NodeStatsResolver extends MonitoringIndexNameResolver.Timestamped> statsByShard = new HashMap<>(); statsByShard.put(index, Collections.singletonList(new IndexShardStats(shardId, new ShardStats[]{shardStats}))); - List threadPoolStats = Arrays.asList(new ThreadPoolStats.Stats(ThreadPool.Names.INDEX, 0, 0, 0, 0, 0, 0), + List threadPoolStats = Arrays.asList( new ThreadPoolStats.Stats(ThreadPool.Names.BULK, 0, 0, 0, 0, 0, 0), - new ThreadPoolStats.Stats(ThreadPool.Names.SEARCH, 0, 0, 0, 0, 0, 0) + new ThreadPoolStats.Stats(ThreadPool.Names.GENERIC, 0, 0, 0, 0, 0, 0), + new ThreadPoolStats.Stats(ThreadPool.Names.GET, 0, 0, 0, 0, 0, 0), + new ThreadPoolStats.Stats(ThreadPool.Names.INDEX, 0, 0, 0, 0, 0, 0), + new ThreadPoolStats.Stats(ThreadPool.Names.MANAGEMENT, 0, 0, 0, 0, 0, 0), + new ThreadPoolStats.Stats(ThreadPool.Names.SEARCH, 0, 0, 0, 0, 0, 0), + new ThreadPoolStats.Stats(InternalWatchExecutor.THREAD_POOL_NAME, 0, 0, 0, 0, 0, 0) ); return new NodeStats(new DiscoveryNode("node_0", DummyTransportAddress.INSTANCE, Version.CURRENT), 0, new NodeIndicesStats(new CommonStats(), statsByShard), OsProbe.getInstance().osStats(), From 1bf3a93e4fa8e7555ec639f82b9d9253dcc3189a Mon Sep 17 00:00:00 2001 From: jaymode Date: Fri, 25 Mar 2016 10:43:30 -0400 Subject: [PATCH 3/4] test: fix IndexPrivilegeTests after removal of predefined privileges Original commit: elastic/x-pack-elasticsearch@6b913449b347919ac6e93bf1cd26858690003bfe --- .../integration/IndexPrivilegeTests.java | 67 +++++++------------ 1 file changed, 23 insertions(+), 44 deletions(-) diff --git a/elasticsearch/x-pack/shield/src/test/java/org/elasticsearch/integration/IndexPrivilegeTests.java b/elasticsearch/x-pack/shield/src/test/java/org/elasticsearch/integration/IndexPrivilegeTests.java index dde8de82277..8dd3ccbf4e5 100644 --- a/elasticsearch/x-pack/shield/src/test/java/org/elasticsearch/integration/IndexPrivilegeTests.java +++ b/elasticsearch/x-pack/shield/src/test/java/org/elasticsearch/integration/IndexPrivilegeTests.java @@ -22,10 +22,10 @@ import static org.hamcrest.Matchers.is; //test is just too slow, please fix it to not be sleep-based @BadApple(bugUrl = "https://github.com/elastic/x-plugins/issues/1007") -@ESIntegTestCase.ClusterScope(randomDynamicTemplates = false) +@ESIntegTestCase.ClusterScope(randomDynamicTemplates = false, maxNumDataNodes = 2) public class IndexPrivilegeTests extends AbstractPrivilegeTestCase { - private String jsonDoc = "{ \"name\" : \"elasticsearch\"}"; + private String jsonDoc = "{ \"name\" : \"elasticsearch\", \"body\": \"foo bar\" }"; public static final String ROLES = "all_cluster_role:\n" + @@ -42,6 +42,10 @@ public class IndexPrivilegeTests extends AbstractPrivilegeTestCase { " indices:\n" + " - names: 'a'\n" + " privileges: [ read ]\n" + + "read_b_role:\n" + + " indices:\n" + + " - names: 'b'\n" + + " privileges: [ read ]\n" + "write_a_role:\n" + " indices:\n" + " - names: 'a'\n" + @@ -50,14 +54,6 @@ public class IndexPrivilegeTests extends AbstractPrivilegeTestCase { " indices:\n" + " - names: [ 'a', 'b' ]\n" + " privileges: [ read ]\n" + - "get_b_role:\n" + - " indices:\n" + - " - names: 'b'\n" + - " privileges: [ get ]\n" + - "search_b_role:\n" + - " indices:\n" + - " - names: 'b'\n" + - " privileges: [ search ]\n" + "all_regex_ab_role:\n" + " indices:\n" + " - names: '/a|b/'\n" + @@ -66,10 +62,10 @@ public class IndexPrivilegeTests extends AbstractPrivilegeTestCase { " indices:\n" + " - names: 'a*'\n" + " privileges: [ manage ]\n" + - "data_access_all_role:\n" + + "read_write_all_role:\n" + " indices:\n" + " - names: '*'\n" + - " privileges: [ data_access ]\n" + + " privileges: [ read, write ]\n" + "create_c_role:\n" + " indices:\n" + " - names: 'c'\n" + @@ -78,10 +74,10 @@ public class IndexPrivilegeTests extends AbstractPrivilegeTestCase { " indices:\n" + " - names: 'b'\n" + " privileges: [ monitor ]\n" + - "crud_a_role:\n" + + "read_write_a_role:\n" + " indices:\n" + " - names: 'a'\n" + - " privileges: [ crud ]\n" + + " privileges: [ read, write ]\n" + "delete_b_role:\n" + " indices:\n" + " - names: 'b'\n" + @@ -90,10 +86,6 @@ public class IndexPrivilegeTests extends AbstractPrivilegeTestCase { " indices:\n" + " - names: 'a'\n" + " privileges: [ index ]\n" + - "search_a_role:\n" + - " indices:\n" + - " - names: 'a'\n" + - " privileges: [ search ]\n" + "\n"; public static final String USERS = @@ -107,7 +99,6 @@ public class IndexPrivilegeTests extends AbstractPrivilegeTestCase { "u7:" + USERS_PASSWD_HASHED + "\n"+ "u8:" + USERS_PASSWD_HASHED + "\n"+ "u9:" + USERS_PASSWD_HASHED + "\n" + - "u10:" + USERS_PASSWD_HASHED + "\n" + "u11:" + USERS_PASSWD_HASHED + "\n" + "u12:" + USERS_PASSWD_HASHED + "\n" + "u13:" + USERS_PASSWD_HASHED + "\n" + @@ -118,19 +109,17 @@ public class IndexPrivilegeTests extends AbstractPrivilegeTestCase { "all_cluster_role:admin\n" + "all_a_role:u1,u2,u6\n" + "read_a_role:u1,u5,u14\n" + + "read_b_role:u3,u5,u6,u8,u13\n" + "write_a_role:u9\n" + "read_ab_role:u2,u4,u9\n" + - "get_b_role:u3,u5,u8,u10\n" + - "search_b_role:u6,u10,u13\n" + "all_regex_ab_role:u3\n" + - "manage_starts_with_a_role:u4,u15\n" + - "data_access_all_role:u12\n" + + "manage_starts_with_a_role:u4\n" + + "read_write_all_role:u12\n" + "create_c_role:u11\n" + "monitor_b_role:u14\n" + - "crud_a_role:u12\n" + + "read_write_a_role:u12\n" + "delete_b_role:u11\n" + - "index_a_role:u13\n" + - "search_a_role:u15\n" ; + "index_a_role:u13\n"; @Override protected Settings nodeSettings(int nodeOrdinal) { @@ -193,7 +182,7 @@ public class IndexPrivilegeTests extends AbstractPrivilegeTestCase { } public void testUserU3() throws Exception { - // u3 has get b role, but all access to a* and b* via regex + // u3 has read b role, but all access to a* and b* via regex assertUserIsAllowed("u3", "all", "a"); assertUserIsAllowed("u3", "all", "b"); assertUserIsDenied("u3", "all", "c"); @@ -216,21 +205,20 @@ public class IndexPrivilegeTests extends AbstractPrivilegeTestCase { } public void testUserU5() throws Exception { - // u5 may read a and get b + // u5 may read a and read b assertUserIsAllowed("u5", "read", "a"); assertUserIsDenied("u5", "manage", "a"); assertUserIsDenied("u5", "write", "a"); - assertUserIsAllowed("u5", "get", "b"); + assertUserIsAllowed("u5", "read", "b"); assertUserIsDenied("u5", "manage", "b"); assertUserIsDenied("u5", "write", "b"); - assertAccessIsDenied("u5", "GET", "/b/_search"); } public void testUserU6() throws Exception { - // u6 has all access on a and search access on b + // u6 has all access on a and read access on b assertUserIsAllowed("u6", "all", "a"); - assertUserIsAllowed("u6", "search", "b"); + assertUserIsAllowed("u6", "read", "b"); assertUserIsDenied("u6", "manage", "b"); assertUserIsDenied("u6", "write", "b"); assertUserIsDenied("u6", "all", "c"); @@ -244,7 +232,7 @@ public class IndexPrivilegeTests extends AbstractPrivilegeTestCase { } public void testUserU8() throws Exception { - // u8 has admin access and get access on b + // u8 has admin access and read access on b assertUserIsAllowed("u8", "all", "a"); assertUserIsAllowed("u8", "all", "b"); assertUserIsAllowed("u8", "all", "c"); @@ -260,15 +248,6 @@ public class IndexPrivilegeTests extends AbstractPrivilegeTestCase { assertUserIsDenied("u9", "all", "c"); } - public void testUserU10() throws Exception { - // u10 has access on get/search on b - assertUserIsDenied("u10", "all", "a"); - assertUserIsDenied("u10", "manage", "b"); - assertUserIsDenied("u10", "write", "b"); - assertUserIsAllowed("u10", "search", "b"); - assertUserIsDenied("u10", "all", "c"); - } - public void testUserU11() throws Exception { // u11 has access to create c and delete b assertUserIsDenied("u11", "all", "a"); @@ -295,7 +274,7 @@ public class IndexPrivilegeTests extends AbstractPrivilegeTestCase { } public void testUserU13() throws Exception { - // u13 has search access on b and index access on a + // u13 has read access on b and index access on a assertUserIsDenied("u13", "manage", "a"); assertUserIsAllowed("u13", "index", "a"); assertUserIsDenied("u13", "delete", "a"); @@ -303,7 +282,7 @@ public class IndexPrivilegeTests extends AbstractPrivilegeTestCase { assertUserIsDenied("u13", "manage", "b"); assertUserIsDenied("u13", "write", "b"); - assertUserIsAllowed("u13", "search", "b"); + assertUserIsAllowed("u13", "read", "b"); assertUserIsDenied("u13", "all", "c"); } From 2397158d20af14045e4cec8f85c5c9450a120285 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Fri, 25 Mar 2016 16:40:38 +0100 Subject: [PATCH 4/4] Fix ClusterStateTests Original commit: elastic/x-pack-elasticsearch@7bca8abe67ee92524bfb712bf9c7bf63ae557859 --- .../resolver/cluster/ClusterStateTests.java | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/elasticsearch/x-pack/marvel/src/test/java/org/elasticsearch/marvel/agent/resolver/cluster/ClusterStateTests.java b/elasticsearch/x-pack/marvel/src/test/java/org/elasticsearch/marvel/agent/resolver/cluster/ClusterStateTests.java index fb88e3c6c9c..e0be9b86cbe 100644 --- a/elasticsearch/x-pack/marvel/src/test/java/org/elasticsearch/marvel/agent/resolver/cluster/ClusterStateTests.java +++ b/elasticsearch/x-pack/marvel/src/test/java/org/elasticsearch/marvel/agent/resolver/cluster/ClusterStateTests.java @@ -5,7 +5,7 @@ */ package org.elasticsearch.marvel.agent.resolver.cluster; -import org.apache.lucene.util.LuceneTestCase.BadApple; +import org.apache.lucene.util.LuceneTestCase; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.common.settings.Settings; @@ -24,6 +24,7 @@ import java.util.Map; import java.util.concurrent.TimeUnit; import static org.elasticsearch.index.query.QueryBuilders.matchQuery; +import static org.elasticsearch.marvel.MonitoredSystem.ES; import static org.elasticsearch.marvel.agent.exporter.MarvelTemplateUtils.TEMPLATE_VERSION; import static org.elasticsearch.marvel.agent.resolver.MonitoringIndexNameResolver.Fields.SOURCE_NODE; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; @@ -32,7 +33,7 @@ import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.core.Is.is; //test is just too slow, please fix it to not be sleep-based -@BadApple(bugUrl = "https://github.com/elastic/x-plugins/issues/1007") +@LuceneTestCase.BadApple(bugUrl = "https://github.com/elastic/x-plugins/issues/1007") @ClusterScope(scope = Scope.TEST) public class ClusterStateTests extends MarvelIntegTestCase { @@ -105,11 +106,17 @@ public class ClusterStateTests extends MarvelIntegTestCase { public void testClusterStateNodes() throws Exception { final long nbNodes = internalCluster().size(); + MonitoringIndexNameResolver.Timestamped timestampedResolver = new MockTimestampedIndexNameResolver(ES, TEMPLATE_VERSION); + assertNotNull(timestampedResolver); + + String timestampedIndex = timestampedResolver.indexPattern(); + awaitIndexExists(timestampedIndex); + logger.debug("--> waiting for documents to be collected"); awaitMarvelDocsCount(greaterThanOrEqualTo(nbNodes), ClusterStateNodeResolver.TYPE); logger.debug("--> searching for monitoring documents of type [{}]", ClusterStateNodeResolver.TYPE); - SearchResponse response = client().prepareSearch().setTypes(ClusterStateNodeResolver.TYPE).get(); + SearchResponse response = client().prepareSearch(timestampedIndex).setTypes(ClusterStateNodeResolver.TYPE).get(); assertThat(response.getHits().getTotalHits(), greaterThanOrEqualTo(nbNodes)); logger.debug("--> checking that every document contains the expected fields"); @@ -133,10 +140,10 @@ public class ClusterStateTests extends MarvelIntegTestCase { logger.debug("--> check that node attributes are indexed"); assertThat(client().prepareSearch().setSize(0) - .setIndices(MONITORING_INDICES_PREFIX + TEMPLATE_VERSION + "-*") + .setIndices(timestampedIndex) .setTypes(ClusterStateNodeResolver.TYPE) - .setQuery(QueryBuilders.matchQuery(SOURCE_NODE.underscore().toString() + ".attributes.custom", randomInt) - ).get().getHits().getTotalHits(), greaterThan(0L)); + .setQuery(QueryBuilders.matchQuery(SOURCE_NODE.underscore().toString() + ".attributes.custom", randomInt)) + .get().getHits().getTotalHits(), greaterThan(0L)); logger.debug("--> cluster state nodes successfully collected"); } @@ -144,7 +151,7 @@ public class ClusterStateTests extends MarvelIntegTestCase { public void testDiscoveryNodes() throws Exception { final long nbNodes = internalCluster().size(); - MonitoringIndexNameResolver.Data dataResolver = internalCluster().getInstance(MonitoringIndexNameResolver.Data.class); + MonitoringIndexNameResolver.Data dataResolver = new MockDataIndexNameResolver(TEMPLATE_VERSION); assertNotNull(dataResolver); String dataIndex = dataResolver.indexPattern(); @@ -154,7 +161,7 @@ public class ClusterStateTests extends MarvelIntegTestCase { awaitMarvelDocsCount(greaterThanOrEqualTo(nbNodes), DiscoveryNodeResolver.TYPE); logger.debug("--> searching for monitoring documents of type [{}]", DiscoveryNodeResolver.TYPE); - SearchResponse response = client().prepareSearch().setTypes(DiscoveryNodeResolver.TYPE).get(); + SearchResponse response = client().prepareSearch(dataIndex).setTypes(DiscoveryNodeResolver.TYPE).get(); assertThat(response.getHits().getTotalHits(), greaterThanOrEqualTo(nbNodes)); logger.debug("--> checking that every document contains the expected fields"); @@ -188,7 +195,7 @@ public class ClusterStateTests extends MarvelIntegTestCase { assertThat(client().prepareGet(dataIndex, DiscoveryNodeResolver.TYPE, nodeId).get().isExists(), is(true)); // checks that document is not indexed - assertHitCount(client().prepareSearch().setSize(0) + assertHitCount(client().prepareSearch(dataIndex).setSize(0) .setTypes(DiscoveryNodeResolver.TYPE) .setQuery(QueryBuilders.boolQuery() .should(matchQuery("node.id", nodeId))