Add generic function to retrieve config value with metadata (#15083)

`config.ResolveConfigParam` returns the value of a configuration for any
subsystem based on checking env, config store, and default value. Also returns info
about which config source returned the value.

This is useful to return info about config params overridden via env in the user
APIs. Currently implemented only for OpenID subsystem, but will be extended for
others subsequently.

cmd/config-current.go

@@ -338,7 +338,7 @@ func validateSubSysConfig(s config.Config, subSys string, objAPI ObjectLayer) er
 			etcdClnt.Close()
 		}
 	case config.IdentityOpenIDSubSys:
-		if _, err := openid.LookupConfig(s[config.IdentityOpenIDSubSys],
+		if _, err := openid.LookupConfig(s,
 			NewGatewayHTTPTransport(), xhttp.DrainBody, globalSite.Region); err != nil {
 			return err
 		}
@@ -560,7 +560,7 @@ func lookupConfigs(s config.Config, objAPI ObjectLayer) {
 		logger.LogIf(ctx, fmt.Errorf("CRITICAL: enabling %s is not recommended in a production environment", xtls.EnvIdentityTLSSkipVerify))
 	}
 
-	globalOpenIDConfig, err = openid.LookupConfig(s[config.IdentityOpenIDSubSys],
+	globalOpenIDConfig, err = openid.LookupConfig(s,
 		NewGatewayHTTPTransport(), xhttp.DrainBody, globalSite.Region)
 	if err != nil {
 		logger.LogIf(ctx, fmt.Errorf("Unable to initialize OpenID: %w", err))

internal/config/config.go

@@ -940,3 +940,197 @@ func (c Config) SetKVS(s string, defaultKVS map[string]KVS) (dynamic bool, err e
 	c[subSys][tgt] = currKVS
 	return dynamic, nil
 }
+
+// CheckValidKeys - checks if the config parameters for the given subsystem and
+// target are valid. It checks both the configuration store as well as
+// environment variables.
+func (c Config) CheckValidKeys(subSys string, deprecatedKeys []string) error {
+	defKVS, ok := DefaultKVS[subSys]
+	if !ok {
+		return fmt.Errorf("Subsystem %s does not exist", subSys)
+	}
+
+	// Make a list of valid keys for the subsystem including the `comment`
+	// key.
+	validKeys := make([]string, 0, len(defKVS)+1)
+	for _, param := range defKVS {
+		validKeys = append(validKeys, param.Key)
+	}
+	validKeys = append(validKeys, Comment)
+
+	subSysEnvVars := env.List(fmt.Sprintf("%s%s", EnvPrefix, strings.ToUpper(subSys)))
+
+	// Set of env vars for the sub-system to validate.
+	candidates := set.CreateStringSet(subSysEnvVars...)
+
+	// Remove all default target env vars from the candidates set (as they
+	// are valid).
+	for _, param := range validKeys {
+		paramEnvName := getEnvVarName(subSys, Default, param)
+		candidates.Remove(paramEnvName)
+	}
+
+	isSingleTarget := SubSystemsSingleTargets.Contains(subSys)
+	if isSingleTarget && len(candidates) > 0 {
+		return fmt.Errorf("The following environment variables are unknown: %s",
+			strings.Join(candidates.ToSlice(), ", "))
+	}
+
+	if !isSingleTarget {
+		// Validate other env vars for all targets.
+		envVars := candidates.ToSlice()
+		for _, envVar := range envVars {
+			for _, param := range validKeys {
+				pEnvName := getEnvVarName(subSys, Default, param) + Default
+				if len(envVar) > len(pEnvName) && strings.HasPrefix(envVar, pEnvName) {
+					// This envVar is valid - it has a
+					// non-empty target.
+					candidates.Remove(envVar)
+				}
+			}
+		}
+
+		// Whatever remains are invalid env vars - return an error.
+		if len(candidates) > 0 {
+			return fmt.Errorf("The following environment variables are unknown: %s",
+				strings.Join(candidates.ToSlice(), ", "))
+		}
+	}
+
+	validKeysSet := set.CreateStringSet(validKeys...)
+	validKeysSet = validKeysSet.Difference(set.CreateStringSet(deprecatedKeys...))
+	kvsMap := c[subSys]
+	for tgt, kvs := range kvsMap {
+		invalidKV := KVS{}
+		for _, kv := range kvs {
+			if !validKeysSet.Contains(kv.Key) {
+				invalidKV = append(invalidKV, kv)
+			}
+		}
+		if len(invalidKV) > 0 {
+			return Errorf(
+				"found invalid keys (%s) for '%s:%s' sub-system, use 'mc admin config reset myminio %s:%s' to fix invalid keys",
+				invalidKV.String(), subSys, tgt, subSys, tgt)
+		}
+	}
+	return nil
+}
+
+// GetAvailableTargets - returns a list of targets configured for the given
+// subsystem (whether they are enabled or not). A target could be configured via
+// environment variables or via the configuration store. The default target is
+// `_` and is always returned.
+func (c Config) GetAvailableTargets(subSys string) ([]string, error) {
+	if SubSystemsSingleTargets.Contains(subSys) {
+		return []string{Default}, nil
+	}
+
+	defKVS, ok := DefaultKVS[subSys]
+	if !ok {
+		return nil, fmt.Errorf("Subsystem %s does not exist", subSys)
+	}
+
+	kvsMap := c[subSys]
+	s := set.NewStringSet()
+
+	// Add all targets that are configured in the config store.
+	for k := range kvsMap {
+		s.Add(k)
+	}
+
+	// Add targets that are configured via environment variables.
+	for _, param := range defKVS {
+		envVarPrefix := getEnvVarName(subSys, Default, param.Key) + Default
+		envsWithPrefix := env.List(envVarPrefix)
+		for _, k := range envsWithPrefix {
+			tgtName := strings.TrimPrefix(k, envVarPrefix)
+			if tgtName != "" {
+				s.Add(tgtName)
+			}
+		}
+	}
+
+	return s.ToSlice(), nil
+}
+
+func getEnvVarName(subSys, target, param string) string {
+	if target == Default {
+		return fmt.Sprintf("%s%s_%s", EnvPrefix, strings.ToUpper(subSys), strings.ToUpper(param))
+	}
+
+	return fmt.Sprintf("%s%s_%s_%s", EnvPrefix, strings.ToUpper(subSys), strings.ToUpper(param), target)
+}
+
+var resolvableSubsystems = set.CreateStringSet(IdentityOpenIDSubSys)
+
+// ValueSource represents the source of a config parameter value.
+type ValueSource uint8
+
+// Constants for ValueSource
+const (
+	ValueSourceAbsent ValueSource = iota // this is an error case
+	ValueSourceDef
+	ValueSourceCfg
+	ValueSourceEnv
+)
+
+// ResolveConfigParam returns the effective value of a configuration parameter,
+// within a subsystem and subsystem target. The effective value is, in order of
+// decreasing precedence:
+//
+// 1. the value of the corresponding environment variable if set,
+// 2. the value of the parameter in the config store if set,
+// 3. the default value,
+//
+// This function only works for a subset of sub-systems, others return
+// `ValueSourceAbsent`. FIXME: some parameters have custom environment
+// variables for which support needs to be added.
+func (c Config) ResolveConfigParam(subSys, target, cfgParam string) (value string, cs ValueSource) {
+	// cs = ValueSourceAbsent initially as it is iota by default.
+
+	// Initially only support OpenID
+	if !resolvableSubsystems.Contains(subSys) {
+		return
+	}
+
+	// Check if config param requested is valid.
+	defKVS, ok := DefaultKVS[subSys]
+	if !ok {
+		return
+	}
+
+	defValue, isFound := defKVS.Lookup(cfgParam)
+	if !isFound {
+		return
+	}
+
+	if target == "" {
+		target = Default
+	}
+
+	envVar := getEnvVarName(subSys, target, cfgParam)
+
+	// Lookup Env var.
+	value = env.Get(envVar, "")
+	if value != "" {
+		cs = ValueSourceEnv
+		return
+	}
+
+	// Lookup config store.
+	if subSysStore, ok := c[subSys]; ok {
+		if kvs, ok2 := subSysStore[target]; ok2 {
+			var ok3 bool
+			value, ok3 = kvs.Lookup(cfgParam)
+			if ok3 {
+				cs = ValueSourceCfg
+				return
+			}
+		}
+	}
+
+	// Return the default value.
+	value = defValue
+	cs = ValueSourceDef
+	return
+}

internal/config/identity/openid/jwt_test.go

@@ -243,7 +243,7 @@ func TestKeycloakProviderInitialization(t *testing.T) {
 	testKvs.Set(Vendor, "keycloak")
 	testKvs.Set(KeyCloakRealm, "TestRealm")
 	testKvs.Set(KeyCloakAdminURL, "http://keycloak.test/auth/admin")
-	cfgGet := func(env, param string) string {
+	cfgGet := func(param string) string {
 		return testKvs.Get(param)
 	}
 

internal/config/identity/openid/openid.go

@@ -36,49 +36,32 @@ import (
 	"github.com/minio/minio/internal/config"
 	"github.com/minio/minio/internal/config/identity/openid/provider"
 	"github.com/minio/minio/internal/hash/sha256"
-	"github.com/minio/pkg/env"
 	iampolicy "github.com/minio/pkg/iam/policy"
 	xnet "github.com/minio/pkg/net"
 )
 
 // OpenID keys and envs.
 const (
-	JwksURL       = "jwks_url"
+	ClientID      = "client_id"
+	ClientSecret  = "client_secret"
 	ConfigURL     = "config_url"
 	ClaimName     = "claim_name"
 	ClaimUserinfo = "claim_userinfo"
-	ClaimPrefix   = "claim_prefix"
-	ClientID      = "client_id"
-	ClientSecret  = "client_secret"
 	RolePolicy    = "role_policy"
 	DisplayName   = "display_name"
 
-	Vendor             = "vendor"
 	Scopes             = "scopes"
 	RedirectURI        = "redirect_uri"
 	RedirectURIDynamic = "redirect_uri_dynamic"
+	Vendor             = "vendor"
 
 	// Vendor specific ENV only enabled if the Vendor matches == "vendor"
 	KeyCloakRealm    = "keycloak_realm"
 	KeyCloakAdminURL = "keycloak_admin_url"
 
-	EnvIdentityOpenIDEnable             = "MINIO_IDENTITY_OPENID_ENABLE"
-	EnvIdentityOpenIDVendor             = "MINIO_IDENTITY_OPENID_VENDOR"
-	EnvIdentityOpenIDClientID           = "MINIO_IDENTITY_OPENID_CLIENT_ID"
-	EnvIdentityOpenIDClientSecret       = "MINIO_IDENTITY_OPENID_CLIENT_SECRET"
-	EnvIdentityOpenIDURL                = "MINIO_IDENTITY_OPENID_CONFIG_URL"
-	EnvIdentityOpenIDClaimName          = "MINIO_IDENTITY_OPENID_CLAIM_NAME"
-	EnvIdentityOpenIDClaimUserInfo      = "MINIO_IDENTITY_OPENID_CLAIM_USERINFO"
-	EnvIdentityOpenIDClaimPrefix        = "MINIO_IDENTITY_OPENID_CLAIM_PREFIX"
-	EnvIdentityOpenIDRolePolicy         = "MINIO_IDENTITY_OPENID_ROLE_POLICY"
-	EnvIdentityOpenIDRedirectURI        = "MINIO_IDENTITY_OPENID_REDIRECT_URI"
-	EnvIdentityOpenIDRedirectURIDynamic = "MINIO_IDENTITY_OPENID_REDIRECT_URI_DYNAMIC"
-	EnvIdentityOpenIDScopes             = "MINIO_IDENTITY_OPENID_SCOPES"
-	EnvIdentityOpenIDDisplayName        = "MINIO_IDENTITY_OPENID_DISPLAY_NAME"
-
-	// Vendor specific ENVs only enabled if the Vendor matches == "vendor"
-	EnvIdentityOpenIDKeyCloakRealm    = "MINIO_IDENTITY_OPENID_KEYCLOAK_REALM"
-	EnvIdentityOpenIDKeyCloakAdminURL = "MINIO_IDENTITY_OPENID_KEYCLOAK_ADMIN_URL"
+	// Removed params
+	JwksURL     = "jwks_url"
+	ClaimPrefix = "claim_prefix"
 )
 
 // DefaultKVS - default config for OpenID config
@@ -191,7 +174,7 @@ func (r *Config) Clone() Config {
 }
 
 // LookupConfig lookup jwks from config, override with any ENVs.
-func LookupConfig(kvsMap map[string]config.KVS, transport http.RoundTripper, closeRespFn func(io.ReadCloser), serverRegion string) (c Config, err error) {
+func LookupConfig(s config.Config, transport http.RoundTripper, closeRespFn func(io.ReadCloser), serverRegion string) (c Config, err error) {
 	openIDClientTransport := http.DefaultTransport
 	if transport != nil {
 		openIDClientTransport = transport
@@ -209,48 +192,28 @@ func LookupConfig(kvsMap map[string]config.KVS, transport http.RoundTripper, clo
 		closeRespFn:      closeRespFn,
 	}
 
-	// Make a copy of the config we received so we can mutate it safely.
-	kvsMap2 := make(map[string]config.KVS, len(kvsMap))
-	for k, v := range kvsMap {
-		kvsMap2[k] = v
-	}
-
-	// Add in each configuration name found from environment variables, i.e.
-	// if we see MINIO_IDENTITY_OPENID_CONFIG_URL_2, we add the key "2" to
-	// `kvsMap2` if it does not already exist.
-	envs := env.List(EnvIdentityOpenIDURL + config.Default)
-	for _, k := range envs {
-		cfgName := strings.TrimPrefix(k, EnvIdentityOpenIDURL+config.Default)
-		if cfgName == "" {
-			return c, config.Errorf("Environment variable must have a non-empty config name: %s", k)
-		}
-
-		// It is possible that some variables were specified via config
-		// commands and some variables are intended to be overridden
-		// from the environment, so we ensure that the key is not
-		// overwritten in `kvsMap2` as it may have existing config.
-		if _, ok := kvsMap2[cfgName]; !ok {
-			kvsMap2[cfgName] = DefaultKVS
-		}
-	}
-
 	var (
 		hasLegacyPolicyMapping = false
 		seenClientIDs          = set.NewStringSet()
 	)
-	for cfgName, kvs := range kvsMap2 {
-		// remove this since we have removed support for this already.
-		kvs.Delete(JwksURL)
 
-		if err = config.CheckValidKeys(config.IdentityOpenIDSubSys, kvs, DefaultKVS); err != nil {
-			return c, err
-		}
+	// remove this since we have removed support for this already.
+	deprecatedKeys := []string{JwksURL}
+	if err := s.CheckValidKeys(config.IdentityOpenIDSubSys, deprecatedKeys); err != nil {
+		return c, err
+	}
 
-		getCfgVal := func(envVar, cfgParam string) string {
-			if cfgName != config.Default {
-				envVar += config.Default + cfgName
-			}
-			return env.Get(envVar, kvs.Get(cfgParam))
+	openIDTargets, err := s.GetAvailableTargets(config.IdentityOpenIDSubSys)
+	if err != nil {
+		return c, err
+	}
+
+	for _, cfgName := range openIDTargets {
+		getCfgVal := func(cfgParam string) string {
+			// As parameters are already validated, we skip checking
+			// if the config param was found.
+			val, _ := s.ResolveConfigParam(config.IdentityOpenIDSubSys, cfgName, cfgParam)
+			return val
 		}
 
 		// In the past, when only one openID provider was allowed, there
@@ -261,7 +224,7 @@ func LookupConfig(kvsMap map[string]config.KVS, transport http.RoundTripper, clo
 		// setting, otherwise we treat it as enabled if some important
 		// parameters are non-empty.
 		var (
-			cfgEnableVal        = getCfgVal(EnvIdentityOpenIDEnable, config.Enable)
+			cfgEnableVal        = getCfgVal(config.Enable)
 			isExplicitlyEnabled = false
 		)
 		if cfgEnableVal != "" {
@@ -281,7 +244,7 @@ func LookupConfig(kvsMap map[string]config.KVS, transport http.RoundTripper, clo
 		}
 
 		p := newProviderCfgFromConfig(getCfgVal)
-		configURL := getCfgVal(EnvIdentityOpenIDURL, ConfigURL)
+		configURL := getCfgVal(ConfigURL)
 
 		if !isExplicitlyEnabled {
 			enabled = true
@@ -315,7 +278,7 @@ func LookupConfig(kvsMap map[string]config.KVS, transport http.RoundTripper, clo
 			return c, errors.New("please specify config_url to enable fetching claims from UserInfo endpoint")
 		}
 
-		if scopeList := getCfgVal(EnvIdentityOpenIDScopes, Scopes); scopeList != "" {
+		if scopeList := getCfgVal(Scopes); scopeList != "" {
 			var scopes []string
 			for _, scope := range strings.Split(scopeList, ",") {
 				scope = strings.TrimSpace(scope)

internal/config/identity/openid/providercfg.go

@@ -52,17 +52,17 @@ type providerCfg struct {
 	provider provider.Provider
 }
 
-func newProviderCfgFromConfig(getCfgVal func(env, cfgName string) string) providerCfg {
+func newProviderCfgFromConfig(getCfgVal func(cfgName string) string) providerCfg {
 	return providerCfg{
-		DisplayName:        getCfgVal(EnvIdentityOpenIDDisplayName, DisplayName),
-		ClaimName:          getCfgVal(EnvIdentityOpenIDClaimName, ClaimName),
-		ClaimUserinfo:      getCfgVal(EnvIdentityOpenIDClaimUserInfo, ClaimUserinfo) == config.EnableOn,
-		ClaimPrefix:        getCfgVal(EnvIdentityOpenIDClaimPrefix, ClaimPrefix),
-		RedirectURI:        getCfgVal(EnvIdentityOpenIDRedirectURI, RedirectURI),
-		RedirectURIDynamic: getCfgVal(EnvIdentityOpenIDRedirectURIDynamic, RedirectURIDynamic) == config.EnableOn,
-		ClientID:           getCfgVal(EnvIdentityOpenIDClientID, ClientID),
-		ClientSecret:       getCfgVal(EnvIdentityOpenIDClientSecret, ClientSecret),
-		RolePolicy:         getCfgVal(EnvIdentityOpenIDRolePolicy, RolePolicy),
+		DisplayName:        getCfgVal(DisplayName),
+		ClaimName:          getCfgVal(ClaimName),
+		ClaimUserinfo:      getCfgVal(ClaimUserinfo) == config.EnableOn,
+		ClaimPrefix:        getCfgVal(ClaimPrefix),
+		RedirectURI:        getCfgVal(RedirectURI),
+		RedirectURIDynamic: getCfgVal(RedirectURIDynamic) == config.EnableOn,
+		ClientID:           getCfgVal(ClientID),
+		ClientSecret:       getCfgVal(ClientSecret),
+		RolePolicy:         getCfgVal(RolePolicy),
 	}
 }
 
@@ -72,16 +72,16 @@ const (
 
 // initializeProvider initializes if any additional vendor specific information
 // was provided, initialization will return an error initial login fails.
-func (p *providerCfg) initializeProvider(cfgGet func(string, string) string, transport http.RoundTripper) error {
-	vendor := cfgGet(EnvIdentityOpenIDVendor, Vendor)
+func (p *providerCfg) initializeProvider(cfgGet func(string) string, transport http.RoundTripper) error {
+	vendor := cfgGet(Vendor)
 	if vendor == "" {
 		return nil
 	}
 	var err error
 	switch vendor {
 	case keyCloakVendor:
-		adminURL := cfgGet(EnvIdentityOpenIDKeyCloakAdminURL, KeyCloakAdminURL)
-		realm := cfgGet(EnvIdentityOpenIDKeyCloakRealm, KeyCloakRealm)
+		adminURL := cfgGet(KeyCloakAdminURL)
+		realm := cfgGet(KeyCloakRealm)
 		p.provider, err = provider.KeyCloak(
 			provider.WithAdminURL(adminURL),
 			provider.WithOpenIDConfig(provider.DiscoveryDoc(p.DiscoveryDoc)),