E56C
Skip to content

storage: Disallow volume.security.shared on cephfs #14633

Merged
tomponline merged 4 commits intocanonical:mainfrom
hamistao:cephfs_tests
Dec 13, 2024
Merged

storage: Disallow volume.security.shared on cephfs #14633
tomponline merged 4 commits intocanonical:mainfrom
hamistao:cephfs_tests

Conversation

@hamistao
Copy link
Copy Markdown
@hamistao hamistao commented Dec 10, 2024

This adds a check to deny enabling security.shared on a cephfs pool. Also includes tests for this check and for setting security.shared in many volume types.

@hamistao hamistao force-pushed the cephfs_tests branch 2 times, most recently from dae00b1 to a211f9c Compare December 11, 2024 19:17
@hamistao
Copy link
Copy Markdown
Author
hamistao commented Dec 11, 2024

@masnax Could you please just look at the last commit and confirm if my changes there make sense?

@hamistao hamistao force-pushed the cephfs_tests branch 2 times, most recently from b12cc51 to fb08a39 Compare December 12, 2024 04:53
@hamistao hamistao marked this pull request as ready for review December 12, 2024 04:54
@hamistao hamistao marked this pull request as draft December 12, 2024 05:18
@hamistao hamistao marked this pull request as ready for review December 12, 2024 06:21
tomponline
tomponline reviewed Dec 12, 2024
View reviewed changes
Comment thread lxd/storage/drivers/driver_cephfs.go Outdated
tomponline
tomponline reviewed Dec 12, 2024
View reviewed changes
Comment thread test/suites/storage_local_volume_handling.sh
tomponline
tomponline reviewed Dec 12, 2024
View reviewed changes
Comment thread test/suites/storage_driver_cephfs.sh
@tomponline
Copy link
Copy Markdown
Member
tomponline commented Dec 12, 2024

@hamistao ceph tests failing

+ lxc storage create cephfs cephfs source=cephfs/Q5A
++ timeout --foreground 120 /home/runner/go/bin/lxc storage create cephfs cephfs source=cephfs/Q5A --verbose
Error: Invalid value for option "volume.security.shared":

@hamistao hamistao force-pushed the cephfs_tests branch 2 times, most recently from a0261ad to 0f45208 Compare December 13, 2024 04:47
hamistao added 4 commits December 13, 2024 02:16
lxd/storage/drivers/cephfs: Disallow set volume.security.shared on …
a5b1222 
…cephfs

Signed-off-by: hamistao <pedro.ribeiro@canonical.com>
test/suites: Test security.shared is not allowed for cephfs
ed162af 
Signed-off-by: hamistao <pedro.ribeiro@canonical.com>
test/suites: Test security.shared is only allowed in block vols
9eabb51 
Signed-off-by: hamistao <pedro.ribeiro@canonical.com>
test/suites: Fix ineffective tests
12a4cc2 
!A or `true` is always true, so if we want to test the outcome of A, we
should do !A or `false`

Signed-off-by: hamistao <pedro.ribeiro@canonical.com>
@hamistao hamistao marked this pull request as draft December 13, 2024 05:19
@hamistao hamistao marked this pull request as ready for review December 13, 2024 12:32
@hamistao
Copy link
Copy Markdown
Author
hamistao commented Dec 13, 2024

@tomponline Tests fixed and comments addressed

tomponline
tomponline approved these changes Dec 13, 2024
View reviewed changes
Copy link
Copy Markdown
Member
@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

Thanks!

@tomponline tomponline merged commit 264ab92 into canonical:main Dec 13, 2024
@hamistao hamistao deleted the cephfs_tests branch December 13, 2024 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tomponline tomponline tomponline approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@hamistao @tomponline
0