Skip to content

Commit

Permalink
Introduce private settings (#33327)
Browse files Browse the repository at this point in the history
This commit introduces the formal notion of a private setting. This
enables us to register some settings that we had previously not
registered as fully-fledged settings to avoid them being exposed via
APIs such as the create index API. For example, we had hacks in the
codebase to allow index.version.created to be passed around inside of
settings objects, but was not registered as a setting so that if a user
tried to use the setting on any API then they would get an
exception. This prevented users from setting index.version.created on
index creation, or updating it via the index settings API. By
introducing private settings, we can continue to reject these attempts,
yet now we can represent these settings as actual settings. In this
change, we register index.version.created as an actual setting. We do
not cutover all settings that we had been treating as private in this
pull request, it is already quite large due to moving some tests around
to account for the fact that some tests need to be able to set the
index.version.created. This can be done in a follow-up change.
  • Loading branch information
jasontedor authored Sep 3, 2018
1 parent 79db16f commit 09bf4e5
Show file tree
Hide file tree
Showing 60 changed files with 1,408 additions and 734 deletions.
1 change: 0 additions & 1 deletion buildSrc/src/main/resources/checkstyle_suppressions.xml
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,6 @@
<suppress files="server[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]mapper[/\\]DocumentMapperMergeTests.java" checks="LineLength" />
<suppress files="server[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]mapper[/\\]DocumentParserTests.java" checks="LineLength" />
<suppress files="server[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]mapper[/\\]DynamicMappingTests.java" checks="LineLength" />
<suppress files="server[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]mapper[/\\]ExternalFieldMapperTests.java" checks="LineLength" />
<suppress files="server[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]mapper[/\\]ExternalMapper.java" checks="LineLength" />
<suppress files="server[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]mapper[/\\]ExternalMetadataMapper.java" checks="LineLength" />
<suppress files="server[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]mapper[/\\]FieldNamesFieldMapperTests.java" checks="LineLength" />
Expand Down
23 changes: 16 additions & 7 deletions server/src/main/java/org/elasticsearch/Version.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ public class Version implements Comparable<Version>, ToXContentFragment {
* values below 25 are for alpha builder (since 5.0), and above 25 and below 50 are beta builds, and below 99 are RC builds, with 99
* indicating a release the (internal) format of the id is there so we can easily do after/before checks on the id
*/
public static final int V_EMPTY_ID = 0;
public static final Version V_EMPTY = new Version(V_EMPTY_ID, org.apache.lucene.util.Version.LATEST);
public static final int V_6_0_0_alpha1_ID = 6000001;
public static final Version V_6_0_0_alpha1 =
new Version(V_6_0_0_alpha1_ID, org.apache.lucene.util.Version.LUCENE_7_0_0);
Expand Down Expand Up @@ -167,6 +169,8 @@ public static Version fromId(int id) {
return V_6_0_0_alpha2;
case V_6_0_0_alpha1_ID:
return V_6_0_0_alpha1;
case V_EMPTY_ID:
return V_EMPTY;
default:
return new Version(id, org.apache.lucene.util.Version.LATEST);
}
Expand All @@ -179,11 +183,14 @@ public static Version fromId(int id) {
* {@value IndexMetaData#SETTING_VERSION_CREATED}
*/
public static Version indexCreated(Settings indexSettings) {
final Version indexVersion = indexSettings.getAsVersion(IndexMetaData.SETTING_VERSION_CREATED, null);
if (indexVersion == null) {
throw new IllegalStateException(
"[" + IndexMetaData.SETTING_VERSION_CREATED + "] is not present in the index settings for index with uuid: ["
+ indexSettings.get(IndexMetaData.SETTING_INDEX_UUID) + "]");
final Version indexVersion = IndexMetaData.SETTING_INDEX_VERSION_CREATED.get(indexSettings);
if (indexVersion == V_EMPTY) {
final String message = String.format(
Locale.ROOT,
"[%s] is not present in the index settings for index with UUID [%s]",
IndexMetaData.SETTING_INDEX_VERSION_CREATED.getKey(),
indexSettings.get(IndexMetaData.SETTING_INDEX_UUID));
throw new IllegalStateException(message);
}
return indexVersion;
}
Expand Down Expand Up @@ -470,8 +477,10 @@ public static List<Version> getDeclaredVersions(final Class<?> versionClass) {
if (field.getType() != Version.class) {
continue;
}
if ("CURRENT".equals(field.getName())) {
continue;
switch (field.getName()) {
case "CURRENT":
case "V_EMPTY":
continue;
}
assert field.getName().matches("V(_\\d+)+(_(alpha|beta|rc)\\d+)?") : field.getName();
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import org.elasticsearch.cluster.metadata.IndexGraveyard;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.metadata.MetaData;
import org.elasticsearch.cluster.metadata.MetaDataCreateIndexService;
import org.elasticsearch.cluster.metadata.MetaDataDeleteIndexService;
import org.elasticsearch.cluster.metadata.MetaDataIndexAliasesService;
import org.elasticsearch.cluster.metadata.MetaDataIndexStateService;
Expand All @@ -51,10 +50,10 @@
import org.elasticsearch.cluster.routing.allocation.decider.RebalanceOnlyWhenActiveAllocationDecider;
import org.elasticsearch.cluster.routing.allocation.decider.ReplicaAfterPrimaryActiveAllocationDecider;
import org.elasticsearch.cluster.routing.allocation.decider.ResizeAllocationDecider;
import org.elasticsearch.cluster.routing.allocation.decider.RestoreInProgressAllocationDecider;
import org.elasticsearch.cluster.routing.allocation.decider.SameShardAllocationDecider;
import org.elasticsearch.cluster.routing.allocation.decider.ShardsLimitAllocationDecider;
import org.elasticsearch.cluster.routing.allocation.decider.SnapshotInProgressAllocationDecider;
import org.elasticsearch.cluster.routing.allocation.decider.RestoreInProgressAllocationDecider;
import org.elasticsearch.cluster.routing.allocation.decider.ThrottlingAllocationDecider;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.ParseField;
Expand Down Expand Up @@ -268,7 +267,6 @@ protected void configure() {
bind(AllocationService.class).toInstance(allocationService);
bind(ClusterService.class).toInstance(clusterService);
bind(NodeConnectionsService.class).asEagerSingleton();
bind(MetaDataCreateIndexService.class).asEagerSingleton();
bind(MetaDataDeleteIndexService.class).asEagerSingleton();
bind(MetaDataIndexStateService.class).asEagerSingleton();
bind(MetaDataMappingService.class).asEagerSingleton();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
Expand All @@ -46,7 +45,6 @@
*/
public class AliasValidator extends AbstractComponent {

@Inject
public AliasValidator(Settings settings) {
super(settings);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,10 @@ public Iterator<Setting<Integer>> settings() {
Setting.boolSetting(SETTING_READ_ONLY_ALLOW_DELETE, false, Property.Dynamic, Property.IndexScope);

public static final String SETTING_VERSION_CREATED = "index.version.created";

public static final Setting<Version> SETTING_INDEX_VERSION_CREATED =
Setting.versionSetting(SETTING_VERSION_CREATED, Version.V_EMPTY, Property.IndexScope, Property.PrivateIndex);

public static final String SETTING_VERSION_CREATED_STRING = "index.version.created_string";
public static final String SETTING_VERSION_UPGRADED = "index.version.upgraded";
public static final String SETTING_VERSION_UPGRADED_STRING = "index.version.upgraded_string";
Expand Down Expand Up @@ -1312,8 +1316,8 @@ public static IndexMetaData fromXContent(XContentParser parser) throws IOExcepti
*/
public static Settings addHumanReadableSettings(Settings settings) {
Settings.Builder builder = Settings.builder().put(settings);
Version version = settings.getAsVersion(SETTING_VERSION_CREATED, null);
if (version != null) {
Version version = SETTING_INDEX_VERSION_CREATED.get(settings);
if (version != Version.V_EMPTY) {
builder.put(SETTING_VERSION_CREATED_STRING, version.toString());
}
Version versionUpgraded = settings.getAsVersion(SETTING_VERSION_UPGRADED, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@
import org.elasticsearch.common.ValidationException;
import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.io.PathUtils;
import org.elasticsearch.common.settings.IndexScopedSettings;
import org.elasticsearch.common.settings.Setting;
Expand Down Expand Up @@ -94,7 +93,6 @@
import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_INDEX_UUID;
import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_REPLICAS;
import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS;
import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_VERSION_CREATED;

/**
* Service responsible for submitting create index requests
Expand All @@ -111,13 +109,19 @@ public class MetaDataCreateIndexService extends AbstractComponent {
private final IndexScopedSettings indexScopedSettings;
private final ActiveShardsObserver activeShardsObserver;
private final NamedXContentRegistry xContentRegistry;

@Inject
public MetaDataCreateIndexService(Settings settings, ClusterService clusterService,
IndicesService indicesService, AllocationService allocationService,
AliasValidator aliasValidator, Environment env,
IndexScopedSettings indexScopedSettings, ThreadPool threadPool,
NamedXContentRegistry xContentRegistry) {
private final boolean forbidPrivateIndexSettings;

public MetaDataCreateIndexService(
final Settings settings,
final ClusterService clusterService,
final IndicesService indicesService,
final AllocationService allocationService,
final AliasValidator aliasValidator,
final Environment env,
final IndexScopedSettings indexScopedSettings,
final ThreadPool threadPool,
final NamedXContentRegistry xContentRegistry,
final boolean forbidPrivateIndexSettings) {
super(settings);
this.clusterService = clusterService;
this.indicesService = indicesService;
Expand All @@ -127,6 +131,7 @@ public MetaDataCreateIndexService(Settings settings, ClusterService clusterServi
this.indexScopedSettings = indexScopedSettings;
this.activeShardsObserver = new ActiveShardsObserver(settings, clusterService, threadPool);
this.xContentRegistry = xContentRegistry;
this.forbidPrivateIndexSettings = forbidPrivateIndexSettings;
}

/**
Expand Down Expand Up @@ -348,10 +353,10 @@ public ClusterState execute(ClusterState currentState) throws Exception {
}
// now, put the request settings, so they override templates
indexSettingsBuilder.put(request.settings());
if (indexSettingsBuilder.get(SETTING_VERSION_CREATED) == null) {
DiscoveryNodes nodes = currentState.nodes();
if (indexSettingsBuilder.get(IndexMetaData.SETTING_INDEX_VERSION_CREATED.getKey()) == null) {
final DiscoveryNodes nodes = currentState.nodes();
final Version createdVersion = Version.min(Version.CURRENT, nodes.getSmallestNonClientNodeVersion());
indexSettingsBuilder.put(SETTING_VERSION_CREATED, createdVersion);
indexSettingsBuilder.put(IndexMetaData.SETTING_INDEX_VERSION_CREATED.getKey(), createdVersion);
}
if (indexSettingsBuilder.get(SETTING_NUMBER_OF_SHARDS) == null) {
final int numberOfShards = getNumberOfShards(indexSettingsBuilder);
Expand All @@ -373,7 +378,7 @@ public ClusterState execute(ClusterState currentState) throws Exception {
final Settings idxSettings = indexSettingsBuilder.build();
int numTargetShards = IndexMetaData.INDEX_NUMBER_OF_SHARDS_SETTING.get(idxSettings);
final int routingNumShards;
final Version indexVersionCreated = idxSettings.getAsVersion(IndexMetaData.SETTING_VERSION_CREATED, null);
final Version indexVersionCreated = IndexMetaData.SETTING_INDEX_VERSION_CREATED.get(idxSettings);
final IndexMetaData sourceMetaData = recoverFromIndex == null ? null :
currentState.metaData().getIndexSafe(recoverFromIndex);
if (sourceMetaData == null || sourceMetaData.getNumberOfShards() == 1) {
Expand Down Expand Up @@ -559,7 +564,9 @@ static int getNumberOfShards(final Settings.Builder indexSettingsBuilder) {
// TODO: this logic can be removed when the current major version is 8
assert Version.CURRENT.major == 7;
final int numberOfShards;
if (Version.fromId(Integer.parseInt(indexSettingsBuilder.get(SETTING_VERSION_CREATED))).before(Version.V_7_0_0_alpha1)) {
final Version indexVersionCreated =
Version.fromId(Integer.parseInt(indexSettingsBuilder.get(IndexMetaData.SETTING_INDEX_VERSION_CREATED.getKey())));
if (indexVersionCreated.before(Version.V_7_0_0_alpha1)) {
numberOfShards = 5;
} else {
numberOfShards = 1;
Expand All @@ -580,19 +587,20 @@ public void onFailure(String source, Exception e) {

private void validate(CreateIndexClusterStateUpdateRequest request, ClusterState state) {
validateIndexName(request.index(), state);
validateIndexSettings(request.index(), request.settings());
validateIndexSettings(request.index(), request.settings(), forbidPrivateIndexSettings);
}

public void validateIndexSettings(String indexName, Settings settings) throws IndexCreationException {
List<String> validationErrors = getIndexSettingsValidationErrors(settings);
public void validateIndexSettings(
final String indexName, final Settings settings, final boolean forbidPrivateIndexSettings) throws IndexCreationException {
List<String> validationErrors = getIndexSettingsValidationErrors(settings, forbidPrivateIndexSettings);
if (validationErrors.isEmpty() == false) {
ValidationException validationException = new ValidationException();
validationException.addValidationErrors(validationErrors);
throw new IndexCreationException(indexName, validationException);
}
}

List<String> getIndexSettingsValidationErrors(Settings settings) {
List<String> getIndexSettingsValidationErrors(final Settings settings, final boolean forbidPrivateIndexSettings) {
String customPath = IndexMetaData.INDEX_DATA_PATH_SETTING.get(settings);
List<String> validationErrors = new ArrayList<>();
if (Strings.isEmpty(customPath) == false && env.sharedDataFile() == null) {
Expand All @@ -603,6 +611,16 @@ List<String> getIndexSettingsValidationErrors(Settings settings) {
validationErrors.add("custom path [" + customPath + "] is not a sub-path of path.shared_data [" + env.sharedDataFile() + "]");
}
}
if (forbidPrivateIndexSettings) {
for (final String key : settings.keySet()) {
final Setting<?> setting = indexScopedSettings.get(key);
if (setting == null) {
assert indexScopedSettings.isPrivateSetting(key);
} else if (setting.isPrivateIndex()) {
validationErrors.add("private index setting [" + key + "] can not be set explicitly");
}
}
}
return validationErrors;
}

Expand Down Expand Up @@ -737,7 +755,7 @@ static void prepareResizeIndexSettings(
}

indexSettingsBuilder
.put(IndexMetaData.SETTING_VERSION_CREATED, sourceMetaData.getCreationVersion())
.put(IndexMetaData.SETTING_INDEX_VERSION_CREATED.getKey(), sourceMetaData.getCreationVersion())
.put(IndexMetaData.SETTING_VERSION_UPGRADED, sourceMetaData.getUpgradedVersion())
.put(builder.build())
.put(IndexMetaData.SETTING_ROUTING_PARTITION_SIZE, sourceMetaData.getRoutingPartitionSize())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
package org.elasticsearch.cluster.metadata;

import com.carrotsearch.hppc.cursors.ObjectCursor;

import org.apache.lucene.util.CollectionUtil;
import org.elasticsearch.Version;
import org.elasticsearch.action.admin.indices.alias.Alias;
Expand Down Expand Up @@ -301,7 +300,7 @@ private void validate(PutRequest request) {
validationErrors.add(t.getMessage());
}
}
List<String> indexSettingsValidation = metaDataCreateIndexService.getIndexSettingsValidationErrors(request.settings);
List<String> indexSettingsValidation = metaDataCreateIndexService.getIndexSettingsValidationErrors(request.settings, true);
validationErrors.addAll(indexSettingsValidation);
if (!validationErrors.isEmpty()) {
ValidationException validationException = new ValidationException();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public void updateSettings(final UpdateSettingsClusterStateUpdateRequest request
indexScopedSettings.validate(
normalizedSettings.filter(s -> Regex.isSimpleMatchPattern(s) == false), // don't validate wildcards
false, // don't validate dependencies here we check it below never allow to change the number of shards
true); // validate internal index settings
true); // validate internal or private index settings
for (String key : normalizedSettings.keySet()) {
Setting setting = indexScopedSettings.get(key);
boolean isWildcard = setting == null && Regex.isSimpleMatchPattern(key);
Expand Down
Loading

0 comments on commit 09bf4e5

Please sign in to comment.