From 7088605cc11c52c2777ab613dfc5c2a9816006e4 Mon Sep 17 00:00:00 2001 From: Mohammed Al Sahaf Date: Sun, 2 Jun 2024 14:40:56 +0300 Subject: [PATCH] cmd: fix regression in auto-detect of Caddyfile (#6362) * cmd: fix regression in auto-detect of Caddyfile Signed-off-by: Mohammed Al Sahaf * fix typo Co-authored-by: Git'Fellow <12234510+solracsf@users.noreply.github.com> * add tests * address review comments --------- Signed-off-by: Mohammed Al Sahaf Co-authored-by: Git'Fellow <12234510+solracsf@users.noreply.github.com> --- cmd/main.go | 61 ++++++++++++++++++++++----------- cmd/main_test.go | 89 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 130 insertions(+), 20 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 1f7d6156b..6d164d184 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -107,6 +107,44 @@ func LoadConfig(configFile, adapterName string) ([]byte, string, error) { return loadConfigWithLogger(caddy.Log(), configFile, adapterName) } +func isCaddyfile(configFile, adapterName string) (bool, error) { + if adapterName == "caddyfile" { + return true, nil + } + + // as a special case, if a config file starts with "caddyfile" or + // has a ".caddyfile" extension, and no adapter is specified, and + // no adapter module name matches the extension, assume + // caddyfile adapter for convenience + baseConfig := strings.ToLower(filepath.Base(configFile)) + baseConfigExt := filepath.Ext(baseConfig) + startsOrEndsInCaddyfile := strings.HasPrefix(baseConfig, "caddyfile") || strings.HasSuffix(baseConfig, ".caddyfile") + extNotCaddyfileOrJSON := (baseConfigExt != "" && baseConfigExt != ".caddyfile" && baseConfigExt != ".json") + + if baseConfigExt == ".json" { + return false, nil + } + + // If the adapter is not specified, + // the config file starts with "caddyfile", + // the config file has an extension, + // and isn't a JSON file (e.g. Caddyfile.yaml), + // then we don't know what the config format is. + if adapterName == "" && startsOrEndsInCaddyfile && extNotCaddyfileOrJSON { + return false, fmt.Errorf("ambiguous config file format; please specify adapter (use --adapter)") + } + + // If the config file starts or ends with "caddyfile", + // the extension of the config file is not ".json", AND + // the user did not specify an adapter, then we assume it's Caddyfile. + if startsOrEndsInCaddyfile && + baseConfigExt != ".json" && + adapterName == "" { + return true, nil + } + return false, nil +} + func loadConfigWithLogger(logger *zap.Logger, configFile, adapterName string) ([]byte, string, error) { // if no logger is provided, use a nop logger // just so we don't have to check for nil @@ -157,27 +195,10 @@ func loadConfigWithLogger(logger *zap.Logger, configFile, adapterName string) ([ } } - // as a special case, if a config file starts with "caddyfile" or - // has a ".caddyfile" extension, and no adapter is specified, and - // no adapter module name matches the extension, assume - // caddyfile adapter for convenience - baseConfig := strings.ToLower(filepath.Base(configFile)) - baseConfigExt := filepath.Ext(baseConfig) - startsOrEndsInCaddyfile := strings.HasPrefix(baseConfig, "caddyfile") || strings.HasSuffix(baseConfig, ".caddyfile") - - // If the adapter is not specified, the config file is not starts with "caddyfile", and isn't a JSON file (e.g. Caddyfile.yaml), - // then we don't know what the config format is. - if adapterName == "" && startsOrEndsInCaddyfile && baseConfigExt != ".caddyfile" && baseConfigExt != ".json" { - return nil, "", fmt.Errorf("ambiguous config file format; please specify adapter (use --adapter)") - } - - // If the config file starts or ends with "caddyfile", - // the extension of the config file is not ".json", AND - // the user did not specify an adapter, then we assume it's Caddyfile. - if startsOrEndsInCaddyfile && - baseConfigExt != ".json" && - adapterName == "" { + if yes, err := isCaddyfile(configFile, adapterName); yes { adapterName = "caddyfile" + } else if err != nil { + return nil, "", err } // load config adapter diff --git a/cmd/main_test.go b/cmd/main_test.go index 90e8194f6..9816b61e9 100644 --- a/cmd/main_test.go +++ b/cmd/main_test.go @@ -168,3 +168,92 @@ here" } } } + +func Test_isCaddyfile(t *testing.T) { + type args struct { + configFile string + adapterName string + } + tests := []struct { + name string + args args + want bool + wantErr bool + }{ + { + name: "bare Caddyfile without adapter", + args: args{ + configFile: "Caddyfile", + adapterName: "", + }, + want: true, + wantErr: false, + }, + { + name: "local Caddyfile without adapter", + args: args{ + configFile: "./Caddyfile", + adapterName: "", + }, + want: true, + wantErr: false, + }, + { + name: "local caddyfile with adapter", + args: args{ + configFile: "./Caddyfile", + adapterName: "caddyfile", + }, + want: true, + wantErr: false, + }, + { + name: "ends with .caddyfile with adapter", + args: args{ + configFile: "./conf.caddyfile", + adapterName: "caddyfile", + }, + want: true, + wantErr: false, + }, + { + name: "ends with .caddyfile without adapter", + args: args{ + configFile: "./conf.caddyfile", + adapterName: "", + }, + want: true, + wantErr: false, + }, + { + name: "config is Caddyfile.yaml without adapter", + args: args{ + configFile: "./Caddyfile.yaml", + adapterName: "", + }, + want: false, + wantErr: true, + }, + { + name: "json is not caddyfile but not error", + args: args{ + configFile: "./Caddyfile.json", + adapterName: "", + }, + want: false, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := isCaddyfile(tt.args.configFile, tt.args.adapterName) + if (err != nil) != tt.wantErr { + t.Errorf("isCaddyfile() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("isCaddyfile() = %v, want %v", got, tt.want) + } + }) + } +}