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

No path in proxy or endpoint #516

Merged
merged 10 commits into from
Jun 7, 2022
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ Unreleased changes are available as `avenga/couper:edge` container.
* **Removed**
* support for `beta_oidc` block (use [`oidc` block](./docs/REFERENCE.md#oidc-block) instead) ([#475](https://github.com/avenga/couper/pull/475))
* support for `beta_oauth_authorization_url` and `beta_oauth_verifier` functions (use `oauth2_authorization_url` and `oauth2_verifier` [functions](./docs/REFERENCE.md#functions) instead) ([#475](https://github.com/avenga/couper/pull/475))
* `path` attribute from `endpoint` (and `proxy`) block; use `path` attribute in `backend` block instead ([#516](https://github.com/avenga/couper/pull/516))

---

Expand Down
1 change: 1 addition & 0 deletions config/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ func (b Backend) Inline() interface{} {
Hostname string `hcl:"hostname,optional"`
LogFields map[string]hcl.Expression `hcl:"custom_log_fields,optional"`
Origin string `hcl:"origin,optional"`
Path string `hcl:"path,optional"`
PathPrefix string `hcl:"path_prefix,optional"`
ProxyURL string `hcl:"proxy,optional"`
ResponseStatus *uint8 `hcl:"set_response_status,optional"`
Expand Down
15 changes: 15 additions & 0 deletions config/configload/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ var (

func ValidateConfigSchema(body hcl.Body, obj interface{}) hcl.Diagnostics {
attrs, blocks, diags := getSchemaComponents(body, obj)
diags = enhanceErrors(diags, obj)
diags = filterValidErrors(attrs, blocks, diags)

for _, block := range blocks {
Expand All @@ -39,6 +40,20 @@ func ValidateConfigSchema(body hcl.Body, obj interface{}) hcl.Diagnostics {
return uniqueErrors(diags)
}

// enhanceErrors enhances diagnostics e.g. by providing a hint how to solve the issue
func enhanceErrors(diags hcl.Diagnostics, obj interface{}) hcl.Diagnostics {
_, isEndpoint := obj.(*config.Endpoint)
_, isProxy := obj.(*config.Proxy)
for _, err := range diags {
if err.Summary == summUnsupportedAttr && (isEndpoint || isProxy) {
if matches := reFetchUnexpectedArg.FindStringSubmatch(err.Detail); matches != nil && matches[1] == `"path"` {
err.Detail = err.Detail + ` Use the "path" attribute in a backend block instead.`
}
}
}
return diags
}

// filterValidErrors ignores certain schema related errors due to their specific non hcl conform implementation.
// Related attributes and blocks will be logic checked with LoadConfig.
// TODO: Improve checkObjectFields to remove this filter requirement. E.g. optional label struct tag
Expand Down
99 changes: 79 additions & 20 deletions config/configload/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -418,18 +418,9 @@ defaults {
},
}

logger, _ := logrustest.NewNullLogger()
log := logger.WithContext(context.TODO())

for _, tt := range tests {
t.Run(tt.name, func(subT *testing.T) {
conf, err := LoadBytes([]byte(tt.hcl), "couper.hcl")
if conf != nil {
tmpStoreCh := make(chan struct{})
defer close(tmpStoreCh)

_, err = runtime.NewServerConfiguration(conf, log, cache.New(log, tmpStoreCh))
}
_, err := LoadBytes([]byte(tt.hcl), "couper.hcl")

var errMsg string
if err != nil {
Expand Down Expand Up @@ -531,18 +522,9 @@ func TestPermissionMixed(t *testing.T) {
},
}

logger, _ := logrustest.NewNullLogger()
log := logger.WithContext(context.TODO())

for _, tt := range tests {
t.Run(tt.name, func(subT *testing.T) {
conf, err := LoadBytes([]byte(tt.hcl), "couper.hcl")
if conf != nil {
tmpStoreCh := make(chan struct{})
defer close(tmpStoreCh)

_, err = runtime.NewServerConfiguration(conf, log, cache.New(log, tmpStoreCh))
}
_, err := LoadBytes([]byte(tt.hcl), "couper.hcl")

var errMsg string
if err != nil {
Expand All @@ -556,3 +538,80 @@ func TestPermissionMixed(t *testing.T) {
})
}
}

func TestPathAttr(t *testing.T) {
tests := []struct {
name string
hcl string
error string
}{
{
"path in endpoint: error",
`server {
endpoint "/**" {
path = "/a/**"
}
}`,
"couper.hcl:3,5-9: Unsupported argument; An argument named \"path\" is not expected here. Use the \"path\" attribute in a backend block instead.",
},
{
"path in proxy: error",
`server {
endpoint "/**" {
proxy {
path = "/a/**"
}
}
}`,
"couper.hcl:4,7-11: Unsupported argument; An argument named \"path\" is not expected here. Use the \"path\" attribute in a backend block instead.",
},
{
"path in referenced backend: ok",
`server {
endpoint "/**" {
proxy {
backend = "a"
}
}
}
definitions {
backend "a" {
path = "/a/**"
}
}`,
"",
},
{
"path in refined backend: ok",
`server {
endpoint "/**" {
proxy {
backend "a" {
path = "/a/**"
}
}
}
}
definitions {
backend "a" {
}
}`,
"",
},
}

for _, tt := range tests {
t.Run(tt.name, func(subT *testing.T) {
_, err := LoadBytes([]byte(tt.hcl), "couper.hcl")

var errMsg string
if err != nil {
errMsg = err.Error()
}

if tt.error != errMsg {
subT.Errorf("%q: Unexpected configuration error:\n\tWant: %q\n\tGot: %q", tt.name, tt.error, errMsg)
}
})
}
}
3 changes: 0 additions & 3 deletions config/meta/attributes.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@ type Attributes struct {
AddResponseHeaders map[string]string `hcl:"add_response_headers,optional"`
DelResponseHeaders []string `hcl:"remove_response_headers,optional"`
SetResponseHeaders map[string]string `hcl:"set_response_headers,optional"`

// Other
Path string `hcl:"path,optional"`
}

func SchemaWithAttributes(schema *hcl.BodySchema) *hcl.BodySchema {
Expand Down
7 changes: 3 additions & 4 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -222,11 +222,11 @@ Couper's main concept is exposing APIs via the configuration block `endpoint`, f
server "example"{

endpoint "/public/**"{
path = "/**"

proxy {
backend {
origin = "https://httpbin.org"
path = "/**"
}
}
}
Expand All @@ -247,11 +247,11 @@ server "example" {

endpoint "/private/**" {
access_control = ["accessToken"]
path = "/**"

proxy {
backend {
origin = "https://httpbin.org"
path = "/**"
}
}
}
Expand Down Expand Up @@ -283,9 +283,8 @@ api "my_api" {
}

endpoint "/cart/**" {
path = "/api/v1/**"
proxy {
url = "http://cartservice:8080"
url = "http://cartservice:8080/api/v1/**"
}
}

Expand Down
5 changes: 1 addition & 4 deletions docs/REFERENCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,7 @@ as JSON error with an error body payload. This can be customized via `error_file
### Endpoint Block

`endpoint` blocks define the entry points of Couper. The required _label_
defines the path suffix for the incoming client request. The `path` attribute
changes the path for the outgoing request (compare
[path mapping example](README.md#routing-path-mapping)). Each `endpoint` block must
defines the path suffix for the incoming client request. Each `endpoint` block must
produce an explicit or implicit client response.

| Block name | Context | Label | Nested block(s) |
Expand All @@ -141,7 +139,6 @@ produce an explicit or implicit client response.
| Attribute(s) | Type | Default | Description | Characteristic(s) | Example |
|:------------------------|:-----------------|:--------|:------------------------------------------------------------------------------------------------------------------|:------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:---------------------------------------------------------------------|
| `request_body_limit` | string | `"64MiB"` | Configures the maximum buffer size while accessing `request.form_body` or `request.json_body` content. | ⚠ Valid units are: `KiB, MiB, GiB` | `request_body_limit = "200KiB"` |
| `path` | string | - | Changeable part of the upstream URL. Changes the path suffix of the outgoing request. | - | - |
afflerbach marked this conversation as resolved.
Show resolved Hide resolved
| `access_control` | tuple (string) | - | Sets predefined [Access Control](#access-control) for `endpoint` block context. | - | `access_control = ["foo"]` |
| `disable_access_control` | tuple (string) | - | Disables access controls by name. | - | `disable_access_control = ["foo"]` |
| `allowed_methods` | tuple (string) | `["*"]` == `["GET", "HEAD", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"]` | Sets allowed methods _overriding_ a default set in the containing `api` block. Requests with a method that is not allowed result in an error response with a `405 Method Not Allowed` status. | The default value `"*"` can be combined with additional methods. Methods are matched case-insensitively. `Access-Control-Allow-Methods` is only sent in response to a [CORS](#cors-block) preflight request, if the method requested by `Access-Control-Request-Method` is an allowed method. | `allowed_methods = ["GET", "POST"]` or `allowed_methods = ["*", "BREW"]` |
Expand Down
24 changes: 19 additions & 5 deletions eval/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,19 +100,34 @@ func SetBody(req *http.Request, body []byte) {
parseForm(req)
}

func getPathAttribute(body hcl.Body) (*hcl.Attribute, error) {
bodyContent, _, diags := body.PartialContent(config.BackendInlineSchema)
if diags.HasErrors() {
return nil, errors.Evaluation.With(diags)
}
return bodyContent.Attributes[attrPath], nil
}

func ApplyRequestContext(httpCtx *hcl.EvalContext, body hcl.Body, req *http.Request) error {
if req == nil {
return nil
}

headerCtx := req.Header

attrs, err := getAllAttributes(body)
pathAttr, err := getPathAttribute(body)
if err != nil {
return err
}

if err = evalPathAttr(req, attrs, httpCtx); err != nil {
if pathAttr != nil {
if err = evalPathAttr(req, pathAttr, httpCtx); err != nil {
return err
}
}

attrs, err := getAllAttributes(body)
if err != nil {
return err
}

Expand Down Expand Up @@ -264,9 +279,8 @@ func getFormParams(ctx *hcl.EvalContext, req *http.Request, attrs map[string]*hc
return nil
}

func evalPathAttr(req *http.Request, attrs map[string]*hcl.Attribute, httpCtx *hcl.EvalContext) error {
pathAttr, ok := attrs[attrPath]
if !ok {
func evalPathAttr(req *http.Request, pathAttr *hcl.Attribute, httpCtx *hcl.EvalContext) error {
if pathAttr == nil {
return nil
}
path := req.URL.Path
Expand Down
9 changes: 4 additions & 5 deletions server/http_backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,10 @@ func TestBackend_WithoutOrigin(t *testing.T) {
path string
message string
}{
{"/proxy/path", `configuration error: anonymous_6_13: the origin attribute has to contain an absolute URL with a valid hostname: ""`},
{"/proxy/backend-path", `configuration error: anonymous_15_13: the origin attribute has to contain an absolute URL with a valid hostname: ""`},
{"/proxy/url", `configuration error: anonymous_24_13: the origin attribute has to contain an absolute URL with a valid hostname: ""`},
{"/request/backend-path", `configuration error: anonymous_37_15: the origin attribute has to contain an absolute URL with a valid hostname: ""`},
{"/request/url", `configuration error: anonymous_46_13: the origin attribute has to contain an absolute URL with a valid hostname: ""`},
{"/proxy/backend-path", `configuration error: anonymous_6_13: the origin attribute has to contain an absolute URL with a valid hostname: ""`},
{"/proxy/url", `configuration error: anonymous_15_13: the origin attribute has to contain an absolute URL with a valid hostname: ""`},
{"/request/backend-path", `configuration error: anonymous_28_15: the origin attribute has to contain an absolute URL with a valid hostname: ""`},
{"/request/url", `configuration error: anonymous_37_15: the origin attribute has to contain an absolute URL with a valid hostname: ""`},
} {
t.Run(tc.path, func(st *testing.T) {
hook.Reset()
Expand Down
Loading