Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(datasources): Fix aws+sm bug when reading secrets not starting with slash #2284

Merged
merged 1 commit into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
fix(datasources): Fix aws+sm bug when reading secrets not starting wi…
…th slash

Signed-off-by: Dave Henderson <[email protected]>
  • Loading branch information
hairyhenderson committed Dec 16, 2024
commit a99ac4cbcb111e675c2eca509f8e9883f98c5d68
9 changes: 9 additions & 0 deletions internal/datafs/fsurl.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,15 @@ func SplitFSMuxURL(in *url.URL) (*url.URL, string) {
}

return &u, base
case "aws+sm":
// An aws+sm URL can either be opaque or have a path with a leading
// slash. If it's opaque, the URL must not contain a leading slash. If
// it has a path, the URL must begin with a slash.
if u.Opaque != "" {
return &url.URL{Scheme: u.Scheme}, u.Opaque
} else {
return &url.URL{Scheme: u.Scheme, Path: "/"}, strings.TrimLeft(u.Path, "/")
}
}

// trim leading and trailing slashes - they are not part of a valid path
Expand Down
20 changes: 20 additions & 0 deletions internal/datafs/fsurl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,26 @@ func TestSplitFSMuxURL(t *testing.T) {
"merge:///",
"vault:///foo/bar|foo|git+ssh://[email protected]/hairyhenderson/go-which.git//a/b/c/d",
},
{
"aws+sm:foo",
"aws+sm:",
"foo",
},
{
"aws+sm:foo/bar",
"aws+sm:",
"foo/bar",
},
{
"aws+sm:/foo/bar",
"aws+sm:///",
"foo/bar",
},
{
"aws+sm:/foo",
"aws+sm:///",
"foo",
},
}

for _, d := range testdata {
Expand Down
2 changes: 2 additions & 0 deletions internal/datafs/fsys.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ func FSysForPath(ctx context.Context, path string) (fs.FS, error) {
switch u.Scheme {
case "git+http", "git+https", "git+ssh", "git":
// no-op, these are handled
case "aws+sm":
// An aws+sm URL can be opaque, best not disturb it
case "", "file", "git+file":
// default to "/" so we have a rooted filesystem for all schemes, but also
// support volumes on Windows
Expand Down
4 changes: 2 additions & 2 deletions internal/datafs/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,13 +122,13 @@ func (d *dsReader) readFileContent(ctx context.Context, u *url.URL, hdr http.Hea
// leaking into the filesystem layer
u = removeQueryParam(u, overrideType)

u, fname := SplitFSMuxURL(u)

fsys, err := FSysForPath(ctx, u.String())
if err != nil {
return nil, fmt.Errorf("fsys for path %v: %w", u, err)
}

u, fname := SplitFSMuxURL(u)

// need to support absolute paths on local filesystem too
// TODO: this is a hack, probably fix this?
if u.Scheme == "file" && runtime.GOOS != "windows" {
Expand Down
Loading