From 2c5eb7ca1ad5e95c8c6bd1240c7173978954924a Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Fri, 21 Jan 2022 13:01:29 -0800 Subject: [PATCH] ensure windows file path is respected (#4726) * ensure windows file path is respected The current --config flag breaks backwards compatibility for Windows users as a common path is to use is C: as is used in the collector-contrib MSI for example. * use single character instead of hardcoded C * update changelog --- CHANGELOG.md | 4 +++ service/config_provider.go | 11 ++++++-- service/config_provider_test.go | 48 +++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 508d410217f..585f466826c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,10 @@ - Remove support to some arches and platforms from `ocb` (opentelemetry-collector-builder) (#4710) - Remove deprecated legacy path ("v1/trace") support for otlp http receiver (#4720) +## 🧰 Bug fixes 🧰 + +- Ensure Windows path (e.g: C:) is recognized as a file path (#4726) + ## 💡 Enhancements 💡 - `service.NewConfigProvider`: copy slice argument, disallow changes from caller to the input slice (#4729) diff --git a/service/config_provider.go b/service/config_provider.go index c25ab29f6f4..a477f536b9a 100644 --- a/service/config_provider.go +++ b/service/config_provider.go @@ -17,6 +17,7 @@ package service // import "go.opentelemetry.io/collector/service" import ( "context" "fmt" + "regexp" "strings" "sync" @@ -185,13 +186,19 @@ func (cm *configProvider) Shutdown(ctx context.Context) error { return errs } +// follows drive-letter specification: +// https://tools.ietf.org/id/draft-kerwin-file-scheme-07.html#syntax +var driverLetterRegexp = regexp.MustCompile("^[A-z]:") + func (cm *configProvider) mergeRetrieve(ctx context.Context) (configmapprovider.Retrieved, error) { var retrs []configmapprovider.Retrieved retCfgMap := config.NewMap() for _, location := range cm.locations { - // For backwards compatibility, empty url scheme means "file". + // For backwards compatibility: + // - empty url scheme means "file". + // - "^[A-z]:" also means "file" scheme := "file" - if idx := strings.Index(location, ":"); idx != -1 { + if idx := strings.Index(location, ":"); idx != -1 && !driverLetterRegexp.MatchString(location) { scheme = location[:idx] } else { location = scheme + ":" + location diff --git a/service/config_provider_test.go b/service/config_provider_test.go index fe69cd60048..ae5b38de845 100644 --- a/service/config_provider_test.go +++ b/service/config_provider_test.go @@ -312,3 +312,51 @@ func TestConfigProvider_ShutdownClosesWatch(t *testing.T) { assert.NoError(t, cfgW.Shutdown(context.Background())) watcherWG.Wait() } + +func TestBackwardsCompatibilityForFilePath(t *testing.T) { + factories, errF := componenttest.NopFactories() + require.NoError(t, errF) + + tests := []struct { + name string + location string + errMessage string + }{ + { + name: "unix", + location: `/test`, + errMessage: "unable to read the file file:/test", + }, + { + name: "file_unix", + location: `file:/test`, + errMessage: "unable to read the file file:/test", + }, + { + name: "windows_C", + location: `C:\test`, + errMessage: "unable to read the file file:C:\\test", + }, + { + name: "windows_z", + location: `z:\test`, + errMessage: "unable to read the file file:z:\\test", + }, + { + name: "file_windows", + location: `file:C:\test`, + errMessage: "unable to read the file file:C:\\test", + }, + { + name: "invalid_scheme", + location: `LL:\test`, + errMessage: "scheme LL is not supported for location", + }, + } + for _, tt := range tests { + provider := NewDefaultConfigProvider(tt.location, []string{}) + _, err := provider.Get(context.Background(), factories) + assert.Contains(t, err.Error(), tt.errMessage, tt.name) + } + +}