Fix nil pointer exception in service.NewConfigProvider (#5245)

Deprecated `ConfigProviderSettings.Unmarshaler` is still being used in `service.NewConfigProvider` causing nil pointer exception it's not provided.
open-telemetry · Apr 22, 2022 · 87ab5de · 87ab5de
1 parent dd4096b
commit 87ab5de
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 22 deletions.
diff --git a/service/config_provider.go b/service/config_provider.go
@@ -129,11 +129,16 @@ func NewConfigProvider(set ConfigProviderSettings) (ConfigProvider, error) {
 	mapConvertersCopy := make([]config.MapConverterFunc, len(set.MapConverters))
 	copy(mapConvertersCopy, set.MapConverters)
 
+	unmarshaler := set.Unmarshaler
+	if unmarshaler == nil {
+		unmarshaler = configunmarshaler.NewDefault()
+	}
+
 	return &configProvider{
 		locations:           locationsCopy,
 		configMapProviders:  mapProvidersCopy,
 		configMapConverters: mapConvertersCopy,
-		configUnmarshaler:   set.Unmarshaler,
+		configUnmarshaler:   unmarshaler,
 		watcher:             make(chan error, 1),
 	}, nil
 }

diff --git a/service/config_provider_test.go b/service/config_provider_test.go
@@ -91,11 +91,10 @@ func TestConfigProvider_Errors(t *testing.T) {
 		expectShutdownErr bool
 	}{
 		{
-			name:              "retrieve_err",
-			locations:         []string{"mock:", "not_supported:"},
-			parserProvider:    []config.MapProvider{&mockProvider{}},
-			configUnmarshaler: configunmarshaler.NewDefault(),
-			expectNewErr:      true,
+			name:           "retrieve_err",
+			locations:      []string{"mock:", "not_supported:"},
+			parserProvider: []config.MapProvider{&mockProvider{}},
+			expectNewErr:   true,
 		},
 		{
 			name:      "retrieve_err",
@@ -104,16 +103,14 @@ func TestConfigProvider_Errors(t *testing.T) {
 				&mockProvider{},
 				&mockProvider{scheme: "err", errR: errors.New("retrieve_err")},
 			},
-			configUnmarshaler: configunmarshaler.NewDefault(),
-			expectNewErr:      true,
+			expectNewErr: true,
 		},
 		{
-			name:              "converter_err",
-			locations:         []string{"mock:", filepath.Join("testdata", "otelcol-nop.yaml")},
-			parserProvider:    []config.MapProvider{&mockProvider{}, filemapprovider.New()},
-			cfgMapConverters:  []config.MapConverterFunc{func(context.Context, *config.Map) error { return errors.New("converter_err") }},
-			configUnmarshaler: configunmarshaler.NewDefault(),
-			expectNewErr:      true,
+			name:             "converter_err",
+			locations:        []string{"mock:", filepath.Join("testdata", "otelcol-nop.yaml")},
+			parserProvider:   []config.MapProvider{&mockProvider{}, filemapprovider.New()},
+			cfgMapConverters: []config.MapConverterFunc{func(context.Context, *config.Map) error { return errors.New("converter_err") }},
+			expectNewErr:     true,
 		},
 		{
 			name:              "unmarshal_err",
@@ -123,11 +120,10 @@ func TestConfigProvider_Errors(t *testing.T) {
 			expectNewErr:      true,
 		},
 		{
-			name:              "validation_err",
-			locations:         []string{"mock:", filepath.Join("testdata", "otelcol-invalid.yaml")},
-			parserProvider:    []config.MapProvider{&mockProvider{}, filemapprovider.New()},
-			configUnmarshaler: configunmarshaler.NewDefault(),
-			expectNewErr:      true,
+			name:           "validation_err",
+			locations:      []string{"mock:", filepath.Join("testdata", "otelcol-invalid.yaml")},
+			parserProvider: []config.MapProvider{&mockProvider{}, filemapprovider.New()},
+			expectNewErr:   true,
 		},
 		{
 			name:      "watch_err",
@@ -137,8 +133,7 @@ func TestConfigProvider_Errors(t *testing.T) {
 				require.NoError(t, err)
 				return []config.MapProvider{&mockProvider{}, &mockProvider{scheme: "err", retM: cfgMap, errW: errors.New("watch_err")}}
 			}(),
-			configUnmarshaler: configunmarshaler.NewDefault(),
-			expectWatchErr:    true,
+			expectWatchErr: true,
 		},
 		{
 			name:      "close_err",
@@ -151,7 +146,6 @@ func TestConfigProvider_Errors(t *testing.T) {
 					&mockProvider{scheme: "err", retM: cfgMap, errC: errors.New("close_err")},
 				}
 			}(),
-			configUnmarshaler: configunmarshaler.NewDefault(),
 			expectShutdownErr: true,
 		},
 	}