From 38aaec3135abc7597068eb97c9dbf54b6b9cbe09 Mon Sep 17 00:00:00 2001 From: Omar Polo Date: Tue, 9 Jun 2026 12:07:07 +0000 Subject: [PATCH 1/3] config: move config-related code from utils no reason to splatter the code for the configuration handling among different packages, try to regroup everything under config. --- appcontext/appcontext.go | 3 +- config/load.go | 208 +++++++++++ utils/config_test.go => config/load_test.go | 86 +++-- config/loadfile.go | 128 +++++++ utils/oldconfig.go => config/old.go | 16 +- utils/oldconfig_test.go => config/old_test.go | 2 +- subcommands/config/config.go | 14 +- subcommands/config/config_test.go | 8 +- subcommands/sync/sync_test.go | 6 +- utils/config.go | 351 ------------------ 10 files changed, 401 insertions(+), 421 deletions(-) create mode 100644 config/load.go rename utils/config_test.go => config/load_test.go (82%) create mode 100644 config/loadfile.go rename utils/oldconfig.go => config/old.go (60%) rename utils/oldconfig_test.go => config/old_test.go (99%) delete mode 100644 utils/config.go diff --git a/appcontext/appcontext.go b/appcontext/appcontext.go index 4b3f9bc96..19b59b400 100644 --- a/appcontext/appcontext.go +++ b/appcontext/appcontext.go @@ -6,7 +6,6 @@ import ( "github.com/PlakarKorp/pkg" "github.com/PlakarKorp/plakar/config" "github.com/PlakarKorp/plakar/cookies" - "github.com/PlakarKorp/plakar/utils" ) type AppContext struct { @@ -97,7 +96,7 @@ func (c *AppContext) GetPkgManager() *pkg.Manager { } func (c *AppContext) ReloadConfig() error { - cfg, err := utils.LoadConfig(c.ConfigDir) + cfg, err := config.Load(c.ConfigDir) if err != nil { return err } diff --git a/config/load.go b/config/load.go new file mode 100644 index 000000000..dec944cce --- /dev/null +++ b/config/load.go @@ -0,0 +1,208 @@ +package config + +import ( + "fmt" + "io" + "os" + "path/filepath" + + "go.yaml.in/yaml/v3" +) + +const CONFIG_VERSION = "v1.0.0" + +type ( + storesConfig struct { + Version string `yaml:"version"` + Default string `yaml:"default,omitempty"` + Stores map[string]map[string]string `yaml:"stores"` + } + + sourcesConfig struct { + Version string `yaml:"version"` + Sources map[string]map[string]string `yaml:"sources"` + } + + destinationsConfig struct { + Version string `yaml:"version"` + Destinations map[string]map[string]string `yaml:"destinations"` + } +) + +func load(file string, dst any) error { + f, err := os.Open(file) + if err != nil { + if os.IsNotExist(err) { + return err + } + return fmt.Errorf("failed to open file %s: %w", file, err) + } + defer f.Close() + + info, err := f.Stat() + if err != nil { + return fmt.Errorf("failed to stat file %s: %w", file, err) + } + if info.Size() == 0 { + return nil + } + + // try to load the new format + err = yaml.NewDecoder(f).Decode(dst) + var version string + switch t := dst.(type) { + case *storesConfig: + version = t.Version + case *destinationsConfig: + version = t.Version + case *sourcesConfig: + version = t.Version + default: + return fmt.Errorf("invalid configuration type %v", t) + } + if err == nil && version == CONFIG_VERSION { + return nil + } + + // fallback to the previous format + _, err = f.Seek(0, io.SeekStart) + if err != nil { + return fmt.Errorf("failed to rewind config file: %w", err) + } + + switch t := dst.(type) { + case *storesConfig: + err = yaml.NewDecoder(f).Decode(&t.Stores) + if err == nil { + for k, v := range t.Stores { + if _, ok := v[".isDefault"]; ok { + if t.Default != "" { + return fmt.Errorf("multiple default store") + } + t.Default = k + delete(v, ".isDefault") + } + } + } + case *destinationsConfig: + err = yaml.NewDecoder(f).Decode(&t.Destinations) + case *sourcesConfig: + err = yaml.NewDecoder(f).Decode(&t.Sources) + } + if err != nil { + return fmt.Errorf("failed to parse config file: %w", err) + } + + return nil +} + +func loadFallback(dir string) (*Config, error) { + // Load old config if found + oldpath := filepath.Join(dir, "plakar.yml") + cfg, err := LoadOldConfigIfExists(oldpath) + if err != nil { + return nil, fmt.Errorf("error reading file %s: %w", oldpath, err) + } + + // Save the config in the new format right now + if err := Save(dir, cfg); err != nil { + return nil, fmt.Errorf("failed to update config file: %w", err) + } + // Do we want to remove the old file? + return cfg, nil +} + +func Load(dir string) (*Config, error) { + sources := sourcesConfig{} + destinations := destinationsConfig{} + stores := storesConfig{} + + err := load(filepath.Join(dir, "sources.yml"), &sources) + if err != nil { + if os.IsNotExist(err) { + return loadFallback(dir) + } + return nil, err + } + + err = load(filepath.Join(dir, "destinations.yml"), &destinations) + if err != nil { + if os.IsNotExist(err) { + return loadFallback(dir) + } + return nil, err + } + + err = load(filepath.Join(dir, "stores.yml"), &stores) + if err != nil && os.IsNotExist(err) { + // try to load former file + err = load(filepath.Join(dir, "klosets.yml"), &stores) + } + if err != nil { + if os.IsNotExist(err) { + return loadFallback(dir) + } + return nil, err + } + + cfg := NewConfig() + cfg.Sources = sources.Sources + cfg.Destinations = destinations.Destinations + cfg.Repositories = stores.Stores + cfg.DefaultRepository = stores.Default + + return cfg, nil +} + +func save(file string, src any) error { + tmpFile, err := os.CreateTemp(filepath.Dir(file), "config.*.yml") + if err != nil { + return err + } + + err = yaml.NewEncoder(tmpFile).Encode(src) + tmpFile.Close() + + if err == nil { + err = os.Rename(tmpFile.Name(), file) + } + + if err != nil { + os.Remove(tmpFile.Name()) + return err + } + + return nil +} + +func Save(dir string, cfg *Config) error { + // Create the config directory if it doesn't exist + err := os.MkdirAll(dir, 0700) + if err != nil { + return err + } + + err = save(filepath.Join(dir, "sources.yml"), sourcesConfig{ + Version: CONFIG_VERSION, + Sources: cfg.Sources, + }) + if err != nil { + return err + } + err = save(filepath.Join(dir, "destinations.yml"), destinationsConfig{ + Version: CONFIG_VERSION, + Destinations: cfg.Destinations, + }) + if err != nil { + return err + } + err = save(filepath.Join(dir, "stores.yml"), storesConfig{ + Version: CONFIG_VERSION, + Default: cfg.DefaultRepository, + Stores: cfg.Repositories, + }) + if err != nil { + return err + } + return nil +} diff --git a/utils/config_test.go b/config/load_test.go similarity index 82% rename from utils/config_test.go rename to config/load_test.go index 5f88a86ab..0a8b43c6a 100644 --- a/utils/config_test.go +++ b/config/load_test.go @@ -1,35 +1,33 @@ -package utils +package config import ( "os" "path/filepath" "strings" "testing" - - "github.com/PlakarKorp/plakar/config" ) -func TestLoadConfigEmptyDirFallsBackAndReturnsBlank(t *testing.T) { +func TestLoadEmptyDirFallsBackAndReturnsBlank(t *testing.T) { dir := t.TempDir() - cfg, err := LoadConfig(dir) + cfg, err := Load(dir) if err != nil { - t.Fatalf("LoadConfig: %v", err) + t.Fatalf("Load: %v", err) } if cfg == nil { t.Fatal("expected non-nil config") } } -func TestSaveAndLoadConfigRoundTrip(t *testing.T) { +func TestSaveAndLoadRoundTrip(t *testing.T) { dir := t.TempDir() - cfg := config.NewConfig() + cfg := NewConfig() cfg.DefaultRepository = "home" cfg.Repositories["home"] = map[string]string{"location": "/var/data/repo"} cfg.Sources["src"] = map[string]string{"location": "fs:///var/source"} cfg.Destinations["dst"] = map[string]string{"location": "fs:///var/dest"} - if err := SaveConfig(dir, cfg); err != nil { - t.Fatalf("SaveConfig: %v", err) + if err := Save(dir, cfg); err != nil { + t.Fatalf("Save: %v", err) } // All three files should exist. @@ -39,9 +37,9 @@ func TestSaveAndLoadConfigRoundTrip(t *testing.T) { } } - loaded, err := LoadConfig(dir) + loaded, err := Load(dir) if err != nil { - t.Fatalf("LoadConfig after save: %v", err) + t.Fatalf("Load after save: %v", err) } if loaded.DefaultRepository != "home" { t.Fatalf("DefaultRepository = %q, want home", loaded.DefaultRepository) @@ -57,7 +55,7 @@ func TestSaveAndLoadConfigRoundTrip(t *testing.T) { } } -func TestLoadConfigFallbackFromOldFormat(t *testing.T) { +func TestLoadFallbackFromOldFormat(t *testing.T) { dir := t.TempDir() old := ` default-repo: legacy @@ -72,9 +70,9 @@ remotes: t.Fatalf("write old config: %v", err) } - cfg, err := LoadConfig(dir) + cfg, err := Load(dir) if err != nil { - t.Fatalf("LoadConfig: %v", err) + t.Fatalf("Load: %v", err) } if cfg.DefaultRepository != "legacy" { t.Fatalf("DefaultRepository = %q, want legacy", cfg.DefaultRepository) @@ -90,7 +88,7 @@ remotes: } } -func TestLoadConfigKlosetsYmlFallback(t *testing.T) { +func TestLoadKlosetsYmlFallback(t *testing.T) { dir := t.TempDir() // Provide sources.yml + destinations.yml + the older klosets.yml; loader // should fall through to klosets.yml when stores.yml is absent. @@ -104,16 +102,16 @@ func TestLoadConfigKlosetsYmlFallback(t *testing.T) { mustWrite("destinations.yml", "version: v1.0.0\ndestinations:\n d:\n location: fs:///d\n") mustWrite("klosets.yml", "version: v1.0.0\ndefault: r\nstores:\n r:\n location: fs:///r\n") - cfg, err := LoadConfig(dir) + cfg, err := Load(dir) if err != nil { - t.Fatalf("LoadConfig: %v", err) + t.Fatalf("Load: %v", err) } if cfg.DefaultRepository != "r" { t.Fatalf("DefaultRepository = %q, want r (read from klosets.yml)", cfg.DefaultRepository) } } -func TestLoadConfigBackwardCompatPreviousStoreFormat(t *testing.T) { +func TestLoadBackwardCompatPreviousStoreFormat(t *testing.T) { // Old store file format: top-level map keyed by name; an entry can carry // `.isDefault: true` to mark itself as default. dir := t.TempDir() @@ -126,9 +124,9 @@ func TestLoadConfigBackwardCompatPreviousStoreFormat(t *testing.T) { mustWrite("destinations.yml", "version: v1.0.0\ndestinations: {}\n") mustWrite("stores.yml", "home:\n location: /var/h\n .isDefault: \"true\"\nother:\n location: /var/o\n") - cfg, err := LoadConfig(dir) + cfg, err := Load(dir) if err != nil { - t.Fatalf("LoadConfig: %v", err) + t.Fatalf("Load: %v", err) } if cfg.DefaultRepository != "home" { t.Fatalf("DefaultRepository = %q, want home (from .isDefault)", cfg.DefaultRepository) @@ -141,7 +139,7 @@ func TestLoadConfigBackwardCompatPreviousStoreFormat(t *testing.T) { } } -func TestLoadConfigMultipleDefaultsIsError(t *testing.T) { +func TestLoadMultipleDefaultsIsError(t *testing.T) { dir := t.TempDir() mustWrite := func(name, body string) { if err := os.WriteFile(filepath.Join(dir, name), []byte(body), 0o600); err != nil { @@ -152,7 +150,7 @@ func TestLoadConfigMultipleDefaultsIsError(t *testing.T) { mustWrite("destinations.yml", "version: v1.0.0\ndestinations: {}\n") mustWrite("stores.yml", "a:\n .isDefault: \"true\"\nb:\n .isDefault: \"true\"\n") - if _, err := LoadConfig(dir); err == nil { + if _, err := Load(dir); err == nil { t.Fatal("expected error for multiple default stores, got nil") } } @@ -166,7 +164,7 @@ key2 = value2 [section2] foo = bar `) - got, err := LoadINI(rd) + got, err := loadINI(rd) if err != nil { t.Fatalf("LoadINI: %v", err) } @@ -179,7 +177,7 @@ foo = bar } func TestLoadINIBad(t *testing.T) { - if _, err := LoadINI(strings.NewReader("[unclosed")); err == nil { + if _, err := loadINI(strings.NewReader("[unclosed")); err == nil { t.Fatal("expected parse error, got nil") } } @@ -191,7 +189,7 @@ remote: port: 22 ssl: true `) - got, err := LoadYAML(rd) + got, err := loadYAML(rd) if err != nil { t.Fatalf("LoadYAML: %v", err) } @@ -209,7 +207,7 @@ remote: func TestLoadYAMLSkipsScalarTopLevel(t *testing.T) { // Top-level scalar keys are skipped rather than producing an error. rd := strings.NewReader("version: v1.0.0\nremote:\n location: x\n") - got, err := LoadYAML(rd) + got, err := loadYAML(rd) if err != nil { t.Fatalf("LoadYAML: %v", err) } @@ -222,14 +220,14 @@ func TestLoadYAMLSkipsScalarTopLevel(t *testing.T) { } func TestLoadYAMLBad(t *testing.T) { - if _, err := LoadYAML(strings.NewReader("not: [valid: yaml")); err == nil { + if _, err := loadYAML(strings.NewReader("not: [valid: yaml")); err == nil { t.Fatal("expected parse error, got nil") } } func TestLoadJSON(t *testing.T) { rd := strings.NewReader(`{"a":{"k":"v"},"b":{"x":"y"}}`) - got, err := LoadJSON(rd) + got, err := loadJSON(rd) if err != nil { t.Fatalf("LoadJSON: %v", err) } @@ -239,7 +237,7 @@ func TestLoadJSON(t *testing.T) { } func TestLoadJSONBad(t *testing.T) { - if _, err := LoadJSON(strings.NewReader("nope")); err == nil { + if _, err := loadJSON(strings.NewReader("nope")); err == nil { t.Fatal("expected parse error, got nil") } } @@ -265,15 +263,15 @@ func TestToString(t *testing.T) { } } -func TestGetConfYAMLWithLocation(t *testing.T) { +func TestLoadFileYAMLWithLocation(t *testing.T) { rd := strings.NewReader(` remote: location: ssh://host port: 22 `) - got, err := GetConf(rd, "") + got, err := LoadFile(rd, "") if err != nil { - t.Fatalf("GetConf: %v", err) + t.Fatalf("LoadFile: %v", err) } if got["remote"]["location"] != "ssh://host" { t.Fatalf("location = %q", got["remote"]["location"]) @@ -283,25 +281,25 @@ remote: } } -func TestGetConfYAMLMissingLocationIsError(t *testing.T) { +func TestLoadFileYAMLMissingLocationIsError(t *testing.T) { rd := strings.NewReader(` remote: port: 22 `) - if _, err := GetConf(rd, ""); err == nil { + if _, err := LoadFile(rd, ""); err == nil { t.Fatal("expected error for missing 'location', got nil") } } -func TestGetConfThirdPartyRewritesKeys(t *testing.T) { +func TestLoadFileThirdPartyRewritesKeys(t *testing.T) { rd := strings.NewReader(` remote: host: example.com port: 22 `) - got, err := GetConf(rd, "rclone") + got, err := LoadFile(rd, "rclone") if err != nil { - t.Fatalf("GetConf: %v", err) + t.Fatalf("LoadFile: %v", err) } if got["remote"]["location"] != "rclone://" { t.Fatalf("location = %q, want rclone://", got["remote"]["location"]) @@ -318,29 +316,29 @@ remote: } } -func TestGetConfINISource(t *testing.T) { +func TestLoadFileINISource(t *testing.T) { rd := strings.NewReader(` [remote] location = fs:///x `) - got, err := GetConf(rd, "") + got, err := LoadFile(rd, "") if err != nil { - t.Fatalf("GetConf: %v", err) + t.Fatalf("LoadFile: %v", err) } if got["remote"]["location"] != "fs:///x" { t.Fatalf("location = %q", got["remote"]["location"]) } } -func TestGetConfStripsEmptyValues(t *testing.T) { +func TestLoadFileStripsEmptyValues(t *testing.T) { rd := strings.NewReader(` remote: location: fs:///x empty: "" `) - got, err := GetConf(rd, "") + got, err := LoadFile(rd, "") if err != nil { - t.Fatalf("GetConf: %v", err) + t.Fatalf("LoadFile: %v", err) } if _, has := got["remote"]["empty"]; has { t.Fatal("empty value should have been stripped") diff --git a/config/loadfile.go b/config/loadfile.go new file mode 100644 index 000000000..594e9573e --- /dev/null +++ b/config/loadfile.go @@ -0,0 +1,128 @@ +package config + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "slices" + + "gopkg.in/ini.v1" + + "go.yaml.in/yaml/v3" +) + +func toString(v any) string { + switch t := v.(type) { + case string: + return t + case int, int64, float64, bool: + return fmt.Sprintf("%v", t) + default: + return "" + } +} + +func loadINI(rd io.Reader) (map[string]map[string]string, error) { + cfg, err := ini.Load(rd) + if err != nil { + return nil, err + } + + keysMap := make(map[string]struct{}) + result := make(map[string]map[string]string) + for _, section := range cfg.Sections() { + name := section.Name() + if name == ini.DefaultSection { + continue + } + keysMap[name] = struct{}{} + result[name] = make(map[string]string) + for _, key := range section.Keys() { + result[name][key.Name()] = key.Value() + } + } + return result, nil +} + +func loadYAML(rd io.Reader) (map[string]map[string]string, error) { + var raw map[string]any + decoder := yaml.NewDecoder(rd) + if err := decoder.Decode(&raw); err != nil { + return nil, err + } + + result := make(map[string]map[string]string) + for section, value := range raw { + sectionMap, ok := value.(map[string]any) + if !ok { + continue // skip non-object top-level keys + } + result[section] = make(map[string]string) + for k, v := range sectionMap { + result[section][k] = toString(v) + } + } + + return result, nil +} + +func loadJSON(rd io.Reader) (map[string]map[string]string, error) { + var raw map[string]map[string]string + decoder := json.NewDecoder(rd) + if err := decoder.Decode(&raw); err != nil { + return nil, err + } + return raw, nil +} + +// LoadFile attempts to load a file from the given reader, which could +// be YAML, JSON or INI, and parse it into a nested map which can be used +// then to fill one of the fields of Config. +func LoadFile(rd io.Reader, thirdParty string) (map[string]map[string]string, error) { + data, err := io.ReadAll(rd) + if err != nil { + return nil, fmt.Errorf("failed to read config data: %w", err) + } + + var configMap map[string]map[string]string + if configMap, err = loadYAML(bytes.NewReader(data)); err == nil { + } else if configMap, err = loadJSON(bytes.NewReader(data)); err == nil { + } else if configMap, err = loadINI(bytes.NewReader(data)); err != nil { + return nil, fmt.Errorf("failed to parse config data: %w", err) + } + + if thirdParty != "" { + for _, section := range configMap { + var ignore []string + for key, value := range section { + if slices.Contains(ignore, key) { + continue + } + if value != "" { + newKey := thirdParty + "_" + key + section[newKey] = value + ignore = append(ignore, newKey) + } + delete(section, key) + } + section["location"] = thirdParty + "://" + } + } + + seenLocation := false + for _, section := range configMap { + for key, value := range section { + if key == "location" { + seenLocation = true + } + if value == "" { + delete(section, key) + } + } + } + if !seenLocation { + return nil, fmt.Errorf("missing 'location' key in config data, `-rclone` import option missing ?") + } + return configMap, nil +} diff --git a/utils/oldconfig.go b/config/old.go similarity index 60% rename from utils/oldconfig.go rename to config/old.go index 7c7ee2b18..a9f4c7351 100644 --- a/utils/oldconfig.go +++ b/config/old.go @@ -1,4 +1,4 @@ -package utils +package config import ( "fmt" @@ -6,18 +6,16 @@ import ( "os" "go.yaml.in/yaml/v3" - - "github.com/PlakarKorp/plakar/config" ) type OldConfig struct { - DefaultRepository string `yaml:"default-repo"` - Repositories map[string]config.RepositoryConfig `yaml:"repositories"` - Remotes map[string]config.SourceConfig `yaml:"remotes"` + DefaultRepository string `yaml:"default-repo"` + Repositories map[string]RepositoryConfig `yaml:"repositories"` + Remotes map[string]SourceConfig `yaml:"remotes"` } -func LoadOldConfigIfExists(configFile string) (*config.Config, error) { - cfg := config.NewConfig() +func LoadOldConfigIfExists(configFile string) (*Config, error) { + cfg := NewConfig() f, err := os.Open(configFile) if err != nil { @@ -36,7 +34,7 @@ func LoadOldConfigIfExists(configFile string) (*config.Config, error) { cfg.DefaultRepository = old.DefaultRepository cfg.Repositories = old.Repositories cfg.Sources = old.Remotes - cfg.Destinations = make(map[string]config.DestinationConfig) + cfg.Destinations = make(map[string]DestinationConfig) for key, val := range cfg.Sources { res := make(map[string]string) maps.Copy(res, val) diff --git a/utils/oldconfig_test.go b/config/old_test.go similarity index 99% rename from utils/oldconfig_test.go rename to config/old_test.go index 9b5404f36..86cca53dd 100644 --- a/utils/oldconfig_test.go +++ b/config/old_test.go @@ -1,4 +1,4 @@ -package utils +package config import ( "os" diff --git a/subcommands/config/config.go b/subcommands/config/config.go index 0e1c1567b..3301d85b5 100644 --- a/subcommands/config/config.go +++ b/subcommands/config/config.go @@ -30,8 +30,8 @@ import ( "github.com/PlakarKorp/kloset/connectors/importer" "github.com/PlakarKorp/kloset/connectors/storage" "github.com/PlakarKorp/plakar/appcontext" + "github.com/PlakarKorp/plakar/config" "github.com/PlakarKorp/plakar/subcommands" - "github.com/PlakarKorp/plakar/utils" "go.yaml.in/yaml/v3" "gopkg.in/ini.v1" ) @@ -110,7 +110,7 @@ func dispatchSubcommand(ctx *appcontext.AppContext, cmd string, subcmd string, a } cfgMap[name][key] = val } - return utils.SaveConfig(ctx.ConfigDir, ctx.Config) + return config.Save(ctx.ConfigDir, ctx.Config) case "check": p := flag.NewFlagSet("check", flag.ExitOnError) @@ -199,7 +199,7 @@ func dispatchSubcommand(ctx *appcontext.AppContext, cmd string, subcmd string, a thirdParty = "rclone" } - newConfMap, err := utils.GetConf(rd, thirdParty) + newConfMap, err := config.LoadFile(rd, thirdParty) if err != nil { return fmt.Errorf("failed to load config: %w", err) } @@ -240,7 +240,7 @@ func dispatchSubcommand(ctx *appcontext.AppContext, cmd string, subcmd string, a } } } - return utils.SaveConfig(ctx.ConfigDir, ctx.Config) + return config.Save(ctx.ConfigDir, ctx.Config) case "ping": p := flag.NewFlagSet("ping", flag.ExitOnError) @@ -320,7 +320,7 @@ func dispatchSubcommand(ctx *appcontext.AppContext, cmd string, subcmd string, a return fmt.Errorf("%s %q does not exist", cmd, name) } delete(cfgMap, name) - return utils.SaveConfig(ctx.ConfigDir, ctx.Config) + return config.Save(ctx.ConfigDir, ctx.Config) case "set": p := flag.NewFlagSet("set", flag.ExitOnError) @@ -344,7 +344,7 @@ func dispatchSubcommand(ctx *appcontext.AppContext, cmd string, subcmd string, a } cfgMap[name][key] = val } - return utils.SaveConfig(ctx.ConfigDir, ctx.Config) + return config.Save(ctx.ConfigDir, ctx.Config) case "show": var opt_json bool @@ -447,7 +447,7 @@ func dispatchSubcommand(ctx *appcontext.AppContext, cmd string, subcmd string, a } delete(cfgMap[name], key) } - return utils.SaveConfig(ctx.ConfigDir, ctx.Config) + return config.Save(ctx.ConfigDir, ctx.Config) default: return fmt.Errorf("usage: plakar %s [add|check|import|ping|rm|set|show|unset]", cmd) diff --git a/subcommands/config/config_test.go b/subcommands/config/config_test.go index 7f6746704..d7d93bee6 100644 --- a/subcommands/config/config_test.go +++ b/subcommands/config/config_test.go @@ -8,7 +8,7 @@ import ( "github.com/PlakarKorp/kloset/repository" "github.com/PlakarKorp/plakar/appcontext" - "github.com/PlakarKorp/plakar/utils" + "github.com/PlakarKorp/plakar/config" "github.com/stretchr/testify/require" ) @@ -37,7 +37,7 @@ func TestConfigEmpty(t *testing.T) { }) configPath := filepath.Join(tmpDir, "config.yaml") - cfg, err := utils.LoadOldConfigIfExists(configPath) + cfg, err := config.LoadOldConfigIfExists(configPath) require.NoError(t, err) ctx := appcontext.NewAppContext() @@ -105,7 +105,7 @@ func TestCmdRemote(t *testing.T) { }) configPath := filepath.Join(tmpDir, "config.yaml") - cfg, err := utils.LoadOldConfigIfExists(configPath) + cfg, err := config.LoadOldConfigIfExists(configPath) require.NoError(t, err) ctx := appcontext.NewAppContext() ctx.Config = cfg @@ -157,7 +157,7 @@ func TestCmdRepository(t *testing.T) { }) configPath := filepath.Join(tmpDir, "config.yaml") - cfg, err := utils.LoadOldConfigIfExists(configPath) + cfg, err := config.LoadOldConfigIfExists(configPath) require.NoError(t, err) ctx := appcontext.NewAppContext() ctx.Config = cfg diff --git a/subcommands/sync/sync_test.go b/subcommands/sync/sync_test.go index 5ca30acc0..f8f108ae4 100644 --- a/subcommands/sync/sync_test.go +++ b/subcommands/sync/sync_test.go @@ -13,8 +13,8 @@ import ( "github.com/PlakarKorp/kloset/repository" "github.com/PlakarKorp/kloset/snapshot" "github.com/PlakarKorp/plakar/appcontext" + "github.com/PlakarKorp/plakar/config" ptesting "github.com/PlakarKorp/plakar/testing" - "github.com/PlakarKorp/plakar/utils" "github.com/stretchr/testify/require" ) @@ -107,13 +107,13 @@ func _TestExecuteCmdSyncWithEncryption(t *testing.T) { // need to recreate configuration to store passphrase on peer repo opt_configfile := strings.TrimPrefix(peerRepo.Root(), "fs://") - cfg, err := utils.LoadConfig(opt_configfile) + cfg, err := config.Load(opt_configfile) require.NoError(t, err) lctx.Config = cfg lctx.Config.Repositories["peerRepo"] = make(map[string]string) lctx.Config.Repositories["peerRepo"]["passphrase"] = string(passphrase) lctx.Config.Repositories["peerRepo"]["location"] = peerRepo.Root() - err = utils.SaveConfig(opt_configfile, lctx.Config) + err = config.Save(opt_configfile, lctx.Config) require.NoError(t, err) indexId := snap.Header.GetIndexID() diff --git a/utils/config.go b/utils/config.go deleted file mode 100644 index 4dbe512b8..000000000 --- a/utils/config.go +++ /dev/null @@ -1,351 +0,0 @@ -package utils - -import ( - "bytes" - "encoding/json" - "fmt" - "io" - "os" - "path/filepath" - "slices" - - "gopkg.in/ini.v1" - - "github.com/PlakarKorp/plakar/config" - "go.yaml.in/yaml/v3" -) - -type configHandler struct { - Path string -} - -const CONFIG_VERSION = "v1.0.0" - -type storesConfig struct { - Version string `yaml:"version"` - Default string `yaml:"default,omitempty"` - Stores map[string]map[string]string `yaml:"stores"` -} - -type sourcesConfig struct { - Version string `yaml:"version"` - Sources map[string]map[string]string `yaml:"sources"` -} - -type destinationsConfig struct { - Version string `yaml:"version"` - Destinations map[string]map[string]string `yaml:"destinations"` -} - -func newConfigHandler(path string) *configHandler { - return &configHandler{ - Path: path, - } -} - -func (cl *configHandler) Load() (*config.Config, error) { - sources := sourcesConfig{} - destinations := destinationsConfig{} - stores := storesConfig{} - - err := cl.load("sources.yml", &sources) - if err != nil { - if os.IsNotExist(err) { - return cl.LoadFallback() - } - return nil, err - } - - err = cl.load("destinations.yml", &destinations) - if err != nil { - if os.IsNotExist(err) { - return cl.LoadFallback() - } - return nil, err - } - - err = cl.load("stores.yml", &stores) - if err != nil && os.IsNotExist(err) { - // try to load former file - err = cl.load("klosets.yml", &stores) - } - if err != nil { - if os.IsNotExist(err) { - return cl.LoadFallback() - } - return nil, err - } - - cfg := config.NewConfig() - cfg.Sources = sources.Sources - cfg.Destinations = destinations.Destinations - cfg.Repositories = stores.Stores - cfg.DefaultRepository = stores.Default - return cfg, nil -} - -func (cl *configHandler) LoadFallback() (*config.Config, error) { - // Load old config if found - oldpath := filepath.Join(cl.Path, "plakar.yml") - cfg, err := LoadOldConfigIfExists(oldpath) - if err != nil { - return nil, fmt.Errorf("error reading file %s: %w", oldpath, err) - } - - // Save the config in the new format right now - err = SaveConfig(cl.Path, cfg) - if err != nil { - return nil, fmt.Errorf("failed to update config file: %w", err) - } - // Do we want to remove the old file? - return cfg, nil -} - -func (cl *configHandler) Save(cfg *config.Config) error { - // Create the config directory if it doesn't exist - err := os.MkdirAll(cl.Path, 0700) - if err != nil { - return err - } - - err = cl.save("sources.yml", sourcesConfig{ - Version: CONFIG_VERSION, - Sources: cfg.Sources, - }) - if err != nil { - return err - } - err = cl.save("destinations.yml", destinationsConfig{ - Version: CONFIG_VERSION, - Destinations: cfg.Destinations, - }) - if err != nil { - return err - } - err = cl.save("stores.yml", storesConfig{ - Version: CONFIG_VERSION, - Default: cfg.DefaultRepository, - Stores: cfg.Repositories, - }) - if err != nil { - return err - } - return nil -} - -func (cl *configHandler) load(filename string, dst any) error { - path := filepath.Join(cl.Path, filename) - f, err := os.Open(path) - if err != nil { - if os.IsNotExist(err) { - return err - } - return fmt.Errorf("failed to open file %s: %w", path, err) - } - defer f.Close() - - info, err := f.Stat() - if err != nil { - return fmt.Errorf("failed to stat file %s: %w", path, err) - } - if info.Size() == 0 { - return nil - } - - // try to load the new format - err = yaml.NewDecoder(f).Decode(dst) - var version string - switch t := dst.(type) { - case *storesConfig: - version = t.Version - case *destinationsConfig: - version = t.Version - case *sourcesConfig: - version = t.Version - default: - return fmt.Errorf("invalid configuration type %v", t) - } - if err == nil && version == CONFIG_VERSION { - return nil - } - - // fallback to the previous format - _, err = f.Seek(0, io.SeekStart) - if err != nil { - return fmt.Errorf("failed to rewind config file: %w", err) - } - - switch t := dst.(type) { - case *storesConfig: - err = yaml.NewDecoder(f).Decode(&t.Stores) - if err == nil { - for k, v := range t.Stores { - if _, ok := v[".isDefault"]; ok { - if t.Default != "" { - return fmt.Errorf("multiple default store") - } - t.Default = k - delete(v, ".isDefault") - } - } - } - case *destinationsConfig: - err = yaml.NewDecoder(f).Decode(&t.Destinations) - case *sourcesConfig: - err = yaml.NewDecoder(f).Decode(&t.Sources) - } - if err != nil { - return fmt.Errorf("failed to parse config file: %w", err) - } - - return nil -} - -func (cl *configHandler) save(filename string, src any) error { - path := filepath.Join(cl.Path, filename) - tmpFile, err := os.CreateTemp(cl.Path, "config.*.yml") - if err != nil { - return err - } - - err = yaml.NewEncoder(tmpFile).Encode(src) - tmpFile.Close() - - if err == nil { - err = os.Rename(tmpFile.Name(), path) - } - - if err != nil { - os.Remove(tmpFile.Name()) - return err - } - - return nil -} - -func LoadConfig(configDir string) (*config.Config, error) { - cl := newConfigHandler(configDir) - cfg, err := cl.Load() - if err != nil { - return nil, err - } - return cfg, nil -} - -func SaveConfig(configDir string, cfg *config.Config) error { - return newConfigHandler(configDir).Save(cfg) -} - -// toString converts various primitive types to string. -func toString(v interface{}) string { - switch t := v.(type) { - case string: - return t - case int, int64, float64, bool: - return fmt.Sprintf("%v", t) - default: - return "" - } -} - -func LoadINI(rd io.Reader) (map[string]map[string]string, error) { - cfg, err := ini.Load(rd) - if err != nil { - return nil, err - } - - keysMap := make(map[string]struct{}) - result := make(map[string]map[string]string) - for _, section := range cfg.Sections() { - name := section.Name() - if name == ini.DefaultSection { - continue - } - keysMap[name] = struct{}{} - result[name] = make(map[string]string) - for _, key := range section.Keys() { - result[name][key.Name()] = key.Value() - } - } - return result, nil -} - -func LoadYAML(rd io.Reader) (map[string]map[string]string, error) { - var raw map[string]interface{} - decoder := yaml.NewDecoder(rd) - if err := decoder.Decode(&raw); err != nil { - return nil, err - } - - result := make(map[string]map[string]string) - for section, value := range raw { - sectionMap, ok := value.(map[string]interface{}) - if !ok { - continue // skip non-object top-level keys - } - result[section] = make(map[string]string) - for k, v := range sectionMap { - result[section][k] = toString(v) - } - } - - return result, nil -} - -// LoadJSON loads a JSON object and returns a nested map[string]map[string]string. -func LoadJSON(rd io.Reader) (map[string]map[string]string, error) { - var raw map[string]map[string]string - decoder := json.NewDecoder(rd) - if err := decoder.Decode(&raw); err != nil { - return nil, err - } - return raw, nil -} - -func GetConf(rd io.Reader, thirdParty string) (map[string]map[string]string, error) { - data, err := io.ReadAll(rd) - if err != nil { - return nil, fmt.Errorf("failed to read config data: %w", err) - } - - var configMap map[string]map[string]string - if configMap, err = LoadYAML(bytes.NewReader(data)); err == nil { - } else if configMap, err = LoadJSON(bytes.NewReader(data)); err == nil { - } else if configMap, err = LoadINI(bytes.NewReader(data)); err != nil { - return nil, fmt.Errorf("failed to parse config data: %w", err) - } - - if thirdParty != "" { - for _, section := range configMap { - var ignore []string - for key, value := range section { - if slices.Contains(ignore, key) { - continue - } - if value != "" { - newKey := thirdParty + "_" + key - section[newKey] = value - ignore = append(ignore, newKey) - } - delete(section, key) - } - section["location"] = thirdParty + "://" - } - } - - seenLocation := false - for _, section := range configMap { - for key, value := range section { - if key == "location" { - seenLocation = true - } - if value == "" { - delete(section, key) - } - } - } - if !seenLocation { - return nil, fmt.Errorf("missing 'location' key in config data, `-rclone` import option missing ?") - } - return configMap, nil -} From f7f78827f07c381f280689a20e60dcf059f23ac8 Mon Sep 17 00:00:00 2001 From: Omar Polo Date: Tue, 9 Jun 2026 11:47:39 +0000 Subject: [PATCH 2/3] config: inline toString() (more or less) for string, numbers and boolean it's the same behaviour; now it could stringify other types that before were yielding only "", but shouldn't matter much here. Also, no need to special case strings since it's not a performance-sensitive part, and the gained readability matters. --- config/load_test.go | 25 ++----------------------- config/loadfile.go | 13 +------------ 2 files changed, 3 insertions(+), 35 deletions(-) diff --git a/config/load_test.go b/config/load_test.go index 0a8b43c6a..465bec649 100644 --- a/config/load_test.go +++ b/config/load_test.go @@ -197,10 +197,10 @@ remote: t.Fatalf("location = %q", got["remote"]["location"]) } if got["remote"]["port"] != "22" { - t.Fatalf("port = %q (toString should convert int)", got["remote"]["port"]) + t.Fatalf("port = %q (should be converted from int)", got["remote"]["port"]) } if got["remote"]["ssl"] != "true" { - t.Fatalf("ssl = %q (toString should convert bool)", got["remote"]["ssl"]) + t.Fatalf("ssl = %q (should be converted from bool)", got["remote"]["ssl"]) } } @@ -242,27 +242,6 @@ func TestLoadJSONBad(t *testing.T) { } } -func TestToString(t *testing.T) { - cases := []struct { - in any - want string - }{ - {"hello", "hello"}, - {42, "42"}, - {int64(7), "7"}, - {3.14, "3.14"}, - {true, "true"}, - {false, "false"}, - {nil, ""}, - {[]int{1, 2}, ""}, - } - for _, c := range cases { - if got := toString(c.in); got != c.want { - t.Errorf("toString(%v) = %q, want %q", c.in, got, c.want) - } - } -} - func TestLoadFileYAMLWithLocation(t *testing.T) { rd := strings.NewReader(` remote: diff --git a/config/loadfile.go b/config/loadfile.go index 594e9573e..502335eca 100644 --- a/config/loadfile.go +++ b/config/loadfile.go @@ -12,17 +12,6 @@ import ( "go.yaml.in/yaml/v3" ) -func toString(v any) string { - switch t := v.(type) { - case string: - return t - case int, int64, float64, bool: - return fmt.Sprintf("%v", t) - default: - return "" - } -} - func loadINI(rd io.Reader) (map[string]map[string]string, error) { cfg, err := ini.Load(rd) if err != nil { @@ -60,7 +49,7 @@ func loadYAML(rd io.Reader) (map[string]map[string]string, error) { } result[section] = make(map[string]string) for k, v := range sectionMap { - result[section][k] = toString(v) + result[section][k] = fmt.Sprint(v) } } From 3f1adffd4e5cb6b13dca6abd2ee80801d871e04a Mon Sep 17 00:00:00 2001 From: Omar Polo Date: Tue, 9 Jun 2026 12:58:54 +0000 Subject: [PATCH 3/3] introduce a helper for yaml.NewEncoder() to always close it unlike json.NewEncoder(), yaml Encoder needs to be Close()d to properly flush. Introduce an helper and use it across the codebase to avoid forgetting to do so. --- config/load.go | 3 ++- subcommands/config/config.go | 4 ++-- subcommands/service/show.go | 4 ++-- utils/config_policy.go | 4 ++-- utils/marshal.go | 16 ++++++++++++++++ 5 files changed, 24 insertions(+), 7 deletions(-) create mode 100644 utils/marshal.go diff --git a/config/load.go b/config/load.go index dec944cce..86bb9646b 100644 --- a/config/load.go +++ b/config/load.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" + "github.com/PlakarKorp/plakar/utils" "go.yaml.in/yaml/v3" ) @@ -160,7 +161,7 @@ func save(file string, src any) error { return err } - err = yaml.NewEncoder(tmpFile).Encode(src) + err = utils.YAMLEncode(tmpFile, src) tmpFile.Close() if err == nil { diff --git a/subcommands/config/config.go b/subcommands/config/config.go index 3301d85b5..8387eac20 100644 --- a/subcommands/config/config.go +++ b/subcommands/config/config.go @@ -32,7 +32,7 @@ import ( "github.com/PlakarKorp/plakar/appcontext" "github.com/PlakarKorp/plakar/config" "github.com/PlakarKorp/plakar/subcommands" - "go.yaml.in/yaml/v3" + "github.com/PlakarKorp/plakar/utils" "gopkg.in/ini.v1" ) @@ -415,7 +415,7 @@ func dispatchSubcommand(ctx *appcontext.AppContext, cmd string, subcmd string, a } else if opt_ini { err = MarshalINISections(name, cfgMap[name], ctx.Stdout) } else { - err = yaml.NewEncoder(ctx.Stdout).Encode(map[string]map[string]string{name: cfgMap[name]}) + err = utils.YAMLEncode(ctx.Stdout, map[string]map[string]string{name: cfgMap[name]}) } if err != nil { return fmt.Errorf("failed to encode store %q: %w", name, err) diff --git a/subcommands/service/show.go b/subcommands/service/show.go index a2871029e..6bbf1c8d8 100644 --- a/subcommands/service/show.go +++ b/subcommands/service/show.go @@ -8,7 +8,7 @@ import ( "github.com/PlakarKorp/kloset/repository" "github.com/PlakarKorp/plakar/appcontext" "github.com/PlakarKorp/plakar/subcommands" - "go.yaml.in/yaml/v3" + "github.com/PlakarKorp/plakar/utils" ) type ServiceShow struct { @@ -56,7 +56,7 @@ func (cmd *ServiceShow) Execute(ctx *appcontext.AppContext, repo *repository.Rep if cmd.AsJson { err = json.NewEncoder(ctx.Stdout).Encode(map[string]any{cmd.Service: config}) } else { - err = yaml.NewEncoder(ctx.Stdout).Encode(map[string]any{cmd.Service: config}) + err = utils.YAMLEncode(ctx.Stdout, map[string]any{cmd.Service: config}) } if err != nil { return 1, fmt.Errorf("failed to encode config: %w", err) diff --git a/utils/config_policy.go b/utils/config_policy.go index 000738fc5..0fd4f6b93 100644 --- a/utils/config_policy.go +++ b/utils/config_policy.go @@ -204,7 +204,7 @@ func (c *policiesConfig) SaveToFile(filename string) error { if err != nil { return err } - err = yaml.NewEncoder(tmpFile).Encode(c) + err = YAMLEncode(tmpFile, c) tmpFile.Close() if err == nil { err = os.Rename(tmpFile.Name(), filename) @@ -234,7 +234,7 @@ func (c *policiesConfig) Dump(w io.Writer, format string, names []string) error case "json": err = json.NewEncoder(w).Encode(map[string]*locate.LocateOptions{name: c.Policies[name]}) case "yaml": - err = yaml.NewEncoder(w).Encode(map[string]*locate.LocateOptions{name: c.Policies[name]}) + err = YAMLEncode(w, map[string]*locate.LocateOptions{name: c.Policies[name]}) default: return fmt.Errorf("unknown format %q", format) } diff --git a/utils/marshal.go b/utils/marshal.go new file mode 100644 index 000000000..6cc2433e2 --- /dev/null +++ b/utils/marshal.go @@ -0,0 +1,16 @@ +package utils + +import ( + "io" + + "go.yaml.in/yaml/v3" +) + +func YAMLEncode(w io.Writer, x any) error { + enc := yaml.NewEncoder(w) + if err := enc.Encode(x); err != nil { + enc.Close() + return err + } + return enc.Close() +}