-
Notifications
You must be signed in to change notification settings - Fork 931
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
storage: Disallow volume.security.shared
on cephfs
#14633
base: main
Are you sure you want to change the base?
Conversation
dae00b1
to
a211f9c
Compare
@masnax Could you please just look at the last commit and confirm if my changes there make sense? |
a211f9c
to
b12cc51
Compare
…cephfs Signed-off-by: hamistao <[email protected]>
Signed-off-by: hamistao <[email protected]>
Signed-off-by: hamistao <[email protected]>
Signed-off-by: hamistao <[email protected]>
b12cc51
to
fb08a39
Compare
@@ -437,6 +437,8 @@ func (d *cephfs) Validate(config map[string]string) error { | |||
// shortdesc: Whether the CephFS file system was empty on creation time | |||
// scope: global | |||
"volatile.pool.pristine": validate.IsAny, | |||
// Override general 'security.shared' rule since only cephfs does not support block volumes. | |||
"volume.security.shared": func(value string) error { return fmt.Errorf("") }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
empty error message looks odd?
|
||
# security.shared is only allowed for block volumes | ||
! lxc storage volume set "${pool}" vol1 security.shared true || false | ||
! lxc storage volume set "${pool}" isoVol security.shared true || false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So does security.shared
not work for iso volumes because they are allowed to be attached concurrently to multiple instances (as this should be allowed because they are readonly)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, they are sharable by default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please can you expand the commit message as to why these were ineffective?
@hamistao ceph tests failing
|
This adds a check to deny enabling
security.shared
on a cephfs pool. Also includes tests for this check and for settingsecurity.shared
in many volume types.