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

[config/confighttp] Deprecate CORSSettings, use CORSConfig instead #9392

Merged
merged 2 commits into from
Jan 26, 2024
Merged
Show file tree
Hide file tree
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
25 changes: 25 additions & 0 deletions .chloggen/corssettings_corsconfig.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: deprecation

# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
component: confighttp

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Deprecate CORSSettings, use CORSConfig instead

# One or more tracking issues or pull requests related to the change
issues: [6767]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:

# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: [api]
9 changes: 7 additions & 2 deletions config/confighttp/confighttp.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ type HTTPServerSettings struct {
TLSSetting *configtls.TLSServerSetting `mapstructure:"tls"`

// CORS configures the server for HTTP cross-origin resource sharing (CORS).
CORS *CORSSettings `mapstructure:"cors"`
CORS *CORSConfig `mapstructure:"cors"`

// Auth for this receiver
Auth *configauth.Authentication `mapstructure:"auth"`
Expand Down Expand Up @@ -397,7 +397,12 @@ func responseHeadersHandler(handler http.Handler, headers map[string]configopaqu

// CORSSettings configures a receiver for HTTP cross-origin resource sharing (CORS).
// See the underlying https://github.com/rs/cors package for details.
type CORSSettings struct {
// Deprecated: [v0.94.0] Use CORSConfig instead
type CORSSettings CORSConfig
atoulme marked this conversation as resolved.
Show resolved Hide resolved

// CORSConfig configures a receiver for HTTP cross-origin resource sharing (CORS).
// See the underlying https://github.com/rs/cors package for details.
type CORSConfig struct {
// AllowedOrigins sets the allowed values of the Origin header for
// HTTP/JSON requests to an OTLP receiver. An origin may contain a
// wildcard (*) to replace 0 or more characters (e.g.,
Expand Down
26 changes: 13 additions & 13 deletions config/confighttp/confighttp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -746,7 +746,7 @@ func TestHttpCors(t *testing.T) {
tests := []struct {
name string

*CORSSettings
*CORSConfig

allowedWorks bool
disallowedWorks bool
Expand All @@ -760,14 +760,14 @@ func TestHttpCors(t *testing.T) {
},
{
name: "emptyCORS",
CORSSettings: &CORSSettings{},
CORSConfig: &CORSConfig{},
allowedWorks: false,
disallowedWorks: false,
extraHeaderWorks: false,
},
{
name: "OriginCORS",
CORSSettings: &CORSSettings{
CORSConfig: &CORSConfig{
AllowedOrigins: []string{"allowed-*.com"},
},
allowedWorks: true,
Expand All @@ -776,7 +776,7 @@ func TestHttpCors(t *testing.T) {
},
{
name: "CacheableCORS",
CORSSettings: &CORSSettings{
CORSConfig: &CORSConfig{
AllowedOrigins: []string{"allowed-*.com"},
MaxAge: 360,
},
Expand All @@ -786,7 +786,7 @@ func TestHttpCors(t *testing.T) {
},
{
name: "HeaderCORS",
CORSSettings: &CORSSettings{
CORSConfig: &CORSConfig{
AllowedOrigins: []string{"allowed-*.com"},
AllowedHeaders: []string{"ExtraHeader"},
},
Expand All @@ -800,7 +800,7 @@ func TestHttpCors(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
hss := &HTTPServerSettings{
Endpoint: "localhost:0",
CORS: tt.CORSSettings,
CORS: tt.CORSConfig,
}

ln, err := hss.ToListener()
Expand All @@ -821,18 +821,18 @@ func TestHttpCors(t *testing.T) {
url := fmt.Sprintf("http://%s", ln.Addr().String())

expectedStatus := http.StatusNoContent
if tt.CORSSettings == nil || len(tt.AllowedOrigins) == 0 {
if tt.CORSConfig == nil || len(tt.AllowedOrigins) == 0 {
expectedStatus = http.StatusOK
}

// Verify allowed domain gets responses that allow CORS.
verifyCorsResp(t, url, "allowed-origin.com", tt.CORSSettings, false, expectedStatus, tt.allowedWorks)
verifyCorsResp(t, url, "allowed-origin.com", tt.CORSConfig, false, expectedStatus, tt.allowedWorks)

// Verify allowed domain and extra headers gets responses that allow CORS.
verifyCorsResp(t, url, "allowed-origin.com", tt.CORSSettings, true, expectedStatus, tt.extraHeaderWorks)
verifyCorsResp(t, url, "allowed-origin.com", tt.CORSConfig, true, expectedStatus, tt.extraHeaderWorks)

// Verify disallowed domain gets responses that disallow CORS.
verifyCorsResp(t, url, "disallowed-origin.com", tt.CORSSettings, false, expectedStatus, tt.disallowedWorks)
verifyCorsResp(t, url, "disallowed-origin.com", tt.CORSConfig, false, expectedStatus, tt.disallowedWorks)

require.NoError(t, s.Close())
})
Expand All @@ -842,7 +842,7 @@ func TestHttpCors(t *testing.T) {
func TestHttpCorsInvalidSettings(t *testing.T) {
hss := &HTTPServerSettings{
Endpoint: "localhost:0",
CORS: &CORSSettings{AllowedHeaders: []string{"some-header"}},
CORS: &CORSConfig{AllowedHeaders: []string{"some-header"}},
}

// This effectively does not enable CORS but should also not cause an error
Expand All @@ -858,7 +858,7 @@ func TestHttpCorsInvalidSettings(t *testing.T) {
func TestHttpCorsWithSettings(t *testing.T) {
hss := &HTTPServerSettings{
Endpoint: "localhost:0",
CORS: &CORSSettings{
CORS: &CORSConfig{
AllowedOrigins: []string{"*"},
},
Auth: &configauth.Authentication{
Expand Down Expand Up @@ -944,7 +944,7 @@ func TestHttpServerHeaders(t *testing.T) {
}
}

func verifyCorsResp(t *testing.T, url string, origin string, set *CORSSettings, extraHeader bool, wantStatus int, wantAllowed bool) {
func verifyCorsResp(t *testing.T, url string, origin string, set *CORSConfig, extraHeader bool, wantStatus int, wantAllowed bool) {
req, err := http.NewRequest(http.MethodOptions, url, nil)
require.NoError(t, err, "Error creating trace OPTIONS request: %v", err)
req.Header.Set("Origin", origin)
Expand Down
4 changes: 2 additions & 2 deletions receiver/otlpreceiver/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,10 @@ Config defines configuration for OTLP receiver.
|-----------------------|-----------------------------------------------------------|--------------|-----------------------------------------------------------------------------------------------------------------------------------------|
| endpoint | string | 0.0.0.0:4318 | Endpoint configures the listening address for the server. |
| tls | [configtls-TLSServerSetting](#configtls-tlsserversetting) | <no value> | TLSSetting struct exposes TLS client configuration. |
| cors | [confighttp-CORSSettings](#confighttp-corssettings) | <no value> | CORSSettings configures a receiver for HTTP cross-origin resource sharing (CORS). |
| cors | [confighttp-CORSConfig](#confighttp-corsconfig) | <no value> | CORSConfig configures a receiver for HTTP cross-origin resource sharing (CORS). |
| max_request_body_size | int | 0 | MaxRequestBodySize configures the maximum allowed body size in bytes for a single request. The default `0` means there's no restriction |

### confighttp-CORSSettings
### confighttp-CORSConfig

| Name | Type | Default | Docs |
|-----------------|----------|------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
Expand Down
2 changes: 1 addition & 1 deletion receiver/otlpreceiver/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func TestUnmarshalConfig(t *testing.T) {
KeyFile: "test.key",
},
},
CORS: &confighttp.CORSSettings{
CORS: &confighttp.CORSConfig{
AllowedOrigins: []string{"https://*.test.com", "https://test.com"},
MaxAge: 7200,
},
Expand Down
Loading