Skip to content

Commit

Permalink
ensure windows file path is respected (#4726)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
Alex Boten committed Jan 21, 2022
1 parent 1ce4e41 commit 2c5eb7c
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 2 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
11 changes: 9 additions & 2 deletions service/config_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package service // import "go.opentelemetry.io/collector/service"
import (
"context"
"fmt"
"regexp"
"strings"
"sync"

Expand Down Expand Up @@ -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
Expand Down
48 changes: 48 additions & 0 deletions service/config_provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

}

0 comments on commit 2c5eb7c

Please sign in to comment.