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

Enforce security.shifted and security.unmapped to be mutually exclusive. #12316

Conversation

markylaing
Copy link
Contributor

Adds a check to the common driver volume validation that the security.shifted and security.unmapped config options are only set on custom volumes and cannot be set at the same time.

@markylaing markylaing added the Bug Confirmed to be a bug label Sep 26, 2023
@markylaing markylaing self-assigned this Sep 26, 2023
Copy link
Contributor

@monstermunchkin monstermunchkin left a comment

Choose a reason for hiding this comment

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

Do you think we need tests for setting security.shifted and security.unmapped on non-custom volumes?

MusicDin
MusicDin previously approved these changes Sep 26, 2023
@markylaing
Copy link
Contributor Author

Do you think we need tests for setting security.shifted and security.unmapped on non-custom volumes?

It turns out that these are only added to the validation rules if the volume is a custom filesystem volume:

lxd/lxd/storage/utils.go

Lines 444 to 447 in 188c290

if (vol == nil) || (vol != nil && vol.Type() == drivers.VolumeTypeCustom && vol.ContentType() == drivers.ContentTypeFS) {
rules["security.shifted"] = validate.Optional(validate.IsBool)
rules["security.unmapped"] = validate.Optional(validate.IsBool)
}

So the extra validation is unnecessary. I'll remove the extra checks and just ensure they are mutually exclusive.

'security.unmapped' and 'security.shifted' only make sense for custom
volumes and they may not be set together.

Signed-off-by: Mark Laing <mark.laing@canonical.com>
This will be checked via the call to ValidateVolume above, which calls
the validator in the common driver.

Signed-off-by: Mark Laing <mark.laing@canonical.com>
Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing markylaing force-pushed the storage-volume-config-mutually-exclusive branch from b355745 to 527479b Compare September 26, 2023 10:15
@monstermunchkin monstermunchkin merged commit 88114ee into canonical:main Sep 26, 2023
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Confirmed to be a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants