Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.x] Fix geoip databases index access after system feature migration (#121196) #121953

Merged
merged 4 commits into from
Feb 7, 2025
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 5 additions & 0 deletions docs/changelog/121196.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 121196
summary: Fix geoip databases index access after system feature migration
area: Ingest Node
type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,16 @@ public void testGeoIpSystemFeaturesMigration() throws Exception {

// before the upgrade, Kibana should work
assertBusy(() -> testGetStarAsKibana(List.of("my-index-00001"), List.of()));

// as should a normal get *
assertBusy(() -> testGetStar(List.of("my-index-00001"), List.of()));
} else {
// after the upgrade, but before the migration, Kibana should work
assertBusy(() -> testGetStarAsKibana(List.of("my-index-00001"), maybeSecurityIndex));

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

// migrate the system features and give the cluster a moment to settle
Request migrateSystemFeatures = new Request("POST", "/_migration/system_features");
assertOK(client().performRequest(migrateSystemFeatures));
Expand All @@ -130,9 +136,10 @@ public void testGeoIpSystemFeaturesMigration() throws Exception {
assertBusy(() -> testIndexGeoDoc());

// after the migration, Kibana should work
if (useSecurity == false) { // BUT IT DOESN'T if security is enabled
assertBusy(() -> testGetStarAsKibana(List.of("my-index-00001"), maybeSecurityIndex));
}
assertBusy(() -> testGetStarAsKibana(List.of("my-index-00001"), maybeSecurityIndex));

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

Request disableDownloader = new Request("PUT", "/_cluster/settings");
disableDownloader.setJsonEntity("""
Expand Down Expand Up @@ -211,6 +218,23 @@ private void testIndexGeoDoc() throws IOException {
assertEquals("Sweden", doc.evaluate("_source.geo.country_name"));
}

private void testGetStar(List<String> indexNames, @Nullable List<String> additionalIndexNames) throws IOException {
Request getStar = new Request("GET", "*?expand_wildcards=all");
getStar.setOptions(
RequestOptions.DEFAULT.toBuilder().setWarningsHandler(WarningsHandler.PERMISSIVE) // we don't care about warnings, just errors
);
Response response = client().performRequest(getStar);
assertOK(response);

if (additionalIndexNames != null && additionalIndexNames.isEmpty() == false) {
indexNames = new ArrayList<>(indexNames); // recopy into a mutable list
indexNames.addAll(additionalIndexNames);
}

Map<String, Object> map = responseAsMap(response);
assertThat(map.keySet(), is(new HashSet<>(indexNames)));
}

private void testGetStarAsKibana(List<String> indexNames, @Nullable List<String> additionalIndexNames) throws IOException {
Request getStar = new Request("GET", "*?expand_wildcards=all");
getStar.setOptions(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,13 @@ public static boolean isIndexVisible(
final boolean isHidden = indexAbstraction.isHidden();
boolean isVisible = isHidden == false || indicesOptions.expandWildcardsHidden() || isVisibleDueToImplicitHidden(expression, index);
if (indexAbstraction.getType() == IndexAbstraction.Type.ALIAS) {
if (indexAbstraction.isSystem()) {
// check if it is net new
if (resolver.getNetNewSystemIndexPredicate().test(indexAbstraction.getName())) {
return isSystemIndexVisible(resolver, indexAbstraction);
}
}

// it's an alias, ignore expandWildcardsOpen and expandWildcardsClosed.
// complicated to support those options with aliases pointing to multiple indices...
isVisible = isVisible && indicesOptions.ignoreAliases() == false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,24 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.core.Tuple;
import org.elasticsearch.index.IndexVersion;
import org.elasticsearch.indices.EmptySystemIndices;
import org.elasticsearch.indices.InvalidIndexNameException;
import org.elasticsearch.indices.SystemIndexDescriptor;
import org.elasticsearch.indices.SystemIndices;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xcontent.XContentBuilder;

import java.io.IOException;
import java.io.UncheckedIOException;
import java.util.List;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.function.Supplier;

import static org.elasticsearch.index.mapper.MapperService.SINGLE_MAPPING_NAME;
import static org.elasticsearch.indices.SystemIndices.SYSTEM_INDEX_ACCESS_CONTROL_HEADER_KEY;
import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.either;
Expand Down Expand Up @@ -220,13 +229,80 @@ private boolean isIndexVisible(String index, String selector) {
"*",
selector,
index,
IndicesOptions.strictExpandOpen(),
IndicesOptions.strictExpandHidden(),
metadata,
indexNameExpressionResolver,
true
);
}

public void testIsNetNewSystemIndexVisible() {
final Settings settings = Settings.builder()
.put("index.number_of_replicas", 0)
.put("index.number_of_shards", 1)
.put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current())
.build();

final Settings hiddenSettings = Settings.builder().put(settings).put("index.hidden", true).build();

final IndexMetadata foo = IndexMetadata.builder(".foo").settings(hiddenSettings).system(true).build();
final IndexMetadata barReindexed = IndexMetadata.builder(".bar-reindexed")
.settings(hiddenSettings)
.system(true)
.putAlias(AliasMetadata.builder(".bar").isHidden(true).build())
.build();
final IndexMetadata other = IndexMetadata.builder("other").settings(settings).build();

final SystemIndexDescriptor fooDescriptor = SystemIndexDescriptor.builder()
.setDescription("foo indices")
.setOrigin("foo origin")
.setVersionMetaKey("version")
.setPrimaryIndex(".foo")
.setIndexPattern(".foo*")
.setSettings(settings)
.setMappings(mappings())
.setNetNew()
.build();
final SystemIndexDescriptor barDescriptor = SystemIndexDescriptor.builder()
.setDescription("bar indices")
.setOrigin("bar origin")
.setVersionMetaKey("version")
.setPrimaryIndex(".bar")
.setIndexPattern(".bar*")
.setSettings(settings)
.setMappings(mappings())
.setNetNew()
.build();
final SystemIndices systemIndices = new SystemIndices(
List.of(new SystemIndices.Feature("name", "description", List.of(fooDescriptor, barDescriptor)))
);

final ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
threadContext.putHeader(SYSTEM_INDEX_ACCESS_CONTROL_HEADER_KEY, "false");
indexNameExpressionResolver = new IndexNameExpressionResolver(threadContext, systemIndices);
indexAbstractionResolver = new IndexAbstractionResolver(indexNameExpressionResolver);

metadata = Metadata.builder().put(foo, true).put(barReindexed, true).put(other, true).build();

assertThat(isIndexVisible("other", "*"), is(true));
assertThat(isIndexVisible(".foo", "*"), is(false));
assertThat(isIndexVisible(".bar", "*"), is(false));
}

private static XContentBuilder mappings() {
try (XContentBuilder builder = jsonBuilder()) {
return builder.startObject()
.startObject(SINGLE_MAPPING_NAME)
.startObject("_meta")
.field(SystemIndexDescriptor.VERSION_META_KEY, 0)
.endObject()
.endObject()
.endObject();
} catch (IOException e) {
throw new UncheckedIOException(e);
}
}

private List<String> resolveAbstractionsSelectorNotAllowed(List<String> expressions) {
return resolveAbstractions(expressions, IndicesOptions.strictExpandHiddenNoSelectors(), defaultMask);
}
Expand Down