Skip to content

Commit

Permalink
Add new flag to check whether alias exists on remove (#58100)
Browse files Browse the repository at this point in the history
This allows doing true CAS operations on aliases, making sure that an alias is actually properly
moved from a given source index onto a given target index. This is useful to ensure that an
alias is actually moved from a given index to another one, and not just added to another index.
  • Loading branch information
ywelsch authored Jun 18, 2020
1 parent 1f6c953 commit b305454
Show file tree
Hide file tree
Showing 7 changed files with 90 additions and 6 deletions.
4 changes: 4 additions & 0 deletions docs/reference/indices/aliases.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@ unless overriden in the request using the `expand_wildcards` parameter,
similar to <<index-hidden,hidden indices>>. This property must be set to the
same value on all indices that share an alias. Defaults to `false`.

`must_exist`::
(Optional, boolean)
If `true`, the alias to remove must exist. Defaults to `false`.

`is_write_index`::
(Optional, boolean)
If `true`, assigns the index as an alias's write index.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ public static class AliasActions implements AliasesRequest, Writeable, ToXConten
private static final ParseField SEARCH_ROUTING = new ParseField("search_routing", "searchRouting", "search-routing");
private static final ParseField IS_WRITE_INDEX = new ParseField("is_write_index");
private static final ParseField IS_HIDDEN = new ParseField("is_hidden");
private static final ParseField MUST_EXIST = new ParseField("must_exist");

private static final ParseField ADD = new ParseField("add");
private static final ParseField REMOVE = new ParseField("remove");
Expand Down Expand Up @@ -195,6 +196,7 @@ 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);
private static final ObjectParser<AliasActions, Void> REMOVE_INDEX_PARSER = parser(REMOVE_INDEX.getPreferredName(),
Expand Down Expand Up @@ -234,6 +236,7 @@ private static ObjectParser<AliasActions, Void> parser(String name, Supplier<Ali
private String searchRouting;
private Boolean writeIndex;
private Boolean isHidden;
private Boolean mustExist;

public AliasActions(AliasActions.Type type) {
this.type = type;
Expand All @@ -255,6 +258,11 @@ public AliasActions(StreamInput in) throws IOException {
isHidden = in.readOptionalBoolean();
}
originalAliases = in.readStringArray();
if (in.getVersion().onOrAfter(Version.V_8_0_0)) {
mustExist = in.readOptionalBoolean();
} else {
mustExist = null;
}
}

@Override
Expand All @@ -271,6 +279,9 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeOptionalBoolean(isHidden);
}
out.writeStringArray(originalAliases);
if (out.getVersion().onOrAfter(Version.V_8_0_0)) {
out.writeOptionalBoolean(mustExist);
}
}

