diff --git a/historyserver/pkg/storage/gcs/gcs_handler.go b/historyserver/pkg/storage/gcs/gcs_handler.go index 5d121a7ea0d..00a4dbc85bb 100644 --- a/historyserver/pkg/storage/gcs/gcs_handler.go +++ b/historyserver/pkg/storage/gcs/gcs_handler.go @@ -173,13 +173,23 @@ func (h *RayLogsHandler) List() []utils.ClusterInfo { return clusterList } +// contentMatchGlob builds the object search pattern for GetContent, anchored at +// the configured root dir the same way ListFiles and List anchor their prefixes. +// A leading "**/" would let the search escape the root dir entirely, because +// matchGlob treats "**" as matching across "/", so a bucket shared by more than +// one root dir could serve a file belonging to a different deployment. +// An empty root dir keeps the previous unanchored pattern. +func contentMatchGlob(rootDir string, clusterId string, fileName string) string { + return strings.TrimPrefix(path.Join(rootDir, "**", clusterId+"*", "**", fileName), "/") +} + func (h *RayLogsHandler) GetContent(clusterId string, fileName string) io.Reader { ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() bucket := h.StorageClient.Bucket(h.GCSBucket) query := &gstorage.Query{ - MatchGlob: "**/" + clusterId + "*/**/" + fileName, + MatchGlob: contentMatchGlob(h.RootDir, clusterId, fileName), } objectIterator := bucket.Objects(ctx, query) fileAttrs, err := objectIterator.Next() diff --git a/historyserver/pkg/storage/gcs/gcs_handler_test.go b/historyserver/pkg/storage/gcs/gcs_handler_test.go index c757fa1d0f7..90c364a2237 100644 --- a/historyserver/pkg/storage/gcs/gcs_handler_test.go +++ b/historyserver/pkg/storage/gcs/gcs_handler_test.go @@ -274,6 +274,51 @@ func TestList(t *testing.T) { } } +// contentMatchGlob is asserted directly because fake-gcs-server does not +// implement matchGlob, so no test in this package can observe what the pattern +// actually selects. This checks the pattern that gets sent, not its effect. +func TestContentMatchGlobIsRooted(t *testing.T) { + tests := []struct { + name string + rootDir string + clusterID string + fileName string + want string + }{ + { + name: "rooted", + rootDir: "ray_historyserver", + clusterID: "clusters/clusterA_ns", + fileName: "session123/logs/important.log", + want: "ray_historyserver/**/clusters/clusterA_ns*/**/session123/logs/important.log", + }, + { + name: "no root dir keeps the previous unanchored pattern", + rootDir: "", + clusterID: "clusters/clusterA_ns", + fileName: "session123/logs/important.log", + want: "**/clusters/clusterA_ns*/**/session123/logs/important.log", + }, + { + name: "leading slash on root dir is trimmed", + rootDir: "/ray_historyserver", + clusterID: "clusterA", + fileName: "important.log", + want: "ray_historyserver/**/clusterA*/**/important.log", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := contentMatchGlob(tt.rootDir, tt.clusterID, tt.fileName) + if got != tt.want { + t.Errorf("contentMatchGlob(%q, %q, %q) = %q, want %q", + tt.rootDir, tt.clusterID, tt.fileName, got, tt.want) + } + }) + } +} + func TestGetContent(t *testing.T) { clusterID := "clusterA" fileName := "important.log"