Enforce scheme name restrictions to all confmap.Provider implementations.

CHANGELOG.md

@@ -47,6 +47,7 @@
 - Bump to opentelemetry-proto v0.19.0. (#5823)
 - Expose `Scope.Attributes` in pdata (#5826)
 - Add support to handle 404, 405 http error code as permanent errors in OTLP exporter (#5827)
+- Enforce scheme name restrictions to all `confmap.Provider` implementations. (#5861)
 
 ### 🧰 Bug fixes 🧰
 

confmap/confmaptest/configtest.go

@@ -15,17 +15,46 @@
 package confmaptest // import "go.opentelemetry.io/collector/confmap/confmaptest"
 
 import (
-	"context"
+	"fmt"
+	"io/ioutil"
+	"path/filepath"
+	"regexp"
+
+	"gopkg.in/yaml.v2"
 
 	"go.opentelemetry.io/collector/confmap"
-	"go.opentelemetry.io/collector/confmap/provider/fileprovider"
 )
 
 // LoadConf loads a confmap.Conf from file, and does NOT validate the configuration.
 func LoadConf(fileName string) (*confmap.Conf, error) {
-	ret, err := fileprovider.New().Retrieve(context.Background(), "file:"+fileName, nil)
+	// Clean the path before using it.
+	content, err := ioutil.ReadFile(filepath.Clean(fileName))
 	if err != nil {
+		return nil, fmt.Errorf("unable to read the file %v: %w", fileName, err)
+	}
+
+	var rawConf map[string]interface{}
+	if err = yaml.Unmarshal(content, &rawConf); err != nil {
 		return nil, err
 	}
-	return ret.AsConf()
+
+	return confmap.NewFromStringMap(rawConf), nil
+}
+
+var schemeValidator = regexp.MustCompile("^[A-Za-z][A-Za-z0-9+.-]+$")
+
+// ValidateProviderScheme enforces that given confmap.Provider.Scheme() object is following the restriction defined by the collector:
+//   - Checks that the scheme name follows the restrictions defined https://datatracker.ietf.org/doc/html/rfc3986#section-3.1
+//   - Checks that the scheme name has at leas two characters per the confmap.Provider.Scheme() comment.
+func ValidateProviderScheme(p confmap.Provider) error {
+	scheme := p.Scheme()
+	if len(scheme) < 2 {
+		return fmt.Errorf("scheme must be at least 2 characters long: %q", scheme)
+	}
+
+	if !schemeValidator.MatchString(scheme) {
+		return fmt.Errorf("scheme names consist of a sequence of characters beginning with a letter and followed by any combination of letters, digits, \"+\", \".\", or \"-\": %q", scheme)
+	}
+
+	return nil
 }

confmap/confmaptest/configtest_test.go

@@ -15,20 +15,56 @@
 package confmaptest
 
 import (
+	"context"
 	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"go.opentelemetry.io/collector/confmap"
 )
 
-func TestLoadConfigMap_FileNotFound(t *testing.T) {
+func TestLoadConfFileNotFound(t *testing.T) {
 	_, err := LoadConf("file/not/found")
 	assert.Error(t, err)
 }
 
-func TestLoadConfigMap(t *testing.T) {
+func TestLoadConfInvalidYAML(t *testing.T) {
+	_, err := LoadConf(filepath.Join("testdata", "invalid.yaml"))
+	require.Error(t, err)
+}
+
+func TestLoadConf(t *testing.T) {
 	cfg, err := LoadConf(filepath.Join("testdata", "simple.yaml"))
 	require.NoError(t, err)
 	assert.Equal(t, map[string]interface{}{"floating": 3.14}, cfg.ToStringMap())
 }
+
+func TestValidateProviderScheme(t *testing.T) {
+	assert.NoError(t, ValidateProviderScheme(&schemeProvider{scheme: "file"}))
+	assert.NoError(t, ValidateProviderScheme(&schemeProvider{scheme: "s3"}))
+	assert.NoError(t, ValidateProviderScheme(&schemeProvider{scheme: "a.l-l+"}))
+	// Too short.
+	assert.Error(t, ValidateProviderScheme(&schemeProvider{scheme: "a"}))
+	// Invalid first character.
+	assert.Error(t, ValidateProviderScheme(&schemeProvider{scheme: "3s"}))
+	// Invalid underscore character.
+	assert.Error(t, ValidateProviderScheme(&schemeProvider{scheme: "all_"}))
+}
+
+type schemeProvider struct {
+	scheme string
+}
+
+func (s schemeProvider) Retrieve(context.Context, string, confmap.WatcherFunc) (confmap.Retrieved, error) {
+	return confmap.Retrieved{}, nil
+}
+
+func (s schemeProvider) Scheme() string {
+	return s.scheme
+}
+
+func (s schemeProvider) Shutdown(ctx context.Context) error {
+	return nil
+}

confmap/confmaptest/testdata/invalid.yaml

@@ -0,0 +1 @@
+[invalid,

confmap/provider.go

@@ -41,9 +41,13 @@ type Provider interface {
 	//
 	// `uri` must follow the "<scheme>:<opaque_data>" format. This format is compatible
 	// with the URI definition (see https://datatracker.ietf.org/doc/html/rfc3986). The "<scheme>"
-	// must be always included in the `uri`. The scheme supported by any provider MUST be at
-	// least 2 characters long to avoid conflicting with a driver-letter identifier as specified
-	// in https://tools.ietf.org/id/draft-kerwin-file-scheme-07.html#syntax.
+	// must be always included in the `uri`. The "<scheme>" supported by any provider:
+	//   - MUST consist of a sequence of characters beginning with a letter and followed by any
+	//     combination of letters, digits, plus ("+"), period ("."), or hyphen ("-").
+	//     See https://datatracker.ietf.org/doc/html/rfc3986#section-3.1.
+	//   - MUST be at least 2 characters long to avoid conflicting with a driver-letter identifier as specified
+	//     in https://tools.ietf.org/id/draft-kerwin-file-scheme-07.html#syntax.
+	//   - For testing, all implementation MUST check that confmaptest.ValidateProviderScheme returns no error.
 	//
 	// `watcher` callback is called when the config changes. watcher may be called from
 	// a different go routine. After watcher is called Retrieved.Get should be called

confmap/provider/envprovider/provider_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"go.opentelemetry.io/collector/confmap"
+	"go.opentelemetry.io/collector/confmap/confmaptest"
 )
 
 const envSchemePrefix = schemeName + ":"
@@ -34,6 +35,10 @@ exporters:
     endpoint: "localhost:4317"
 `
 
+func TestValidateProviderScheme(t *testing.T) {
+	assert.NoError(t, confmaptest.ValidateProviderScheme(New()))
+}
+
 func TestEmptyName(t *testing.T) {
 	env := New()
 	_, err := env.Retrieve(context.Background(), "", nil)

confmap/provider/fileprovider/provider_test.go

@@ -24,10 +24,15 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"go.opentelemetry.io/collector/confmap"
+	"go.opentelemetry.io/collector/confmap/confmaptest"
 )
 
 const fileSchemePrefix = schemeName + ":"
 
+func TestValidateProviderScheme(t *testing.T) {
+	assert.NoError(t, confmaptest.ValidateProviderScheme(New()))
+}
+
 func TestEmptyName(t *testing.T) {
 	fp := New()
 	_, err := fp.Retrieve(context.Background(), "", nil)

confmap/provider/yamlprovider/provider_test.go

@@ -19,8 +19,14 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+
+	"go.opentelemetry.io/collector/confmap/confmaptest"
 )
 
+func TestValidateProviderScheme(t *testing.T) {
+	assert.NoError(t, confmaptest.ValidateProviderScheme(New()))
+}
+
 func TestEmpty(t *testing.T) {
 	sp := New()
 	_, err := sp.Retrieve(context.Background(), "", nil)