diff --git a/cmd/iceberg/args_test.go b/cmd/iceberg/args_test.go index 8de1c7bc1..9d0bd2ac4 100644 --- a/cmd/iceberg/args_test.go +++ b/cmd/iceberg/args_test.go @@ -288,6 +288,27 @@ func TestArgsParsing(t *testing.T) { } } +func TestResolveCatalogName(t *testing.T) { + tests := []struct { + name string + explicitFlags map[string]bool + flagValue string + want string + }{ + // explicit --catalog-name wins and is passed through + {"explicit flag wins", map[string]bool{"catalog-name": true}, "prod", "prod"}, + // no flag: return "" so ParseConfig falls back to default-catalog / "default" + {"no flag returns empty string", map[string]bool{}, "default", ""}, + // other flags set but not catalog-name: still falls back + {"other flags set only", map[string]bool{"uri": true}, "default", ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, resolveCatalogName(tt.explicitFlags, tt.flagValue)) + }) + } +} + func TestMergeConfAwsProfile(t *testing.T) { fileCfg := &config.CatalogConfig{AwsProfile: "file-profile"} diff --git a/cmd/iceberg/main.go b/cmd/iceberg/main.go index 98b682916..31425caed 100644 --- a/cmd/iceberg/main.go +++ b/cmd/iceberg/main.go @@ -237,9 +237,14 @@ func main() { } } - fileCfg := config.ParseConfig(config.LoadConfig(args.Config), args.CatalogName) - if fileCfg != nil { + configData := config.LoadConfig(args.Config) + fileCfg, err := config.ParseConfig(configData, resolveCatalogName(explicitFlags, args.CatalogName)) + if err != nil { + log.Printf("warning: failed to parse config file: %v", err) + } else if fileCfg != nil { mergeConf(fileCfg, &args, explicitFlags) + } else if len(configData) > 0 { + log.Printf("warning: catalog %q not found in config file", args.CatalogName) } // Validate nested subcommands before catalog init. @@ -791,6 +796,17 @@ func loadTable(ctx context.Context, output Output, cat catalog.Catalog, id strin return tbl } +// resolveCatalogName returns the catalog name to pass to ParseConfig. +// When --catalog-name was given explicitly it wins; otherwise "" is returned +// so ParseConfig can fall back through default-catalog -> "default". +func resolveCatalogName(explicitFlags map[string]bool, flagValue string) string { + if explicitFlags["catalog-name"] { + return flagValue + } + + return "" +} + // mergeConf applies values from the file config into args for any option // that was not explicitly provided on the command line. explicitFlags is a set // of flag names (without the "--" prefix) that appeared in os.Args so that diff --git a/config/config.go b/config/config.go index 0eef270fc..0491c76a6 100644 --- a/config/config.go +++ b/config/config.go @@ -70,18 +70,33 @@ func LoadConfig(configPath string) []byte { return file } -func ParseConfig(file []byte, catalogName string) *CatalogConfig { +// ParseConfig unmarshals the config file once and resolves the catalog to use. +// When catalogName is empty, it falls back to the file's default-catalog field and +// then to the built-in "default" name. It returns the matching CatalogConfig, or +// nil/nil if no such catalog is defined in the file. A non-nil error means the +// file could not be parsed and should be surfaced to the user. +func ParseConfig(file []byte, catalogName string) (*CatalogConfig, error) { + if len(file) == 0 { + return nil, nil + } var config Config - err := yaml.Unmarshal(file, &config) - if err != nil { - return nil + if err := yaml.Unmarshal(file, &config); err != nil { + return nil, err + } + + if catalogName == "" { + catalogName = config.DefaultCatalog + } + if catalogName == "" { + catalogName = "default" } + res, ok := config.Catalogs[catalogName] if !ok { - return nil + return nil, nil } - return &res + return &res, nil } func fromConfigFiles() Config { diff --git a/config/config_test.go b/config/config_test.go index cddad1264..ac496f38c 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -21,6 +21,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) var testArgs = []struct { @@ -28,9 +29,9 @@ var testArgs = []struct { catName string expected *CatalogConfig }{ - // config file does not exist - {nil, "default", nil}, - // config does not have default catalog + // config file does not exist; empty name falls back to built-in "default" + {nil, "", nil}, + // config does not have requested catalog {[]byte(` catalog: custom-catalog: @@ -115,12 +116,50 @@ catalog: AwsProfile: "my-aws-profile", }, }, + // empty name resolves via the file's default-catalog field + { + []byte(` +default-catalog: custom-catalog +catalog: + custom-catalog: + type: rest + uri: http://localhost:8181/ + warehouse: my_wh +`), "", + &CatalogConfig{ + CatalogType: "rest", + URI: "http://localhost:8181/", + Warehouse: "my_wh", + }, + }, + // explicit name takes precedence over the file's default-catalog + { + []byte(` +default-catalog: custom-catalog +catalog: + custom-catalog: + type: rest + uri: http://localhost:8181/ + other: + type: rest + uri: http://other:8181/ +`), "other", + &CatalogConfig{ + CatalogType: "rest", + URI: "http://other:8181/", + }, + }, } func TestParseConfig(t *testing.T) { for _, tt := range testArgs { - actual := ParseConfig([]byte(tt.file), tt.catName) - + actual, err := ParseConfig(tt.file, tt.catName) + require.NoError(t, err) assert.Equal(t, tt.expected, actual) } } + +func TestParseConfigMalformed(t *testing.T) { + _, err := ParseConfig([]byte(":\t[broken yaml"), "default") + assert.Error(t, err) +}