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

Add cors_allowed_headers option to confighttp #2454

Merged
merged 3 commits into from
Feb 22, 2021
Merged
Changes from 1 commit
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
Prev Previous commit
Remove WithLogger option
The capability is out of scope of the PR
pmm-sumo committed Feb 15, 2021
commit fc414ee67a79fcbd702b2a54deb13dd01d137058
13 changes: 1 addition & 12 deletions config/confighttp/confighttp.go
Original file line number Diff line number Diff line change
@@ -21,7 +21,6 @@ import (
"time"

"github.com/rs/cors"
"go.uber.org/zap"

"go.opentelemetry.io/collector/config/configtls"
"go.opentelemetry.io/collector/internal/middleware"
@@ -144,18 +143,10 @@ func (hss *HTTPServerSettings) ToListener() (net.Listener, error) {
// returned by HTTPServerSettings.ToServer().
type toServerOptions struct {
errorHandler middleware.ErrorHandler
logger *zap.Logger
}

type ToServerOption func(opts *toServerOptions)

// WithLogger allows to specify optional logger used during initialization.
func WithLogger(logger *zap.Logger) ToServerOption {
return func(opts *toServerOptions) {
opts.logger = logger
}
}

// WithErrorHandler overrides the HTTP error handler that gets invoked
// when there is a failure inside middleware.HTTPContentDecompressor.
func WithErrorHandler(e middleware.ErrorHandler) ToServerOption {
@@ -172,10 +163,8 @@ func (hss *HTTPServerSettings) ToServer(handler http.Handler, opts ...ToServerOp
if len(hss.CorsOrigins) > 0 {
co := cors.Options{AllowedOrigins: hss.CorsOrigins, AllowedHeaders: hss.CorsHeaders}
handler = cors.New(co).Handler(handler)
} else if len(hss.CorsHeaders) > 0 && serverOpts.logger != nil {
serverOpts.logger.Warn(
"CORS needs to be enabled via `cors_allowed_origins` for `cors_allowed_headers` to have an effect")
}
// TODO: emit a warning when non-empty CorsHeaders and empty CorsOrigins.

handler = middleware.HTTPContentDecompressor(
handler,
3 changes: 1 addition & 2 deletions config/confighttp/confighttp_test.go
Original file line number Diff line number Diff line change
@@ -27,7 +27,6 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap"

"go.opentelemetry.io/collector/config/configtls"
)
@@ -391,7 +390,7 @@ func TestHttpCorsInvalidSettings(t *testing.T) {
}

// 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) {}), WithLogger(zap.NewNop()))
s := hss.ToServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
require.NotNil(t, s)
require.NoError(t, s.Close())
}
1 change: 0 additions & 1 deletion receiver/otlpreceiver/otlp.go
Original file line number Diff line number Diff line change
@@ -143,7 +143,6 @@ func (r *otlpReceiver) startProtocolServers(host component.Host) error {
r.serverHTTP = r.cfg.HTTP.ToServer(
r.gatewayMux,
confighttp.WithErrorHandler(errorHandler),
confighttp.WithLogger(r.logger),
)
err = r.startHTTPServer(r.cfg.HTTP, host)
if err != nil {