Skip to content

Commit 3fa4d59

Browse files
committed
fix(cli): make --config reach doctor too
Review finding from Copilot on PR #776. Declaring the flag globally fixed the rejection for daemon, validate and config-show, but doctor was still constructed without the pre-parsed value, so `ofelia --config=x doctor` was accepted and then disregarded: it reported on /etc/ofelia/config.ini and told the user the file was missing. Accepting a flag and ignoring it is worse than rejecting it — nothing says the path was dropped. doctor is also the reason the default cannot simply be handed to every command: given no path it searches well-known locations, and a pre-filled default would take that away silently. The default is therefore resolved in run() for the commands that need a concrete file, while doctor keeps the raw value, so an absent flag still means 'go and find it'. Verified against the binary in all four combinations: doctor with the flag before and after the subcommand uses the given file, doctor without it still auto-detects (./ofelia.ini), and validate is unchanged. Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
1 parent 4706fea commit 3fa4d59

2 files changed

Lines changed: 61 additions & 6 deletions

File tree

e2e/cli_exit_codes_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,3 +186,43 @@ func TestE2E_GlobalConfigFlagIsHonoured(t *testing.T) {
186186
missing, stdout, stderr)
187187
}
188188
}
189+
190+
// TestE2E_GlobalConfigFlagReachesDoctor covers the subcommand that made the
191+
// first version of this fix incomplete.
192+
//
193+
// doctor searches well-known locations when it is given no path, so it is the
194+
// one command that must NOT receive a pre-filled default — and it was
195+
// therefore also the one constructed without the pre-parsed value at all, so
196+
// `ofelia --config=x doctor` was accepted and then ignored, reporting on
197+
// /etc/ofelia/config.ini instead. Accepting a flag and disregarding it is
198+
// worse than rejecting it: nothing tells the user their path was dropped.
199+
func TestE2E_GlobalConfigFlagReachesDoctor(t *testing.T) {
200+
t.Parallel()
201+
202+
configPath := writeConfig(t, `[global]
203+
log-level = info
204+
205+
[job-local "hello"]
206+
schedule = @every 1h
207+
command = echo hi
208+
`)
209+
210+
for _, args := range [][]string{
211+
{"--config=" + configPath, "doctor"},
212+
{"doctor", "--config=" + configPath},
213+
} {
214+
t.Run(strings.Join(args, " "), func(t *testing.T) {
215+
t.Parallel()
216+
217+
stdout, stderr, _ := runCommand(t, args...)
218+
out := stdout + stderr
219+
220+
if !strings.Contains(out, configPath) {
221+
t.Errorf("doctor did not report on the config it was given (%s):\n%s", configPath, out)
222+
}
223+
if strings.Contains(out, "Config file not found: /etc/ofelia/config.ini") {
224+
t.Errorf("doctor fell back to the default despite an explicit --config:\n%s", out)
225+
}
226+
})
227+
}
228+
}

ofelia.go

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,17 @@ func main() {
6565
// position a user would naturally write them.
6666
type globalOptions struct {
6767
LogLevel string `long:"log-level" description:"Set log level (overrides config)"`
68-
ConfigFile string `long:"config" description:"Configuration file path" default:"/etc/ofelia/config.ini"`
68+
ConfigFile string `long:"config" description:"Configuration file path (default: /etc/ofelia/config.ini)"`
6969
}
7070

71+
// defaultConfigFile is where ofelia looks when --config is absent.
72+
//
73+
// It is resolved here rather than declared as a struct-tag default so that an
74+
// unset flag stays distinguishable from an explicit one: `doctor` searches a
75+
// list of well-known locations when given no path, and a pre-filled default
76+
// would silently take that away.
77+
const defaultConfigFile = "/etc/ofelia/config.ini"
78+
7179
// run holds what main used to do and returns the process exit code instead of
7280
// ending the process, so the exit status is a value tests can assert on.
7381
func run(args []string) int {
@@ -85,8 +93,15 @@ func run(args []string) int {
8593
preParser := flags.NewParser(&pre, flags.IgnoreUnknown)
8694
_, _ = preParser.ParseArgs(args)
8795

96+
// Commands that read a config need a concrete path; doctor is handed the
97+
// raw value so an absent flag still means "go and find it".
98+
configFile := pre.ConfigFile
99+
if configFile == "" {
100+
configFile = defaultConfigFile
101+
}
102+
88103
if pre.LogLevel == "" {
89-
cfg, err := ini.LoadSources(ini.LoadOptions{AllowShadows: true, InsensitiveKeys: true}, pre.ConfigFile)
104+
cfg, err := ini.LoadSources(ini.LoadOptions{AllowShadows: true, InsensitiveKeys: true}, configFile)
90105
if err == nil {
91106
if sec, err := cfg.GetSection("global"); err == nil {
92107
pre.LogLevel = cli.ExpandEnvVars(sec.Key("log-level").String())
@@ -109,19 +124,19 @@ func run(args []string) int {
109124
"daemon",
110125
"daemon process",
111126
"",
112-
&cli.DaemonCommand{Logger: logger, LevelVar: levelVar, LogLevel: pre.LogLevel, ConfigFile: pre.ConfigFile},
127+
&cli.DaemonCommand{Logger: logger, LevelVar: levelVar, LogLevel: pre.LogLevel, ConfigFile: configFile},
113128
)
114129
_, _ = parser.AddCommand(
115130
"validate",
116131
"validates the config file",
117132
"",
118-
&cli.ValidateCommand{Logger: logger, LevelVar: levelVar, LogLevel: pre.LogLevel, ConfigFile: pre.ConfigFile},
133+
&cli.ValidateCommand{Logger: logger, LevelVar: levelVar, LogLevel: pre.LogLevel, ConfigFile: configFile},
119134
)
120135
_, _ = parser.AddCommand(
121136
"config",
122137
"shows the effective runtime configuration",
123138
"",
124-
&cli.ConfigShowCommand{Logger: logger, LevelVar: levelVar, LogLevel: pre.LogLevel, ConfigFile: pre.ConfigFile},
139+
&cli.ConfigShowCommand{Logger: logger, LevelVar: levelVar, LogLevel: pre.LogLevel, ConfigFile: configFile},
125140
)
126141
_, _ = parser.AddCommand(
127142
"init",
@@ -133,7 +148,7 @@ func run(args []string) int {
133148
"doctor",
134149
"diagnose Ofelia configuration and environment health",
135150
"",
136-
&cli.DoctorCommand{Logger: logger, LevelVar: levelVar, LogLevel: pre.LogLevel},
151+
&cli.DoctorCommand{Logger: logger, LevelVar: levelVar, LogLevel: pre.LogLevel, ConfigFile: pre.ConfigFile},
137152
)
138153
_, _ = parser.AddCommand(
139154
"hash-password",

0 commit comments

Comments
 (0)