MM-43077 Allow inline JSON in config.json for advanced logging config (#23324)

* add AdvancedLoggingJSON to LogSettings and deprecate AdvancedLoggingConfig
* allow embedded JSON in config for advanced logging.

server/config/logconfigsrc.go

@@ -7,13 +7,20 @@ import (
 	"encoding/json"
 	"errors"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"sync"
 
 	"github.com/mattermost/mattermost-server/server/public/shared/mlog"
 )
 
+const (
+	LogConfigSrcTypeJSON LogConfigSrcType = "json"
+	LogConfigSrcTypeFile LogConfigSrcType = "file"
+)
+
 type LogSrcListener func(old, new mlog.LoggerConfiguration)
+type LogConfigSrcType string
 
 // LogConfigSrc abstracts the Advanced Logging configuration so that implementations can
 // fetch from file, database, etc.
@@ -22,7 +29,10 @@ type LogConfigSrc interface {
 	Get() mlog.LoggerConfiguration
 
 	// Set updates the dsn specifying the source and reloads
-	Set(dsn string, configStore *Store) (err error)
+	Set(dsn []byte, configStore *Store) (err error)
+
+	// GetType returns the type of config source (JSON, file, ...)
+	GetType() LogConfigSrcType
 
 	// Close cleans up resources.
 	Close() error
@@ -30,8 +40,8 @@ type LogConfigSrc interface {
 
 // NewLogConfigSrc creates an advanced logging configuration source, backed by a
 // file, JSON string, or database.
-func NewLogConfigSrc(dsn string, configStore *Store) (LogConfigSrc, error) {
-	if dsn == "" {
+func NewLogConfigSrc(dsn json.RawMessage, configStore *Store) (LogConfigSrc, error) {
+	if len(dsn) == 0 {
 		return nil, errors.New("dsn should not be empty")
 	}
 
@@ -39,17 +49,28 @@ func NewLogConfigSrc(dsn string, configStore *Store) (LogConfigSrc, error) {
 		return nil, errors.New("configStore should not be nil")
 	}
 
-	dsn = strings.TrimSpace(dsn)
-
+	//  check if embedded JSON
 	if isJSONMap(dsn) {
 		return newJSONSrc(dsn)
 	}
 
-	path := dsn
+	// Now we're treating the DSN as a string which may contain escaped JSON or be a filespec.
+	str := strings.TrimSpace(string(dsn))
+	if s, err := strconv.Unquote(str); err == nil {
+		str = s
+	}
+
+	// check if escaped JSON
+	strBytes := []byte(str)
+	if isJSONMap(strBytes) {
+		return newJSONSrc(strBytes)
+	}
+
 	// If this is a file based config we need the full path so it can be watched.
-	if strings.HasPrefix(configStore.String(), "file://") && !filepath.IsAbs(dsn) {
+	path := str
+	if strings.HasPrefix(configStore.String(), "file://") && !filepath.IsAbs(path) {
 		configPath := strings.TrimPrefix(configStore.String(), "file://")
-		path = filepath.Join(filepath.Dir(configPath), dsn)
+		path = filepath.Join(filepath.Dir(configPath), path)
 	}
 
 	return newFileSrc(path, configStore)
@@ -63,7 +84,7 @@ type jsonSrc struct {
 	cfg   mlog.LoggerConfiguration
 }
 
-func newJSONSrc(data string) (*jsonSrc, error) {
+func newJSONSrc(data json.RawMessage) (*jsonSrc, error) {
 	src := &jsonSrc{}
 	return src, src.Set(data, nil)
 }
@@ -76,8 +97,8 @@ func (src *jsonSrc) Get() mlog.LoggerConfiguration {
 }
 
 // Set updates the JSON specifying the source and reloads
-func (src *jsonSrc) Set(data string, _ *Store) error {
-	cfg, err := logTargetCfgFromJSON([]byte(data))
+func (src *jsonSrc) Set(data []byte, _ *Store) error {
+	cfg, err := logTargetCfgFromJSON(data)
 	if err != nil {
 		return err
 	}
@@ -86,6 +107,11 @@ func (src *jsonSrc) Set(data string, _ *Store) error {
 	return nil
 }
 
+// GetType returns the config source type.
+func (src *jsonSrc) GetType() LogConfigSrcType {
+	return LogConfigSrcTypeJSON
+}
+
 func (src *jsonSrc) set(cfg mlog.LoggerConfiguration) {
 	src.mutex.Lock()
 	defer src.mutex.Unlock()
@@ -112,7 +138,7 @@ func newFileSrc(path string, configStore *Store) (*fileSrc, error) {
 	src := &fileSrc{
 		path: path,
 	}
-	if err := src.Set(path, configStore); err != nil {
+	if err := src.Set([]byte(path), configStore); err != nil {
 		return nil, err
 	}
 	return src, nil
@@ -128,8 +154,8 @@ func (src *fileSrc) Get() mlog.LoggerConfiguration {
 // Set updates the dsn specifying the file source and reloads.
 // The file will be watched for changes and reloaded as needed,
 // and all listeners notified.
-func (src *fileSrc) Set(path string, configStore *Store) error {
-	data, err := configStore.GetFile(path)
+func (src *fileSrc) Set(path []byte, configStore *Store) error {
+	data, err := configStore.GetFile(string(path))
 	if err != nil {
 		return err
 	}
@@ -143,6 +169,11 @@ func (src *fileSrc) Set(path string, configStore *Store) error {
 	return nil
 }
 
+// GetType returns the config source type.
+func (src *fileSrc) GetType() LogConfigSrcType {
+	return LogConfigSrcTypeFile
+}
+
 func (src *fileSrc) set(cfg mlog.LoggerConfiguration) {
 	src.mutex.Lock()
 	defer src.mutex.Unlock()

server/config/logconfigsrc_test.go

@@ -4,36 +4,41 @@
 package config
 
 import (
+	"encoding/json"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
-const (
-	validJSON = `{"file":{ "Type":"file"}}`
-	badJSON   = `{"file":{ Type="file"}}`
+var (
+	validJSON        = []byte(`{"file":{ "Type":"file"}}`)
+	badJSON          = []byte(`{"file":{ Type="file"}}`)
+	validEscapedJSON = []byte(`"{\"file\":{ \"Type\":\"file\"}}"`)
+	badEscapedJSON   = []byte(`"{\"file\":{ Type:\"file\"}}"`)
 )
 
 func TestNewLogConfigSrc(t *testing.T) {
 	store := NewTestMemoryStore()
 	require.NotNil(t, store)
-	err := store.SetFile("advancedlogging.conf", []byte(validJSON))
+	err := store.SetFile("advancedlogging.conf", validJSON)
 	require.NoError(t, err)
 
 	tests := []struct {
 		name        string
-		dsn         string
+		dsn         json.RawMessage
 		configStore *Store
 		wantErr     bool
 		wantType    LogConfigSrc
 	}{
-		{name: "empty dsn", dsn: "", configStore: store, wantErr: true, wantType: nil},
-		{name: "garbage dsn", dsn: "!@wfejwcevioj", configStore: store, wantErr: true, wantType: nil},
+		{name: "empty dsn", dsn: []byte(""), configStore: store, wantErr: true, wantType: nil},
+		{name: "garbage dsn", dsn: []byte("!@wfejwcevioj"), configStore: store, wantErr: true, wantType: nil},
 		{name: "valid json dsn", dsn: validJSON, configStore: store, wantErr: false, wantType: &jsonSrc{}},
 		{name: "invalid json dsn", dsn: badJSON, configStore: store, wantErr: true, wantType: nil},
-		{name: "valid filespec dsn", dsn: "advancedlogging.conf", configStore: store, wantErr: false, wantType: &fileSrc{}},
-		{name: "invalid filespec dsn", dsn: "/nobody/here.conf", configStore: store, wantErr: true, wantType: nil},
+		{name: "valid escaped json dsn", dsn: validEscapedJSON, configStore: store, wantErr: false, wantType: &jsonSrc{}},
+		{name: "invalid escaped json dsn", dsn: badEscapedJSON, configStore: store, wantErr: true, wantType: nil},
+		{name: "valid filespec dsn", dsn: []byte("advancedlogging.conf"), configStore: store, wantErr: false, wantType: &fileSrc{}},
+		{name: "invalid filespec dsn", dsn: []byte("/nobody/here.conf"), configStore: store, wantErr: true, wantType: nil},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

server/config/migrate.go

@@ -32,9 +32,11 @@ func Migrate(from, to string) error {
 		*sourceConfig.SamlSettings.PrivateKeyFile,
 	}
 
-	// Only migrate advanced logging config if it is not embedded JSON.
-	if !isJSONMap(*sourceConfig.LogSettings.AdvancedLoggingConfig) {
-		files = append(files, *sourceConfig.LogSettings.AdvancedLoggingConfig)
+	// Only migrate advanced logging config if it is a filespec.
+	dsn := sourceConfig.LogSettings.GetAdvancedLoggingConfig()
+	cfgSource, err := NewLogConfigSrc(dsn, source)
+	if err == nil && cfgSource.GetType() == LogConfigSrcTypeFile {
+		files = append(files, string(dsn))
 	}
 
 	files = append(files, sourceConfig.PluginSettings.SignaturePublicKeyFiles...)

server/config/utils.go

@@ -179,9 +179,10 @@ func IsDatabaseDSN(dsn string) bool {
 		strings.HasPrefix(dsn, "postgresql://")
 }
 
-func isJSONMap(data string) bool {
+func isJSONMap(data []byte) bool {
 	var m map[string]any
-	return json.Unmarshal([]byte(data), &m) == nil
+	err := json.Unmarshal(data, &m)
+	return err == nil
 }
 
 func GetValueByPath(path []string, obj any) (any, bool) {

server/config/utils_test.go

@@ -221,7 +221,7 @@ func TestIsJSONMap(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := isJSONMap(tt.data); got != tt.want {
+			if got := isJSONMap([]byte(tt.data)); got != tt.want {
 				t.Errorf("isJSONMap() = %v, want %v", got, tt.want)
 			}
 		})