From ff7da90ccded952599ec916455a4612feb30a07a Mon Sep 17 00:00:00 2001 From: Dmitriy Gertsog Date: Fri, 22 Nov 2024 18:27:23 +0300 Subject: [PATCH] modules: tt.yaml modules/directory support list Closes #1012 @TarantoolBot document Title: Allow multiple directories with modules The logic of working with configuration file tt.yaml has been updated, now modules/directory can be specified both as a single line and as a list. **Usage example** Old behavior: ```yaml modules: directory: modules ``` Possibility to specify a list of directories: ```yaml modules: directory: - modules - /ext/path/modules - other_modules ``` If a relative path is specified, the search is performed in the subfolders below, relative to the tt.yaml file. If an absolute path is specified, external modules are searched according to the specified path. --- CHANGELOG.md | 1 + cli/cfg/dump_test.go | 9 ++- cli/config/config.go | 8 ++- cli/configure/configure.go | 49 +++++++++++++-- cli/configure/configure_test.go | 63 ++++++++++++++++++- .../testdata/modules_cfg/tt-modules1.yaml | 3 + .../testdata/modules_cfg/tt-modules2.yml | 4 ++ .../testdata/modules_cfg/tt-modules3.yaml | 6 ++ .../testdata/modules_cfg/tt-modules4.yml | 3 + .../testdata/modules_cfg/tt-modules5.yaml | 3 + cli/init/init.go | 3 +- cli/init/init_test.go | 2 +- cli/init/templates/tt.yaml.default | 2 +- cli/modules/modules.go | 3 +- cli/pack/common.go | 35 ++++++----- cli/pack/common_test.go | 8 +-- test/integration/cfg/test_dump.py | 6 +- test/integration/init/test_init.py | 2 +- 18 files changed, 171 insertions(+), 39 deletions(-) create mode 100644 cli/configure/testdata/modules_cfg/tt-modules1.yaml create mode 100644 cli/configure/testdata/modules_cfg/tt-modules2.yml create mode 100644 cli/configure/testdata/modules_cfg/tt-modules3.yaml create mode 100644 cli/configure/testdata/modules_cfg/tt-modules4.yml create mode 100644 cli/configure/testdata/modules_cfg/tt-modules5.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index cb2b5f0c6..20673e471 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. time format. - `tt connect`: add new `--evaler` option to support for customizing the way user input is processed. +- `tt.yaml`: allows to specify a list of modules directories. ### Changed diff --git a/cli/cfg/dump_test.go b/cli/cfg/dump_test.go index c4507ac3e..ff4c59d92 100644 --- a/cli/cfg/dump_test.go +++ b/cli/cfg/dump_test.go @@ -93,7 +93,8 @@ env: restart_on_failure: false tarantoolctl_layout: false modules: - directory: /root/modules + directory: + - /root/modules app: run_dir: var/run log_dir: var/log @@ -129,7 +130,8 @@ env: restart_on_failure: false tarantoolctl_layout: false modules: - directory: %[1]s/my_modules + directory: + - %[1]s/my_modules app: run_dir: var/run log_dir: var/log @@ -166,7 +168,8 @@ env: restart_on_failure: false tarantoolctl_layout: false modules: - directory: /root/modules + directory: + - /root/modules app: run_dir: var/run log_dir: var/log diff --git a/cli/config/config.go b/cli/config/config.go index 25aa009c6..c4dd77e40 100644 --- a/cli/config/config.go +++ b/cli/config/config.go @@ -21,11 +21,15 @@ package config // ee: // credential_path: path +// FieldStringArrayType a custom type used with mapstructure's hook to accept values +// as a single string as well as a list of strings. +type FieldStringArrayType []string + // ModuleOpts is used to store all module options. type ModulesOpts struct { - // Directory is a path to directory where the external modules + // Directories is a list of paths to directories where the external modules // are stored. - Directory string + Directories FieldStringArrayType `mapstructure:"directory" yaml:"directory"` } // EEOpts is used to store tarantool-ee options. diff --git a/cli/configure/configure.go b/cli/configure/configure.go index 877e79131..57d4c6236 100644 --- a/cli/configure/configure.go +++ b/cli/configure/configure.go @@ -5,6 +5,7 @@ import ( "os" "os/exec" "path/filepath" + "reflect" "strings" "syscall" @@ -103,7 +104,7 @@ func getSystemAppOpts() *config.AppOpts { // GetDefaultCliOpts returns `CliOpts` filled with default values. func GetSystemCliOpts() *config.CliOpts { modules := config.ModulesOpts{ - Directory: ModulesPath, + Directories: []string{ModulesPath}, } ee := config.EEOpts{ CredPath: "", @@ -128,7 +129,7 @@ func GetSystemCliOpts() *config.CliOpts { // GetDefaultCliOpts returns `CliOpts` filled with default values. func GetDefaultCliOpts() *config.CliOpts { modules := config.ModulesOpts{ - Directory: ModulesPath, + Directories: []string{ModulesPath}, } ee := config.EEOpts{ CredPath: "", @@ -179,6 +180,24 @@ func adjustPathWithConfigLocation(filePath string, configDir string, return filepath.Abs(filepath.Join(configDir, filePath)) } +func adjustListPathWithConfigLocation(listPaths []string, configDir string, + defaultDirName string) ([]string, error) { + if len(listPaths) == 0 { + listPaths = append(listPaths, defaultDirName) + } + + result := make([]string, 0, len(listPaths)) + for _, path := range listPaths { + path, err := adjustPathWithConfigLocation(path, configDir, defaultDirName) + if err != nil { + return result, err + } + result = append(result, path) + + } + return result, nil +} + // resolveConfigPaths resolves all paths in config relative to specified location, and // sets uninitialized values to defaults. func updateCliOpts(cliOpts *config.CliOpts, configDir string) error { @@ -211,8 +230,8 @@ func updateCliOpts(cliOpts *config.CliOpts, configDir string) error { } if cliOpts.Modules != nil { - if cliOpts.Modules.Directory, err = adjustPathWithConfigLocation(cliOpts.Modules.Directory, - configDir, ModulesPath); err != nil { + if cliOpts.Modules.Directories, err = adjustListPathWithConfigLocation( + cliOpts.Modules.Directories, configDir, ModulesPath); err != nil { return err } } @@ -227,6 +246,26 @@ func updateCliOpts(cliOpts *config.CliOpts, configDir string) error { return nil } +func decodeStringAsArrayField(from reflect.Type, to reflect.Type, value interface{}) ( + interface{}, error) { + if to != reflect.TypeOf(config.FieldStringArrayType{}) || from.Kind() != reflect.String { + return value, nil + } + return []string{value.(string)}, nil +} + +func decodeConfig(input map[string]any, cfg *config.CliOpts) error { + decoder_config := mapstructure.DecoderConfig{ + Result: cfg, + DecodeHook: mapstructure.ComposeDecodeHookFunc(decodeStringAsArrayField), + } + decoder, err := mapstructure.NewDecoder(&decoder_config) + if err != nil { + return err + } + return decoder.Decode(input) +} + // GetCliOpts returns Tarantool CLI options from the config file // located at path configurePath. func GetCliOpts(configurePath string, repository integrity.Repository) ( @@ -250,7 +289,7 @@ func GetCliOpts(configurePath string, repository integrity.Repository) ( return nil, "", fmt.Errorf("failed to parse Tarantool CLI configuration: %s", err) } - if err := mapstructure.Decode(rawConfigOpts, &cfg); err != nil { + if err := decodeConfig(rawConfigOpts, cfg); err != nil { return nil, "", fmt.Errorf("failed to parse Tarantool CLI configuration: %s", err) } diff --git a/cli/configure/configure_test.go b/cli/configure/configure_test.go index 3c669cb2b..7c3be8a75 100644 --- a/cli/configure/configure_test.go +++ b/cli/configure/configure_test.go @@ -334,6 +334,67 @@ func TestUpdateCliOpts(t *testing.T) { assert.Equal(t, "./var/lib/vinyl", cliOpts.App.VinylDir) assert.Equal(t, "./var/lib/snap", cliOpts.App.MemtxDir) assert.Equal(t, filepath.Join(configDir, "..", "include_dir"), cliOpts.Env.IncludeDir) - assert.Equal(t, filepath.Join(configDir, ModulesPath), cliOpts.Modules.Directory) + assert.Equal(t, 1, len(cliOpts.Modules.Directories)) + assert.Equal(t, filepath.Join(configDir, ModulesPath), cliOpts.Modules.Directories[0]) assert.Equal(t, configDir, cliOpts.Env.InstancesEnabled) } + +func TestGetCliOpts_modules_directory(t *testing.T) { + work_dir, err := os.Getwd() + require.NoError(t, err) + work_dir = filepath.Join(work_dir, "testdata/modules_cfg") + + tests := []struct { + name string + config string + modules_dir config.FieldStringArrayType + cfg_path string + }{ + { + name: "Single string relative path", + config: "tt-modules1", + modules_dir: []string{filepath.Join(work_dir, "modules-dir")}, + cfg_path: "tt-modules1.yaml", + }, + { + name: "Single entry list", + config: "tt-modules2", + modules_dir: []string{filepath.Join(work_dir, "modules-dir")}, + cfg_path: "tt-modules2.yml", + }, + { + name: "Multiple entries list", + config: "tt-modules3.", + modules_dir: []string{ + filepath.Join(work_dir, "modules-dir"), + "/ext/path/modules", + filepath.Join(work_dir, "local_modules"), + }, + cfg_path: "tt-modules3.yaml", + }, + { + name: "Empty list = default value", + config: "tt-modules4.yaml", + modules_dir: []string{filepath.Join(work_dir, "modules")}, + cfg_path: "tt-modules4.yml", + }, + { + name: "Single string absolute path", + config: "tt-modules5.yml", + modules_dir: []string{"/ext/path/modules"}, + cfg_path: "tt-modules5.yaml", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockRepo := newMockRepository() + config := filepath.Join(work_dir, tt.config) + opts, cfg, err := GetCliOpts(config, &mockRepo) + require.NoError(t, err) + require.NotNil(t, opts.Modules) + require.Equal(t, tt.modules_dir, opts.Modules.Directories) + require.Equal(t, filepath.Join(work_dir, tt.cfg_path), cfg) + }) + } +} diff --git a/cli/configure/testdata/modules_cfg/tt-modules1.yaml b/cli/configure/testdata/modules_cfg/tt-modules1.yaml new file mode 100644 index 000000000..afc467f0b --- /dev/null +++ b/cli/configure/testdata/modules_cfg/tt-modules1.yaml @@ -0,0 +1,3 @@ +# Config with single string with relative path to modules directory. +modules: + directory: modules-dir diff --git a/cli/configure/testdata/modules_cfg/tt-modules2.yml b/cli/configure/testdata/modules_cfg/tt-modules2.yml new file mode 100644 index 000000000..2f2b70311 --- /dev/null +++ b/cli/configure/testdata/modules_cfg/tt-modules2.yml @@ -0,0 +1,4 @@ +# Config with single list entry of relative path to modules directory. +modules: + directory: + - modules-dir diff --git a/cli/configure/testdata/modules_cfg/tt-modules3.yaml b/cli/configure/testdata/modules_cfg/tt-modules3.yaml new file mode 100644 index 000000000..382065e29 --- /dev/null +++ b/cli/configure/testdata/modules_cfg/tt-modules3.yaml @@ -0,0 +1,6 @@ +# Multiple directory items. +modules: + directory: + - modules-dir + - /ext/path/modules + - local_modules diff --git a/cli/configure/testdata/modules_cfg/tt-modules4.yml b/cli/configure/testdata/modules_cfg/tt-modules4.yml new file mode 100644 index 000000000..4f6818a61 --- /dev/null +++ b/cli/configure/testdata/modules_cfg/tt-modules4.yml @@ -0,0 +1,3 @@ +# Empty config value, to use default. +modules: + directory: diff --git a/cli/configure/testdata/modules_cfg/tt-modules5.yaml b/cli/configure/testdata/modules_cfg/tt-modules5.yaml new file mode 100644 index 000000000..acebff894 --- /dev/null +++ b/cli/configure/testdata/modules_cfg/tt-modules5.yaml @@ -0,0 +1,3 @@ +# Config with single string with absolute path to modules directory. +modules: + directory: /ext/path/modules diff --git a/cli/init/init.go b/cli/init/init.go index 89a69be0e..3dabaed71 100644 --- a/cli/init/init.go +++ b/cli/init/init.go @@ -168,11 +168,12 @@ func generateTtEnv(configPath string, sourceCfg configData) error { directoriesToCreate := []string{ cfg.Env.InstancesEnabled, - cfg.Modules.Directory, cfg.Env.IncludeDir, cfg.Env.BinDir, cfg.Repo.Install, } + // FIXME: Need select only internal directories https://github.com/tarantool/tt/issues/1014 + directoriesToCreate = append(directoriesToCreate, cfg.Modules.Directories...) for _, templatesPathOpts := range cfg.Templates { directoriesToCreate = append(directoriesToCreate, templatesPathOpts.Path) } diff --git a/cli/init/init_test.go b/cli/init/init_test.go index 1feed894a..7d222c1b1 100644 --- a/cli/init/init_test.go +++ b/cli/init/init_test.go @@ -63,7 +63,7 @@ func checkDefaultEnv(t *testing.T, configName string, instancesEnabled string) { assert.Equal(t, "var/run", cfg.App.RunDir) assert.Equal(t, "var/log", cfg.App.LogDir) assert.Equal(t, "bin", cfg.Env.BinDir) - assert.Equal(t, "modules", cfg.Modules.Directory) + assert.Equal(t, config.FieldStringArrayType{"modules"}, cfg.Modules.Directories) assert.Equal(t, "distfiles", cfg.Repo.Install) assert.Equal(t, "include", cfg.Env.IncludeDir) assert.Equal(t, "templates", cfg.Templates[0].Path) diff --git a/cli/init/templates/tt.yaml.default b/cli/init/templates/tt.yaml.default index 5375c4074..112426685 100644 --- a/cli/init/templates/tt.yaml.default +++ b/cli/init/templates/tt.yaml.default @@ -1,6 +1,6 @@ modules: # Directory where the external modules are stored. - directory: {{ .Modules.Directory }} + directory: {{ .Modules.Directories }} env: # Restart instance on failure. diff --git a/cli/modules/modules.go b/cli/modules/modules.go index f94439fcc..9e32b58d2 100644 --- a/cli/modules/modules.go +++ b/cli/modules/modules.go @@ -94,7 +94,8 @@ func getExternalModulesDir(cmdCtx *cmdcontext.CmdCtx, cliOpts *config.CliOpts) ( // 1. If a directory field is specified; // 2. Specified path exists; // 3. Path points to not a directory. - modulesDir := cliOpts.Modules.Directory + // FIXME: Add working with a list https://github.com/tarantool/tt/issues/1014 + modulesDir := cliOpts.Modules.Directories[0] if info, err := os.Stat(modulesDir); err == nil { // TODO: Add warning in next patches, discussion // what if the file exists, but access is denied, etc. diff --git a/cli/pack/common.go b/cli/pack/common.go index 3f352da37..6d726ef38 100644 --- a/cli/pack/common.go +++ b/cli/pack/common.go @@ -115,7 +115,7 @@ func ttEnvironmentFilters(packCtx *PackCtx, cliOpts *config.CliOpts) []func( cliOpts.Env.InstancesEnabled, cliOpts.Env.BinDir) } if cliOpts.Modules != nil { - envPaths = append(envPaths, cliOpts.Modules.Directory) + envPaths = append(envPaths, cliOpts.Modules.Directories...) } if cliOpts.Repo != nil { envPaths = append(envPaths, cliOpts.Repo.Install) @@ -204,24 +204,27 @@ func updateEnvPath(basePath string, packCtx *PackCtx, cliOpts *config.CliOpts) ( // copyEnvModules copies tt modules. func copyEnvModules(bundleEnvPath string, packCtx *PackCtx, cliOpts, newOpts *config.CliOpts) { if packCtx.WithoutModules || packCtx.CartridgeCompat || cliOpts.Modules == nil || - cliOpts.Modules.Directory == "" { + len(cliOpts.Modules.Directories) == 0 { return } - if !util.IsDir(cliOpts.Modules.Directory) { - log.Debugf("Skip copying modules from %q: does not exist or not a directory", - cliOpts.Modules.Directory) - } else { - dir, err := os.Open(cliOpts.Modules.Directory) - if err != nil { - log.Warnf("cannot open %q for reading: %s", cliOpts.Modules.Directory, err) - } - if files, _ := dir.Readdir(1); len(files) == 0 { - return // No modules. - } - if err := copy.Copy(cliOpts.Modules.Directory, - util.JoinPaths(bundleEnvPath, newOpts.Modules.Directory)); err != nil { - log.Warnf("Failed to copy modules from %q: %s", cliOpts.Modules.Directory, err) + for _, directory := range cliOpts.Modules.Directories { + if !util.IsDir(directory) { + log.Debugf("Skip copying modules from %q: does not exist or not a directory", + directory) + } else { + dir, err := os.Open(directory) + if err != nil { + log.Warnf("cannot open %q for reading: %s", directory, err) + } + if files, _ := dir.Readdir(1); len(files) == 0 { + return // No modules. + } + // FIXME: Add working with a list https://github.com/tarantool/tt/issues/1014 + if err := copy.Copy(directory, + util.JoinPaths(bundleEnvPath, newOpts.Modules.Directories[0])); err != nil { + log.Warnf("Failed to copy modules from %q: %s", directory, err) + } } } } diff --git a/cli/pack/common_test.go b/cli/pack/common_test.go index e7fc81b68..7d83d67de 100644 --- a/cli/pack/common_test.go +++ b/cli/pack/common_test.go @@ -243,7 +243,7 @@ func Test_createNewOpts(t *testing.T) { RunDir: "var/run", }, Modules: &config.ModulesOpts{ - Directory: "modules", + Directories: []string{"modules"}, }, Repo: &config.RepoOpts{ Rocks: "", @@ -280,7 +280,7 @@ func Test_createNewOpts(t *testing.T) { RunDir: "var/run", }, Modules: &config.ModulesOpts{ - Directory: "modules", + Directories: []string{"modules"}, }, Repo: &config.RepoOpts{ Rocks: "", @@ -317,7 +317,7 @@ func Test_createNewOpts(t *testing.T) { RunDir: "/var/run/tarantool/bundle", }, Modules: &config.ModulesOpts{ - Directory: "modules", + Directories: []string{"modules"}, }, Repo: &config.RepoOpts{ Rocks: "", @@ -360,7 +360,7 @@ func Test_createNewOpts(t *testing.T) { RunDir: "var/run", }, Modules: &config.ModulesOpts{ - Directory: "modules", + Directories: []string{"modules"}, }, Repo: &config.RepoOpts{ Rocks: "", diff --git a/test/integration/cfg/test_dump.py b/test/integration/cfg/test_dump.py index a64ba0300..2159d731c 100644 --- a/test/integration/cfg/test_dump.py +++ b/test/integration/cfg/test_dump.py @@ -30,7 +30,7 @@ def test_cfg_dump_default(tt_cmd, tmp_path): assert f"vinyl_dir: {os.path.join('lib', 'vinyl')}" in output assert f"log_dir: {os.path.join('./var', 'log')}" in output assert f"inc_dir: {os.path.join(tmp_path, 'include')}" in output - assert f"directory: {os.path.join(tmp_path, 'new_modules')}" in output + assert f"modules:\n directory:\n - {os.path.join(tmp_path, 'new_modules')}" in output assert f"distfiles: {os.path.join(tmp_path, 'distfiles')}" in output assert f"instances_enabled: {tmp_path}" in output assert f"templates:\n- path: {os.path.join(tmp_path, 'templates')}" in output @@ -110,7 +110,7 @@ def test_cfg_dump_default_no_config(tt_cmd, tmp_path): assert f"memtx_dir: {os.path.join('var', 'lib')}" in output assert f"log_dir: {os.path.join('var', 'log')}" in output assert f"inc_dir: {os.path.join(tmp_path, 'include')}" in output - assert f"directory: {os.path.join(tmp_path, 'modules')}" in output + assert f"modules:\n directory:\n - {os.path.join(tmp_path, 'modules')}" in output assert f"distfiles: {os.path.join(tmp_path, 'distfiles')}" in output assert f"instances_enabled: {tmp_path}" in output assert f"templates:\n- path: {os.path.join(tmp_path, 'templates')}" in output @@ -144,7 +144,7 @@ def test_cfg_dump_default_no_config(tt_cmd, tmp_path): assert f"memtx_dir: {os.path.join('var', 'lib')}" in output assert f"log_dir: {os.path.join('var', 'log')}" in output assert f"inc_dir: {os.path.join(tmp_path, 'include')}" in output - assert f"directory: {os.path.join(tmp_path, 'modules')}" in output + assert f"modules:\n directory:\n - {os.path.join(tmp_path, 'modules')}" in output assert f"distfiles: {os.path.join(tmp_path, 'distfiles')}" in output assert "instances_enabled: ." in output assert f"templates:\n- path: {os.path.join(tmp_path, 'templates')}" in output diff --git a/test/integration/init/test_init.py b/test/integration/init/test_init.py index 32b8d853c..3c827037e 100644 --- a/test/integration/init/test_init.py +++ b/test/integration/init/test_init.py @@ -69,7 +69,7 @@ def test_init_missing_configs(tt_cmd, tmp_path): assert data_loaded["app"]["memtx_dir"] == "var/lib" assert data_loaded["env"]["instances_enabled"] == "instances.enabled" assert not data_loaded["env"]["tarantoolctl_layout"] - assert data_loaded["modules"]["directory"] == "modules" + assert data_loaded["modules"]["directory"] == ["modules"] assert data_loaded["env"]["bin_dir"] == "bin" assert data_loaded["templates"][0]["path"] == "templates" assert data_loaded["repo"]["distfiles"] == "distfiles"