From 0b16276f0861cc38707c6fda0846592969900b55 Mon Sep 17 00:00:00 2001 From: Vladimir Dementyev Date: Wed, 16 Oct 2024 14:15:58 -0700 Subject: [PATCH] fix: file config must have lower precedence than env/cli --- CHANGELOG.md | 2 + cli/options.go | 129 +++++++++++++++++++++++++++------- config/config.go | 54 +++++++------- features/file_config.testfile | 16 ++++- 4 files changed, 144 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cb4646b8..506af02a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## master +- Fix configuration parameters precedence (file -> env -> CLI). ([@palkan][]) + - Telemetry improvements. ([@palkan][]) ## 1.5.4 (2024-10-08) diff --git a/cli/options.go b/cli/options.go index a9417f15..f2197d90 100644 --- a/cli/options.go +++ b/cli/options.go @@ -5,6 +5,7 @@ import ( "os" "regexp" "strings" + "time" "github.com/anycable/anycable-go/config" "github.com/anycable/anycable-go/node" @@ -86,31 +87,10 @@ func NewConfigFromCLI(args []string, opts ...cliOption) (*config.Config, error, &cli.BoolFlag{ Name: "ignore-config-path", Usage: "Ignore configuration files", - Action: func(ctx *cli.Context, val bool) error { - if val { - c.ConfigFilePath = "none" - } - return nil - }, }, &cli.StringFlag{ Name: "config-path", Usage: "Path to the TOML configuration file", - Action: func(ctx *cli.Context, path string) error { - if c.ConfigFilePath == "none" { - c.ConfigFilePath = "" - return nil - } - - c.ConfigFilePath = path - - // check if file exists and try to load config from it - if err := c.LoadFromFile(); err != nil { - return err - } - - return nil - }, }, &cli.BoolFlag{ Name: "print-config", @@ -141,13 +121,35 @@ func NewConfigFromCLI(args []string, opts ...cliOption) (*config.Config, error, app := &cli.App{ Name: "anycable-go", Version: version.Version(), - Usage: "AnyCable-Go, The WebSocket server for https://anycable.io", + Usage: "AnyCable-Go, a real-time server for https://anycable.io", HideHelpCommand: true, Flags: flags, Action: func(nc *cli.Context) error { cliInterrupted = false return nil }, + Before: func(ctx *cli.Context) error { + ignored := ctx.Bool("ignore-config-path") + + if ignored { + return nil + } + + val := ctx.String("config-path") + + if val == "" { + return nil + } + + c.ConfigFilePath = val + + // check if file exists and try to load config from it + if err := c.LoadFromFile(); err != nil { + return err + } + + return nil + }, } for _, o := range opts { @@ -162,7 +164,7 @@ func NewConfigFromCLI(args []string, opts ...cliOption) (*config.Config, error, return &config.Config{}, err, false } - // helpOrVersionWereShown = false indicates that the default action has been run. + // cliInterrupted = false indicates that the default action has been run. // true means that help/version message was displayed. // // Unfortunately, cli module does not support another way of detecting if or which @@ -992,6 +994,7 @@ func logCLIFlags(c *config.Config) []cli.Flag { &cli.BoolFlag{ Name: "debug", Usage: "Enable debug mode (more verbose logging)", + Value: c.Log.Debug, Destination: &c.Log.Debug, }, }) @@ -1319,40 +1322,112 @@ func withDefaults(category string, flags []cli.Flag) []cli.Flag { if len(v.EnvVars) == 0 { v.EnvVars = []string{nameToEnvVarName(v.Name)} } + if v.Destination != nil { + dest := v.Destination + v.Destination = nil + + if v.Action == nil { + v.Action = func(ctx *cli.Context, setVal int) error { + *dest = setVal + return nil + } + } + } case *cli.Int64Flag: v.Category = category if len(v.EnvVars) == 0 { v.EnvVars = []string{nameToEnvVarName(v.Name)} } + if v.Destination != nil { + dest := v.Destination + v.Destination = nil + + if v.Action == nil { + v.Action = func(ctx *cli.Context, setVal int64) error { + *dest = setVal + return nil + } + } + } case *cli.Float64Flag: v.Category = category if len(v.EnvVars) == 0 { v.EnvVars = []string{nameToEnvVarName(v.Name)} } + if v.Destination != nil { + dest := v.Destination + v.Destination = nil + + if v.Action == nil { + v.Action = func(ctx *cli.Context, setVal float64) error { + *dest = setVal + return nil + } + } + } case *cli.DurationFlag: v.Category = category if len(v.EnvVars) == 0 { v.EnvVars = []string{nameToEnvVarName(v.Name)} } + if v.Destination != nil { + dest := v.Destination + v.Destination = nil + + if v.Action == nil { + v.Action = func(ctx *cli.Context, setVal time.Duration) error { + *dest = setVal + return nil + } + } + } case *cli.BoolFlag: v.Category = category if len(v.EnvVars) == 0 { v.EnvVars = []string{nameToEnvVarName(v.Name)} } + if v.Destination != nil { + dest := v.Destination + v.Destination = nil + + if v.Action == nil { + v.Action = func(ctx *cli.Context, setVal bool) error { + *dest = setVal + return nil + } + } + } case *cli.StringFlag: v.Category = category if len(v.EnvVars) == 0 { v.EnvVars = []string{nameToEnvVarName(v.Name)} } + if v.Destination != nil { + dest := v.Destination + v.Destination = nil + + if v.Action == nil { + v.Action = func(ctx *cli.Context, setVal string) error { + *dest = setVal + return nil + } + } + } case *cli.PathFlag: v.Category = category if len(v.EnvVars) == 0 { v.EnvVars = []string{nameToEnvVarName(v.Name)} } - case *cli.TimestampFlag: - v.Category = category - if len(v.EnvVars) == 0 { - v.EnvVars = []string{nameToEnvVarName(v.Name)} + if v.Destination != nil { + dest := v.Destination + v.Destination = nil + + if v.Action == nil { + v.Action = func(ctx *cli.Context, setVal string) error { + *dest = setVal + return nil + } + } } } } diff --git a/config/config.go b/config/config.go index 58f31406..e5550e71 100644 --- a/config/config.go +++ b/config/config.go @@ -30,34 +30,34 @@ import ( type Config struct { ID string `toml:"node_id"` UserProvidedID bool - Secret string `toml:"secret"` - BroadcastKey string `toml:"broadcast_key"` - SkipAuth bool `toml:"noauth"` - PublicMode bool `toml:"public"` - BroadcastAdapters []string `toml:"broadcast_adapters"` - PubSubAdapter string `toml:"pubsub_adapter"` - UserPresets []string `toml:"presets"` - Log logger.Config - Server server.Config - App node.Config - WS ws.Config - RPC rpc.Config - Broker broker.Config - Redis rconfig.RedisConfig - LegacyRedisBroadcast broadcast.LegacyRedisConfig - RedisBroadcast broadcast.RedisConfig - NATSBroadcast broadcast.LegacyNATSConfig - HTTPBroadcast broadcast.HTTPConfig - RedisPubSub pubsub.RedisConfig - NATSPubSub pubsub.NATSConfig - NATS nconfig.NATSConfig + Secret string `toml:"secret"` + BroadcastKey string `toml:"broadcast_key"` + SkipAuth bool `toml:"noauth"` + PublicMode bool `toml:"public"` + BroadcastAdapters []string `toml:"broadcast_adapters"` + PubSubAdapter string `toml:"pubsub_adapter"` + UserPresets []string `toml:"presets"` + Log logger.Config `toml:"logging"` + Server server.Config `toml:"server"` + App node.Config `toml:"app"` + WS ws.Config `toml:"ws"` + RPC rpc.Config `toml:"rpc"` + Broker broker.Config `toml:"broker"` + Redis rconfig.RedisConfig `toml:"redis"` + LegacyRedisBroadcast broadcast.LegacyRedisConfig `toml:"redis_pubsub_broadcast"` + RedisBroadcast broadcast.RedisConfig `toml:"redis_stream_broadcast"` + NATSBroadcast broadcast.LegacyNATSConfig `toml:"nats_broadcast"` + HTTPBroadcast broadcast.HTTPConfig `toml:"http_broadcast"` + RedisPubSub pubsub.RedisConfig `toml:"redis_pubsub"` + NATSPubSub pubsub.NATSConfig `toml:"nats_pubsub"` + NATS nconfig.NATSConfig `toml:"nats"` DisconnectorDisabled bool - DisconnectQueue node.DisconnectQueueConfig - Metrics metrics.Config - JWT identity.JWTConfig - EmbeddedNats enats.Config - SSE sse.Config - Streams streams.Config + DisconnectQueue node.DisconnectQueueConfig `toml:"disconnector"` + Metrics metrics.Config `toml:"metrics"` + JWT identity.JWTConfig `toml:"jwt"` + EmbeddedNats enats.Config `toml:"embedded_nats"` + SSE sse.Config `toml:"sse"` + Streams streams.Config `toml:"streams"` ConfigFilePath string } diff --git a/features/file_config.testfile b/features/file_config.testfile index 22018542..438976b7 100644 --- a/features/file_config.testfile +++ b/features/file_config.testfile @@ -1,6 +1,6 @@ # Generate a configuration file with provided values run :anycable_gen_config, - ["sh", "-c", './dist/anycable-go --noauth --port 2024 --broadcast_adapter=http,redisx --sse --metrics_tags=env:production,node_id:xyz --statsd_tags_format=datadog --print-config > ./anycable.toml'], + ["sh", "-c", './dist/anycable-go --debug --noauth --port 2024 --broadcast_adapter=http,redisx --sse --metrics_tags=env:production,node_id:xyz --statsd_tags_format=datadog --print-config > ./anycable.toml'], env: {"ANYCABLE_SECRET" => "file-secret", "ANYCABLE_NODE_ID" => "node-1", "ANYCABLE_STATSD_HOST" => "localhost:8125"}, clean_env: true unless File.exist?("anycable.toml") @@ -24,6 +24,7 @@ assert_equal("node ID", "node-1", config["node_id"]) assert_equal("noauth", true, config["noauth"]) assert_equal("secret", "file-secret", config["secret"]) assert_equal("broadcast adapters", %w[http redisx], config["broadcast_adapters"]) +assert_equal("debug", true, config.dig("logging", "debug")) # nested params assert_equal("server.port", 2024, config.dig("server", "port")) @@ -43,12 +44,16 @@ config = PerfectTOML.parse(stdout(:anycable_ignore_config_path)) $errors.clear assert_equal("node ID", nil, config["node_id"]) -assert_equal("secret", nil, config["secret"]) +assert_equal("secret", "none", config["secret"]) + +if $errors.any? + fail $errors.join("\n") +end # Overriding config data run :anycable_override_config, "dist/anycable-go --port=2025 --statsd_tags_format=influx --print-config", clean_env: true, - env: {"ANYCABLE_NODE_ID" => "node-2", "ANYCABLE_SECRET" => "override"} + env: {"ANYCABLE_NODE_ID" => "node-2", "ANYCABLE_PORT" => "312", "ANYCABLE_SECRET" => "override", "ANYCABLE_DEBUG" => "false"} config = PerfectTOML.parse(stdout(:anycable_override_config)) @@ -66,3 +71,8 @@ assert_equal("sse.enabled", true, config.dig("sse", "enabled")) assert_equal("metrics.tags", {"env" => "production", "node_id" => "xyz"}, config.dig("metrics", "tags")) assert_equal("metrics.statsd.host", "localhost:8125", config.dig("metrics", "statsd", "host")) assert_equal("metrics.statsd.tags_format", "influx", config.dig("metrics", "statsd", "tags_format")) +assert_equal("debug", nil, config.dig("logging", "debug")) + +if $errors.any? + fail $errors.join("\n") +end