Skip to content

Commit a895875

Browse files
joegallojoegallo
authored
Fix geoip databases index access after system feature migration (again) (#122938)
1 parent 274be79 commit a895875

File tree

4 files changed

+118
-21
lines changed

4 files changed

+118
-21
lines changed

‎docs/changelog/122938.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 122938
2+
summary: Fix geoip databases index access after system feature migration (again)
3+
area: Ingest Node
4+
type: bug
5+
issues: []

‎modules/ingest-geoip/qa/full-cluster-restart/src/javaRestTest/java/org/elasticsearch/ingest/geoip/FullClusterRestartIT.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,13 +123,19 @@ public void testGeoIpSystemFeaturesMigration() throws Exception {
123123

124124
// as should a normal get *
125125
assertBusy(() -> testGetStar(List.of("my-index-00001"), maybeSecurityIndex));
126+
127+
// and getting data streams
128+
assertBusy(() -> testGetDatastreams());
126129
} else {
127130
// after the upgrade, but before the migration, Kibana should work
128131
assertBusy(() -> testGetStarAsKibana(List.of("my-index-00001"), maybeSecurityIndex));
129132

130133
// as should a normal get *
131134
assertBusy(() -> testGetStar(List.of("my-index-00001"), maybeSecurityIndex));
132135

136+
// and getting data streams
137+
assertBusy(() -> testGetDatastreams());
138+
133139
// migrate the system features and give the cluster a moment to settle
134140
Request migrateSystemFeatures = new Request("POST", "/_migration/system_features");
135141
assertOK(client().performRequest(migrateSystemFeatures));
@@ -144,6 +150,9 @@ public void testGeoIpSystemFeaturesMigration() throws Exception {
144150
// as should a normal get *
145151
assertBusy(() -> testGetStar(List.of("my-index-00001"), maybeSecurityIndexReindexed));
146152

153+
// and getting data streams
154+
assertBusy(() -> testGetDatastreams());
155+
147156
Request disableDownloader = new Request("PUT", "/_cluster/settings");
148157
disableDownloader.setJsonEntity("""
149158
{"persistent": {"ingest.geoip.downloader.enabled": false}}
@@ -257,4 +266,15 @@ private void testGetStarAsKibana(List<String> indexNames, @Nullable List<String>
257266
Map<String, Object> map = responseAsMap(response);
258267
assertThat(map.keySet(), is(new HashSet<>(indexNames)));
259268
}
269+
270+
private void testGetDatastreams() throws IOException {
271+
Request getStar = new Request("GET", "_data_stream");
272+
getStar.setOptions(
273+
RequestOptions.DEFAULT.toBuilder().setWarningsHandler(WarningsHandler.PERMISSIVE) // we don't care about warnings, just errors
274+
);
275+
Response response = client().performRequest(getStar);
276+
assertOK(response);
277+
278+
// note: we don't actually care about the response, just that there was one and that it didn't error out on us
279+
}
260280
}

‎server/src/main/java/org/elasticsearch/cluster/metadata/IndexAbstractionResolver.java‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.elasticsearch.common.regex.Regex;
1515
import org.elasticsearch.core.Nullable;
1616
import org.elasticsearch.core.Tuple;
17+
import org.elasticsearch.index.Index;
1718
import org.elasticsearch.index.IndexNotFoundException;
1819
import org.elasticsearch.indices.SystemIndices.SystemIndexAccessLevel;
1920

@@ -158,7 +159,26 @@ public static boolean isIndexVisible(
158159
if (indexAbstraction.isSystem()) {
159160
// check if it is net new
160161
if (resolver.getNetNewSystemIndexPredicate().test(indexAbstraction.getName())) {
161-
return isSystemIndexVisible(resolver, indexAbstraction);
162+
// don't give this code any particular credit for being *correct*. it's just trying to resolve a combination of
163+
// issues in a way that happens to *work*. there's probably a better way of writing things such that this won't
164+
// be necessary, but for the moment, it happens to be expedient to write things this way.
165+
166+
// unwrap the alias and re-run the function on the write index of the alias -- that is, the alias is visible if
167+
// the concrete index that it refers to is visible
168+
Index writeIndex = indexAbstraction.getWriteIndex();
169+
if (writeIndex == null) {
170+
return false;
171+
} else {
172+
return isIndexVisible(
173+
expression,
174+
selectorString,
175+
writeIndex.getName(),
176+
indicesOptions,
177+
metadata,
178+
resolver,
179+
includeDataStreams
180+
);
181+
}
162182
}
163183
}
164184

‎server/src/test/java/org/elasticsearch/cluster/metadata/IndexAbstractionResolverTests.java‎

Lines changed: 72 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import java.util.function.Supplier;
3030

3131
import static org.elasticsearch.index.mapper.MapperService.SINGLE_MAPPING_NAME;
32+
import static org.elasticsearch.indices.SystemIndices.EXTERNAL_SYSTEM_INDEX_ACCESS_CONTROL_HEADER_KEY;
3233
import static org.elasticsearch.indices.SystemIndices.SYSTEM_INDEX_ACCESS_CONTROL_HEADER_KEY;
3334
import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder;
3435
import static org.hamcrest.Matchers.contains;
@@ -218,18 +219,6 @@ public void testIsIndexVisible() {
218219
assertThat(isIndexVisible("data-stream1", "failures"), is(true));
219220
}
220221

221-
private boolean isIndexVisible(String index, String selector) {
222-
return IndexAbstractionResolver.isIndexVisible(
223-
"*",
224-
selector,
225-
index,
226-
IndicesOptions.strictExpandHidden(),
227-
metadata,
228-
indexNameExpressionResolver,
229-
true
230-
);
231-
}
232-
233222
public void testIsNetNewSystemIndexVisible() {
234223
final Settings settings = Settings.builder()
235224
.put("index.number_of_replicas", 0)
@@ -269,16 +258,71 @@ public void testIsNetNewSystemIndexVisible() {
269258
List.of(new SystemIndices.Feature("name", "description", List.of(fooDescriptor, barDescriptor)))
270259
);
271260

272-
final ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
273-
threadContext.putHeader(SYSTEM_INDEX_ACCESS_CONTROL_HEADER_KEY, "false");
274-
indexNameExpressionResolver = new IndexNameExpressionResolver(threadContext, systemIndices);
275-
indexAbstractionResolver = new IndexAbstractionResolver(indexNameExpressionResolver);
276-
277261
metadata = Metadata.builder().put(foo, true).put(barReindexed, true).put(other, true).build();
278262

279-
assertThat(isIndexVisible("other", "*"), is(true));
280-
assertThat(isIndexVisible(".foo", "*"), is(false));
281-
assertThat(isIndexVisible(".bar", "*"), is(false));
263+
// these indices options are for the GET _data_streams case
264+
final IndicesOptions noHiddenNoAliases = IndicesOptions.builder()
265+
.wildcardOptions(
266+
IndicesOptions.WildcardOptions.builder()
267+
.matchOpen(true)
268+
.matchClosed(true)
269+
.includeHidden(false)
270+
.resolveAliases(false)
271+
.build()
272+
)
273+
.build();
274+
275+
{
276+
final ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
277+
threadContext.putHeader(SYSTEM_INDEX_ACCESS_CONTROL_HEADER_KEY, "true");
278+
indexNameExpressionResolver = new IndexNameExpressionResolver(threadContext, systemIndices);
279+
indexAbstractionResolver = new IndexAbstractionResolver(indexNameExpressionResolver);
280+
281+
// this covers the GET * case -- with system access, you can see everything
282+
assertThat(isIndexVisible("other", "*"), is(true));
283+
assertThat(isIndexVisible(".foo", "*"), is(true));
284+
assertThat(isIndexVisible(".bar", "*"), is(true));
285+
286+
// but if you don't ask for hidden and aliases, you won't see hidden indices or aliases, naturally
287+
assertThat(isIndexVisible("other", "*", noHiddenNoAliases), is(true));
288+
assertThat(isIndexVisible(".foo", "*", noHiddenNoAliases), is(false));
289+
assertThat(isIndexVisible(".bar", "*", noHiddenNoAliases), is(false));
290+
}
291+
292+
{
293+
final ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
294+
threadContext.putHeader(SYSTEM_INDEX_ACCESS_CONTROL_HEADER_KEY, "false");
295+
indexNameExpressionResolver = new IndexNameExpressionResolver(threadContext, systemIndices);
296+
indexAbstractionResolver = new IndexAbstractionResolver(indexNameExpressionResolver);
297+
298+
// this covers the GET * case -- without system access, you can't see everything
299+
assertThat(isIndexVisible("other", "*"), is(true));
300+
assertThat(isIndexVisible(".foo", "*"), is(false));
301+
assertThat(isIndexVisible(".bar", "*"), is(false));
302+
303+
// no difference here in the datastream case, you can't see these then, either
304+
assertThat(isIndexVisible("other", "*", noHiddenNoAliases), is(true));
305+
assertThat(isIndexVisible(".foo", "*", noHiddenNoAliases), is(false));
306+
assertThat(isIndexVisible(".bar", "*", noHiddenNoAliases), is(false));
307+
}
308+
309+
{
310+
final ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
311+
threadContext.putHeader(SYSTEM_INDEX_ACCESS_CONTROL_HEADER_KEY, "true");
312+
threadContext.putHeader(EXTERNAL_SYSTEM_INDEX_ACCESS_CONTROL_HEADER_KEY, "some-elastic-product");
313+
indexNameExpressionResolver = new IndexNameExpressionResolver(threadContext, systemIndices);
314+
indexAbstractionResolver = new IndexAbstractionResolver(indexNameExpressionResolver);
315+
316+
// this covers the GET * case -- with product (only) access, you can't see everything
317+
assertThat(isIndexVisible("other", "*"), is(true));
318+
assertThat(isIndexVisible(".foo", "*"), is(false));
319+
assertThat(isIndexVisible(".bar", "*"), is(false));
320+
321+
// no difference here in the datastream case, you can't see these then, either
322+
assertThat(isIndexVisible("other", "*", noHiddenNoAliases), is(true));
323+
assertThat(isIndexVisible(".foo", "*", noHiddenNoAliases), is(false));
324+
assertThat(isIndexVisible(".bar", "*", noHiddenNoAliases), is(false));
325+
}
282326
}
283327

284328
private static XContentBuilder mappings() {
@@ -306,4 +350,12 @@ private List<String> resolveAbstractionsSelectorAllowed(List<String> expressions
306350
private List<String> resolveAbstractions(List<String> expressions, IndicesOptions indicesOptions, Supplier<Set<String>> mask) {
307351
return indexAbstractionResolver.resolveIndexAbstractions(expressions, indicesOptions, metadata, mask, (idx) -> true, true);
308352
}
353+
354+
private boolean isIndexVisible(String index, String selector) {
355+
return isIndexVisible(index, selector, IndicesOptions.strictExpandHidden());
356+
}
357+
358+
private boolean isIndexVisible(String index, String selector, IndicesOptions indicesOptions) {
359+
return IndexAbstractionResolver.isIndexVisible("*", selector, index, indicesOptions, metadata, indexNameExpressionResolver, true);
360+
}
309361
}

0 commit comments

Comments
 (0)