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

bug: need to check ID between path and body #566

Closed
nic-chen opened this issue Oct 20, 2020 · 13 comments · Fixed by #1067
Closed

bug: need to check ID between path and body #566

nic-chen opened this issue Oct 20, 2020 · 13 comments · Fixed by #1067
Labels
backend bug Something isn't working good first issue Good for newcomers
Milestone

Comments

@nic-chen
Copy link
Member

nic-chen commented Oct 20, 2020

Describe the bug

When using the put method to modify or create routes or other resources, there are two places where you can pass ID, path and body.
If the IDS passed in these two places are inconsistent, we cannot be sure which one is correct.

Example:

$ curl http://127.0.0.1:9000/apisix/admin/routes/1 -H 'Authorization: eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjE2MDcwNDc1NTksImlhdCI6MTYwNzA0Mzk1OSwic3ViIjoiYWRtaW4ifQ.CnrVJEVp6hsSZvhUOmkqM3z1Y4_12deI2jgdJem4OdU' -X PUT -i -d '
{
    "id": "2",
    "uri": "/index.html",
    "upstream": {
        "type": "roundrobin",
        "nodes": {
            "39.97.63.215:80": 1
        }
    }
}'

Solution

If the IDs passed by path and body are inconsistent, we should reject and return an error message.

BTW, route, upstream, ssl, service, consumer all have this issue.

@juzhiyuan juzhiyuan added this to the 1.6 milestone Oct 20, 2020
@nic-chen nic-chen modified the milestones: 2.0, 2.1 Oct 21, 2020
@nic-chen nic-chen self-assigned this Nov 30, 2020
@membphis membphis added backend bug Something isn't working labels Dec 4, 2020
@membphis membphis changed the title [refactor] need to check ID between path and body bug: need to check ID between path and body Dec 4, 2020
@nic-chen nic-chen added the good first issue Good for newcomers label Dec 6, 2020
@jinchizhou
Copy link
Contributor

I'll play around with this issue tomorrow!

@nic-chen
Copy link
Member Author

nic-chen commented Dec 6, 2020

I'll play around with this issue tomorrow!

that's great. thanks.

@jinchizhou
Copy link
Contributor

Hey @nic-chen ,

When setting up, I ran into the issue