/**
Expand Down Expand Up @@ -455,6 +466,18 @@ public Boolean isHidden() {
return isHidden;
}

public AliasActions mustExist(Boolean mustExist) {
if (type != Type.REMOVE) {
throw new IllegalArgumentException("[" + MUST_EXIST.getPreferredName() + "] is unsupported for [" + type + "]");
}
this.mustExist = mustExist;
return this;
}

public Boolean mustExist() {
return mustExist;
}

@Override
public String[] aliases() {
return aliases;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ protected void masterOperation(Task task, final IndicesAliasesRequest request, f
break;
case REMOVE:
for (String alias : concreteAliases(action, state.metadata(), index.getName())) {
finalActions.add(new AliasAction.Remove(index.getName(), alias));
finalActions.add(new AliasAction.Remove(index.getName(), alias, action.mustExist()));
}
break;
case REMOVE_INDEX:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ static List<AliasAction> rolloverAliasToNewIndex(String oldIndex, String newInde
} else {
return List.of(
new AliasAction.Add(newIndex, alias, null, null, null, null, isHidden),
new AliasAction.Remove(oldIndex, alias));
new AliasAction.Remove(oldIndex, alias, null));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,16 +149,19 @@ boolean apply(NewAliasValidator aliasValidator, Metadata.Builder metadata, Index
*/
public static class Remove extends AliasAction {
private final String alias;
@Nullable
private final Boolean mustExist;

/**
* Build the operation.
*/
public Remove(String index, String alias) {
public Remove(String index, String alias, @Nullable Boolean mustExist) {
super(index);
if (false == Strings.hasText(alias)) {
throw new IllegalArgumentException("[alias] is required");
}
this.alias = alias;
this.mustExist = mustExist;
}

/**
Expand All @@ -176,6 +179,9 @@ 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");
}
return false;
}
metadata.put(IndexMetadata.builder(index).removeAlias(alias));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,17 @@ public void testBadOptionsInNonIndex() {
assertEquals("[filter] is unsupported for [" + action.actionType() + "]", e.getMessage());
}

public void testMustExistOption() {
final boolean mustExist = randomBoolean();
AliasActions removeAliasAction = AliasActions.remove();
assertNull(removeAliasAction.mustExist());
removeAliasAction.mustExist(mustExist);
assertEquals(mustExist, removeAliasAction.mustExist());
AliasActions action = randomBoolean() ? AliasActions.add() : AliasActions.removeIndex();
Exception e = expectThrows(IllegalArgumentException.class, () -> action.mustExist(mustExist));
assertEquals("[must_exist] is unsupported for [" + action.actionType() + "]", e.getMessage());
}

public void testParseAdd() throws IOException {
String[] indices = generateRandomStringArray(10, 5, false, false);
String[] aliases = generateRandomStringArray(10, 5, false, false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public void testAddAndRemove() {
// Remove the alias from it while adding another one
before = after;
after = service.applyAliasActions(before, Arrays.asList(
new AliasAction.Remove(index, "test"),
new AliasAction.Remove(index, "test", null),
new AliasAction.Add(index, "test_2", null, null, null, null, null)));
assertNull(after.metadata().getIndicesLookup().get("test"));
alias = after.metadata().getIndicesLookup().get("test_2");
Expand All @@ -95,12 +95,52 @@ public void testAddAndRemove() {

// Now just remove on its own
before = after;
after = service.applyAliasActions(before, singletonList(new AliasAction.Remove(index, "test_2")));
after = service.applyAliasActions(before, singletonList(new AliasAction.Remove(index, "test_2", randomBoolean())));
assertNull(after.metadata().getIndicesLookup().get("test"));
assertNull(after.metadata().getIndicesLookup().get("test_2"));
assertAliasesVersionIncreased(index, before, after);
}

public void testMustExist() {
// Create a state with a single index
String index = randomAlphaOfLength(5);
ClusterState before = createIndex(ClusterState.builder(ClusterName.DEFAULT).build(), index);

// Add an alias to it
ClusterState after = service.applyAliasActions(before, singletonList(new AliasAction.Add(index, "test", null, null, null, null,
null)));
IndexAbstraction alias = after.metadata().getIndicesLookup().get("test");
assertNotNull(alias);
assertThat(alias.getType(), equalTo(IndexAbstraction.Type.ALIAS));
assertThat(alias.getIndices(), contains(after.metadata().index(index)));
assertAliasesVersionIncreased(index, before, after);

// Remove the alias from it with mustExist == true while adding another one
before = after;
after = service.applyAliasActions(before, Arrays.asList(
new AliasAction.Remove(index, "test", true),
new AliasAction.Add(index, "test_2", null, null, null, null, null)));
assertNull(after.metadata().getIndicesLookup().get("test"));
alias = after.metadata().getIndicesLookup().get("test_2");
assertNotNull(alias);
assertThat(alias.getType(), equalTo(IndexAbstraction.Type.ALIAS));
assertThat(alias.getIndices(), contains(after.metadata().index(index)));
assertAliasesVersionIncreased(index, before, after);

// Now just remove on its own
before = after;
after = service.applyAliasActions(before, singletonList(new AliasAction.Remove(index, "test_2", randomBoolean())));
assertNull(after.metadata().getIndicesLookup().get("test"));
assertNull(after.metadata().getIndicesLookup().get("test_2"));
assertAliasesVersionIncreased(index, before, after);

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

public void testMultipleIndices() {
final var length = randomIntBetween(2, 8);
final var indices = new HashSet<String>(length);
Expand Down Expand Up @@ -183,7 +223,7 @@ public void testAliasesVersionUnchangedWhenActionsAreIdempotent() {
// now perform a remove and add for each alias which is idempotent, the resulting aliases are unchanged
final var removeAndAddActions = new ArrayList<AliasAction>(2 * length);
for (final var aliasName : aliasNames) {
removeAndAddActions.add(new AliasAction.Remove(index, aliasName));
removeAndAddActions.add(new AliasAction.Remove(index, aliasName, null));
removeAndAddActions.add(new AliasAction.Add(index, aliasName, null, null, null, null, null));
}
final ClusterState afterRemoveAndAddAlias = service.applyAliasActions(afterAddingAlias, removeAndAddActions);
Expand Down

0 comments on commit b305454

Please sign in to comment.