caddyfile: Introduce basic linting and fmt check (#3923)

* caddyfile: Introduce basic linting and fmt check

This will help encourage people to keep their Caddyfiles tidy.

* Remove unrelated tests

I am not sure that testing the output of warnings here is quite the
right idea; these tests are just for syntax and parsing success.
This commit is contained in:
Matt Holt 2021-01-04 11:11:36 -07:00 committed by GitHub
parent 1b453dd4fb
commit c8557dc00b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 40 additions and 54 deletions

View File

@ -15,6 +15,7 @@
package caddyfile
import (
"bytes"
"encoding/json"
"fmt"
@ -51,11 +52,17 @@ func (a Adapter) Adapt(body []byte, options map[string]interface{}) ([]byte, []c
return nil, warnings, err
}
marshalFunc := json.Marshal
if options["pretty"] == "true" {
marshalFunc = caddyconfig.JSONIndent
// 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'",
})
}
result, err := marshalFunc(cfg)
result, err := json.Marshal(cfg)
return result, warnings, err
}

View File

@ -93,12 +93,6 @@ func JSONModuleObject(val interface{}, fieldName, fieldVal string, warnings *[]W
return result
}
// JSONIndent is used to JSON-marshal the final resulting Caddy
// configuration in a consistent, human-readable way.
func JSONIndent(val interface{}) ([]byte, error) {
return json.MarshalIndent(val, "", "\t")
}
// RegisterAdapter registers a config adapter with the given name.
// This should usually be done at init-time. It panics if the
// adapter cannot be registered successfully.

View File

