From 7a127d97e302aa96662dfc4f9202b88ef96313a0 Mon Sep 17 00:00:00 2001 From: Stephen Crawford Date: Thu, 22 Dec 2022 17:10:26 -0500 Subject: [PATCH 01/12] Added test cases Signed-off-by: Stephen Crawford --- .../opensearch/snapshots/SnapshotUtils.java | 27 ++++++++++++------- .../snapshots/SnapshotUtilsTests.java | 2 ++ 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java b/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java index 3ef3523961df8..26ed0667936a0 100644 --- a/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java +++ b/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java @@ -41,14 +41,9 @@ import org.opensearch.index.IndexNotFoundException; import org.opensearch.index.IndexSettings; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashSet; -import java.util.List; -import java.util.Set; -import java.util.HashMap; -import java.util.Map; +import java.text.ParseException; +import java.text.RuleBasedCollator; +import java.util.*; /** * Snapshot utilities @@ -69,6 +64,8 @@ public static List filterIndices(List availableIndices, String[] if (IndexNameExpressionResolver.isAllIndices(Arrays.asList(selectedIndices))) { return availableIndices; } + String[] orderedIndices = + Set result = null; for (int i = 0; i < selectedIndices.length; i++) { String indexOrPattern = selectedIndices[i]; @@ -89,8 +86,8 @@ public static List filterIndices(List availableIndices, String[] result = new HashSet<>(); } } else if (indexOrPattern.charAt(0) == '-') { - // if its the first, fill it with all the indices... - if (i == 0) { + // if its the first index pattern, fill it with all the indices... + if (i == 0 && selectedIndices.length == 1) { result = new HashSet<>(availableIndices); } add = false; @@ -178,3 +175,13 @@ public static void validateSnapshotsBackingAnyIndex( } } } + +public class IndexPatternComparator implements Comparator { + + @Override + public int compare(String index1, String index2) { + + return .compare(firstPlayer.getRanking(), secondPlayer.getRanking()); + } + +} diff --git a/server/src/test/java/org/opensearch/snapshots/SnapshotUtilsTests.java b/server/src/test/java/org/opensearch/snapshots/SnapshotUtilsTests.java index 8dae5026a18bc..365bc033d58fb 100644 --- a/server/src/test/java/org/opensearch/snapshots/SnapshotUtilsTests.java +++ b/server/src/test/java/org/opensearch/snapshots/SnapshotUtilsTests.java @@ -65,6 +65,8 @@ public void testIndexNameFiltering() { assertIndexNameFiltering(new String[] { "foo", "bar", "baz" }, new String[] { "-ba*" }, new String[] { "foo" }); assertIndexNameFiltering(new String[] { "foo", "bar", "baz" }, new String[] { "+ba*" }, new String[] { "bar", "baz" }); assertIndexNameFiltering(new String[] { "foo", "bar", "baz" }, new String[] { "+bar", "+foo" }, new String[] { "bar", "foo" }); + assertIndexNameFiltering(new String[] { "foo", "bar", "baz" }, new String[] { "-bar", "b*" }, new String[] { "baz" }); + assertIndexNameFiltering(new String[] { "foo", "bar", "baz" }, new String[] { "b*", "-bar" }, new String[] { "baz" }); assertIndexNameFiltering( new String[] { "foo", "bar", "baz" }, new String[] { "zzz", "bar" }, From 827870c531a1c608cc1d4a01802275f25a5e50ba Mon Sep 17 00:00:00 2001 From: Stephen Crawford Date: Fri, 23 Dec 2022 09:20:10 -0500 Subject: [PATCH 02/12] reset snapshot utils file Signed-off-by: Stephen Crawford --- .../src/main/java/org/opensearch/snapshots/SnapshotUtils.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java b/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java index 26ed0667936a0..5737ce79159e8 100644 --- a/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java +++ b/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java @@ -41,8 +41,6 @@ import org.opensearch.index.IndexNotFoundException; import org.opensearch.index.IndexSettings; -import java.text.ParseException; -import java.text.RuleBasedCollator; import java.util.*; /** @@ -64,7 +62,6 @@ public static List filterIndices(List availableIndices, String[] if (IndexNameExpressionResolver.isAllIndices(Arrays.asList(selectedIndices))) { return availableIndices; } - String[] orderedIndices = Set result = null; for (int i = 0; i < selectedIndices.length; i++) { From 6a63feb4c06d35e58ea9f7af241712849c4735cd Mon Sep 17 00:00:00 2001 From: Stephen Crawford Date: Fri, 23 Dec 2022 10:02:30 -0500 Subject: [PATCH 03/12] Add comparator Signed-off-by: Stephen Crawford --- .../opensearch/snapshots/SnapshotUtils.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java b/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java index 5737ce79159e8..be3bb1d25a825 100644 --- a/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java +++ b/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java @@ -63,6 +63,15 @@ public static List filterIndices(List availableIndices, String[] return availableIndices; } + Arrays.sort(selectedIndices, (o1, o2) -> { + char o1FirstChar = o1.charAt(0); + char o2FirstChar = o2.charAt(0); + if (o1FirstChar == '-' && o2FirstChar != '-') { // Make '-' lower priority then everything + return 1; + } + return o1.compareTo(o2); + }); + Set result = null; for (int i = 0; i < selectedIndices.length; i++) { String indexOrPattern = selectedIndices[i]; @@ -173,12 +182,3 @@ public static void validateSnapshotsBackingAnyIndex( } } -public class IndexPatternComparator implements Comparator { - - @Override - public int compare(String index1, String index2) { - - return .compare(firstPlayer.getRanking(), secondPlayer.getRanking()); - } - -} From a9a9c1417c7eca659bf69889518d95691a3917e5 Mon Sep 17 00:00:00 2001 From: Stephen Crawford Date: Fri, 23 Dec 2022 10:51:25 -0500 Subject: [PATCH 04/12] Add comparator and sorting method Signed-off-by: Stephen Crawford --- .../java/org/opensearch/snapshots/SnapshotUtils.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java b/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java index be3bb1d25a825..92206ef21db16 100644 --- a/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java +++ b/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java @@ -41,6 +41,7 @@ import org.opensearch.index.IndexNotFoundException; import org.opensearch.index.IndexSettings; +import java.lang.reflect.Array; import java.util.*; /** @@ -63,11 +64,15 @@ public static List filterIndices(List availableIndices, String[] return availableIndices; } - Arrays.sort(selectedIndices, (o1, o2) -> { + selectedIndices = Arrays.stream(selectedIndices).filter(s -> !s.isEmpty()).toArray(a -> new String[a]); // Remove all empty strings + Arrays.sort(selectedIndices, (o1, o2) -> { // Make '-' lower priority then everything + char o1FirstChar = o1.charAt(0); char o2FirstChar = o2.charAt(0); - if (o1FirstChar == '-' && o2FirstChar != '-') { // Make '-' lower priority then everything + if (o1FirstChar == '-' && o2FirstChar != '-') { return 1; + } else if (o1FirstChar != '-' && o2FirstChar == '-') { + return -1; } return o1.compareTo(o2); }); From 36deec5c5efc1d55454a64ddacb50e0f90cbc539 Mon Sep 17 00:00:00 2001 From: Stephen Crawford Date: Fri, 23 Dec 2022 11:01:19 -0500 Subject: [PATCH 05/12] Add changelog Signed-off-by: Stephen Crawford --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07f70f9f4a7aa..f83c2dfa1dfca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add CI bundle pattern to distribution download ([#5348](https://github.com/opensearch-project/OpenSearch/pull/5348)) - Add support for ppc64le architecture ([#5459](https://github.com/opensearch-project/OpenSearch/pull/5459)) - ### Dependencies - Bumps `log4j-core` from 2.18.0 to 2.19.0 - Bumps `reactor-netty-http` from 1.0.18 to 1.0.23 @@ -65,6 +64,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Changed http code on create index API with bad input raising NotXContentException from 500 to 400 ([#4773](https://github.com/opensearch-project/OpenSearch/pull/4773)) - Change http code for DecommissioningFailedException from 500 to 400 ([#5283](https://github.com/opensearch-project/OpenSearch/pull/5283)) - Pre conditions check before updating weighted routing metadata ([#4955](https://github.com/opensearch-project/OpenSearch/pull/4955)) +- Correct index parsing logic for SnapshotUtils ([5626]https://github.com/opensearch-project/OpenSearch/pull/5626) ### Deprecated From 019a88f7cd8534446f1b0ea834c52d911e6b4f07 Mon Sep 17 00:00:00 2001 From: Stephen Crawford Date: Fri, 23 Dec 2022 11:02:25 -0500 Subject: [PATCH 06/12] fix import resolving Signed-off-by: Stephen Crawford --- .../java/org/opensearch/snapshots/SnapshotUtils.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java b/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java index 92206ef21db16..a566e580e0819 100644 --- a/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java +++ b/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java @@ -41,8 +41,14 @@ import org.opensearch.index.IndexNotFoundException; import org.opensearch.index.IndexSettings; -import java.lang.reflect.Array; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.HashMap; +import java.util.Map; /** * Snapshot utilities From 2a43f9ddf9b8dfa607d0800d2f858a831d466014 Mon Sep 17 00:00:00 2001 From: Stephen Crawford Date: Fri, 23 Dec 2022 11:05:33 -0500 Subject: [PATCH 07/12] Spotless apply Signed-off-by: Stephen Crawford --- server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java b/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java index a566e580e0819..862ff66f286f3 100644 --- a/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java +++ b/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java @@ -192,4 +192,3 @@ public static void validateSnapshotsBackingAnyIndex( } } } - From 9a2ee3f2e1791aaee3b539e6bf91f5351ea311e5 Mon Sep 17 00:00:00 2001 From: Stephen Crawford Date: Fri, 23 Dec 2022 11:27:14 -0500 Subject: [PATCH 08/12] Remove length ==1 Signed-off-by: Stephen Crawford --- .../src/main/java/org/opensearch/snapshots/SnapshotUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java b/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java index 862ff66f286f3..330103849b3a4 100644 --- a/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java +++ b/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java @@ -104,7 +104,7 @@ public static List filterIndices(List availableIndices, String[] } } else if (indexOrPattern.charAt(0) == '-') { // if its the first index pattern, fill it with all the indices... - if (i == 0 && selectedIndices.length == 1) { + if (i == 0) { result = new HashSet<>(availableIndices); } add = false; From 5862e0758d09cd8d91352a8cec93a5c504da4149 Mon Sep 17 00:00:00 2001 From: Stephen Crawford Date: Tue, 27 Dec 2022 10:09:36 -0500 Subject: [PATCH 09/12] Add a double negated parsing test Signed-off-by: Stephen Crawford --- .../test/java/org/opensearch/snapshots/SnapshotUtilsTests.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/test/java/org/opensearch/snapshots/SnapshotUtilsTests.java b/server/src/test/java/org/opensearch/snapshots/SnapshotUtilsTests.java index 365bc033d58fb..4a378acc39b6d 100644 --- a/server/src/test/java/org/opensearch/snapshots/SnapshotUtilsTests.java +++ b/server/src/test/java/org/opensearch/snapshots/SnapshotUtilsTests.java @@ -67,6 +67,7 @@ public void testIndexNameFiltering() { assertIndexNameFiltering(new String[] { "foo", "bar", "baz" }, new String[] { "+bar", "+foo" }, new String[] { "bar", "foo" }); assertIndexNameFiltering(new String[] { "foo", "bar", "baz" }, new String[] { "-bar", "b*" }, new String[] { "baz" }); assertIndexNameFiltering(new String[] { "foo", "bar", "baz" }, new String[] { "b*", "-bar" }, new String[] { "baz" }); + assertIndexNameFiltering(new String[] { "foo", "bar", "baz" }, new String[] { "-bar", "-baz" }, new String[] { "foo" }); assertIndexNameFiltering( new String[] { "foo", "bar", "baz" }, new String[] { "zzz", "bar" }, From 6beb70e3eed1117706a026c11272ab8350e8293c Mon Sep 17 00:00:00 2001 From: Stephen Crawford Date: Fri, 30 Dec 2022 09:46:18 -0500 Subject: [PATCH 10/12] Standardize snapshot indices parsing so that combinations of included and excluded indices are treated the same regardless of the order they are listed in Signed-off-by: Stephen Crawford --- CHANGELOG.md | 2 +- .../opensearch/snapshots/SnapshotUtils.java | 26 +++++++++---------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f83c2dfa1dfca..229b4d7fb29d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,7 +64,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Changed http code on create index API with bad input raising NotXContentException from 500 to 400 ([#4773](https://github.com/opensearch-project/OpenSearch/pull/4773)) - Change http code for DecommissioningFailedException from 500 to 400 ([#5283](https://github.com/opensearch-project/OpenSearch/pull/5283)) - Pre conditions check before updating weighted routing metadata ([#4955](https://github.com/opensearch-project/OpenSearch/pull/4955)) -- Correct index parsing logic for SnapshotUtils ([5626]https://github.com/opensearch-project/OpenSearch/pull/5626) +- Standardize snapshot indices parsing so that combinations of included and excluded indices are treated the same regardless of the order they are listed in ([#5626](https://github.com/opensearch-project/OpenSearch/pull/5626)) ### Deprecated diff --git a/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java b/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java index 330103849b3a4..32e95891ff697 100644 --- a/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java +++ b/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java @@ -49,6 +49,8 @@ import java.util.Set; import java.util.HashMap; import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.Stream; /** * Snapshot utilities @@ -70,18 +72,14 @@ public static List filterIndices(List availableIndices, String[] return availableIndices; } - selectedIndices = Arrays.stream(selectedIndices).filter(s -> !s.isEmpty()).toArray(a -> new String[a]); // Remove all empty strings - Arrays.sort(selectedIndices, (o1, o2) -> { // Make '-' lower priority then everything - - char o1FirstChar = o1.charAt(0); - char o2FirstChar = o2.charAt(0); - if (o1FirstChar == '-' && o2FirstChar != '-') { - return 1; - } else if (o1FirstChar != '-' && o2FirstChar == '-') { - return -1; - } - return o1.compareTo(o2); - }); + // Move the exclusions to end of list to ensure they are processed + // after explicitly selected indices are chosen. + final List excludesAtEndSelectedIndices = Stream.concat( + Arrays.stream(selectedIndices) + .filter(s -> s.isEmpty() || s.charAt(0) != '-'), + Arrays.stream(selectedIndices) + .filter(s -> !s.isEmpty() && s.charAt(0) == '-')) + .collect(Collectors.toUnmodifiableList()); Set result = null; for (int i = 0; i < selectedIndices.length; i++) { @@ -103,7 +101,9 @@ public static List filterIndices(List availableIndices, String[] result = new HashSet<>(); } } else if (indexOrPattern.charAt(0) == '-') { - // if its the first index pattern, fill it with all the indices... + // If the first index pattern is an exclusion, then all patterns are exclusions due to the + // reordering logic above. In this case, the request is interpreted as "include all indexes except + // those matching the exclusions" so we add all indices here and then remove the ones that match the exclusion patterns. if (i == 0) { result = new HashSet<>(availableIndices); } From 4ea9c63d1e9eac14481808204e40ae0a805c8f68 Mon Sep 17 00:00:00 2001 From: Stephen Crawford Date: Fri, 30 Dec 2022 10:21:49 -0500 Subject: [PATCH 11/12] Spotless Apply Signed-off-by: Stephen Crawford --- .../main/java/org/opensearch/snapshots/SnapshotUtils.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java b/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java index 32e95891ff697..2889543f5763a 100644 --- a/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java +++ b/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java @@ -75,11 +75,9 @@ public static List filterIndices(List availableIndices, String[] // Move the exclusions to end of list to ensure they are processed // after explicitly selected indices are chosen. final List excludesAtEndSelectedIndices = Stream.concat( - Arrays.stream(selectedIndices) - .filter(s -> s.isEmpty() || s.charAt(0) != '-'), - Arrays.stream(selectedIndices) - .filter(s -> !s.isEmpty() && s.charAt(0) == '-')) - .collect(Collectors.toUnmodifiableList()); + Arrays.stream(selectedIndices).filter(s -> s.isEmpty() || s.charAt(0) != '-'), + Arrays.stream(selectedIndices).filter(s -> !s.isEmpty() && s.charAt(0) == '-') + ).collect(Collectors.toUnmodifiableList()); Set result = null; for (int i = 0; i < selectedIndices.length; i++) { From c7d07bdd204493c2e172fb69b49532ebebb19ce4 Mon Sep 17 00:00:00 2001 From: Stephen Crawford Date: Tue, 3 Jan 2023 09:30:46 -0500 Subject: [PATCH 12/12] Fix index exclusion behavior in snapshot restore and clone APIs Signed-off-by: Stephen Crawford --- CHANGELOG.md | 4 ++-- .../src/main/java/org/opensearch/snapshots/SnapshotUtils.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 298f15f144449..66db5af37ef43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,7 +60,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Changed http code on create index API with bad input raising NotXContentException from 500 to 400 ([#4773](https://github.com/opensearch-project/OpenSearch/pull/4773)) - Change http code for DecommissioningFailedException from 500 to 400 ([#5283](https://github.com/opensearch-project/OpenSearch/pull/5283)) - Pre conditions check before updating weighted routing metadata ([#4955](https://github.com/opensearch-project/OpenSearch/pull/4955)) -- Standardize snapshot indices parsing so that combinations of included and excluded indices are treated the same regardless of the order they are listed in ([#5626](https://github.com/opensearch-project/OpenSearch/pull/5626)) ### Deprecated @@ -78,7 +77,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Remove LegacyESVersion.V_7_6_ and V_7_7_ Constants ([#4837](https://github.com/opensearch-project/OpenSearch/pull/4837)) - Remove LegacyESVersion.V_7_10_ Constants ([#5018](https://github.com/opensearch-project/OpenSearch/pull/5018)) - Remove Version.V_1_ Constants ([#5021](https://github.com/opensearch-project/OpenSearch/pull/5021)) -- Remove --enable-preview feature flag since Apache Lucene now patches class files ([#5642](https://github.com/opensearch-project/OpenSearch/pull/5642)) ### Fixed - Fix 'org.apache.hc.core5.http.ParseException: Invalid protocol version' under JDK 16+ ([#4827](https://github.com/opensearch-project/OpenSearch/pull/4827)) @@ -115,6 +113,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Apply cluster manager throttling settings during bootstrap ([#5524](https://github.com/opensearch-project/OpenSearch/pull/5524)) - Update thresholds map when cluster manager throttling setting is removed ([#5524](https://github.com/opensearch-project/OpenSearch/pull/5524)) - Fix backward compatibility for static cluster manager throttling threshold setting ([#5633](https://github.com/opensearch-project/OpenSearch/pull/5633)) +- Fix index exclusion behavior in snapshot restore and clone APIs ([#5626](https://github.com/opensearch-project/OpenSearch/pull/5626)) + ### Security [Unreleased 3.0]: https://github.com/opensearch-project/OpenSearch/compare/2.4...HEAD diff --git a/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java b/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java index 2889543f5763a..5c2efba008652 100644 --- a/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java +++ b/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java @@ -80,8 +80,8 @@ public static List filterIndices(List availableIndices, String[] ).collect(Collectors.toUnmodifiableList()); Set result = null; - for (int i = 0; i < selectedIndices.length; i++) { - String indexOrPattern = selectedIndices[i]; + for (int i = 0; i < excludesAtEndSelectedIndices.size(); i++) { + String indexOrPattern = excludesAtEndSelectedIndices.get(i); boolean add = true; if (!indexOrPattern.isEmpty()) { if (availableIndices.contains(indexOrPattern)) {