make api-run
cd api/ && go run .
{"level":"warn","ts":"2020-12-07T11:15:29.958-0800","caller":"clientv3/retry_interceptor.go:62","msg":"retrying of unary invoker failed","target":"endpoint://client-b1e38523-73fc-4b26-a698-0f0d447376d4/127.0.0.1:2379","attempt":0,"error":"rpc error: code = DeadlineExceeded desc = latest balancer error: all SubConns are in TransientFailure, latest connection error: connection error: desc = \"transport: Error while dialing dial tcp 127.0.0.1:2379: connect: connection refused\""}
panic: etcd get failed: context deadline exceeded`

goroutine 1 [running]:
main.main()
        /Users/jordanlee/Documents/jinchi/apisix-dashboard/api/main.go:53 +0x7c5
exit status 2
make: *** [api-run] Error 1

The doc says to change api/conf/conf.yaml. Is there anything I need to change? Thanks!

@jinchizhou
Copy link
Contributor

I exported ETCDCTL_API = 2 and got

jordans-mbp-4:apisix-dashboard jordanlee$ export ETCDCTL_API=2
jordans-mbp-4:apisix-dashboard jordanlee$ make api-run
cd api/ && go run .
# github.com/apisix/manager-api/internal/utils
internal/utils/closer 2.go:28:19: cannot use c (type Closer) as type Closer in append
internal/utils/closer.go:22:2: _closers redeclared in this block
        previous declaration at internal/utils/closer 2.go:22:2
internal/utils/closer.go:25:6: Closer redeclared in this block
        previous declaration at internal/utils/closer 2.go:25:6
internal/utils/closer.go:27:6: AppendToClosers redeclared in this block
        previous declaration at internal/utils/closer 2.go:27:24
internal/utils/closer.go:28:19: cannot use c (type Closer) as type Closer in append
internal/utils/closer.go:31:6: CloseAll redeclared in this block
        previous declaration at internal/utils/closer 2.go:31:6
# github.com/apisix/manager-api/internal/utils/consts
internal/utils/consts/api_error.go:24:6: WrapperHandle redeclared in this block
        previous declaration at internal/utils/consts/api_error 2.go:24:6
internal/utils/consts/api_error.go:26:6: ErrorWrapper redeclared in this block
        previous declaration at internal/utils/consts/api_error 2.go:26:41
internal/utils/consts/api_error.go:27:9: ErrorWrapper.func1 redeclared in this block
        previous declaration at internal/utils/consts/api_error 2.go:27:9
internal/utils/consts/api_error.go:38:6: ApiError redeclared in this block
        previous declaration at internal/utils/consts/api_error 2.go:38:6
internal/utils/consts/api_error.go:44:21: ApiError.Error redeclared in this block
        previous declaration at internal/utils/consts/api_error 2.go:44:6
internal/utils/consts/api_error.go:48:6: InvalidParam redeclared in this block
        previous declaration at internal/utils/consts/api_error 2.go:48:36
internal/utils/consts/api_error.go:52:6: SystemError redeclared in this block
        previous declaration at internal/utils/consts/api_error 2.go:52:35
# github.com/apisix/manager-api/internal/handler/healthz
internal/handler/healthz/healthz.go:27:6: Handler redeclared in this block
        previous declaration at internal/handler/healthz/healthz 2.go:27:6
internal/handler/healthz/healthz.go:30:6: NewHandler redeclared in this block
        previous declaration at internal/handler/healthz/healthz 2.go:30:43
internal/handler/healthz/healthz.go:34:19: (*Handler).ApplyRoute redeclared in this block
        previous declaration at internal/handler/healthz/healthz 2.go:34:6
internal/handler/healthz/healthz.go:38:19: (*Handler).healthZHandler redeclared in this block
        previous declaration at internal/handler/healthz/healthz 2.go:38:6
make: *** [api-run] Error 2

@nic-chen
Copy link
Member Author

nic-chen commented Dec 8, 2020

Hi @jinchizhou , what's your ETCD version ? 3.4+ is required.

@nic-chen
Copy link
Member Author

nic-chen commented Dec 8, 2020

you could contact my QQ if convenient. we could quickly communicate these env issues.

@jinchizhou
Copy link
Contributor

Unfortunately my windows computer doesn't have the capability to support ETCD. Sorry, looks like I can't work on this open source repo :(.

@nic-chen
Copy link
Member Author

nic-chen commented Dec 8, 2020

Unfortunately my windows computer doesn't have the capability to support ETCD. Sorry, looks like I can't work on this open source repo :(.

@jinchizhou It doesn't matter. but if you still have the willing you could contact, we could resolved it together.

@membphis
Copy link
Member

membphis commented Dec 8, 2020

Hi @jinchizhou , what's your ETCD version ? 3.4+ is required.

Does Manage API have steps to check etcd version? I think this step is important.

@membphis membphis modified the milestones: 2.2, 2.3 Dec 11, 2020
@jinchizhou
Copy link
Contributor

hey @nic-chen , can I contact you?

@nic-chen
Copy link
Member Author

nic-chen commented Dec 12, 2020

hey @nic-chen , can I contact you?

sure

@jinchizhou
Copy link
Contributor

jinchizhou commented Dec 12, 2020

I see. I'm in the US, not sure if that is why you cannot add me on wechat. Could I possibly sync up with you through Google Hangouts? or Zoom?

@nic-chen
Copy link
Member Author

nic-chen commented Dec 12, 2020

I see. I'm in the US, not sure if that is why you cannot add me on wechat. Could I possibly sync up with you through Google Hangouts? or Zoom? You can contact me through email

OK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants