From 88246b669f5dc1b7a2ea95fbbbe4192fe8d7e606 Mon Sep 17 00:00:00 2001 From: Sanket Date: Sat, 2 Mar 2024 16:51:01 +0530 Subject: [PATCH 1/7] newDefault for confighttp package structs --- .chloggen/confighttp-add-newdefault-func.yaml | 25 +++++++++++++++++++ config/confighttp/confighttp.go | 9 +++++++ config/confighttp/confighttp_test.go | 6 +++++ 3 files changed, 40 insertions(+) create mode 100644 .chloggen/confighttp-add-newdefault-func.yaml diff --git a/.chloggen/confighttp-add-newdefault-func.yaml b/.chloggen/confighttp-add-newdefault-func.yaml new file mode 100644 index 00000000000..eb88a85fa46 --- /dev/null +++ b/.chloggen/confighttp-add-newdefault-func.yaml @@ -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: breaking + +# 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: Adds `NewDefault*` functions for all the config structs. + +# One or more tracking issues or pull requests related to the change +issues: [9655] + +# (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] diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index 303075571db..7d5e0d03ce3 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -286,6 +286,11 @@ type ServerConfig struct { ResponseHeaders map[string]configopaque.String `mapstructure:"response_headers"` } +// NewDefaultServerConfig creates new ServerConfig with default values set +func NewDefaultServerConfig() ServerConfig { + return ServerConfig{} +} + // Deprecated: [v0.99.0] Use ToListener instead. func (hss *ServerConfig) ToListenerContext(ctx context.Context) (net.Listener, error) { return hss.ToListener(ctx) @@ -447,6 +452,10 @@ type CORSConfig struct { MaxAge int `mapstructure:"max_age"` } +// NewDefaultCORSConfig creates a new CORSConfig with defaults value set +func NewDefaultCORSConfig() CORSConfig { + return CORSConfig{} +} func authInterceptor(next http.Handler, server auth.Server) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctx, err := server.Authenticate(r.Context(), r.Header) diff --git a/config/confighttp/confighttp_test.go b/config/confighttp/confighttp_test.go index 635f9415a2f..4a6b5fbaa70 100644 --- a/config/confighttp/confighttp_test.go +++ b/config/confighttp/confighttp_test.go @@ -49,6 +49,12 @@ var ( nonExistingID = component.MustNewID("nonexisting") ) +func TestNewDefaultServerConfig(t *testing.T) { + expectedServerConfig := ServerConfig{} + serverConfig := NewDefaultServerConfig() + require.Equal(t, expectedServerConfig, serverConfig) +} + func TestAllHTTPClientSettings(t *testing.T) { host := &mockHost{ ext: map[component.ID]component.Component{ From 05b35c8ff0febef02994bb798ba7855458397b12 Mon Sep 17 00:00:00 2001 From: Sanket Date: Sat, 2 Mar 2024 18:12:41 +0530 Subject: [PATCH 2/7] remaining tests --- config/confighttp/confighttp_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/config/confighttp/confighttp_test.go b/config/confighttp/confighttp_test.go index 4a6b5fbaa70..f6d9877819b 100644 --- a/config/confighttp/confighttp_test.go +++ b/config/confighttp/confighttp_test.go @@ -55,6 +55,12 @@ func TestNewDefaultServerConfig(t *testing.T) { require.Equal(t, expectedServerConfig, serverConfig) } +func TestNewDefaultCORSConfig(t *testing.T){ + expectedCORSConfig := CORSConfig{} + corsConfig := NewDefaultCORSConfig() + require.Equal(t, expectedCORSConfig, corsConfig) +} + func TestAllHTTPClientSettings(t *testing.T) { host := &mockHost{ ext: map[component.ID]component.Component{ From f77a13ca15b1a085209fc80ad0b9de90d219ec7a Mon Sep 17 00:00:00 2001 From: Sanket Date: Sat, 2 Mar 2024 18:30:59 +0530 Subject: [PATCH 3/7] go format --- config/confighttp/confighttp_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/confighttp/confighttp_test.go b/config/confighttp/confighttp_test.go index f6d9877819b..9440c5e8754 100644 --- a/config/confighttp/confighttp_test.go +++ b/config/confighttp/confighttp_test.go @@ -55,7 +55,7 @@ func TestNewDefaultServerConfig(t *testing.T) { require.Equal(t, expectedServerConfig, serverConfig) } -func TestNewDefaultCORSConfig(t *testing.T){ +func TestNewDefaultCORSConfig(t *testing.T) { expectedCORSConfig := CORSConfig{} corsConfig := NewDefaultCORSConfig() require.Equal(t, expectedCORSConfig, corsConfig) From 2fd098550cb2c3b11cdea1976fe112726def89a4 Mon Sep 17 00:00:00 2001 From: Sanket Date: Mon, 4 Mar 2024 17:02:13 +0530 Subject: [PATCH 4/7] change_type- enhancement --- .chloggen/confighttp-add-newdefault-func.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.chloggen/confighttp-add-newdefault-func.yaml b/.chloggen/confighttp-add-newdefault-func.yaml index eb88a85fa46..d4f7f369947 100644 --- a/.chloggen/confighttp-add-newdefault-func.yaml +++ b/.chloggen/confighttp-add-newdefault-func.yaml @@ -1,7 +1,7 @@ # Use this changelog template to create an entry for release notes. # One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' -change_type: breaking +change_type: enhancement # The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) component: confighttp From 90fb2034af484009ee292e2b27cff9eff4337b2b Mon Sep 17 00:00:00 2001 From: Sanket Date: Mon, 11 Mar 2024 00:12:09 +0530 Subject: [PATCH 5/7] Added the check for map value set to nil, hence initialised it with empty map instead --- config/confighttp/confighttp.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index 7d5e0d03ce3..7fbd6290ad5 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -105,8 +105,14 @@ type ClientConfig struct { // the default values of 'MaxIdleConns' and 'IdleConnTimeout'. // Other config options are not added as they are initialized with 'zero value' by GoLang as default. // We encourage to use this function to create an object of ClientConfig. -func NewDefaultClientConfig() ClientConfig { +func (c *ClientConfig) NewDefaultClientConfig() ClientConfig { // The default values are taken from the values of 'DefaultTransport' of 'http' package. + + //Check Whether the headers are set to nil, if yes then set it to default map. + if c.Headers == nil { + c.Headers = make(map[string]configopaque.String) + } + maxIdleConns := 100 idleConnTimeout := 90 * time.Second @@ -287,7 +293,13 @@ type ServerConfig struct { } // NewDefaultServerConfig creates new ServerConfig with default values set -func NewDefaultServerConfig() ServerConfig { +func (c *ServerConfig) NewDefaultServerConfig() ServerConfig { + + //Check Whether the ResponseHeaders are set to nil, if yes then set it to default map. + if c.ResponseHeaders == nil { + c.ResponseHeaders = make(map[string]configopaque.String) + } + return ServerConfig{} } @@ -456,6 +468,7 @@ type CORSConfig struct { func NewDefaultCORSConfig() CORSConfig { return CORSConfig{} } + func authInterceptor(next http.Handler, server auth.Server) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctx, err := server.Authenticate(r.Context(), r.Header) From cbeb9339310a369f082478095e56752220e480ad Mon Sep 17 00:00:00 2001 From: Sanket Date: Mon, 11 Mar 2024 01:51:17 +0530 Subject: [PATCH 6/7] initialised map fields with non-nil map values, also added the necessary tests --- config/confighttp/confighttp.go | 20 +++++++++----------- config/confighttp/confighttp_test.go | 5 ++++- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index 7fbd6290ad5..4126fc86ed2 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -105,18 +105,17 @@ type ClientConfig struct { // the default values of 'MaxIdleConns' and 'IdleConnTimeout'. // Other config options are not added as they are initialized with 'zero value' by GoLang as default. // We encourage to use this function to create an object of ClientConfig. -func (c *ClientConfig) NewDefaultClientConfig() ClientConfig { +func NewDefaultClientConfig() ClientConfig { // The default values are taken from the values of 'DefaultTransport' of 'http' package. - //Check Whether the headers are set to nil, if yes then set it to default map. - if c.Headers == nil { - c.Headers = make(map[string]configopaque.String) - } + // Configure with the non-nil map value. + headers := make(map[string]configopaque.String) maxIdleConns := 100 idleConnTimeout := 90 * time.Second return ClientConfig{ + Headers: headers, MaxIdleConns: &maxIdleConns, IdleConnTimeout: &idleConnTimeout, } @@ -293,14 +292,13 @@ type ServerConfig struct { } // NewDefaultServerConfig creates new ServerConfig with default values set -func (c *ServerConfig) NewDefaultServerConfig() ServerConfig { +func NewDefaultServerConfig() ServerConfig { + // Create a new map for ResponseHeaders + responseHeaders := make(map[string]configopaque.String) - //Check Whether the ResponseHeaders are set to nil, if yes then set it to default map. - if c.ResponseHeaders == nil { - c.ResponseHeaders = make(map[string]configopaque.String) + return ServerConfig{ + ResponseHeaders: responseHeaders, } - - return ServerConfig{} } // Deprecated: [v0.99.0] Use ToListener instead. diff --git a/config/confighttp/confighttp_test.go b/config/confighttp/confighttp_test.go index 9440c5e8754..b001837663e 100644 --- a/config/confighttp/confighttp_test.go +++ b/config/confighttp/confighttp_test.go @@ -50,7 +50,9 @@ var ( ) func TestNewDefaultServerConfig(t *testing.T) { - expectedServerConfig := ServerConfig{} + expectedServerConfig := ServerConfig{ + ResponseHeaders: map[string]configopaque.String{}, + } serverConfig := NewDefaultServerConfig() require.Equal(t, expectedServerConfig, serverConfig) } @@ -251,6 +253,7 @@ func TestPartialHTTPClientSettings(t *testing.T) { func TestDefaultHTTPClientSettings(t *testing.T) { httpClientSettings := NewDefaultClientConfig() + assert.EqualValues(t, map[string]configopaque.String{}, httpClientSettings.Headers) assert.EqualValues(t, 100, *httpClientSettings.MaxIdleConns) assert.EqualValues(t, 90*time.Second, *httpClientSettings.IdleConnTimeout) } From 89d1c0d239bf1f9f957c77efd227b6518c221696 Mon Sep 17 00:00:00 2001 From: Sanket Date: Mon, 11 Mar 2024 16:46:11 +0530 Subject: [PATCH 7/7] no need to initialised with one more variable --- config/confighttp/confighttp.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index 4126fc86ed2..c2ded2684d9 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -108,14 +108,11 @@ type ClientConfig struct { func NewDefaultClientConfig() ClientConfig { // The default values are taken from the values of 'DefaultTransport' of 'http' package. - // Configure with the non-nil map value. - headers := make(map[string]configopaque.String) - maxIdleConns := 100 idleConnTimeout := 90 * time.Second return ClientConfig{ - Headers: headers, + Headers: make(map[string]configopaque.String), MaxIdleConns: &maxIdleConns, IdleConnTimeout: &idleConnTimeout, } @@ -293,11 +290,9 @@ type ServerConfig struct { // NewDefaultServerConfig creates new ServerConfig with default values set func NewDefaultServerConfig() ServerConfig { - // Create a new map for ResponseHeaders - responseHeaders := make(map[string]configopaque.String) return ServerConfig{ - ResponseHeaders: responseHeaders, + ResponseHeaders: make(map[string]configopaque.String), } }