refactor(dependency): replace marshal-based env binding with type-walk

The previous approach marshalled a zero config value to YAML and loaded it
into a scratch viper to discover keys. This missed nil-pointer optional
sections (e.g. scheduler server.tls) and embedded squashed structs.

Replace it with a recursive reflect.Type walk that follows mapstructure tags
directly — the same tags viper.Unmarshal uses — handling pointer descent,
`,squash` embedded structs, and untagged fields (decoded by field name).
Tests are added for squashed and untagged field overrides.

Signed-off-by: Gaius <gaius.qi@gmail.com>

Author: gaius-qi

cmd/dependency/dependency.go

@@ -17,7 +17,6 @@
 package dependency
 
 import (
-	"bytes"
 	"context"
 	"errors"
 	"fmt"
@@ -77,64 +76,74 @@ func InitCommandAndConfig(cmd *cobra.Command, useConfigFile bool, config any) {
 			panic(fmt.Errorf("bind common flags to viper: %w", err))
 		}
 
-		// Config for binding env
+		// Env overrides: AutomaticEnv resolves env values for keys viper
+		// already knows (config file, flags); bindEnvsFromConfig registers
+		// every config struct key so overrides also reach keys absent from
+		// the config file (spf13/viper#761).
 		viper.SetEnvPrefix(rootName)
 		viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))
 		viper.AutomaticEnv()
-		_ = viper.BindEnv("config")
-
-		// Bind env for keys absent from the config file
 		bindEnvsFromConfig(config)
 
+		// "config" is a flag-only key, not part of the config struct.
+		_ = viper.BindEnv("config")
+
 		// Add common cmds only on root cmd
 		cmd.AddCommand(VersionCmd)
 		cmd.AddCommand(newDocCommand(cmd.Name()))
 		cmd.AddCommand(PluginCmd)
 	}
 }
 
-// bindEnvsFromConfig binds an env for every config key, so AutomaticEnv can
-// override keys that are not present in the config file.
+// bindEnvsFromConfig binds an env for every key in the config struct, so
+// AutomaticEnv can override keys that are not present in the config file.
+// Viper only applies env overrides in Unmarshal for keys it already knows
+// (spf13/viper#761), so each key must be bound explicitly.
 func bindEnvsFromConfig(config any) {
-	schema := reflect.New(reflect.TypeOf(config).Elem()).Interface()
-	materializeStructPtrs(reflect.ValueOf(schema).Elem())
-
-	b, err := yaml.Marshal(schema)
-	if err != nil {
-		panic(fmt.Errorf("marshal config for env binding: %w", err))
-	}
+	bindEnvs(reflect.TypeOf(config), "")
+}
 
