[lxc-devel] [lxd/master] storage pools: fix + improve config validation
brauner on Github
lxc-bot at linuxcontainers.org
Tue Apr 11 19:22:15 UTC 2017
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 364 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20170411/688760ea/attachment.bin>
-------------- next part --------------
From 91d85e7d6406b69fef15a7da711454eb62fe2c70 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 11 Apr 2017 12:22:20 +0200
Subject: [PATCH 1/2] storage pools: fix + improve config validation
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
lxd/storage_pools_config.go | 84 ++++++++++++++++++++++-----------------------
1 file changed, 41 insertions(+), 43 deletions(-)
diff --git a/lxd/storage_pools_config.go b/lxd/storage_pools_config.go
index 076be9a..e8e1726 100644
--- a/lxd/storage_pools_config.go
+++ b/lxd/storage_pools_config.go
@@ -10,7 +10,11 @@ import (
)
var storagePoolConfigKeys = map[string]func(value string) error{
- "source": shared.IsAny,
+ // valid drivers: lvm
+ "lvm.thinpool_name": shared.IsAny,
+ "lvm.vg_name": shared.IsAny,
+
+ // valid drivers: btrfs, lvm, zfs
"size": func(value string) error {
if value == "" {
return nil
@@ -19,10 +23,17 @@ var storagePoolConfigKeys = map[string]func(value string) error{
_, err := shared.ParseByteSizeString(value)
return err
},
- "volume.block.mount_options": shared.IsAny,
+
+ // valid drivers: btrfs, dir, lvm, zfs
+ "source": shared.IsAny,
+
+ // valid drivers: lvm
"volume.block.filesystem": func(value string) error {
return shared.IsOneOf(value, []string{"ext4", "xfs"})
},
+ "volume.block.mount_options": shared.IsAny,
+
+ // valid drivers: lvm
"volume.size": func(value string) error {
if value == "" {
return nil
@@ -31,12 +42,14 @@ var storagePoolConfigKeys = map[string]func(value string) error{
_, err := shared.ParseByteSizeString(value)
return err
},
- "volume.zfs.use_refquota": shared.IsBool,
+
+ // valid drivers: zfs
"volume.zfs.remove_snapshots": shared.IsBool,
- "lvm.thinpool_name": shared.IsAny,
- "lvm.vg_name": shared.IsAny,
- "zfs.pool_name": shared.IsAny,
- "zfs.clone_copy": shared.IsBool,
+ "volume.zfs.use_refquota": shared.IsBool,
+
+ // valid drivers: zfs
+ "zfs.clone_copy": shared.IsBool,
+ "zfs.pool_name": shared.IsAny,
}
func storagePoolValidateConfig(name string, driver string, config map[string]string) error {
@@ -47,57 +60,42 @@ func storagePoolValidateConfig(name string, driver string, config map[string]str
return err
}
+ // Check whether the config properties for the driver container sane
+ // values.
for key, val := range config {
// User keys are not validated.
if strings.HasPrefix(key, "user.") {
continue
}
- // Validate storage pool config keys.
- validator, ok := storagePoolConfigKeys[key]
- if !ok {
- return fmt.Errorf("Invalid storage pool configuration key: %s", key)
- }
-
- err := validator(val)
- if err != nil {
- return err
- }
-
- if driver != "zfs" || driver == "dir" {
- if config["volume.zfs.use_refquota"] != "" {
- return fmt.Errorf("The key volume.zfs.use_refquota cannot be used with non zfs storage pools.")
- }
-
- if config["volume.zfs.remove_snapshots"] != "" {
- return fmt.Errorf("The key volume.zfs.remove_snapshots cannot be used with non zfs storage pools.")
- }
-
- if config["zfs.pool_name"] != "" {
- return fmt.Errorf("The key zfs.pool_name cannot be used with non zfs storage pools.")
+ prfx := strings.HasPrefix
+ if driver != "zfs" {
+ if prfx(key, "volume.zfs.") || prfx(key, "zfs.") {
+ return fmt.Errorf("The key %s cannot be used with %s storage pools.", key, strings.ToUpper(driver))
}
+ }
- if config["zfs.clone_copy"] != "" {
- return fmt.Errorf("The key zfs.clone_copy cannot be used with non zfs storage pools.")
+ if driver != "lvm" {
+ if prfx(key, "lvm.") || prfx(key, "volume.block.") || key == "volume.size" {
+ return fmt.Errorf("The key %s cannot be used with %s storage pools.", key, strings.ToUpper(driver))
}
}
if driver == "dir" {
- if config["size"] != "" {
- return fmt.Errorf("The key size cannot be used with dir storage pools.")
- }
-
- if config["volume.block.mount_options"] != "" {
- return fmt.Errorf("The key volume.block.mount_options cannot be used with dir storage pools.")
+ if key == "size" {
+ return fmt.Errorf("The key %s cannot be used with %s storage pools.", key, strings.ToUpper(driver))
}
+ }
- if config["volume.block.filesystem"] != "" {
- return fmt.Errorf("The key volume.block.filesystem cannot be used with dir storage pools.")
- }
+ // Validate storage pool config keys.
+ validator, ok := storagePoolConfigKeys[key]
+ if !ok {
+ return fmt.Errorf("Invalid storage pool configuration key: %s", key)
+ }
- if config["volume.size"] != "" {
- return fmt.Errorf("The key volume.size cannot be used with dir storage pools.")
- }
+ err := validator(val)
+ if err != nil {
+ return err
}
}
From c3443e0b1781a8c2f4f58002ef4908bbca9f8254 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 11 Apr 2017 12:22:55 +0200
Subject: [PATCH 2/2] tests: add additional storage pool tests
This adds tests to verify:
1. no invalid storage pool config keys for a given driver can be set
2. all valid storage pool config keys for a given driver can be set
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
test/suites/storage.sh | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 69 insertions(+)
diff --git a/test/suites/storage.sh b/test/suites/storage.sh
index 63596df..b10fc05 100644
--- a/test/suites/storage.sh
+++ b/test/suites/storage.sh
@@ -37,6 +37,26 @@ test_storage() {
configure_loop_device loop_file_1 loop_device_1
# shellcheck disable=SC2154
lxc storage create "lxdtest-$(basename "${LXD_DIR}")-pool2" zfs source="${loop_device_1}"
+
+ # Test that no invalid zfs storage pool configuration keys can be set.
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-zfs-pool-config" zfs lvm.thinpool_name=bla
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-zfs-pool-config" zfs lvm.vg_name=bla
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-zfs-pool-config" zfs volume.block.filesystem=ext4
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-zfs-pool-config" zfs volume.block.mount_options=discard
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-zfs-pool-config" zfs volume.size=2GB
+
+ # Test that all valid zfs storage pool configuration keys can be set.
+ lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-zfs-pool-config" zfs volume.zfs.remove_snapshots=true
+ lxc storage delete "lxdtest-$(basename "${LXD_DIR}")-invalid-zfs-pool-config"
+
+ lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-zfs-pool-config" zfs volume.zfs.use_refquota=true
+ lxc storage delete "lxdtest-$(basename "${LXD_DIR}")-invalid-zfs-pool-config"
+
+ lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-zfs-pool-config" zfs zfs.clone_copy=true
+ lxc storage delete "lxdtest-$(basename "${LXD_DIR}")-invalid-zfs-pool-config"
+
+ lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-zfs-pool-config" zfs zfs.pool_name=bla
+ lxc storage delete "lxdtest-$(basename "${LXD_DIR}")-invalid-zfs-pool-config"
fi
if which btrfs >/dev/null 2>&1; then
@@ -50,6 +70,17 @@ test_storage() {
# Check that we cannot create storage pools inside of ${LXD_DIR} other than ${LXD_DIR}/storage-pools/{pool_name}.
! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-pool5_under_lxd_dir" btrfs source="${LXD_DIR}"
+
+ # Test that no invalid btrfs storage pool configuration keys can be set.
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-btrfs-pool-config" btrfs lvm.thinpool_name=bla
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-btrfs-pool-config" btrfs lvm.vg_name=bla
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-btrfs-pool-config" btrfs volume.block.filesystem=ext4
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-btrfs-pool-config" btrfs volume.block.mount_options=discard
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-btrfs-pool-config" btrfs volume.size=2GB
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-btrfs-pool-config" btrfs volume.zfs.remove_snapshots=true
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-btrfs-pool-config" btrfs volume.zfs.use_refquota=true
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-btrfs-pool-config" btrfs zfs.clone_copy=true
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-btrfs-pool-config" btrfs zfs.pool_name=bla
fi
# Create dir pool.
@@ -63,6 +94,18 @@ test_storage() {
lxc storage delete "lxdtest-$(basename "${LXD_DIR}")-pool5_under_lxd_dir"
+ # Test that no invalid dir storage pool configuration keys can be set.
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-dir-pool-config" dir lvm.thinpool_name=bla
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-dir-pool-config" dir lvm.vg_name=bla
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-dir-pool-config" dir size=10GB
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-dir-pool-config" dir volume.block.filesystem=ext4
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-dir-pool-config" dir volume.block.mount_options=discard
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-dir-pool-config" dir volume.size=2GB
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-dir-pool-config" dir volume.zfs.remove_snapshots=true
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-dir-pool-config" dir volume.zfs.use_refquota=true
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-dir-pool-config" dir zfs.clone_copy=true
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-dir-pool-config" dir zfs.pool_name=bla
+
if which lvdisplay >/dev/null 2>&1; then
# Create lvm pool.
configure_loop_device loop_file_3 loop_device_3
@@ -101,6 +144,32 @@ test_storage() {
lxc storage create "lxdtest-$(basename "${LXD_DIR}")-pool13" lvm source="${loop_device_8}" lvm.vg_name="lxdtest-$(basename "${LXD_DIR}")-pool13-dummy_vg_4" volume.size=25MB
lxc storage create "lxdtest-$(basename "${LXD_DIR}")-pool14" lvm volume.size=25MB
+
+ # Test that no invalid lvm storage pool configuration keys can be set.
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-lvm-pool-config" lvm volume.zfs.remove_snapshots=true
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-lvm-pool-config" lvm volume.zfs_use_refquota=true
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-lvm-pool-config" lvm zfs.clone_copy=true
+ ! lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-lvm-pool-config" lvm zfs.pool_name=bla
+
+ # Test that all valid lvm storage pool configuration keys can be set.
+ lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-lvm-pool-config" lvm lvm.thinpool_name=bla
+ lxc storage delete "lxdtest-$(basename "${LXD_DIR}")-invalid-lvm-pool-config"
+
+ lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-lvm-pool-config" lvm lvm.vg_name=bla
+ lxc storage delete "lxdtest-$(basename "${LXD_DIR}")-invalid-lvm-pool-config"
+
+ lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-lvm-pool-config" lvm size=10GB
+ lxc storage delete "lxdtest-$(basename "${LXD_DIR}")-invalid-lvm-pool-config"
+
+ lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-lvm-pool-config" lvm volume.block.filesystem=ext4
+ lxc storage delete "lxdtest-$(basename "${LXD_DIR}")-invalid-lvm-pool-config"
+
+ lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-lvm-pool-config" lvm volume.block.mount_options=discard
+ lxc storage delete "lxdtest-$(basename "${LXD_DIR}")-invalid-lvm-pool-config"
+
+ lxc storage create "lxdtest-$(basename "${LXD_DIR}")-invalid-lvm-pool-config" lvm volume.size=2GB
+ lxc storage delete "lxdtest-$(basename "${LXD_DIR}")-invalid-lvm-pool-config"
+
fi
# Set default storage pool for image import.
More information about the lxc-devel
mailing list