Skip to content
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.

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

Bugfix/496 rollover alias with filters #4499

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [Segment Replication] Ignore lock file when testing cleanupAndPreserveLatestCommitPoint ([#4544](https://github.com/opensearch-project/OpenSearch/pull/4544))
- Updated jackson to 2.13.4 and snakeyml to 1.32 ([#4556](https://github.com/opensearch-project/OpenSearch/pull/4556))
- [Bug]: Fixed invalid location of JDK dependency for arm64 architecture([#4613](https://github.com/opensearch-project/OpenSearch/pull/4613))
- [Bug]: Alias filter lost after rollover ([#4499](https://github.com/opensearch-project/OpenSearch/pull/4499))

Copy link
Member

Choose a reason for hiding this comment

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

Leave the extra line please?

### Security
- CVE-2022-25857 org.yaml:snakeyaml DOS vulnerability ([#4341](https://github.com/opensearch-project/OpenSearch/pull/4341))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ private RolloverResult rolloverAlias(
ClusterState newState = createIndexService.applyCreateIndexRequest(currentState, createIndexClusterStateRequest, silent);
newState = indexAliasesService.applyAliasActions(
newState,
rolloverAliasToNewIndex(sourceIndexName, rolloverIndexName, explicitWriteIndex, aliasMetadata.isHidden(), aliasName)
rolloverAliasToNewIndex(sourceIndexName, rolloverIndexName, explicitWriteIndex, aliasMetadata, aliasName)
);

RolloverInfo rolloverInfo = new RolloverInfo(aliasName, metConditions, threadPool.absoluteTimeInMillis());
Expand Down Expand Up @@ -309,20 +309,46 @@ static List<AliasAction> rolloverAliasToNewIndex(
String oldIndex,
String newIndex,
boolean explicitWriteIndex,
@Nullable Boolean isHidden,
AliasMetadata aliasMetadata,
String alias
) {
String filterAsString = aliasMetadata.getFilter() != null ? aliasMetadata.getFilter().string() : null;

if (explicitWriteIndex) {
return Collections.unmodifiableList(
Arrays.asList(
new AliasAction.Add(newIndex, alias, null, null, null, true, isHidden),
new AliasAction.Add(oldIndex, alias, null, null, null, false, isHidden)
new AliasAction.Add(
newIndex,
alias,
filterAsString,
aliasMetadata.getIndexRouting(),
aliasMetadata.getSearchRouting(),
true,
aliasMetadata.isHidden()
),
new AliasAction.Add(
oldIndex,
alias,
filterAsString,
aliasMetadata.getIndexRouting(),
aliasMetadata.getSearchRouting(),
false,
aliasMetadata.isHidden()
)
)
);
} else {
return Collections.unmodifiableList(
Arrays.asList(
new AliasAction.Add(newIndex, alias, null, null, null, null, isHidden),
new AliasAction.Add(
newIndex,
alias,
filterAsString,
aliasMetadata.getIndexRouting(),
aliasMetadata.getSearchRouting(),
null,
aliasMetadata.isHidden()
),
new AliasAction.Remove(oldIndex, alias, null)
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,18 @@ public String getAlias() {
return alias;
}

public String getFilter() {
return filter;
}

public String getSearchRouting() {
return searchRouting;
}

public String getIndexRouting() {
return indexRouting;
}

public Boolean writeIndex() {
return writeIndex;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,13 @@ public void testRolloverAliasActions() {
String sourceIndex = randomAlphaOfLength(10);
String targetIndex = randomAlphaOfLength(10);

List<AliasAction> actions = MetadataRolloverService.rolloverAliasToNewIndex(sourceIndex, targetIndex, false, null, sourceAlias);
List<AliasAction> actions = MetadataRolloverService.rolloverAliasToNewIndex(
sourceIndex,
targetIndex,
false,
createDefaultAliasMetadata(sourceAlias, null),
sourceAlias
);
assertThat(actions, hasSize(2));
boolean foundAdd = false;
boolean foundRemove = false;
Expand All @@ -149,7 +155,13 @@ public void testRolloverAliasActionsWithExplicitWriteIndex() {
String sourceAlias = randomAlphaOfLength(10);
String sourceIndex = randomAlphaOfLength(10);
String targetIndex = randomAlphaOfLength(10);
List<AliasAction> actions = MetadataRolloverService.rolloverAliasToNewIndex(sourceIndex, targetIndex, true, null, sourceAlias);
List<AliasAction> actions = MetadataRolloverService.rolloverAliasToNewIndex(
sourceIndex,
targetIndex,
true,
createDefaultAliasMetadata(sourceAlias, null),
sourceAlias
);

assertThat(actions, hasSize(2));
boolean foundAddWrite = false;
Expand All @@ -172,11 +184,64 @@ public void testRolloverAliasActionsWithExplicitWriteIndex() {
assertTrue(foundRemoveWrite);
}

public void testRolloverAliasActionsWithFilterAndExplicitWriteIndex() {
String sourceAlias = randomAlphaOfLength(10);
String sourceIndex = randomAlphaOfLength(10);
String targetIndex = randomAlphaOfLength(10);
String indexRouting = randomAlphaOfLength(10);
String sourceRouting = randomAlphaOfLength(10);
AliasMetadata aliasMetadata = createAliasMetadata(
sourceAlias,
Collections.singletonMap(randomAlphaOfLength(2), randomAlphaOfLength(2)),
indexRouting,
sourceRouting,
true
);

List<AliasAction> actions = MetadataRolloverService.rolloverAliasToNewIndex(
sourceIndex,
targetIndex,
true,
aliasMetadata,
sourceAlias
);

assertThat(actions, hasSize(2));
boolean foundAddWrite = false;
boolean foundRemoveWrite = false;
for (AliasAction action : actions) {
AliasAction.Add addAction = (AliasAction.Add) action;
if (action.getIndex().equals(targetIndex)) {
assertEquals(sourceAlias, addAction.getAlias());
assertEquals(aliasMetadata.filter().string(), addAction.getFilter());
assertEquals(indexRouting, addAction.getIndexRouting());
assertEquals(sourceRouting, addAction.getSearchRouting());

assertTrue(addAction.writeIndex());
foundAddWrite = true;
} else if (action.getIndex().equals(sourceIndex)) {
assertEquals(sourceAlias, addAction.getAlias());
assertFalse(addAction.writeIndex());
foundRemoveWrite = true;
} else {
throw new AssertionError("Unknown index [" + action.getIndex() + "]");
}
}
assertTrue(foundAddWrite);
assertTrue(foundRemoveWrite);
}

public void testRolloverAliasActionsWithHiddenAliasAndExplicitWriteIndex() {
String sourceAlias = randomAlphaOfLength(10);
String sourceIndex = randomAlphaOfLength(10);
String targetIndex = randomAlphaOfLength(10);
List<AliasAction> actions = MetadataRolloverService.rolloverAliasToNewIndex(sourceIndex, targetIndex, true, true, sourceAlias);
List<AliasAction> actions = MetadataRolloverService.rolloverAliasToNewIndex(
sourceIndex,
targetIndex,
true,
createDefaultAliasMetadata(sourceAlias, true),
sourceAlias
);

assertThat(actions, hasSize(2));
boolean foundAddWrite = false;
Expand All @@ -202,11 +267,66 @@ public void testRolloverAliasActionsWithHiddenAliasAndExplicitWriteIndex() {
assertTrue(foundRemoveWrite);
}

public void testRolloverAliasActionsWithFilterAndHiddenAliasAndImplicitWriteIndex() {
String sourceAlias = randomAlphaOfLength(10);
String sourceIndex = randomAlphaOfLength(10);
String targetIndex = randomAlphaOfLength(10);
String indexRouting = randomAlphaOfLength(10);
String sourceRouting = randomAlphaOfLength(10);
AliasMetadata aliasMetadata = createAliasMetadata(
sourceAlias,
Collections.singletonMap(randomAlphaOfLength(2), randomAlphaOfLength(2)),
indexRouting,
sourceRouting,
true
);

List<AliasAction> actions = MetadataRolloverService.rolloverAliasToNewIndex(
sourceIndex,
targetIndex,
false,
aliasMetadata,
sourceAlias
);

assertThat(actions, hasSize(2));
boolean foundAddWrite = false;
boolean foundRemoveWrite = false;
for (AliasAction action : actions) {
if (action.getIndex().equals(targetIndex)) {
assertThat(action, instanceOf(AliasAction.Add.class));
AliasAction.Add addAction = (AliasAction.Add) action;
assertEquals(sourceAlias, addAction.getAlias());
assertThat(addAction.writeIndex(), nullValue());
assertTrue(addAction.isHidden());
assertEquals(aliasMetadata.filter().string(), addAction.getFilter());
assertEquals(indexRouting, addAction.getIndexRouting());
assertEquals(sourceRouting, addAction.getSearchRouting());
foundAddWrite = true;
} else if (action.getIndex().equals(sourceIndex)) {
assertThat(action, instanceOf(AliasAction.Remove.class));
AliasAction.Remove removeAction = (AliasAction.Remove) action;
assertEquals(sourceAlias, removeAction.getAlias());
foundRemoveWrite = true;
} else {
throw new AssertionError("Unknown index [" + action.getIndex() + "]");
}
}
assertTrue(foundAddWrite);
assertTrue(foundRemoveWrite);
}

public void testRolloverAliasActionsWithHiddenAliasAndImplicitWriteIndex() {
String sourceAlias = randomAlphaOfLength(10);
String sourceIndex = randomAlphaOfLength(10);
String targetIndex = randomAlphaOfLength(10);
List<AliasAction> actions = MetadataRolloverService.rolloverAliasToNewIndex(sourceIndex, targetIndex, false, true, sourceAlias);
List<AliasAction> actions = MetadataRolloverService.rolloverAliasToNewIndex(
sourceIndex,
targetIndex,
false,
createDefaultAliasMetadata(sourceAlias, true),
sourceAlias
);

assertThat(actions, hasSize(2));
boolean foundAddWrite = false;
Expand Down Expand Up @@ -1010,4 +1130,23 @@ private static IndexMetadata createMetadata(String indexName) {
.settings(settings)
.build();
}

private static AliasMetadata createDefaultAliasMetadata(String alias, Boolean isHidden) {
return AliasMetadata.builder(alias).isHidden(isHidden).build();
}

private static AliasMetadata createAliasMetadata(
String alias,
Map filter,
String indexRouting,
String searchRouting,
Boolean isHidden
) {
return AliasMetadata.builder(alias)
.isHidden(isHidden)
.filter(filter)
.indexRouting(indexRouting)
.searchRouting(searchRouting)
.build();
}
}