From 9d80a9d4a06ab15fe1932ac04216ba71a5a92d83 Mon Sep 17 00:00:00 2001 From: Pavan Yekbote Date: Mon, 21 Oct 2024 19:40:07 -0700 Subject: [PATCH 1/7] fix: ensure system indices are processed without templates Signed-off-by: Pavan Yekbote --- .../cluster/metadata/MetadataCreateIndexService.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java index abda5dad25e4e..d50d0b0addef3 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java @@ -443,6 +443,13 @@ public ClusterState applyCreateIndexRequest( // The backing index may have a different name or prefix than the data stream name. final String name = request.dataStreamName() != null ? request.dataStreamName() : request.index(); + // Do not apply any templates to system indices + // Using applyCreateIndexRequestWithV1Templates with empty list instead of applyCreateIndexRequestWithV2Template with null + // template as applyCreateIndexRequestWithV2Template has assertions when template is null + if (systemIndices.isSystemIndex(name)) { + return applyCreateIndexRequestWithV1Templates(currentState, request, silent, Collections.emptyList(), metadataTransformer); + } + // Check to see if a v2 template matched final String v2Template = MetadataIndexTemplateService.findV2Template( currentState.metadata(), From f51c296882ea4908b948a9c98bb5186a1591da44 Mon Sep 17 00:00:00 2001 From: Pavan Yekbote Date: Mon, 21 Oct 2024 20:40:53 -0700 Subject: [PATCH 2/7] refactor: overloaded method for creating without templates Signed-off-by: Pavan Yekbote --- .../metadata/MetadataCreateIndexService.java | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java index d50d0b0addef3..a9dfac2c29686 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java @@ -434,6 +434,14 @@ public ClusterState applyCreateIndexRequest( // in which case templates don't apply, so create the index from the source metadata return applyCreateIndexRequestWithExistingMetadata(currentState, request, silent, sourceMetadata, metadataTransformer); } else { + // The backing index may have a different name or prefix than the data stream name. + final String name = request.dataStreamName() != null ? request.dataStreamName() : request.index(); + + // Do not apply any templates to system indices + if (systemIndices.isSystemIndex(name)) { + return applyCreateIndexRequestWithNoTemplates(currentState, request, silent, metadataTransformer); + } + // Hidden indices apply templates slightly differently (ignoring wildcard '*' // templates), so we need to check to see if the request is creating a hidden index // prior to resolving which templates it matches @@ -441,15 +449,6 @@ public ClusterState applyCreateIndexRequest( ? IndexMetadata.INDEX_HIDDEN_SETTING.get(request.settings()) : null; - // The backing index may have a different name or prefix than the data stream name. - final String name = request.dataStreamName() != null ? request.dataStreamName() : request.index(); - // Do not apply any templates to system indices - // Using applyCreateIndexRequestWithV1Templates with empty list instead of applyCreateIndexRequestWithV2Template with null - // template as applyCreateIndexRequestWithV2Template has assertions when template is null - if (systemIndices.isSystemIndex(name)) { - return applyCreateIndexRequestWithV1Templates(currentState, request, silent, Collections.emptyList(), metadataTransformer); - } - // Check to see if a v2 template matched final String v2Template = MetadataIndexTemplateService.findV2Template( currentState.metadata(), @@ -688,6 +687,17 @@ public void addRemoteStoreCustomMetadata(IndexMetadata.Builder tmpImdBuilder, bo tmpImdBuilder.putCustom(IndexMetadata.REMOTE_STORE_CUSTOM_KEY, remoteCustomData); } + public ClusterState applyCreateIndexRequestWithNoTemplates( + final ClusterState currentState, + final CreateIndexClusterStateUpdateRequest request, + final boolean silent, + final BiConsumer metadataTransformer + ) throws Exception { + // Using applyCreateIndexRequestWithV1Templates with empty list instead of applyCreateIndexRequestWithV2Template + // with null template as applyCreateIndexRequestWithV2Template has assertions when template is null + return applyCreateIndexRequestWithV1Templates(currentState, request, silent, Collections.emptyList(), metadataTransformer); + } + private ClusterState applyCreateIndexRequestWithV1Templates( final ClusterState currentState, final CreateIndexClusterStateUpdateRequest request, From d838ec2e9e9102cfb41e95ad97ef880c4cff8bf8 Mon Sep 17 00:00:00 2001 From: Pavan Yekbote Date: Tue, 22 Oct 2024 13:31:06 -0700 Subject: [PATCH 3/7] test: adding test to check call for notemplates on system index Signed-off-by: Pavan Yekbote --- .../metadata/MetadataCreateIndexService.java | 2 +- .../MetadataCreateIndexServiceTests.java | 60 +++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java index a9dfac2c29686..012070a58c230 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java @@ -698,7 +698,7 @@ public ClusterState applyCreateIndexRequestWithNoTemplates( return applyCreateIndexRequestWithV1Templates(currentState, request, silent, Collections.emptyList(), metadataTransformer); } - private ClusterState applyCreateIndexRequestWithV1Templates( + public ClusterState applyCreateIndexRequestWithV1Templates( final ClusterState currentState, final CreateIndexClusterStateUpdateRequest request, final boolean silent, diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java index 1fdd038053eb6..5b648d20ab242 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java @@ -183,7 +183,11 @@ import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.startsWith; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; public class MetadataCreateIndexServiceTests extends OpenSearchTestCase { @@ -2522,6 +2526,62 @@ public void testApplyContextWithSettingsOverlap() throws IOException { } } + public void testApplyCreateIndexRequestWithNoTemplates() throws Exception { + BiConsumer mockMetadataTransformer = mock(BiConsumer.class); + SystemIndices mockSystemIndices = mock(SystemIndices.class); + when(mockSystemIndices.isSystemIndex(anyString())).thenReturn(true); + + ClusterService clusterService = mock(ClusterService.class); + ClusterState clusterState = ClusterState.builder(org.opensearch.cluster.ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)) + .metadata(Metadata.EMPTY_METADATA) + .build(); + + ThreadPool threadPool = new TestThreadPool(getTestName()); + + ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + when(clusterService.getSettings()).thenReturn(Settings.EMPTY); + when(clusterService.getClusterSettings()).thenReturn(clusterSettings); + when(clusterService.state()).thenReturn(clusterState); + + MetadataCreateIndexService metadataCreateIndexService = spy( + new MetadataCreateIndexService( + Settings.EMPTY, + clusterService, + indicesServices, + null, + null, + createTestShardLimitService(randomIntBetween(1, 1000), false, clusterService), + new Environment(Settings.builder().put("path.home", "dummy").build(), null), + IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, + threadPool, + null, + mockSystemIndices, + true, + new AwarenessReplicaBalance(Settings.EMPTY, clusterService.getClusterSettings()), + DefaultRemoteStoreSettings.INSTANCE, + repositoriesServiceSupplier + ) + ); + + metadataCreateIndexService.applyCreateIndexRequest(clusterState, request, false, mockMetadataTransformer); + verify(metadataCreateIndexService).applyCreateIndexRequestWithNoTemplates( + eq(clusterState), + eq(request), + eq(false), + eq(mockMetadataTransformer) + ); + + verify(metadataCreateIndexService).applyCreateIndexRequestWithV1Templates( + eq(clusterState), + eq(request), + eq(false), + eq(Collections.emptyList()), + eq(mockMetadataTransformer) + ); + + threadPool.shutdown(); + } + private IndexTemplateMetadata addMatchingTemplate(Consumer configurator) { IndexTemplateMetadata.Builder builder = templateMetadataBuilder("template1", "te*"); configurator.accept(builder); From e1090f065bce4918a2e451cc7610d523640cf0e1 Mon Sep 17 00:00:00 2001 From: Pavan Yekbote Date: Wed, 23 Oct 2024 09:27:06 -0700 Subject: [PATCH 4/7] refactor: cchange modifier to package private and add entry in changelog Signed-off-by: Pavan Yekbote --- CHANGELOG.md | 1 + .../cluster/metadata/MetadataCreateIndexService.java | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76b65a6cd70dc..c0b7e09fc772e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,6 +91,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fix array hashCode calculation in ResyncReplicationRequest ([#16378](https://github.com/opensearch-project/OpenSearch/pull/16378)) - Fix missing fields in task index mapping to ensure proper task result storage ([#16201](https://github.com/opensearch-project/OpenSearch/pull/16201)) - Fix typo super->sb in method toString() of RemoteStoreNodeAttribute ([#15362](https://github.com/opensearch-project/OpenSearch/pull/15362)) +- Ensure index templates are not applied to system indices ([#16418](https://github.com/opensearch-project/OpenSearch/pull/16418)) ### Security diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java index 012070a58c230..1af2e3710ad72 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java @@ -687,7 +687,7 @@ public void addRemoteStoreCustomMetadata(IndexMetadata.Builder tmpImdBuilder, bo tmpImdBuilder.putCustom(IndexMetadata.REMOTE_STORE_CUSTOM_KEY, remoteCustomData); } - public ClusterState applyCreateIndexRequestWithNoTemplates( + ClusterState applyCreateIndexRequestWithNoTemplates( final ClusterState currentState, final CreateIndexClusterStateUpdateRequest request, final boolean silent, @@ -698,7 +698,7 @@ public ClusterState applyCreateIndexRequestWithNoTemplates( return applyCreateIndexRequestWithV1Templates(currentState, request, silent, Collections.emptyList(), metadataTransformer); } - public ClusterState applyCreateIndexRequestWithV1Templates( + ClusterState applyCreateIndexRequestWithV1Templates( final ClusterState currentState, final CreateIndexClusterStateUpdateRequest request, final boolean silent, From 852fb7951a57d314ab05e83f7f118b4e9ad95f96 Mon Sep 17 00:00:00 2001 From: Pavan Yekbote Date: Thu, 24 Oct 2024 19:51:43 -0700 Subject: [PATCH 5/7] test: adding IT test Signed-off-by: Pavan Yekbote --- .../opensearch/http/SystemIndexRestIT.java | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/qa/smoke-test-http/src/test/java/org/opensearch/http/SystemIndexRestIT.java b/qa/smoke-test-http/src/test/java/org/opensearch/http/SystemIndexRestIT.java index 9f2d686251947..45564b2a77f91 100644 --- a/qa/smoke-test-http/src/test/java/org/opensearch/http/SystemIndexRestIT.java +++ b/qa/smoke-test-http/src/test/java/org/opensearch/http/SystemIndexRestIT.java @@ -123,6 +123,76 @@ public void testSystemIndexAccessBlockedByDefault() throws Exception { } } + public void testSystemIndexCreatedWithoutAnyTemplates() throws Exception { + // create template + { + Request templateRequest = new Request("POST", "_component_template/error_mapping_test_template"); + String jsonBody = "{\n" + + " \"template\": {\n" + + " \"mappings\": {\n" + + " \"properties\": {\n" + + " \"error\" : {\n" + + " \"type\": \"nested\",\n" + + " \"properties\": {\n" + + " \"message\": {\n" + + " \"type\": \"text\"\n" + + " },\n" + + " \"status\": {\n" + + " \"type\": \"integer\"\n" + + " }\n" + + " }\n" + + " }\n" + + " }\n" + + " }\n" + + " }\n" + + "}"; + + templateRequest.setJsonEntity(jsonBody); + Response resp = getRestClient().performRequest(templateRequest); + assertThat(resp.getStatusLine().getStatusCode(), equalTo(200)); + } + + + // apply template to indices + { + Request applyTemplateRequest = new Request("POST", "_index_template/match_all_test_template"); + String jsonBody = "{\n" + + " \"index_patterns\": [\n" + + " \"*system-idx*\"\n" + + " ],\n" + + " \"template\": {\n" + + " \"settings\": {}\n" + + " },\n" + + " \"priority\": 10,\n" + + " \"composed_of\": [\n" + + " \"error_mapping_test_template\"\n" + + " ],\n" + + " \"version\": 1\n" + + "}"; + + applyTemplateRequest.setJsonEntity(jsonBody); + Response resp = getRestClient().performRequest(applyTemplateRequest); + assertThat(resp.getStatusLine().getStatusCode(), equalTo(200)); + } + + // create system index - success + { + Request indexRequest = new Request("PUT", "/" + SystemIndexTestPlugin.SYSTEM_INDEX_NAME); + String jsonBody = "{\n" + + " \"mappings\": {\n" + + " \"properties\": {\n" + + " \"error\": {\n" + + " \"type\": \"text\"\n" + + " }\n" + + " }\n" + + " }\n" + + "}"; + indexRequest.setJsonEntity(jsonBody); + Response resp = getRestClient().performRequest(indexRequest); + assertThat(resp.getStatusLine().getStatusCode(), equalTo(200)); + } + } + private void assertDeprecationWarningOnAccess(String queryPattern, String warningIndexName) throws IOException { String expectedWarning = "this request accesses system indices: [" + warningIndexName + "], but in a " + "future major version, direct access to system indices will be prevented by default"; From a19f1e34f4a38661545731ca29541fd36b820c63 Mon Sep 17 00:00:00 2001 From: Pavan Yekbote Date: Tue, 29 Oct 2024 10:43:56 -0700 Subject: [PATCH 6/7] refactor: remove UT and add private modifiers Signed-off-by: Pavan Yekbote --- .../metadata/MetadataCreateIndexService.java | 4 +- .../MetadataCreateIndexServiceTests.java | 56 ------------------- 2 files changed, 2 insertions(+), 58 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java index 77877ecdf0a47..727a08b615050 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java @@ -682,7 +682,7 @@ public void addRemoteStoreCustomMetadata(IndexMetadata.Builder tmpImdBuilder, bo tmpImdBuilder.putCustom(IndexMetadata.REMOTE_STORE_CUSTOM_KEY, remoteCustomData); } - ClusterState applyCreateIndexRequestWithNoTemplates( + private ClusterState applyCreateIndexRequestWithNoTemplates( final ClusterState currentState, final CreateIndexClusterStateUpdateRequest request, final boolean silent, @@ -693,7 +693,7 @@ ClusterState applyCreateIndexRequestWithNoTemplates( return applyCreateIndexRequestWithV1Templates(currentState, request, silent, Collections.emptyList(), metadataTransformer); } - ClusterState applyCreateIndexRequestWithV1Templates( + private ClusterState applyCreateIndexRequestWithV1Templates( final ClusterState currentState, final CreateIndexClusterStateUpdateRequest request, final boolean silent, diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java index e2e4bee10c3cf..9c8dffcee684c 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java @@ -2563,62 +2563,6 @@ public void testApplyContextWithSettingsOverlap() throws IOException { } } - public void testApplyCreateIndexRequestWithNoTemplates() throws Exception { - BiConsumer mockMetadataTransformer = mock(BiConsumer.class); - SystemIndices mockSystemIndices = mock(SystemIndices.class); - when(mockSystemIndices.isSystemIndex(anyString())).thenReturn(true); - - ClusterService clusterService = mock(ClusterService.class); - ClusterState clusterState = ClusterState.builder(org.opensearch.cluster.ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)) - .metadata(Metadata.EMPTY_METADATA) - .build(); - - ThreadPool threadPool = new TestThreadPool(getTestName()); - - ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - when(clusterService.getSettings()).thenReturn(Settings.EMPTY); - when(clusterService.getClusterSettings()).thenReturn(clusterSettings); - when(clusterService.state()).thenReturn(clusterState); - - MetadataCreateIndexService metadataCreateIndexService = spy( - new MetadataCreateIndexService( - Settings.EMPTY, - clusterService, - indicesServices, - null, - null, - createTestShardLimitService(randomIntBetween(1, 1000), false, clusterService), - new Environment(Settings.builder().put("path.home", "dummy").build(), null), - IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, - threadPool, - null, - mockSystemIndices, - true, - new AwarenessReplicaBalance(Settings.EMPTY, clusterService.getClusterSettings()), - DefaultRemoteStoreSettings.INSTANCE, - repositoriesServiceSupplier - ) - ); - - metadataCreateIndexService.applyCreateIndexRequest(clusterState, request, false, mockMetadataTransformer); - verify(metadataCreateIndexService).applyCreateIndexRequestWithNoTemplates( - eq(clusterState), - eq(request), - eq(false), - eq(mockMetadataTransformer) - ); - - verify(metadataCreateIndexService).applyCreateIndexRequestWithV1Templates( - eq(clusterState), - eq(request), - eq(false), - eq(Collections.emptyList()), - eq(mockMetadataTransformer) - ); - - threadPool.shutdown(); - } - private IndexTemplateMetadata addMatchingTemplate(Consumer configurator) { IndexTemplateMetadata.Builder builder = templateMetadataBuilder("template1", "te*"); configurator.accept(builder); From d80f7d0a0b041592374e00b9ee2b00c06ac2b79e Mon Sep 17 00:00:00 2001 From: Pavan Yekbote Date: Tue, 29 Oct 2024 10:46:03 -0700 Subject: [PATCH 7/7] refactor: spotless changes Signed-off-by: Pavan Yekbote --- .../cluster/metadata/MetadataCreateIndexServiceTests.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java index 9c8dffcee684c..0bb9ec28a1efc 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java @@ -184,11 +184,7 @@ import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.startsWith; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; public class MetadataCreateIndexServiceTests extends OpenSearchTestCase {