MM-14143 config cleanup final (#10374)

* TestGetLicenseFileFromDisk: avoid using fileutils.FindConfigFile

* config: abstract config-related file access, extend memory store

* simplify config validate to avoid file knowledge

* fix relative file tests

* cluster: fix ConfigChanged event

The old and new configurations were swapped when notifying the enterprise code of configuration changes, creating needless instability in propagating config updates across a cluster.

* config/database: ignore duplicates

* test cleanup

* remove unnecessary Save() in test

utils/fileutils/fileutils.go

@@ -85,15 +85,3 @@ func FindDir(dir string) (string, bool) {
 
 	return found, true
 }
-
-// FindConfigFile attempts to find an existing configuration file. fileName can be an absolute or
-// relative path or name such as "/opt/mattermost/config.json" or simply "config.json". An empty
-// string is returned if no configuration is found.
-func FindConfigFile(fileName string) (path string) {
-	found := FindFile(filepath.Join("config", fileName))
-	if found == "" {
-		found = FindPath(fileName, []string{"."}, nil)
-	}
-
-	return found
-}

utils/fileutils/fileutils_test.go

@@ -14,175 +14,6 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestFindConfigFile(t *testing.T) {
-	t.Run("config.json in current working directory, not inside config/", func(t *testing.T) {
-		// Force a unique working directory
-		cwd, err := ioutil.TempDir("", "")
-		require.NoError(t, err)
-		defer os.RemoveAll(cwd)
-
-		prevDir, err := os.Getwd()
-		require.NoError(t, err)
-		defer os.Chdir(prevDir)
-		os.Chdir(cwd)
-
-		configJson, err := filepath.Abs("config.json")
-		require.NoError(t, err)
-		require.NoError(t, ioutil.WriteFile(configJson, []byte("{}"), 0600))
-
-		// Relative paths end up getting symlinks fully resolved.
-		configJsonResolved, err := filepath.EvalSymlinks(configJson)
-		require.NoError(t, err)
-
-		assert.Equal(t, configJsonResolved, FindConfigFile("config.json"))
-	})
-
-	t.Run("config/config.json from various paths", func(t *testing.T) {
-		// Create the following directory structure:
-		// tmpDir1/
-		//   config/
-		//     config.json
-		//   tmpDir2/
-		//     tmpDir3/
-		//       tmpDir4/
-		//         tmpDir5/
-		tmpDir1, err := ioutil.TempDir("", "")
-		require.NoError(t, err)
-		defer os.RemoveAll(tmpDir1)
-
-		err = os.Mkdir(filepath.Join(tmpDir1, "config"), 0700)
-		require.NoError(t, err)
-
-		tmpDir2, err := ioutil.TempDir(tmpDir1, "")
-		require.NoError(t, err)
-
-		tmpDir3, err := ioutil.TempDir(tmpDir2, "")
-		require.NoError(t, err)
-
-		tmpDir4, err := ioutil.TempDir(tmpDir3, "")
-		require.NoError(t, err)
-
-		tmpDir5, err := ioutil.TempDir(tmpDir4, "")
-		require.NoError(t, err)
-
-		configJson := filepath.Join(tmpDir1, "config", "config.json")
-		require.NoError(t, ioutil.WriteFile(configJson, []byte("{}"), 0600))
-
-		// Relative paths end up getting symlinks fully resolved, so use this below as necessary.
-		configJsonResolved, err := filepath.EvalSymlinks(configJson)
-		require.NoError(t, err)
-
-		testCases := []struct {
-			Description string
-			Cwd         *string
-			FileName    string
-			Expected    string
-		}{
-			{
-				"absolute path to config.json",
-				nil,
-				configJson,
-				configJson,
-			},
-			{
-				"absolute path to config.json from directory containing config.json",
-				&tmpDir1,
-				configJson,
-				configJson,
-			},
-			{
-				"relative path to config.json from directory containing config.json",
-				&tmpDir1,
-				"config.json",
-				configJsonResolved,
-			},
-			{
-				"subdirectory of directory containing config.json",
-				&tmpDir2,
-				"config.json",
-				configJsonResolved,
-			},
-			{
-				"twice-nested subdirectory of directory containing config.json",
-				&tmpDir3,
-				"config.json",
-				configJsonResolved,
-			},
-			{
-				"thrice-nested subdirectory of directory containing config.json",
-				&tmpDir4,
-				"config.json",
-				configJsonResolved,
-			},
-			{
-				"can't find from four nesting levels deep",
-				&tmpDir5,
-				"config.json",
-				"",
-			},
-		}
-
-		for _, testCase := range testCases {
-			t.Run(testCase.Description, func(t *testing.T) {
-				if testCase.Cwd != nil {
-					prevDir, err := os.Getwd()
-					require.NoError(t, err)
-					defer os.Chdir(prevDir)
-					os.Chdir(*testCase.Cwd)
-				}
-
-				assert.Equal(t, testCase.Expected, FindConfigFile(testCase.FileName))
-			})
-		}
-	})
-
-	t.Run("config/config.json relative to executable", func(t *testing.T) {
-		osExecutable, err := os.Executable()
-		require.NoError(t, err)
-		osExecutableDir := filepath.Dir(osExecutable)
-
-		// Force a working directory different than the executable.
-		cwd, err := ioutil.TempDir("", "")
-		require.NoError(t, err)
-		defer os.RemoveAll(cwd)
-
-		prevDir, err := os.Getwd()
-		require.NoError(t, err)
-		defer os.Chdir(prevDir)
-		os.Chdir(cwd)
-
-		testCases := []struct {
-			Description  string
-			RelativePath string
-		}{
-			{
-				"config/config.json",
-				".",
-			},
-			{
-				"../config/config.json",
-				"../",
-			},
-		}
-
-		for _, testCase := range testCases {
-			t.Run(testCase.Description, func(t *testing.T) {
-				// Install the config in config/config.json relative to the executable
-				configJson := filepath.Join(osExecutableDir, testCase.RelativePath, "config", "config.json")
-				require.NoError(t, os.Mkdir(filepath.Dir(configJson), 0700))
-				require.NoError(t, ioutil.WriteFile(configJson, []byte("{}"), 0600))
-				defer os.RemoveAll(filepath.Dir(configJson))
-
-				// Relative paths end up getting symlinks fully resolved.
-				configJsonResolved, err := filepath.EvalSymlinks(configJson)
-				require.NoError(t, err)
-
-				assert.Equal(t, configJsonResolved, FindConfigFile("config.json"))
-			})
-		}
-	})
-}
-
 func TestFindFile(t *testing.T) {
 	t.Run("files from various paths", func(t *testing.T) {
 		// Create the following directory structure:

utils/license_test.go

@@ -4,9 +4,12 @@
 package utils
 
 import (
+	"io/ioutil"
+	"os"
 	"testing"
 
-	"github.com/mattermost/mattermost-server/utils/fileutils"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestValidateLicense(t *testing.T) {
@@ -34,17 +37,21 @@ func TestGetLicenseFileLocation(t *testing.T) {
 }
 
 func TestGetLicenseFileFromDisk(t *testing.T) {
-	fileBytes := GetLicenseFileFromDisk("thisfileshouldnotexist.mattermost-license")
-	if len(fileBytes) > 0 {
-		t.Fatal("invalid bytes")
-	}
+	t.Run("missing file", func(t *testing.T) {
+		fileBytes := GetLicenseFileFromDisk("thisfileshouldnotexist.mattermost-license")
+		assert.Empty(t, fileBytes, "invalid bytes")
+	})
 
-	fileBytes = GetLicenseFileFromDisk(fileutils.FindConfigFile("config.json"))
-	if len(fileBytes) == 0 { // a valid bytes but should be a fail license
-		t.Fatal("invalid bytes")
-	}
+	t.Run("not a license file", func(t *testing.T) {
+		f, err := ioutil.TempFile("", "TestGetLicenseFileFromDisk")
+		require.NoError(t, err)
+		defer os.Remove(f.Name())
+		ioutil.WriteFile(f.Name(), []byte("not a license"), 0777)
 
-	if success, _ := ValidateLicense(fileBytes); success {
-		t.Fatal("should have been an invalid file")
-	}
+		fileBytes := GetLicenseFileFromDisk(f.Name())
+		require.NotEmpty(t, fileBytes, "should have read the file")
+
+		success, _ := ValidateLicense(fileBytes)
+		assert.False(t, success, "should have been an invalid file")
+	})
 }

utils/utils.go

@@ -7,11 +7,9 @@ import (
 	"net"
 	"net/http"
 	"net/url"
-	"os"
 	"strings"
 
 	"github.com/mattermost/mattermost-server/model"
-	"github.com/mattermost/mattermost-server/utils/fileutils"
 )
 
 func StringInSlice(a string, slice []string) bool {
@@ -40,17 +38,6 @@ func StringArrayIntersection(arr1, arr2 []string) []string {
 	return result
 }
 
-func FileExistsInConfigFolder(filename string) bool {
-	if len(filename) == 0 {
-		return false
-	}
-
-	if _, err := os.Stat(fileutils.FindConfigFile(filename)); err == nil {
-		return true
-	}
-	return false
-}
-
 func RemoveDuplicatesFromStringArray(arr []string) []string {
 	result := make([]string, 0, len(arr))
 	seen := make(map[string]bool)