Make searchable snapshot indexes read-only but allow deletion #4764

Vishalks · 2022-10-13T05:14:07Z

Signed-off-by: Vishal Sarda vsarda@amazon.com

Description

The changes in this PR are made on top of #4680. This PR makes the index restored from remote snapshot as read only, but it can be deleted. The write blocks on this index continue to be applicable even when explicitly set to false by user using "index.blocks.read_only_allow_delete": false. For any operations other than read on such an index, we throw an unsupported exception.

Issues Resolved

#4655

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Vishal Sarda <vsarda@amazon.com>

github-actions · 2022-10-13T05:40:37Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/4554/
CommitID: 5c7b8d6

Signed-off-by: Vishal Sarda <vsarda@amazon.com>

github-actions · 2022-10-13T17:55:20Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/4601/
CommitID: 00d6f47

github-actions · 2022-10-13T17:57:21Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/4602/
CommitID: 0a85671

Bukhtawar · 2022-10-13T18:23:47Z

server/src/main/java/org/opensearch/snapshots/RestoreService.java

@@ -1227,6 +1227,7 @@ private static IndexMetadata addSnapshotToIndexSettings(IndexMetadata metadata,
            .put(IndexSettings.SEARCHABLE_SNAPSHOT_ID_UUID.getKey(), snapshot.getSnapshotId().getUUID())
            .put(IndexSettings.SEARCHABLE_SNAPSHOT_ID_NAME.getKey(), snapshot.getSnapshotId().getName())
            .put(IndexSettings.SEARCHABLE_SNAPSHOT_INDEX_ID.getKey(), indexId.getId())
+            .put(IndexMetadata.APIBlock.READ_ONLY_ALLOW_DELETE.settingName(), true)


With DiskThresholdDecider be able to revert this?

…read only block by disk threshold monitor Signed-off-by: Vishal Sarda <vsarda@amazon.com>

Vishalks · 2022-10-19T06:55:07Z

server/src/main/java/org/opensearch/cluster/routing/allocation/DiskThresholdMonitor.java

@@ -356,6 +357,7 @@ public void onNewInfo(ClusterInfo info) {
        final Set<String> indicesToAutoRelease = StreamSupport.stream(state.routingTable().indicesRouting().spliterator(), false)
            .map(c -> c.key)
            .filter(index -> indicesNotToAutoRelease.contains(index) == false)
+            .filter(index -> !state.getMetadata().settings().get(IndexModule.INDEX_STORE_TYPE_SETTING.getKey()).equals(IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey()))


@Bukhtawar I have added this so that disk threshold monitor filters out index backed by remote snapshot from list of indices to be released, but currently I can't find a way to test it locally (discussed more details on slack). Can you suggest a way to test this in run time (somehow the monitor selects some indices to auto release but not the remote ones)?

Can we explore using a separate block so that concerns are separate and we don't run into each other?

github-actions · 2022-10-19T07:12:41Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/4913/
CommitID: 932a6ac

andrross · 2022-10-19T15:39:48Z

Looks like a test failure related to this change: https://build.ci.opensearch.org/job/gradle-check/4913/testReport/junit/org.opensearch.index.reindex/DeleteByQueryBasicTests/testDeleteByQueryOnReadOnlyAllowDeleteIndex/

Looking at the log I found this:

java.lang.NullPointerException: Cannot invoke "String.equals(Object)" because the return value of "org.opensearch.common.settings.Settings.get(String)" is null
	at org.opensearch.cluster.routing.allocation.DiskThresholdMonitor.lambda$onNewInfo$4(DiskThresholdMonitor.java:360) ~[opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]

Signed-off-by: Vishal Sarda <vsarda@amazon.com>

github-actions · 2022-10-21T08:18:42Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/5152/
CommitID: aee4e97

github-actions · 2022-10-21T08:45:24Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/5156/
CommitID: 5019b33

andrross · 2022-10-21T14:21:39Z

Can you add some tests? Maybe some additional tests to SearchableSnapshotIT

github-actions · 2022-10-21T14:30:25Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/5174/
CommitID: 5019b33

github-actions · 2022-10-21T17:14:01Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/5193/
CommitID: 5019b33

andrross · 2022-10-25T16:47:13Z

The problem with this approach is that the following operation works and will undo this setting:

PUT my_index/_settings
{
    "index": {
        "blocks.write": false
    }
}

I think some setting changes make sense for searchable snapshots and others don't. A reasonable approach is probably to block all setting changes initially for searchable snapshots and then work through the design and implementation of allowing particular operations where appropriate. That is the ultimate goal of issue #4655.

Signed-off-by: Vishal Sarda <vsarda@amazon.com>

Vishalks · 2022-10-25T21:55:29Z

The problem with this approach is that the following operation works and will undo this setting:
PUT my_index/_settings
{
    "index": {
        "blocks.write": false
    }
}
I think some setting changes make sense for searchable snapshots and others don't. A reasonable approach is probably to block all setting changes initially for searchable snapshots and then work through the design and implementation of allowing particular operations where appropriate. That is the ultimate goal of issue #4655.

Isn't that API allowed to be only called by the admin? If yes, letting admin tweak the index settings seems reasonable to me.

github-actions · 2022-10-25T22:22:38Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/5618/
CommitID: f5f1f16

andrross · 2022-10-25T22:55:02Z

Isn't that API allowed to be only called by the admin? If yes, letting admin tweak the index settings seems reasonable to me.

A searchable snapshot index cannot be writable, so it's wrong to let the API block be removed, admin or not. There's a whole host of settings that don't apply to an inherently read-only index.

Signed-off-by: Vishal Sarda <vsarda@amazon.com>

Vishalks · 2022-10-26T18:44:22Z

Isn't that API allowed to be only called by the admin? If yes, letting admin tweak the index settings seems reasonable to me.

A searchable snapshot index cannot be writable, so it's wrong to let the API block be removed, admin or not. There's a whole host of settings that don't apply to an inherently read-only index.

I assumed that since #4655 mentions 'user should not be able to change index settings', letting them affected through admin should be fine (admin > user). Seems like it's not the case.

github-actions · 2022-10-26T19:00:13Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/5701/
CommitID: e2f7b25
Please examine the workflow log, locate, and copy-paste the failure below, then iterate to green.
Is the failure a flaky test unrelated to your change?

stephen-crawford · 2022-10-26T19:17:02Z

Hi @Vishalks, I am reaching out from the security team to check in and see if there was anything you may need from our end. I have been going to through active issues tagged for the upcoming release and trying to check in with any that look like they may brush with us. Let me know if you need anything from us.

andrross · 2022-10-26T20:37:50Z

Hey @scrawfor99 thanks for checking in. This task is about configuring static API blocks for the experimental searchable snapshot indexes. There shouldn't need to be any changes from the security team.

server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java

CHANGELOG.md

Signed-off-by: Vishal Sarda <vsarda@amazon.com>

github-actions · 2022-10-27T09:56:24Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/5773/
CommitID: e37308e
Please examine the workflow log, locate, and copy-paste the failure below, then iterate to green.
Is the failure a flaky test unrelated to your change?

github-actions · 2022-10-27T10:22:44Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/5777/
CommitID: f4b0564
Please examine the workflow log, locate, and copy-paste the failure below, then iterate to green.
Is the failure a flaky test unrelated to your change?

github-actions · 2022-10-27T17:12:11Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/5797/
CommitID: f4b0564
Please examine the workflow log, locate, and copy-paste the failure below, then iterate to green.
Is the failure a flaky test unrelated to your change?

kotwanikunal · 2022-10-27T17:27:20Z

rg.opensearch.qa.verify_version_constants.VerifyVersionConstantsIT > testLuceneVersionConstant FAILED
    java.lang.AssertionError: 
    Expected: <9.4.1>
         but: was <9.4.0>
        at __randomizedtesting.SeedInfo.seed([4E6DBD0DFC297DE6:C3088051DB21BEAF]:0)
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
        at org.junit.Assert.assertThat(Assert.java:964)
        at org.junit.Assert.assertThat(Assert.java:930)
        at org.opensearch.qa.verify_version_constants.VerifyVersionConstantsIT.testLuceneVersionConstant(VerifyVersionConstantsIT.java:56)

@Vishalks can you please rebase the commit?

github-actions · 2022-10-27T18:02:16Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/5802/
CommitID: 5cb3101

opensearch-trigger-bot · 2022-10-27T18:15:51Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-4764-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 c4fc1bc0f29bc7a005e4755be5d2e1b22b7de72a
# Push it to GitHub
git push --set-upstream origin backport/backport-4764-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-4764-to-2.x.

…arch-project#4764) This commit adds a new cluster block that is added by default to all searchable snapshot indexes to block writes and metadata changes. Signed-off-by: Vishal Sarda <vsarda@amazon.com> (cherry picked from commit c4fc1bc) Signed-off-by: Vishal Sarda <vsarda@amazon.com>

…#4958) This commit adds a new cluster block that is added by default to all searchable snapshot indexes to block writes and metadata changes. (cherry picked from commit c4fc1bc) Signed-off-by: Vishal Sarda <vsarda@amazon.com> Co-authored-by: Vishal Sarda <Vishalks@users.noreply.github.com>

Make the index restored by remote snapshot as read only but deletable

5c7b8d6

Signed-off-by: Vishal Sarda <vsarda@amazon.com>

Vishalks changed the title ~~Make the index restored by remote snapshot as read only but deletable~~ Make the index restored by remote snapshot as read only but allow deletion Oct 13, 2022

Vishalks added 2 commits October 13, 2022 10:22

Changelog updation

00d6f47

Signed-off-by: Vishal Sarda <vsarda@amazon.com>

Merge branch 'main' into make_remote_snapshot_index_read_only

0a85671

Signed-off-by: Vishal Sarda <vsarda@amazon.com>

Vishalks marked this pull request as ready for review October 13, 2022 17:36

Vishalks requested review from a team and reta as code owners October 13, 2022 17:36

Bukhtawar reviewed Oct 13, 2022

View reviewed changes

Filtering out remote snapshot based index from being released out of …

932a6ac

…read only block by disk threshold monitor Signed-off-by: Vishal Sarda <vsarda@amazon.com>

Vishalks commented Oct 19, 2022

View reviewed changes

Vishalks added 2 commits October 21, 2022 13:13

Replacing read only allow delete block with write block

aee4e97

Signed-off-by: Vishal Sarda <vsarda@amazon.com>

Merge branch 'main' into make_remote_snapshot_index_read_only

5019b33

Signed-off-by: Vishal Sarda <vsarda@amazon.com>

Adding Integration tests for searchable snapshot

f5f1f16

Signed-off-by: Vishal Sarda <vsarda@amazon.com>

introducing new cluster block for remote snapshot

e2f7b25

Signed-off-by: Vishal Sarda <vsarda@amazon.com>

stephen-crawford mentioned this pull request Oct 26, 2022

Release Version 2.4.0 opensearch-project/security#2160

Closed

23 tasks

andrross reviewed Oct 26, 2022

View reviewed changes

server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java Outdated Show resolved Hide resolved

CHANGELOG.md Outdated Show resolved Hide resolved

Vishalks added 2 commits October 27, 2022 15:03

PR review implementation

e37308e

Signed-off-by: Vishal Sarda <vsarda@amazon.com>

Gradle precommit

f4b0564

Signed-off-by: Vishal Sarda <vsarda@amazon.com>

Merge branch 'main' into make_remote_snapshot_index_read_only

5cb3101

andrross changed the title ~~Make the index restored by remote snapshot as read only but allow deletion~~ Make searchable snapshot indexes read-only but allow deletion Oct 27, 2022

andrross approved these changes Oct 27, 2022

View reviewed changes

andrross merged commit c4fc1bc into opensearch-project:main Oct 27, 2022

andrross added backport 2.x Backport to 2.x branch v2.4.0 'Issues and PRs related to version v2.4.0' labels Oct 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make searchable snapshot indexes read-only but allow deletion #4764

Make searchable snapshot indexes read-only but allow deletion #4764

Vishalks commented Oct 13, 2022 •

edited

Loading

github-actions bot commented Oct 13, 2022

github-actions bot commented Oct 13, 2022

github-actions bot commented Oct 13, 2022

Bukhtawar Oct 13, 2022

Vishalks Oct 19, 2022

Bukhtawar Oct 19, 2022

github-actions bot commented Oct 19, 2022

andrross commented Oct 19, 2022

github-actions bot commented Oct 21, 2022

github-actions bot commented Oct 21, 2022

andrross commented Oct 21, 2022

github-actions bot commented Oct 21, 2022

github-actions bot commented Oct 21, 2022

andrross commented Oct 25, 2022

Vishalks commented Oct 25, 2022

github-actions bot commented Oct 25, 2022

andrross commented Oct 25, 2022

Vishalks commented Oct 26, 2022

github-actions bot commented Oct 26, 2022

stephen-crawford commented Oct 26, 2022

andrross commented Oct 26, 2022

github-actions bot commented Oct 27, 2022

github-actions bot commented Oct 27, 2022

github-actions bot commented Oct 27, 2022

kotwanikunal commented Oct 27, 2022

github-actions bot commented Oct 27, 2022

opensearch-trigger-bot bot commented Oct 27, 2022

Make searchable snapshot indexes read-only but allow deletion #4764

Make searchable snapshot indexes read-only but allow deletion #4764

Conversation

Vishalks commented Oct 13, 2022 • edited Loading

Description

Issues Resolved

github-actions bot commented Oct 13, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Oct 13, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Oct 13, 2022

Gradle Check (Jenkins) Run Completed with:

Bukhtawar Oct 13, 2022

Choose a reason for hiding this comment

Vishalks Oct 19, 2022

Choose a reason for hiding this comment

Bukhtawar Oct 19, 2022

Choose a reason for hiding this comment

github-actions bot commented Oct 19, 2022

Gradle Check (Jenkins) Run Completed with:

andrross commented Oct 19, 2022

github-actions bot commented Oct 21, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Oct 21, 2022

Gradle Check (Jenkins) Run Completed with:

andrross commented Oct 21, 2022

github-actions bot commented Oct 21, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Oct 21, 2022

Gradle Check (Jenkins) Run Completed with:

andrross commented Oct 25, 2022

Vishalks commented Oct 25, 2022

github-actions bot commented Oct 25, 2022

Gradle Check (Jenkins) Run Completed with:

andrross commented Oct 25, 2022

Vishalks commented Oct 26, 2022

github-actions bot commented Oct 26, 2022

Gradle Check (Jenkins) Run Completed with:

stephen-crawford commented Oct 26, 2022

andrross commented Oct 26, 2022

github-actions bot commented Oct 27, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Oct 27, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Oct 27, 2022

Gradle Check (Jenkins) Run Completed with:

kotwanikunal commented Oct 27, 2022

github-actions bot commented Oct 27, 2022

Gradle Check (Jenkins) Run Completed with:

opensearch-trigger-bot bot commented Oct 27, 2022

Vishalks commented Oct 13, 2022 •

edited

Loading