apisix icon indicating copy to clipboard operation
apisix copied to clipboard

fix: fix `namespaces` in informer factory

Open dongjiang1989 opened this issue 1 year ago • 7 comments

Description

This PR fix url path splicing bug.

endpoints api url: /api/v1/namespaces/{namespace}/endpoints/{name}

ref: https://docs.openshift.com/container-platform/4.8/rest_api/network_apis/endpoints-core-v1.html#apiv1namespacesnamespaceendpointsname

Checklist

  • [x] I have explained the need for this PR and the problem it solves
  • [x] I have explained the changes or the new features added to this PR
  • [x] I have added tests corresponding to this change
  • [x] I have updated the documentation to reflect this change
  • [ ] I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

dongjiang1989 avatar Feb 05 '24 13:02 dongjiang1989

@dongjiang1989 please explain your bugfix, why is it required and also add test cases. Thanks.

shreemaan-abhishek avatar Feb 06 '24 04:02 shreemaan-abhishek

@dongjiang1989 please explain your bugfix, why is it required and also add test cases. Thanks.

This is url path splicing bug.

endpoints api url: /api/v1/namespaces/{namespace}/endpoints/{name} ref: https://docs.openshift.com/container-platform/4.8/rest_api/network_apis/endpoints-core-v1.html#apiv1namespacesnamespaceendpointsname

dongjiang1989 avatar Feb 06 '24 08:02 dongjiang1989

Could you add test cases to cover this?

monkeyDluffy6017 avatar Feb 06 '24 09:02 monkeyDluffy6017

@dongjiang1989 you could follow these test cases: https://github.com/apache/apisix/blob/master/t/kubernetes/discovery/kubernetes2.t

monkeyDluffy6017 avatar Feb 07 '24 10:02 monkeyDluffy6017

@dongjiang1989 you could follow these test cases: https://github.com/apache/apisix/blob/master/t/kubernetes/discovery/kubernetes2.t

Thanks. I got it!

dongjiang1989 avatar Feb 08 '24 01:02 dongjiang1989

If possible, please also provide some reproduction steps to show how the current state leads to a bug/failure. Thanks.

shreemaan-abhishek avatar Feb 10 '24 17:02 shreemaan-abhishek

@zhixiongdu027 Could you help to review this pr?

monkeyDluffy6017 avatar Feb 18 '24 03:02 monkeyDluffy6017

ignore e2e test case

dongjiang1989 avatar Feb 20 '24 01:02 dongjiang1989

It's a very little typo, @AlinsRan and i have checked twice, so i think missing test cases is ok

monkeyDluffy6017 avatar Feb 22 '24 06:02 monkeyDluffy6017

A little weird I want to say, does this mean that before this pr being merged, the apisix k8s discovery function is always broken?

caibirdme avatar Jul 15 '24 15:07 caibirdme