[Feat] [history server] Correctly use `glob` in `logs/` endpoint by machichima · Pull Request #4526 · ray-project/kuberay

machichima · 2026-02-21T14:39:19Z

Why are these changes needed?

Previous the logs/ endpoint does not do glob pattern matching on the file path, instead just do trimsuffix.
This PR makes the logs/ endpoint respect the glob parameter and use it correctly.

Manual Test Result

On dead cluster:

❯ NODE_ID=$(curl -s -b ~/cookies.txt "http://localhost:8080/nodes?view=summary" | jq -r '.data.summary[0][].raylet.nodeId')
curl -b ~/cookies.txt "http://localhost:8080/api/v0/logs?node_id=${NODE_ID}&glob=raylet*"
{"data":{"result":{"raylet":["raylet.err","raylet.out"]}},"msg":"","result":true}%

Related issue number

Checks

I've made sure the tests are passing.
Testing Strategy
- Unit tests
- Manual tests
- This PR is not tested :(

Signed-off-by: machichima <nary12321@gmail.com>

Copilot

Pull request overview

This PR improves the /api/v0/logs endpoint in the history server to correctly implement glob pattern matching for log file filtering. Previously, the endpoint incorrectly used strings.TrimSuffix on the glob parameter instead of proper pattern matching. The fix introduces the doublestar/v4 library for robust glob support and updates the endpoint to properly parse glob patterns, separate the base directory from the pattern, and filter files accordingly.

Changes:

Added doublestar/v4 dependency for proper glob pattern matching
Updated /api/v0/logs endpoint to correctly parse and apply glob patterns, extracting base directory paths when present
Added comprehensive E2E test coverage for glob pattern matching with both matching and non-matching patterns

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
historyserver/go.mod	Added doublestar/v4 dependency for glob pattern support
historyserver/go.sum	Updated checksums for new dependency
historyserver/pkg/historyserver/router.go	Updated getNodeLogs to properly parse glob patterns using SplitPattern and extract base directories
historyserver/pkg/historyserver/reader.go	Implemented glob filtering in _getNodeLogs using doublestar.Match
historyserver/test/support/constant.go	Added EndpointLogs constant for test usage
historyserver/test/e2e/historyserver_test.go	Added comprehensive test coverage for glob pattern functionality

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-21T15:09:18Z

historyserver/pkg/historyserver/router.go


 	ws.Route(ws.GET("/v0/logs").To(s.getNodeLogs).Filter(s.CookieHandle).
-		Doc("get appliations").Param(ws.QueryParameter("node_id", "node_id")).
+		Doc("get appliations").


Spelling error: "appliations" should be "applications".

Copilot · 2026-02-21T15:09:18Z

historyserver/pkg/historyserver/router.go

+	var folder, glob string
 	if req.QueryParameter("folder") != "" {
 		folder = req.QueryParameter("folder")
 	}
 	if req.QueryParameter("glob") != "" {
-		folder = req.QueryParameter("glob")
-		folder = strings.TrimSuffix(folder, "*")
+		glob = req.QueryParameter("glob")
+		// SplitPattern splits e.g. "logs/raylet*" into base="logs/" and pattern="raylet*",
+		// so we can use base as the storage directory prefix and pattern for matching.
+		// For a flat pattern like "raylet*", base is "." which we treat as no subdirectory.
+		base, pattern := doublestar.SplitPattern(glob)
+		glob = pattern
+		if base != "." {
+			folder = base
+		}
 	}
-	data, err := s._getNodeLogs(clusterNameID+"_"+clusterNamespace, sessionName, req.QueryParameter("node_id"), folder)
+	data, err := s._getNodeLogs(clusterNameID+"_"+clusterNamespace, sessionName, req.QueryParameter("node_id"), folder, glob)


The node_id, folder, and glob parameters should be validated to prevent path traversal attacks, similar to the validation performed in getNodeLogFile (lines 988-995). Add validation using fs.ValidPath for these parameters to prevent malicious inputs like "../../../etc/passwd".

Updated in 021bca5

Copilot · 2026-02-21T15:09:19Z

historyserver/pkg/historyserver/router.go

+		// For a flat pattern like "raylet*", base is "." which we treat as no subdirectory.
+		base, pattern := doublestar.SplitPattern(glob)
+		glob = pattern
+		if base != "." {


When both the folder and glob query parameters are provided, the folder extracted from the glob pattern will silently overwrite the explicit folder parameter. This could lead to unexpected behavior. Consider either: (1) returning an error when both parameters are provided with conflicting values, (2) merging the folder and glob base path (e.g., path.Join(folder, base)), or (3) documenting that the glob parameter takes precedence and ignoring the folder parameter when glob contains a path.

Suggested change

if base != "." {

if base != "." {

// If both folder and glob specify a folder and they conflict, return a clear error.

if folder != "" && folder != base {

resp.WriteErrorString(http.StatusBadRequest, "conflicting 'folder' and 'glob' query parameters")

return

}

The folder parameter is not supported in Ray Dashboard API, deleted it to make history server endpoint consistent with Ray Dashboard API

AndySung320 · 2026-02-22T00:20:49Z

historyserver/test/e2e/historyserver_test.go

 }

+// testNodeLogsEndpointDeadCluster verifies that the /api/v0/logs endpoint correctly lists and filters
+// log files for a dead cluster using the glob and folder query parameters.


The folder parameter hasn't been covered in this e2e tests.
Since SplitPattern can implicitly set folder from a glob, would it be better adding a test case that uses folder directly to verify the directory scoping works correctly ?
Also a case where glob contains a path prefix to ensure SplitPattern correctly populates folder from it.

I removed the folder parameter in 021bca5 as Ray Dashboard API does not support this.

Added the test case to specify path prefix with wildcard in 3973098

Signed-off-by: machichima <nary12321@gmail.com>

historyserver/pkg/historyserver/router.go

Signed-off-by: machichima <nary12321@gmail.com>

Signed-off-by: Future-Outlier <eric901201@gmail.com>

Future-Outlier

lgtm, I added more test in this PR, thank you

Future-Outlier · 2026-02-22T14:24:21Z

cc @rueian to merge, tks!

Signed-off-by: Future-Outlier <eric901201@gmail.com>

historyserver/pkg/historyserver/reader.go

historyserver/pkg/historyserver/router.go

Signed-off-by: machichima <nary12321@gmail.com>

Signed-off-by: Han-Ju Chen (Future-Outlier) <eric901201@gmail.com>

andrewsykim · 2026-02-26T15:55:31Z

historyserver/test/support/constant.go


 const (
+	EndpointLogs          = "/api/v0/logs"
+	EndpointLogsFile      = "/api/v0/logs/file"


Line 5-10 seem like a duplicate of 11-17?

fixed, I didn't solve the merge conflict well...

Signed-off-by: Future-Outlier <eric901201@gmail.com>

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

cookies.txt

Signed-off-by: Future-Outlier <eric901201@gmail.com>

machichima added 4 commits February 21, 2026 20:26

feat: implement glob matching and prefix extract

bfadda9

Signed-off-by: machichima <nary12321@gmail.com>

feat: split base and prefix and match better

2bba61e

Signed-off-by: machichima <nary12321@gmail.com>

test: add logs/ dead cluster e2e test

0365747

Signed-off-by: machichima <nary12321@gmail.com>

refactor: remove unused function

6de08be

Signed-off-by: machichima <nary12321@gmail.com>

Future-Outlier self-assigned this Feb 21, 2026

Future-Outlier requested a review from Copilot February 21, 2026 14:55

Copilot started reviewing on behalf of Future-Outlier February 21, 2026 14:56 View session

Copilot AI reviewed Feb 21, 2026

View reviewed changes

AndySung320 reviewed Feb 22, 2026

View reviewed changes

fix: add path validation

021bca5

Signed-off-by: machichima <nary12321@gmail.com>

cursor bot reviewed Feb 22, 2026

View reviewed changes

historyserver/pkg/historyserver/router.go Outdated Show resolved Hide resolved

machichima and others added 4 commits February 22, 2026 19:15

test: add glob directory test

ed7e19f

Signed-off-by: machichima <nary12321@gmail.com>

fix: remove folder query parameter

843ab08

Signed-off-by: machichima <nary12321@gmail.com>

test: update test case to be directory + wildcard

3973098

Signed-off-by: machichima <nary12321@gmail.com>

better test

845f583

Signed-off-by: Future-Outlier <eric901201@gmail.com>

Future-Outlier approved these changes Feb 22, 2026

View reviewed changes

Future-Outlier moved this to can be merged in @Future-Outlier's kuberay project Feb 22, 2026

Future-Outlier added this to @Future-Outlier's kuberay project Feb 22, 2026

Future-Outlier added 2 commits February 23, 2026 12:41

fix conflict

e6c930a

Signed-off-by: Future-Outlier <eric901201@gmail.com>

upadte

bdccc56

Signed-off-by: Future-Outlier <eric901201@gmail.com>

cursor bot reviewed Feb 24, 2026

View reviewed changes

historyserver/pkg/historyserver/reader.go Show resolved Hide resolved

historyserver/pkg/historyserver/router.go Show resolved Hide resolved

machichima and others added 3 commits February 25, 2026 20:07

feat: include recurse listing

d65921a

Signed-off-by: machichima <nary12321@gmail.com>

test: add test for recurrsive match

23dbd13

Signed-off-by: machichima <nary12321@gmail.com>

Merge branch 'master' into hs-logs-glob

398627c

Signed-off-by: Han-Ju Chen (Future-Outlier) <eric901201@gmail.com>

andrewsykim reviewed Feb 26, 2026

View reviewed changes

update

fb4a57c

Signed-off-by: Future-Outlier <eric901201@gmail.com>

cursor bot reviewed Feb 26, 2026

View reviewed changes

cookies.txt Outdated Show resolved Hide resolved

update

511d8c0

Signed-off-by: Future-Outlier <eric901201@gmail.com>

-		if base != "." {
+		if base != "." {
+			// If both folder and glob specify a folder and they conflict, return a clear error.
+			if folder != "" && folder != base {
+				resp.WriteErrorString(http.StatusBadRequest, "conflicting 'folder' and 'glob' query parameters")
+				return
+			}

Conversation

machichima commented Feb 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why are these changes needed?

Manual Test Result

Related issue number

Checks

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

machichima Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

machichima Feb 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AndySung320 Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

machichima Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Future-Outlier left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Future-Outlier commented Feb 22, 2026

Uh oh!

Uh oh!

Uh oh!

andrewsykim Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Future-Outlier Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

machichima commented Feb 21, 2026 •

edited

Loading

machichima Feb 22, 2026 •

edited

Loading

Future-Outlier left a comment •

edited

Loading