Skip to content

Commit

Permalink
Validate index format agreement for system index descriptors (#85173) (
Browse files Browse the repository at this point in the history
…#85289)

* Validate index format for system indices
* Add bwc test for watcher meta version issue
* Update docs/changelog/85173.yaml
  • Loading branch information
williamrandolph committed Mar 23, 2022
1 parent 882f789 commit 6563e01
Show file tree
Hide file tree
Showing 8 changed files with 157 additions and 6 deletions.
5 changes: 5 additions & 0 deletions docs/changelog/85173.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 85173
summary: Validate index format agreement for system index descriptors
area: Infra/Core
type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,10 @@ public SystemIndexDescriptor(String indexPattern, String description, Type type,
Strings.requireNonEmpty(primaryIndex, "Must supply primaryIndex for a managed system index");
Strings.requireNonEmpty(versionMetaKey, "Must supply versionMetaKey for a managed system index");
Strings.requireNonEmpty(origin, "Must supply origin for a managed system index");
if (settings.getAsInt(IndexMetadata.INDEX_FORMAT_SETTING.getKey(), 0) != indexFormat) {
throw new IllegalArgumentException("Descriptor index format does not match index format in managed settings");
}
this.mappingVersion = extractVersionFromMappings(mappings, versionMetaKey);
;
} else {
this.mappingVersion = null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,24 @@ public void testSpecialCharactersAreReplacedWhenConvertingToAutomaton() {
assertFalse("the leading dot got dropped", automaton.run("system-index-1"));
}

public void testManagedSystemIndexMustHaveMatchingIndexFormat() {
SystemIndexDescriptor.Builder builder = SystemIndexDescriptor.builder()
.setIndexPattern(".system*")
.setDescription("system stuff")
.setPrimaryIndex(".system-1")
.setAliasName(".system")
.setType(Type.INTERNAL_MANAGED)
.setMappings(MAPPINGS)
.setSettings(Settings.builder().put("index.format", 5).build())
.setIndexFormat(0)
.setVersionMetaKey("version")
.setOrigin("system");

IllegalArgumentException e = expectThrows(IllegalArgumentException.class, builder::build);

assertThat(e.getMessage(), equalTo("Descriptor index format does not match index format in managed settings"));
}

private SystemIndexDescriptor.Builder priorSystemIndexDescriptorBuilder() {
return SystemIndexDescriptor.builder()
.setIndexPattern(".system*")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ public void testManagerSkipsDescriptorsThatAreNotManaged() {
.setPrimaryIndex(".bar-1")
.setMappings(getMappings())
.setSettings(getSettings())
.setIndexFormat(6)
.setVersionMetaKey("version")
.setOrigin("FAKE_ORIGIN")
.build();
Expand Down Expand Up @@ -131,6 +132,7 @@ public void testManagerSkipsDescriptorsForIndicesThatDoNotExist() {
.setPrimaryIndex(".foo-1")
.setMappings(getMappings())
.setSettings(getSettings())
.setIndexFormat(6)
.setVersionMetaKey("version")
.setOrigin("FAKE_ORIGIN")
.build();
Expand All @@ -139,6 +141,7 @@ public void testManagerSkipsDescriptorsForIndicesThatDoNotExist() {
.setPrimaryIndex(".bar-1")
.setMappings(getMappings())
.setSettings(getSettings())
.setIndexFormat(6)
.setVersionMetaKey("version")
.setOrigin("FAKE_ORIGIN")
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import org.apache.lucene.util.automaton.Automaton;
import org.elasticsearch.Version;
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.indices.ExecutorNames;
Expand Down Expand Up @@ -154,12 +155,15 @@ public class TestRestrictedIndices {
RESOLVER = TestIndexNameExpressionResolver.newInstance(systemIndices);
}

private static SystemIndexDescriptor.Builder getInitializedDescriptorBuilder() {
return SystemIndexDescriptor.builder().setMappings(mockMappings()).setSettings(Settings.EMPTY).setVersionMetaKey("version");
private static SystemIndexDescriptor.Builder getInitializedDescriptorBuilder(int indexFormat) {
return SystemIndexDescriptor.builder()
.setMappings(mockMappings())
.setSettings(Settings.builder().put(IndexMetadata.INDEX_FORMAT_SETTING.getKey(), indexFormat).build())
.setVersionMetaKey("version");
}

private static SystemIndexDescriptor getMainSecurityDescriptor() {
return getInitializedDescriptorBuilder()
return getInitializedDescriptorBuilder(7)
// This can't just be `.security-*` because that would overlap with the tokens index pattern
.setIndexPattern(".security-[0-9]+*")
.setPrimaryIndex(RestrictedIndicesNames.INTERNAL_SECURITY_MAIN_INDEX_7)
Expand All @@ -172,7 +176,7 @@ private static SystemIndexDescriptor getMainSecurityDescriptor() {
}

private static SystemIndexDescriptor getSecurityTokensDescriptor() {
return getInitializedDescriptorBuilder().setIndexPattern(".security-tokens-[0-9]+*")
return getInitializedDescriptorBuilder(7).setIndexPattern(".security-tokens-[0-9]+*")
.setPrimaryIndex(RestrictedIndicesNames.INTERNAL_SECURITY_TOKENS_INDEX_7)
.setDescription("Contains auth token data")
.setAliasName(SECURITY_TOKENS_ALIAS)
Expand All @@ -183,9 +187,10 @@ private static SystemIndexDescriptor getSecurityTokensDescriptor() {
}

private static SystemIndexDescriptor getAsyncSearchDescriptor() {
return getInitializedDescriptorBuilder().setIndexPattern(XPackPlugin.ASYNC_RESULTS_INDEX + "*")
return getInitializedDescriptorBuilder(0).setIndexPattern(XPackPlugin.ASYNC_RESULTS_INDEX + "*")
.setDescription("Async search results")
.setPrimaryIndex(XPackPlugin.ASYNC_RESULTS_INDEX)
.setIndexFormat(0)
.setOrigin(ASYNC_SEARCH_ORIGIN)
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -772,6 +772,7 @@ public Collection<SystemIndexDescriptor> getSystemIndexDescriptors(Settings sett
.setSettings(getWatchesIndexSettings())
.setVersionMetaKey("version")
.setOrigin(WATCHER_ORIGIN)
.setIndexFormat(6)
.build(),
SystemIndexDescriptor.builder()
.setIndexPattern(TriggeredWatchStoreField.INDEX_NAME + "*")
Expand All @@ -781,6 +782,7 @@ public Collection<SystemIndexDescriptor> getSystemIndexDescriptors(Settings sett
.setSettings(getTriggeredWatchesIndexSettings())
.setVersionMetaKey("version")
.setOrigin(WATCHER_ORIGIN)
.setIndexFormat(6)
.build()
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,20 @@
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.analysis.AnalysisRegistry;
import org.elasticsearch.index.engine.InternalEngineFactory;
import org.elasticsearch.indices.SystemIndexDescriptor;
import org.elasticsearch.indices.TestIndexNameExpressionResolver;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.IndexSettingsModule;
import org.elasticsearch.threadpool.ExecutorBuilder;
import org.elasticsearch.xpack.core.watcher.watch.Watch;
import org.elasticsearch.xpack.watcher.notification.NotificationService;

import java.util.Collection;
import java.util.Collections;
import java.util.List;

import static java.util.Collections.emptyMap;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.is;
import static org.mockito.Mockito.mock;
Expand Down Expand Up @@ -108,6 +111,16 @@ public void testReloadDisabled() {
verifyNoMoreInteractions(mockService);
}

public void testWatcherSystemIndicesFormat() {
Settings settings = Settings.builder().put("xpack.watcher.enabled", false).put("path.home", createTempDir()).build();
Watcher watcher = new Watcher(settings);

Collection<SystemIndexDescriptor> descriptors = watcher.getSystemIndexDescriptors(settings);
for (SystemIndexDescriptor descriptor : descriptors) {
assertThat(descriptor.getIndexFormat(), equalTo(6));
}
}

private class TestWatcher extends Watcher {

TestWatcher(Settings settings, NotificationService<?> service) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/*
* 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.xpack.restart;

import org.apache.http.util.EntityUtils;
import org.elasticsearch.Version;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.RequestOptions;
import org.elasticsearch.client.Response;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.upgrades.AbstractFullClusterRestartTestCase;

import java.nio.charset.StandardCharsets;
import java.util.Base64;
import java.util.concurrent.TimeUnit;

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.not;

public class WatcherMappingUpdateIT extends AbstractFullClusterRestartTestCase {

@Override
protected Settings restClientSettings() {
String token = "Basic " + Base64.getEncoder().encodeToString("test_user:x-pack-test-password".getBytes(StandardCharsets.UTF_8));
return Settings.builder().put(ThreadContext.PREFIX + ".Authorization", token).build();
}

public void testMappingsAreUpdated() throws Exception {
if (isRunningAgainstOldCluster()) {
// post a watch
Request putWatchRequest = new Request("PUT", "_watcher/watch/log_error_watch");
putWatchRequest.setJsonEntity("""
{
"trigger" : {
"schedule" : { "interval" : "10s" }
},
"input" : {
"search" : {
"request" : {
"indices" : [ "logs" ],
"body" : {
"query" : {
"match" : { "message": "error" }
}
}
}
}
}
}
""");
client().performRequest(putWatchRequest);

if (getOldClusterVersion().onOrAfter(Version.V_7_13_0)) {
assertMappingVersion(".watches", getOldClusterVersion());
} else {
// watches indices from before 7.10 do not have mapping versions in _meta
assertNoMappingVersion(".watches");
}
} else {
assertMappingVersion(".watches", Version.CURRENT);
}
}

private void assertMappingVersion(String index, Version clusterVersion) throws Exception {
assertBusy(() -> {
Request mappingRequest = new Request("GET", index + "/_mappings");
mappingRequest.setOptions(getWarningHandlerOptions(index));
Response response = client().performRequest(mappingRequest);
String responseBody = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8);
assertThat(responseBody, containsString("\"version\":\"" + clusterVersion + "\""));
}, 60L, TimeUnit.SECONDS);
}

private void assertNoMappingVersion(String index) throws Exception {
assertBusy(() -> {
Request mappingRequest = new Request("GET", index + "/_mappings");
if (isRunningAgainstOldCluster() == false || getOldClusterVersion().onOrAfter(Version.V_7_10_0)) {
mappingRequest.setOptions(getWarningHandlerOptions(index));
}
Response response = client().performRequest(mappingRequest);
String responseBody = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8);
assertThat(responseBody, not(containsString("\"version\":\"")));
}, 60L, TimeUnit.SECONDS);
}

private RequestOptions.Builder getWarningHandlerOptions(String index) {
return RequestOptions.DEFAULT.toBuilder()
.setWarningsHandler(w -> w.contains(getWatcherSystemIndexWarning(index)) == false || w.size() != 1);
}

private String getWatcherSystemIndexWarning(String index) {
return "this request accesses system indices: ["
+ index
+ "], but in a future major version, "
+ "direct access to system indices will be prevented by default";
}
}

0 comments on commit 6563e01

Please sign in to comment.