config: unify Get{Source,Store,Destination} behaviour

* let all of them to resolve internally the "at aliases" syntax
* all of them return a map or an error, not a boolean
* they verify internally that location is set

The visible changes are:
* the message for an alias not found or a location missing is
  slightly different
* passphrase_cmd gets replaced everywhere: I've checked all the
  currently packaged importers and exporters, and no-one is using it,
  I'm fine with this trade-off for now, in the near future it'll get
  replaced by a better mechanism anyway.

Author: omar-polo

config/config.go

@@ -1,31 +1,31 @@
 package config
 
 import (
+	"errors"
 	"fmt"
 	"net/url"
 	"path"
 	"path/filepath"
 	"strings"
+)
 
-	"maps"
+var (
+	ErrNoLocation = errors.New("location is missing")
+	ErrNotFound   = errors.New("configuration not found")
 )
 
 type Config struct {
 	DefaultRepository string
-	Repositories      map[string]RepositoryConfig
-	Sources           map[string]SourceConfig
-	Destinations      map[string]DestinationConfig
+	Repositories      map[string]map[string]string
+	Sources           map[string]map[string]string
+	Destinations      map[string]map[string]string
 }
 
-type RepositoryConfig = map[string]string
-type SourceConfig = map[string]string
-type DestinationConfig = map[string]string
-
 func NewConfig() *Config {
 	return &Config{
-		Repositories: make(map[string]RepositoryConfig),
-		Sources:      make(map[string]SourceConfig),
-		Destinations: make(map[string]DestinationConfig),
+		Repositories: make(map[string]map[string]string),
+		Sources:      make(map[string]map[string]string),
+		Destinations: make(map[string]map[string]string),
 	}
 }
 
@@ -34,76 +34,45 @@ func (c *Config) HasRepository(name string) bool {
 	return ok
 }
 
