From 87b2cf9d715bf2bb4a19e28e13f4e701c9b9b555 Mon Sep 17 00:00:00 2001 From: Stevan Buzejic <30922513+stevanbz@users.noreply.github.com> Date: Fri, 30 Sep 2022 18:04:45 +0200 Subject: [PATCH] Bugfix/496 rollover alias with filters (#4499) * 496: Added filters, indexRouting and searchRouting when doing rollover on alias Signed-off-by: Stevan Buzejic * 496: Removed unused map property in metada alias rollover tests Signed-off-by: Stevan Buzejic Signed-off-by: Stevan Buzejic Co-authored-by: Stevan Buzejic --- CHANGELOG.md | 1 + .../rollover/MetadataRolloverService.java | 36 ++++- .../cluster/metadata/AliasAction.java | 12 ++ .../MetadataRolloverServiceTests.java | 147 +++++++++++++++++- 4 files changed, 187 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b24b0264d65a5..149463f04fea3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -88,6 +88,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)) ### Security - CVE-2022-25857 org.yaml:snakeyaml DOS vulnerability ([#4341](https://github.com/opensearch-project/OpenSearch/pull/4341)) diff --git a/server/src/main/java/org/opensearch/action/admin/indices/rollover/MetadataRolloverService.java b/server/src/main/java/org/opensearch/action/admin/indices/rollover/MetadataRolloverService.java index a40ac35091082..c3862bb115b21 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/rollover/MetadataRolloverService.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/rollover/MetadataRolloverService.java @@ -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()); @@ -309,20 +309,46 @@ static List 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) ) ); diff --git a/server/src/main/java/org/opensearch/cluster/metadata/AliasAction.java b/server/src/main/java/org/opensearch/cluster/metadata/AliasAction.java index a53f8411b2549..46702a0d78caf 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/AliasAction.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/AliasAction.java @@ -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; } diff --git a/server/src/test/java/org/opensearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java b/server/src/test/java/org/opensearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java index d54c17041e96f..8521673743d23 100644 --- a/server/src/test/java/org/opensearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java +++ b/server/src/test/java/org/opensearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java @@ -126,7 +126,13 @@ public void testRolloverAliasActions() { String sourceIndex = randomAlphaOfLength(10); String targetIndex = randomAlphaOfLength(10); - List actions = MetadataRolloverService.rolloverAliasToNewIndex(sourceIndex, targetIndex, false, null, sourceAlias); + List actions = MetadataRolloverService.rolloverAliasToNewIndex( + sourceIndex, + targetIndex, + false, + createDefaultAliasMetadata(sourceAlias, null), + sourceAlias + ); assertThat(actions, hasSize(2)); boolean foundAdd = false; boolean foundRemove = false; @@ -149,7 +155,13 @@ public void testRolloverAliasActionsWithExplicitWriteIndex() { String sourceAlias = randomAlphaOfLength(10); String sourceIndex = randomAlphaOfLength(10); String targetIndex = randomAlphaOfLength(10); - List actions = MetadataRolloverService.rolloverAliasToNewIndex(sourceIndex, targetIndex, true, null, sourceAlias); + List actions = MetadataRolloverService.rolloverAliasToNewIndex( + sourceIndex, + targetIndex, + true, + createDefaultAliasMetadata(sourceAlias, null), + sourceAlias + ); assertThat(actions, hasSize(2)); boolean foundAddWrite = false; @@ -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 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 actions = MetadataRolloverService.rolloverAliasToNewIndex(sourceIndex, targetIndex, true, true, sourceAlias); + List actions = MetadataRolloverService.rolloverAliasToNewIndex( + sourceIndex, + targetIndex, + true, + createDefaultAliasMetadata(sourceAlias, true), + sourceAlias + ); assertThat(actions, hasSize(2)); boolean foundAddWrite = false; @@ -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 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 actions = MetadataRolloverService.rolloverAliasToNewIndex(sourceIndex, targetIndex, false, true, sourceAlias); + List actions = MetadataRolloverService.rolloverAliasToNewIndex( + sourceIndex, + targetIndex, + false, + createDefaultAliasMetadata(sourceAlias, true), + sourceAlias + ); assertThat(actions, hasSize(2)); boolean foundAddWrite = false; @@ -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(); + } }