diff --git a/src/cli/config/account.go b/src/cli/config/account.go index 22a481b57..b511088fc 100644 --- a/src/cli/config/account.go +++ b/src/cli/config/account.go @@ -30,6 +30,9 @@ func m365Overrides(in map[string]string) map[string]string { } } +// add m365 config key names that require path related validations +var m365PathKeys = []string{} + // configureAccount builds a complete account configuration from a mix of // viper properties and manual overrides. func configureAccount( @@ -57,7 +60,7 @@ func configureAccount( return acct, clues.New("unsupported account provider: [" + providerType + "]") } - if err := mustMatchConfig(vpr, m365Overrides(overrides)); err != nil { + if err := mustMatchConfig(vpr, m365Overrides(overrides), m365PathKeys); err != nil { return acct, clues.Wrap(err, "verifying m365 configs in corso config file") } } diff --git a/src/cli/config/config.go b/src/cli/config/config.go index 6eab83fea..43af90606 100644 --- a/src/cli/config/config.go +++ b/src/cli/config/config.go @@ -4,6 +4,7 @@ import ( "context" "os" "path/filepath" + "slices" "strings" "github.com/alcionai/clues" @@ -16,6 +17,7 @@ import ( "github.com/alcionai/corso/src/pkg/account" "github.com/alcionai/corso/src/pkg/control/repository" "github.com/alcionai/corso/src/pkg/logger" + "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/storage" ) @@ -333,7 +335,7 @@ var constToTomlKeyMap = map[string]string{ // If any value differs from the viper value, an error is returned. // values in m that aren't stored in the config are ignored. // TODO(pandeyabs): This code is currently duplicated in 2 places. -func mustMatchConfig(vpr *viper.Viper, m map[string]string) error { +func mustMatchConfig(vpr *viper.Viper, m map[string]string, pathKeys []string) error { for k, v := range m { if len(v) == 0 { continue // empty variables will get caught by configuration validators, if necessary @@ -345,7 +347,16 @@ func mustMatchConfig(vpr *viper.Viper, m map[string]string) error { } vv := vpr.GetString(tomlK) - if v != vv { + areEqual := false + + // some of the values maybe paths, hence they require more than just string equality + if len(pathKeys) > 0 && slices.Contains(pathKeys, k) { + areEqual = path.ArePathsEquivalent(v, vv) + } else { + areEqual = v == vv + } + + if !areEqual { return clues.New("value of " + k + " (" + v + ") does not match corso configuration value (" + vv + ")") } } diff --git a/src/cli/config/config_test.go b/src/cli/config/config_test.go index e4f61803e..2fe27208b 100644 --- a/src/cli/config/config_test.go +++ b/src/cli/config/config_test.go @@ -216,6 +216,8 @@ func (suite *ConfigSuite) TestMustMatchConfig() { s3Cfg := &storage.S3Config{Bucket: bkt} m365 := account.M365Config{AzureTenantID: tid} + m365PathKeys := []string{} + err = writeRepoConfigWithViper(vpr, s3Cfg, m365, repository.Options{}, "repoid") require.NoError(t, err, "writing repo config", clues.ToCore(err)) @@ -272,7 +274,7 @@ func (suite *ConfigSuite) TestMustMatchConfig() { } for _, test := range table { suite.Run(test.name, func() { - test.errCheck(suite.T(), mustMatchConfig(vpr, test.input), clues.ToCore(err)) + test.errCheck(suite.T(), mustMatchConfig(vpr, test.input, m365PathKeys), clues.ToCore(err)) }) } } diff --git a/src/pkg/path/path.go b/src/pkg/path/path.go index 6e9c7cd11..e49c394ed 100644 --- a/src/pkg/path/path.go +++ b/src/pkg/path/path.go @@ -52,6 +52,7 @@ package path import ( "fmt" + "path/filepath" "strings" "github.com/alcionai/clues" @@ -317,6 +318,16 @@ func Split(segment string) []string { return res } +func ArePathsEquivalent(path1, path2 string) bool { + normalizedPath1 := strings.TrimSpace(filepath.Clean(path1)) + normalizedPath2 := strings.TrimSpace(filepath.Clean(path2)) + + normalizedPath1 = strings.TrimSuffix(normalizedPath1, string(filepath.Separator)) + normalizedPath2 = strings.TrimSuffix(normalizedPath2, string(filepath.Separator)) + + return normalizedPath1 == normalizedPath2 +} + // --------------------------------------------------------------------------- // Unexported Helpers // --------------------------------------------------------------------------- diff --git a/src/pkg/path/path_test.go b/src/pkg/path/path_test.go index d71a30f07..404523de3 100644 --- a/src/pkg/path/path_test.go +++ b/src/pkg/path/path_test.go @@ -574,3 +574,74 @@ func (suite *PathUnitSuite) TestBuildRestorePaths() { }) } } + +func (suite *PathUnitSuite) TestArePathsEquivalent() { + tests := []struct { + name string + path1 string + path2 string + equivalent bool + }{ + // Test cases with equivalent paths + { + name: "same linux normal path", + path1: "/path/to/dir", + path2: "/path/to/dir", + equivalent: true, + }, + { + name: "same windows normal path", + path1: "C:\\Users\\User\\Documents", + path2: "C:\\Users\\User\\Documents", + equivalent: true, + }, + { + name: "same linux extra trailing slash path", + path1: "/path/with/trailing/slash/", + path2: "/path/with/trailing/slash", + equivalent: true, + }, + { + name: "same linux relative path", + path1: "relative/path", + path2: "./relative/path", + equivalent: true, + }, + + // Test cases with non-equivalent paths + { + name: "different linux normal path", + path1: "/path/to/dir", + path2: "/path/to/different/dir", + equivalent: false, + }, + { + name: "different windows normal path", + path1: "C:\\Users\\User\\Documents", + path2: "D:\\Users\\User\\Documents", + equivalent: false, + }, + { + name: "different linux extra trailing slash path", + path1: "/path/with/trailing/slash/", + path2: "/different/path/with/trailing/slash", + equivalent: false, + }, + { + name: "different linux relative path", + path1: "relative/path", + path2: "../relative/path", + equivalent: false, + }, + } + + for _, test := range tests { + t := suite.T() + suite.Run(test.name, func() { + result := ArePathsEquivalent(test.path1, test.path2) + if result != test.equivalent { + t.Errorf("Paths %s and %s are not equivalent as expected.", test.path1, test.path2) + } + }) + } +} diff --git a/src/pkg/storage/filesystem.go b/src/pkg/storage/filesystem.go index 08dacc62c..2826ad211 100644 --- a/src/pkg/storage/filesystem.go +++ b/src/pkg/storage/filesystem.go @@ -1,10 +1,13 @@ package storage import ( + "strings" + "github.com/alcionai/clues" "github.com/spf13/cast" "github.com/alcionai/corso/src/internal/common/str" + "github.com/alcionai/corso/src/pkg/path" ) const ( @@ -16,6 +19,9 @@ var fsConstToTomlKeyMap = map[string]string{ FilesystemPath: FilesystemPath, } +// add filesystem config key names that require path related validations +var fsPathKeys = []string{FilesystemPath} + type FilesystemConfig struct { Path string } @@ -77,13 +83,17 @@ func (c *FilesystemConfig) ApplyConfigOverrides( } // This is matching override values from config file. - if err := mustMatchConfig(g, fsConstToTomlKeyMap, fsOverrides(overrides)); err != nil { + if err := mustMatchConfig(g, fsConstToTomlKeyMap, fsOverrides(overrides), fsPathKeys); err != nil { return clues.Wrap(err, "verifying storage configs in corso config file") } } } - c.Path = str.First(overrides[FilesystemPath], c.Path) + sanitizePath := func(p string) string { + return path.TrimTrailingSlash(strings.TrimSpace(p)) + } + + c.Path = str.First(sanitizePath(overrides[FilesystemPath]), sanitizePath(c.Path)) return c.validate() } diff --git a/src/pkg/storage/s3.go b/src/pkg/storage/s3.go index 7f2e8688f..825228c23 100644 --- a/src/pkg/storage/s3.go +++ b/src/pkg/storage/s3.go @@ -62,6 +62,9 @@ var s3constToTomlKeyMap = map[string]string{ StorageProviderTypeKey: StorageProviderTypeKey, } +// add s3 config key names that require path related validations +var s3PathKeys = []string{} + func (s Storage) ToS3Config() (*S3Config, error) { return buildS3ConfigFromMap(s.Config) } @@ -176,7 +179,7 @@ func (c *S3Config) ApplyConfigOverrides( return clues.New("unsupported storage provider: [" + providerType + "]") } - if err := mustMatchConfig(kvg, s3constToTomlKeyMap, s3Overrides(overrides)); err != nil { + if err := mustMatchConfig(kvg, s3constToTomlKeyMap, s3Overrides(overrides), s3PathKeys); err != nil { return clues.Stack(err) } } diff --git a/src/pkg/storage/storage.go b/src/pkg/storage/storage.go index c695ea992..cb7ee4791 100644 --- a/src/pkg/storage/storage.go +++ b/src/pkg/storage/storage.go @@ -2,11 +2,13 @@ package storage import ( "fmt" + "slices" "github.com/alcionai/clues" "github.com/spf13/cast" "github.com/alcionai/corso/src/internal/common" + "github.com/alcionai/corso/src/pkg/path" ) var ErrVerifyingConfigStorage = clues.New("verifying configs in corso config file") @@ -156,6 +158,7 @@ func mustMatchConfig( g Getter, tomlMap map[string]string, m map[string]string, + pathKeys []string, ) error { for k, v := range m { if len(v) == 0 { @@ -168,7 +171,17 @@ func mustMatchConfig( } vv := cast.ToString(g.Get(tomlK)) - if v != vv { + + areEqual := false + + // some of the values maybe paths, hence they require more than just string equality check + if len(pathKeys) > 0 && slices.Contains(pathKeys, k) { + areEqual = path.ArePathsEquivalent(v, vv) + } else { + areEqual = v == vv + } + + if !areEqual { err := clues.New("value of " + k + " (" + v + ") does not match corso configuration value (" + vv + ")") return clues.Stack(ErrVerifyingConfigStorage, err) } diff --git a/src/pkg/storage/storage_test.go b/src/pkg/storage/storage_test.go index 095ea363c..ccba54bc6 100644 --- a/src/pkg/storage/storage_test.go +++ b/src/pkg/storage/storage_test.go @@ -62,3 +62,131 @@ func (suite *StorageUnitSuite) TestNewStorage() { }) } } + +type testGetter struct { + storeMap map[string]string +} + +func (tg testGetter) Get(key string) any { + val, ok := tg.storeMap[key] + if ok { + return val + } + + return "" +} + +func (suite *StorageUnitSuite) TestMustMatchConfig() { + t := suite.T() + + table := []struct { + name string + tomlMap map[string]string + overrideMap map[string]string + getterMap map[string]string + pathKeys []string + errorCheck assert.ErrorAssertionFunc + }{ + { + name: "s3 config match", + tomlMap: s3constToTomlKeyMap, + overrideMap: map[string]string{ + Bucket: "test-bucket", + Endpoint: "https://aws.s3", + Prefix: "test-prefix", + "additional-key": "additional-value", + }, + getterMap: map[string]string{ + "bucket": "test-bucket", + "endpoint": "https://aws.s3", + "prefix": "test-prefix", + }, + errorCheck: assert.NoError, + }, + { + name: "s3 config match - bucket mismatch", + tomlMap: s3constToTomlKeyMap, + overrideMap: map[string]string{ + Bucket: "test-bucket", + Endpoint: "https://aws.s3", + Prefix: "test-prefix", + "additional-key": "additional-value", + }, + getterMap: map[string]string{ + "bucket": "test-bucket-new", + "endpoint": "https://aws.s3", + "prefix": "test-prefix", + }, + errorCheck: assert.Error, + }, + { + name: "s3 config match - endpoint mismatch", + tomlMap: s3constToTomlKeyMap, + overrideMap: map[string]string{ + Bucket: "test-bucket", + Endpoint: "https://aws.s3", + Prefix: "test-prefix", + "additional-key": "additional-value", + }, + getterMap: map[string]string{ + "bucket": "test-bucket", + "endpoint": "https://aws.s3/new", + "prefix": "test-prefix", + }, + errorCheck: assert.Error, + }, + { + name: "s3 config match - prefix mismatch", + tomlMap: s3constToTomlKeyMap, + overrideMap: map[string]string{ + Bucket: "test-bucket", + Endpoint: "https://aws.s3", + Prefix: "test-prefix", + "additional-key": "additional-value", + }, + getterMap: map[string]string{ + "bucket": "test-bucket", + "endpoint": "https://aws.s3", + "prefix": "test-prefix-new", + }, + errorCheck: assert.Error, + }, + { + name: "filesystem config match - success case", + tomlMap: fsConstToTomlKeyMap, + overrideMap: map[string]string{ + StorageProviderTypeKey: "filesystem", + FilesystemPath: "/path/to/dir", + "additional-key": "additional-value", + }, + getterMap: map[string]string{ + "provider": "filesystem", + "path": "/path/to/dir", + }, + pathKeys: []string{FilesystemPath}, + errorCheck: assert.NoError, + }, + { + name: "filesystem config match - path mismatch", + tomlMap: fsConstToTomlKeyMap, + overrideMap: map[string]string{ + StorageProviderTypeKey: "filesystem", + FilesystemPath: "/path/to/dir", + "additional-key": "additional-value", + }, + getterMap: map[string]string{ + "provider": "filesystem", + "path": "/path/to/dir/new", + }, + pathKeys: []string{FilesystemPath}, + errorCheck: assert.Error, + }, + } + for _, test := range table { + suite.Run(test.name, func() { + tg := testGetter{test.getterMap} + err := mustMatchConfig(tg, test.tomlMap, test.overrideMap, test.pathKeys) + test.errorCheck(t, err) + }) + } +}