1
0
Эх сурвалжийг харах

Only disable unlicensed pattern_text templating when mapper is used (#135561)

Currently we always set the `index.mapping.pattern_text.disable_templating`
setting for every standard-mode index. This is a bit excessive, so this PR
updates the logic to only include the setting if pattern_text field mappers 
are in use.
Jordan Powers 6 өдөр өмнө
parent
commit
e9c2510048

+ 13 - 1
test/framework/src/main/java/org/elasticsearch/index/MapperTestUtils.java

@@ -11,6 +11,7 @@ package org.elasticsearch.index;
 
 import org.elasticsearch.TransportVersion;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
+import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
 import org.elasticsearch.env.Environment;
@@ -61,6 +62,17 @@ public class MapperTestUtils {
         Settings settings,
         IndicesModule indicesModule,
         String indexName
+    ) throws IOException {
+        return newMapperService(xContentRegistry, tempDir, settings, indicesModule, indexName, new Setting<?>[0]);
+    }
+
+    public static MapperService newMapperService(
+        NamedXContentRegistry xContentRegistry,
+        Path tempDir,
+        Settings settings,
+        IndicesModule indicesModule,
+        String indexName,
+        Setting<?>[] additionalSettings
     ) throws IOException {
         Settings.Builder settingsBuilder = Settings.builder().put(Environment.PATH_HOME_SETTING.getKey(), tempDir).put(settings);
         if (settings.get(IndexMetadata.SETTING_VERSION_CREATED) == null) {
@@ -68,7 +80,7 @@ public class MapperTestUtils {
         }
         Settings finalSettings = settingsBuilder.build();
         MapperRegistry mapperRegistry = indicesModule.getMapperRegistry();
-        IndexSettings indexSettings = IndexSettingsModule.newIndexSettings(indexName, finalSettings);
+        IndexSettings indexSettings = IndexSettingsModule.newIndexSettings(indexName, finalSettings, additionalSettings);
         IndexAnalyzers indexAnalyzers = createTestAnalysis(indexSettings, finalSettings).indexAnalyzers;
         SimilarityService similarityService = new SimilarityService(indexSettings, null, Collections.emptyMap());
         BitsetFilterCache bitsetFilterCache = new BitsetFilterCache(indexSettings, BitsetFilterCache.Listener.NOOP);

+ 36 - 7
x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsdbIndexModeSettingsProvider.java

@@ -11,6 +11,7 @@ import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.lucene.util.SetOnce;
 import org.elasticsearch.Version;
+import org.elasticsearch.action.admin.cluster.stats.MappingVisitor;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
 import org.elasticsearch.cluster.metadata.MetadataIndexTemplateService;
 import org.elasticsearch.cluster.metadata.ProjectMetadata;
@@ -19,6 +20,7 @@ import org.elasticsearch.common.compress.CompressedXContent;
 import org.elasticsearch.common.regex.Regex;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentHelper;
+import org.elasticsearch.common.xcontent.support.XContentMapValues;
 import org.elasticsearch.core.CheckedFunction;
 import org.elasticsearch.core.Strings;
 import org.elasticsearch.index.IndexMode;
@@ -35,12 +37,15 @@ import org.elasticsearch.index.mapper.ObjectMapper;
 import org.elasticsearch.index.mapper.SourceFieldMapper;
 import org.elasticsearch.xcontent.XContentType;
 import org.elasticsearch.xpack.logsdb.patterntext.PatternTextFieldMapper;
+import org.elasticsearch.xpack.logsdb.patterntext.PatternTextFieldType;
 
 import java.io.IOException;
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.function.Supplier;
 
@@ -192,13 +197,13 @@ final class LogsdbIndexModeSettingsProvider implements IndexSettingProvider {
             }
         }
 
-        if (licenseService.allowPatternTextTemplating(isTemplateValidation) == false) {
+        if (licenseService.allowPatternTextTemplating(isTemplateValidation) == false && mappingHints.maybeUsesPatternText) {
             additionalSettings.put(PatternTextFieldMapper.DISABLE_TEMPLATING_SETTING.getKey(), true);
         }
     }
 
-    record MappingHints(boolean hasSyntheticSourceUsage, boolean sortOnHostName, boolean addHostNameField) {
-        static MappingHints EMPTY = new MappingHints(false, false, false);
+    record MappingHints(boolean hasSyntheticSourceUsage, boolean sortOnHostName, boolean addHostNameField, boolean maybeUsesPatternText) {
+        static MappingHints EMPTY = new MappingHints(false, false, false, false);
     }
 
     private static boolean matchesLogsPattern(final String name) {
@@ -237,16 +242,17 @@ final class LogsdbIndexModeSettingsProvider implements IndexSettingProvider {
                 hasSyntheticSourceUsage = sourceMode == SourceFieldMapper.Mode.SYNTHETIC;
                 if (IndexSortConfig.INDEX_SORT_FIELD_SETTING.get(indexTemplateAndCreateRequestSettings).isEmpty() == false) {
                     // Custom sort config, no point for further checks on [host.name] field.
-                    return new MappingHints(hasSyntheticSourceUsage, false, false);
+                    return new MappingHints(hasSyntheticSourceUsage, false, false, true);
                 }
                 if (IndexSettings.LOGSDB_SORT_ON_HOST_NAME.get(indexTemplateAndCreateRequestSettings)
                     && IndexSettings.LOGSDB_ADD_HOST_NAME_FIELD.get(indexTemplateAndCreateRequestSettings)) {
                     // Settings for adding and sorting on [host.name] are already set, propagate them.
-                    return new MappingHints(hasSyntheticSourceUsage, true, true);
+                    return new MappingHints(hasSyntheticSourceUsage, true, true, true);
                 }
             }
 
             try (var mapperService = mapperServiceFactory.get().apply(tmpIndexMetadata)) {
+                boolean maybeUsesPatternText = PatternTextFieldMapper.DISABLE_TEMPLATING_SETTING.get(indexTemplateAndCreateRequestSettings);
                 // combinedTemplateMappings can be null when creating system indices
                 // combinedTemplateMappings can be empty when creating a normal index that doesn't match any template and without mapping.
                 if (combinedTemplateMappings == null || combinedTemplateMappings.isEmpty()) {
@@ -258,9 +264,17 @@ final class LogsdbIndexModeSettingsProvider implements IndexSettingProvider {
                     // The _doc.properties.host* is needed to determine whether host.name field can be injected.
                     // The _doc.subobjects is needed to determine whether subobjects is enabled.
                     List<CompressedXContent> filteredMappings = new ArrayList<>(combinedTemplateMappings.size());
+                    String[] mappingIncludesArray = MAPPING_INCLUDES.toArray(String[]::new);
                     for (CompressedXContent mappingSource : combinedTemplateMappings) {
                         var ref = mappingSource.compressedReference();
-                        var map = XContentHelper.convertToMap(ref, true, XContentType.JSON, MAPPING_INCLUDES, Set.of()).v2();
+                        Map<String, Object> map;
+                        if (maybeUsesPatternText == false) {
+                            var fullMap = XContentHelper.convertToMap(ref, true, XContentType.JSON).v2();
+                            maybeUsesPatternText = checkMappingForPatternText(fullMap);
+                            map = XContentMapValues.filter(fullMap, mappingIncludesArray, new String[0]);
+                        } else {
+                            map = XContentHelper.convertToMap(ref, true, XContentType.JSON, MAPPING_INCLUDES, Set.of()).v2();
+                        }
                         filteredMappings.add(new CompressedXContent(map));
                     }
                     combinedTemplateMappings = filteredMappings;
@@ -277,7 +291,7 @@ final class LogsdbIndexModeSettingsProvider implements IndexSettingProvider {
                     || addHostNameField
                     || (hostName instanceof NumberFieldMapper nfm && nfm.fieldType().hasDocValues())
                     || (hostName instanceof KeywordFieldMapper kfm && kfm.fieldType().hasDocValues());
-                return new MappingHints(hasSyntheticSourceUsage, sortOnHostName, addHostNameField);
+                return new MappingHints(hasSyntheticSourceUsage, sortOnHostName, addHostNameField, maybeUsesPatternText);
             }
         } catch (AssertionError | Exception e) {
             // In case invalid mappings or setting are provided, then mapper service creation can fail.
@@ -288,6 +302,21 @@ final class LogsdbIndexModeSettingsProvider implements IndexSettingProvider {
         }
     }
 
+    @SuppressWarnings("unchecked")
+    private boolean checkMappingForPatternText(Map<String, Object> mapping) {
+        var docMapping = mapping.get("_doc");
+        if ((docMapping instanceof Map) == false) {
+            return false;
+        }
+        boolean[] usesPatternText = { false };
+        MappingVisitor.visitMapping((Map<String, Object>) docMapping, (field, fieldMapping) -> {
+            if (Objects.equals(fieldMapping.get("type"), PatternTextFieldType.CONTENT_TYPE)) {
+                usesPatternText[0] = true;
+            }
+        });
+        return usesPatternText[0];
+    }
+
     // Create a dummy IndexMetadata instance that can be used to create a MapperService in order to check whether synthetic source is used:
     private IndexMetadata buildIndexMetadataForMapperService(
         String indexName,

+ 53 - 16
x-pack/plugin/logsdb/src/test/java/org/elasticsearch/xpack/logsdb/LogsdbIndexModeSettingsProviderTests.java

@@ -18,6 +18,7 @@ import org.elasticsearch.cluster.metadata.MetadataIndexTemplateService;
 import org.elasticsearch.cluster.metadata.ProjectMetadata;
 import org.elasticsearch.cluster.metadata.Template;
 import org.elasticsearch.common.compress.CompressedXContent;
+import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.core.Tuple;
 import org.elasticsearch.index.IndexMode;
@@ -26,9 +27,12 @@ import org.elasticsearch.index.IndexSortConfig;
 import org.elasticsearch.index.IndexVersion;
 import org.elasticsearch.index.MapperTestUtils;
 import org.elasticsearch.index.mapper.SourceFieldMapper;
+import org.elasticsearch.indices.IndicesModule;
 import org.elasticsearch.license.License;
 import org.elasticsearch.license.LicenseService;
 import org.elasticsearch.license.MockLicenseState;
+import org.elasticsearch.license.XPackLicenseState;
+import org.elasticsearch.license.internal.XPackLicenseStatus;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xpack.logsdb.patterntext.PatternTextFieldMapper;
 import org.junit.Before;
@@ -72,6 +76,7 @@ public class LogsdbIndexModeSettingsProviderTests extends ESTestCase {
         """;
 
     private LogsdbLicenseService logsdbLicenseService;
+    private LogsdbLicenseService basicLogsdbLicenseService;
     private final AtomicInteger newMapperServiceCounter = new AtomicInteger();
 
     @Before
@@ -84,6 +89,13 @@ public class LogsdbIndexModeSettingsProviderTests extends ESTestCase {
         logsdbLicenseService = new LogsdbLicenseService(Settings.EMPTY);
         logsdbLicenseService.setLicenseState(licenseState);
         logsdbLicenseService.setLicenseService(mockLicenseService);
+
+        var basicLicenseState = new XPackLicenseState(() -> 0L, new XPackLicenseStatus(License.OperationMode.BASIC, true, null));
+        var basicLicenseService = mock(LicenseService.class);
+        when(basicLicenseService.getLicense()).thenReturn(null);
+        basicLogsdbLicenseService = new LogsdbLicenseService(Settings.EMPTY);
+        basicLogsdbLicenseService.setLicenseState(basicLicenseState);
+        basicLogsdbLicenseService.setLicenseService(basicLicenseService);
     }
 
     private LogsdbIndexModeSettingsProvider withSyntheticSourceDemotionSupport(boolean enabled) {
@@ -121,13 +133,23 @@ public class LogsdbIndexModeSettingsProviderTests extends ESTestCase {
     }
 
     private Settings generateLogsdbSettings(Settings settings, String mapping, Version version) throws IOException {
-        var provider = new LogsdbIndexModeSettingsProvider(
-            logsdbLicenseService,
-            Settings.builder().put("cluster.logsdb.enabled", true).build()
-        );
+        return generateLogsdbSettings(settings, mapping, version, logsdbLicenseService);
+    }
+
+    private Settings generateLogsdbSettings(Settings settings, String mapping, Version version, LogsdbLicenseService licenseService)
+        throws IOException {
+        var provider = new LogsdbIndexModeSettingsProvider(licenseService, Settings.builder().put("cluster.logsdb.enabled", true).build());
+        var logsdbPlugin = new LogsDBPlugin(settings);
         provider.init(im -> {
             newMapperServiceCounter.incrementAndGet();
-            return MapperTestUtils.newMapperService(xContentRegistry(), createTempDir(), im.getSettings(), im.getIndex().getName());
+            return MapperTestUtils.newMapperService(
+                xContentRegistry(),
+                createTempDir(),
+                im.getSettings(),
+                new IndicesModule(List.of(logsdbPlugin)),
+                im.getIndex().getName(),
+                logsdbPlugin.getSettings().stream().filter(Setting::hasIndexScope).toArray(Setting<?>[]::new)
+            );
         }, IndexVersion::current, () -> version, true, true);
         Settings.Builder settingsBuilder = builder();
         provider.provideAdditionalSettings(
@@ -959,19 +981,34 @@ public class LogsdbIndexModeSettingsProviderTests extends ESTestCase {
         assertThat(IndexMetadata.INDEX_ROUTING_PATH.get(result), empty());
     }
 
-    public void testPatternTextNotAllowedByLicense() throws Exception {
-        MockLicenseState licenseState = MockLicenseState.createMock();
-        when(licenseState.copyCurrentLicenseState()).thenReturn(licenseState);
-        when(licenseState.isAllowed(same(LogsdbLicenseService.PATTERN_TEXT_TEMPLATING_FEATURE))).thenReturn(false);
-        logsdbLicenseService = new LogsdbLicenseService(Settings.EMPTY);
-        logsdbLicenseService.setLicenseState(licenseState);
+    public void testPatternTextNotAllowedByLicense() throws IOException {
+        String[] patternTextLicenceCheckedFieldMappings = {
+            "{\"_doc\":{\"properties\":{\"message\":{\"type\":\"pattern_text\"}}}}",
+            "{\"_doc\":{\"properties\":{\"error\":{\"properties\":{\"message\":{\"type\":\"pattern_text\"}}}}}}",
+            "{\"_doc\":{\"properties\":{\"foo\":{\"type\":\"pattern_text\"}}}}",
+            "{\"_doc\":{\"properties\":{\"bar\":{\"properties\":{\"baz\":{\"type\":\"pattern_text\"}}}}}}" };
 
-        var settings = Settings.builder()
-            .put(IndexSortConfig.INDEX_SORT_FIELD_SETTING.getKey(), "host,message")
-            .put(IndexSettings.LOGSDB_ROUTE_ON_SORT_FIELDS.getKey(), true)
+        var expectedSettings = Settings.builder()
+            .put(IndexSettings.LOGSDB_ADD_HOST_NAME_FIELD.getKey(), true)
+            .put(IndexSettings.LOGSDB_SORT_ON_HOST_NAME.getKey(), true)
+            .put(PatternTextFieldMapper.DISABLE_TEMPLATING_SETTING.getKey(), true)
             .build();
-        Settings result = generateLogsdbSettings(settings);
-        assertTrue(PatternTextFieldMapper.DISABLE_TEMPLATING_SETTING.get(result));
+
+        for (String mapping : patternTextLicenceCheckedFieldMappings) {
+            var result = generateLogsdbSettings(Settings.EMPTY, mapping, Version.CURRENT, basicLogsdbLicenseService);
+            assertEquals(expectedSettings, result);
+        }
+    }
+
+    public void testPatternTextNotAllowedByLicenseAlreadyDisallowed() throws IOException {
+        Settings settings = Settings.builder().put(PatternTextFieldMapper.DISABLE_TEMPLATING_SETTING.getKey(), "true").build();
+        var result = generateLogsdbSettings(settings, null, Version.CURRENT, basicLogsdbLicenseService);
+        var expected = Settings.builder()
+            .put(IndexSettings.LOGSDB_ADD_HOST_NAME_FIELD.getKey(), true)
+            .put(IndexSettings.LOGSDB_SORT_ON_HOST_NAME.getKey(), true)
+            .put(PatternTextFieldMapper.DISABLE_TEMPLATING_SETTING.getKey(), true)
+            .build();
+        assertEquals(expected, result);
     }
 
     public void testSortAndHostNamePropagateValue() throws Exception {

+ 6 - 22
x-pack/plugin/logsdb/src/test/java/org/elasticsearch/xpack/logsdb/LogsdbIndexSettingsProviderLegacyLicenseTests.java

@@ -19,7 +19,6 @@ import org.elasticsearch.license.LicenseService;
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.license.internal.XPackLicenseStatus;
 import org.elasticsearch.test.ESTestCase;
-import org.elasticsearch.xpack.logsdb.patterntext.PatternTextFieldMapper;
 import org.junit.Before;
 
 import java.io.IOException;
@@ -77,10 +76,7 @@ public class LogsdbIndexSettingsProviderLegacyLicenseTests extends ESTestCase {
             builder
         );
         var result = builder.build();
-        var expected = Settings.builder()
-            .put(IndexSettings.INDEX_MAPPER_SOURCE_MODE_SETTING.getKey(), "STORED")
-            .put(PatternTextFieldMapper.DISABLE_TEMPLATING_SETTING.getKey(), true)
-            .build();
+        var expected = Settings.builder().put(IndexSettings.INDEX_MAPPER_SOURCE_MODE_SETTING.getKey(), "STORED").build();
         assertEquals(expected, result);
     }
 
@@ -101,17 +97,11 @@ public class LogsdbIndexSettingsProviderLegacyLicenseTests extends ESTestCase {
             builder
         );
         var result = builder.build();
-        Settings expectedAdditionalSettings = Settings.builder()
-            .put(PatternTextFieldMapper.DISABLE_TEMPLATING_SETTING.getKey(), true)
-            .build();
-        assertEquals(expectedAdditionalSettings, result);
+        assertEquals(Settings.EMPTY, result);
     }
 
     public void testGetAdditionalIndexSettingsProfiling() throws IOException {
         Settings settings = Settings.builder().put(IndexSettings.INDEX_MAPPER_SOURCE_MODE_SETTING.getKey(), "SYNTHETIC").build();
-        Settings expectedAdditionalSettings = Settings.builder()
-            .put(PatternTextFieldMapper.DISABLE_TEMPLATING_SETTING.getKey(), true)
-            .build();
         for (String dataStreamName : new String[] { "profiling-metrics", "profiling-events" }) {
             String indexName = DataStream.getDefaultBackingIndexName(dataStreamName, 0);
             Settings.Builder builder = Settings.builder();
@@ -127,14 +117,14 @@ public class LogsdbIndexSettingsProviderLegacyLicenseTests extends ESTestCase {
                 builder
             );
             var result = builder.build();
-            assertEquals(expectedAdditionalSettings, result);
+            assertEquals(Settings.EMPTY, result);
         }
 
         for (String indexName : new String[] { ".profiling-sq-executables", ".profiling-sq-leafframes", ".profiling-stacktraces" }) {
             Settings.Builder builder = Settings.builder();
             provider.provideAdditionalSettings(indexName, null, null, null, null, settings, List.of(), IndexVersion.current(), builder);
             var result = builder.build();
-            assertEquals(expectedAdditionalSettings, result);
+            assertEquals(Settings.EMPTY, result);
         }
     }
 
@@ -155,10 +145,7 @@ public class LogsdbIndexSettingsProviderLegacyLicenseTests extends ESTestCase {
             builder
         );
         var result = builder.build();
-        Settings expectedAdditionalSettings = Settings.builder()
-            .put(PatternTextFieldMapper.DISABLE_TEMPLATING_SETTING.getKey(), true)
-            .build();
-        assertEquals(expectedAdditionalSettings, result);
+        assertEquals(Settings.EMPTY, result);
     }
 
     public void testGetAdditionalIndexSettingsTsdbAfterCutoffDate() throws Exception {
@@ -202,10 +189,7 @@ public class LogsdbIndexSettingsProviderLegacyLicenseTests extends ESTestCase {
         );
 
         var result = builder.build();
-        var expected = Settings.builder()
-            .put(IndexSettings.INDEX_MAPPER_SOURCE_MODE_SETTING.getKey(), "STORED")
-            .put(PatternTextFieldMapper.DISABLE_TEMPLATING_SETTING.getKey(), true)
-            .build();
+        var expected = Settings.builder().put(IndexSettings.INDEX_MAPPER_SOURCE_MODE_SETTING.getKey(), "STORED").build();
         assertEquals(expected, result);
     }
 }