@ -10,7 +10,6 @@ import (
func TestLogDirectiveSyntax(t *testing.T) {
for i, tc := range []struct {
input string
expectWarn bool
expectError bool
}{
{
@ -18,7 +17,6 @@ func TestLogDirectiveSyntax(t *testing.T) {
log
}
`,
expectWarn: false,
expectError: false,
},
{
@ -28,7 +26,6 @@ func TestLogDirectiveSyntax(t *testing.T) {
}
}
`,
expectWarn: false,
expectError: false,
},
{
@ -38,7 +35,6 @@ func TestLogDirectiveSyntax(t *testing.T) {
}
}
`,
expectWarn: false,
expectError: true,
},
} {
@ -47,12 +43,7 @@ func TestLogDirectiveSyntax(t *testing.T) {
ServerType: ServerType{},
}
_, warnings, err := adapter.Adapt([]byte(tc.input), nil)
if len(warnings) > 0 != tc.expectWarn {
t.Errorf("Test %d warning expectation failed Expected: %v, got %v", i, tc.expectWarn, warnings)
continue
}
_, _, err := adapter.Adapt([]byte(tc.input), nil)
if err != nil != tc.expectError {
t.Errorf("Test %d error expectation failed Expected: %v, got %s", i, tc.expectError, err)

View File

@ -9,7 +9,6 @@ import (
func TestMatcherSyntax(t *testing.T) {
for i, tc := range []struct {
input string
expectWarn bool
expectError bool
}{
{
@ -18,7 +17,6 @@ func TestMatcherSyntax(t *testing.T) {
query showdebug=1
}
`,
expectWarn: false,
expectError: false,
},
{
@ -27,7 +25,6 @@ func TestMatcherSyntax(t *testing.T) {
query bad format
}
`,
expectWarn: false,
expectError: true,
},
{
@ -38,7 +35,6 @@ func TestMatcherSyntax(t *testing.T) {
}
}
`,
expectWarn: false,
expectError: false,
},
{
@ -47,14 +43,12 @@ func TestMatcherSyntax(t *testing.T) {
not path /somepath*
}
`,
expectWarn: false,
expectError: false,
},
{
input: `http://localhost
@debug not path /somepath*
`,
expectWarn: false,
expectError: false,
},
{
@ -63,7 +57,6 @@ func TestMatcherSyntax(t *testing.T) {
}
http://localhost
`,
expectWarn: false,
expectError: true,
},
} {
@ -72,12 +65,7 @@ func TestMatcherSyntax(t *testing.T) {
ServerType: ServerType{},
}
_, warnings, err := adapter.Adapt([]byte(tc.input), nil)
if len(warnings) > 0 != tc.expectWarn {
t.Errorf("Test %d warning expectation failed Expected: %v, got %v", i, tc.expectWarn, warnings)
continue
}
_, _, err := adapter.Adapt([]byte(tc.input), nil)
if err != nil != tc.expectError {
t.Errorf("Test %d error expectation failed Expected: %v, got %s", i, tc.expectError, err)
@ -119,7 +107,6 @@ func TestSpecificity(t *testing.T) {
func TestGlobalOptions(t *testing.T) {
for i, tc := range []struct {
input string
expectWarn bool
expectError bool
}{
{
@ -129,7 +116,6 @@ func TestGlobalOptions(t *testing.T) {
}
:80
`,
expectWarn: false,
expectError: false,
},
{
@ -139,7 +125,6 @@ func TestGlobalOptions(t *testing.T) {
}
:80
`,
expectWarn: false,
expectError: false,
},
{
@ -149,7 +134,6 @@ func TestGlobalOptions(t *testing.T) {
}
:80
`,
expectWarn: false,
expectError: false,
},
{
@ -161,7 +145,6 @@ func TestGlobalOptions(t *testing.T) {
}
:80
`,
expectWarn: false,
expectError: true,
},
{
@ -174,7 +157,6 @@ func TestGlobalOptions(t *testing.T) {
}
:80
`,
expectWarn: false,
expectError: false,
},
{
@ -187,7 +169,6 @@ func TestGlobalOptions(t *testing.T) {
}
:80
`,
expectWarn: false,
expectError: false,
},
{
@ -200,7 +181,6 @@ func TestGlobalOptions(t *testing.T) {
}
:80
`,
expectWarn: false,
expectError: true,
},
{
@ -213,7 +193,6 @@ func TestGlobalOptions(t *testing.T) {
}
:80
`,
expectWarn: false,
expectError: true,
},
} {
@ -222,12 +201,7 @@ func TestGlobalOptions(t *testing.T) {
ServerType: ServerType{},
}
_, warnings, err := adapter.Adapt([]byte(tc.input), nil)
if len(warnings) > 0 != tc.expectWarn {
t.Errorf("Test %d warning expectation failed Expected: %v, got %v", i, tc.expectWarn, warnings)
continue
}
_, _, err := adapter.Adapt([]byte(tc.input), nil)
if err != nil != tc.expectError {
t.Errorf("Test %d error expectation failed Expected: %v, got %s", i, tc.expectError, err)

View File

@ -336,7 +336,6 @@ func CompareAdapt(t *testing.T, rawConfig string, adapterName string, expectedRe
}
options := make(map[string]interface{})
options["pretty"] = "true"
result, warnings, err := cfgAdapter.Adapt([]byte(rawConfig), options)
if err != nil {
@ -344,6 +343,14 @@ func CompareAdapt(t *testing.T, rawConfig string, adapterName string, expectedRe
return false
}
// prettify results to keep tests human-manageable
var prettyBuf bytes.Buffer
err = json.Indent(&prettyBuf, result, "", "\t")
if err != nil {
return false
}
result = prettyBuf.Bytes()
if len(warnings) > 0 {
for _, w := range warnings {
t.Logf("warning: directive: %s : %s", w.Directive, w.Message)

View File

@ -463,17 +463,22 @@ func cmdAdaptConfig(fl Flags) (int, error) {
fmt.Errorf("reading input file: %v", err)
}
opts := make(map[string]interface{})
if adaptCmdPrettyFlag {
opts["pretty"] = "true"
}
opts["filename"] = adaptCmdInputFlag
opts := map[string]interface{}{"filename": adaptCmdInputFlag}
adaptedConfig, warnings, err := cfgAdapter.Adapt(input, opts)
if err != nil {
return caddy.ExitCodeFailedStartup, err
}
if adaptCmdPrettyFlag {
var prettyBuf bytes.Buffer
err = json.Indent(&prettyBuf, adaptedConfig, "", "\t")
if err != nil {
return caddy.ExitCodeFailedStartup, err
}
adaptedConfig = prettyBuf.Bytes()
}
// print warnings to stderr
for _, warn := range warnings {
msg := warn.Message

View File

@ -15,6 +15,7 @@
package reverseproxy
import (
"log"
"net"
"net/http"
"net/url"
@ -497,6 +498,13 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
case 1:
err = headers.CaddyfileHeaderOp(h.Headers.Request, args[0], "", "")
case 2:
// some lint checks, I guess
if strings.EqualFold(args[0], "host") && (args[1] == "{hostport}" || args[1] == "{http.request.hostport}") {
log.Printf("[WARNING] Unnecessary header_up ('Host' field): the reverse proxy's default behavior is to pass headers to the upstream")
}
if strings.EqualFold(args[0], "x-forwarded-proto") && (args[1] == "{scheme}" || args[1] == "{http.request.scheme}") {
log.Printf("[WARNING] Unnecessary header_up ('X-Forwarded-Proto' field): the reverse proxy's default behavior is to pass headers to the upstream")
}
err = headers.CaddyfileHeaderOp(h.Headers.Request, args[0], args[1], "")
case 3:
err = headers.CaddyfileHeaderOp(h.Headers.Request, args[0], args[1], args[2])