-func (c *Config) GetRepository(name string) (map[string]string, error) {
+func (c *Config) get(name, kind string, configs map[string]map[string]string) (map[string]string, error) {
 	if !strings.HasPrefix(name, "@") {
 		return map[string]string{"location": name}, nil
 	}
 
-	name, rootOverride := resolveRootOverride(name)
+	name, rootOverride := resolveRootOverride(name[1:])
 
-	kv, ok := c.Repositories[name[1:]]
+	kv, ok := configs[name]
 	if !ok {
-		return nil, fmt.Errorf("could not resolve repository: %s", name)
+		return nil, fmt.Errorf("%s %w: %q", kind, ErrNotFound, name)
 	}
-	if _, ok := kv["location"]; !ok {
-		return nil, fmt.Errorf("repository %s has no location", name)
-	} else {
-		res := make(map[string]string)
-		maps.Copy(res, kv)
 
-		location, err := applyRootOverride(res["location"], rootOverride)
-		if err != nil {
-			return nil, err
-		}
-		res["location"] = location
-		return res, nil
+	if _, ok := kv["location"]; !ok {
+		return nil, fmt.Errorf("%s %w", kind, ErrNoLocation)
 	}
+
+	return resolve(kv, rootOverride)
+}
+
+func (c *Config) GetRepository(name string) (map[string]string, error) {
+	return c.get(name, "repository", c.Repositories)
 }
 
 func (c *Config) HasSource(name string) bool {
 	_, ok := c.Sources[name]
 	return ok
 }
 
-func (c *Config) GetSource(name string) (map[string]string, bool) {
-	name, rootOverride := resolveRootOverride(name)
-
-	if kv, ok := c.Sources[name]; !ok {
-		return nil, false
-	} else {
-		res := make(map[string]string)
-		maps.Copy(res, kv)
-
-		location, err := applyRootOverride(res["location"], rootOverride)
-		if err != nil {
-			return nil, false
-		}
-		res["location"] = location
-		return res, ok
-	}
+func (c *Config) GetSource(name string) (map[string]string, error) {
+	return c.get(name, "source", c.Sources)
 }
 
 func (c *Config) HasDestination(name string) bool {
 	_, ok := c.Destinations[name]
 	return ok
 }
 
-func (c *Config) GetDestination(name string) (map[string]string, bool) {
-	name, rootOverride := resolveRootOverride(name)
-
-	if kv, ok := c.Destinations[name]; !ok {
-		return nil, false
-	} else {
-		res := make(map[string]string)
-		maps.Copy(res, kv)
-
-		location, err := applyRootOverride(res["location"], rootOverride)
-		if err != nil {
-			return nil, false
-		}
-		res["location"] = location
-		return res, ok
-	}
+func (c *Config) GetDestination(name string) (map[string]string, error) {
+	return c.get(name, "destination", c.Destinations)
 }
 
 func resolveRootOverride(name string) (string, string) {

config/config_extra_test.go

@@ -21,22 +21,23 @@ func TestHasDestination(t *testing.T) {
 	c := NewConfig()
 	require.False(t, c.HasDestination("missing"))
 
-	c.Destinations["dst"] = DestinationConfig{"location": "/tmp/out"}
+	c.Destinations["dst"] = map[string]string{"location": "/tmp/out"}
 	require.True(t, c.HasDestination("dst"))
 }
 
 func TestGetDestination(t *testing.T) {
 	c := NewConfig()
 
 	// missing
-	got, ok := c.GetDestination("missing")
-	require.False(t, ok)
+	got, err := c.GetDestination("@missing")
+	require.Error(t, err)
+	require.ErrorIs(t, err, ErrNotFound)
 	require.Nil(t, got)
 
 	// present
-	c.Destinations["dst"] = DestinationConfig{"location": "/tmp/out", "extra": "yes"}
-	got, ok = c.GetDestination("dst")
-	require.True(t, ok)
+	c.Destinations["dst"] = map[string]string{"location": "/tmp/out", "extra": "yes"}
+	got, err = c.GetDestination("@dst")
+	require.NoError(t, err)
 	require.Equal(t, "/tmp/out", got["location"])
 	require.Equal(t, "yes", got["extra"])
 
@@ -54,9 +55,9 @@ func TestResolveRootOverride(t *testing.T) {
 		{"plain", "plain", ""},
 		{"name:/abs/path", "name", "/abs/path"},
 		{"name:rel/path", "name", "rel/path"},
-		{"name:", "name", ""},   // trailing colon -> empty override
-		{":/abs", "", "/abs"},   // leading colon -> empty name
-		{"a:b:c", "a", "b:c"},   // only the first colon splits
+		{"name:", "name", ""}, // trailing colon -> empty override
+		{":/abs", "", "/abs"}, // leading colon -> empty name
+		{"a:b:c", "a", "b:c"}, // only the first colon splits
 	}
 	for _, c := range cases {
 		t.Run(c.input, func(t *testing.T) {
@@ -110,7 +111,7 @@ func TestApplyRootOverride_URL_BadLocationReturnsError(t *testing.T) {
 
 func TestGetRepository_RootOverrideAppliedToLocalLocation(t *testing.T) {
 	c := NewConfig()
-	c.Repositories["repo"] = RepositoryConfig{"location": "/var/backups"}
+	c.Repositories["repo"] = map[string]string{"location": "/var/backups"}
 
 	got, err := c.GetRepository("@repo:/elsewhere")
 	require.NoError(t, err)
@@ -123,7 +124,7 @@ func TestGetRepository_RootOverrideAppliedToLocalLocation(t *testing.T) {
 
 func TestGetRepository_RootOverrideAppliedToURL(t *testing.T) {
 	c := NewConfig()
-	c.Repositories["repo"] = RepositoryConfig{"location": "s3://bucket/base"}
+	c.Repositories["repo"] = map[string]string{"location": "s3://bucket/base"}
 
 	got, err := c.GetRepository("@repo:sub")
 	require.NoError(t, err)
@@ -132,7 +133,7 @@ func TestGetRepository_RootOverrideAppliedToURL(t *testing.T) {
 
 func TestGetRepository_RootOverridePropagatesURLParseError(t *testing.T) {
 	c := NewConfig()
-	c.Repositories["repo"] = RepositoryConfig{"location": "ht\x00tp://x"}
+	c.Repositories["repo"] = map[string]string{"location": "ht\x00tp://x"}
 
 	_, err := c.GetRepository("@repo:/x")
 	require.Error(t, err)
@@ -149,44 +150,44 @@ func TestGetRepository_DirectPathPassesThrough(t *testing.T) {
 
 func TestGetSource_RootOverrideAppliedToLocalLocation(t *testing.T) {
 	c := NewConfig()
-	c.Sources["src"] = SourceConfig{"location": "/data"}
+	c.Sources["src"] = map[string]string{"location": "/data"}
 
-	got, ok := c.GetSource("src:/elsewhere")
-	require.True(t, ok)
+	got, err := c.GetSource("@src:/elsewhere")
+	require.NoError(t, err)
 	require.Equal(t, "/elsewhere", got["location"])
 }
 
 func TestGetSource_RootOverrideReturnsFalseOnURLParseError(t *testing.T) {
 	c := NewConfig()
 	// GetSource swallows the applyRootOverride error and returns ok=false.
-	c.Sources["src"] = SourceConfig{"location": "ht\x00tp://x"}
+	c.Sources["src"] = map[string]string{"location": "ht\x00tp://x"}
 
-	_, ok := c.GetSource("src:/x")
-	require.False(t, ok)
+	_, err := c.GetSource("@src:/x")
+	require.Error(t, err)
 }
 
 func TestGetDestination_RootOverrideAppliedToLocalLocation(t *testing.T) {
 	c := NewConfig()
-	c.Destinations["dst"] = DestinationConfig{"location": "/out"}
+	c.Destinations["dst"] = map[string]string{"location": "/out"}
 
-	got, ok := c.GetDestination("dst:sub")
-	require.True(t, ok)
+	got, err := c.GetDestination("@dst:sub")
+	require.NoError(t, err)
 	require.Equal(t, "/out/sub", got["location"])
 }
 
 func TestGetDestination_RootOverrideReturnsFalseOnURLParseError(t *testing.T) {
 	c := NewConfig()
-	c.Destinations["dst"] = DestinationConfig{"location": "ht\x00tp://x"}
+	c.Destinations["dst"] = map[string]string{"location": "ht\x00tp://x"}
 
-	_, ok := c.GetDestination("dst:/x")
-	require.False(t, ok)
+	_, err := c.GetDestination("@dst:/x")
+	require.Error(t, err)
 }
 
 func TestGetRepository_ResolveRootOverrideStripsAtPrefix(t *testing.T) {
 	// Sanity: the "@" prefix is on the *full* token, then the colon split
 	// happens on what's left. So "@name:override" -> name lookup is "name".
 	c := NewConfig()
-	c.Repositories["repo"] = RepositoryConfig{"location": "/x"}
+	c.Repositories["repo"] = map[string]string{"location": "/x"}
 
 	got, err := c.GetRepository("@repo:sub")
 	require.NoError(t, err)

config/config_test.go

@@ -8,20 +8,20 @@ import (
 
 func TestHasRepository(t *testing.T) {
 	cfg := &Config{
-		Repositories: make(map[string]RepositoryConfig),
+		Repositories: make(map[string]map[string]string),
 	}
 
 	// Test non-existent repository
 	require.False(t, cfg.HasRepository("test-repo"))
 
 	// Test existing repository
-	cfg.Repositories["test-repo"] = RepositoryConfig{"location": "/test/path"}
+	cfg.Repositories["test-repo"] = map[string]string{"location": "/test/path"}
 	require.True(t, cfg.HasRepository("test-repo"))
 }
 
 func TestGetRepository(t *testing.T) {
 	cfg := &Config{
-		Repositories: make(map[string]RepositoryConfig),
+		Repositories: make(map[string]map[string]string),
 	}
 
 	// Test direct path
@@ -34,43 +34,62 @@ func TestGetRepository(t *testing.T) {
 	require.Error(t, err)
 
 	// Test repository without location
-	cfg.Repositories["test-repo"] = RepositoryConfig{"other": "value"}
+	cfg.Repositories["test-repo"] = map[string]string{"other": "value"}
 	_, err = cfg.GetRepository("@test-repo")
 	require.Error(t, err)
 
 	// Test valid repository
-	cfg.Repositories["test-repo"] = RepositoryConfig{"location": "/test/path"}
+	cfg.Repositories["test-repo"] = map[string]string{"location": "/test/path"}
 	repo, err = cfg.GetRepository("@test-repo")
 	require.NoError(t, err)
 	require.Equal(t, "/test/path", repo["location"])
 }
 
 func TestHasSource(t *testing.T) {
 	cfg := &Config{
-		Sources: make(map[string]SourceConfig),
+		Sources: make(map[string]map[string]string),
 	}
 
 	// Test non-existent source
 	require.False(t, cfg.HasSource("test-source"))
 
 	// Test existing source
-	cfg.Sources["test-source"] = SourceConfig{"url": "test://url"}
+	cfg.Sources["test-source"] = map[string]string{"url": "test://url"}
 	require.True(t, cfg.HasSource("test-source"))
 }
 
 func TestGetSource(t *testing.T) {
 	cfg := &Config{
-		Sources: make(map[string]SourceConfig),
+		Sources: make(map[string]map[string]string),
 	}
 
 	// Test non-existent source
-	source, ok := cfg.GetSource("test-source")
-	require.False(t, ok)
+	source, err := cfg.GetSource("@test-source")
+	require.Error(t, err)
+	require.ErrorIs(t, err, ErrNotFound)
 	require.Nil(t, source)
 
 	// Test existing source
-	cfg.Sources["test-source"] = SourceConfig{"url": "test://url"}
-	source, ok = cfg.GetSource("test-source")
-	require.True(t, ok)
+	cfg.Sources["test-source"] = map[string]string{
+		"url":      "test://url",
+		"location": "xxx",
+	}
+	source, err = cfg.GetSource("@test-source")
+	require.NoError(t, err)
 	require.Equal(t, "test://url", source["url"])
 }
+
+func TestGetNoLocation(t *testing.T) {
+	cfg := Config{
+		Sources: map[string]map[string]string{
+			"foo": {
+				"option_a": "true",
+			},
+		},
+	}
+
+	opts, err := cfg.get("@foo", "source", cfg.Sources)
+	require.Error(t, err)
+	require.ErrorIs(t, err, ErrNoLocation)
+	require.Nil(t, opts)
+}

config/old.go

@@ -9,9 +9,9 @@ import (
 )
 
 type OldConfig struct {
-	DefaultRepository string                      `yaml:"default-repo"`
-	Repositories      map[string]RepositoryConfig `yaml:"repositories"`
-	Remotes           map[string]SourceConfig     `yaml:"remotes"`
+	DefaultRepository string                       `yaml:"default-repo"`
+	Repositories      map[string]map[string]string `yaml:"repositories"`
+	Remotes           map[string]map[string]string `yaml:"remotes"`
 }
 
 func LoadOldConfigIfExists(configFile string) (*Config, error) {
@@ -34,7 +34,7 @@ func LoadOldConfigIfExists(configFile string) (*Config, error) {
 	cfg.DefaultRepository = old.DefaultRepository
 	cfg.Repositories = old.Repositories
 	cfg.Sources = old.Remotes
-	cfg.Destinations = make(map[string]DestinationConfig)
+	cfg.Destinations = make(map[string]map[string]string)
 	for key, val := range cfg.Sources {
 		res := make(map[string]string)
 		maps.Copy(res, val)