Add CORS handling to query API

[EranOfer]

Requirement - what kind of business use case are you trying to solve?

I love jaeger I think it's a great tool,

I want to use jaeger data with Costume UI.
I think the best approach for me is to use HTTP JSON (internal) API

Problem - what in Jaeger blocks you from solving the requirement?

Without support CORS headers (Access-Control-Allow-Origin: *)
I can't use the query service from UI applications running on different hosts.

[yurishkuro]

More generally, we should support a config file for the UI that enables various site-specific headers. For example, in our internal build of the UI we're mounting middleware that adds Content-Security-Policy header, to allow embedding Jaeger UI views into other web tools.

[joe-elliott]

@yurishkuro Can you assign to me? I'd be glad to give this one a shot.

[yurishkuro]

Assigned. Please write a proposal for the configuration format first.

[joe-elliott]

Thanks. Proposing the ability of the operator to add arbitrary key value pairs under a new config key: query.additional-headers.

Config file:

query:
  additional-headers:
    access-control-allow-origin: blerg
    whatever: thing

CLI:

--query.additional-headers "access-control-allow-origin=blerg,whatever=thing"

These headers would then be added via middleware here

jaeger/cmd/query/app/http_handler.go

Line 123 in 9cd7a7e

func (aH *APIHandler) handleFunc(

which should impact all jaeger-query http api endpoints.

[yurishkuro]

hm, perhaps. When I look at how we're setting "Content-Security-Policy", there are like 10 lines of content for the header, separated with semi-colons. Do you think comma as a separator between different headers is sufficient, if we use a CLI flag?

[joe-elliott]

CLI would just generally be difficult to configure very complex or large headers regardless of how we structure it. Certainly for a 10 line header the config file would generally be preferred.

Unfortunately, comma is a valid character in an HTTP header value. Another option would be to do this:

--query.additional-headers "access-control-allow-origin=blerg" --query.additional-headers "whatever=thing"

It's a little more verbose but would be parseable without ambiguity.

[yurishkuro]

I never tried it - does spf13/cobra allow repeating the same flag this way? If it does, then I think it's a better solution than using a separator character.

[joe-elliott]

So, technically, this is possible by doing this:
https://stackoverflow.com/questions/28322997/how-to-get-a-list-of-values-into-a-flag-in-golang

Unfortunately all CLI flags are treated as strings
https://github.com/spf13/viper/blob/master/viper.go#L1069-L1089

So in the String() method:

func (i *arrayFlags) String() string {
    return "my string representation"
}

I would have to return the values delimited by an illegal character in an HTTP header value and then parse them once I get them from viper. So gross but doable.

The other option would be to play nice with viper and just do a comma delimited list of header key/value pairs (which is what it wants for a string slice). However in this case if you really needed a comma in your header value you'd have to put it in a config file. Obviously, this could get a bit confusing.

[yurishkuro]

spf13/cobra#661 seems like string slice is already supported in pflag.

[joe-elliott]

It is supported in pflag, but in the initialization code I am given a viper object:

func (qOpts *QueryOptions) InitFromViper(v *viper.Viper) *QueryOptions {
...
}

By debugging into calls likes v.GetString() or v.GetStringSlice() I can see that internally viper is holding a pFlag object. However, in that code I linked above it always calls flag.ValueString() which drops whatever the internal object type is and always returns its string representation (which is then converted back to the requested type).

If there's a way to get the raw pFlag struct through viper, I cannot find it.

[yurishkuro]

yeah, the abstraction doesn't seem to be there. But it may still work as long as we're ok with comma as a separator, which is not a valid character for an http header values, because if it's used it is treated by the HTTP protocol as a separator, equivalent to repeated singe-valued headers. The pflag converts slice into comma-separated list:

func (s *stringSliceValue) String() string {
	str, _ := writeAsCSV(*s.value)
	return "[" + str + "]"
}

and viper parses it back:

		case "stringSlice":
			s := strings.TrimPrefix(flag.ValueString(), "[")
			s = strings.TrimSuffix(s, "]")
			res, _ := readAsCSV(s)
			return res

[joe-elliott]

Ok, I can get that to work. It does require this: https://stackoverflow.com/questions/28322997/how-to-get-a-list-of-values-into-a-flag-in-golang. If I don't use this custom flag var then I simply get the last value specified when calling GetStringSlice.

This means that the cli would look like:

--query.additional-headers "access-control-allow-origin=blerg" --query.additional-headers "whatever=thing"

and the yaml:

query:
  additional-headers:
  - access-control-allow-origin=blerg
  - whatever=thing

[joe-elliott]

@yurishkuro do you think this is ok to move forward with?

[yurishkuro]

sgtm