From 160d19999982c4facd32c4bddced5a7dc91e8a40 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Tue, 19 Jan 2021 14:21:11 -0700 Subject: [PATCH] caddytest: Update Caddyfile tests for formatting, HTTP-only blocks Previous commit improved the Caddyfile adapter so it doesn't unnecessarily add names to "skip" in "auto_https" when the server is already HTTP-only. This commit updates the tests to reflect that change, while also fixing the Caddyfile formatting in many of the tests. We also print the line number of the divergence between input and formatted version in Caddyfile adapt warnings - very useful for finding initial formatting problems. --- caddyconfig/caddyfile/adapter.go | 36 +++++++-- caddytest/caddytest.go | 6 +- .../caddyfile_adapt/global_options_acme.txt | 2 +- .../global_server_options_multi.txt | 9 +-- .../global_server_options_single.txt | 2 +- .../integration/caddyfile_adapt/header.txt | 2 +- .../caddyfile_adapt/http_only_on_domain.txt | 7 +- .../caddyfile_adapt/matchers_in_route.txt | 12 +-- .../php_fastcgi_subdirectives.txt | 16 ++-- .../caddyfile_adapt/request_body.txt | 3 +- ...reverse_proxy_empty_non_http_transport.txt | 2 +- .../reverse_proxy_h2c_shorthand.txt | 74 +++++++++---------- .../tls_automation_policies_2.txt | 8 +- .../tls_client_auth_cert_file.txt | 2 +- .../tls_client_auth_inline_cert.txt | 4 +- .../tls_conn_policy_consolidate.txt | 7 +- caddytest/integration/caddyfile_adapt_test.go | 5 +- cmd/commandfuncs.go | 6 +- 18 files changed, 103 insertions(+), 100 deletions(-) diff --git a/caddyconfig/caddyfile/adapter.go b/caddyconfig/caddyfile/adapter.go index 185816b8..7f5ebc5f 100644 --- a/caddyconfig/caddyfile/adapter.go +++ b/caddyconfig/caddyfile/adapter.go @@ -53,13 +53,9 @@ func (a Adapter) Adapt(body []byte, options map[string]interface{}) ([]byte, []c } // lint check: see if input was properly formatted; sometimes messy files files parse - // successfully but result in logical errors because the Caddyfile is a bad format - // TODO: also perform this check on imported files - if !bytes.Equal(Format(body), body) { - warnings = append(warnings, caddyconfig.Warning{ - File: filename, - Message: "file is not formatted with 'caddy fmt'", - }) + // successfully but result in logical errors (the Caddyfile is a bad format, I'm sorry) + if warning, different := formattingDifference(filename, body); different { + warnings = append(warnings, warning) } result, err := json.Marshal(cfg) @@ -67,6 +63,32 @@ func (a Adapter) Adapt(body []byte, options map[string]interface{}) ([]byte, []c return result, warnings, err } +// formattingDifference returns a warning and true if the formatted version +// is any different from the input; empty warning and false otherwise. +// TODO: also perform this check on imported files +func formattingDifference(filename string, body []byte) (caddyconfig.Warning, bool) { + formatted := Format(body) + if bytes.Equal(formatted, body) { + return caddyconfig.Warning{}, false + } + + // find where the difference is + line := 1 + for i, ch := range body { + if i >= len(formatted) || ch != formatted[i] { + break + } + if ch == '\n' { + line++ + } + } + return caddyconfig.Warning{ + File: filename, + Line: line, + Message: "input is not formatted with 'caddy fmt'", + }, true +} + // Unmarshaler is a type that can unmarshal // Caddyfile tokens to set itself up for a // JSON encoding. The goal of an unmarshaler diff --git a/caddytest/caddytest.go b/caddytest/caddytest.go index c4c26876..b3896e7a 100644 --- a/caddytest/caddytest.go +++ b/caddytest/caddytest.go @@ -327,7 +327,7 @@ func (tc *Tester) AssertRedirect(requestURI string, expectedToLocation string, e } // CompareAdapt adapts a config and then compares it against an expected result -func CompareAdapt(t *testing.T, rawConfig string, adapterName string, expectedResponse string) bool { +func CompareAdapt(t *testing.T, filename, rawConfig string, adapterName string, expectedResponse string) bool { cfgAdapter := caddyconfig.GetAdapter(adapterName) if cfgAdapter == nil { @@ -353,7 +353,7 @@ func CompareAdapt(t *testing.T, rawConfig string, adapterName string, expectedRe if len(warnings) > 0 { for _, w := range warnings { - t.Logf("warning: directive: %s : %s", w.Directive, w.Message) + t.Logf("warning: %s:%d: %s: %s", filename, w.Line, w.Directive, w.Message) } } @@ -388,7 +388,7 @@ func CompareAdapt(t *testing.T, rawConfig string, adapterName string, expectedRe // AssertAdapt adapts a config and then tests it against an expected result func AssertAdapt(t *testing.T, rawConfig string, adapterName string, expectedResponse string) { - ok := CompareAdapt(t, rawConfig, adapterName, expectedResponse) + ok := CompareAdapt(t, "Caddyfile", rawConfig, adapterName, expectedResponse) if !ok { t.Fail() } diff --git a/caddytest/integration/caddyfile_adapt/global_options_acme.txt b/caddytest/integration/caddyfile_adapt/global_options_acme.txt index 0f1d78fd..fb95588a 100644 --- a/caddytest/integration/caddyfile_adapt/global_options_acme.txt +++ b/caddytest/integration/caddyfile_adapt/global_options_acme.txt @@ -9,7 +9,7 @@ } acme_ca https://example.com acme_eab { - key_id 4K2scIVbBpNd-78scadB2g + key_id 4K2scIVbBpNd-78scadB2g mac_key abcdefghijklmnopqrstuvwx-abcdefghijklnopqrstuvwxyz12ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefgh } acme_ca_root /path/to/ca.crt diff --git a/caddytest/integration/caddyfile_adapt/global_server_options_multi.txt b/caddytest/integration/caddyfile_adapt/global_server_options_multi.txt index 653eee53..90c02e5e 100644 --- a/caddytest/integration/caddyfile_adapt/global_server_options_multi.txt +++ b/caddytest/integration/caddyfile_adapt/global_server_options_multi.txt @@ -16,7 +16,7 @@ } } -foo.com { +foo.com { } http://bar.com { @@ -64,12 +64,7 @@ http://bar.com { ], "terminal": true } - ], - "automatic_https": { - "skip": [ - "bar.com" - ] - } + ] }, "srv2": { "listen": [ diff --git a/caddytest/integration/caddyfile_adapt/global_server_options_single.txt b/caddytest/integration/caddyfile_adapt/global_server_options_single.txt index 5a5c64cd..d81d0323 100644 --- a/caddytest/integration/caddyfile_adapt/global_server_options_single.txt +++ b/caddytest/integration/caddyfile_adapt/global_server_options_single.txt @@ -18,7 +18,7 @@ } } -foo.com { +foo.com { } ---------- diff --git a/caddytest/integration/caddyfile_adapt/header.txt b/caddytest/integration/caddyfile_adapt/header.txt index 19a5b41a..223839ef 100644 --- a/caddytest/integration/caddyfile_adapt/header.txt +++ b/caddytest/integration/caddyfile_adapt/header.txt @@ -4,7 +4,7 @@ header ?John "von Neumann" header -Wolfram header { - Grace: "Hopper" # some users habitually suffix field names with a colon + Grace: "Hopper" # some users habitually suffix field names with a colon +Ray "Solomonoff" ?Tim "Berners-Lee" defer diff --git a/caddytest/integration/caddyfile_adapt/http_only_on_domain.txt b/caddytest/integration/caddyfile_adapt/http_only_on_domain.txt index 83e77598..d2792423 100644 --- a/caddytest/integration/caddyfile_adapt/http_only_on_domain.txt +++ b/caddytest/integration/caddyfile_adapt/http_only_on_domain.txt @@ -46,12 +46,7 @@ http://a.caddy.localhost { ], "terminal": true } - ], - "automatic_https": { - "skip": [ - "a.caddy.localhost" - ] - } + ] } } } diff --git a/caddytest/integration/caddyfile_adapt/matchers_in_route.txt b/caddytest/integration/caddyfile_adapt/matchers_in_route.txt index 01609b03..8c587b59 100644 --- a/caddytest/integration/caddyfile_adapt/matchers_in_route.txt +++ b/caddytest/integration/caddyfile_adapt/matchers_in_route.txt @@ -1,10 +1,10 @@ :80 { - route { - # unused matchers should not panic - # see https://github.com/caddyserver/caddy/issues/3745 - @matcher1 path /path1 - @matcher2 path /path2 - } + route { + # unused matchers should not panic + # see https://github.com/caddyserver/caddy/issues/3745 + @matcher1 path /path1 + @matcher2 path /path2 + } } ---------- { diff --git a/caddytest/integration/caddyfile_adapt/php_fastcgi_subdirectives.txt b/caddytest/integration/caddyfile_adapt/php_fastcgi_subdirectives.txt index 6733a9e2..90d1633b 100644 --- a/caddytest/integration/caddyfile_adapt/php_fastcgi_subdirectives.txt +++ b/caddytest/integration/caddyfile_adapt/php_fastcgi_subdirectives.txt @@ -1,15 +1,15 @@ :8884 php_fastcgi localhost:9000 { - # some php_fastcgi-specific subdirectives - split .php .php5 - env VAR1 value1 - env VAR2 value2 - root /var/www - index index.php5 + # some php_fastcgi-specific subdirectives + split .php .php5 + env VAR1 value1 + env VAR2 value2 + root /var/www + index index.php5 - # passed through to reverse_proxy (directive order doesn't matter!) - lb_policy random + # passed through to reverse_proxy (directive order doesn't matter!) + lb_policy random } ---------- { diff --git a/caddytest/integration/caddyfile_adapt/request_body.txt b/caddytest/integration/caddyfile_adapt/request_body.txt index 458b7392..1e4fd471 100644 --- a/caddytest/integration/caddyfile_adapt/request_body.txt +++ b/caddytest/integration/caddyfile_adapt/request_body.txt @@ -1,6 +1,7 @@ localhost + request_body { - max_size 1MB + max_size 1MB } ---------- { diff --git a/caddytest/integration/caddyfile_adapt/reverse_proxy_empty_non_http_transport.txt b/caddytest/integration/caddyfile_adapt/reverse_proxy_empty_non_http_transport.txt index b2618033..bcbe29b6 100644 --- a/caddytest/integration/caddyfile_adapt/reverse_proxy_empty_non_http_transport.txt +++ b/caddytest/integration/caddyfile_adapt/reverse_proxy_empty_non_http_transport.txt @@ -1,7 +1,7 @@ :8884 reverse_proxy 127.0.0.1:65535 { - transport fastcgi + transport fastcgi } ---------- { diff --git a/caddytest/integration/caddyfile_adapt/reverse_proxy_h2c_shorthand.txt b/caddytest/integration/caddyfile_adapt/reverse_proxy_h2c_shorthand.txt index 94f22086..75ce9607 100644 --- a/caddytest/integration/caddyfile_adapt/reverse_proxy_h2c_shorthand.txt +++ b/caddytest/integration/caddyfile_adapt/reverse_proxy_h2c_shorthand.txt @@ -1,38 +1,38 @@ -:8884 - -reverse_proxy h2c://localhost:8080 ----------- -{ - "apps": { - "http": { - "servers": { - "srv0": { - "listen": [ - ":8884" - ], - "routes": [ - { - "handle": [ - { - "handler": "reverse_proxy", - "transport": { - "protocol": "http", - "versions": [ - "h2c", - "2" - ] - }, - "upstreams": [ - { - "dial": "localhost:8080" - } - ] - } - ] - } - ] - } - } - } - } +:8884 + +reverse_proxy h2c://localhost:8080 +---------- +{ + "apps": { + "http": { + "servers": { + "srv0": { + "listen": [ + ":8884" + ], + "routes": [ + { + "handle": [ + { + "handler": "reverse_proxy", + "transport": { + "protocol": "http", + "versions": [ + "h2c", + "2" + ] + }, + "upstreams": [ + { + "dial": "localhost:8080" + } + ] + } + ] + } + ] + } + } + } + } } \ No newline at end of file diff --git a/caddytest/integration/caddyfile_adapt/tls_automation_policies_2.txt b/caddytest/integration/caddyfile_adapt/tls_automation_policies_2.txt index 9be0a19a..17196ec0 100644 --- a/caddytest/integration/caddyfile_adapt/tls_automation_policies_2.txt +++ b/caddytest/integration/caddyfile_adapt/tls_automation_policies_2.txt @@ -1,5 +1,4 @@ # issue #3953 - { cert_issuer zerossl api_key } @@ -58,12 +57,7 @@ http://example.net { ], "terminal": true } - ], - "automatic_https": { - "skip": [ - "example.net" - ] - } + ] } } }, diff --git a/caddytest/integration/caddyfile_adapt/tls_client_auth_cert_file.txt b/caddytest/integration/caddyfile_adapt/tls_client_auth_cert_file.txt index 1e68a4df..aaa5abff 100644 --- a/caddytest/integration/caddyfile_adapt/tls_client_auth_cert_file.txt +++ b/caddytest/integration/caddyfile_adapt/tls_client_auth_cert_file.txt @@ -3,7 +3,7 @@ localhost respond "hello from localhost" tls { client_auth { - mode request + mode request trusted_ca_cert_file ../caddy.ca.cer } } diff --git a/caddytest/integration/caddyfile_adapt/tls_client_auth_inline_cert.txt b/caddytest/integration/caddyfile_adapt/tls_client_auth_inline_cert.txt index 028d3b19..4cd45813 100644 --- a/caddytest/integration/caddyfile_adapt/tls_client_auth_inline_cert.txt +++ b/caddytest/integration/caddyfile_adapt/tls_client_auth_inline_cert.txt @@ -3,8 +3,8 @@ localhost respond "hello from localhost" tls { client_auth { - mode request - trusted_ca_cert MIIDSzCCAjOgAwIBAgIUfIRObjWNUA4jxQ/0x8BOCvE2Vw4wDQYJKoZIhvcNAQELBQAwFjEUMBIGA1UEAwwLRWFzeS1SU0EgQ0EwHhcNMTkwODI4MTYyNTU5WhcNMjkwODI1MTYyNTU5WjAWMRQwEgYDVQQDDAtFYXN5LVJTQSBDQTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAK5m5elxhQfMp/3aVJ4JnpN9PUSz6LlP6LePAPFU7gqohVVFVtDkChJAG3FNkNQNlieVTja/bgH9IcC6oKbROwdY1h0MvNV8AHHigvl03WuJD8g2ReVFXXwsnrPmKXCFzQyMI6TYk3m2gYrXsZOU1GLnfMRC3KAMRgE2F45twOs9hqG169YJ6mM2eQjzjCHWI6S2/iUYvYxRkCOlYUbLsMD/AhgAf1plzg6LPqNxtdlwxZnA0ytgkmhK67HtzJu0+ovUCsMv0RwcMhsEo9T8nyFAGt9XLZ63X5WpBCTUApaAUhnG0XnerjmUWb6eUWw4zev54sEfY5F3x002iQaW6cECAwEAAaOBkDCBjTAdBgNVHQ4EFgQU4CBUbZsS2GaNIkGRz/cBsD5ivjswUQYDVR0jBEowSIAU4CBUbZsS2GaNIkGRz/cBsD5ivjuhGqQYMBYxFDASBgNVBAMMC0Vhc3ktUlNBIENBghR8hE5uNY1QDiPFD/THwE4K8TZXDjAMBgNVHRMEBTADAQH/MAsGA1UdDwQEAwIBBjANBgkqhkiG9w0BAQsFAAOCAQEAKB3V4HIzoiO/Ch6WMj9bLJ2FGbpkMrcb/Eq01hT5zcfKD66lVS1MlK+cRL446Z2b2KDP1oFyVs+qmrmtdwrWgD+nfe2sBmmIHo9m9KygMkEOfG3MghGTEcS+0cTKEcoHYWYyOqQh6jnedXY8Cdm4GM1hAc9MiL3/sqV8YCVSLNnkoNysmr06/rZ0MCUZPGUtRmfd0heWhrfzAKw2HLgX+RAmpOE2MZqWcjvqKGyaRiaZks4nJkP6521aC2Lgp0HhCz1j8/uQ5ldoDszCnu/iro0NAsNtudTMD+YoLQxLqdleIh6CW+illc2VdXwj7mn6J04yns9jfE2jRjW/yTLFuQ== + mode request + trusted_ca_cert MIIDSzCCAjOgAwIBAgIUfIRObjWNUA4jxQ/0x8BOCvE2Vw4wDQYJKoZIhvcNAQELBQAwFjEUMBIGA1UEAwwLRWFzeS1SU0EgQ0EwHhcNMTkwODI4MTYyNTU5WhcNMjkwODI1MTYyNTU5WjAWMRQwEgYDVQQDDAtFYXN5LVJTQSBDQTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAK5m5elxhQfMp/3aVJ4JnpN9PUSz6LlP6LePAPFU7gqohVVFVtDkChJAG3FNkNQNlieVTja/bgH9IcC6oKbROwdY1h0MvNV8AHHigvl03WuJD8g2ReVFXXwsnrPmKXCFzQyMI6TYk3m2gYrXsZOU1GLnfMRC3KAMRgE2F45twOs9hqG169YJ6mM2eQjzjCHWI6S2/iUYvYxRkCOlYUbLsMD/AhgAf1plzg6LPqNxtdlwxZnA0ytgkmhK67HtzJu0+ovUCsMv0RwcMhsEo9T8nyFAGt9XLZ63X5WpBCTUApaAUhnG0XnerjmUWb6eUWw4zev54sEfY5F3x002iQaW6cECAwEAAaOBkDCBjTAdBgNVHQ4EFgQU4CBUbZsS2GaNIkGRz/cBsD5ivjswUQYDVR0jBEowSIAU4CBUbZsS2GaNIkGRz/cBsD5ivjuhGqQYMBYxFDASBgNVBAMMC0Vhc3ktUlNBIENBghR8hE5uNY1QDiPFD/THwE4K8TZXDjAMBgNVHRMEBTADAQH/MAsGA1UdDwQEAwIBBjANBgkqhkiG9w0BAQsFAAOCAQEAKB3V4HIzoiO/Ch6WMj9bLJ2FGbpkMrcb/Eq01hT5zcfKD66lVS1MlK+cRL446Z2b2KDP1oFyVs+qmrmtdwrWgD+nfe2sBmmIHo9m9KygMkEOfG3MghGTEcS+0cTKEcoHYWYyOqQh6jnedXY8Cdm4GM1hAc9MiL3/sqV8YCVSLNnkoNysmr06/rZ0MCUZPGUtRmfd0heWhrfzAKw2HLgX+RAmpOE2MZqWcjvqKGyaRiaZks4nJkP6521aC2Lgp0HhCz1j8/uQ5ldoDszCnu/iro0NAsNtudTMD+YoLQxLqdleIh6CW+illc2VdXwj7mn6J04yns9jfE2jRjW/yTLFuQ== } } ---------- diff --git a/caddytest/integration/caddyfile_adapt/tls_conn_policy_consolidate.txt b/caddytest/integration/caddyfile_adapt/tls_conn_policy_consolidate.txt index ba6827e0..68e89b0d 100644 --- a/caddytest/integration/caddyfile_adapt/tls_conn_policy_consolidate.txt +++ b/caddytest/integration/caddyfile_adapt/tls_conn_policy_consolidate.txt @@ -75,12 +75,7 @@ http://b.b https://b.b:8443 { ], "terminal": true } - ], - "automatic_https": { - "skip": [ - "b.b" - ] - } + ] }, "srv2": { "listen": [ diff --git a/caddytest/integration/caddyfile_adapt_test.go b/caddytest/integration/caddyfile_adapt_test.go index bfe22ada..8794fe93 100644 --- a/caddytest/integration/caddyfile_adapt_test.go +++ b/caddytest/integration/caddyfile_adapt_test.go @@ -32,14 +32,15 @@ func TestCaddyfileAdaptToJSON(t *testing.T) { } // split the Caddyfile (first) and JSON (second) parts + // (append newline to Caddyfile to match formatter expectations) parts := strings.Split(string(data), "----------") - caddyfile, json := strings.TrimSpace(parts[0]), strings.TrimSpace(parts[1]) + caddyfile, json := strings.TrimSpace(parts[0])+"\n", strings.TrimSpace(parts[1]) // replace windows newlines in the json with unix newlines json = winNewlines.ReplaceAllString(json, "\n") // run the test - ok := caddytest.CompareAdapt(t, caddyfile, "caddyfile", json) + ok := caddytest.CompareAdapt(t, filename, caddyfile, "caddyfile", json) if !ok { t.Errorf("failed to adapt %s", filename) } diff --git a/cmd/commandfuncs.go b/cmd/commandfuncs.go index bf24c240..3bf4b8df 100644 --- a/cmd/commandfuncs.go +++ b/cmd/commandfuncs.go @@ -529,6 +529,9 @@ func cmdAdaptConfig(fl Flags) (int, error) { adaptedConfig = prettyBuf.Bytes() } + // print result to stdout + fmt.Println(string(adaptedConfig)) + // print warnings to stderr for _, warn := range warnings { msg := warn.Message @@ -538,9 +541,6 @@ func cmdAdaptConfig(fl Flags) (int, error) { fmt.Fprintf(os.Stderr, "[WARNING][%s] %s:%d: %s\n", adaptCmdAdapterFlag, warn.File, warn.Line, msg) } - // print result to stdout - fmt.Println(string(adaptedConfig)) - // validate output if requested if adaptCmdValidateFlag { var cfg *caddy.Config