From 2f1e2a16e989a9423d0cc357767b664e28d86166 Mon Sep 17 00:00:00 2001 From: Alex Schneider Date: Wed, 1 Dec 2021 13:58:18 +0100 Subject: [PATCH 1/6] Validate for unique keys in all HCL map attributes --- config/configload/load.go | 2 +- config/configload/schema.go | 64 ++++++++++++++++++++++++++++--------- 2 files changed, 50 insertions(+), 16 deletions(-) diff --git a/config/configload/load.go b/config/configload/load.go index f8176c92d..b19152c1d 100644 --- a/config/configload/load.go +++ b/config/configload/load.go @@ -80,7 +80,7 @@ func LoadBytes(src []byte, filename string) (*config.Couper, error) { } func LoadConfig(body hcl.Body, src []byte, filename string) (*config.Couper, error) { - if diags := ValidateConfigSchema(body, &config.Couper{}); diags.HasErrors() { + if diags := ValidateConfigSchema(body, &config.Couper{}, src); diags.HasErrors() { return nil, diags } diff --git a/config/configload/schema.go b/config/configload/schema.go index e1d45d578..535e8b16f 100644 --- a/config/configload/schema.go +++ b/config/configload/schema.go @@ -1,12 +1,15 @@ package configload import ( + "fmt" "reflect" "regexp" + "strings" "github.com/avenga/couper/config" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/gohcl" + "github.com/hashicorp/hcl/v2/hclsyntax" ) const ( @@ -17,17 +20,18 @@ const ( var ( reFetchUnsupportedName = regexp.MustCompile(`\"([^"]+)\"`) - reFetchLabeledName = regexp.MustCompile(`All (.*) blocks must have .* labels \(.*\).`) - reFetchUnlabeledName = regexp.MustCompile(`No labels are expected for (.*) blocks.`) - reFetchUnexpectedArg = regexp.MustCompile(`An argument named (.*) is not expected here.`) + reFetchLabeledName = regexp.MustCompile(`All (.*) blocks must have .* labels \(.*\)\.`) + reFetchUnlabeledName = regexp.MustCompile(`No labels are expected for (.*) blocks\.`) + reFetchUnexpectedArg = regexp.MustCompile(`An argument named (.*) is not expected here\.`) + reFetchUniqueKey = regexp.MustCompile(`Key must be unique for (.*)\.`) ) -func ValidateConfigSchema(body hcl.Body, obj interface{}) hcl.Diagnostics { - attrs, blocks, diags := getSchemaComponents(body, obj) +func ValidateConfigSchema(body hcl.Body, obj interface{}, src []byte) hcl.Diagnostics { + attrs, blocks, diags := getSchemaComponents(body, obj, src) diags = filterValidErrors(attrs, blocks, diags) for _, block := range blocks { - diags = diags.Extend(checkObjectFields(block, obj)) + diags = diags.Extend(checkObjectFields(block, obj, src)) } return uniqueErrors(diags) @@ -55,6 +59,10 @@ func filterValidErrors(attrs hcl.Attributes, blocks hcl.Blocks, diags hcl.Diagno errors = errors.Append(err) continue } + if match := reFetchUniqueKey.MatchString(err.Detail); match { + errors = errors.Append(err) + continue + } errors = errors.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, @@ -83,7 +91,7 @@ func filterValidErrors(attrs hcl.Attributes, blocks hcl.Blocks, diags hcl.Diagno return errors } -func checkObjectFields(block *hcl.Block, obj interface{}) hcl.Diagnostics { +func checkObjectFields(block *hcl.Block, obj interface{}, src []byte) hcl.Diagnostics { var errors hcl.Diagnostics var checked bool @@ -102,7 +110,7 @@ func checkObjectFields(block *hcl.Block, obj interface{}) hcl.Diagnostics { if field.Anonymous { o := reflect.New(field.Type).Interface() - errors = errors.Extend(checkObjectFields(block, o)) + errors = errors.Extend(checkObjectFields(block, o, src)) continue } @@ -121,7 +129,7 @@ func checkObjectFields(block *hcl.Block, obj interface{}) hcl.Diagnostics { if field.Type.Kind() == reflect.Ptr { o := reflect.New(field.Type.Elem()).Interface() - errors = errors.Extend(ValidateConfigSchema(block.Body, o)) + errors = errors.Extend(ValidateConfigSchema(block.Body, o, src)) continue } else if field.Type.Kind() == reflect.Slice { @@ -152,7 +160,7 @@ func checkObjectFields(block *hcl.Block, obj interface{}) hcl.Diagnostics { } o := reflect.New(elem).Interface() - errors = errors.Extend(ValidateConfigSchema(block.Body, o)) + errors = errors.Extend(ValidateConfigSchema(block.Body, o, src)) continue } @@ -166,14 +174,14 @@ func checkObjectFields(block *hcl.Block, obj interface{}) hcl.Diagnostics { if !checked { if i, ok := obj.(config.Inline); ok { - errors = errors.Extend(checkObjectFields(block, i.Inline())) + errors = errors.Extend(checkObjectFields(block, i.Inline(), src)) } } return errors } -func getSchemaComponents(body hcl.Body, obj interface{}) (hcl.Attributes, hcl.Blocks, hcl.Diagnostics) { +func getSchemaComponents(body hcl.Body, obj interface{}, src []byte) (hcl.Attributes, hcl.Blocks, hcl.Diagnostics) { var ( attrs = make(hcl.Attributes) blocks hcl.Blocks @@ -196,17 +204,17 @@ func getSchemaComponents(body hcl.Body, obj interface{}) (hcl.Attributes, hcl.Bl schema = config.WithErrorHandlerSchema(schema) } - attrs, blocks, errors = completeSchemaComponents(body, schema, attrs, blocks, errors) + attrs, blocks, errors = completeSchemaComponents(body, schema, attrs, blocks, errors, src) if i, ok := obj.(config.Inline); ok { - attrs, blocks, errors = completeSchemaComponents(body, i.Schema(true), attrs, blocks, errors) + attrs, blocks, errors = completeSchemaComponents(body, i.Schema(true), attrs, blocks, errors, src) } return attrs, blocks, errors } func completeSchemaComponents(body hcl.Body, schema *hcl.BodySchema, attrs hcl.Attributes, - blocks hcl.Blocks, errors hcl.Diagnostics) (hcl.Attributes, hcl.Blocks, hcl.Diagnostics) { + blocks hcl.Blocks, errors hcl.Diagnostics, src []byte) (hcl.Attributes, hcl.Blocks, hcl.Diagnostics) { content, diags := body.Content(schema) @@ -235,6 +243,32 @@ func completeSchemaComponents(body hcl.Body, schema *hcl.BodySchema, attrs hcl.A if content != nil { for name, attr := range content.Attributes { + if expr, ok := attr.Expr.(*hclsyntax.ObjectConsExpr); ok { + unique := make(map[string]hcl.Range) + + for _, item := range expr.Items { + keyRange := item.KeyExpr.Range() + if keyRange.CanSliceBytes(src) { + key := keyRange.SliceBytes(src) + lwrKey := strings.ToLower(string(key)) + + if previous, exist := unique[lwrKey]; exist { + errors = errors.Append(&hcl.Diagnostic{ + Subject: &keyRange, + Severity: hcl.DiagError, + Summary: fmt. + Sprintf("key must be unique: '%s' was previously defined at: %s", + lwrKey, + previous.String()), + Detail: "Key must be unique for " + string(key) + ".", + }) + } + + unique[lwrKey] = keyRange + } + } + } + attrs[name] = attr } From 186e1d18329fc1be74cfdf2f949db0d188cce216 Mon Sep 17 00:00:00 2001 From: Alex Schneider Date: Wed, 1 Dec 2021 13:58:47 +0100 Subject: [PATCH 2/6] Fix test --- eval/context_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/eval/context_test.go b/eval/context_test.go index 17d6b5a34..d97591703 100644 --- a/eval/context_test.go +++ b/eval/context_test.go @@ -143,7 +143,6 @@ func TestDefaultEnvVariables(t *testing.T) { defaults { environment_variables = { ORIGIN = "FOO" - TIMEOUT = "41" TIMEOUT = "42" IGNORED = "bar" } From 28e60821561ab1ea69cb704e4efa13ff3189d415 Mon Sep 17 00:00:00 2001 From: Alex Schneider Date: Wed, 1 Dec 2021 14:28:48 +0100 Subject: [PATCH 3/6] Remove unused var --- server/http_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/http_test.go b/server/http_test.go index b94dd5689..fac6b9ab4 100644 --- a/server/http_test.go +++ b/server/http_test.go @@ -361,7 +361,7 @@ server "zipzip" { // Trigger panic req.Header.Set("x-close", "dont") - res, err = newClient().Do(req) + _, err = newClient().Do(req) helper.Must(err) for _, entry := range loghook.AllEntries() { From b418c42c61f257ccddeac31cf099d57db9392a37 Mon Sep 17 00:00:00 2001 From: Alex Schneider Date: Wed, 1 Dec 2021 15:13:12 +0100 Subject: [PATCH 4/6] Cleanup code --- config/configload/load.go | 41 +++----------------------- config/configload/validate.go | 55 ----------------------------------- 2 files changed, 4 insertions(+), 92 deletions(-) diff --git a/config/configload/load.go b/config/configload/load.go index b19152c1d..b162dbdd3 100644 --- a/config/configload/load.go +++ b/config/configload/load.go @@ -39,7 +39,6 @@ const ( var regexProxyRequestLabel = regexp.MustCompile(`^[a-zA-Z0-9_]+$`) var envContext *hcl.EvalContext -var configBytes []byte func init() { envContext = eval.NewContext(nil, nil).HCLContext() @@ -104,8 +103,6 @@ func LoadConfig(body hcl.Body, src []byte, filename string) (*config.Couper, err Settings: &defaults, } - configBytes = src[:] - schema, _ := gohcl.ImpliedBodySchema(couperConfig) content, diags := body.Content(schema) if content == nil { @@ -115,6 +112,7 @@ func LoadConfig(body hcl.Body, src []byte, filename string) (*config.Couper, err // Read possible reference definitions first. Those are the // base for refinement merges during server block read out. var definedBackends Backends + var err error for _, outerBlock := range content.Blocks { switch outerBlock.Type { @@ -136,9 +134,6 @@ func LoadConfig(body hcl.Body, src []byte, filename string) (*config.Couper, err }} } - if err := uniqueAttributeKey(be.Body); err != nil { - return nil, err - } definedBackends = append(definedBackends, NewBackend(name, be.Body)) } } @@ -148,11 +143,6 @@ func LoadConfig(body hcl.Body, src []byte, filename string) (*config.Couper, err } for _, oauth2Config := range couperConfig.Definitions.OAuth2AC { - err := uniqueAttributeKey(oauth2Config.Remain) - if err != nil { - return nil, err - } - bodyContent, _, diags := oauth2Config.HCLBody().PartialContent(oauth2Config.Schema(true)) if diags.HasErrors() { return nil, diags @@ -166,11 +156,6 @@ func LoadConfig(body hcl.Body, src []byte, filename string) (*config.Couper, err } for _, oidcConfig := range couperConfig.Definitions.OIDC { - err := uniqueAttributeKey(oidcConfig.Remain) - if err != nil { - return nil, err - } - bodyContent, _, diags := oidcConfig.HCLBody().PartialContent(oidcConfig.Schema(true)) if diags.HasErrors() { return nil, diags @@ -184,11 +169,6 @@ func LoadConfig(body hcl.Body, src []byte, filename string) (*config.Couper, err } for _, jwtConfig := range couperConfig.Definitions.JWT { - err := uniqueAttributeKey(jwtConfig.Remain) - if err != nil { - return nil, err - } - if jwtConfig.JWKsURL != "" { bodyContent, _, diags := jwtConfig.HCLBody().PartialContent(jwtConfig.Schema(true)) if diags.HasErrors() { @@ -436,11 +416,9 @@ func getBackendReference(definedBackends Backends, be config.BackendReference) ( } func refineEndpoints(definedBackends Backends, endpoints config.Endpoints, check bool) error { - for _, endpoint := range endpoints { - if err := uniqueAttributeKey(endpoint.Remain); err != nil { - return err - } + var err error + for _, endpoint := range endpoints { if check && endpoint.Pattern == "" { var r hcl.Range if endpoint.Remain != nil { @@ -501,11 +479,6 @@ func refineEndpoints(definedBackends Backends, endpoints config.Endpoints, check proxyConfig.Remain = proxyBlock.Body - err := uniqueAttributeKey(proxyConfig.Remain) - if err != nil { - return err - } - proxyConfig.Backend, err = newBackend(definedBackends, proxyConfig) if err != nil { return err @@ -542,11 +515,6 @@ func refineEndpoints(definedBackends Backends, endpoints config.Endpoints, check reqConfig.Remain = MergeBodies([]hcl.Body{leftOvers, hclbody.New(content)}) - err := uniqueAttributeKey(reqConfig.Remain) - if err != nil { - return err - } - reqConfig.Backend, err = newBackend(definedBackends, reqConfig) if err != nil { return err @@ -754,8 +722,7 @@ func newBackend(definedBackends Backends, inlineConfig config.Inline) (hcl.Body, bend = MergeBodies([]hcl.Body{bend, wrapped}) } - diags := uniqueAttributeKey(bend) - return bend, diags + return bend, nil } func createCatchAllEndpoint() *config.Endpoint { diff --git a/config/configload/validate.go b/config/configload/validate.go index adc7254f3..888d86c58 100644 --- a/config/configload/validate.go +++ b/config/configload/validate.go @@ -2,65 +2,10 @@ package configload import ( "fmt" - "strings" - "github.com/hashicorp/hcl/v2/hclsyntax" - - "github.com/avenga/couper/config/meta" "github.com/hashicorp/hcl/v2" ) -func uniqueAttributeKey(body hcl.Body) error { - if body == nil { - return nil - } - - content, _, diags := body.PartialContent(meta.AttributesSchema) - if diags.HasErrors() { - return diags - } - - if content == nil || len(content.Attributes) == 0 { - return nil - } - - for _, metaAttr := range meta.AttributesSchema.Attributes { - attr, ok := content.Attributes[metaAttr.Name] - if !strings.HasPrefix(metaAttr.Name, "set_") && !strings.HasPrefix(metaAttr.Name, "add_") || !ok { - continue - } - - expr, ok := attr.Expr.(*hclsyntax.ObjectConsExpr) - if !ok { - continue - } - - unique := make(map[string]hcl.Range) - - for _, item := range expr.Items { - keyRange := item.KeyExpr.Range() - if keyRange.CanSliceBytes(configBytes) { - key := keyRange.SliceBytes(configBytes) - lwrKey := strings.ToLower(string(key)) - if previous, exist := unique[lwrKey]; exist { - return hcl.Diagnostics{ - &hcl.Diagnostic{ - Subject: &keyRange, - Severity: hcl.DiagError, - Summary: fmt. - Sprintf("key must be unique: '%s' was previously defined at: %s", - lwrKey, - previous.String()), - }, - } - } - unique[lwrKey] = keyRange - } - } - } - return nil -} - func validLabelName(name string, hr *hcl.Range) error { if !regexProxyRequestLabel.MatchString(name) { return hcl.Diagnostics{&hcl.Diagnostic{ From 478e30378e49348d201f7e915732c51672f91203 Mon Sep 17 00:00:00 2001 From: Alex Schneider Date: Wed, 1 Dec 2021 15:26:41 +0100 Subject: [PATCH 5/6] Test --- main_test.go | 1 + server/testdata/settings/12_couper.hcl | 10 ++++++++++ 2 files changed, 11 insertions(+) create mode 100644 server/testdata/settings/12_couper.hcl diff --git a/main_test.go b/main_test.go index e7f7463af..a55776f39 100644 --- a/main_test.go +++ b/main_test.go @@ -26,6 +26,7 @@ func Test_realmain(t *testing.T) { }{ {"verify", []string{"couper", "verify", "-f", base + "/10_couper.hcl"}, nil, `10_couper.hcl:2,3-6: Unsupported block type; Blocks of type \"foo\" are not expected here.`, 1}, {"verify w/o server", []string{"couper", "verify", "-f", base + "/11_couper.hcl"}, nil, `configuration error: missing 'server' block"`, 1}, + {"verify unique map-attr keys", []string{"couper", "verify", "-f", base + "/12_couper.hcl"}, nil, `12_couper.hcl:7,7-15: key must be unique: 'test-key' was previously defined at: 12_couper.hcl:6,7-15;`, 1}, {"common log format & info log level /wo file", []string{"couper", "run"}, nil, `level=error msg="failed to load configuration: open couper.hcl: no such file or directory" build=dev`, 1}, {"common log format via env /wo file", []string{"couper", "run", "-log-format", "json"}, []string{"COUPER_LOG_FORMAT=common"}, `level=error msg="failed to load configuration: open couper.hcl: no such file or directory" build=dev`, 1}, {"info log level via env /wo file", []string{"couper", "run", "-log-level", "debug"}, []string{"COUPER_LOG_LEVEL=info"}, `level=error msg="failed to load configuration: open couper.hcl: no such file or directory" build=dev`, 1}, diff --git a/server/testdata/settings/12_couper.hcl b/server/testdata/settings/12_couper.hcl new file mode 100644 index 000000000..374c8694d --- /dev/null +++ b/server/testdata/settings/12_couper.hcl @@ -0,0 +1,10 @@ +server { + files { + document_root = "./" + + set_response_headers = { + test-key = "value" + test-key = "value" + } + } +} From 168c64f73e0d4f1b7b876890e45584d45e9e2840 Mon Sep 17 00:00:00 2001 From: Alex Schneider Date: Wed, 1 Dec 2021 15:30:53 +0100 Subject: [PATCH 6/6] Changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae007c43c..54097122e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ Unreleased changes are available as `avenga/couper:edge` container. * **Changed** * Missing [scope or roles claims](./docs/REFERENCE.md#jwt-block), or scope or roles claim with unsupported values are now ignored instead of causing an error ([#380](https://github.com/avenga/couper/issues/380)) + * Improved the validation for unique keys in all map-attributes in the config ([#403](https://github.com/avenga/couper/pull/403)) * **Fixed** * build-date configuration for binary and docker builds ([#396](https://github.com/avenga/couper/pull/396))