-
Notifications
You must be signed in to change notification settings - Fork 327
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
feat(kuma-cp) endpoints for fetch resources from all meshes #657
Conversation
@@ -15,26 +15,28 @@ import ( | |||
rest_errors "github.com/Kong/kuma/pkg/core/rest/errors" | |||
) | |||
|
|||
type overviewWs struct { | |||
type dataplaneOverviewEndpoints struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why *Endpoint
instead of *Ws
? And I guess it should be consistent with the file name which is *_ws.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ws is WebService from go-restful. It feel like endpoints is better name here since we add endpoints to existing webservice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't want to rename files to avoid conflicts with community PR as well as for it to be easier to review, but since I've got +1 on this then let me change it.
func (r *overviewWs) AddToWs(ws *restful.WebService) { | ||
ws.Route(ws.GET("/{mesh}/dataplanes+insights/{name}").To(r.inspectDataplane). | ||
func (r *dataplaneOverviewEndpoints) addFindEndpoint(ws *restful.WebService, pathPrefix string) { | ||
ws.Route(ws.GET(pathPrefix+"/dataplanes+insights/{name}").To(r.inspectDataplane). | ||
Doc("Inspect a dataplane"). | ||
Param(ws.PathParameter("name", "Name of a dataplane").DataType("string")). | ||
Param(ws.PathParameter("mesh", "Name of a mesh").DataType("string")). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we delete this Param
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or it is expected to be equal to empty string for "all mesh fetch"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is used endpoints.addListEndpoint(ws, "/meshes/{mesh}")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and it is expected to be empty string on the endpoints fetched resources from all meshes
} | ||
} | ||
|
||
type resourceEndpoints struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And probably here makes sense to rename the file to resourse_endpoint
?
f7bd7bc
to
7c88beb
Compare
Summary
Endpoints for fetching resources from all meshes.
Ex.
/traffic-permissions
next to/meshes/{mesh}/traffic-permissions
endpoint