Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Errors when config-merge #483

Merged
merged 20 commits into from
Apr 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ Unreleased changes are available as `avenga/couper:edge` container.
* Multiple labels for [`error_handler` blocks](./docs/ERRORS.md#error_handler-specification) ([#462](https://github.com/avenga/couper/pull/462))
* [`error_handler` blocks](./docs/REFERENCE.md#error-handler-block) for an error type defined in both `endpoint` and `api` ([#469](https://github.com/avenga/couper/pull/469))
* Request methods are treated case-insensitively when comparing them to methods in the `allowed_methods` attribute of [`api`](./docs/REFERENCE.md#api-block) or [`endpoint`](./docs/REFERENCE.md#endpoint-block) blocks ([#478](https://github.com/avenga/couper/pull/478))
* Do not allow multiple `backend` blocks in `proxy` and `request` blocks ([#483](https://github.com/avenga/couper/pull/483))

* **Removed**
* support for `beta_oidc` block (use [`oidc` block](./docs/REFERENCE.md#oidc-block) instead) ([#475](https://github.com/avenga/couper/pull/475))
Expand Down
2 changes: 1 addition & 1 deletion config/configload/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func (h *helper) configureACBackends() error {
acs = append(acs, ac)
}

for _, ac := range append(h.config.Definitions.OIDC) {
for _, ac := range h.config.Definitions.OIDC {
acs = append(acs, ac)
}

Expand Down
213 changes: 140 additions & 73 deletions config/configload/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,49 +13,10 @@ import (
"github.com/zclconf/go-cty/cty"
)

func errorUniqueLabels(block *hclsyntax.Block) error {
defRange := block.DefRange()

return hcl.Diagnostics{
&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: fmt.Sprintf("All %s blocks must have unique labels.", block.Type),
Subject: &defRange,
},
}
}

// absPath replaces the given attribute path expression with an absolute path
// related to its filename if not already an absolute one.
func absPath(attr *hclsyntax.Attribute) hclsyntax.Expression {
value, diags := attr.Expr.Value(envContext)
if diags.HasErrors() || strings.Index(value.AsString(), "/") == 0 {
return attr.Expr // Return unchanged in error cases and for absolute path values.
}

return &hclsyntax.LiteralValueExpr{
Val: cty.StringVal(
path.Join(filepath.Dir(attr.SrcRange.Filename), value.AsString()),
),
SrcRange: attr.SrcRange,
}
}

func absBackendBlock(backendBlock *hclsyntax.Block) {
for _, block := range backendBlock.Body.Blocks {
if block.Type == "openapi" {
if attr, ok := block.Body.Attributes["file"]; ok {
block.Body.Attributes["file"].Expr = absPath(attr)
}
} else if block.Type == oauth2 {
for _, innerBlock := range block.Body.Blocks {
if innerBlock.Type == backend {
absBackendBlock(innerBlock) // Recursive call
}
}
}
}
}
const (
errMultipleBackends = "Multiple definitions of backend are not allowed."
errUniqueLabels = "All %s blocks must have unique labels."
)

func mergeServers(bodies []*hclsyntax.Body) (hclsyntax.Blocks, error) {
type (
Expand Down Expand Up @@ -110,7 +71,7 @@ func mergeServers(bodies []*hclsyntax.Body) (hclsyntax.Blocks, error) {

if len(bodies) > 1 {
if _, ok := uniqueServerLabels[serverKey]; ok {
return nil, errorUniqueLabels(outerBlock)
return nil, newMergeError(errUniqueLabels, outerBlock)
}

uniqueServerLabels[serverKey] = struct{}{}
Expand Down Expand Up @@ -166,17 +127,15 @@ func mergeServers(bodies []*hclsyntax.Body) (hclsyntax.Blocks, error) {
}

if block.Type == endpoint {
if err := absInBackends(block); err != nil { // Backend block inside a free endpoint block
return nil, err
}

if len(block.Labels) == 0 {
return nil, errorUniqueLabels(block)
return nil, newMergeError(errUniqueLabels, block)
}

results[serverKey].endpoints[block.Labels[0]] = block

for _, innerBlock := range block.Body.Blocks {
if innerBlock.Type == backend {
absBackendBlock(innerBlock) // Backend block inside a free endpoint block
}
}
} else if block.Type == api {
var apiKey string

Expand All @@ -186,7 +145,7 @@ func mergeServers(bodies []*hclsyntax.Body) (hclsyntax.Blocks, error) {

if len(bodies) > 1 {
if _, ok := uniqueAPILabels[apiKey]; ok {
return nil, errorUniqueLabels(block)
return nil, newMergeError(errUniqueLabels, block)
}

uniqueAPILabels[apiKey] = struct{}{}
Expand All @@ -211,12 +170,22 @@ func mergeServers(bodies []*hclsyntax.Body) (hclsyntax.Blocks, error) {

for _, subBlock := range block.Body.Blocks {
if subBlock.Type == endpoint {
if err := absInBackends(subBlock); err != nil {
return nil, err
}

if len(subBlock.Labels) == 0 {
return nil, errorUniqueLabels(subBlock)
return nil, newMergeError(errUniqueLabels, subBlock)
}

results[serverKey].apis[apiKey].endpoints[subBlock.Labels[0]] = subBlock
} else if subBlock.Type == errorHandler {
if err := absInBackends(subBlock); err != nil {
return nil, err
}

ehKey := newErrorHandlerKey(subBlock)

results[serverKey].apis[apiKey].errorHandler[ehKey] = subBlock
} else {
results[serverKey].apis[apiKey].blocks[subBlock.Type] = subBlock
Expand Down Expand Up @@ -299,7 +268,7 @@ func mergeDefinitions(bodies []*hclsyntax.Body) (*hclsyntax.Block, error) {
}

if len(innerBlock.Labels) == 0 {
return nil, errorUniqueLabels(innerBlock)
return nil, newMergeError(errUniqueLabels, innerBlock)
}

definitionsBlock[innerBlock.Type][innerBlock.Labels[0]] = innerBlock
Expand All @@ -313,16 +282,35 @@ func mergeDefinitions(bodies []*hclsyntax.Body) (*hclsyntax.Block, error) {
}
}

// Count the "backend" blocks and "backend" attributes to
// forbid multiple backend definitions.

var backends int

for _, block := range innerBlock.Body.Blocks {
if block.Type == errorHandler {
if attr, ok := innerBlock.Body.Attributes["error_file"]; ok {
innerBlock.Body.Attributes["error_file"].Expr = absPath(attr)
if attr, ok := block.Body.Attributes["error_file"]; ok {
block.Body.Attributes["error_file"].Expr = absPath(attr)
}

if err := absInBackends(block); err != nil {
return nil, err
}
} else if block.Type == backend {
absBackendBlock(innerBlock) // Backend block inside a AC block
absBackendBlock(block) // Backend block inside an AC block

backends++
}
}

if _, ok := innerBlock.Body.Attributes[backend]; ok {
backends++
}

if backends > 1 {
return nil, newMergeError(errMultipleBackends, innerBlock)
}

if innerBlock.Type == backend {
absBackendBlock(innerBlock) // Backend block inside the definitions block
}
Expand All @@ -347,22 +335,6 @@ func mergeDefinitions(bodies []*hclsyntax.Body) (*hclsyntax.Block, error) {
}, 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) {
if len(block.Labels) == 0 {
return key
}

var sorted []string
for _, l := range block.Labels {
sorted = append(sorted, strings.Split(l, errorHandlerLabelSep)...)
}
sort.Strings(sorted)
key = strings.Join(sorted, errorHandlerLabelSep)
return key
}

func mergeDefaults(bodies []*hclsyntax.Body) (*hclsyntax.Block, error) {
attrs := make(hclsyntax.Attributes)
envVars := make(map[string]cty.Value)
Expand Down Expand Up @@ -433,3 +405,98 @@ func mergeSettings(bodies []*hclsyntax.Body) *hclsyntax.Block {
},
}
}

func newMergeError(msg string, block *hclsyntax.Block) error {
defRange := block.DefRange()

return hcl.Diagnostics{
&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: fmt.Sprintf(msg, block.Type),
Subject: &defRange,
},
}
}

// absPath replaces the given attribute path expression with an absolute path
// related to its filename if not already an absolute one.
func absPath(attr *hclsyntax.Attribute) hclsyntax.Expression {
value, diags := attr.Expr.Value(envContext)
if diags.HasErrors() || strings.Index(value.AsString(), "/") == 0 {
return attr.Expr // Return unchanged in error cases and for absolute path values.
}

return &hclsyntax.LiteralValueExpr{
Val: cty.StringVal(
path.Join(filepath.Dir(attr.SrcRange.Filename), value.AsString()),
),
SrcRange: attr.SrcRange,
}
}

func absBackendBlock(backendBlock *hclsyntax.Block) {
for _, block := range backendBlock.Body.Blocks {
if block.Type == "openapi" {
if attr, ok := block.Body.Attributes["file"]; ok {
block.Body.Attributes["file"].Expr = absPath(attr)
}
} else if block.Type == oauth2 {
for _, innerBlock := range block.Body.Blocks {
if innerBlock.Type == backend {
absBackendBlock(innerBlock) // Recursive call
}
}
}
}
}

// absInBackends searches for "backend" blocks inside a proxy or request block to
// be able to rewrite relative pathes. Additionally, the function counts the "backend"
// blocks and "backend" attributes to forbid multiple backend definitions.
func absInBackends(block *hclsyntax.Block) error {
for _, subBlock := range block.Body.Blocks {
if subBlock.Type == errorHandler {
return absInBackends(subBlock) // Recursive call
}

if subBlock.Type != proxy && subBlock.Type != request {
continue
}

var backends int

for _, subSubBlock := range subBlock.Body.Blocks {
if subSubBlock.Type == backend {
absBackendBlock(subSubBlock) // Backend block inside a proxy or request block

backends++
}
}

if _, ok := subBlock.Body.Attributes[backend]; ok {
backends++
}

if backends > 1 {
return newMergeError(errMultipleBackends, block)
}
}

return 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) {
if len(block.Labels) == 0 {
return key
}

var sorted []string
for _, l := range block.Labels {
sorted = append(sorted, strings.Split(l, errorHandlerLabelSep)...)
}
sort.Strings(sorted)

return strings.Join(sorted, errorHandlerLabelSep)
}
2 changes: 1 addition & 1 deletion config/configload/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func invalidOriginRefinement(reference, params hcl.Body) error {

if paramOrigin != nil && refOrigin != nil {
if paramOrigin.Expr != refOrigin.Expr {
return newDiagErr(&paramOrigin.Range, fmt.Sprintf("backend reference: origin must be equal"))
return newDiagErr(&paramOrigin.Range, "backend reference: origin must be equal")
}
}
return nil
Expand Down
27 changes: 25 additions & 2 deletions server/multi_files_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ package server_test
import (
"io"
"net/http"
"path/filepath"
"strings"
"testing"

"github.com/avenga/couper/config/configload"
"github.com/avenga/couper/internal/test"
)

Expand All @@ -27,7 +29,7 @@ func TestMultiFiles_Server(t *testing.T) {
req, err := http.NewRequest(http.MethodGet, "http://example.com:8080/", nil)
helper.Must(err)

res, err := client.Do(req)
_, err = client.Do(req)
if err == nil || err.Error() != `Get "http://example.com:8080/": dial tcp4 127.0.0.1:8080: connect: connection refused` {
t.Error("Expected hosts port override to 9080")
}
Expand Down Expand Up @@ -60,7 +62,7 @@ func TestMultiFiles_Server(t *testing.T) {
req.Header.Set(k, v)
}

res, err = client.Do(req)
res, err := client.Do(req)
helper.Must(err)

if res.StatusCode != tc.expStatus {
Expand Down Expand Up @@ -175,3 +177,24 @@ func TestMultiFiles_Definitions(t *testing.T) {
t.Errorf("Unexpected body given: %s", s)
}
}

func TestMultiFiles_MultipleBackends(t *testing.T) {
type testCase struct {
config string
}

for _, tc := range []testCase{
{"testdata/multi/backends/errors/ac.hcl"},
{"testdata/multi/backends/errors/ac_eh.hcl"},
{"testdata/multi/backends/errors/ep.hcl"},
{"testdata/multi/backends/errors/api_ep.hcl"},
} {
t.Run(tc.config, func(st *testing.T) {
_, err := configload.LoadFiles(filepath.Join(testWorkingDir, tc.config), "")

if !strings.Contains(err.Error(), "Multiple definitions of backend are not allowed.") {
st.Errorf("Unexpected error: %s", err.Error())
}
})
}
}
6 changes: 0 additions & 6 deletions server/mux.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,6 @@ type Mux struct {
spaRoot *pathpattern.Node
}

var fileMethods = []string{
http.MethodGet,
http.MethodHead,
http.MethodOptions,
}

const (
serverOptionsKey = "serverContextOptions"
wildcardReplacement = "/{_couper_wildcardMatch*}"
Expand Down
8 changes: 8 additions & 0 deletions server/testdata/multi/backends/errors/ac.hcl
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
definitions {
jwt "JWT" {
backend {
origin = "https:/example.com"
}
backend = "BE"
}
}
Loading