Skip to content

Commit

Permalink
[config/confighttp] Deprecate CORSSettings, use CORSConfig instead (#…
Browse files Browse the repository at this point in the history
…9392)

**Description:**
Deprecate CORSSettings, use CORSConfig instead

**Link to tracking Issue:**
#6767
  • Loading branch information
atoulme committed Jan 26, 2024
1 parent d2e35bd commit 9082cb1
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 18 deletions.
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

// 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

0 comments on commit 9082cb1

Please sign in to comment.