diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java index ff47d5ce94292..2cd1a2d5b3fea 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java @@ -1213,6 +1213,8 @@ private void createConfiguration() { // Don't wait for state, just start up quickly. This will also allow new and old nodes in the BWC case to become the master baseConfig.put("discovery.initial_state_timeout", "0s"); + baseConfig.put("action.destructive_requires_name", "false"); + HashSet overriden = new HashSet<>(baseConfig.keySet()); overriden.retainAll(settings.keySet()); overriden.removeAll(OVERRIDABLE_SETTINGS); diff --git a/distribution/docker/docker-compose.yml b/distribution/docker/docker-compose.yml index f187186c59409..62704cc6510c8 100644 --- a/distribution/docker/docker-compose.yml +++ b/distribution/docker/docker-compose.yml @@ -30,6 +30,7 @@ services: - xpack.http.ssl.verification_mode=certificate - xpack.security.transport.ssl.verification_mode=certificate - xpack.license.self_generated.type=trial + - action.destructive_requires_name=false volumes: - ./build/repo:/tmp/es-repo - ./build/certs/testnode.pem:/usr/share/elasticsearch/config/testnode.pem @@ -72,6 +73,7 @@ services: - xpack.http.ssl.verification_mode=certificate - xpack.security.transport.ssl.verification_mode=certificate - xpack.license.self_generated.type=trial + - action.destructive_requires_name=false volumes: - ./build/repo:/tmp/es-repo - ./build/certs/testnode.pem:/usr/share/elasticsearch/config/testnode.pem diff --git a/qa/remote-clusters/docker-compose-oss.yml b/qa/remote-clusters/docker-compose-oss.yml index 1a77d1bc9999d..911058f123a6d 100644 --- a/qa/remote-clusters/docker-compose-oss.yml +++ b/qa/remote-clusters/docker-compose-oss.yml @@ -16,6 +16,7 @@ services: - cluster.routing.allocation.disk.watermark.high=1b - cluster.routing.allocation.disk.watermark.flood_stage=1b - node.store.allow_mmap=false + - action.destructive_requires_name=false volumes: - ./build/oss-repo:/tmp/es-repo - ./build/logs/oss-1:/usr/share/elasticsearch/logs @@ -50,6 +51,7 @@ services: - cluster.routing.allocation.disk.watermark.high=1b - cluster.routing.allocation.disk.watermark.flood_stage=1b - node.store.allow_mmap=false + - action.destructive_requires_name=false volumes: - ./build/oss-repo:/tmp/es-repo - ./build/logs/oss-2:/usr/share/elasticsearch/logs diff --git a/qa/remote-clusters/docker-compose.yml b/qa/remote-clusters/docker-compose.yml index debf863c58067..bcbbe9c37d259 100644 --- a/qa/remote-clusters/docker-compose.yml +++ b/qa/remote-clusters/docker-compose.yml @@ -30,6 +30,7 @@ services: - xpack.http.ssl.verification_mode=certificate - xpack.security.transport.ssl.verification_mode=certificate - xpack.license.self_generated.type=trial + - action.destructive_requires_name=false volumes: - ./build/repo:/tmp/es-repo - ./build/certs/testnode.pem:/usr/share/elasticsearch/config/testnode.pem @@ -82,6 +83,7 @@ services: - xpack.http.ssl.verification_mode=certificate - xpack.security.transport.ssl.verification_mode=certificate - xpack.license.self_generated.type=trial + - action.destructive_requires_name=false volumes: - ./build/repo:/tmp/es-repo - ./build/certs/testnode.pem:/usr/share/elasticsearch/config/testnode.pem diff --git a/server/src/main/java/org/elasticsearch/action/support/DestructiveOperations.java b/server/src/main/java/org/elasticsearch/action/support/DestructiveOperations.java index 5a72520f03357..a37087113f4a6 100644 --- a/server/src/main/java/org/elasticsearch/action/support/DestructiveOperations.java +++ b/server/src/main/java/org/elasticsearch/action/support/DestructiveOperations.java @@ -8,6 +8,9 @@ package org.elasticsearch.action.support; +import org.elasticsearch.common.collect.List; +import org.elasticsearch.common.logging.DeprecationCategory; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; @@ -20,11 +23,17 @@ */ public final class DestructiveOperations { + private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(DestructiveOperations.class); + /** * Setting which controls whether wildcard usage (*, prefix*, _all) is allowed. */ public static final Setting REQUIRES_NAME_SETTING = Setting.boolSetting("action.destructive_requires_name", false, Property.Dynamic, Property.NodeScope); + + static final String DESTRUCTIVE_REQUIRES_NAME_DEPRECATION_WARNING = "setting [action.destructive_requires_name] will " + + "default to true in 8.0, set explicitly to false to preserve current behavior"; + /** * The "match none" pattern, "*,-*", will never actually be destructive * because it operates on no indices. If plugins or other components add @@ -33,41 +42,60 @@ public final class DestructiveOperations { * indices, relying on the core code to handle this situation. */ private static final String[] MATCH_NONE_PATTERN = {"*", "-*"}; - private volatile boolean destructiveRequiresName; + private volatile boolean isDestructiveRequiresNameSet; + public DestructiveOperations(Settings settings, ClusterSettings clusterSettings) { destructiveRequiresName = REQUIRES_NAME_SETTING.get(settings); - clusterSettings.addSettingsUpdateConsumer(REQUIRES_NAME_SETTING, this::setDestructiveRequiresName); + isDestructiveRequiresNameSet = REQUIRES_NAME_SETTING.exists(settings); + clusterSettings.addSettingsUpdateConsumer( + this::setDestructiveRequiresName, + List.of(REQUIRES_NAME_SETTING) + ); } - private void setDestructiveRequiresName(boolean destructiveRequiresName) { - this.destructiveRequiresName = destructiveRequiresName; + private void setDestructiveRequiresName(Settings settings) { + this.isDestructiveRequiresNameSet = REQUIRES_NAME_SETTING.exists(settings); + this.destructiveRequiresName = REQUIRES_NAME_SETTING.get(settings); } /** * Fail if there is wildcard usage in indices and the named is required for destructive operations. */ public void failDestructive(String[] aliasesOrIndices) { - if (destructiveRequiresName == false) { + if (this.isDestructiveRequiresNameSet && this.destructiveRequiresName == false) { return; } if (aliasesOrIndices == null || aliasesOrIndices.length == 0) { - throw new IllegalArgumentException("Wildcard expressions or all indices are not allowed"); + checkWildCardOK(); } else if (aliasesOrIndices.length == 1) { if (hasWildcardUsage(aliasesOrIndices[0])) { - throw new IllegalArgumentException("Wildcard expressions or all indices are not allowed"); + checkWildCardOK(); } } else if (Arrays.equals(aliasesOrIndices, MATCH_NONE_PATTERN) == false) { for (String aliasesOrIndex : aliasesOrIndices) { if (hasWildcardUsage(aliasesOrIndex)) { - throw new IllegalArgumentException("Wildcard expressions or all indices are not allowed"); + checkWildCardOK(); } } } } + private void checkWildCardOK() { + if (this.isDestructiveRequiresNameSet == false) { + deprecationLogger.deprecate( + DeprecationCategory.SETTINGS, + "destructive_requires_name_default", + DESTRUCTIVE_REQUIRES_NAME_DEPRECATION_WARNING + ); + } + if (this.destructiveRequiresName) { + throw new IllegalArgumentException("Wildcard expressions or all indices are not allowed"); + } + } + private static boolean hasWildcardUsage(String aliasOrIndex) { return "_all".equals(aliasOrIndex) || aliasOrIndex.indexOf('*') != -1; } diff --git a/server/src/test/java/org/elasticsearch/action/support/DestructiveOperationsTests.java b/server/src/test/java/org/elasticsearch/action/support/DestructiveOperationsTests.java index 1d88393124822..e170955a6b8f8 100644 --- a/server/src/test/java/org/elasticsearch/action/support/DestructiveOperationsTests.java +++ b/server/src/test/java/org/elasticsearch/action/support/DestructiveOperationsTests.java @@ -71,6 +71,64 @@ public void testNonDestructive() { } } + /** + * Test that we get a deprecation warning if we try a wildcard deletion and + * action.destructive_requires_name is unset. + */ + public void testDeprecationOfDefaultValue() { + DestructiveOperations destructiveOperations = new DestructiveOperations(Settings.EMPTY, + new ClusterSettings(Settings.EMPTY, Set.of(DestructiveOperations.REQUIRES_NAME_SETTING))); + { + destructiveOperations.failDestructive(new String[]{"index-*"}); + assertWarnings(DestructiveOperations.DESTRUCTIVE_REQUIRES_NAME_DEPRECATION_WARNING); + } + { + destructiveOperations.failDestructive(new String[]{"index"}); + // no warning + } + { + destructiveOperations.failDestructive(new String[]{}); + assertWarnings(DestructiveOperations.DESTRUCTIVE_REQUIRES_NAME_DEPRECATION_WARNING); + } + } + + /** + * Test that applying settings enables and disables the deprecation warning. + */ + public void testDeprecationWarningClusterSettingsSync() { + ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, Set.of(DestructiveOperations.REQUIRES_NAME_SETTING)); + DestructiveOperations destructiveOperations = new DestructiveOperations(Settings.EMPTY, clusterSettings); + + { + // empty settings gives us a warning + destructiveOperations.failDestructive(new String[]{"index-*"}); + assertWarnings(DestructiveOperations.DESTRUCTIVE_REQUIRES_NAME_DEPRECATION_WARNING); + } + { + // setting to "true" removes the deprecation warning and fails the operation + clusterSettings.applySettings(Settings.builder() + .put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), "true") + .build()); + + assertFailsDestructive(new String[]{"index-*"}); + } + { + // restoring the empty setting restores the warning + clusterSettings.applySettings(Settings.EMPTY); + + destructiveOperations.failDestructive(new String[]{"index-*"}); + assertWarnings(DestructiveOperations.DESTRUCTIVE_REQUIRES_NAME_DEPRECATION_WARNING); + } + { + // explicitly set to false: no warning, no failure + clusterSettings.applySettings(Settings.builder() + .put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), "false") + .build()); + + destructiveOperations.failDestructive(new String[]{"index-*"}); + } + } + private void assertFailsDestructive(String[] aliasesOrIndices) { IllegalArgumentException e = expectThrows( IllegalArgumentException.class, diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java index 7efbab650cb5b..08b09731630e3 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java @@ -45,6 +45,7 @@ import org.elasticsearch.action.search.ClearScrollResponse; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.support.DefaultShardOperationFailedException; +import org.elasticsearch.action.support.DestructiveOperations; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.bootstrap.JavaVersion; import org.elasticsearch.client.AdminClient; @@ -1849,6 +1850,7 @@ private int getNumClientNodes() { */ protected Settings nodeSettings(int nodeOrdinal) { Settings.Builder builder = Settings.builder() + .put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), false) // Default the watermarks to absurdly low to prevent the tests // from failing on nodes without enough disk space .put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING.getKey(), "1b") diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java index 8ee8111722b63..8d47239e8291c 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java @@ -12,6 +12,7 @@ import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse; import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder; import org.elasticsearch.action.admin.indices.get.GetIndexResponse; +import org.elasticsearch.action.support.DestructiveOperations; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.client.Client; import org.elasticsearch.client.Requests; @@ -190,6 +191,7 @@ private Node newNode() { Settings settings = Settings.builder() .put(ClusterName.CLUSTER_NAME_SETTING.getKey(), InternalTestCluster.clusterName("single-node-cluster", random().nextLong())) + .put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), false) .put(Environment.PATH_HOME_SETTING.getKey(), tempDir) .put(Environment.PATH_REPO_SETTING.getKey(), tempDir.resolve("repo")) // TODO: use a consistent data path for custom paths diff --git a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java index 2c64d3986fc91..00e28a9f247bf 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java @@ -26,6 +26,7 @@ import org.elasticsearch.action.admin.cluster.node.stats.NodeStats; import org.elasticsearch.action.admin.indices.stats.CommonStatsFlags; import org.elasticsearch.action.admin.indices.stats.CommonStatsFlags.Flag; +import org.elasticsearch.action.support.DestructiveOperations; import org.elasticsearch.action.support.replication.TransportReplicationAction; import org.elasticsearch.client.Client; import org.elasticsearch.client.transport.TransportClient; @@ -402,6 +403,7 @@ public InternalTestCluster( // TODO: currently we only randomize "cluster.no_master_block" between "write" and "metadata_write", as "all" is fragile // and fails shards when a master abdicates, which breaks many tests. builder.put(NoMasterBlockService.NO_MASTER_BLOCK_SETTING.getKey(), randomFrom(random,"write", "metadata_write")); + builder.put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), false); defaultSettings = builder.build(); executor = EsExecutors.newScaling("internal_test_cluster_executor", 0, Integer.MAX_VALUE, 0, TimeUnit.SECONDS, EsExecutors.daemonThreadFactory("test_" + clusterName), new ThreadContext(Settings.EMPTY)); diff --git a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/CcrIntegTestCase.java b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/CcrIntegTestCase.java index f09321d3a628f..0d6ba4242ef10 100644 --- a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/CcrIntegTestCase.java +++ b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/CcrIntegTestCase.java @@ -22,6 +22,7 @@ import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.support.ActiveShardCount; +import org.elasticsearch.action.support.DestructiveOperations; import org.elasticsearch.action.support.master.AcknowledgedRequest; import org.elasticsearch.analysis.common.CommonAnalysisPlugin; import org.elasticsearch.client.Client; @@ -277,6 +278,7 @@ private NodeConfigurationSource createNodeConfigurationSource(final String leade builder.put(XPackSettings.WATCHER_ENABLED.getKey(), false); builder.put(XPackSettings.MACHINE_LEARNING_ENABLED.getKey(), false); builder.put(LicenseService.SELF_GENERATED_LICENSE_TYPE.getKey(), "trial"); + builder.put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), false); // Let cluster state api return quickly in order to speed up auto follow tests: builder.put(CcrSettings.CCR_WAIT_FOR_METADATA_TIMEOUT.getKey(), TimeValue.timeValueMillis(100)); if (leaderCluster) {