From 16edbf2728c91038ae94c8ecf2808de5cac0d49e Mon Sep 17 00:00:00 2001 From: Alex Schneider Date: Mon, 22 Aug 2022 13:21:25 +0200 Subject: [PATCH 01/17] Docu --- .../content/2.configuration/4.block/definitions.md | 2 +- docs/website/content/2.configuration/4.block/endpoint.md | 6 ++++++ docs/website/content/2.configuration/4.block/proxy.md | 9 ++++++++- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/docs/website/content/2.configuration/4.block/definitions.md b/docs/website/content/2.configuration/4.block/definitions.md index 6a56f957a..1544a3ef0 100644 --- a/docs/website/content/2.configuration/4.block/definitions.md +++ b/docs/website/content/2.configuration/4.block/definitions.md @@ -6,4 +6,4 @@ Use the `definitions` block to define configurations you want to reuse. | Block name | Context | Label | Nested block(s) | |:--------------|:--------|:---------|:---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| `definitions` | - | no label | [Backend Block(s)](backend), [Basic Auth Block(s)](basic_auth), [JWT Block(s)](jwt), [JWT Signing Profile Block(s)](jwt_signing_profile), [SAML Block(s)](saml), [OAuth2 AC Block(s)](oauth2), [OIDC Block(s)](oidc) | +| `definitions` | - | no label | [Backend Block(s)](backend), [Basic Auth Block(s)](basic_auth), [JWT Block(s)](jwt), [JWT Signing Profile Block(s)](jwt_signing_profile), [SAML Block(s)](saml), [OAuth2 AC Block(s)](oauth2), [OIDC Block(s)](oidc), [Proxy Block(s)](proxy) | diff --git a/docs/website/content/2.configuration/4.block/endpoint.md b/docs/website/content/2.configuration/4.block/endpoint.md index 101927b2c..ca82968e7 100644 --- a/docs/website/content/2.configuration/4.block/endpoint.md +++ b/docs/website/content/2.configuration/4.block/endpoint.md @@ -100,6 +100,12 @@ values: [ "default": "", "description": "Location of the error file template." }, + { + "name": "proxy", + "type": "string", + "default": "", + "description": "proxy block reference" + }, { "name": "remove_form_params", "type": "object", diff --git a/docs/website/content/2.configuration/4.block/proxy.md b/docs/website/content/2.configuration/4.block/proxy.md index b51714603..d1dbcafdc 100644 --- a/docs/website/content/2.configuration/4.block/proxy.md +++ b/docs/website/content/2.configuration/4.block/proxy.md @@ -6,8 +6,9 @@ The `proxy` block creates and executes a proxy request to a backend service. | Block name | Context | Label | Nested block(s) | |:-----------|:----------------------------------|:-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| `proxy` | [Endpoint Block](endpoint) | ⚠ A `proxy` block or [Request Block](request) w/o a label has an implicit label `"default"`. Only **one** `proxy` block or [Request Block](request) w/ label `"default"` per [Endpoint Block](endpoint) is allowed. | [Backend Block](backend) (⚠ required, if no [Backend Block](backend) reference is defined or no `url` attribute is set.), [Websockets Block](websockets) (⚠ Either websockets attribute or block is allowed.) | +| `proxy` | [Endpoint Block](endpoint) | See `Label` description below. | [Backend Block](backend) (⚠ required, if no [Backend Block](backend) reference is defined or no `url` attribute is set.), [Websockets Block](websockets) (⚠ Either websockets attribute or block is allowed.) | +**Label:** If defined in an [Endpoint Block](endpoint), a `proxy` block or [Request Block](request) w/o a label has an implicit name `"default"`. If defined in the [Definitions Block](definitions), the label of `proxy` is used as reference in [Endpoint Blocks](endpoint) and the name can be defined via `name` attribute. Only **one** `proxy` block or [Request Block](request) w/ label `"default"` per [Endpoint Block](endpoint) is allowed. ::attributes --- @@ -48,6 +49,12 @@ values: [ "default": "[]", "description": "If defined, the response status code will be verified against this list of codes. If the status code not included in this list an `unexpected_status` error will be thrown which can be handled with an [`error_handler`](error_handler)." }, + { + "name": "name", + "type": "string", + "default": "default", + "description": "Defines the proxy request name. Allowed only in the [Definitions Block](definitions)." + }, { "name": "remove_form_params", "type": "object", From 05587c8903f3f6339702f917c8da77f395446b71 Mon Sep 17 00:00:00 2001 From: Alex Schneider Date: Mon, 22 Aug 2022 13:21:41 +0200 Subject: [PATCH 02/17] Tests --- server/http_endpoints_test.go | 45 +++++++++++++++++++++++++ server/testdata/endpoints/18_couper.hcl | 26 ++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 server/testdata/endpoints/18_couper.hcl diff --git a/server/http_endpoints_test.go b/server/http_endpoints_test.go index b3041c998..0eb3e658c 100644 --- a/server/http_endpoints_test.go +++ b/server/http_endpoints_test.go @@ -961,3 +961,48 @@ func TestEndpointACBufferOptions(t *testing.T) { }) } } + +func TestEndpoint_ReusableProxies(t *testing.T) { + client := test.NewHTTPClient() + helper := test.New(t) + + shutdown, hook := newCouper(filepath.Join(testdataPath, "18_couper.hcl"), helper) + defer shutdown() + + type testCase struct { + path string + name string + expStatus int + } + + for _, tc := range []testCase{ + {"/abcdef", "abcdef", 204}, + {"/default", "default", 204}, + } { + t.Run(tc.path, func(st *testing.T) { + h := test.New(st) + + req, err := http.NewRequest(http.MethodGet, "http://example.com:8080"+tc.path, nil) + h.Must(err) + + hook.Reset() + + res, err := client.Do(req) + h.Must(err) + + if res.StatusCode != tc.expStatus { + st.Errorf("want: %d, got: %d", tc.expStatus, res.StatusCode) + } + + for _, e := range hook.AllEntries() { + if e.Data["type"] != "couper_backend" { + continue + } + + if name := e.Data["request"].(logging.Fields)["name"]; name != tc.name { + st.Errorf("want: %s, got: %s", tc.name, name) + } + } + }) + } +} diff --git a/server/testdata/endpoints/18_couper.hcl b/server/testdata/endpoints/18_couper.hcl new file mode 100644 index 000000000..ddfdfdc59 --- /dev/null +++ b/server/testdata/endpoints/18_couper.hcl @@ -0,0 +1,26 @@ +server "couper" { + endpoint "/abcdef" { + proxy = "test" + response { + status = 204 + } + } + + endpoint "/default" { + proxy = "defaultName" + response { + status = 204 + } + } +} + +definitions { + proxy "defaultName" { + url = "${env.COUPER_TEST_BACKEND_ADDR}/anything" + } + + proxy "test" { + name = "abcdef" + url = "${env.COUPER_TEST_BACKEND_ADDR}/anything" + } +} From 5138ef53b9bcc202880575b5812b892b43f8cf0f Mon Sep 17 00:00:00 2001 From: Alex Schneider Date: Mon, 22 Aug 2022 13:23:38 +0200 Subject: [PATCH 03/17] More tests --- server/http_endpoints_test.go | 1 + server/testdata/endpoints/18_couper.hcl | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/server/http_endpoints_test.go b/server/http_endpoints_test.go index 0eb3e658c..86ab50b4b 100644 --- a/server/http_endpoints_test.go +++ b/server/http_endpoints_test.go @@ -977,6 +977,7 @@ func TestEndpoint_ReusableProxies(t *testing.T) { for _, tc := range []testCase{ {"/abcdef", "abcdef", 204}, + {"/reuse", "abcdef", 204}, {"/default", "default", 204}, } { t.Run(tc.path, func(st *testing.T) { diff --git a/server/testdata/endpoints/18_couper.hcl b/server/testdata/endpoints/18_couper.hcl index ddfdfdc59..cf01a439b 100644 --- a/server/testdata/endpoints/18_couper.hcl +++ b/server/testdata/endpoints/18_couper.hcl @@ -5,6 +5,12 @@ server "couper" { status = 204 } } + endpoint "/reuse" { + proxy = "test" + response { + status = 204 + } + } endpoint "/default" { proxy = "defaultName" From b6ca1d5a6f7803e39ec48b5f163f7b8b6ab331d4 Mon Sep 17 00:00:00 2001 From: Alex Schneider Date: Mon, 22 Aug 2022 16:31:26 +0200 Subject: [PATCH 04/17] Implementation of reusable proxy blocks --- config/configload/load.go | 4 +-- config/configload/merge.go | 65 ++++++++++++++++++++++++++++++++------ 2 files changed, 57 insertions(+), 12 deletions(-) diff --git a/config/configload/load.go b/config/configload/load.go index cb4b98c9f..58f289284 100644 --- a/config/configload/load.go +++ b/config/configload/load.go @@ -181,12 +181,12 @@ func LoadFiles(filesList []string, env string) (*config.Couper, error) { settingsBlock := mergeSettings(parsedBodies) - definitionsBlock, err := mergeDefinitions(parsedBodies) + definitionsBlock, proxies, err := mergeDefinitions(parsedBodies) if err != nil { return nil, err } - serverBlocks, err := mergeServers(parsedBodies) + serverBlocks, err := mergeServers(parsedBodies, proxies) if err != nil { return nil, err } diff --git a/config/configload/merge.go b/config/configload/merge.go index abc92260c..9dc06a9a3 100644 --- a/config/configload/merge.go +++ b/config/configload/merge.go @@ -14,11 +14,12 @@ import ( ) const ( - errMultipleBackends = "Multiple definitions of backend are not allowed." - errUniqueLabels = "All %s blocks must have unique labels." + errMissingReferencedProxy = "The %s references a non-defined proxy block." + errMultipleBackends = "Multiple definitions of backend are not allowed." + errUniqueLabels = "All %s blocks must have unique labels." ) -func mergeServers(bodies []*hclsyntax.Body) (hclsyntax.Blocks, error) { +func mergeServers(bodies []*hclsyntax.Body, proxies map[string]*hclsyntax.Block) (hclsyntax.Blocks, error) { type ( namedBlocks map[string]*hclsyntax.Block apiDefinition struct { @@ -151,6 +152,21 @@ func mergeServers(bodies []*hclsyntax.Body) (hclsyntax.Blocks, error) { return nil, newMergeError(errUniqueLabels, block) } + if attr, ok := block.Body.Attributes[proxy]; ok { + reference, err := attrStringValue(attr) + if err != nil { + return nil, err + } + + delete(block.Body.Attributes, proxy) + + if proxyBlock, ok := proxies[reference]; !ok { + return nil, newMergeError(errMissingReferencedProxy, block) + } else { + block.Body.Blocks = append(block.Body.Blocks, proxyBlock) + } + } + results[serverKey].endpoints[block.Labels[0]] = block } else if block.Type == api { var apiKey string @@ -401,11 +417,12 @@ func mergeServers(bodies []*hclsyntax.Body) (hclsyntax.Blocks, error) { return mergedServers, nil } -func mergeDefinitions(bodies []*hclsyntax.Body) (*hclsyntax.Block, error) { +func mergeDefinitions(bodies []*hclsyntax.Body) (*hclsyntax.Block, map[string]*hclsyntax.Block, error) { type data map[string]*hclsyntax.Block type list map[string]data definitionsBlock := make(list) + proxiesList := make(data) for _, body := range bodies { for _, outerBlock := range body.Blocks { @@ -416,11 +433,9 @@ func mergeDefinitions(bodies []*hclsyntax.Body) (*hclsyntax.Block, error) { } if len(innerBlock.Labels) == 0 { - return nil, newMergeError(errUniqueLabels, innerBlock) + return nil, nil, newMergeError(errUniqueLabels, innerBlock) } - definitionsBlock[innerBlock.Type][innerBlock.Labels[0]] = innerBlock - // Count the "backend" blocks and "backend" attributes to // forbid multiple backend definitions. @@ -429,7 +444,7 @@ func mergeDefinitions(bodies []*hclsyntax.Body) (*hclsyntax.Block, error) { for _, block := range innerBlock.Body.Blocks { if block.Type == errorHandler { if err := absInBackends(block); err != nil { - return nil, err + return nil, nil, err } } else if block.Type == backend { backends++ @@ -441,7 +456,28 @@ func mergeDefinitions(bodies []*hclsyntax.Body) (*hclsyntax.Block, error) { } if backends > 1 { - return nil, newMergeError(errMultipleBackends, innerBlock) + return nil, nil, newMergeError(errMultipleBackends, innerBlock) + } + + if innerBlock.Type != proxy { + definitionsBlock[innerBlock.Type][innerBlock.Labels[0]] = innerBlock + } else { + label := innerBlock.Labels[0] + + if attr, ok := innerBlock.Body.Attributes["name"]; ok { + name, err := attrStringValue(attr) + if err != nil { + return nil, nil, err + } + + innerBlock.Labels[0] = name + + delete(innerBlock.Body.Attributes, "name") + } else { + innerBlock.Labels[0] = defaultNameLabel + } + + proxiesList[label] = innerBlock } } } @@ -461,7 +497,7 @@ func mergeDefinitions(bodies []*hclsyntax.Body) (*hclsyntax.Block, error) { Body: &hclsyntax.Body{ Blocks: blocks, }, - }, nil + }, proxiesList, nil } func mergeDefaults(bodies []*hclsyntax.Body) (*hclsyntax.Block, error) { @@ -592,6 +628,15 @@ func absInBackends(block *hclsyntax.Block) error { return nil } +func attrStringValue(attr *hclsyntax.Attribute) (string, error) { + v, err := eval.Value(nil, attr.Expr) + if err != nil { + return "", err + } + + return v.AsString(), nil +} + // newErrorHandlerKey returns a merge key based on a possible mixed error-kind format. // "label1" and/or "label2 label3" results in "label1 label2 label3". func newErrorHandlerKey(block *hclsyntax.Block) (key string) { From ab8b059f69961546255abe0933a877184ca83ad3 Mon Sep 17 00:00:00 2001 From: Alex Schneider Date: Mon, 22 Aug 2022 16:52:20 +0200 Subject: [PATCH 05/17] Changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cb87f8238..964267c50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ Unreleased changes are available as `avenga/couper:edge` container. * used go version in `version` command ([#552](https://github.com/avenga/couper/pull/552)) * new `grant_type`s `"password"` and `"urn:ietf:params:oauth:grant-type:jwt-bearer"` with related attributes for [`oauth2` block](https://docs.couper.io/configuration/block/oauth2) ([#555](https://github.com/avenga/couper/pull/555)) * [`beta_token_request` block](https://docs.couper.io/configuration/block/token_request), [`backend`](https://docs.couper.io/configuration/variables#backend) and [`beta_token_response`](https://docs.couper.io/configuration/variables#beta_token_response) variables and `beta_token(s)` properties of [`backends` variable](https://docs.couper.io/configuration/variables#backends) ([#517](https://github.com/avenga/couper/pull/517)) + * reusable [`proxy` block](https://docs.couper.io/configuration/block/proxy) ([#561](https://github.com/avenga/couper/pull/561)) * **Changed** * Starting will now fail if `environment` blocks are used without `COUPER_ENVIRONMENT` being set ([#546](https://github.com/avenga/couper/pull/546)) From fa71c6874c215562ba6f1331220a2aa039c08fbf Mon Sep 17 00:00:00 2001 From: Alex Schneider Date: Fri, 26 Aug 2022 10:09:49 +0200 Subject: [PATCH 06/17] Reusable proxies in api-endpoints, too --- config/configload/merge.go | 15 +++++++++++++++ server/http_endpoints_test.go | 3 +++ server/testdata/endpoints/18_couper.hcl | 22 ++++++++++++++++++++++ 3 files changed, 40 insertions(+) diff --git a/config/configload/merge.go b/config/configload/merge.go index 9dc06a9a3..ccf6b3abc 100644 --- a/config/configload/merge.go +++ b/config/configload/merge.go @@ -214,6 +214,21 @@ func mergeServers(bodies []*hclsyntax.Body, proxies map[string]*hclsyntax.Block) return nil, newMergeError(errUniqueLabels, subBlock) } + if attr, ok := subBlock.Body.Attributes[proxy]; ok { + reference, err := attrStringValue(attr) + if err != nil { + return nil, err + } + + delete(subBlock.Body.Attributes, proxy) + + if proxyBlock, ok := proxies[reference]; !ok { + return nil, newMergeError(errMissingReferencedProxy, subBlock) + } else { + subBlock.Body.Blocks = append(subBlock.Body.Blocks, proxyBlock) + } + } + results[serverKey].apis[apiKey].endpoints[subBlock.Labels[0]] = subBlock } else if subBlock.Type == errorHandler { if err := absInBackends(subBlock); err != nil { diff --git a/server/http_endpoints_test.go b/server/http_endpoints_test.go index 86ab50b4b..97eb925f2 100644 --- a/server/http_endpoints_test.go +++ b/server/http_endpoints_test.go @@ -979,6 +979,9 @@ func TestEndpoint_ReusableProxies(t *testing.T) { {"/abcdef", "abcdef", 204}, {"/reuse", "abcdef", 204}, {"/default", "default", 204}, + {"/api-abcdef", "abcdef", 204}, + {"/api-reuse", "abcdef", 204}, + {"/api-default", "default", 204}, } { t.Run(tc.path, func(st *testing.T) { h := test.New(st) diff --git a/server/testdata/endpoints/18_couper.hcl b/server/testdata/endpoints/18_couper.hcl index cf01a439b..cabc9a91b 100644 --- a/server/testdata/endpoints/18_couper.hcl +++ b/server/testdata/endpoints/18_couper.hcl @@ -18,6 +18,28 @@ server "couper" { status = 204 } } + + api { + endpoint "/api-abcdef" { + proxy = "test" + response { + status = 204 + } + } + endpoint "/api-reuse" { + proxy = "test" + response { + status = 204 + } + } + + endpoint "/api-default" { + proxy = "defaultName" + response { + status = 204 + } + } + } } definitions { From 46577d7f536e1870673a9b2e914a31bb82a3ae28 Mon Sep 17 00:00:00 2001 From: Alex Schneider Date: Fri, 26 Aug 2022 11:16:20 +0200 Subject: [PATCH 07/17] DRY --- config/configload/merge.go | 49 ++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/config/configload/merge.go b/config/configload/merge.go index ccf6b3abc..286f4e743 100644 --- a/config/configload/merge.go +++ b/config/configload/merge.go @@ -152,19 +152,8 @@ func mergeServers(bodies []*hclsyntax.Body, proxies map[string]*hclsyntax.Block) return nil, newMergeError(errUniqueLabels, block) } - if attr, ok := block.Body.Attributes[proxy]; ok { - reference, err := attrStringValue(attr) - if err != nil { - return nil, err - } - - delete(block.Body.Attributes, proxy) - - if proxyBlock, ok := proxies[reference]; !ok { - return nil, newMergeError(errMissingReferencedProxy, block) - } else { - block.Body.Blocks = append(block.Body.Blocks, proxyBlock) - } + if err := addProxy(block, proxies); err != nil { + return nil, err } results[serverKey].endpoints[block.Labels[0]] = block @@ -214,19 +203,8 @@ func mergeServers(bodies []*hclsyntax.Body, proxies map[string]*hclsyntax.Block) return nil, newMergeError(errUniqueLabels, subBlock) } - if attr, ok := subBlock.Body.Attributes[proxy]; ok { - reference, err := attrStringValue(attr) - if err != nil { - return nil, err - } - - delete(subBlock.Body.Attributes, proxy) - - if proxyBlock, ok := proxies[reference]; !ok { - return nil, newMergeError(errMissingReferencedProxy, subBlock) - } else { - subBlock.Body.Blocks = append(subBlock.Body.Blocks, proxyBlock) - } + if err := addProxy(subBlock, proxies); err != nil { + return nil, err } results[serverKey].apis[apiKey].endpoints[subBlock.Labels[0]] = subBlock @@ -643,6 +621,25 @@ func absInBackends(block *hclsyntax.Block) error { return nil } +func addProxy(block *hclsyntax.Block, proxies map[string]*hclsyntax.Block) error { + if attr, ok := block.Body.Attributes[proxy]; ok { + reference, err := attrStringValue(attr) + if err != nil { + return err + } + + delete(block.Body.Attributes, proxy) + + if proxyBlock, ok := proxies[reference]; !ok { + return newMergeError(errMissingReferencedProxy, block) + } else { + block.Body.Blocks = append(block.Body.Blocks, proxyBlock) + } + } + + return nil +} + func attrStringValue(attr *hclsyntax.Attribute) (string, error) { v, err := eval.Value(nil, attr.Expr) if err != nil { From 406951e2121650b2a8df4bcaa67c94f3d016c825 Mon Sep 17 00:00:00 2001 From: Alex Schneider Date: Fri, 26 Aug 2022 11:16:33 +0200 Subject: [PATCH 08/17] Simplify tests --- server/http_endpoints_test.go | 4 ++-- server/testdata/endpoints/18_couper.hcl | 6 ------ 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/server/http_endpoints_test.go b/server/http_endpoints_test.go index 97eb925f2..99c7e8614 100644 --- a/server/http_endpoints_test.go +++ b/server/http_endpoints_test.go @@ -978,10 +978,10 @@ func TestEndpoint_ReusableProxies(t *testing.T) { for _, tc := range []testCase{ {"/abcdef", "abcdef", 204}, {"/reuse", "abcdef", 204}, - {"/default", "default", 204}, + {"/default", "default", 200}, {"/api-abcdef", "abcdef", 204}, {"/api-reuse", "abcdef", 204}, - {"/api-default", "default", 204}, + {"/api-default", "default", 200}, } { t.Run(tc.path, func(st *testing.T) { h := test.New(st) diff --git a/server/testdata/endpoints/18_couper.hcl b/server/testdata/endpoints/18_couper.hcl index cabc9a91b..6f9cde3a9 100644 --- a/server/testdata/endpoints/18_couper.hcl +++ b/server/testdata/endpoints/18_couper.hcl @@ -14,9 +14,6 @@ server "couper" { endpoint "/default" { proxy = "defaultName" - response { - status = 204 - } } api { @@ -35,9 +32,6 @@ server "couper" { endpoint "/api-default" { proxy = "defaultName" - response { - status = 204 - } } } } From 13bce5d5e1fa527ea80151152786c86821561cf0 Mon Sep 17 00:00:00 2001 From: Alex Schneider Date: Wed, 31 Aug 2022 11:13:28 +0200 Subject: [PATCH 09/17] Reference the attribute in the error message --- config/configload/merge.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/config/configload/merge.go b/config/configload/merge.go index 286f4e743..9a1813f39 100644 --- a/config/configload/merge.go +++ b/config/configload/merge.go @@ -14,9 +14,8 @@ import ( ) const ( - errMissingReferencedProxy = "The %s references a non-defined proxy block." - errMultipleBackends = "Multiple definitions of backend are not allowed." - errUniqueLabels = "All %s blocks must have unique labels." + errMultipleBackends = "Multiple definitions of backend are not allowed." + errUniqueLabels = "All %s blocks must have unique labels." ) func mergeServers(bodies []*hclsyntax.Body, proxies map[string]*hclsyntax.Block) (hclsyntax.Blocks, error) { @@ -631,7 +630,9 @@ func addProxy(block *hclsyntax.Block, proxies map[string]*hclsyntax.Block) error delete(block.Body.Attributes, proxy) if proxyBlock, ok := proxies[reference]; !ok { - return newMergeError(errMissingReferencedProxy, block) + sr := attr.Expr.StartRange() + + return newDiagErr(&sr, "proxy reference is not defined") } else { block.Body.Blocks = append(block.Body.Blocks, proxyBlock) } From 586f2d5630e7c9a0b696494cbd485653d00e93ba Mon Sep 17 00:00:00 2001 From: Alex Schneider Date: Wed, 31 Aug 2022 11:22:25 +0200 Subject: [PATCH 10/17] Check for string type Co-authored-by: Johannes Koch <53434855+johakoch@users.noreply.github.com> --- config/configload/merge.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/config/configload/merge.go b/config/configload/merge.go index 286f4e743..c160b98d7 100644 --- a/config/configload/merge.go +++ b/config/configload/merge.go @@ -646,6 +646,11 @@ func attrStringValue(attr *hclsyntax.Attribute) (string, error) { return "", err } + if v.Type() != cty.String { + sr := attr.Expr.StartRange() + return "", newDiagErr(&sr, fmt.Sprintf("%s must evaluate to string", attr.Name)) + } + return v.AsString(), nil } From 9c34313ee09f70f2ac56e8e1b83aac1bcdfe8aeb Mon Sep 17 00:00:00 2001 From: Alex Schneider Date: Wed, 31 Aug 2022 11:27:16 +0200 Subject: [PATCH 11/17] No need to delete before throwing error. --- config/configload/merge.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/configload/merge.go b/config/configload/merge.go index 2da08893e..b24c5fb7a 100644 --- a/config/configload/merge.go +++ b/config/configload/merge.go @@ -627,13 +627,13 @@ func addProxy(block *hclsyntax.Block, proxies map[string]*hclsyntax.Block) error return err } - delete(block.Body.Attributes, proxy) - if proxyBlock, ok := proxies[reference]; !ok { sr := attr.Expr.StartRange() return newDiagErr(&sr, "proxy reference is not defined") } else { + delete(block.Body.Attributes, proxy) + block.Body.Blocks = append(block.Body.Blocks, proxyBlock) } } From cb11db27c699c4bf6773fd41d3bad8da0aa96c22 Mon Sep 17 00:00:00 2001 From: Alex Schneider Date: Wed, 31 Aug 2022 11:36:43 +0200 Subject: [PATCH 12/17] Test non-string proxy reference --- main_test.go | 1 + server/testdata/settings/19_couper.hcl | 5 +++++ 2 files changed, 6 insertions(+) create mode 100644 server/testdata/settings/19_couper.hcl diff --git a/main_test.go b/main_test.go index 3d50ee252..0ebecd8f9 100644 --- a/main_test.go +++ b/main_test.go @@ -52,6 +52,7 @@ func Test_realmain(t *testing.T) { {"invalid method in allowed_methods in endpoint", []string{"couper", "run", "-f", base + "/14_couper.hcl"}, nil, `level=error msg="%s/14_couper.hcl:3,5-35: method contains invalid character(s); " build=dev`, 1}, {"invalid method in allowed_methods in api", []string{"couper", "run", "-f", base + "/15_couper.hcl"}, nil, `level=error msg="%s/15_couper.hcl:3,5-35: method contains invalid character(s); " build=dev`, 1}, {"rate_limit block in anonymous backend", []string{"couper", "run", "-f", base + "/17_couper.hcl"}, nil, `level=error msg="configuration error: anonymous_3_11: anonymous backend (\"anonymous_3_11\") cannot define 'beta_rate_limit' block(s)" build=dev`, 1}, + {"non-string proxy reference", []string{"couper", "run", "-f", base + "/19_couper.hcl"}, nil, `level=error msg="%s/19_couper.hcl:3,13-14: proxy must evaluate to string; " build=dev`, 1}, } for _, tt := range tests { t.Run(tt.name, func(subT *testing.T) { diff --git a/server/testdata/settings/19_couper.hcl b/server/testdata/settings/19_couper.hcl new file mode 100644 index 000000000..fa263b956 --- /dev/null +++ b/server/testdata/settings/19_couper.hcl @@ -0,0 +1,5 @@ +server { + endpoint "/" { + proxy = 1 + } +} From fded88efbb4219bfc535f006df4a3c5e2ac2fd2b Mon Sep 17 00:00:00 2001 From: Alex Schneider Date: Wed, 31 Aug 2022 13:38:49 +0200 Subject: [PATCH 13/17] Test if a proxy reference does not exist --- main_test.go | 1 + server/testdata/settings/20_couper.hcl | 5 +++++ 2 files changed, 6 insertions(+) create mode 100644 server/testdata/settings/20_couper.hcl diff --git a/main_test.go b/main_test.go index 0ebecd8f9..30a838d41 100644 --- a/main_test.go +++ b/main_test.go @@ -53,6 +53,7 @@ func Test_realmain(t *testing.T) { {"invalid method in allowed_methods in api", []string{"couper", "run", "-f", base + "/15_couper.hcl"}, nil, `level=error msg="%s/15_couper.hcl:3,5-35: method contains invalid character(s); " build=dev`, 1}, {"rate_limit block in anonymous backend", []string{"couper", "run", "-f", base + "/17_couper.hcl"}, nil, `level=error msg="configuration error: anonymous_3_11: anonymous backend (\"anonymous_3_11\") cannot define 'beta_rate_limit' block(s)" build=dev`, 1}, {"non-string proxy reference", []string{"couper", "run", "-f", base + "/19_couper.hcl"}, nil, `level=error msg="%s/19_couper.hcl:3,13-14: proxy must evaluate to string; " build=dev`, 1}, + {"proxy reference does not exist", []string{"couper", "run", "-f", base + "/20_couper.hcl"}, nil, `level=error msg="%s/20_couper.hcl:3,14-17: proxy reference is not defined; " build=dev`, 1}, } for _, tt := range tests { t.Run(tt.name, func(subT *testing.T) { diff --git a/server/testdata/settings/20_couper.hcl b/server/testdata/settings/20_couper.hcl new file mode 100644 index 000000000..cdbbc5060 --- /dev/null +++ b/server/testdata/settings/20_couper.hcl @@ -0,0 +1,5 @@ +server { + endpoint "/" { + proxy = "foo" + } +} From a4c458ed34dfef0b2a6ff1809f7f9ab31e9ebf2d Mon Sep 17 00:00:00 2001 From: Alex Schneider Date: Wed, 31 Aug 2022 13:55:38 +0200 Subject: [PATCH 14/17] Test unique proxy labels --- config/configload/validate.go | 6 ++++++ config/configload/validate_test.go | 11 +++++++++++ 2 files changed, 17 insertions(+) diff --git a/config/configload/validate.go b/config/configload/validate.go index 44dd88703..d2187aa39 100644 --- a/config/configload/validate.go +++ b/config/configload/validate.go @@ -43,6 +43,7 @@ func validateBody(body hcl.Body, afterMerge bool) error { uniqueBackends := make(map[string]struct{}) uniqueACs := make(map[string]struct{}) uniqueJWTSigningProfiles := make(map[string]struct{}) + uniqueProxies := make(map[string]struct{}) for _, innerBlock := range outerBlock.Body.Blocks { if !afterMerge { if len(innerBlock.Labels) == 0 { @@ -92,6 +93,11 @@ func validateBody(body hcl.Body, afterMerge bool) error { if err != nil { return err } + case proxy: + if _, set := uniqueProxies[label]; set { + return newDiagErr(&labelRange, "proxy labels must be unique") + } + uniqueProxies[label] = struct{}{} } } } else if outerBlock.Type == server { diff --git a/config/configload/validate_test.go b/config/configload/validate_test.go index 9756fc37f..9afa86979 100644 --- a/config/configload/validate_test.go +++ b/config/configload/validate_test.go @@ -284,6 +284,17 @@ func Test_validateBody(t *testing.T) { }`, "couper.hcl:5,15-20: backend labels must be unique; ", }, + { + "duplicate proxy labels", + `server {} + definitions { + proxy "foo" { + } + proxy "foo" { + } + }`, + "couper.hcl:5,13-18: proxy labels must be unique; ", + }, { "missing basic_auth label", `server {} From eee1c3b43c1207a7cd9de4f904dff13627463f5e Mon Sep 17 00:00:00 2001 From: Alex Schneider Date: Wed, 31 Aug 2022 14:12:42 +0200 Subject: [PATCH 15/17] Fix merge error --- config/configload/merge.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/config/configload/merge.go b/config/configload/merge.go index dcfaf772f..b31a6783c 100644 --- a/config/configload/merge.go +++ b/config/configload/merge.go @@ -424,8 +424,6 @@ func mergeDefinitions(bodies []*hclsyntax.Body) (*hclsyntax.Block, map[string]*h definitionsBlock[innerBlock.Type] = make(data) } - definitionsBlock[innerBlock.Type][innerBlock.Labels[0]] = innerBlock - // Count the "backend" blocks and "backend" attributes to // forbid multiple backend definitions. From 5b7b3d9d5ad4b221386435e9c60c3b5e4a802b6a Mon Sep 17 00:00:00 2001 From: Alex Schneider Date: Wed, 31 Aug 2022 14:15:45 +0200 Subject: [PATCH 16/17] Check for unique proxy labels only before merge --- config/configload/validate.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/config/configload/validate.go b/config/configload/validate.go index d2187aa39..0ad87f0fb 100644 --- a/config/configload/validate.go +++ b/config/configload/validate.go @@ -94,10 +94,12 @@ func validateBody(body hcl.Body, afterMerge bool) error { return err } case proxy: - if _, set := uniqueProxies[label]; set { - return newDiagErr(&labelRange, "proxy labels must be unique") + if !afterMerge { + if _, set := uniqueProxies[label]; set { + return newDiagErr(&labelRange, "proxy labels must be unique") + } + uniqueProxies[label] = struct{}{} } - uniqueProxies[label] = struct{}{} } } } else if outerBlock.Type == server { From f5dae55caff9e18a3c5a83fc7e7ff3b89b85c9f0 Mon Sep 17 00:00:00 2001 From: Alex Schneider Date: Wed, 31 Aug 2022 14:20:01 +0200 Subject: [PATCH 17/17] WS Co-authored-by: Johannes Koch <53434855+johakoch@users.noreply.github.com> --- config/configload/validate_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/configload/validate_test.go b/config/configload/validate_test.go index 9afa86979..7f1980a10 100644 --- a/config/configload/validate_test.go +++ b/config/configload/validate_test.go @@ -288,7 +288,7 @@ func Test_validateBody(t *testing.T) { "duplicate proxy labels", `server {} definitions { - proxy "foo" { + proxy "foo" { } proxy "foo" { }