From 590719e6c68124f4e1d26d023d9a2c58c1e84f2d Mon Sep 17 00:00:00 2001 From: Stevan Buzejic Date: Mon, 12 Sep 2022 20:27:35 +0200 Subject: [PATCH] 496: Removed unused map property in metada alias rollover tests Signed-off-by: Stevan Buzejic --- CHANGELOG.md | 1 + .../rollover/MetadataRolloverService.java | 36 ++++++-- .../cluster/metadata/AliasAction.java | 12 ++- .../cluster/metadata/AliasMetadata.java | 7 -- .../MetadataRolloverServiceTests.java | 83 +++++++++++++++---- 5 files changed, 107 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d241a94a0dcb3..128700160e505 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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)) ### 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 5fda6e087562f..22e4342c4413d 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 @@ -312,17 +312,43 @@ static List rolloverAliasToNewIndex( AliasMetadata aliasMetada, String alias ) { + String filterAsString = aliasMetada.getFilter() != null ? aliasMetada.getFilter().string() : null; + if (explicitWriteIndex) { return Collections.unmodifiableList( Arrays.asList( - new AliasAction.Add(newIndex, alias, aliasMetada.getFilterAsString(), aliasMetada.getIndexRouting(), aliasMetada.getSearchRouting(), true, aliasMetada.isHidden()), - new AliasAction.Add(oldIndex, alias, aliasMetada.getFilterAsString(), aliasMetada.getIndexRouting(), aliasMetada.getSearchRouting(), false, aliasMetada.isHidden()) + new AliasAction.Add( + newIndex, + alias, + filterAsString, + aliasMetada.getIndexRouting(), + aliasMetada.getSearchRouting(), + true, + aliasMetada.isHidden() + ), + new AliasAction.Add( + oldIndex, + alias, + filterAsString, + aliasMetada.getIndexRouting(), + aliasMetada.getSearchRouting(), + false, + aliasMetada.isHidden() + ) ) ); } else { return Collections.unmodifiableList( Arrays.asList( - new AliasAction.Add(newIndex, alias, aliasMetada.getFilterAsString(), aliasMetada.getIndexRouting(), aliasMetada.getSearchRouting(), null, aliasMetada.isHidden()), + new AliasAction.Add( + newIndex, + alias, + filterAsString, + aliasMetada.getIndexRouting(), + aliasMetada.getSearchRouting(), + null, + aliasMetada.isHidden() + ), new AliasAction.Remove(oldIndex, alias, null) ) ); @@ -385,8 +411,8 @@ static void validate(Metadata metadata, String rolloverTarget, String newIndexNa + indexAbstraction.getType().getDisplayName() + "] but one of [" + Strings.collectionToCommaDelimitedString( - VALID_ROLLOVER_TARGETS.stream().map(IndexAbstraction.Type::getDisplayName).collect(Collectors.toList()) - ) + VALID_ROLLOVER_TARGETS.stream().map(IndexAbstraction.Type::getDisplayName).collect(Collectors.toList()) + ) + "] was expected" ); } 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 19b3ba81b2e3d..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,11 +138,17 @@ public String getAlias() { return alias; } - public String getFilter(){ return filter; } + public String getFilter() { + return filter; + } - public String getSearchRouting() { return searchRouting; } + public String getSearchRouting() { + return searchRouting; + } - public String getIndexRouting() { return indexRouting; } + public String getIndexRouting() { + return indexRouting; + } public Boolean writeIndex() { return writeIndex; diff --git a/server/src/main/java/org/opensearch/cluster/metadata/AliasMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/AliasMetadata.java index 7a065258c99c4..dc22af42ac801 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/AliasMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/AliasMetadata.java @@ -129,13 +129,6 @@ public CompressedXContent getFilter() { return filter(); } - public String getFilterAsString() { - if(filter == null){ - return null; - } - return filter.string(); - } - public boolean filteringRequired() { return filter != null; } 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 784aed093df2d..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 @@ -43,7 +43,6 @@ import org.opensearch.cluster.DataStreamTestHelper; import org.opensearch.cluster.metadata.AliasAction; import org.opensearch.cluster.metadata.AliasMetadata; -import org.opensearch.cluster.metadata.AliasMetadata.Builder; import org.opensearch.cluster.metadata.AliasValidator; import org.opensearch.cluster.metadata.ComponentTemplate; import org.opensearch.cluster.metadata.ComposableIndexTemplate; @@ -101,7 +100,6 @@ import java.util.Map; import static java.util.Collections.emptyMap; -import static org.hamcrest.Matchers.in; import static org.opensearch.cluster.DataStreamTestHelper.generateMapping; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; @@ -128,7 +126,13 @@ public void testRolloverAliasActions() { String sourceIndex = randomAlphaOfLength(10); String targetIndex = randomAlphaOfLength(10); - List actions = MetadataRolloverService.rolloverAliasToNewIndex(sourceIndex, targetIndex, false, createDefaultAliasMetadata(sourceAlias, null), sourceAlias); + List actions = MetadataRolloverService.rolloverAliasToNewIndex( + sourceIndex, + targetIndex, + false, + createDefaultAliasMetadata(sourceAlias, null), + sourceAlias + ); assertThat(actions, hasSize(2)); boolean foundAdd = false; boolean foundRemove = false; @@ -151,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, createDefaultAliasMetadata(sourceAlias, null), sourceAlias); + List actions = MetadataRolloverService.rolloverAliasToNewIndex( + sourceIndex, + targetIndex, + true, + createDefaultAliasMetadata(sourceAlias, null), + sourceAlias + ); assertThat(actions, hasSize(2)); boolean foundAddWrite = false; @@ -178,12 +188,23 @@ public void testRolloverAliasActionsWithFilterAndExplicitWriteIndex() { String sourceAlias = randomAlphaOfLength(10); String sourceIndex = randomAlphaOfLength(10); String targetIndex = randomAlphaOfLength(10); - Map filter = Collections.singletonMap(randomAlphaOfLength(2), randomAlphaOfLength(2)); String indexRouting = randomAlphaOfLength(10); String sourceRouting = randomAlphaOfLength(10); - AliasMetadata aliasMetadata = createAliasMetadata(sourceAlias, filter, indexRouting, sourceRouting, true); + AliasMetadata aliasMetadata = createAliasMetadata( + sourceAlias, + Collections.singletonMap(randomAlphaOfLength(2), randomAlphaOfLength(2)), + indexRouting, + sourceRouting, + true + ); - List actions = MetadataRolloverService.rolloverAliasToNewIndex(sourceIndex, targetIndex, true, aliasMetadata, sourceAlias); + List actions = MetadataRolloverService.rolloverAliasToNewIndex( + sourceIndex, + targetIndex, + true, + aliasMetadata, + sourceAlias + ); assertThat(actions, hasSize(2)); boolean foundAddWrite = false; @@ -192,7 +213,7 @@ public void testRolloverAliasActionsWithFilterAndExplicitWriteIndex() { AliasAction.Add addAction = (AliasAction.Add) action; if (action.getIndex().equals(targetIndex)) { assertEquals(sourceAlias, addAction.getAlias()); - assertEquals(aliasMetadata.getFilterAsString(), addAction.getFilter()); + assertEquals(aliasMetadata.filter().string(), addAction.getFilter()); assertEquals(indexRouting, addAction.getIndexRouting()); assertEquals(sourceRouting, addAction.getSearchRouting()); @@ -214,7 +235,13 @@ public void testRolloverAliasActionsWithHiddenAliasAndExplicitWriteIndex() { String sourceAlias = randomAlphaOfLength(10); String sourceIndex = randomAlphaOfLength(10); String targetIndex = randomAlphaOfLength(10); - List actions = MetadataRolloverService.rolloverAliasToNewIndex(sourceIndex, targetIndex, true, createDefaultAliasMetadata(sourceAlias,true), sourceAlias); + List actions = MetadataRolloverService.rolloverAliasToNewIndex( + sourceIndex, + targetIndex, + true, + createDefaultAliasMetadata(sourceAlias, true), + sourceAlias + ); assertThat(actions, hasSize(2)); boolean foundAddWrite = false; @@ -244,12 +271,23 @@ public void testRolloverAliasActionsWithFilterAndHiddenAliasAndImplicitWriteInde String sourceAlias = randomAlphaOfLength(10); String sourceIndex = randomAlphaOfLength(10); String targetIndex = randomAlphaOfLength(10); - Map filter = Collections.singletonMap(randomAlphaOfLength(2), randomAlphaOfLength(2)); String indexRouting = randomAlphaOfLength(10); String sourceRouting = randomAlphaOfLength(10); - AliasMetadata aliasMetadata = createAliasMetadata(sourceAlias, filter, indexRouting, sourceRouting, true); + AliasMetadata aliasMetadata = createAliasMetadata( + sourceAlias, + Collections.singletonMap(randomAlphaOfLength(2), randomAlphaOfLength(2)), + indexRouting, + sourceRouting, + true + ); - List actions = MetadataRolloverService.rolloverAliasToNewIndex(sourceIndex, targetIndex, false, aliasMetadata, sourceAlias); + List actions = MetadataRolloverService.rolloverAliasToNewIndex( + sourceIndex, + targetIndex, + false, + aliasMetadata, + sourceAlias + ); assertThat(actions, hasSize(2)); boolean foundAddWrite = false; @@ -261,7 +299,7 @@ public void testRolloverAliasActionsWithFilterAndHiddenAliasAndImplicitWriteInde assertEquals(sourceAlias, addAction.getAlias()); assertThat(addAction.writeIndex(), nullValue()); assertTrue(addAction.isHidden()); - assertEquals(aliasMetadata.getFilterAsString(), addAction.getFilter()); + assertEquals(aliasMetadata.filter().string(), addAction.getFilter()); assertEquals(indexRouting, addAction.getIndexRouting()); assertEquals(sourceRouting, addAction.getSearchRouting()); foundAddWrite = true; @@ -282,7 +320,13 @@ public void testRolloverAliasActionsWithHiddenAliasAndImplicitWriteIndex() { String sourceAlias = randomAlphaOfLength(10); String sourceIndex = randomAlphaOfLength(10); String targetIndex = randomAlphaOfLength(10); - List actions = MetadataRolloverService.rolloverAliasToNewIndex(sourceIndex, targetIndex, false, createDefaultAliasMetadata(sourceAlias,true), sourceAlias); + List actions = MetadataRolloverService.rolloverAliasToNewIndex( + sourceIndex, + targetIndex, + false, + createDefaultAliasMetadata(sourceAlias, true), + sourceAlias + ); assertThat(actions, hasSize(2)); boolean foundAddWrite = false; @@ -1091,9 +1135,14 @@ private static AliasMetadata createDefaultAliasMetadata(String alias, Boolean is 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) + private static AliasMetadata createAliasMetadata( + String alias, + Map filter, + String indexRouting, + String searchRouting, + Boolean isHidden + ) { + return AliasMetadata.builder(alias) .isHidden(isHidden) .filter(filter) .indexRouting(indexRouting)