From b4f71362e9317ea3ffe64cbfc7c38b4960a849c6 Mon Sep 17 00:00:00 2001 From: Klaus Post Date: Mon, 19 Dec 2022 20:10:14 +0100 Subject: [PATCH] Avoid config migration on every startup (#16278) --- cmd/admin-handlers-config-kv.go | 6 ++-- cmd/admin-handlers-idp-config.go | 4 +-- cmd/admin-handlers.go | 2 +- cmd/config-current.go | 7 +++-- cmd/config-current_test.go | 2 +- cmd/config-migrate.go | 15 ++++++++++ cmd/config-migrate_test.go | 4 +-- cmd/config.go | 49 ++++++++++++++++++++------------ 8 files changed, 59 insertions(+), 30 deletions(-) diff --git a/cmd/admin-handlers-config-kv.go b/cmd/admin-handlers-config-kv.go index 40fe92077..5b6e7d499 100644 --- a/cmd/admin-handlers-config-kv.go +++ b/cmd/admin-handlers-config-kv.go @@ -71,7 +71,7 @@ func (a adminAPIHandlers) DelConfigKVHandler(w http.ResponseWriter, r *http.Requ return } - cfg, err := readServerConfig(ctx, objectAPI) + cfg, err := readServerConfig(ctx, objectAPI, nil) if err != nil { writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL) return @@ -142,7 +142,7 @@ func (a adminAPIHandlers) SetConfigKVHandler(w http.ResponseWriter, r *http.Requ return } - cfg, err := readServerConfig(ctx, objectAPI) + cfg, err := readServerConfig(ctx, objectAPI, nil) if err != nil { writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL) return @@ -296,7 +296,7 @@ func (a adminAPIHandlers) RestoreConfigHistoryKVHandler(w http.ResponseWriter, r return } - cfg, err := readServerConfig(ctx, objectAPI) + cfg, err := readServerConfig(ctx, objectAPI, nil) if err != nil { writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL) return diff --git a/cmd/admin-handlers-idp-config.go b/cmd/admin-handlers-idp-config.go index a6640b764..7e9ea07f8 100644 --- a/cmd/admin-handlers-idp-config.go +++ b/cmd/admin-handlers-idp-config.go @@ -107,7 +107,7 @@ func (a adminAPIHandlers) addOrUpdateIDPHandler(ctx context.Context, w http.Resp cfgData = subSys + tgtSuffix + config.KvSpaceSeparator + string(reqBytes) } - cfg, err := readServerConfig(ctx, objectAPI) + cfg, err := readServerConfig(ctx, objectAPI, nil) if err != nil { writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL) return @@ -408,7 +408,7 @@ func (a adminAPIHandlers) DeleteIdentityProviderCfg(w http.ResponseWriter, r *ht return } - cfg, err := readServerConfig(ctx, objectAPI) + cfg, err := readServerConfig(ctx, objectAPI, nil) if err != nil { writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL) return diff --git a/cmd/admin-handlers.go b/cmd/admin-handlers.go index 0540b482d..69aedeb6a 100644 --- a/cmd/admin-handlers.go +++ b/cmd/admin-handlers.go @@ -2135,7 +2135,7 @@ func fetchHealthInfo(healthCtx context.Context, objectAPI ObjectLayer, query *ur getAndWriteMinioConfig := func() { if query.Get("minioconfig") == "true" { - config, err := readServerConfig(healthCtx, objectAPI) + config, err := readServerConfig(healthCtx, objectAPI, nil) if err != nil { healthInfo.Minio.Config = madmin.MinioConfig{ Error: err.Error(), diff --git a/cmd/config-current.go b/cmd/config-current.go index 503324319..61a7b5c59 100644 --- a/cmd/config-current.go +++ b/cmd/config-current.go @@ -782,13 +782,14 @@ func newSrvConfig(objAPI ObjectLayer) error { } func getValidConfig(objAPI ObjectLayer) (config.Config, error) { - return readServerConfig(GlobalContext, objAPI) + return readServerConfig(GlobalContext, objAPI, nil) } // loadConfig - loads a new config from disk, overrides params // from env if found and valid -func loadConfig(objAPI ObjectLayer) error { - srvCfg, err := getValidConfig(objAPI) +// data is optional. If nil it will be loaded from backend. +func loadConfig(objAPI ObjectLayer, data []byte) error { + srvCfg, err := readServerConfig(GlobalContext, objAPI, data) if err != nil { return err } diff --git a/cmd/config-current_test.go b/cmd/config-current_test.go index 7299312c3..ceaae7f03 100644 --- a/cmd/config-current_test.go +++ b/cmd/config-current_test.go @@ -61,7 +61,7 @@ func TestServerConfig(t *testing.T) { } // Initialize server config. - if err := loadConfig(objLayer); err != nil { + if err := loadConfig(objLayer, nil); err != nil { t.Fatalf("Unable to initialize from updated config file %s", err) } } diff --git a/cmd/config-migrate.go b/cmd/config-migrate.go index f10230d27..343b0047e 100644 --- a/cmd/config-migrate.go +++ b/cmd/config-migrate.go @@ -2533,6 +2533,21 @@ func checkConfigVersion(objAPI ObjectLayer, configFile string, version string) ( } } + if version == "kvs" { + // Check api config values are present. + var vcfg struct { + API struct { + E []struct { + Key string `json:"key"` + Value string `json:"value"` + } `json:"_"` + } `json:"api"` + } + if err = json.Unmarshal(data, &vcfg); err != nil { + return false, nil, nil + } + return len(vcfg.API.E) > 0, data, nil + } var versionConfig struct { Version string `json:"version"` } diff --git a/cmd/config-migrate_test.go b/cmd/config-migrate_test.go index da1143e24..ff0a8207d 100644 --- a/cmd/config-migrate_test.go +++ b/cmd/config-migrate_test.go @@ -65,7 +65,7 @@ func TestServerConfigMigrateV1(t *testing.T) { } // Initialize server config and check again if everything is fine - if err := loadConfig(objLayer); err != nil { + if err := loadConfig(objLayer, nil); err != nil { t.Fatalf("Unable to initialize from updated config file %s", err) } } @@ -207,7 +207,7 @@ func TestServerConfigMigrateV2toV33(t *testing.T) { } // Initialize server config and check again if everything is fine - if err := loadConfig(objLayer); err != nil { + if err := loadConfig(objLayer, nil); err != nil { t.Fatalf("Unable to initialize from updated config file %s", err) } diff --git a/cmd/config.go b/cmd/config.go index e6e7cbd46..c9fb4ee37 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -152,30 +152,34 @@ func saveServerConfig(ctx context.Context, objAPI ObjectLayer, cfg interface{}) return saveConfig(ctx, objAPI, configFile, data) } -func readServerConfig(ctx context.Context, objAPI ObjectLayer) (config.Config, error) { +// data is optional. If nil it will be loaded from backend. +func readServerConfig(ctx context.Context, objAPI ObjectLayer, data []byte) (config.Config, error) { srvCfg := config.New() - configFile := path.Join(minioConfigPrefix, minioConfigFile) - data, err := readConfig(ctx, objAPI, configFile) - if err != nil { - if errors.Is(err, errConfigNotFound) { - lookupConfigs(srvCfg, objAPI) - return srvCfg, nil - } - return nil, err - } - - if GlobalKMS != nil && !utf8.Valid(data) { - data, err = config.DecryptBytes(GlobalKMS, data, kms.Context{ - minioMetaBucket: path.Join(minioMetaBucket, configFile), - }) + var err error + if len(data) == 0 { + configFile := path.Join(minioConfigPrefix, minioConfigFile) + data, err = readConfig(ctx, objAPI, configFile) if err != nil { - lookupConfigs(srvCfg, objAPI) + if errors.Is(err, errConfigNotFound) { + lookupConfigs(srvCfg, objAPI) + return srvCfg, nil + } return nil, err } + + if GlobalKMS != nil && !utf8.Valid(data) { + data, err = config.DecryptBytes(GlobalKMS, data, kms.Context{ + minioMetaBucket: path.Join(minioMetaBucket, configFile), + }) + if err != nil { + lookupConfigs(srvCfg, objAPI) + return nil, err + } + } } json := jsoniter.ConfigCompatibleWithStandardLibrary - if err = json.Unmarshal(data, &srvCfg); err != nil { + if err := json.Unmarshal(data, &srvCfg); err != nil { return nil, err } @@ -212,6 +216,15 @@ func initConfig(objAPI ObjectLayer) error { } } + // Check if the config version is latest (kvs), if not migrate. + ok, data, err := checkConfigVersion(objAPI, path.Join(minioConfigPrefix, minioConfigFile), "kvs") + if err != nil && !errors.Is(err, errConfigNotFound) { + return err + } + if ok { + return loadConfig(objAPI, data) + } + // Migrates ${HOME}/.minio/config.json or config.json.deprecated // to '/.minio.sys/config/config.json' // ignore if the file doesn't exist. @@ -232,5 +245,5 @@ func initConfig(objAPI ObjectLayer) error { return fmt.Errorf("migrateMinioSysConfigToKV: %w", err) } - return loadConfig(objAPI) + return loadConfig(objAPI, nil) }