Skip to content

Commit ddda5f2

Browse files
williamrandolphwilliamrandolph
committed
Allow the "*,-*" ("match none") pattern for destructive actions when destructive_requires_name is true (#68021)
Since the "*,-*" pattern resolves to "no indices", it makes a normally destructive action into a non-destructive one. Rather than throwing a wildcards-not-allowed exception, we can allow this pattern to pass without triggering an exception. This allows the security layer to safely use a "*,-*" pattern to indicate a "no indices" result for its index resolution step, which is important because otherwise we get wildcards-not-allowed exceptions when trying to delete nonexistent concrete indices. For simplicity, we require exactly "*,-*", rather than any other wildcards that might be logically equivalent.
1 parent d975922 commit ddda5f2

5 files changed

Lines changed: 183 additions & 1 deletion

File tree

‎rest-api-spec/src/main/resources/rest-api-spec/test/indices.delete/20_destructive_wildcard.yml‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
setup:
3+
- do:
4+
cluster.put_settings:
5+
body:
6+
transient:
7+
action.destructive_requires_name: "true"
8+
flat_settings: true
9+
---
10+
teardown:
11+
- do:
12+
cluster.put_settings:
13+
body:
14+
transient:
15+
action.destructive_requires_name: "false"
16+
flat_settings: true
17+
---
18+
"Delete nonexistent concrete index with wildcard expansion disallowed":
19+
- do:
20+
indices.delete:
21+
index: index3
22+
ignore_unavailable: true

‎server/src/internalClusterTest/java/org/elasticsearch/operateAllIndices/DestructiveOperationsIT.java‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.elasticsearch.test.ESIntegTestCase;
2828
import org.junit.After;
2929

30+
import static org.elasticsearch.cluster.metadata.IndexMetadata.APIBlock.WRITE;
3031
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
3132
import static org.hamcrest.Matchers.equalTo;
3233

@@ -48,6 +49,8 @@ public void testDeleteIndexIsRejected() throws Exception {
4849

4950
// Should succeed, since no wildcards
5051
assertAcked(client().admin().indices().prepareDelete("1index").get());
52+
// Special "match none" pattern succeeds, since non-destructive
53+
assertAcked(client().admin().indices().prepareDelete("*", "-*").get());
5154

5255
expectThrows(IllegalArgumentException.class, () -> client().admin().indices().prepareDelete("i*").get());
5356
expectThrows(IllegalArgumentException.class, () -> client().admin().indices().prepareDelete("_all").get());
@@ -82,6 +85,8 @@ public void testCloseIndexIsRejected() throws Exception {
8285

8386
// Should succeed, since no wildcards
8487
assertAcked(client().admin().indices().prepareClose("1index").get());
88+
// Special "match none" pattern succeeds, since non-destructive
89+
assertAcked(client().admin().indices().prepareClose("*", "-*").get());
8590

8691
expectThrows(IllegalArgumentException.class, () -> client().admin().indices().prepareClose("i*").get());
8792
expectThrows(IllegalArgumentException.class, () -> client().admin().indices().prepareClose("_all").get());
@@ -118,6 +123,9 @@ public void testOpenIndexIsRejected() throws Exception {
118123
createIndex("index1", "1index");
119124
assertAcked(client().admin().indices().prepareClose("1index", "index1").get());
120125

126+
// Special "match none" pattern succeeds, since non-destructive
127+
assertAcked(client().admin().indices().prepareOpen("*", "-*").get());
128+
121129
expectThrows(IllegalArgumentException.class, () -> client().admin().indices().prepareOpen("i*").get());
122130
expectThrows(IllegalArgumentException.class, () -> client().admin().indices().prepareOpen("_all").get());
123131
}
@@ -144,4 +152,46 @@ public void testOpenIndexDefaultBehaviour() throws Exception {
144152
assertEquals(IndexMetadata.State.OPEN, indexMetadataObjectObjectCursor.value.getState());
145153
}
146154
}
155+
156+
public void testAddIndexBlockIsRejected() throws Exception {
157+
Settings settings = Settings.builder()
158+
.put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), true)
159+
.build();
160+
assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(settings));
161+
162+
createIndex("index1", "1index");
163+
164+
// Should succeed, since no wildcards
165+
assertAcked(client().admin().indices().prepareAddBlock(WRITE,"1index").get());
166+
// Special "match none" pattern succeeds, since non-destructive
167+
assertAcked(client().admin().indices().prepareAddBlock(WRITE,"*", "-*").get());
168+
169+
expectThrows(IllegalArgumentException.class,
170+
() -> client().admin().indices().prepareAddBlock(WRITE,"i*").get());
171+
expectThrows(IllegalArgumentException.class,
172+
() -> client().admin().indices().prepareAddBlock(WRITE, "_all").get());
173+
}
174+
175+
public void testAddIndexBlockDefaultBehaviour() throws Exception {
176+
if (randomBoolean()) {
177+
Settings settings = Settings.builder()
178+
.put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), false)
179+
.build();
180+
assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(settings));
181+
}
182+
183+
createIndex("index1", "1index");
184+
185+
if (randomBoolean()) {
186+
assertAcked(client().admin().indices().prepareAddBlock(WRITE, "_all").get());
187+
} else {
188+
assertAcked(client().admin().indices().prepareAddBlock(WRITE, "*").get());
189+
}
190+
191+
ClusterState state = client().admin().cluster().prepareState().get().getState();
192+
assertTrue("write block is set on index1",
193+
state.getBlocks().hasIndexBlock("index1", IndexMetadata.INDEX_WRITE_BLOCK));
194+
assertTrue("write block is set on 1index",
195+
state.getBlocks().hasIndexBlock("1index", IndexMetadata.INDEX_WRITE_BLOCK));
196+
}
147197
}

