[Feat][apiserver] Support CORS config #3711
Conversation
Signed-off-by: Chi-Sheng Liu <[email protected]>
MortalHappiness
force-pushed
the
feature/apiserver-cors-config
branch
from
6c97057 to
fe6d95d
Compare
apiserver/cmd/main.go
Outdated
| @@ -165,8 +165,27 @@ func startHttpProxy() { | |||
| topMux = http.NewServeMux() | |||
| } | |||
|
|
|||
| // Seems /apis (matches /apis/v1alpha1/clusters) works fine | |||
| topMux.Handle("/", runtimeMux) | |||
| if allowedOrigin := os.Getenv("CORS_ALLOW_ORIGIN"); allowedOrigin != "" { | |||
There was a problem hiding this comment.
Print a log so that users can know whether CORS is enabled or not?
| @@ -11,6 +11,9 @@ image: | |||
| tag: nightly | |||
| pullPolicy: IfNotPresent | |||
|
|
|||
| cors: | |||
| # allowOrigin: "*" | |||
There was a problem hiding this comment.
What does "*" mean? Can you add some comments to explain what are valid values and the expected behavior?
| @@ -52,6 +52,13 @@ spec: | |||
| httpGet: | |||
| path: /healthz | |||
| port: http | |||
| env: | |||
There was a problem hiding this comment.
Maybe pass to args instead so that we don't need to handle the complex merge logic of env in the future.
apiserver/cmd/main.go
Outdated
| @@ -165,8 +165,27 @@ func startHttpProxy() { | |||
| topMux = http.NewServeMux() | |||
| } | |||
|
|
|||
| // Seems /apis (matches /apis/v1alpha1/clusters) works fine | |||
| topMux.Handle("/", runtimeMux) | |||
| if allowedOrigin := os.Getenv("CORS_ALLOW_ORIGIN"); allowedOrigin != "" { | |||
There was a problem hiding this comment.
This seems to be a breaking change for users when users use their old Values.yaml with the new api server image.
The same YAML with old image will enable CORS, but CORS will be disabled with the new image.
There was a problem hiding this comment.
What do you mean? If users use old values.yaml, then CORS_ALLOW_ORIGIN won't be set, and the else block will be executed. So topMux.Handle("/", runtimeMux) will still be run. There is no behavior change.
apiserver/cmd/main.go
Outdated
| }) | ||
| } | ||
|
|
||
| topMux.Handle("/", corsHandler(runtimeMux)) |
There was a problem hiding this comment.
Could we just use https://github.com/rs/cors?
Signed-off-by: Chi-Sheng Liu <[email protected]>
Signed-off-by: Chi-Sheng Liu <[email protected]>
Signed-off-by: Chi-Sheng Liu <[email protected]>
Why are these changes needed?
Add CORS config to apiserver v1, so that KubeRay dashboard can be easily run.
Manual testing:
Related issue number
N/A
Checks