From e833e7b6c4e81284ef4e5bcb29cea9d9dad6b963 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Sat, 12 Oct 2024 18:55:27 +0200 Subject: [PATCH] Add feature flag for subobjects auto (#114616) --- .../rest/yaml/CcsCommonYamlTestSuiteIT.java | 3 +- .../yaml/RcsCcsCommonYamlTestSuiteIT.java | 1 + ...okeTestMultiNodeClientYamlTestSuiteIT.java | 1 + .../test/rest/ClientYamlTestSuiteIT.java | 1 + .../index/mapper/ObjectMapper.java | 4 +- .../index/mapper/DynamicTemplatesTests.java | 8 +++ .../index/mapper/ObjectMapperTests.java | 50 +++++++++++-------- .../test/cluster/FeatureFlag.java | 1 + .../xpack/test/rest/XPackRestIT.java | 1 + ...CoreWithSecurityClientYamlTestSuiteIT.java | 1 + x-pack/qa/runtime-fields/build.gradle | 1 + 11 files changed, 49 insertions(+), 23 deletions(-) diff --git a/qa/ccs-common-rest/src/yamlRestTest/java/org/elasticsearch/test/rest/yaml/CcsCommonYamlTestSuiteIT.java b/qa/ccs-common-rest/src/yamlRestTest/java/org/elasticsearch/test/rest/yaml/CcsCommonYamlTestSuiteIT.java index 8ce1bfdc61f6b..3a24427df24a3 100644 --- a/qa/ccs-common-rest/src/yamlRestTest/java/org/elasticsearch/test/rest/yaml/CcsCommonYamlTestSuiteIT.java +++ b/qa/ccs-common-rest/src/yamlRestTest/java/org/elasticsearch/test/rest/yaml/CcsCommonYamlTestSuiteIT.java @@ -89,7 +89,8 @@ public class CcsCommonYamlTestSuiteIT extends ESClientYamlSuiteTestCase { .setting("xpack.security.enabled", "false") // geohex_grid requires gold license .setting("xpack.license.self_generated.type", "trial") - .feature(FeatureFlag.TIME_SERIES_MODE); + .feature(FeatureFlag.TIME_SERIES_MODE) + .feature(FeatureFlag.SUB_OBJECTS_AUTO_ENABLED); private static ElasticsearchCluster remoteCluster = ElasticsearchCluster.local() .name(REMOTE_CLUSTER_NAME) diff --git a/qa/ccs-common-rest/src/yamlRestTest/java/org/elasticsearch/test/rest/yaml/RcsCcsCommonYamlTestSuiteIT.java b/qa/ccs-common-rest/src/yamlRestTest/java/org/elasticsearch/test/rest/yaml/RcsCcsCommonYamlTestSuiteIT.java index acdd540ca7b9d..5ada1e941266a 100644 --- a/qa/ccs-common-rest/src/yamlRestTest/java/org/elasticsearch/test/rest/yaml/RcsCcsCommonYamlTestSuiteIT.java +++ b/qa/ccs-common-rest/src/yamlRestTest/java/org/elasticsearch/test/rest/yaml/RcsCcsCommonYamlTestSuiteIT.java @@ -91,6 +91,7 @@ public class RcsCcsCommonYamlTestSuiteIT extends ESClientYamlSuiteTestCase { .setting("xpack.security.remote_cluster_server.ssl.enabled", "false") .setting("xpack.security.remote_cluster_client.ssl.enabled", "false") .feature(FeatureFlag.TIME_SERIES_MODE) + .feature(FeatureFlag.SUB_OBJECTS_AUTO_ENABLED) .user("test_admin", "x-pack-test-password"); private static ElasticsearchCluster fulfillingCluster = ElasticsearchCluster.local() diff --git a/qa/smoke-test-multinode/src/yamlRestTest/java/org/elasticsearch/smoketest/SmokeTestMultiNodeClientYamlTestSuiteIT.java b/qa/smoke-test-multinode/src/yamlRestTest/java/org/elasticsearch/smoketest/SmokeTestMultiNodeClientYamlTestSuiteIT.java index c68d27b883c53..e53c0564be297 100644 --- a/qa/smoke-test-multinode/src/yamlRestTest/java/org/elasticsearch/smoketest/SmokeTestMultiNodeClientYamlTestSuiteIT.java +++ b/qa/smoke-test-multinode/src/yamlRestTest/java/org/elasticsearch/smoketest/SmokeTestMultiNodeClientYamlTestSuiteIT.java @@ -35,6 +35,7 @@ public class SmokeTestMultiNodeClientYamlTestSuiteIT extends ESClientYamlSuiteTe // The first node does not have the ingest role so we're sure ingest requests are forwarded: .node(0, n -> n.setting("node.roles", "[master,data,ml,remote_cluster_client,transform]")) .feature(FeatureFlag.TIME_SERIES_MODE) + .feature(FeatureFlag.SUB_OBJECTS_AUTO_ENABLED) .build(); public SmokeTestMultiNodeClientYamlTestSuiteIT(@Name("yaml") ClientYamlTestCandidate testCandidate) { diff --git a/rest-api-spec/src/yamlRestTest/java/org/elasticsearch/test/rest/ClientYamlTestSuiteIT.java b/rest-api-spec/src/yamlRestTest/java/org/elasticsearch/test/rest/ClientYamlTestSuiteIT.java index 2b20e35019424..084e212a913b2 100644 --- a/rest-api-spec/src/yamlRestTest/java/org/elasticsearch/test/rest/ClientYamlTestSuiteIT.java +++ b/rest-api-spec/src/yamlRestTest/java/org/elasticsearch/test/rest/ClientYamlTestSuiteIT.java @@ -36,6 +36,7 @@ public class ClientYamlTestSuiteIT extends ESClientYamlSuiteTestCase { .module("health-shards-availability") .module("data-streams") .feature(FeatureFlag.TIME_SERIES_MODE) + .feature(FeatureFlag.SUB_OBJECTS_AUTO_ENABLED) .build(); public ClientYamlTestSuiteIT(@Name("yaml") ClientYamlTestCandidate testCandidate) { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java index 5e63fee8c5adc..70c4a3ac213a2 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java @@ -15,6 +15,7 @@ import org.elasticsearch.common.Explicit; import org.elasticsearch.common.logging.DeprecationCategory; import org.elasticsearch.common.logging.DeprecationLogger; +import org.elasticsearch.common.util.FeatureFlag; import org.elasticsearch.common.xcontent.support.XContentMapValues; import org.elasticsearch.core.Nullable; import org.elasticsearch.features.NodeFeature; @@ -41,6 +42,7 @@ public class ObjectMapper extends Mapper { private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(ObjectMapper.class); + public static final FeatureFlag SUB_OBJECTS_AUTO_FEATURE_FLAG = new FeatureFlag("sub_objects_auto"); public static final String CONTENT_TYPE = "object"; static final String STORE_ARRAY_SOURCE_PARAM = "store_array_source"; @@ -74,7 +76,7 @@ public static Subobjects from(Object node) { if (value.equalsIgnoreCase("false")) { return DISABLED; } - if (value.equalsIgnoreCase("auto")) { + if (SUB_OBJECTS_AUTO_FEATURE_FLAG.isEnabled() && value.equalsIgnoreCase("auto")) { return AUTO; } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java index 7f430cf676809..7e9a196faaa26 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java @@ -1377,6 +1377,7 @@ public void testSubobjectsFalseWithInnerNestedFromDynamicTemplate() { } public void testSubobjectsAutoFlatPaths() throws IOException { + assumeTrue("only test when feature flag for subobjects auto is enabled", ObjectMapper.SUB_OBJECTS_AUTO_FEATURE_FLAG.isEnabled()); MapperService mapperService = createDynamicTemplateAutoSubobjects(); ParsedDocument doc = mapperService.documentMapper().parse(source(b -> { b.field("foo.metric.count", 10); @@ -1389,6 +1390,7 @@ public void testSubobjectsAutoFlatPaths() throws IOException { } public void testSubobjectsAutoStructuredPaths() throws IOException { + assumeTrue("only test when feature flag for subobjects auto is enabled", ObjectMapper.SUB_OBJECTS_AUTO_FEATURE_FLAG.isEnabled()); MapperService mapperService = createDynamicTemplateAutoSubobjects(); ParsedDocument doc = mapperService.documentMapper().parse(source(b -> { b.startObject("foo"); @@ -1411,6 +1413,7 @@ public void testSubobjectsAutoStructuredPaths() throws IOException { } public void testSubobjectsAutoArrayOfObjects() throws IOException { + assumeTrue("only test when feature flag for subobjects auto is enabled", ObjectMapper.SUB_OBJECTS_AUTO_FEATURE_FLAG.isEnabled()); MapperService mapperService = createDynamicTemplateAutoSubobjects(); ParsedDocument doc = mapperService.documentMapper().parse(source(b -> { b.startObject("foo"); @@ -1444,6 +1447,7 @@ public void testSubobjectsAutoArrayOfObjects() throws IOException { } public void testSubobjectAutoDynamicNested() throws IOException { + assumeTrue("only test when feature flag for subobjects auto is enabled", ObjectMapper.SUB_OBJECTS_AUTO_FEATURE_FLAG.isEnabled()); DocumentMapper mapper = createDocumentMapper(topMapping(b -> { b.startArray("dynamic_templates"); { @@ -1482,6 +1486,7 @@ public void testSubobjectAutoDynamicNested() throws IOException { } public void testRootSubobjectAutoDynamicNested() throws IOException { + assumeTrue("only test when feature flag for subobjects auto is enabled", ObjectMapper.SUB_OBJECTS_AUTO_FEATURE_FLAG.isEnabled()); DocumentMapper mapper = createDocumentMapper(topMapping(b -> { b.startArray("dynamic_templates"); { @@ -1515,6 +1520,7 @@ public void testRootSubobjectAutoDynamicNested() throws IOException { } public void testDynamicSubobjectsAutoDynamicFalse() throws Exception { + assumeTrue("only test when feature flag for subobjects auto is enabled", ObjectMapper.SUB_OBJECTS_AUTO_FEATURE_FLAG.isEnabled()); // verify that we read the dynamic value properly from the parent mapper. DocumentParser#dynamicOrDefault splits the field // name where dots are found, but it does that only for the parent prefix e.g. metrics.service and not for the leaf suffix time.max DocumentMapper mapper = createDocumentMapper(topMapping(b -> { @@ -1578,6 +1584,7 @@ public void testDynamicSubobjectsAutoDynamicFalse() throws Exception { } public void testSubobjectsAutoWithInnerNestedFromDynamicTemplate() throws IOException { + assumeTrue("only test when feature flag for subobjects auto is enabled", ObjectMapper.SUB_OBJECTS_AUTO_FEATURE_FLAG.isEnabled()); DocumentMapper mapper = createDocumentMapper(topMapping(b -> { b.startArray("dynamic_templates"); { @@ -2045,6 +2052,7 @@ public void testSubobjectsFalseFlattened() throws IOException { } public void testSubobjectsAutoFlattened() throws IOException { + assumeTrue("only test when feature flag for subobjects auto is enabled", ObjectMapper.SUB_OBJECTS_AUTO_FEATURE_FLAG.isEnabled()); String mapping = """ { "_doc": { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/ObjectMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/ObjectMapperTests.java index 64eee39532c31..3b77015fde415 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/ObjectMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/ObjectMapperTests.java @@ -169,27 +169,29 @@ public void testMergeEnabledForIndexTemplates() throws IOException { assertEquals(ObjectMapper.Subobjects.ENABLED, objectMapper.subobjects()); assertTrue(objectMapper.sourceKeepMode().isEmpty()); - // Setting 'enabled' to true is allowed, and updates the mapping. - update = Strings.toString( - XContentFactory.jsonBuilder() - .startObject() - .startObject("properties") - .startObject("object") - .field("type", "object") - .field("enabled", true) - .field("subobjects", "auto") - .field(ObjectMapper.STORE_ARRAY_SOURCE_PARAM, true) - .endObject() - .endObject() - .endObject() - ); - mapper = mapperService.merge("type", new CompressedXContent(update), MergeReason.INDEX_TEMPLATE); - - objectMapper = mapper.mappers().objectMappers().get("object"); - assertNotNull(objectMapper); - assertTrue(objectMapper.isEnabled()); - assertEquals(ObjectMapper.Subobjects.AUTO, objectMapper.subobjects()); - assertEquals(Mapper.SourceKeepMode.ARRAYS, objectMapper.sourceKeepMode().orElse(Mapper.SourceKeepMode.NONE)); + if (ObjectMapper.SUB_OBJECTS_AUTO_FEATURE_FLAG.isEnabled()) { + // Setting 'enabled' to true is allowed, and updates the mapping. + update = Strings.toString( + XContentFactory.jsonBuilder() + .startObject() + .startObject("properties") + .startObject("object") + .field("type", "object") + .field("enabled", true) + .field("subobjects", "auto") + .field(ObjectMapper.STORE_ARRAY_SOURCE_PARAM, true) + .endObject() + .endObject() + .endObject() + ); + mapper = mapperService.merge("type", new CompressedXContent(update), MergeReason.INDEX_TEMPLATE); + + objectMapper = mapper.mappers().objectMappers().get("object"); + assertNotNull(objectMapper); + assertTrue(objectMapper.isEnabled()); + assertEquals(ObjectMapper.Subobjects.AUTO, objectMapper.subobjects()); + assertEquals(Mapper.SourceKeepMode.ARRAYS, objectMapper.sourceKeepMode().orElse(Mapper.SourceKeepMode.NONE)); + } } public void testFieldReplacementForIndexTemplates() throws IOException { @@ -503,6 +505,7 @@ public void testSubobjectsCannotBeUpdatedOnRoot() throws IOException { } public void testSubobjectsAuto() throws Exception { + assumeTrue("only test when feature flag for subobjects auto is enabled", ObjectMapper.SUB_OBJECTS_AUTO_FEATURE_FLAG.isEnabled()); MapperService mapperService = createMapperService(mapping(b -> { b.startObject("metrics.service"); { @@ -532,6 +535,7 @@ public void testSubobjectsAuto() throws Exception { } public void testSubobjectsAutoWithInnerObject() throws IOException { + assumeTrue("only test when feature flag for subobjects auto is enabled", ObjectMapper.SUB_OBJECTS_AUTO_FEATURE_FLAG.isEnabled()); MapperService mapperService = createMapperService(mapping(b -> { b.startObject("metrics.service"); { @@ -565,6 +569,7 @@ public void testSubobjectsAutoWithInnerObject() throws IOException { } public void testSubobjectsAutoWithInnerNested() throws IOException { + assumeTrue("only test when feature flag for subobjects auto is enabled", ObjectMapper.SUB_OBJECTS_AUTO_FEATURE_FLAG.isEnabled()); MapperService mapperService = createMapperService(mapping(b -> { b.startObject("metrics.service"); { @@ -586,6 +591,7 @@ public void testSubobjectsAutoWithInnerNested() throws IOException { } public void testSubobjectsAutoRoot() throws Exception { + assumeTrue("only test when feature flag for subobjects auto is enabled", ObjectMapper.SUB_OBJECTS_AUTO_FEATURE_FLAG.isEnabled()); MapperService mapperService = createMapperService(mappingWithSubobjects(b -> { b.startObject("metrics.service.time"); b.field("type", "long"); @@ -606,6 +612,7 @@ public void testSubobjectsAutoRoot() throws Exception { } public void testSubobjectsAutoRootWithInnerObject() throws IOException { + assumeTrue("only test when feature flag for subobjects auto is enabled", ObjectMapper.SUB_OBJECTS_AUTO_FEATURE_FLAG.isEnabled()); MapperService mapperService = createMapperService(mappingWithSubobjects(b -> { b.startObject("metrics.service.time"); { @@ -626,6 +633,7 @@ public void testSubobjectsAutoRootWithInnerObject() throws IOException { } public void testSubobjectsAutoRootWithInnerNested() throws IOException { + assumeTrue("only test when feature flag for subobjects auto is enabled", ObjectMapper.SUB_OBJECTS_AUTO_FEATURE_FLAG.isEnabled()); MapperService mapperService = createMapperService(mappingWithSubobjects(b -> { b.startObject("metrics.service"); b.field("type", "nested"); diff --git a/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/FeatureFlag.java b/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/FeatureFlag.java index aa72d3248812e..ca2300611b4fd 100644 --- a/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/FeatureFlag.java +++ b/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/FeatureFlag.java @@ -18,6 +18,7 @@ public enum FeatureFlag { TIME_SERIES_MODE("es.index_mode_feature_flag_registered=true", Version.fromString("8.0.0"), null), FAILURE_STORE_ENABLED("es.failure_store_feature_flag_enabled=true", Version.fromString("8.12.0"), null), + SUB_OBJECTS_AUTO_ENABLED("es.sub_objects_auto_feature_flag_enabled=true", Version.fromString("8.16.0"), null), CHUNKING_SETTINGS_ENABLED("es.inference_chunking_settings_feature_flag_enabled=true", Version.fromString("8.16.0"), null), INFERENCE_DEFAULT_ELSER("es.inference_default_elser_feature_flag_enabled=true", Version.fromString("8.16.0"), null); diff --git a/x-pack/plugin/src/yamlRestTest/java/org/elasticsearch/xpack/test/rest/XPackRestIT.java b/x-pack/plugin/src/yamlRestTest/java/org/elasticsearch/xpack/test/rest/XPackRestIT.java index 556a417fb5e79..988ee93bda6b4 100644 --- a/x-pack/plugin/src/yamlRestTest/java/org/elasticsearch/xpack/test/rest/XPackRestIT.java +++ b/x-pack/plugin/src/yamlRestTest/java/org/elasticsearch/xpack/test/rest/XPackRestIT.java @@ -43,6 +43,7 @@ public class XPackRestIT extends AbstractXPackRestTest { .setting("xpack.searchable.snapshot.shared_cache.region_size", "256KB") .user("x_pack_rest_user", "x-pack-test-password") .feature(FeatureFlag.TIME_SERIES_MODE) + .feature(FeatureFlag.SUB_OBJECTS_AUTO_ENABLED) .configFile("testnode.pem", Resource.fromClasspath("org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.pem")) .configFile("testnode.crt", Resource.fromClasspath("org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.crt")) .configFile("service_tokens", Resource.fromClasspath("service_tokens")) diff --git a/x-pack/qa/core-rest-tests-with-security/src/yamlRestTest/java/org/elasticsearch/xpack/security/CoreWithSecurityClientYamlTestSuiteIT.java b/x-pack/qa/core-rest-tests-with-security/src/yamlRestTest/java/org/elasticsearch/xpack/security/CoreWithSecurityClientYamlTestSuiteIT.java index fe62d4e2d2639..0b40828b8e86c 100644 --- a/x-pack/qa/core-rest-tests-with-security/src/yamlRestTest/java/org/elasticsearch/xpack/security/CoreWithSecurityClientYamlTestSuiteIT.java +++ b/x-pack/qa/core-rest-tests-with-security/src/yamlRestTest/java/org/elasticsearch/xpack/security/CoreWithSecurityClientYamlTestSuiteIT.java @@ -48,6 +48,7 @@ public class CoreWithSecurityClientYamlTestSuiteIT extends ESClientYamlSuiteTest .setting("xpack.security.autoconfiguration.enabled", "false") .user(USER, PASS) .feature(FeatureFlag.TIME_SERIES_MODE) + .feature(FeatureFlag.SUB_OBJECTS_AUTO_ENABLED) .build(); public CoreWithSecurityClientYamlTestSuiteIT(@Name("yaml") ClientYamlTestCandidate testCandidate) { diff --git a/x-pack/qa/runtime-fields/build.gradle b/x-pack/qa/runtime-fields/build.gradle index 43d6d9463e0d1..986baf867b501 100644 --- a/x-pack/qa/runtime-fields/build.gradle +++ b/x-pack/qa/runtime-fields/build.gradle @@ -44,6 +44,7 @@ subprojects { setting 'xpack.security.enabled', 'false' requiresFeature 'es.index_mode_feature_flag_registered', Version.fromString("8.0.0") + requiresFeature 'es.sub_objects_auto_feature_flag_enabled', Version.fromString("8.16.0") } tasks.named("yamlRestTest").configure {