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 @@ -41,6 +41,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` 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
86 changes: 86 additions & 0 deletions config/configload/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -556,3 +556,89 @@ 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" {
}
}`,
"",
},
}

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))
}
johakoch marked this conversation as resolved.
Show resolved Hide resolved

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
1 change: 0 additions & 1 deletion docs/REFERENCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,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
86 changes: 0 additions & 86 deletions server/http_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -784,9 +784,6 @@ func TestHTTPServer_PathPrefix(t *testing.T) {
{"/v1", expectation{
Path: "/xxx/xxx/v1",
}},
{"/v1/uuu/foo", expectation{
Path: "/xxx/xxx/api/foo",
}},
{"/v1/vvv/foo", expectation{
Path: "/xxx/xxx/api/foo",
}},
Expand Down Expand Up @@ -851,25 +848,6 @@ func TestHTTPServer_BackendLogPath(t *testing.T) {
}
}

func TestHTTPServer_BackendLogPathInEndpoint(t *testing.T) {
client := newClient()
helper := test.New(t)

shutdown, hook := newCouper("testdata/integration/api/08_couper.hcl", helper)
defer shutdown()

req, err := http.NewRequest(http.MethodGet, "http://example.com:8080/abc?query#fragment", nil)
helper.Must(err)

hook.Reset()
_, err = client.Do(req)
helper.Must(err)

if p := hook.AllEntries()[0].Data["request"].(logging.Fields)["path"]; p != "/new/path/abc?query" {
t.Errorf("Unexpected path given: %s", p)
}
}

func TestHTTPServer_BackendLogRequestProto(t *testing.T) {
client := newClient()
helper := test.New(t)
Expand Down Expand Up @@ -3015,70 +2993,6 @@ func TestHTTPServer_request_variables(t *testing.T) {
}
}

func TestHTTPServer_Endpoint_Evaluation_Inheritance(t *testing.T) {
client := newClient()

for _, confFile := range []string{"02_couper.hcl", "03_couper.hcl"} {
confPath := path.Join("testdata/integration/endpoint_eval", confFile)

type expectation struct {
Path string
ResponseStatus int
}

type testCase struct {
reqPath string
exp expectation
}

for _, tc := range []testCase{
{"/endpoint1", expectation{
Path: "/anything",
ResponseStatus: http.StatusOK,
}},
{"/endpoint2", expectation{
Path: "/anything",
ResponseStatus: http.StatusOK,
}},
{"/endpoint3", expectation{
Path: "/unset/by/endpoint",
ResponseStatus: http.StatusNotFound,
}},
{"/endpoint4", expectation{
Path: "/anything",
ResponseStatus: http.StatusOK,
}},
} {
t.Run(confFile+"_"+tc.reqPath, func(subT *testing.T) {
helper := test.New(subT)
shutdown, _ := newCouper(confPath, helper)
defer shutdown()

req, err := http.NewRequest(http.MethodGet, "http://example.com:8080"+tc.reqPath, nil)
helper.Must(err)

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

resBytes, err := io.ReadAll(res.Body)
helper.Must(err)

_ = res.Body.Close()

var jsonResult expectation
err = json.Unmarshal(resBytes, &jsonResult)
if err != nil {
subT.Errorf("unmarshal json: %v: got:\n%s", err, string(resBytes))
}

if !reflect.DeepEqual(jsonResult, tc.exp) {
subT.Errorf("%q: %q:\nwant:\t%#v\ngot:\t%#v\npayload:\n%s", confFile, tc.reqPath, tc.exp, jsonResult, string(resBytes))
}
})
}
}
}

func TestOpenAPIValidateConcurrentRequests(t *testing.T) {
helper := test.New(t)
client := newClient()
Expand Down
9 changes: 0 additions & 9 deletions server/testdata/integration/api/06_couper.hcl
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,6 @@ server "multi-api" {
}
}
}
endpoint "/uuu/**" {
path = "/api/**"
proxy {
backend {
origin = env.COUPER_TEST_BACKEND_ADDR
path_prefix = "/${request.headers.x-val}/xxx/"
}
}
}
endpoint "/vvv/**" {
proxy {
backend {
Expand Down
1 change: 0 additions & 1 deletion server/testdata/integration/api/07_couper.hcl
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
server "api" {
api {
endpoint "/**" {
path = "/new/path/**"
proxy {
backend {
origin = "${env.COUPER_TEST_BACKEND_ADDR}"
Expand Down
12 changes: 0 additions & 12 deletions server/testdata/integration/api/08_couper.hcl

This file was deleted.

1 change: 0 additions & 1 deletion server/testdata/integration/api/09_couper.hcl
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
server "api" {
api {
endpoint "/**" {
path = "/new/path/**"
proxy {
backend {
origin = "${env.COUPER_TEST_BACKEND_ADDR}"
Expand Down
1 change: 0 additions & 1 deletion server/testdata/integration/api/10_couper.hcl
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
server "api" {
api {
endpoint "/**" {
path = "/new/path/**"
proxy {
backend {
origin = "${env.COUPER_TEST_BACKEND_ADDR}"
Expand Down
1 change: 0 additions & 1 deletion server/testdata/integration/api/11_couper.hcl
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
server "api" {
api {
endpoint "/**" {
path = "/new/path/**"
proxy {
backend {
origin = "${env.COUPER_TEST_BACKEND_ADDR}"
Expand Down
1 change: 0 additions & 1 deletion server/testdata/integration/api/12_couper.hcl
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
server "api" {
api {
endpoint "/**" {
path = "/new/path/**"
proxy {
backend {
origin = "${env.COUPER_TEST_BACKEND_ADDR}"
Expand Down
Loading