Skip to content

Add new flag to check whether alias exists on remove #58100

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 18, 2020
Merged

Add new flag to check whether alias exists on remove #58100

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2c14595
Add new flag to check whether alias exists on remove
ywelsch Jun 15, 2020
4917987
add unit test
ywelsch Jun 17, 2020
23c8dfd
extra test
ywelsch Jun 17, 2020
031f6a4
Merge remote-tracking branch 'elastic/master' into alias-must-exist
ywelsch Jun 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/reference/indices/aliases.asciidoc
View file
Open in desktop
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`::
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not sure about the naming here. Initially I went with "required", but I felt that that was a bit too generic, so opted for a word where it's at least clear what is meant.

(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
23 changes: 23 additions & 0 deletions server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion ...main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion ...rc/main/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,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
8 changes: 7 additions & 1 deletion server/src/main/java/org/elasticsearch/cluster/metadata/AliasAction.java
View file
Open in desktop
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
11 changes: 8 additions & 3 deletions ...er/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesServiceTests.java
View file
Open in desktop
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,10 +95,15 @@ 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);

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() {
Expand Down Expand Up @@ -183,7 +188,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