Add cors_allowed_headers option to confighttp (#2454)

* Add cors_allowed_headers option to confighttp

* Enable CORS only when origin is defined

* Remove WithLogger option

The capability is out of scope of the PR

config/confighttp/README.md

@@ -39,6 +39,10 @@ leverage server configuration.
 - [`cors_allowed_origins`](https://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.
+- [`cors_allowed_headers`](https://github.com/rs/cors): When CORS is enabled,
+  can be used to specify an optional list of allowed headers. By default, it includes `Accept`, 
+  `Content-Type`, `X-Requested-With`. `Origin` is also always
+  added to the list. A wildcard (`*`) can be used to match any header.
 - `endpoint`: Valid value syntax available [here](https://github.com/grpc/grpc/blob/master/doc/naming.md)
 - [`tls_settings`](../configtls/README.md)
 
@@ -50,6 +54,8 @@ receivers:
     cors_allowed_origins:
     - https://foo.bar.com
     - https://*.test.com
+    cors_allowed_headers:
+    - ExampleHeader
     endpoint: 0.0.0.0:55690
     protocols:
       http:

config/confighttp/confighttp.go

@@ -114,6 +114,12 @@ type HTTPServerSettings struct {
 	// 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"`
+
+	// CorsHeaders are the allowed CORS headers for HTTP/JSON requests to grpc-gateway adapter
+	// for the OTLP receiver. See github.com/rs/cors
+	// CORS needs to be enabled first by providing a non-empty list in CorsOrigins
+	// A wildcard (*) can be used to match any header.
+	CorsHeaders []string `mapstructure:"cors_allowed_headers"`
 }
 
 func (hss *HTTPServerSettings) ToListener() (net.Listener, error) {
@@ -155,9 +161,11 @@ func (hss *HTTPServerSettings) ToServer(handler http.Handler, opts ...ToServerOp
 		o(serverOpts)
 	}
 	if len(hss.CorsOrigins) > 0 {
-		co := cors.Options{AllowedOrigins: hss.CorsOrigins}
+		co := cors.Options{AllowedOrigins: hss.CorsOrigins, AllowedHeaders: hss.CorsHeaders}
 		handler = cors.New(co).Handler(handler)
 	}
+	// TODO: emit a warning when non-empty CorsHeaders and empty CorsOrigins.
+
 	handler = middleware.HTTPContentDecompressor(
 		handler,
 		middleware.WithErrorHandler(serverOpts.errorHandler),

config/confighttp/confighttp_test.go

@@ -316,37 +316,93 @@ func TestHttpReception(t *testing.T) {
 }
 
 func TestHttpCors(t *testing.T) {
-	hss := &HTTPServerSettings{
-		Endpoint:    "localhost:0",
-		CorsOrigins: []string{"allowed-*.com"},
+	tests := []struct {
+		name             string
+		CorsOrigins      []string
+		CorsHeaders      []string
+		allowedWorks     bool
+		disallowedWorks  bool
+		extraHeaderWorks bool
+	}{
+		{
+			name:             "noCORS",
+			allowedWorks:     false,
+			disallowedWorks:  false,
+			extraHeaderWorks: false,
+		},
+		{
+			name:             "OriginCORS",
+			CorsOrigins:      []string{"allowed-*.com"},
+			CorsHeaders:      []string{},
+			allowedWorks:     true,
+			disallowedWorks:  false,
+			extraHeaderWorks: false,
+		},
+		{
+			name:             "HeaderCORS",
+			CorsOrigins:      []string{"allowed-*.com"},
+			CorsHeaders:      []string{"ExtraHeader"},
+			allowedWorks:     true,
+			disallowedWorks:  false,
+			extraHeaderWorks: true,
+		},
 	}
 
-	ln, err := hss.ToListener()
-	assert.NoError(t, err)
-	s := hss.ToServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
-	go func() {
-		_ = s.Serve(ln)
-	}()
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			hss := &HTTPServerSettings{
+				Endpoint:    "localhost:0",
+				CorsOrigins: tt.CorsOrigins,
+				CorsHeaders: tt.CorsHeaders,
+			}
 
-	// TODO: make starting server deterministic
-	// Wait for the servers to start
-	<-time.After(10 * time.Millisecond)
+			ln, err := hss.ToListener()
+			assert.NoError(t, err)
+			s := hss.ToServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
+			go func() {
+				_ = s.Serve(ln)
+			}()
+
+			// TODO: make starting server deterministic
+			// Wait for the servers to start
+			<-time.After(10 * time.Millisecond)
 
-	url := fmt.Sprintf("http://%s", ln.Addr().String())
+			url := fmt.Sprintf("http://%s", ln.Addr().String())
 
-	// Verify allowed domain gets responses that allow CORS.
-	verifyCorsResp(t, url, "allowed-origin.com", 200, true)
+			// Verify allowed domain gets responses that allow CORS.
+			verifyCorsResp(t, url, "allowed-origin.com", false, 200, tt.allowedWorks)
 
-	// Verify disallowed domain gets responses that disallow CORS.
-	verifyCorsResp(t, url, "disallowed-origin.com", 200, false)
+			// Verify allowed domain and extra headers gets responses that allow CORS.
+			verifyCorsResp(t, url, "allowed-origin.com", true, 200, tt.extraHeaderWorks)
 
+			// Verify disallowed domain gets responses that disallow CORS.
+			verifyCorsResp(t, url, "disallowed-origin.com", false, 200, tt.disallowedWorks)
+
+			require.NoError(t, s.Close())
+		})
+	}
+}
+
+func TestHttpCorsInvalidSettings(t *testing.T) {
+	hss := &HTTPServerSettings{
+		Endpoint:    "localhost:0",
+		CorsHeaders: []string{"some-header"},
+	}
+
+	// This effectively does not enable CORS but should also not cause an error
+	s := hss.ToServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
+	require.NotNil(t, s)
 	require.NoError(t, s.Close())
 }
 
-func verifyCorsResp(t *testing.T, url string, origin string, wantStatus int, wantAllowed bool) {
+func verifyCorsResp(t *testing.T, url string, origin string, extraHeader bool, wantStatus int, wantAllowed bool) {
 	req, err := http.NewRequest("OPTIONS", url, nil)
 	require.NoError(t, err, "Error creating trace OPTIONS request: %v", err)
 	req.Header.Set("Origin", origin)
+	if extraHeader {
+		req.Header.Set("ExtraHeader", "foo")
+		req.Header.Set("Access-Control-Request-Headers", "ExtraHeader")
+	}
 	req.Header.Set("Access-Control-Request-Method", "POST")
 
 	resp, err := http.DefaultClient.Do(req)

receiver/otlpreceiver/README.md

@@ -52,7 +52,8 @@ port is `55681`.
 
 The HTTP/JSON endpoint can also optionally configure
 [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`
+and optionally headers in `cors_allowed_headers`:
 
 ```yaml
 receivers:
@@ -64,4 +65,6 @@ receivers:
         - http://test.com
         # Origins can have wildcards with *, use * by itself to match any origin.
         - https://*.example.com
+        cors_allowed_headers:
+        - TestHeader
 ```

receiver/otlpreceiver/config_test.go

@@ -42,7 +42,7 @@ func TestLoadConfig(t *testing.T) {
 	require.NoError(t, err)
 	require.NotNil(t, cfg)
 
-	assert.Equal(t, len(cfg.Receivers), 9)
+	assert.Equal(t, len(cfg.Receivers), 10)
 
 	assert.Equal(t, cfg.Receivers["otlp"], factory.CreateDefaultConfig())
 
@@ -176,6 +176,21 @@ func TestLoadConfig(t *testing.T) {
 			},
 		})
 
+	assert.Equal(t, cfg.Receivers["otlp/corsheader"],
+		&Config{
+			ReceiverSettings: configmodels.ReceiverSettings{
+				TypeVal: typeStr,
+				NameVal: "otlp/corsheader",
+			},
+			Protocols: Protocols{
+				HTTP: &confighttp.HTTPServerSettings{
+					Endpoint:    "0.0.0.0:55681",
+					CorsOrigins: []string{"https://*.test.com", "https://test.com"},
+					CorsHeaders: []string{"ExampleHeader"},
+				},
+			},
+		})
+
 	assert.Equal(t, cfg.Receivers["otlp/uds"],
 		&Config{
 			ReceiverSettings: configmodels.ReceiverSettings{

receiver/otlpreceiver/testdata/config.yaml

@@ -78,6 +78,15 @@ receivers:
         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.
+  # The following entry demonstrates how to use CORS Header configuration.
+  otlp/corsheader:
+    protocols:
+      http:
+        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.
+        cors_allowed_headers:
+          - ExampleHeader
 processors:
   exampleprocessor: