Skip to content
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

Passthrough CORS allowed origins #260

Merged
merged 7 commits into from
Aug 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions receiver/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,6 @@ The full list of settings exposed for this receiver are documented [here](https:
with detailed sample configurations [here](https://github.com/open-telemetry/opentelemetry-service/blob/master/receiver/opencensusreceiver/testdata/config.yaml).

### Writing with HTTP/JSON
// TODO(ccaraman) The cors setting wasn't ported accidentally. In a follow up
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The next line is a bit outdated, like referring to OpenCensus. Could you reword it to mention Otel and make it clearer that the receiver handles both HTTP and gRPC.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit unclear here - is calling it the OpenCensus receiver incorrect? My understanding was that it's still being referred to as the OpenCensus receiver inside of Otel

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦‍♀ apologies my coffee is fading and rereading it a few more times I see that I was thinking about it in the wrong context.
Ignore this comment!

pr, add the functionality, tests and update this section of documentation.

The OpenCensus receiver for the agent can receive trace export calls via
HTTP/JSON in addition to gRPC. The HTTP/JSON address is the same as gRPC as the
protocol is recognized and processed accordingly.
Expand All @@ -81,13 +78,13 @@ format parallels the gRPC protobuf format, see this

The HTTP/JSON endpoint can also optionally
[CORS](https://fetch.spec.whatwg.org/#cors-protocol), which is enabled by
specifying a list of allowed CORS origins in the `cors_allowed_origins` field:
specifying a list of allowed CORS origins in the `cors-allowed-origins` field:

```yaml
receivers:
opencensus:
endpoint: "localhost:55678"
cors_allowed_origins:
cors-allowed-origins:
- http://test.com
# Origins can have wildcards with *, use * by itself to match any origin.
- https://*.example.com
Expand Down
12 changes: 9 additions & 3 deletions receiver/opencensusreceiver/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ type Config struct {
// TLSCredentials is a (cert_file, key_file) configuration.
TLSCredentials *tlsCredentials `mapstructure:"tls-credentials,omitempty"`

// CorsOrigins are the allowed CORS origins for HTTP/JSON requests to grpc-gateway adapter
// for the OpenCensus receiver. See github.com/rs/cors
// An empty list means that CORS is not enabled at all. A wildcard (*) can be
// used to match any origin or one or more characters of an origin.
CorsOrigins []string `mapstructure:"cors-allowed-origins"`

// Keepalive anchor for all the settings related to keepalive.
Keepalive *serverParametersAndEnforcementPolicy `mapstructure:"keepalive,omitempty"`

Expand All @@ -40,9 +46,6 @@ type Config struct {

// MaxConcurrentStreams sets the limit on the number of concurrent streams to each ServerTransport.
MaxConcurrentStreams uint32 `mapstructure:"max-concurrent-streams,omitempty"`

// TODO(ccaramn): Add cors-allowed-origin setting. It seems to have been accidentally forgotten
// during OC to OTel port.
}

// tlsCredentials holds the fields for TLS credentials
Expand Down Expand Up @@ -89,6 +92,9 @@ func (rOpts *Config) buildOptions() (opts []Option, err error) {
if hasTLSCreds {
opts = append(opts, tlsCredsOption)
}
if len(rOpts.CorsOrigins) > 0 {
opts = append(opts, WithCorsOrigins(rOpts.CorsOrigins))
}

grpcServerOptions := rOpts.grpcServerOptions()
if len(grpcServerOptions) > 0 {
Expand Down
13 changes: 12 additions & 1 deletion receiver/opencensusreceiver/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func TestLoadConfig(t *testing.T) {

// Currently disabled receivers are removed from the total list of receivers so 'opencensus/disabled' doesn't
// contribute to the count.
assert.Equal(t, len(cfg.Receivers), 5)
assert.Equal(t, len(cfg.Receivers), 6)

r0 := cfg.Receivers["opencensus"]
assert.Equal(t, r0, factory.CreateDefaultConfig())
Expand Down Expand Up @@ -109,4 +109,15 @@ func TestLoadConfig(t *testing.T) {
KeyFile: "test.key",
},
})

r5 := cfg.Receivers["opencensus/cors"].(*Config)
assert.Equal(t, r5,
&Config{
ReceiverSettings: configmodels.ReceiverSettings{
TypeVal: typeStr,
NameVal: "opencensus/cors",
Endpoint: "127.0.0.1:55678",
},
CorsOrigins: []string{"https://*.test.com", "https://test.com"},
})
}
7 changes: 7 additions & 0 deletions receiver/opencensusreceiver/testdata/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,13 @@ receivers:
# This receiver is disabled and won't receive any data.
disabled: true


# The following entry demonstrates how to configure the OpenCensus receiver to allow Cross-Origin Resource Sharing (CORS).
# Both fully qualified domain names and the use of wildcards are supported.
opencensus/cors:
cors-allowed-origins:
- https://*.test.com # Wildcard subdomain. Allows domains like https://www.test.com and https://foo.test.com but not https://wwwtest.com.
- https://test.com # Fully qualified domain name. Allows https://test.com only.
processors:
exampleprocessor:

Expand Down