-
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) redirect requests from gui server to api server #520
Conversation
app/kuma-ui/pkg/server/server.go
Outdated
srv := Server{rt.Config().GuiServer} | ||
srv := Server{ | ||
Config: rt.Config().GuiServer, | ||
ApiServerPort: rt.Config().ApiServer.Port, |
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 ApiServerPort
is not a part of Config
?
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.
Moved as a part of the configuration.
app/kuma-ui/pkg/server/server.go
Outdated
@@ -64,6 +76,21 @@ func (g *Server) Start(stop <-chan struct{}) error { | |||
} | |||
} | |||
|
|||
func (g *Server) apiHandler() (http.Handler, error) { | |||
apiServerUrl, err := url.Parse(fmt.Sprintf("http://localhost:%d", g.ApiServerPort)) |
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 do we need to hardcode it here instead of making it a part of configuration ?
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.
Moved as a part of the configuration.
app/kuma-ui/pkg/server/server.go
Outdated
if err := core_runtime.Add(rt, &srv); err != nil { | ||
return err | ||
} | ||
return nil | ||
} | ||
|
||
type Server struct { | ||
Config *gui_server.GuiServerConfig | ||
Config *gui_server.GuiServerConfig |
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 deprecate the old ApiUrl
field ?
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.
Added deprecation.
174f7fb
to
321fba4
Compare
@yskopets @jakubdyszkiewicz what does this mean for the way I'm fetching the API and config? Is there anything I need to change or will it still work as is? |
Summary
Redirect requests from GUI on /api prefix to API Server. This simplifies setup since you don't have to coordinate exposing GUI and API server and configuring proper external address.