Modularize config factory (#8505)

* Modularize config factory

* CR Fixes

* CR Fixes 2

* Fix config validations

* Fix config validations

* Remove work file

* Revert changes

* CR Fixes 2

cmd/lakefs-loadtest/cmd/entry.go

@@ -13,8 +13,8 @@ import (
 	nanoid "github.com/matoous/go-nanoid/v2"
 	"github.com/schollz/progressbar/v3"
 	"github.com/spf13/cobra"
+	configfactory "github.com/treeverse/lakefs/modules/config/factory"
 	"github.com/treeverse/lakefs/pkg/catalog"
-	"github.com/treeverse/lakefs/pkg/config"
 	"github.com/treeverse/lakefs/pkg/kv"
 	"github.com/treeverse/lakefs/pkg/kv/kvparams"
 	"github.com/treeverse/lakefs/pkg/logging"
@@ -50,10 +50,12 @@ var entryCmd = &cobra.Command{
 
 		ctx := cmd.Context()
 
-		conf, err := config.NewConfig("")
+		confInterface, err := configfactory.BuildConfig("")
 		if err != nil {
 			fmt.Printf("config: %s\n", err)
 		}
+		conf := confInterface.GetBaseConfig()
+
 		err = conf.Validate()
 		if err != nil {
 			fmt.Printf("invalid config: %s\n", err)

cmd/lakefs/cmd/flare.go

@@ -39,7 +39,7 @@ var flareCmd = &cobra.Command{
 	Run: func(cmd *cobra.Command, args []string) {
 		flare.SetBaselinePermissions(flare.FlareUmask)
 		now := strings.ReplaceAll(time.Now().String(), " ", "")
-		cfg := loadConfig()
+		cfg := loadConfig().GetBaseConfig()
 		envVarBlacklist := addAppEnvVarPrefix(config.GetSecureStringKeyPaths(cfg))
 		flr, err := flare.NewFlare(flare.WithEnvVarBlacklist(envVarBlacklist))
 		if err != nil {

cmd/lakefs/cmd/kv.go

@@ -35,7 +35,7 @@ var kvGetCmd = &cobra.Command{
 	Short: "Return the value for the given path under the given partition",
 	Args:  cobra.ExactArgs(GetCmdNumArgs),
 	RunE: func(cmd *cobra.Command, args []string) error {
-		cfg := loadConfig()
+		cfg := loadConfig().GetBaseConfig()
 
 		pretty, err := cmd.Flags().GetBool("pretty")
 		if err != nil {
@@ -83,7 +83,7 @@ var kvScanCmd = &cobra.Command{
 	Short: "Scan through keys and values under the given partition. An optional path can be specified as a starting point (inclusive)",
 	Args:  cobra.RangeArgs(ScanCmdMinArgs, ScanCmdMaxArgs),
 	RunE: func(cmd *cobra.Command, args []string) error {
-		cfg := loadConfig()
+		cfg := loadConfig().GetBaseConfig()
 
 		limit, err := cmd.Flags().GetInt("limit")
 		if err != nil {

cmd/lakefs/cmd/migrate.go

@@ -22,7 +22,7 @@ var versionCmd = &cobra.Command{
 	Use:   "version",
 	Short: "Print current migration version and available version",
 	Run: func(cmd *cobra.Command, args []string) {
-		cfg := loadConfig()
+		cfg := loadConfig().GetBaseConfig()
 		kvParams, err := kvparams.NewConfig(&cfg.Database)
 		if err != nil {
 			_, _ = fmt.Fprintf(os.Stderr, "KV params: %s\n", err)
@@ -63,7 +63,7 @@ var upCmd = &cobra.Command{
 	Use:   "up",
 	Short: "Apply all up migrations",
 	Run: func(cmd *cobra.Command, args []string) {
-		cfg := loadConfig()
+		cfg := loadConfig().GetBaseConfig()
 		kvParams, err := kvparams.NewConfig(&cfg.Database)
 		if err != nil {
 			_, _ = fmt.Fprintf(os.Stderr, "KV params: %s\n", err)
@@ -100,7 +100,7 @@ var upCmd = &cobra.Command{
 	},
 }
 
-func DoMigration(ctx context.Context, kvStore kv.Store, _ *config.Config, _ bool) error {
+func DoMigration(ctx context.Context, kvStore kv.Store, _ *config.BaseConfig, _ bool) error {
 	var (
 		version int
 		err     error
@@ -136,7 +136,7 @@ var gotoCmd = &cobra.Command{
 	Use:   "goto",
 	Short: "Migrate to version V.",
 	Run: func(cmd *cobra.Command, args []string) {
-		cfg := loadConfig()
+		cfg := loadConfig().GetBaseConfig()
 		kvParams, err := kvparams.NewConfig(&cfg.Database)
 		if err != nil {
 			_, _ = fmt.Fprintf(os.Stderr, "KV params: %s\n", err)

cmd/lakefs/cmd/migrate_test.go

@@ -33,7 +33,7 @@ func TestDoMigrate(t *testing.T) {
 	})
 
 	t.Run("initial_kv_version", func(t *testing.T) {
-		cfg := config.Config{}
+		cfg := config.BaseConfig{}
 		cfg.Auth.UIConfig.RBAC = config.AuthRBACSimplified
 		cfg.Auth.Encrypt.SecretKey = "test"
 		kvStore := kvtest.GetStore(ctx, t)
@@ -46,7 +46,7 @@ func TestDoMigrate(t *testing.T) {
 	})
 
 	t.Run("from_acl_v1_force", func(t *testing.T) {
-		cfg := config.Config{}
+		cfg := config.BaseConfig{}
 		cfg.Auth.UIConfig.RBAC = config.AuthRBACSimplified
 		kvStore := kvtest.GetStore(ctx, t)
 		require.NoError(t, kv.SetDBSchemaVersion(ctx, kvStore, kv.ACLNoReposMigrateVersion))
@@ -58,7 +58,7 @@ func TestDoMigrate(t *testing.T) {
 	})
 
 	t.Run("from_acl_v2", func(t *testing.T) {
-		cfg := config.Config{}
+		cfg := config.BaseConfig{}
 		cfg.Auth.UIConfig.RBAC = config.AuthRBACSimplified
 		startVer := kv.ACLNoReposMigrateVersion
 		for !kv.IsLatestSchemaVersion(startVer) {
@@ -74,7 +74,7 @@ func TestDoMigrate(t *testing.T) {
 	})
 
 	t.Run("latest_version", func(t *testing.T) {
-		cfg := config.Config{}
+		cfg := config.BaseConfig{}
 		cfg.Auth.UIConfig.RBAC = config.AuthRBACSimplified
 		kvStore := kvtest.GetStore(ctx, t)
 		require.NoError(t, kv.SetDBSchemaVersion(ctx, kvStore, kv.NextSchemaVersion-1))
@@ -86,7 +86,7 @@ func TestDoMigrate(t *testing.T) {
 	})
 
 	t.Run("next_version", func(t *testing.T) {
-		cfg := config.Config{}
+		cfg := config.BaseConfig{}
 		cfg.Auth.UIConfig.RBAC = config.AuthRBACSimplified
 		kvStore := kvtest.GetStore(ctx, t)
 		require.NoError(t, kv.SetDBSchemaVersion(ctx, kvStore, kv.NextSchemaVersion))

cmd/lakefs/cmd/root.go

@@ -11,6 +11,7 @@ import (
 	"github.com/mitchellh/go-homedir"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
+	configfactory "github.com/treeverse/lakefs/modules/config/factory"
 	"github.com/treeverse/lakefs/pkg/block"
 	"github.com/treeverse/lakefs/pkg/config"
 	"github.com/treeverse/lakefs/pkg/kv/local"
@@ -52,7 +53,9 @@ func init() {
 	rootCmd.PersistentFlags().Bool(config.QuickstartConfiguration, false, "Use lakeFS quickstart configuration")
 }
 
-func validateQuickstartEnv(cfg *config.Config) {
+// TODO (niro): All this validation logic should be in the config package
+
+func validateQuickstartEnv(cfg *config.BaseConfig) {
 	if (cfg.Database.Type != local.DriverName && cfg.Database.Type != mem.DriverName) || cfg.Blockstore.Type != block.BlockstoreTypeLocal {
 		_, _ = fmt.Fprint(os.Stderr, "\nFATAL: quickstart mode can only run with local settings\n")
 		os.Exit(1)
@@ -78,27 +81,38 @@ func useConfig(flagName string) bool {
 	return res
 }
 
-func newConfig() (*config.Config, error) {
+func newConfig() (config.Config, error) {
 	name := ""
 	configurations := []string{config.QuickstartConfiguration, config.UseLocalConfiguration}
 	if idx := slices.IndexFunc(configurations, useConfig); idx != -1 {
 		name = configurations[idx]
 	}
 
-	cfg, err := config.NewConfig(name)
+	cfg, err := configfactory.BuildConfig(name)
 	if err != nil {
 		return nil, err
 	}
 
 	if name == config.QuickstartConfiguration {
-		validateQuickstartEnv(cfg)
+		validateQuickstartEnv(cfg.GetBaseConfig())
 	}
-	return cfg, nil
+	return cfg.GetBaseConfig(), nil
 }
 
-func loadConfig() *config.Config {
-	initOnce.Do(initConfig)
+func loadConfig() config.Config {
+	log := logging.ContextUnavailable().WithField("phase", "startup")
+	initOnce.Do(func() {
+		initConfig(log)
+	})
+	// setup config used by the executed command
 	cfg, err := newConfig()
+	if err != nil {
+		log.WithError(err).Fatal("Load config")
+	} else {
+		log.Info("Config loaded")
+	}
+
+	log.WithFields(config.MapLoggingFields(cfg)).Info("Config")
 	if err != nil {
 		fmt.Println("Failed to load config file", err)
 		os.Exit(1)
@@ -107,10 +121,9 @@ func loadConfig() *config.Config {
 }
 
 // initConfig reads in config file and ENV variables if set.
-func initConfig() {
-	logger := logging.ContextUnavailable().WithField("phase", "startup")
+func initConfig(log logging.Logger) {
 	if cfgFile != "" {
-		logger.WithField("file", cfgFile).Info("Configuration file")
+		log.WithField("file", cfgFile).Info("Configuration file")
 		// Use config file from the flag.
 		viper.SetConfigFile(cfgFile)
 	} else {
@@ -128,39 +141,24 @@ func initConfig() {
 
 	// read the configuration file
 	err := viper.ReadInConfig()
-	logger = logger.WithField("file", viper.ConfigFileUsed()) // should be called after SetConfigFile
+	log = log.WithField("file", viper.ConfigFileUsed()) // should be called after SetConfigFile
 	var errFileNotFound viper.ConfigFileNotFoundError
 	if err != nil && !errors.As(err, &errFileNotFound) {
-		logger.WithError(err).Fatal("Failed to find a config file")
+		log.WithError(err).Fatal("Failed to find a config file")
 	}
 	// fallback - try to load the previous supported $HOME/.lakefs.yaml
 	//   if err is set it will be file-not-found based on the previous check
 	if err != nil {
 		fallbackCfgFile := path.Join(getHomeDir(), ".lakefs.yaml")
 		if cfgFile != fallbackCfgFile {
 			viper.SetConfigFile(fallbackCfgFile)
-			logger = logger.WithField("file", viper.ConfigFileUsed()) // should be called after SetConfigFile
+			log = log.WithField("file", viper.ConfigFileUsed()) // should be called after SetConfigFile
 			err = viper.ReadInConfig()
 			if err != nil && !os.IsNotExist(err) {
-				logger.WithError(err).Fatal("Failed to read config file")
+				log.WithError(err).Fatal("Failed to read config file")
 			}
 		}
 	}
-
-	// setup config used by the executed command
-	cfg, err := newConfig()
-	if err != nil {
-		logger.WithError(err).Fatal("Load config")
-	} else {
-		logger.Info("Config loaded")
-	}
-
-	err = cfg.Validate()
-	if err != nil {
-		logger.WithError(err).Fatal("Invalid config")
-	}
-
-	logger.WithFields(config.MapLoggingFields(cfg)).Info("Config")
 }
 
 // getHomeDir find and return the home directory

cmd/lakefs/cmd/run.go

@@ -62,7 +62,7 @@ var (
 	errInvalidAuth    = errors.New("invalid auth configuration")
 )
 
-func checkAuthModeSupport(cfg *config.Config) error {
+func checkAuthModeSupport(cfg *config.BaseConfig) error {
 	if cfg.IsAuthBasic() { // Basic mode
 		return nil
 	}
@@ -75,7 +75,7 @@ func checkAuthModeSupport(cfg *config.Config) error {
 	return nil
 }
 
-func NewAuthService(ctx context.Context, cfg *config.Config, logger logging.Logger, kvStore kv.Store, metadataManager *auth.KVMetadataManager) auth.Service {
+func NewAuthService(ctx context.Context, cfg *config.BaseConfig, logger logging.Logger, kvStore kv.Store, metadataManager *auth.KVMetadataManager) auth.Service {
 	if err := checkAuthModeSupport(cfg); err != nil {
 		logger.WithError(err).Fatal("Unsupported auth mode")
 	}
@@ -137,10 +137,10 @@ var runCmd = &cobra.Command{
 	Short: "Run lakeFS",
 	Run: func(cmd *cobra.Command, args []string) {
 		logger := logging.ContextUnavailable()
-		cfg := loadConfig()
+		cfg := loadConfig().GetBaseConfig()
 		viper.WatchConfig()
 		viper.OnConfigChange(func(in fsnotify.Event) {
-			var c config.Config
+			var c config.BaseConfig
 			if err := config.Unmarshal(&c); err != nil {
 				logger.WithError(err).Error("Failed to unmarshal config while reload")
 				return

cmd/lakefs/cmd/run_test.go

@@ -13,7 +13,7 @@ import (
 
 func TestGetAuthService(t *testing.T) {
 	t.Run("maintain_inviter", func(t *testing.T) {
-		cfg := &config.Config{}
+		cfg := &config.BaseConfig{}
 		cfg.Auth.UIConfig.RBAC = config.AuthRBACInternal
 		cfg.Auth.API.Endpoint = "http://localhost:8000"
 		cfg.Auth.API.SkipHealthCheck = true
@@ -24,7 +24,7 @@ func TestGetAuthService(t *testing.T) {
 		}
 	})
 	t.Run("maintain_service", func(t *testing.T) {
-		cfg := &config.Config{}
+		cfg := &config.BaseConfig{}
 		kvStore := kvtest.GetStore(context.Background(), t)
 		meta := auth.NewKVMetadataManager("serve_test", cfg.Installation.FixedID, cfg.Database.Type, kvStore)
 		cfg.Auth.UIConfig.RBAC = config.AuthRBACNone

cmd/lakefs/cmd/setup.go

@@ -23,7 +23,7 @@ var setupCmd = &cobra.Command{
 	Aliases: []string{"init"},
 	Short:   "Setup a new lakeFS instance with initial credentials",
 	Run: func(cmd *cobra.Command, args []string) {
-		cfg := loadConfig()
+		cfg := loadConfig().GetBaseConfig()
 
 		ctx := cmd.Context()
 		kvParams, err := kvparams.NewConfig(&cfg.Database)
@@ -100,7 +100,7 @@ var setupCmd = &cobra.Command{
 	},
 }
 
-func setupLakeFS(ctx context.Context, cfg *config.Config, metadataManager auth.MetadataManager, authService auth.Service, userName string, accessKeyID string, secretAccessKey string) (*model.Credential, error) {
+func setupLakeFS(ctx context.Context, cfg *config.BaseConfig, metadataManager auth.MetadataManager, authService auth.Service, userName string, accessKeyID string, secretAccessKey string) (*model.Credential, error) {
 	initialized, err := metadataManager.IsInitialized(ctx)
 	if err != nil || initialized {
 		// return on error or if already initialized

cmd/lakefs/cmd/superuser.go

@@ -31,7 +31,7 @@ To do that provide the user name as well as the access key ID to import.
 If the wrong user or credentials were chosen it is possible to delete the user and perform the action again.
 `,
 	Run: func(cmd *cobra.Command, args []string) {
-		cfg := loadConfig()
+		cfg := loadConfig().GetBaseConfig()
 		if cfg.Auth.UIConfig.RBAC == config.AuthRBACExternal {
 			fmt.Printf("Can't create additional admin while using external auth API - auth.api.endpoint is configured.\n")
 			os.Exit(1)

go.work

@@ -2,6 +2,7 @@ go 1.23
 
 use (
 	.
-	./modules/block/factory
 	./webui
+	./modules/block/factory
+	./modules/config/factory
 )

modules/config/factory/build.go

@@ -0,0 +1,25 @@
+package factory
+
+import (
+	"github.com/treeverse/lakefs/pkg/config"
+)
+
+func BuildConfig(cfgType string) (config.Config, error) {
+	c := &config.BaseConfig{}
+	c, err := config.NewConfig(cfgType, c)
+	if err != nil {
+		return nil, err
+	}
+
+	// Perform required validations
+	if err = c.Validate(); err != nil {
+		return nil, err
+	}
+
+	err = c.ValidateDomainNames()
+	if err != nil {
+		return nil, err
+	}
+
+	return c, nil
+}

modules/config/factory/go.mod

@@ -0,0 +1,3 @@
+module github.com/treeverse/lakefs/modules/config/factory
+
+go 1.23

pkg/api/controller.go

@@ -88,7 +88,7 @@ type Migrator interface {
 }
 
 type Controller struct {
-	Config                *config.Config
+	Config                *config.BaseConfig
 	Catalog               *catalog.Catalog
 	Authenticator         auth.Authenticator
 	Auth                  auth.Service
@@ -108,7 +108,7 @@ type Controller struct {
 
 var usageCounter = stats.NewUsageCounter()
 
-func NewController(cfg *config.Config, catalog *catalog.Catalog, authenticator auth.Authenticator, authService auth.Service, authenticationService authentication.Service, blockAdapter block.Adapter, metadataManager auth.MetadataManager, migrator Migrator, collector stats.Collector, cloudMetadataProvider cloud.MetadataProvider, actions actionsHandler, auditChecker AuditChecker, logger logging.Logger, sessionStore sessions.Store, pathProvider upload.PathProvider, usageReporter stats.UsageReporterOperations) *Controller {
+func NewController(cfg *config.BaseConfig, catalog *catalog.Catalog, authenticator auth.Authenticator, authService auth.Service, authenticationService authentication.Service, blockAdapter block.Adapter, metadataManager auth.MetadataManager, migrator Migrator, collector stats.Collector, cloudMetadataProvider cloud.MetadataProvider, actions actionsHandler, auditChecker AuditChecker, logger logging.Logger, sessionStore sessions.Store, pathProvider upload.PathProvider, usageReporter stats.UsageReporterOperations) *Controller {
 	return &Controller{
 		Config:                cfg,
 		Catalog:               catalog,
@@ -4948,7 +4948,7 @@ func (c *Controller) GetTag(w http.ResponseWriter, r *http.Request, repository,
 	writeResponse(w, r, http.StatusOK, response)
 }
 
-func newLoginConfig(c *config.Config) *apigen.LoginConfig {
+func newLoginConfig(c *config.BaseConfig) *apigen.LoginConfig {
 	return &apigen.LoginConfig{
 		RBAC:               &c.Auth.UIConfig.RBAC,
 		LoginUrl:           c.Auth.UIConfig.LoginURL,