Skip to content

Commit

Permalink
Enable must_exist parameter for update aliases API (opensearch-projec…
Browse files Browse the repository at this point in the history
…t#11210)

* Enable must_exist parameter for update aliases API

Signed-off-by: Gao Binlong <[email protected]>

* modify changelog

Signed-off-by: Gao Binlong <[email protected]>

* Fix test failure

Signed-off-by: Gao Binlong <[email protected]>

* Fix test failure

Signed-off-by: Gao Binlong <[email protected]>

* Remove silently when all aliases are non-existing and must_exist is false

Signed-off-by: Gao Binlong <[email protected]>

* Modify some comments to run gradle check again

Signed-off-by: Gao Binlong <[email protected]>

* Add more yaml test

Signed-off-by: Gao Binlong <[email protected]>

---------

Signed-off-by: Gao Binlong <[email protected]>
Signed-off-by: Shivansh Arora <[email protected]>
  • Loading branch information
gaobinlong authored and shiv0408 committed Apr 25, 2024
1 parent dcb1230 commit d2f644f
Show file tree
Hide file tree
Showing 8 changed files with 196 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Implement on behalf of token passing for extensions ([#8679](https://github.com/opensearch-project/OpenSearch/pull/8679))
- Provide service accounts tokens to extensions ([#9618](https://github.com/opensearch-project/OpenSearch/pull/9618))
- [Streaming Indexing] Introduce new experimental server HTTP transport based on Netty 4 and Project Reactor (Reactor Netty) ([#9672](https://github.com/opensearch-project/OpenSearch/pull/9672))
- Enable must_exist parameter for update aliases API ([#11210](https://github.com/opensearch-project/OpenSearch/pull/11210))
- Add back half_float BKD based sort query optimization ([#11024](https://github.com/opensearch-project/OpenSearch/pull/11024))
- Request level coordinator slow logs ([#10650](https://github.com/opensearch-project/OpenSearch/pull/10650))
- Add template snippets support for field and target_field in KV ingest processor ([#10040](https://github.com/opensearch-project/OpenSearch/pull/10040))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
---
"Throw aliases missing exception when removing non-existing alias with setting must_exist to true":
- skip:
version: " - 2.99.99"
reason: "introduced in 3.0"

- do:
indices.create:
index: test_index

- do:
indices.exists_alias:
name: test_alias

- is_false: ''

- do:
catch: /aliases \[test_alias\] missing/
indices.update_aliases:
body:
actions:
- remove:
index: test_index
alias: test_alias
must_exist: true

- do:
catch: /aliases \[testAlias\*\] missing/
indices.update_aliases:
body:
actions:
- remove:
index: test_index
aliases: [ testAlias* ]
must_exist: true

- do:
catch: /\[aliases\] can't be empty/
indices.update_aliases:
body:
actions:
- remove:
index: test_index
aliases: []
must_exist: true

---
"Throw aliases missing exception when all of the specified aliases are non-existing":
- skip:
version: " - 2.99.99"
reason: "introduced in 3.0"

- do:
indices.create:
index: test_index

- do:
indices.exists_alias:
name: test_alias

- is_false: ''

- do:
catch: /aliases \[test\_alias\] missing/
indices.update_aliases:
body:
actions:
- remove:
index: test_index
alias: test_alias

- do:
catch: /aliases \[test\_alias\*\] missing/
indices.update_aliases:
body:
actions:
- remove:
indices: [ test_index ]
aliases: [ test_alias* ]

---
"Remove successfully when some specified aliases are non-existing":
- skip:
version: " - 2.99.99"
reason: "introduced in 3.0"

- do:
indices.create:
index: test_index

- do:
indices.exists_alias:
name: test_alias

- is_false: ''

- do:
indices.update_aliases:
body:
actions:
- add:
indices: [ test_index ]
aliases: [ test_alias ]

- do:
indices.update_aliases:
body:
actions:
- remove:
index: test_index
aliases: [test_alias, test_alias1, test_alias2]
must_exist: false

- match: { acknowledged: true }

---
"Remove silently when all of the specified aliases are non-existing and must_exist is false":
- skip:
version: " - 2.99.99"
reason: "introduced in 3.0"

- do:
indices.create:
index: test_index

- do:
indices.exists_alias:
name: test_alias

- is_false: ''

- do:
indices.update_aliases:
body:
actions:
- remove:
index: test_index
aliases: [test_alias, test_alias1, test_alias2]
must_exist: false

- match: { acknowledged: true }
Original file line number Diff line number Diff line change
Expand Up @@ -220,9 +220,11 @@ private static ObjectParser<AliasActions, Void> parser(String name, Supplier<Ali
ADD_PARSER.declareField(AliasActions::searchRouting, XContentParser::text, SEARCH_ROUTING, ValueType.INT);
ADD_PARSER.declareField(AliasActions::writeIndex, XContentParser::booleanValue, IS_WRITE_INDEX, ValueType.BOOLEAN);
ADD_PARSER.declareField(AliasActions::isHidden, XContentParser::booleanValue, IS_HIDDEN, ValueType.BOOLEAN);
ADD_PARSER.declareField(AliasActions::mustExist, XContentParser::booleanValue, MUST_EXIST, ValueType.BOOLEAN);
}
private static final ObjectParser<AliasActions, Void> REMOVE_PARSER = parser(REMOVE.getPreferredName(), AliasActions::remove);
static {
REMOVE_PARSER.declareField(AliasActions::mustExist, XContentParser::booleanValue, MUST_EXIST, ValueType.BOOLEAN);
}
private static final ObjectParser<AliasActions, Void> REMOVE_INDEX_PARSER = parser(
REMOVE_INDEX.getPreferredName(),
AliasActions::removeIndex
Expand Down Expand Up @@ -554,6 +556,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
if (null != isHidden) {
builder.field(IS_HIDDEN.getPreferredName(), isHidden);
}
if (null != mustExist) {
builder.field(MUST_EXIST.getPreferredName(), mustExist);
}
builder.endObject();
builder.endObject();
return builder;
Expand Down Expand Up @@ -582,6 +587,8 @@ public String toString() {
+ searchRouting
+ ",writeIndex="
+ writeIndex
+ ",mustExist="
+ mustExist
+ "]";
}

Expand All @@ -600,12 +607,13 @@ public boolean equals(Object obj) {
&& Objects.equals(indexRouting, other.indexRouting)
&& Objects.equals(searchRouting, other.searchRouting)
&& Objects.equals(writeIndex, other.writeIndex)
&& Objects.equals(isHidden, other.isHidden);
&& Objects.equals(isHidden, other.isHidden)
&& Objects.equals(mustExist, other.mustExist);
}

@Override
public int hashCode() {
return Objects.hash(type, indices, aliases, filter, routing, indexRouting, searchRouting, writeIndex, isHidden);
return Objects.hash(type, indices, aliases, filter, routing, indexRouting, searchRouting, writeIndex, isHidden, mustExist);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import org.opensearch.cluster.metadata.MetadataIndexAliasesService;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.inject.Inject;
import org.opensearch.common.regex.Regex;
import org.opensearch.core.action.ActionListener;
import org.opensearch.core.common.io.stream.StreamInput;
import org.opensearch.core.index.Index;
Expand All @@ -59,6 +60,7 @@

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
Expand Down Expand Up @@ -220,12 +222,33 @@ private static String[] concreteAliases(IndicesAliasesRequest.AliasActions actio
// for DELETE we expand the aliases
String[] indexAsArray = { concreteIndex };
final Map<String, List<AliasMetadata>> aliasMetadata = metadata.findAliases(action, indexAsArray);
List<String> finalAliases = new ArrayList<>();
Set<String> finalAliases = new HashSet<>();
for (final List<AliasMetadata> curAliases : aliasMetadata.values()) {
for (AliasMetadata aliasMeta : curAliases) {
finalAliases.add(aliasMeta.alias());
}
}

// must_exist can only be set in the Remove Action in Update aliases API,
// we check the value here to make the behavior consistent with Delete aliases API
if (action.mustExist() != null) {
// if must_exist is false, we should make the remove action execute silently,
// so we return the original specified aliases to avoid AliasesNotFoundException
if (!action.mustExist()) {
return action.aliases();
}

// if there is any non-existing aliases specified in the request and must_exist is true, throw exception in advance
if (finalAliases.isEmpty()) {
throw new AliasesNotFoundException(action.aliases());
}
String[] nonExistingAliases = Arrays.stream(action.aliases())
.filter(originalAlias -> finalAliases.stream().noneMatch(finalAlias -> Regex.simpleMatch(originalAlias, finalAlias)))
.toArray(String[]::new);
if (nonExistingAliases.length != 0) {
throw new AliasesNotFoundException(nonExistingAliases);
}
}
return finalAliases.toArray(new String[0]);
} else {
// for ADD and REMOVE_INDEX we just return the current aliases
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.opensearch.action.admin.indices.alias.IndicesAliasesRequest;
import org.opensearch.common.Nullable;
import org.opensearch.core.common.Strings;
import org.opensearch.rest.action.admin.indices.AliasesNotFoundException;

/**
* Individual operation to perform on the cluster state as part of an {@link IndicesAliasesRequest}.
Expand Down Expand Up @@ -225,8 +226,8 @@ boolean removeIndex() {
@Override
boolean apply(NewAliasValidator aliasValidator, Metadata.Builder metadata, IndexMetadata index) {
if (false == index.getAliases().containsKey(alias)) {
if (mustExist != null && mustExist.booleanValue()) {
throw new IllegalArgumentException("required alias [" + alias + "] does not exist");
if (mustExist != null && mustExist) {
throw new AliasesNotFoundException(alias);
}
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ public void testParseRemove() throws IOException {
String[] indices = generateRandomStringArray(10, 5, false, false);
String[] aliases = generateRandomStringArray(10, 5, false, false);
XContentBuilder b = XContentBuilder.builder(randomFrom(XContentType.values()).xContent());
boolean mustExist = randomBoolean();
b.startObject();
{
b.startObject("remove");
Expand All @@ -255,6 +256,9 @@ public void testParseRemove() throws IOException {
} else {
b.field("alias", aliases[0]);
}
if (mustExist) {
b.field("must_exist", true);
}
}
b.endObject();
}
Expand All @@ -265,6 +269,9 @@ public void testParseRemove() throws IOException {
assertEquals(AliasActions.Type.REMOVE, action.actionType());
assertThat(action.indices(), equalTo(indices));
assertThat(action.aliases(), equalTo(aliases));
if (mustExist) {
assertThat(action.mustExist(), equalTo(true));
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import org.opensearch.common.util.set.Sets;
import org.opensearch.core.index.Index;
import org.opensearch.index.IndexNotFoundException;
import org.opensearch.rest.action.admin.indices.AliasesNotFoundException;
import org.opensearch.test.OpenSearchTestCase;
import org.opensearch.test.VersionUtils;

Expand Down Expand Up @@ -164,11 +165,11 @@ public void testMustExist() {

// Show that removing non-existing alias with mustExist == true fails
final ClusterState finalCS = after;
final IllegalArgumentException iae = expectThrows(
IllegalArgumentException.class,
final AliasesNotFoundException iae = expectThrows(
AliasesNotFoundException.class,
() -> service.applyAliasActions(finalCS, singletonList(new AliasAction.Remove(index, "test_2", true)))
);
assertThat(iae.getMessage(), containsString("required alias [test_2] does not exist"));
assertThat(iae.getMessage(), containsString("aliases [test_2] missing"));
}

public void testMultipleIndices() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,11 @@ public static AliasActions randomAliasAction(boolean useStringAsFilter) {
action.isHidden(randomBoolean());
}
}
if (action.actionType() == AliasActions.Type.REMOVE) {
if (randomBoolean()) {
action.mustExist(randomBoolean());
}
}
return action;
}

Expand Down

0 comments on commit d2f644f

Please sign in to comment.