-	keys := viper.New()
-	keys.SetConfigType("yaml")
-	if err := keys.ReadConfig(bytes.NewReader(b)); err != nil {
-		panic(fmt.Errorf("read config for env binding: %w", err))
+// bindEnvs walks a config type over its mapstructure tags — the tags
+// viper.Unmarshal decodes with — and binds every leaf key. Walking the type
+// rather than a value also covers optional sections behind nil pointers
+// (e.g. the scheduler's server.tls), which a marshalled snapshot of the
+// default config would miss.
+func bindEnvs(t reflect.Type, prefix string) {
+	// Descend through pointers (optional sections like server.tls).
+	for t.Kind() == reflect.Pointer {
+		t = t.Elem()
 	}
 
-	for _, key := range keys.AllKeys() {
-		_ = viper.BindEnv(key)
+	if t.Kind() != reflect.Struct {
+		_ = viper.BindEnv(prefix)
+		return
 	}
-}
 
-// materializeStructPtrs allocates nil struct-pointers so their nested keys serialize.
-func materializeStructPtrs(v reflect.Value) {
-	switch v.Kind() {
-	case reflect.Pointer:
-		if v.Type().Elem().Kind() != reflect.Struct {
-			return
+	for i := range t.NumField() {
+		field := t.Field(i)
+		if !field.IsExported() {
+			continue
 		}
-		if v.IsNil() {
-			if !v.CanSet() {
-				return
-			}
-			v.Set(reflect.New(v.Type().Elem()))
+
+		name, opts, _ := strings.Cut(field.Tag.Get("mapstructure"), ",")
+		switch {
+		case name == "-":
+			continue
+		case field.Anonymous && strings.Contains(opts, "squash"):
+			// mapstructure:",squash" flattens the embedded struct's
+			// keys into the parent.
+			bindEnvs(field.Type, prefix)
+			continue
+		case name == "":
+			// Untagged fields decode by field name, matched
+			// case-insensitively.
+			name = field.Name
 		}
-		materializeStructPtrs(v.Elem())
-	case reflect.Struct:
-		for i := 0; i < v.NumField(); i++ {
-			if f := v.Field(i); f.CanSet() {
-				materializeStructPtrs(f)
-			}
+
+		if prefix != "" {
+			name = prefix + "." + name
 		}
+		bindEnvs(field.Type, name)
 	}
 }
 

cmd/dependency/dependency_test.go

@@ -24,6 +24,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"d7y.io/dragonfly/v2/cmd/dependency/base"
 	schedulerconfig "d7y.io/dragonfly/v2/scheduler/config"
 )
 
@@ -34,11 +35,22 @@ type tlsConfig struct {
 	CACert string `yaml:"caCert" mapstructure:"caCert"`
 }
 
-// testConfig mirrors the real config shape: populated scalar defaults plus an
-// optional nested section behind a nil pointer.
+// portRange mirrors manager/config.TCPListenPortRange, whose fields carry no
+// tags and therefore decode by field name.
+type portRange struct {
+	Start int
+	End   int
+}
+
+// testConfig mirrors the real config shape: an embedded squashed section,
+// populated scalar defaults, an optional nested section behind a nil pointer,
+// and a section with untagged fields.
 type testConfig struct {
+	base.Options `yaml:",inline" mapstructure:",squash"`
+
 	Name string     `yaml:"name" mapstructure:"name"`
 	TLS  *tlsConfig `yaml:"tls" mapstructure:"tls"`
+	Port portRange  `yaml:"port" mapstructure:"port"`
 }
 
 // newTestConfig returns the default config: scalar defaults set, optional TLS
@@ -96,6 +108,34 @@ func TestBindEnvsFromConfig_NoEnvKeepsDefaults(t *testing.T) {
 	assert.Nil(t, cfg.TLS, "no env set should leave the optional section nil")
 }
 
+// TestBindEnvsFromConfig_SquashedOverride covers embedded sections tagged
+// mapstructure:",squash" (base.Options in the real configs), whose keys live
+// at the top level rather than under the field name.
+func TestBindEnvsFromConfig_SquashedOverride(t *testing.T) {
+	setupViper("test")
+	t.Setenv("TEST_CONSOLE", "true")
+
+	cfg := newTestConfig()
+	bindEnvsFromConfig(cfg)
+	require.NoError(t, viper.Unmarshal(cfg, initDecoderConfig))
+
+	assert.True(t, cfg.Console, "squashed key should bind at the top level")
+}
+
+// TestBindEnvsFromConfig_UntaggedFieldOverride covers fields without a
+// mapstructure tag (manager/config.TCPListenPortRange), which decode by field
+// name matched case-insensitively.
+func TestBindEnvsFromConfig_UntaggedFieldOverride(t *testing.T) {
+	setupViper("test")
+	t.Setenv("TEST_PORT_START", "65003")
+
+	cfg := newTestConfig()
+	bindEnvsFromConfig(cfg)
+	require.NoError(t, viper.Unmarshal(cfg, initDecoderConfig))
+
+	assert.Equal(t, 65003, cfg.Port.Start, "untagged field should bind by field name")
+}
+
 // TestBindEnvsFromConfig_RealSchedulerConfig exercises the actual production
 // config type end-to-end, including a nested key (Server.TLS.CACert) that lives
 // under a pointer left nil by config.New() — the regression the fix targets.