diff --git a/server/enterprise/elasticsearch/common/common.go b/server/enterprise/elasticsearch/common/common.go index bedc969587d..8b7dc7fafd3 100644 --- a/server/enterprise/elasticsearch/common/common.go +++ b/server/enterprise/elasticsearch/common/common.go @@ -10,6 +10,7 @@ import ( "net/url" "regexp" "runtime" + "slices" "strings" "time" @@ -50,6 +51,22 @@ var ( markdownLinkRe = regexp.MustCompile(URLMarkdownLinkRE) ) +// analysisPluginPrefixes lists the prefixes a managed service can prepend to the component name of +// an analysis plugin. AWS OpenSearch Service reports a plugin installed through associate-package +// as "opensearch-analysis-nori", while the plugins it bundles keep their unprefixed names. +var analysisPluginPrefixes = []string{"", "opensearch-"} + +// HasAnalysisPlugin reports whether the named analysis plugin is present in the plugin list +// reported by the cluster, accepting both the bundled and the prefixed component names. +func HasAnalysisPlugin(plugins []string, name string) bool { + for _, prefix := range analysisPluginPrefixes { + if slices.Contains(plugins, prefix+name) { + return true + } + } + return false +} + type ESPost struct { Id string `json:"id"` TeamId string `json:"team_id"` diff --git a/server/enterprise/elasticsearch/common/common_test.go b/server/enterprise/elasticsearch/common/common_test.go index d03279a9a08..c95875398b0 100644 --- a/server/enterprise/elasticsearch/common/common_test.go +++ b/server/enterprise/elasticsearch/common/common_test.go @@ -36,6 +36,31 @@ func TestElasticsearchBuildPostIndexName(t *testing.T) { assert.Equal(t, eightName, "postsmonth_2017_08") } +func TestHasAnalysisPlugin(t *testing.T) { + installed := []string{"analysis-icu", "analysis-kuromoji", "opensearch-analysis-nori"} + + testCases := []struct { + Name string + Plugins []string + Lookup string + Expected bool + }{ + {"bundled name", installed, "analysis-kuromoji", true}, + {"opensearch prefixed name", installed, "analysis-nori", true}, + {"not installed", installed, "analysis-stempel", false}, + {"no plugins reported", nil, "analysis-nori", false}, + {"unrelated prefix is not accepted", []string{"vendor-analysis-nori"}, "analysis-nori", false}, + {"elasticsearch prefix is not accepted", []string{"elasticsearch-analysis-nori"}, "analysis-nori", false}, + {"a longer name is not a match", []string{"analysis-noris"}, "analysis-nori", false}, + } + + for _, tc := range testCases { + t.Run(tc.Name, func(t *testing.T) { + assert.Equal(t, tc.Expected, HasAnalysisPlugin(tc.Plugins, tc.Lookup)) + }) + } +} + func TestESPostFromPostForIndexing(t *testing.T) { t.Run("any form with text only", func(t *testing.T) { post := model.PostForIndexing{ diff --git a/server/enterprise/elasticsearch/common/test_helpers.go b/server/enterprise/elasticsearch/common/test_helpers.go index cf7a8793166..320005225b9 100644 --- a/server/enterprise/elasticsearch/common/test_helpers.go +++ b/server/enterprise/elasticsearch/common/test_helpers.go @@ -4,12 +4,17 @@ package common import ( + "encoding/json" "fmt" + "slices" + "strings" + "sync" "testing" "github.com/mattermost/mattermost/server/public/model" "github.com/mattermost/mattermost/server/v8/channels/app/password/hashers" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func createPost(userId string, channelId string, message string) *model.Post { @@ -66,6 +71,171 @@ func createFile(creatorID, channelID, postID, content, name, extension string) * return file } +// ClusterRecorder records the request bodies a fake search cluster receives. Its methods are safe +// to call from a test server handler. +type ClusterRecorder struct { + mut sync.Mutex + postsTemplate []byte + searchBodies [][]byte +} + +func (r *ClusterRecorder) RecordPostsTemplate(body []byte) { + r.mut.Lock() + defer r.mut.Unlock() + + r.postsTemplate = body +} + +func (r *ClusterRecorder) RecordSearch(body []byte) { + r.mut.Lock() + defer r.mut.Unlock() + + r.searchBodies = append(r.searchBodies, body) +} + +func (r *ClusterRecorder) PostsTemplate() []byte { + r.mut.Lock() + defer r.mut.Unlock() + + return r.postsTemplate +} + +func (r *ClusterRecorder) SearchBodies() [][]byte { + r.mut.Lock() + defer r.mut.Unlock() + + return slices.Clone(r.searchBodies) +} + +func (r *ClusterRecorder) ResetSearches() { + r.mut.Lock() + defer r.mut.Unlock() + + r.searchBodies = nil +} + +// NodesPluginsResponse builds a response for the node info plugins metric, reporting one node per +// given list of plugin names. +func NodesPluginsResponse(nodePlugins ...[]string) string { + nodes := make([]string, 0, len(nodePlugins)) + for i, pluginNames := range nodePlugins { + plugins := make([]string, 0, len(pluginNames)) + for _, name := range pluginNames { + plugins = append(plugins, fmt.Sprintf(`{"name":%q}`, name)) + } + + name := fmt.Sprintf("node-%d", i+1) + nodes = append(nodes, fmt.Sprintf(`%q:{"name":%q,"plugins":[%s]}`, name, name, strings.Join(plugins, ","))) + } + + return fmt.Sprintf(`{"_nodes":{"total":%d,"successful":%d,"failed":0},"nodes":{%s}}`, + len(nodePlugins), len(nodePlugins), strings.Join(nodes, ",")) +} + +type indexTemplateRequest struct { + Template struct { + Settings struct { + Analysis struct { + Analyzer map[string]json.RawMessage `json:"analyzer"` + } `json:"analysis"` + } `json:"settings"` + Mappings struct { + Properties map[string]struct { + Fields map[string]struct { + Analyzer string `json:"analyzer"` + } `json:"fields"` + } `json:"properties"` + } `json:"mappings"` + } `json:"template"` +} + +func parseIndexTemplate(t *testing.T, body []byte) indexTemplateRequest { + t.Helper() + + var request indexTemplateRequest + require.NoError(t, json.Unmarshal(body, &request)) + + return request +} + +// TemplatePropertyFields returns the sub-fields of the named property mapped to their analyzer, as +// declared by an index template request body. +func TemplatePropertyFields(t *testing.T, body []byte, property string) map[string]string { + t.Helper() + + fields := map[string]string{} + for name, field := range parseIndexTemplate(t, body).Template.Mappings.Properties[property].Fields { + fields[name] = field.Analyzer + } + + return fields +} + +// TemplateAnalyzers returns the names of the analyzers declared by an index template request body. +func TemplateAnalyzers(t *testing.T, body []byte) []string { + t.Helper() + + declared := parseIndexTemplate(t, body).Template.Settings.Analysis.Analyzer + analyzers := make([]string, 0, len(declared)) + for name := range declared { + analyzers = append(analyzers, name) + } + + return analyzers +} + +// SimpleQueryStringFields collects the field list of every simple_query_string clause in a search +// request body. The clauses are collected in an unspecified order. +func SimpleQueryStringFields(t *testing.T, body []byte) [][]string { + t.Helper() + + var doc any + require.NoError(t, json.Unmarshal(body, &doc)) + + var collected [][]string + var walk func(node any) + walk = func(node any) { + switch typed := node.(type) { + case map[string]any: + if clause, ok := typed["simple_query_string"].(map[string]any); ok { + rawFields, ok := clause["fields"].([]any) + require.True(t, ok, "simple_query_string clause without fields: %v", clause) + fields := make([]string, 0, len(rawFields)) + for _, rawField := range rawFields { + field, ok := rawField.(string) + require.True(t, ok, "non-string field name: %v", rawField) + fields = append(fields, field) + } + collected = append(collected, fields) + } + for _, value := range typed { + walk(value) + } + case []any: + for _, value := range typed { + walk(value) + } + } + } + walk(doc) + + return collected +} + +// RequireOnlyBaseFields asserts that the given search field lists cover the base message and +// attachments fields and target no analyzer sub-field such as message.nori. +func RequireOnlyBaseFields(t *testing.T, fields [][]string) { + t.Helper() + + require.Contains(t, fields, []string{"message"}) + require.Contains(t, fields, []string{"attachments"}) + for _, fieldList := range fields { + for _, field := range fieldList { + require.NotContains(t, field, ".", "unexpected analyzer sub-field in %v", fieldList) + } + } +} + func CheckMatchesEqual(t *testing.T, expected model.PostSearchMatches, actual map[string][]string) { a := assert.New(t) diff --git a/server/enterprise/elasticsearch/elasticsearch/elasticsearch.go b/server/enterprise/elasticsearch/elasticsearch/elasticsearch.go index 6952a0120b9..a634b563411 100644 --- a/server/enterprise/elasticsearch/elasticsearch/elasticsearch.go +++ b/server/enterprise/elasticsearch/elasticsearch/elasticsearch.go @@ -171,14 +171,12 @@ func (es *ElasticsearchInterfaceImpl) fetchServerInfo(ctx context.Context, clien es.plugins = nil analysisICUInstalledOnEveryNode := true for _, node := range resp.Nodes { - nodeHasAnalysisICU := false + nodePlugins := make([]string, 0, len(node.Plugins)) for _, plugin := range node.Plugins { - es.plugins = append(es.plugins, plugin.Name) - if plugin.Name == "analysis-icu" { - nodeHasAnalysisICU = true - } + nodePlugins = append(nodePlugins, plugin.Name) } - analysisICUInstalledOnEveryNode = analysisICUInstalledOnEveryNode && nodeHasAnalysisICU + es.plugins = append(es.plugins, nodePlugins...) + analysisICUInstalledOnEveryNode = analysisICUInstalledOnEveryNode && common.HasAnalysisPlugin(nodePlugins, "analysis-icu") } if len(resp.Nodes) > 0 && !analysisICUInstalledOnEveryNode { @@ -216,13 +214,13 @@ func (es *ElasticsearchInterfaceImpl) Start(ctx context.Context) *model.AppError opts := []func(*types.IndexTemplateMapping){} // Set up additional analyzers to use in the post index template if CJK analyzers are enabled if *es.Platform.Config().ElasticsearchSettings.EnableCJKAnalyzers { - if slices.Contains(es.plugins, "analysis-nori") { + if common.HasAnalysisPlugin(es.plugins, "analysis-nori") { opts = append(opts, common.WithNoriAnalyzer()) } - if slices.Contains(es.plugins, "analysis-kuromoji") { + if common.HasAnalysisPlugin(es.plugins, "analysis-kuromoji") { opts = append(opts, common.WithKuromojiAnalyzer()) } - if slices.Contains(es.plugins, "analysis-smartcn") { + if common.HasAnalysisPlugin(es.plugins, "analysis-smartcn") { opts = append(opts, common.WithSmartCNAnalyzer()) } @@ -468,15 +466,15 @@ func (es *ElasticsearchInterfaceImpl) getFieldVariants(fieldName string, query s return variants } - if slices.Contains(es.plugins, "analysis-nori") { + if common.HasAnalysisPlugin(es.plugins, "analysis-nori") { variants = append(variants, fieldName+".nori") } - if slices.Contains(es.plugins, "analysis-kuromoji") { + if common.HasAnalysisPlugin(es.plugins, "analysis-kuromoji") { variants = append(variants, fieldName+".kuromoji") } - if slices.Contains(es.plugins, "analysis-smartcn") { + if common.HasAnalysisPlugin(es.plugins, "analysis-smartcn") { variants = append(variants, fieldName+".smartcn") } diff --git a/server/enterprise/elasticsearch/elasticsearch/elasticsearch_test.go b/server/enterprise/elasticsearch/elasticsearch/elasticsearch_test.go index 7c701b9d11d..8fea39703d0 100644 --- a/server/enterprise/elasticsearch/elasticsearch/elasticsearch_test.go +++ b/server/enterprise/elasticsearch/elasticsearch/elasticsearch_test.go @@ -9,6 +9,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "net/http" "net/http/httptest" "strings" @@ -16,11 +17,14 @@ import ( elastic "github.com/elastic/go-elasticsearch/v8" "github.com/elastic/go-elasticsearch/v8/typedapi/types" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/shared/mlog" "github.com/mattermost/mattermost/server/v8/channels/api4" + "github.com/mattermost/mattermost/server/v8/channels/testlib" "github.com/mattermost/mattermost/server/v8/enterprise/elasticsearch/common" "github.com/mattermost/mattermost/server/v8/platform/shared/filestore" "github.com/mattermost/mattermost/server/v8/platform/shared/filestore/mocks" @@ -312,6 +316,187 @@ func TestStartWithoutAnalysisICUReturnsExplicitError(t *testing.T) { } } +// missingCJKPluginsWarning mirrors the warning Start logs when no CJK analyzer plugin is detected. +const missingCJKPluginsWarning = "EnableCJKAnalyzers is set but no CJK analyzer plugins found installed. Please review elasticsearch settings." + +// pluginsHandler serves the endpoints Start and SearchPosts need, reporting one node per given list +// of plugin names and recording the posts template and search request bodies. +func pluginsHandler(t *testing.T, recorder *common.ClusterRecorder, nodePlugins ...[]string) http.HandlerFunc { + info := infoHandler("8.19.0") + nodesInfo := common.NodesPluginsResponse(nodePlugins...) + + readBody := func(w http.ResponseWriter, r *http.Request) ([]byte, bool) { + body, err := io.ReadAll(r.Body) + if !assert.NoError(t, err) { + http.Error(w, err.Error(), http.StatusInternalServerError) + return nil, false + } + return body, true + } + + return func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Header().Set("X-Elastic-Product", "Elasticsearch") + switch { + case r.Method == http.MethodGet && r.URL.Path == "/": + info(w, r) + case r.Method == http.MethodGet && r.URL.Path == "/_nodes/plugins": + _, _ = fmt.Fprint(w, nodesInfo) + case r.Method == http.MethodPut && strings.Contains(r.URL.Path, "/_index_template/"): + body, ok := readBody(w, r) + if !ok { + return + } + if strings.HasSuffix(r.URL.Path, common.IndexBasePosts) { + recorder.RecordPostsTemplate(body) + } + _, _ = fmt.Fprint(w, `{"acknowledged":true}`) + case strings.HasSuffix(r.URL.Path, "/_search"): + body, ok := readBody(w, r) + if !ok { + return + } + recorder.RecordSearch(body) + _, _ = fmt.Fprint(w, `{"took":1,"timed_out":false,"hits":{"total":{"value":0,"relation":"eq"},"max_score":0,"hits":[]}}`) + default: + w.WriteHeader(http.StatusInternalServerError) + } + } +} + +func setupCJKCluster(t *testing.T, recorder *common.ClusterRecorder, nodePlugins ...[]string) (*api4.TestHelper, *ElasticsearchInterfaceImpl) { + server := httptest.NewServer(pluginsHandler(t, recorder, nodePlugins...)) + t.Cleanup(server.Close) + + t.Setenv("MM_ELASTICSEARCHSETTINGS_CONNECTIONURL", server.URL) + t.Setenv("MM_ELASTICSEARCHSETTINGS_BACKEND", model.ElasticsearchSettingsESBackend) + + th := api4.SetupEnterprise(t) + th.App.Srv().SetLicense(model.NewTestLicense()) + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ElasticsearchSettings.ConnectionURL = server.URL + *cfg.ElasticsearchSettings.EnableIndexing = true + *cfg.ElasticsearchSettings.EnableSearching = true + *cfg.ElasticsearchSettings.EnableCJKAnalyzers = true + }) + + es := &ElasticsearchInterfaceImpl{Platform: th.Server.Platform()} + t.Cleanup(func() { require.Nil(t, es.Stop()) }) + + return th, es +} + +// TestCJKAnalyzersWithPrefixedPluginNames covers a cluster that reports analysis plugins under the +// prefixed component name a managed service assigns to plugins it does not bundle, as AWS +// OpenSearch Service does for the Korean analyzer installed through associate-package. +func TestCJKAnalyzersWithPrefixedPluginNames(t *testing.T) { + namings := []struct { + Name string + Plugins []string + }{ + {"only nori prefixed", []string{"analysis-icu", "opensearch-analysis-nori", "analysis-kuromoji", "analysis-smartcn"}}, + {"every analysis plugin prefixed", []string{"opensearch-analysis-icu", "opensearch-analysis-nori", "opensearch-analysis-kuromoji", "opensearch-analysis-smartcn"}}, + } + + for _, naming := range namings { + t.Run(naming.Name, func(t *testing.T) { + recorder := &common.ClusterRecorder{} + th, es := setupCJKCluster(t, recorder, naming.Plugins) + require.Nil(t, es.Start(context.Background())) + + expectedSubFields := map[string]string{ + "nori": "mm_nori", + "kuromoji": "mm_kuromoji", + "smartcn": "mm_smartcn", + } + + t.Run("the posts index template maps every CJK sub-field", func(t *testing.T) { + template := recorder.PostsTemplate() + require.NotEmpty(t, template, "no posts index template was created") + + require.Equal(t, expectedSubFields, common.TemplatePropertyFields(t, template, "message")) + require.Equal(t, expectedSubFields, common.TemplatePropertyFields(t, template, "attachments")) + require.Subset(t, common.TemplateAnalyzers(t, template), []string{"mm_nori", "mm_kuromoji", "mm_smartcn"}) + }) + + t.Run("no missing plugin warning is logged", func(t *testing.T) { + require.NoError(t, th.TestLogger.Flush()) + testlib.AssertNoLog(t, th.LogBuffer, mlog.LvlWarn.Name, missingCJKPluginsWarning) + }) + + channels := model.ChannelList{{Id: model.NewId(), TeamId: model.NewId(), Type: model.ChannelTypeOpen}} + + searchFields := func(t *testing.T, terms string) [][]string { + t.Helper() + + recorder.ResetSearches() + _, _, appErr := es.SearchPosts(channels, model.ParseSearchParams(terms, 0), 0, 20) + require.Nil(t, appErr) + + bodies := recorder.SearchBodies() + require.Len(t, bodies, 1) + + return common.SimpleQueryStringFields(t, bodies[0]) + } + + t.Run("a CJK query searches the CJK sub-fields", func(t *testing.T) { + fields := searchFields(t, "검색") + require.Contains(t, fields, []string{"message", "message.nori", "message.kuromoji", "message.smartcn"}) + require.Contains(t, fields, []string{"attachments", "attachments.nori", "attachments.kuromoji", "attachments.smartcn"}) + }) + + t.Run("a non-CJK query only searches the base fields", func(t *testing.T) { + common.RequireOnlyBaseFields(t, searchFields(t, "search")) + }) + + t.Run("a CJK query only searches the base fields when CJK analyzers are disabled", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ElasticsearchSettings.EnableCJKAnalyzers = false + }) + defer th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ElasticsearchSettings.EnableCJKAnalyzers = true + }) + + common.RequireOnlyBaseFields(t, searchFields(t, "검색")) + }) + }) + } +} + +// TestCJKAnalyzersWithoutAnyCJKPlugin covers the diagnostic that made this failure mode hard to +// spot: the warning only fires when no CJK analyzer plugin is detected at all. +func TestCJKAnalyzersWithoutAnyCJKPlugin(t *testing.T) { + recorder := &common.ClusterRecorder{} + th, es := setupCJKCluster(t, recorder, []string{"analysis-icu"}) + require.Nil(t, es.Start(context.Background())) + + template := recorder.PostsTemplate() + require.NotEmpty(t, template, "no posts index template was created") + require.Empty(t, common.TemplatePropertyFields(t, template, "message")) + + channels := model.ChannelList{{Id: model.NewId(), TeamId: model.NewId(), Type: model.ChannelTypeOpen}} + _, _, appErr := es.SearchPosts(channels, model.ParseSearchParams("검색", 0), 0, 20) + require.Nil(t, appErr) + + bodies := recorder.SearchBodies() + require.Len(t, bodies, 1) + common.RequireOnlyBaseFields(t, common.SimpleQueryStringFields(t, bodies[0])) + + require.NoError(t, th.TestLogger.Flush()) + testlib.AssertLog(t, th.LogBuffer, mlog.LvlWarn.Name, missingCJKPluginsWarning) +} + +// TestAnalysisICURequirementAcceptsPrefixedNames covers the per-node analysis-icu requirement when +// only some of the nodes report the plugin under a prefixed name. +func TestAnalysisICURequirementAcceptsPrefixedNames(t *testing.T) { + recorder := &common.ClusterRecorder{} + _, es := setupCJKCluster(t, recorder, []string{"opensearch-analysis-icu"}, []string{"analysis-icu"}) + + require.Nil(t, es.Start(context.Background())) + require.Equal(t, int32(1), es.ready.Load()) + require.NotEmpty(t, recorder.PostsTemplate(), "no posts index template was created") +} + func TestWrapElasticsearchTemplateError(t *testing.T) { nestedReason := "Custom Analyzer [mm_lowercaser] failed to find tokenizer under name [icu_tokenizer]" nested := &types.ErrorCause{Type: "illegal_argument_exception", Reason: &nestedReason} diff --git a/server/enterprise/elasticsearch/opensearch/opensearch.go b/server/enterprise/elasticsearch/opensearch/opensearch.go index 26e5faf1813..0ffffee643d 100644 --- a/server/enterprise/elasticsearch/opensearch/opensearch.go +++ b/server/enterprise/elasticsearch/opensearch/opensearch.go @@ -142,14 +142,12 @@ func (os *OpensearchInterfaceImpl) fetchServerInfo(ctx context.Context, client * os.plugins = nil analysisICUInstalledOnEveryNode := true for _, node := range resp.Nodes { - nodeHasAnalysisICU := false + nodePlugins := make([]string, 0, len(node.Plugins)) for _, plugin := range node.Plugins { - os.plugins = append(os.plugins, plugin.Name) - if plugin.Name == "analysis-icu" { - nodeHasAnalysisICU = true - } + nodePlugins = append(nodePlugins, plugin.Name) } - analysisICUInstalledOnEveryNode = analysisICUInstalledOnEveryNode && nodeHasAnalysisICU + os.plugins = append(os.plugins, nodePlugins...) + analysisICUInstalledOnEveryNode = analysisICUInstalledOnEveryNode && common.HasAnalysisPlugin(nodePlugins, "analysis-icu") } if len(resp.Nodes) > 0 && !analysisICUInstalledOnEveryNode { @@ -187,13 +185,13 @@ func (os *OpensearchInterfaceImpl) Start(ctx context.Context) *model.AppError { opts := []func(*types.IndexTemplateMapping){} // Set up additional analyzers to use in the post index template if CJK analyzers are enabled if *os.Platform.Config().ElasticsearchSettings.EnableCJKAnalyzers { - if slices.Contains(os.plugins, "analysis-nori") { + if common.HasAnalysisPlugin(os.plugins, "analysis-nori") { opts = append(opts, common.WithNoriAnalyzer()) } - if slices.Contains(os.plugins, "analysis-kuromoji") { + if common.HasAnalysisPlugin(os.plugins, "analysis-kuromoji") { opts = append(opts, common.WithKuromojiAnalyzer()) } - if slices.Contains(os.plugins, "analysis-smartcn") { + if common.HasAnalysisPlugin(os.plugins, "analysis-smartcn") { opts = append(opts, common.WithSmartCNAnalyzer()) } @@ -429,15 +427,15 @@ func (os *OpensearchInterfaceImpl) getFieldVariants(fieldName string, query stri return variants } - if slices.Contains(os.plugins, "analysis-nori") { + if common.HasAnalysisPlugin(os.plugins, "analysis-nori") { variants = append(variants, fieldName+".nori") } - if slices.Contains(os.plugins, "analysis-kuromoji") { + if common.HasAnalysisPlugin(os.plugins, "analysis-kuromoji") { variants = append(variants, fieldName+".kuromoji") } - if slices.Contains(os.plugins, "analysis-smartcn") { + if common.HasAnalysisPlugin(os.plugins, "analysis-smartcn") { variants = append(variants, fieldName+".smartcn") } diff --git a/server/enterprise/elasticsearch/opensearch/opensearch_test.go b/server/enterprise/elasticsearch/opensearch/opensearch_test.go index 6979a61fd0e..0bad1f5b002 100644 --- a/server/enterprise/elasticsearch/opensearch/opensearch_test.go +++ b/server/enterprise/elasticsearch/opensearch/opensearch_test.go @@ -9,6 +9,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "net/http" "net/http/httptest" "os" @@ -19,11 +20,14 @@ import ( "github.com/opensearch-project/opensearch-go/v4" "github.com/opensearch-project/opensearch-go/v4/opensearchapi" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/shared/mlog" "github.com/mattermost/mattermost/server/v8/channels/api4" + "github.com/mattermost/mattermost/server/v8/channels/testlib" "github.com/mattermost/mattermost/server/v8/enterprise/elasticsearch/common" "github.com/mattermost/mattermost/server/v8/platform/shared/filestore" "github.com/mattermost/mattermost/server/v8/platform/shared/filestore/mocks" @@ -127,6 +131,187 @@ func TestStartWithoutAnalysisICUReturnsExplicitError(t *testing.T) { } } +// missingCJKPluginsWarning mirrors the warning Start logs when no CJK analyzer plugin is detected. +const missingCJKPluginsWarning = "EnableCJKAnalyzers is set but no CJK analyzer plugins found installed. Please review opensearch settings." + +// pluginsHandler serves the endpoints Start and SearchPosts need, reporting one node per given list +// of plugin names and recording the posts template and search request bodies. +func pluginsHandler(t *testing.T, recorder *common.ClusterRecorder, nodePlugins ...[]string) http.HandlerFunc { + info := infoHandler("2.11.0") + nodesInfo := common.NodesPluginsResponse(nodePlugins...) + + readBody := func(w http.ResponseWriter, r *http.Request) ([]byte, bool) { + body, err := io.ReadAll(r.Body) + if !assert.NoError(t, err) { + http.Error(w, err.Error(), http.StatusInternalServerError) + return nil, false + } + return body, true + } + + return func(w http.ResponseWriter, r *http.Request) { + switch { + case r.URL.Path == "/": + info(w, r) + case r.URL.Path == "/_nodes/plugins": + _, _ = fmt.Fprint(w, nodesInfo) + case strings.HasPrefix(r.URL.Path, "/_index_template/"): + body, ok := readBody(w, r) + if !ok { + return + } + if strings.HasSuffix(r.URL.Path, common.IndexBasePosts) { + recorder.RecordPostsTemplate(body) + } + _, _ = fmt.Fprint(w, `{"acknowledged":true}`) + case strings.HasSuffix(r.URL.Path, "/_search"): + body, ok := readBody(w, r) + if !ok { + return + } + recorder.RecordSearch(body) + _, _ = fmt.Fprint(w, `{"took":1,"timed_out":false,"hits":{"total":{"value":0,"relation":"eq"},"max_score":0,"hits":[]}}`) + default: + w.WriteHeader(http.StatusNotFound) + } + } +} + +func setupCJKCluster(t *testing.T, recorder *common.ClusterRecorder, nodePlugins ...[]string) (*api4.TestHelper, *OpensearchInterfaceImpl) { + server := httptest.NewServer(pluginsHandler(t, recorder, nodePlugins...)) + t.Cleanup(server.Close) + + t.Setenv("MM_ELASTICSEARCHSETTINGS_CONNECTIONURL", server.URL) + t.Setenv("MM_ELASTICSEARCHSETTINGS_BACKEND", model.ElasticsearchSettingsOSBackend) + + th := api4.SetupEnterprise(t) + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ElasticsearchSettings.ConnectionURL = server.URL + *cfg.ElasticsearchSettings.Backend = model.ElasticsearchSettingsOSBackend + *cfg.ElasticsearchSettings.EnableIndexing = true + *cfg.ElasticsearchSettings.EnableSearching = true + *cfg.ElasticsearchSettings.EnableCJKAnalyzers = true + }) + th.App.Srv().SetLicense(model.NewTestLicense()) + + impl := &OpensearchInterfaceImpl{Platform: th.Server.Platform()} + t.Cleanup(func() { require.Nil(t, impl.Stop()) }) + + return th, impl +} + +// TestCJKAnalyzersWithPrefixedPluginNames covers a cluster that reports analysis plugins under the +// prefixed component name a managed service assigns to plugins it does not bundle. AWS OpenSearch +// Service reports the Korean analyzer installed through associate-package as +// "opensearch-analysis-nori" while its bundled analyzers keep their unprefixed names. +func TestCJKAnalyzersWithPrefixedPluginNames(t *testing.T) { + namings := []struct { + Name string + Plugins []string + }{ + {"only nori prefixed", []string{"analysis-icu", "opensearch-analysis-nori", "analysis-kuromoji", "analysis-smartcn"}}, + {"every analysis plugin prefixed", []string{"opensearch-analysis-icu", "opensearch-analysis-nori", "opensearch-analysis-kuromoji", "opensearch-analysis-smartcn"}}, + } + + for _, naming := range namings { + t.Run(naming.Name, func(t *testing.T) { + recorder := &common.ClusterRecorder{} + th, impl := setupCJKCluster(t, recorder, naming.Plugins) + require.Nil(t, impl.Start(context.Background())) + + expectedSubFields := map[string]string{ + "nori": "mm_nori", + "kuromoji": "mm_kuromoji", + "smartcn": "mm_smartcn", + } + + t.Run("the posts index template maps every CJK sub-field", func(t *testing.T) { + template := recorder.PostsTemplate() + require.NotEmpty(t, template, "no posts index template was created") + + require.Equal(t, expectedSubFields, common.TemplatePropertyFields(t, template, "message")) + require.Equal(t, expectedSubFields, common.TemplatePropertyFields(t, template, "attachments")) + require.Subset(t, common.TemplateAnalyzers(t, template), []string{"mm_nori", "mm_kuromoji", "mm_smartcn"}) + }) + + t.Run("no missing plugin warning is logged", func(t *testing.T) { + require.NoError(t, th.TestLogger.Flush()) + testlib.AssertNoLog(t, th.LogBuffer, mlog.LvlWarn.Name, missingCJKPluginsWarning) + }) + + channels := model.ChannelList{{Id: model.NewId(), TeamId: model.NewId(), Type: model.ChannelTypeOpen}} + + searchFields := func(t *testing.T, terms string) [][]string { + t.Helper() + + recorder.ResetSearches() + _, _, appErr := impl.SearchPosts(channels, model.ParseSearchParams(terms, 0), 0, 20) + require.Nil(t, appErr) + + bodies := recorder.SearchBodies() + require.Len(t, bodies, 1) + + return common.SimpleQueryStringFields(t, bodies[0]) + } + + t.Run("a CJK query searches the CJK sub-fields", func(t *testing.T) { + fields := searchFields(t, "검색") + require.Contains(t, fields, []string{"message", "message.nori", "message.kuromoji", "message.smartcn"}) + require.Contains(t, fields, []string{"attachments", "attachments.nori", "attachments.kuromoji", "attachments.smartcn"}) + }) + + t.Run("a non-CJK query only searches the base fields", func(t *testing.T) { + common.RequireOnlyBaseFields(t, searchFields(t, "search")) + }) + + t.Run("a CJK query only searches the base fields when CJK analyzers are disabled", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ElasticsearchSettings.EnableCJKAnalyzers = false + }) + defer th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ElasticsearchSettings.EnableCJKAnalyzers = true + }) + + common.RequireOnlyBaseFields(t, searchFields(t, "검색")) + }) + }) + } +} + +// TestCJKAnalyzersWithoutAnyCJKPlugin covers the diagnostic that made this failure mode hard to +// spot: the warning only fires when no CJK analyzer plugin is detected at all. +func TestCJKAnalyzersWithoutAnyCJKPlugin(t *testing.T) { + recorder := &common.ClusterRecorder{} + th, impl := setupCJKCluster(t, recorder, []string{"analysis-icu"}) + require.Nil(t, impl.Start(context.Background())) + + template := recorder.PostsTemplate() + require.NotEmpty(t, template, "no posts index template was created") + require.Empty(t, common.TemplatePropertyFields(t, template, "message")) + + channels := model.ChannelList{{Id: model.NewId(), TeamId: model.NewId(), Type: model.ChannelTypeOpen}} + _, _, appErr := impl.SearchPosts(channels, model.ParseSearchParams("검색", 0), 0, 20) + require.Nil(t, appErr) + + bodies := recorder.SearchBodies() + require.Len(t, bodies, 1) + common.RequireOnlyBaseFields(t, common.SimpleQueryStringFields(t, bodies[0])) + + require.NoError(t, th.TestLogger.Flush()) + testlib.AssertLog(t, th.LogBuffer, mlog.LvlWarn.Name, missingCJKPluginsWarning) +} + +// TestAnalysisICURequirementAcceptsPrefixedNames covers the per-node analysis-icu requirement when +// only some of the nodes report the plugin under a prefixed name. +func TestAnalysisICURequirementAcceptsPrefixedNames(t *testing.T) { + recorder := &common.ClusterRecorder{} + _, impl := setupCJKCluster(t, recorder, []string{"opensearch-analysis-icu"}, []string{"analysis-icu"}) + + require.Nil(t, impl.Start(context.Background())) + require.Equal(t, int32(1), impl.ready.Load()) + require.NotEmpty(t, recorder.PostsTemplate(), "no posts index template was created") +} + func (s *OpensearchInterfaceTestSuite) SetupSuite() { if os.Getenv("IS_CI") == "true" { os.Setenv("MM_ELASTICSEARCHSETTINGS_CONNECTIONURL", "http://opensearch:9201")