openziti / edge

Application-embedded connectivity and zero-trust components
Apache License 2.0
74 stars 19 forks source link

500 in Edge Controller when creating config #1020

Closed gberl002 closed 2 years ago

gberl002 commented 2 years ago

I posted this payload to my edge controller. The same thing seems to work from postman but doesn't work from go test using the API.

POST /edge/management/v1/configs HTTP/1.1
Host: ziti-edge-controller:1280
User-Agent: Go-http-client/1.1
Content-Length: 166
Accept: application/json
Content-Type: application/json
Zt-Session: xxxsecretxxx
Accept-Encoding: gzip

{"configTypeId":"g7cIWbcGg","data":"'{\"protocols\":[\"tcp\"],\"addresses\":[\"simple.web.test\"], \"portRanges\":[{\"low\":80, \"high\":80}]}'","name":"basic.dial"}

This produced a 500 in the controller with the following stack:

ERROR fabric/controller/api.(*timeoutHandler).ServeHTTP.func1.1: panic caught by timeout next: interface conversion: interface {} is string, not map[string]interface {}
goroutine 2754202 [running]:
github.com/openziti/foundation/util/debugz.generateStack(0x2000, 0xc0?)
    /root/go/pkg/mod/github.com/openziti/foundation@v0.17.25/util/debugz/stack.go:39 +0x4c
github.com/openziti/foundation/util/debugz.GenerateLocalStack(...)
    /root/go/pkg/mod/github.com/openziti/foundation@v0.17.25/util/debugz/stack.go:34
github.com/openziti/fabric/controller/api.(*timeoutHandler).ServeHTTP.func1.1()
    /root/go/pkg/mod/github.com/openziti/fabric@v0.17.135/controller/api/timeouts.go:75 +0xa8
panic({0x13b49a0, 0x4001339530})
    /usr/local/go/src/runtime/panic.go:838 +0x20c
github.com/openziti/edge/controller/internal/routes.MapCreateConfigToModel(0x40013393b0)
    /root/go/pkg/mod/github.com/openziti/edge@v0.21.213/controller/internal/routes/config_api_model.go:44 +0x184
github.com/openziti/edge/controller/internal/routes.(*ConfigRouter).Create.func1()
    /root/go/pkg/mod/github.com/openziti/edge@v0.21.213/controller/internal/routes/config_router.go:85 +0x2c
github.com/openziti/edge/controller/internal/routes.CreateWithResponder(0x40021962a0, {0x1a56ee8, 0x40021962a0}, {0x1a2c8f8, 0x40004440f0}, 0x40013392f0?)
    /root/go/pkg/mod/github.com/openziti/edge@v0.21.213/controller/internal/routes/base_router.go:164 +0x44
github.com/openziti/edge/controller/internal/routes.Create(...)
    /root/go/pkg/mod/github.com/openziti/edge@v0.21.213/controller/internal/routes/base_router.go:160
github.com/openziti/edge/controller/internal/routes.(*ConfigRouter).Create(0x4002641248?, 0x1011cb4?, 0x4002641388?, {0x4002d73900?, 0x40013393b0?})
    /root/go/pkg/mod/github.com/openziti/edge@v0.21.213/controller/internal/routes/config_router.go:84 +0x78
github.com/openziti/edge/controller/internal/routes.(*ConfigRouter).Register.func5.1(0x40016513f0?, 0x4001e61b80?)
    /root/go/pkg/mod/github.com/openziti/edge@v0.21.213/controller/internal/routes/config_router.go:62 +0x38
github.com/openziti/edge/controller/env.(*AppEnv).IsAllowed.func1({0x1a453a0, 0x4003876060}, {0x1a3da18, 0x188fde8})
    /root/go/pkg/mod/github.com/openziti/edge@v0.21.213/controller/env/appenv.go:601 +0x2b8
github.com/go-openapi/runtime/middleware.ResponderFunc.WriteResponse(0x138e140?, {0x1a453a0?, 0x4003876060?}, {0x1a3da18?, 0x188fde8?})
    /root/go/pkg/mod/github.com/go-openapi/runtime@v0.24.1/middleware/context.go:69 +0x48
github.com/go-openapi/runtime/middleware.(*Context).Respond(0x4001a239b0, {0x1a453a0?, 0x4003876060}, 0x4002d73900, {0x4000e9ef30?, 0x1, 0x1}, 0x4002d73800, {0x13b9380, 0x40038768a0})
    /root/go/pkg/mod/github.com/go-openapi/runtime@v0.24.1/middleware/context.go:510 +0x50c
github.com/openziti/edge/rest_management_api_server/operations/config.(*CreateConfig).ServeHTTP(0x4000ed8138, {0x1a453a0, 0x4003876060}, 0x4002d73900)
    /root/go/pkg/mod/github.com/openziti/edge@v0.21.213/rest_management_api_server/operations/config/create_config.go:93 +0x274
andrewpmartinez commented 2 years ago

It is because your data property is a string and not an object. The API shouldn't choke on that but I'm curious to see the code used to generate that payload.

gberl002 commented 2 years ago

Here is the code used to generate the payload

    interceptID := getConfigTypeByName(client,"intercept.v1").ID
    interceptData := "'{\"protocols\":[\"tcp\"],\"addresses\":[\"simple.web.test\"], \"portRanges\":[{\"low\":80, \"high\":80}]}'"
    dialConfName := "basic.dial"
    dialConfig := &rest_model.ConfigCreate{
        ConfigTypeID: interceptID,
        Data:         &interceptData,
        Name:         &dialConfName,
    }
andrewpmartinez commented 2 years ago

Looking at the generated code for Data on the rest_model.ConfigCreate struct, it is defining it as an interface{} ... which can basically be anything. However the actual back end requires that to be a map[string]interface{}.

You should change your code to be the following:

    interceptData := map[string]interface{}{
        "protocols": []string{"tcp"},
        "addresses": []string{"simple.web.test"},
        "portRanges": []map[string]interface{}{
            {
                "low": 80, 
                "high": 80,
            },
        },
    }
andrewpmartinez commented 2 years ago

I'll put a fix in for the panic. Providing a string as you did results in a field that isn't checked before use.

gberl002 commented 2 years ago

That fixed it, thanks