‎server/src/main/java/org/elasticsearch/action/support/DestructiveOperations.java‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
import org.elasticsearch.common.settings.Setting.Property;
2525
import org.elasticsearch.common.settings.Settings;
2626

27+
import java.util.Arrays;
28+
2729
/**
2830
* Helper for dealing with destructive operations and wildcard usage.
2931
*/
@@ -34,6 +36,15 @@ public final class DestructiveOperations {
3436
*/
3537
public static final Setting<Boolean> REQUIRES_NAME_SETTING =
3638
Setting.boolSetting("action.destructive_requires_name", false, Property.Dynamic, Property.NodeScope);
39+
/**
40+
* The "match none" pattern, "*,-*", will never actually be destructive
41+
* because it operates on no indices. If plugins or other components add
42+
* their own index resolution layers, they can pass this pattern to the
43+
* core code in order to indicate that an operation won't run on any
44+
* indices, relying on the core code to handle this situation.
45+
*/
46+
private static final String[] MATCH_NONE_PATTERN = {"*", "-*"};
47+
3748
private volatile boolean destructiveRequiresName;
3849

3950
public DestructiveOperations(Settings settings, ClusterSettings clusterSettings) {
@@ -59,7 +70,7 @@ public void failDestructive(String[] aliasesOrIndices) {
5970
if (hasWildcardUsage(aliasesOrIndices[0])) {
6071
throw new IllegalArgumentException("Wildcard expressions or all indices are not allowed");
6172
}
62-
} else {
73+
} else if (Arrays.equals(aliasesOrIndices, MATCH_NONE_PATTERN) == false) {
6374
for (String aliasesOrIndex : aliasesOrIndices) {
6475
if (hasWildcardUsage(aliasesOrIndex)) {
6576
throw new IllegalArgumentException("Wildcard expressions or all indices are not allowed");

‎server/src/test/java/org/elasticsearch/action/support/DestructiveOperationsTests.java‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.action.support;
21+
22+
import org.elasticsearch.common.settings.ClusterSettings;
23+
import org.elasticsearch.common.settings.Settings;
24+
import org.elasticsearch.test.ESTestCase;
25+
26+
import java.util.Set;
27+
28+
import static org.hamcrest.Matchers.equalTo;
29+
30+
public class DestructiveOperationsTests extends ESTestCase {
31+
32+
private DestructiveOperations destructiveOperations;
33+
34+
@Override
35+
public void setUp() throws Exception {
36+
super.setUp();
37+
Settings nodeSettings = Settings.builder()
38+
.put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), "true")
39+
.build();
40+
destructiveOperations = new DestructiveOperations(
41+
nodeSettings,
42+
new ClusterSettings(nodeSettings, Set.of(DestructiveOperations.REQUIRES_NAME_SETTING)));
43+
}
44+
45+
public void testDestructive() {
46+
{
47+
// requests that might resolve to all indices
48+
assertFailsDestructive(null);
49+
assertFailsDestructive(new String[]{});
50+
assertFailsDestructive(new String[]{"_all"});
51+
assertFailsDestructive(new String[]{"*"});
52+
}
53+
{
54+
// various wildcards
55+
assertFailsDestructive(new String[] {"-*"});
56+
assertFailsDestructive(new String[] {"index*"});
57+
assertFailsDestructive(new String[] {"index", "*"});
58+
assertFailsDestructive(new String[] {"index", "-*"});
59+
assertFailsDestructive(new String[] {"index", "test-*-index"});
60+
}
61+
{
62+
// near versions of the "matchNone" pattern
63+
assertFailsDestructive(new String[]{"-*", "*"});
64+
assertFailsDestructive(new String[]{"*", "-*", "*"});
65+
}
66+
}
67+
68+
/**
69+
* Test that non-wildcard expressions or the special "*,-*" don't throw an
70+
* exception. Since {@link DestructiveOperations#failDestructive(String[])}
71+
* has no return value, we run the statements without asserting anything
72+
* about them.
73+
*/
74+
public void testNonDestructive() {
75+
{
76+
// no wildcards
77+
destructiveOperations.failDestructive(new String[]{"index"});
78+
destructiveOperations.failDestructive(new String[]{"index", "-index2"});
79+
}
80+
{
81+
// special "matchNone" pattern
82+
destructiveOperations.failDestructive(new String[]{"*", "-*"});
83+
}
84+
}
85+
86+
private void assertFailsDestructive(String[] aliasesOrIndices) {
87+
IllegalArgumentException e = expectThrows(
88+
IllegalArgumentException.class,
89+
() -> destructiveOperations.failDestructive(aliasesOrIndices));
90+
91+
assertThat(e.getMessage(), equalTo("Wildcard expressions or all indices are not allowed"));
92+
}
93+
}

‎x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/action/filter/DestructiveOperationsTests.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ public void testDeleteIndexDestructiveOperationsRequireName() {
4949
assertEquals("index1", indices[0]);
5050
}
5151

52+
// the "*,-*" pattern is specially handled because it makes a destructive action non-destructive
53+
assertAcked(client().admin().indices().prepareDelete("*", "-*"));
5254
assertAcked(client().admin().indices().prepareDelete("index1"));
5355
}
5456

@@ -114,6 +116,10 @@ public void testOpenCloseIndexDestructiveOperationsRequireName() {
114116
assertEquals("Wildcard expressions or all indices are not allowed", illegalArgumentException.getMessage());
115117
}
116118

119+
// the "*,-*" pattern is specially handled because it makes a destructive action non-destructive
120+
assertAcked(client().admin().indices().prepareClose("*", "-*"));
121+
assertAcked(client().admin().indices().prepareOpen("*", "-*"));
122+
117123
createIndex("index1");
118124
assertAcked(client().admin().indices().prepareClose("index1"));
119125
assertAcked(client().admin().indices().prepareOpen("index1"));

0 commit comments

Comments
 (0)