Skip to content

Commit 8d3634a

Browse files
authored
Drop role cache entries when project is deleted (#131908)
The `CompositeRoleStore` caches roles by project id, which includes an invalidation counter per project. This change clears all entries from the cache and drops the invalidation counter when a project is removed from the cluster.
1 parent def9fa6 commit 8d3634a

File tree

7 files changed

+250
-7
lines changed

7 files changed

+250
-7
lines changed
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.cluster.project;
11+
12+
import org.elasticsearch.cluster.ClusterChangedEvent;
13+
import org.elasticsearch.cluster.metadata.ProjectId;
14+
import org.elasticsearch.cluster.service.ClusterService;
15+
16+
import java.util.function.Consumer;
17+
18+
/**
19+
* Utility class to make it easy to run a block of code whenever a project is deleted (e.g. to cleanup cache entries)
20+
*/
21+
public class ProjectDeletedListener {
22+
23+
private final Consumer<ProjectId> consumer;
24+
25+
public ProjectDeletedListener(Consumer<ProjectId> consumer) {
26+
this.consumer = consumer;
27+
}
28+
29+
public void attach(ClusterService clusterService) {
30+
clusterService.addListener(event -> {
31+
final ClusterChangedEvent.ProjectsDelta delta = event.projectDelta();
32+
delta.removed().forEach(consumer);
33+
});
34+
}
35+
36+
}
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.cluster.project;
11+
12+
import org.elasticsearch.cluster.ClusterName;
13+
import org.elasticsearch.cluster.ClusterState;
14+
import org.elasticsearch.cluster.metadata.Metadata;
15+
import org.elasticsearch.cluster.metadata.ProjectId;
16+
import org.elasticsearch.cluster.metadata.ProjectMetadata;
17+
import org.elasticsearch.cluster.routing.GlobalRoutingTableTestHelper;
18+
import org.elasticsearch.cluster.routing.RoutingTable;
19+
import org.elasticsearch.cluster.service.ClusterService;
20+
import org.elasticsearch.common.util.concurrent.DeterministicTaskQueue;
21+
import org.elasticsearch.test.ClusterServiceUtils;
22+
import org.elasticsearch.test.ESTestCase;
23+
24+
import java.util.HashSet;
25+
import java.util.List;
26+
import java.util.Set;
27+
28+
import static org.hamcrest.Matchers.equalTo;
29+
30+
public class ProjectDeletedListenerTests extends ESTestCase {
31+
32+
public void testInvocation() {
33+
final List<ProjectId> existingProjects = randomList(5, 15, ESTestCase::randomUniqueProjectId);
34+
35+
try (ClusterService clusterService = ClusterServiceUtils.createClusterService(new DeterministicTaskQueue().getThreadPool())) {
36+
final ClusterState.Builder csBuilder = ClusterState.builder(ClusterName.DEFAULT);
37+
existingProjects.forEach(p -> csBuilder.putProjectMetadata(ProjectMetadata.builder(p).build()));
38+
final ClusterState cs0 = csBuilder.build();
39+
40+
ClusterServiceUtils.setState(clusterService, cs0);
41+
42+
final Set<ProjectId> notifiedProjects = new HashSet<>();
43+
var pdl = new ProjectDeletedListener(notifiedProjects::add);
44+
pdl.attach(clusterService);
45+
46+
final Set<ProjectId> projectsToDelete = Set.copyOf(
47+
randomSubsetOf(randomIntBetween(1, existingProjects.size() / 2), existingProjects)
48+
);
49+
final List<ProjectId> projectsToCreate = randomList(0, 3, ESTestCase::randomUniqueProjectId);
50+
51+
final var mdBuilder = Metadata.builder(cs0.metadata());
52+
projectsToDelete.forEach(mdBuilder::removeProject);
53+
projectsToCreate.forEach(p -> mdBuilder.put(ProjectMetadata.builder(p).build()));
54+
var md = mdBuilder.build();
55+
var cs1 = ClusterState.builder(cs0)
56+
.metadata(md)
57+
.routingTable(GlobalRoutingTableTestHelper.buildRoutingTable(md, RoutingTable.Builder::addAsNew))
58+
.build();
59+
ClusterServiceUtils.setState(clusterService, cs1);
60+
61+
assertThat(notifiedProjects, equalTo(projectsToDelete));
62+
}
63+
}
64+
65+
}

test/framework/src/main/java/org/elasticsearch/cluster/project/TestProjectResolvers.java

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,14 @@
1919

2020
import java.util.Collection;
2121
import java.util.Objects;
22+
import java.util.function.Supplier;
2223

2324
/**
2425
* An implementation of {@link ProjectResolver} that handles multiple projects for testing purposes. Not usable in production
2526
*/
2627
public final class TestProjectResolvers {
2728

28-
public static final ProjectResolver DEFAULT_PROJECT_ONLY = singleProject(Metadata.DEFAULT_PROJECT_ID, true);
29+
public static final ProjectResolver DEFAULT_PROJECT_ONLY = singleProject(() -> Metadata.DEFAULT_PROJECT_ID, true);
2930

3031
/**
3132
* @return a ProjectResolver that must only be used in a cluster context. It throws in single project related methods.
@@ -131,6 +132,14 @@ public static ProjectResolver alwaysThrow() {
131132
* The ProjectResolver can work with cluster state containing multiple projects and its supportsMultipleProjects returns true.
132133
*/
133134
public static ProjectResolver singleProject(ProjectId projectId) {
135+
return singleProject(() -> projectId, false);
136+
}
137+
138+
/**
139+
* This method returns a ProjectResolver that gives back the specified project-id when its getProjectId method is called.
140+
* The ProjectResolver can work with cluster state containing multiple projects and its supportsMultipleProjects returns true.
141+
*/
142+
public static ProjectResolver singleProject(Supplier<ProjectId> projectId) {
134143
return singleProject(projectId, false);
135144
}
136145

@@ -140,11 +149,11 @@ public static ProjectResolver singleProject(ProjectId projectId) {
140149
* In addition, the ProjectResolvers returns false for supportsMultipleProjects.
141150
*/
142151
public static ProjectResolver singleProjectOnly(ProjectId projectId) {
143-
return singleProject(projectId, true);
152+
return singleProject(() -> projectId, true);
144153
}
145154

146-
private static ProjectResolver singleProject(ProjectId projectId, boolean only) {
147-
Objects.requireNonNull(projectId);
155+
private static ProjectResolver singleProject(Supplier<ProjectId> projectIdSupplier, boolean only) {
156+
Objects.requireNonNull(projectIdSupplier);
148157
return new ProjectResolver() {
149158

150159
@Override
@@ -157,7 +166,7 @@ public ProjectMetadata getProjectMetadata(Metadata metadata) {
157166

158167
@Override
159168
public ProjectId getProjectId() {
160-
return projectId;
169+
return projectIdSupplier.get();
161170
}
162171

163172
@Override
@@ -170,6 +179,7 @@ public Collection<ProjectId> getProjectIds(ClusterState clusterState) {
170179

171180
@Override
172181
public <E extends Exception> void executeOnProject(ProjectId otherProjectId, CheckedRunnable<E> body) throws E {
182+
final ProjectId projectId = projectIdSupplier.get();
173183
if (projectId.equals(otherProjectId)) {
174184
body.run();
175185
} else {

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1037,6 +1037,7 @@ Collection<Object> createComponents(
10371037
);
10381038
final CompositeRolesStore allRolesStore = new CompositeRolesStore(
10391039
settings,
1040+
clusterService,
10401041
roleProviders,
10411042
privilegeStore,
10421043
threadPool.getThreadContext(),

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStore.java

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@
1212
import org.elasticsearch.action.ActionListener;
1313
import org.elasticsearch.action.ActionRunnable;
1414
import org.elasticsearch.cluster.metadata.ProjectId;
15+
import org.elasticsearch.cluster.project.ProjectDeletedListener;
1516
import org.elasticsearch.cluster.project.ProjectResolver;
17+
import org.elasticsearch.cluster.service.ClusterService;
1618
import org.elasticsearch.common.Strings;
1719
import org.elasticsearch.common.bytes.BytesReference;
1820
import org.elasticsearch.common.cache.Cache;
@@ -23,7 +25,6 @@
2325
import org.elasticsearch.common.util.concurrent.ReleasableLock;
2426
import org.elasticsearch.common.util.concurrent.ThreadContext;
2527
import org.elasticsearch.common.util.set.Sets;
26-
import org.elasticsearch.core.FixForMultiProject;
2728
import org.elasticsearch.core.Nullable;
2829
import org.elasticsearch.core.Tuple;
2930
import org.elasticsearch.license.XPackLicenseState;
@@ -113,7 +114,6 @@ public class CompositeRolesStore {
113114
private final DocumentSubsetBitsetCache dlsBitsetCache;
114115
private final AnonymousUser anonymousUser;
115116

116-
@FixForMultiProject(description = "Deleted projects are never cleared from this map")
117117
private final Map<ProjectId, Long> numInvalidation = new ConcurrentHashMap<>();
118118
private final RoleDescriptorStore roleReferenceResolver;
119119
private final Role superuserRole;
@@ -124,6 +124,7 @@ public class CompositeRolesStore {
124124

125125
public CompositeRolesStore(
126126
Settings settings,
127+
ClusterService clusterService,
127128
RoleProviders roleProviders,
128129
NativePrivilegeStore privilegeStore,
129130
ThreadContext threadContext,
@@ -137,6 +138,8 @@ public CompositeRolesStore(
137138
Executor roleBuildingExecutor,
138139
Consumer<Collection<RoleDescriptor>> effectiveRoleDescriptorsConsumer
139140
) {
141+
new ProjectDeletedListener(this::removeProject).attach(clusterService);
142+
140143
this.roleProviders = roleProviders;
141144
roleProviders.addChangeListener(new RoleProviders.ChangeListener() {
142145
@Override
@@ -633,6 +636,12 @@ public void invalidateProject(ProjectId projectId) {
633636
}
634637
}
635638

639+
final void removeProject(ProjectId projectId) {
640+
numInvalidation.remove(projectId);
641+
negativeLookupCacheHelper.removeKeysIf(key -> key.projectId().equals(projectId));
642+
roleCacheHelper.removeKeysIf(key -> key.projectId().equals(projectId));
643+
}
644+
636645
public void invalidateAll() {
637646
numInvalidation.replaceAll((p, num) -> num + 1);
638647
negativeLookupCache.invalidateAll();
@@ -655,6 +664,11 @@ public void invalidateClusterScopedRoles(Set<String> roles) {
655664
negativeLookupCacheHelper.removeKeysIf(key -> roles.contains(key.value()));
656665
}
657666

667+
// for testing
668+
Iterable<ProjectScoped<RoleKey>> cachedRoles() {
669+
return this.roleCache.keys();
670+
}
671+
658672
public void usageStats(ActionListener<Map<String, Object>> listener) {
659673
final Map<String, Object> usage = new HashMap<>();
660674
usage.put("dls", Map.of("bit_set_cache", dlsBitsetCache.usageStats()));

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolverTests.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,7 @@ public void setup() {
247247
rolesStore = Mockito.spy(
248248
new CompositeRolesStore(
249249
settings,
250+
mock(ClusterService.class),
250251
mock(RoleProviders.class),
251252
mock(NativePrivilegeStore.class),
252253
new ThreadContext(settings),

0 commit comments

Comments
 (0)