[RyoKusnadi]

Closes #10582

Replaces #10600 (closed, and its fork branch is gone), with @nopcoder's review applied.

Change Description

Background

lakectl fs rm -r should delete a folder. fs download -r already adds a trailing / to the path, but fs rm -r didn't.

Bug Fix

1. Problem - lakectl fs rm -r lakefs://repo/main/logs also deletes logs.txt and logs-archive/3.txt, not just logs/1.txt and logs/2.txt.
   How it happens: run the command with a path that has no trailing slash, while another object name starts with the same text.
   Steps: upload logs/1.txt, logs/2.txt, logs.txt, logs-archive/3.txt, then run lakectl fs rm -r lakefs://repo/main/logs.
   Expected: only logs/1.txt and logs/2.txt are deleted. Actual: all four are deleted.
2. Root cause - cmd/lakectl/cmd/fs_rm.go passed the path unchanged as the list prefix, and the listing matches on a raw string prefix.
3. Solution - new shared helper ensureTrailingPathDelimiter in common_helpers.go adds a trailing / to a non-empty path. An empty path (whole branch) is unchanged. fs rm -r now uses it, and so do the two places that already did this by hand: fs download -r and local init (no behavior change there).

Scope: fs ls -r is read-only and left as is, as the issue notes.

Testing Details

No new unit test, per review on #10600. Ran go build ./cmd/lakectl/..., go vet ./cmd/lakectl/cmd/, go test ./cmd/lakectl/cmd/ (77 passed), gofmt -l, and golangci-lint (no findings in the touched files).
Not run: esti system tests or a live lakeFS server, so the four-object steps above were not run end to end.
Environment: master @ e86d145, go1.27.1, darwin/arm64.

Breaking Change?

Behavior change: rm -r on a path without / no longer removes siblings that share the prefix. Anyone relying on that would now need to list the sibling paths themselves.

[RyoKusnadi]

@nopcoder this is #10600 moved over with your review applied: helper renamed to ensureTrailingPathDelimiter in common_helpers.go, reused in fs download and local init, and the test dropped. Mind taking a look?

[nopcoder]

remotePath = ensureTrailingPathDelimiter(remotePath)
*remote.Path = remotePath

we will need the updated remotePath for later - check if it used after this line.

[RyoKusnadi]

Yeah, it is used later: strings.TrimPrefix(listItem.Path, remotePath) when building relPath. The result was the same thanks to the second TrimPrefix of /, but keeping the two in sync is cleaner. Updated in the latest commit. I kept the remotePath != "" guard, since remote.Path is nil for a URI with no path and the assignment would panic there.

[RyoKusnadi]

@nopcoder updated in 2f5f63b, remotePath now carries the trailing delimiter too. Mind taking another look?

[RyoKusnadi]

Heads up on the red "PR Requirements" check: the "Linked issue" step runs gh with TREEVERSE_CI_TOKEN, which is empty on fork PRs, so it fails before it checks anything. #10582 is linked (Closes #10582), so it should pass once it runs with the token.

[nopcoder]

LGTM - left small comment related to a condition I think we can drop.

[nopcoder]

don't think this if is needed

[RyoKusnadi]

Yeah, not needed. The path URI parser never leaves Path nil, and the helper already returns "" as is. Removed in 602e4c3.