Skip to content

Commit 2beed98

Browse files
authored
[confmap] Validate providers scheme when building a Resolver (#10786)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Validate providers schemes when building a Resolver. I don't consider this a breaking change since the providers would be useless if they don't follow this pattern.
1 parent 88adaef commit 2beed98

File tree

4 files changed

+46
-13
lines changed

4 files changed

+46
-13
lines changed
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: enhancement
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
7+
component: confmap
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Check that providers have a correct scheme when building a confmap.Resolver.
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [10786]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# Optional: The change log or logs in which this entry should be included.
21+
# e.g. '[user]' or '[user, api]'
22+
# Include 'user' if the change is relevant to end users.
23+
# Include 'api' if there is a change to a library API.
24+
# Default: '[user]'
25+
change_logs: [api]

confmap/expand_test.go

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,8 @@ func TestResolverDoneNotExpandOldEnvVars(t *testing.T) {
6161
envProvider := newFakeProvider("env", func(context.Context, string, WatcherFunc) (*Retrieved, error) {
6262
return NewRetrieved("some string")
6363
})
64-
emptySchemeProvider := newFakeProvider("", func(context.Context, string, WatcherFunc) (*Retrieved, error) {
65-
return NewRetrieved("some string")
66-
})
6764

68-
resolver, err := NewResolver(ResolverSettings{URIs: []string{"test:"}, ProviderFactories: []ProviderFactory{fileProvider, envProvider, emptySchemeProvider}, ConverterFactories: nil})
65+
resolver, err := NewResolver(ResolverSettings{URIs: []string{"test:"}, ProviderFactories: []ProviderFactory{fileProvider, envProvider}, ConverterFactories: nil})
6966
require.NoError(t, err)
7067

7168
// Test that expanded configs are the same with the simple config with no env vars.
@@ -509,12 +506,8 @@ func TestResolverExpandInvalidScheme(t *testing.T) {
509506
panic("must not be called")
510507
})
511508

512-
resolver, err := NewResolver(ResolverSettings{URIs: []string{"input:"}, ProviderFactories: []ProviderFactory{provider, testProvider}, ConverterFactories: nil})
513-
require.NoError(t, err)
514-
515-
_, err = resolver.Resolve(context.Background())
516-
517-
assert.EqualError(t, err, `invalid uri: "g_c_s:VALUE"`)
509+
_, err := NewResolver(ResolverSettings{URIs: []string{"input:"}, ProviderFactories: []ProviderFactory{provider, testProvider}, ConverterFactories: nil})
510+
assert.ErrorContains(t, err, "invalid 'confmap.Provider' scheme")
518511
}
519512

520513
func TestResolverExpandInvalidOpaqueValue(t *testing.T) {

confmap/resolver.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,17 @@ func NewResolver(set ResolverSettings) (*Resolver, error) {
9696
providers := make(map[string]Provider, len(set.ProviderFactories))
9797
for _, factory := range set.ProviderFactories {
9898
provider := factory.Create(set.ProviderSettings)
99-
providers[provider.Scheme()] = provider
99+
scheme := provider.Scheme()
100+
// Check that the scheme follows the pattern.
101+
if !regexp.MustCompile(schemePattern).MatchString(scheme) {
102+
return nil, fmt.Errorf("invalid 'confmap.Provider' scheme %q", scheme)
103+
}
104+
// Check that the scheme is unique.
105+
if _, ok := providers[scheme]; ok {
106+
return nil, fmt.Errorf("duplicate 'confmap.Provider' scheme %q", scheme)
107+
}
108+
109+
providers[scheme] = provider
100110
}
101111

102112
if set.DefaultScheme != "" {

confmap/resolver_test.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,11 +113,16 @@ func (m *mockConverter) Convert(context.Context, *Conf) error {
113113
return errors.New("converter_err")
114114
}
115115

116-
func TestNewResolverInvalidScheme(t *testing.T) {
117-
_, err := NewResolver(ResolverSettings{URIs: []string{"s_3:has invalid char"}, ProviderFactories: []ProviderFactory{newMockProvider(&mockProvider{scheme: "s_3"})}})
116+
func TestNewResolverInvalidSchemeInURI(t *testing.T) {
117+
_, err := NewResolver(ResolverSettings{URIs: []string{"s_3:has invalid char"}, ProviderFactories: []ProviderFactory{newMockProvider(&mockProvider{scheme: "s3"})}})
118118
assert.EqualError(t, err, `invalid uri: "s_3:has invalid char"`)
119119
}
120120

121+
func TestNewResolverDuplicateScheme(t *testing.T) {
122+
_, err := NewResolver(ResolverSettings{URIs: []string{"mock:something"}, ProviderFactories: []ProviderFactory{newMockProvider(&mockProvider{scheme: "mock"}), newMockProvider(&mockProvider{scheme: "mock"})}})
123+
assert.EqualError(t, err, `duplicate 'confmap.Provider' scheme "mock"`)
124+
}
125+
121126
func TestResolverErrors(t *testing.T) {
122127
tests := []struct {
123128
name string

0 commit comments

Comments
 (0)