client: Fix leading / issue with mds_check_access by kotreshhr · Pull Request #58878 · ceph/ceph

kotreshhr · 2024-07-26T15:01:20Z

The "Client::mds_check_access" expects the target_path without leading '/' as it eventually calls the "MDSCapMatch::match_path" which expects the target_path passed to be with out leading '/' as well.

The single leading '/' was being removed. But absolute path constructed did have leading '//', so removing all the leading '/' was necessary.

This causes the clients not to be able to access a particular path even though it has a rw permission on the specific path.

The patches fixes the leading '//' issue.

Fixes: https://tracker.ceph.com/issues/67212

Contribution Guidelines

To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard cephadm
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox
jenkins test windows
jenkins test rook e2e

kotreshhr · 2024-07-26T15:02:06Z

I need to add a test for this.

vshankar · 2024-07-26T15:20:27Z

src/client/Client.cc

-        path = path.substr(1);    // drop leading /
+
+      // drop any leading / (it might contain leadin '//')
+      while (path.length() && path[0] == '/') {


What do you think about having this is mds_check_access() itself?

src/client/Client.cc

kotreshhr · 2024-07-28T19:08:12Z

jenkins test make check

src/client/Client.cc

batrick

otherwise lgtm

qa/tasks/cephfs/test_admin.py

src/client/Client.cc

vshankar · 2024-07-30T07:46:35Z

This PR is under test in https://tracker.ceph.com/issues/67252.

kotreshhr · 2024-07-30T11:27:28Z

I see a test failure on my run https://pulpito.ceph.com/khiremat-2024-07-30_06:33:39-fs:functional-wip-khiremat-58878-use-mds-auth-caps-issue-0-distro-default-smithi/

Investigating. I will update on findings.

kotreshhr · 2024-07-30T12:52:35Z

I see a test failure on my run https://pulpito.ceph.com/khiremat-2024-07-30_06:33:39-fs:functional-wip-khiremat-58878-use-mds-auth-caps-issue-0-distro-default-smithi/

Investigating. I will update on findings.

It was a test bug. The new_file creation path was in correct w.r.t the mountpoint. Fixed that.

Fixes: https://tracker.ceph.com/issues/67212 Signed-off-by: Kotresh HR <khiremat@redhat.com>

The "Client::mds_check_access" expects the target_path without leading '/' as it eventually calls the "MDSCapMatch::match_path" which expects the target_path passed to be with out leading '/' as well. The single leading '/' was being removed. But absolute path constructed did have leading '//', so removing all the leading '/' was necessary. This causes the clients not to be able to access a particular path even though it has a rw permission on the specific path. The patche fixes the leading '//' issue. Fixes: https://tracker.ceph.com/issues/67212 Signed-off-by: Kotresh HR <khiremat@redhat.com>

kotreshhr · 2024-07-31T05:46:57Z

https://pulpito.ceph.com/khiremat-2024-07-30_17:58:55-fs:functional-wip-khiremat-58878-use-mds-auth-caps-issue-0-distro-default-smithi/

The test has passed. But there is different warning not related to test. See below.

[khiremat@vossi04 7826702]$ egrep " cmd on a specific path|cephfs_test_runner*.*test_fs_read_and_single_path_rw" teuthology.log  
2024-07-30T20:23:41.232 INFO:tasks.cephfs_test_runner:Starting test: test_fs_read_and_single_path_rw (tasks.cephfs.test_admin.TestFsAuthorize)
Tests the file creation using 'touch' cmd on a specific path
2024-07-30T20:23:41.232 INFO:tasks.cephfs_test_runner:test_fs_read_and_single_path_rw (tasks.cephfs.test_admin.TestFsAuthorize)
2024-07-30T20:25:25.868 INFO:tasks.cephfs_test_runner:Tests the file creation using 'touch' cmd on a specific path ... ok
[khiremat@vossi04 7826702]$

kotreshhr · 2024-07-31T05:47:24Z

jenkins retest this please

vshankar · 2024-07-31T06:07:58Z

https://pulpito.ceph.com/khiremat-2024-07-30_17:58:55-fs:functional-wip-khiremat-58878-use-mds-auth-caps-issue-0-distro-default-smithi/

The test has passed. But there is different warning not related to test. See below.

[khiremat@vossi04 7826702]$ egrep " cmd on a specific path|cephfs_test_runner*.*test_fs_read_and_single_path_rw" teuthology.log  
2024-07-30T20:23:41.232 INFO:tasks.cephfs_test_runner:Starting test: test_fs_read_and_single_path_rw (tasks.cephfs.test_admin.TestFsAuthorize)
Tests the file creation using 'touch' cmd on a specific path
2024-07-30T20:23:41.232 INFO:tasks.cephfs_test_runner:test_fs_read_and_single_path_rw (tasks.cephfs.test_admin.TestFsAuthorize)
2024-07-30T20:25:25.868 INFO:tasks.cephfs_test_runner:Tests the file creation using 'touch' cmd on a specific path ... ok
[khiremat@vossi04 7826702]$

Sorry, what warning?

* refs/pull/58878/head: client: Fix leading / issue with mds_check_access qa: Add mds caps test for testing fs read and a path rw Reviewed-by: Xiubo Li <xiubli@redhat.com> Reviewed-by: Patrick Donnelly <pdonnell@redhat.com> Reviewed-by: Venky Shankar <vshankar@redhat.com>

kotreshhr · 2024-07-31T06:53:39Z

https://pulpito.ceph.com/khiremat-2024-07-30_17:58:55-fs:functional-wip-khiremat-58878-use-mds-auth-caps-issue-0-distro-default-smithi/
The test has passed. But there is different warning not related to test. See below.

[khiremat@vossi04 7826702]$ egrep " cmd on a specific path|cephfs_test_runner*.*test_fs_read_and_single_path_rw" teuthology.log  
2024-07-30T20:23:41.232 INFO:tasks.cephfs_test_runner:Starting test: test_fs_read_and_single_path_rw (tasks.cephfs.test_admin.TestFsAuthorize)
Tests the file creation using 'touch' cmd on a specific path
2024-07-30T20:23:41.232 INFO:tasks.cephfs_test_runner:test_fs_read_and_single_path_rw (tasks.cephfs.test_admin.TestFsAuthorize)
2024-07-30T20:25:25.868 INFO:tasks.cephfs_test_runner:Tests the file creation using 'touch' cmd on a specific path ... ok
[khiremat@vossi04 7826702]$

Sorry, what warning?

"2024-07-30T21:00:20.005629+0000 mds.a (mds.0) 1 : cluster [WRN] client could not reconnect as file system flag refuse_client_session is set" in cluster log

vshankar · 2024-07-31T06:58:33Z

https://pulpito.ceph.com/khiremat-2024-07-30_17:58:55-fs:functional-wip-khiremat-58878-use-mds-auth-caps-issue-0-distro-default-smithi/
The test has passed. But there is different warning not related to test. See below.

[khiremat@vossi04 7826702]$ egrep " cmd on a specific path|cephfs_test_runner*.*test_fs_read_and_single_path_rw" teuthology.log  
2024-07-30T20:23:41.232 INFO:tasks.cephfs_test_runner:Starting test: test_fs_read_and_single_path_rw (tasks.cephfs.test_admin.TestFsAuthorize)
Tests the file creation using 'touch' cmd on a specific path
2024-07-30T20:23:41.232 INFO:tasks.cephfs_test_runner:test_fs_read_and_single_path_rw (tasks.cephfs.test_admin.TestFsAuthorize)
2024-07-30T20:25:25.868 INFO:tasks.cephfs_test_runner:Tests the file creation using 'touch' cmd on a specific path ... ok
[khiremat@vossi04 7826702]$

Sorry, what warning?

"2024-07-30T21:00:20.005629+0000 mds.a (mds.0) 1 : cluster [WRN] client could not reconnect as file system flag refuse_client_session is set" in cluster log

That's a known issue - https://tracker.ceph.com/issues/66639

kotreshhr · 2024-07-31T07:01:33Z

jenkins test docs

kotreshhr · 2024-07-31T07:01:49Z

jenkins test make check

kotreshhr · 2024-07-31T07:13:19Z

jenkins test make check arm64

* refs/pull/58878/head: client: Fix leading / issue with mds_check_access qa: Add mds caps test for testing fs read and a path rw Reviewed-by: Patrick Donnelly <pdonnell@redhat.com> Reviewed-by: Xiubo Li <xiubli@redhat.com> Reviewed-by: Venky Shankar <vshankar@redhat.com>

vshankar · 2024-07-31T08:22:17Z

https://pulpito.ceph.com/vshankar-2024-07-31_07:55:21-fs-wip-vshankar-testing-20240730.074544-debug-testing-default-smithi/

The above run is with --filter admin. That should be good enough to merge this patch given its urgency. Will run fs suite run in parallel.

vshankar · 2024-07-31T08:23:30Z

Oh, these unrelated jenkins failures are annoying.

vshankar · 2024-07-31T08:23:43Z

jenkins test make check

vshankar · 2024-07-31T08:23:48Z

jenkins test windows

kotreshhr · 2024-07-31T08:25:24Z

jenkins test windows

vshankar

From /a/vshankar-2024-07-31_07:55:21-fs-wip-vshankar-testing-20240730.074544-debug-testing-default-smithi/7828173 (which is still running)

2024-07-31T08:48:19.401 INFO:tasks.cephfs_test_runner:Tests the file creation using 'touch' cmd on a specific path ... ok

The test has passed. I'm comfortable merging this change albeit prematurely.

vshankar · 2024-07-31T11:09:19Z

https://pulpito.ceph.com/vshankar-2024-07-31_07:55:21-fs-wip-vshankar-testing-20240730.074544-debug-testing-default-smithi/ - all passed.

vshankar · 2024-08-06T09:55:30Z

Just FYI - fs suite run: https://tracker.ceph.com/projects/cephfs/wiki/Main#wip-vshankar-testing-20240731064922-debug

kotreshhr requested review from a team and batrick July 26, 2024 15:01

github-actions bot added the cephfs Ceph File System label Jul 26, 2024

kotreshhr requested a review from vshankar July 26, 2024 15:01

kotreshhr force-pushed the fuse-mds-auth-caps-issue branch from 2a8c6e8 to f00af71 Compare July 26, 2024 15:06

vshankar reviewed Jul 26, 2024

View reviewed changes

batrick requested changes Jul 26, 2024

View reviewed changes

src/client/Client.cc Outdated Show resolved Hide resolved

kotreshhr force-pushed the fuse-mds-auth-caps-issue branch from f00af71 to 9a7693f Compare July 28, 2024 19:02

github-actions bot added the tests label Jul 28, 2024

kotreshhr force-pushed the fuse-mds-auth-caps-issue branch from 9a7693f to 5eb4f6b Compare July 28, 2024 19:09

kotreshhr changed the title ~~client: Fix path argument to mds_check_access~~ client: Fix leading / issue with mds_check_access Jul 28, 2024

lxbsz reviewed Jul 29, 2024

View reviewed changes

src/client/Client.cc Show resolved Hide resolved

vshankar reviewed Jul 29, 2024

View reviewed changes

src/client/Client.cc Show resolved Hide resolved

batrick requested changes Jul 29, 2024

View reviewed changes

qa/tasks/cephfs/test_admin.py Show resolved Hide resolved

lxbsz reviewed Jul 30, 2024

View reviewed changes

src/client/Client.cc Show resolved Hide resolved

vshankar approved these changes Jul 30, 2024

View reviewed changes

vshankar added the wip-vshankar-testing4 label Jul 30, 2024

kotreshhr force-pushed the fuse-mds-auth-caps-issue branch 2 times, most recently from 711a9d4 to 26a2f6b Compare July 30, 2024 12:50

kotreshhr force-pushed the fuse-mds-auth-caps-issue branch 3 times, most recently from db90f1f to a40b089 Compare July 30, 2024 15:35

kotreshhr added 2 commits July 30, 2024 23:18

qa: Add mds caps test for testing fs read and a path rw

983f893

Fixes: https://tracker.ceph.com/issues/67212 Signed-off-by: Kotresh HR <khiremat@redhat.com>

kotreshhr force-pushed the fuse-mds-auth-caps-issue branch from a40b089 to 2e2adb2 Compare July 30, 2024 17:49

batrick approved these changes Jul 30, 2024

View reviewed changes

lxbsz approved these changes Jul 31, 2024

View reviewed changes

vshankar approved these changes Jul 31, 2024

View reviewed changes

vshankar merged commit e23ec03 into ceph:main Jul 31, 2024

vshankar removed the wip-vshankar-testing4 label Jul 31, 2024

This was referenced Aug 1, 2024

squid: client: Fix leading / issue with mds_check_access #58981

Closed

reef: client: Fix leading / issue with mds_check_access #58982

Merged

kotreshhr mentioned this pull request Sep 8, 2025

squid: Fix multifs auth caps check #65359

Open

Conversation

kotreshhr commented Jul 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution Guidelines

Checklist

Uh oh!

kotreshhr commented Jul 26, 2024

Uh oh!

vshankar Jul 26, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kotreshhr commented Jul 28, 2024

Uh oh!

Uh oh!

Uh oh!

batrick left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vshankar commented Jul 30, 2024

Uh oh!

kotreshhr commented Jul 30, 2024

Uh oh!

kotreshhr commented Jul 30, 2024

Uh oh!

kotreshhr commented Jul 31, 2024

Uh oh!

kotreshhr commented Jul 31, 2024

Uh oh!

vshankar commented Jul 31, 2024

Uh oh!

kotreshhr commented Jul 31, 2024

Uh oh!

vshankar commented Jul 31, 2024

Uh oh!

kotreshhr commented Jul 31, 2024

Uh oh!

kotreshhr commented Jul 31, 2024

Uh oh!

kotreshhr commented Jul 31, 2024

Uh oh!

vshankar commented Jul 31, 2024

Uh oh!

vshankar commented Jul 31, 2024

Uh oh!

vshankar commented Jul 31, 2024

Uh oh!

vshankar commented Jul 31, 2024

Uh oh!

kotreshhr commented Jul 31, 2024

Uh oh!

vshankar left a comment

Choose a reason for hiding this comment

Uh oh!

vshankar commented Jul 31, 2024

Uh oh!

vshankar commented Aug 6, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kotreshhr commented Jul 26, 2024 •

edited

Loading