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

pacific: mds: allow all types of mds caps #52583

Closed
wants to merge 3 commits into from

Conversation

rishabh-d-dave
Copy link
Contributor

backport tracker: https://tracker.ceph.com/issues/62028


backport of #51317
parent tracker: https://tracker.ceph.com/issues/59388

this backport was staged using ceph-backport.sh version 16.0.0.6848
find the latest version at https://github.com/ceph/ceph/blob/master/src/script/ceph-backport.sh

@rishabh-d-dave rishabh-d-dave added this to the pacific milestone Jul 21, 2023
@rishabh-d-dave rishabh-d-dave added the cephfs Ceph File System label Jul 21, 2023
@github-actions github-actions bot added the tests label Jul 21, 2023
Comment on lines -75 to +76
root_squash %= (spaces >> lit("root_squash") >> attr(true));
match = -(
(fs_name >> path >> root_squash)[_val = phoenix::construct<MDSCapMatch>(_2, _1, _3)] |
(uid >> gidlist)[_val = phoenix::construct<MDSCapMatch>(_1, _2)] |
(path >> uid >> gidlist)[_val = phoenix::construct<MDSCapMatch>(_1, _2, _3)] |
(fs_name >> path)[_val = phoenix::construct<MDSCapMatch>(_2, _1)] |
(fs_name >> root_squash)[_val = phoenix::construct<MDSCapMatch>(std::string(), _1, _2)] |
(path >> root_squash)[_val = phoenix::construct<MDSCapMatch>(_1, std::string(), _2)] |
(path)[_val = phoenix::construct<MDSCapMatch>(_1)] |
(root_squash)[_val = phoenix::construct<MDSCapMatch>(std::string(), std::string(), _1)] |
(fs_name)[_val = phoenix::construct<MDSCapMatch>(std::string(),
_1)]);
root_squash %= -(spaces >> lit("root_squash") >> attr(true));
match = (fs_name >> path >> root_squash >> uid >> gidlist)[_val = phoenix::construct<MDSCapMatch>(_1, _2, _3, _4, _5)];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vshankar
So far this set of changes hasn't disturbed any tests on the main branch but I've always been worried about merging these changes. Especially now since backports are supposed to be stable compared to main branch.

I've raised this backport since you said backporting is needed but please double check if backporting is fine.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rishabh-d-dave similar comments as in the quincy backport. Please refer that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rishabh-d-dave ping if this change is required for last pacific release...

@github-actions
Copy link

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

MDS caps can contain 5 components: name of a CephFS, a path inside
CephFS, a flag for enabling root squashing mechanism, a UID and list of
GIDs. These 5 components result in 31 combinations, so there can be 31
types of MDS caps. Out of these, the current main branch only allows 11
combinations. This restriction is strange and inappropriate. Ideally,
all combinations should be allowed.

This strange restriction must've been created unintentionally by
previous developers while adding FS name and root squash to MDS caps. A
TODO for a allowing a subset of these combination was also left in
codebase:
https://github.com/ceph/ceph/blob/reef/src/mds/MDSAuthCaps.cc#L69

Fixes: https://tracker.ceph.com/issues/59388
Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit 0c36929)

Conflicts:
	src/mds/MDSAuthCaps.cc
	- Unlike main branch, on Pacific branch, MDSAuthCaps calls
	  string by writing std::string instead of just string.
Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit 248e9da)
Currently we check if the right caps parse successfully and the wrong
caps fail to parse. Increase the test coverage by adding one more
tests -- dump the right caps in to a string after the parsing is
successfully and then re-parse the caps and check if re-parsed caps
dumps to the exact same string.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit bfacee1)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cephfs Ceph File System tests
Projects
None yet
2 participants