add -cors.origin flag to "zed serve" #4334
Conversation
The -cors.origins flag accepts a comma-separated list of CORS allowed origins. Closes #4297.
|
I've tested out this branch at commit 990520e and confirmed that it does the trick for allowing connections from the Grafana data source proxy at origin |
cmd/zed/serve/command.go
Outdated
| @@ -53,6 +53,8 @@ func New(parent charm.Command, f *flag.FlagSet) (charm.Command, error) { | |||
| c.conf.Version = cli.Version | |||
| c.logflags.SetFlags(f) | |||
| f.IntVar(&c.brimfd, "brimfd", -1, "pipe read fd passed by brim to signal brim closure") | |||
| c.conf.CORSAllowedOrigins = []string{"*.observableusercontent.com", "localhost"} | |||
| f.Var((*cli.CommaStringsFlag)(&c.conf.CORSAllowedOrigins), "cors.origins", "comma-separated list of CORS allowed origins") | |||
There was a problem hiding this comment.
I don't really like doing comma strings, can we rather just have the ability for the user to specify the flag multiple times? I know we did this somewhere...
There was a problem hiding this comment.
Yes, happy to make this change.
There was a problem hiding this comment.
Ah yes, this one:
-I source file containing Zed query text (may be used multiple times)
nwt
changed the title
| @@ -30,9 +30,7 @@ func requestIDMiddleware() mux.MiddlewareFunc { | |||
| } | |||
| } | |||
|
|
|||
| var allowedOrigins = []string{"*.observableusercontent.com", "localhost"} | |||
There was a problem hiding this comment.
Note that I've removed these baked-in values!
There was a problem hiding this comment.
Cool, we need to make sure brim uses this new command argument to add these origins.
There was a problem hiding this comment.
I think it'll work fine without them.
We will however need to add https://*.observableusercontent.com on cloud deployments to keep the Observable integration working.
|
I've re-tested this in the branch at commit 1452539 and it's still a functional 👍 for me. The removal of the baked-in values actually works great for my use case since it means that my target user won't even need to invoke the new option, though it's nice to have it there in case someone wants to lock down the service more tightly. |
The -cors.origin flag specifies a CORS allowed origin. The flag may be repeated.
This change removes the two baked-in allowed origins, *.observableusercontent.com and localhost, and replaces them with a default allowed origin of *. As a consequence, "zed serve" with no -cors.origin flag behaves like "zed serve -cors.origin '*'".
Closes #4297.