-
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
chore(kuma-cp) add customizable API #1174
Conversation
fe74d81
to
9372ccf
Compare
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
9372ccf
to
47930eb
Compare
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
47930eb
to
70b2578
Compare
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.
Nice!
ws := new(restful.WebService).Path(path) | ||
ws.Route(ws.GET(subpath).To(handler)) | ||
c.Add(ws) | ||
} |
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.
What will happen if you need something like Auth on this endpoint or filter of logging requests or add an extra method to this webservice?
I think I'd drop this method and do these 3 lines on the APIManager client that will just use Add
. That's not really extensible API, you cannot add filters, change the method, add many methods to ws, add OpenAPI docs etc.
|
||
apiServer := createTestApiServer(resourceStore, cfg, true, metrics, wsManager) | ||
|
||
stop := make(chan 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.
close the channel
return apiServer | ||
} | ||
|
||
var SampleTrafficRouteWsDefinition = definitions.ResourceWsDefinition{ |
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 don't think we need this for a test
pkg/api-server/server.go
Outdated
@@ -71,7 +73,8 @@ func init() { | |||
} | |||
} | |||
|
|||
func NewApiServer(resManager manager.ResourceManager, defs []definitions.ResourceWsDefinition, cfg *kuma_cp.Config, enableGUI bool, metrics metrics.Metrics) (*ApiServer, error) { | |||
func NewApiServer(resManager manager.ResourceManager, wsManager customization.APIInstaller, | |||
defs []definitions.ResourceWsDefinition, cfg *kuma_cp.Config, enableGUI bool, metrics metrics.Metrics) (*ApiServer, error) { |
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.
formatting nit. For me, it's easier to read arguments grouped in one line or every in it's own line so either
func NewApiServer(resManager manager.ResourceManager, wsManager customization.APIInstaller, defs []definitions.ResourceWsDefinition, cfg *kuma_cp.Config, enableGUI bool, metrics metrics.Metrics) (*ApiServer, error) {
or
func NewApiServer(
resManager manager.ResourceManager,
wsManager customization.APIInstaller,
defs []definitions.ResourceWsDefinition,
cfg *kuma_cp.Config,
enableGUI bool,
metrics metrics.Metrics,
) (*ApiServer, error) {
pkg/api-server/server.go
Outdated
@@ -135,6 +138,10 @@ func NewApiServer(resManager manager.ResourceManager, defs []definitions.Resourc | |||
} else { | |||
container.ServeMux.HandleFunc("/gui/", newApiServer.notAvailableHandler) | |||
} | |||
|
|||
// Install the custom WS |
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.
nit: I don't think this comment is needed. I like commenting out code for the following reasons
- explaining why something is happening
- explaining code design and design decisions / tradeoffs
- explaining difficult code when language is crippling the developer to write an easy to understand code
Here it's pretty obvious what is going on
it's |
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
* chore(*) add customizable API * chore(*) move to APIManager * chore(*) introduce APIManagaer, bootstrapBefore and After Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com> (cherry picked from commit 598dedd)
* chore(*) add customizable API * chore(*) move to APIManager * chore(*) introduce APIManagaer, bootstrapBefore and After Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com> (cherry picked from commit 598dedd) Co-authored-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
Summary
Adding customizable API.
Documentation