cmd: fix regression in auto-detect of Caddyfile (#6362)

* cmd: fix regression in auto-detect of Caddyfile Signed-off-by: Mohammed Al Sahaf <[email protected]> * fix typo Co-authored-by: Git'Fellow <[email protected]> * add tests * address review comments --------- Signed-off-by: Mohammed Al Sahaf <[email protected]> Co-authored-by: Git'Fellow <[email protected]>
caddyserver · Jun 2, 2024 · 7088605 · 7088605
1 parent 15faeac
commit 7088605
Show file tree

Hide file tree

Showing 2 changed files with 130 additions and 20 deletions.
diff --git 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
@@ -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)
+			}
+		})
+	}
+}