Add semantic checks

For example, reject MDRaid items with Device=/dev/blah and Filesystem
items without a Filesystem:mount_path=/where_to_mount

Signed-off-by: David Zeuthen <davidz@redhat.com>

9 files changed, 148 insertions, 15 deletions

diff --git a/stc/stcenums.h b/stc/stcenums.h
index 4523503..715c15b 100644
--- a/stc/stcenums.h
+++ b/stc/stcenums.h
@@ -37,6 +37,7 @@ G_BEGIN_DECLS
  * @STC_ERROR_PARSE_ERROR: A configurtion item is invalid.
  * @STC_ERROR_DUPLICATE_ITEM: A configuration item with an existing identifier was encountered.
  * @STC_ERROR_UNRESOLVED_DEPENDENCY: A configuration item has a dependency that cannot be resolved.
+ * @STC_ERROR_INSUFFICIENT_DATA: Insufficient amount of information in configuration item.
  *
  * Error codes for the #STC_ERROR error domain.
  */
@@ -45,7 +46,8 @@ typedef enum
   STC_ERROR_FAILED,
   STC_ERROR_PARSE_ERROR,
   STC_ERROR_DUPLICATE_ITEM,
-  STC_ERROR_UNRESOLVED_DEPENDENCY
+  STC_ERROR_UNRESOLVED_DEPENDENCY,
+  STC_ERROR_INSUFFICIENT_DATA
 } StcError;
 
 /**
diff --git a/stc/stcmonitor.c b/stc/stcmonitor.c
index afbfe7c..9620a76 100644
--- a/stc/stcmonitor.c
+++ b/stc/stcmonitor.c
@@ -532,6 +532,90 @@ stc_monitor_get_item_by_id (StcMonitor          *monitor,
   return ret;
 }
 
+static gboolean
+check_validity (StcMonitor   *monitor,
+                const gchar  *item_id,
+                StcItemType   item_type,
+                const gchar  *target,
+                GHashTable   *options,
+                const gchar  *path,
+                gint          line_no)
+{
+  gboolean ret;
+
+  ret = FALSE;
+
+  switch (item_type)
+    {
+    case STC_ITEM_TYPE_FILESYSTEM:
+      if (!(g_str_has_prefix (target, "Device=") ||
+            g_str_has_prefix (target, "UUID=") ||
+            g_str_has_prefix (target, "Label=")))
+        {
+          stc_monitor_emit_stc_error (monitor,
+                                      path,
+                                      line_no,
+                                      STC_ERROR_FAILED,
+                                      "Filesystem item %s has unsupported target type (Device, UUID and Label is supported).",
+                                      item_id);
+          goto out;
+        }
+      if (g_hash_table_lookup (options, "Filesystem:mount_path") == NULL)
+        {
+          stc_monitor_emit_stc_error (monitor,
+                                      path,
+                                      line_no,
+                                      STC_ERROR_INSUFFICIENT_DATA,
+                                      "Filesystem item %s is missing option Filesystem:mount_path.",
+                                      item_id);
+          goto out;
+        }
+      break;
+
+    case STC_ITEM_TYPE_MD_RAID:
+      if (!(g_str_has_prefix (target, "UUID=") ||
+            g_str_has_prefix (target, "Name=")))
+        {
+          stc_monitor_emit_stc_error (monitor,
+                                      path,
+                                      line_no,
+                                      STC_ERROR_FAILED,
+                                      "MDRaid item %s has unsupported target type (UUID and Name is supported).",
+                                      item_id);
+          goto out;
+        }
+      break;
+
+    case STC_ITEM_TYPE_LUKS:
+      if (!(g_str_has_prefix (target, "Device=") ||
+            g_str_has_prefix (target, "UUID=")))
+        {
+          stc_monitor_emit_stc_error (monitor,
+                                      path,
+                                      line_no,
+                                      STC_ERROR_FAILED,
+                                      "MDRaid item %s has unsupported target type (UUID and Device is supported).",
+                                      item_id);
+          goto out;
+        }
+      break;
+
+    default:
+      stc_monitor_emit_stc_error (monitor,
+                                  path,
+                                  line_no,
+                                  STC_ERROR_FAILED,
+                                  "Unsupported item type %d.",
+                                  item_type);
+      goto out;
+    }
+
+  ret = TRUE;
+
+ out:
+  return ret;
+}
+
 static GList *
 stc_monitor_load_one_file (StcMonitor   *monitor,
                            const gchar  *path,
@@ -613,7 +697,7 @@ stc_monitor_load_one_file (StcMonitor   *monitor,
           goto process_next_item;
         }
 
-      /* resolve target (TODO: bail if more than one of the allowed keys are present) */
+      /* resolve target (TODO: maybe bail if more than one of the allowed keys are present) */
       if ((s = g_key_file_get_string (key_file, group, "Device", NULL)) != NULL)
         {
           target = g_strdup_printf ("Device=%s", s);
@@ -629,13 +713,18 @@ stc_monitor_load_one_file (StcMonitor   *monitor,
           target = g_strdup_printf ("Label=%s", s);
           g_free (s);
         }
+      else if ((s = g_key_file_get_string (key_file, group, "Name", NULL)) != NULL)
+        {
+          target = g_strdup_printf ("Name=%s", s);
+          g_free (s);
+        }
       else
         {
           stc_monitor_emit_stc_error (monitor,
                                       path,
                                       -1,
                                       STC_ERROR_PARSE_ERROR,
-                                      "No target key (supported: Device, UUID, Label) keys found "
+                                      "No target key (supported: Device, UUID, Label, Name) keys found "
                                       "in item %s of type %s.",
                                       item_id,
                                       tokens[0]);
@@ -679,6 +768,18 @@ stc_monitor_load_one_file (StcMonitor   *monitor,
       /* Depends is optional */
       depends_strv = g_key_file_get_string_list (key_file, group, "Depends", NULL, NULL);
 
+      /* Final check: semantics */
+      if (!check_validity (monitor,
+                           item_id,
+                           item_type,
+                           target,
+                           options,
+                           path,
+                           -1))
+        {
+          goto process_next_item;
+        }
+
       sort_key = g_strdup_printf ("%s:%s", path, item_id);
       item = _stc_item_new (item_type,
                             item_id,
diff --git a/stc/test.c b/stc/test.c
index 68b4ebe..22e5d73 100644
--- a/stc/test.c
+++ b/stc/test.c
@@ -187,6 +187,7 @@ test_stc_semi_valid_conf (void)
                    SRCDIR "/testdata/semi_valid_conf/stc.conf: Invalid item identifer `with-dash' in group `Filesystem with-dash'. Only characters in [a-zA-Z0-9_] are supported. (stc-error-quark, 1)\n"
                    SRCDIR "/testdata/semi_valid_conf/stc.conf: Error parsing group name `Filesystem with space' (stc-error-quark, 1)\n"
                    SRCDIR "/testdata/semi_valid_conf/stc.conf: Element 0 of Options, `foo', for item foo of type MDRaid is malformed (no equal sign found). (stc-error-quark, 1)\n"
+                   SRCDIR "/testdata/semi_valid_conf/stc.conf: MDRaid item bar has unsupported target type (UUID and Name is supported). (stc-error-quark, 0)\n"
                    SRCDIR "/testdata/semi_valid_conf/stc.conf.d/91.conf: Encountered item with duplicate id `multiple_instances_same_id'. Ignoring previous items. (stc-error-quark, 2)\n"
                    SRCDIR "/testdata/semi_valid_conf/stc.conf: Item ZItemWithUnresolved has unresolved dependency NonExisting. (stc-error-quark, 3)\n"
                    );
@@ -203,35 +204,35 @@ test_stc_semi_valid_conf (void)
                    "target       `Device=/dev/sda'\n"
                    "nick-name    `(none)'\n"
                    "type          filesystem\n"
-                   "options       (none)\n"
+                   "options       Filesystem:mount_path -> `/mnt/10'\n"
                    "dependencies  (none)\n"
                    "\n"
                    "id           `in_pri_50'\n"
                    "target       `Device=/dev/sda'\n"
                    "nick-name    `(none)'\n"
                    "type          filesystem\n"
-                   "options       (none)\n"
+                   "options       Filesystem:mount_path -> `/mnt/50'\n"
                    "dependencies  (none)\n"
                    "\n"
                    "id           `in_pri_90'\n"
                    "target       `Device=/dev/sda'\n"
                    "nick-name    `(none)'\n"
                    "type          filesystem\n"
-                   "options       (none)\n"
+                   "options       Filesystem:mount_path -> `/mnt/90'\n"
                    "dependencies  (none)\n"
                    "\n"
                    "id           `multiple_instances_same_id'\n"
                    "target       `Device=/dev/multiple_instances_same_id/second'\n"
                    "nick-name    `(none)'\n"
                    "type          filesystem\n"
-                   "options       (none)\n"
+                   "options       Filesystem:mount_path -> `/mnt/m_second'\n"
                    "dependencies  (none)\n"
                    "\n"
                    "id           `BigStorage'\n"
                    "target       `Device=/dev/disk/md-uuid-01234:56789'\n"
                    "nick-name    `BigStorage'\n"
                    "type          filesystem\n"
-                   "options       (none)\n"
+                   "options       Filesystem:mount_path -> `/mnt/BigStorage'\n"
                    "dependencies  BigStorage_mdraid\n"
                    "\n"
                    "id           `BigStorage_mdraid'\n"
@@ -260,9 +261,16 @@ test_stc_semi_valid_conf (void)
                    "target       `Device=/dev/sda'\n"
                    "nick-name    `(none)'\n"
                    "type          filesystem\n"
-                   "options       (none)\n"
+                   "options       Filesystem:mount_path -> `/mnt/Z'\n"
                    "dependencies  NonExisting\n"
                    "\n"
+                   "id           `ZOtherRaid'\n"
+                   "target       `Name=homehost:array_name'\n"
+                   "nick-name    `Other RAID'\n"
+                   "type          md-raid\n"
+                   "options       (none)\n"
+                   "dependencies  (none)\n"
+                   "\n"
                    );
   g_string_free (str2, TRUE);
 
@@ -421,7 +429,8 @@ test_stc_monitoring (void)
   error = NULL;
   rc = g_file_set_contents (filename,
                             "[Filesystem item1]\n"
-                            "Device=/dev/sdb\n",
+                            "Device=/dev/sdb\n"
+                            "Options=Filesystem:mount_path=/item1",
                             -1,
                             &error);
   g_assert_no_error (error);
@@ -436,9 +445,11 @@ test_stc_monitoring (void)
   rc = g_file_set_contents (filename,
                             "[Filesystem item1]\n"
                             "Device=/dev/sdb\n"
+                            "Options=Filesystem:mount_path=/item1"
                             "\n"
                             "[Filesystem item2]\n"
-                            "Device=/dev/sdc\n",
+                            "Device=/dev/sdc\n"
+                            "Options=Filesystem:mount_path=/item2",
                             -1,
                             &error);
   g_assert_no_error (error);
@@ -452,7 +463,8 @@ test_stc_monitoring (void)
   error = NULL;
   rc = g_file_set_contents (filename,
                             "[Filesystem item2]\n"
-                            "Device=/dev/sdc\n",
+                            "Device=/dev/sdc\n"
+                            "Options=Filesystem:mount_path=/item2",
                             -1,
                             &error);
   g_assert_no_error (error);
@@ -466,7 +478,8 @@ test_stc_monitoring (void)
   error = NULL;
   rc = g_file_set_contents (filename,
                             "[Filesystem item2]\n"
-                            "Device=/dev/sda\n",
+                            "Device=/dev/sda\n"
+                            "Options=Filesystem:mount_path=/item2",
                             -1,
                             &error);
   g_assert_no_error (error);
@@ -480,7 +493,8 @@ test_stc_monitoring (void)
   error = NULL;
   rc = g_file_set_contents (filename,
                             "[Filesystem item3]\n"
-                            "Device=/dev/sda\n",
+                            "Device=/dev/sda\n"
+                            "Options=Filesystem:mount_path=/item3",
                             -1,
                             &error);
   g_assert_no_error (error);
diff --git a/stc/testdata/semi_valid_conf/stc.conf b/stc/testdata/semi_valid_conf/stc.conf
index a99a2d2..1a5c071 100644
--- a/stc/testdata/semi_valid_conf/stc.conf
+++ b/stc/testdata/semi_valid_conf/stc.conf
@@ -4,7 +4,7 @@
 [Filesystem BigStorage]
 NickName=BigStorage
 Device=/dev/disk/md-uuid-01234:56789
-Filesystem:mount_path=/mnt/BigStorage;
+Options=Filesystem:mount_path=/mnt/BigStorage
 Depends=BigStorage_mdraid
 Auto=yes
 
@@ -25,8 +25,13 @@ Options=LUKS:password=xyz123
 
 [Filesystem ZItemWithUnresolved]
 Device=/dev/sda
+Options=Filesystem:mount_path=/mnt/Z
 Depends=NonExisting
 
+[MDRaid ZOtherRaid]
+NickName=Other RAID
+Name=homehost:array_name
+
 # ...and that we reject invalid items
 #
 
@@ -46,3 +51,8 @@ Depends=NonExisting
 [MDRaid foo]
 UUID=01234:56789
 Options=foo
+
+# can't use Device for MDRaid (it doesn't exist yet)
+[MDRaid bar]
+NickName=Other RAID
+Device=/dev/md0
diff --git a/stc/testdata/semi_valid_conf/stc.conf.d/10.conf b/stc/testdata/semi_valid_conf/stc.conf.d/10.conf
index 73b0c95..33dd844 100644
--- a/stc/testdata/semi_valid_conf/stc.conf.d/10.conf
+++ b/stc/testdata/semi_valid_conf/stc.conf.d/10.conf
@@ -1,3 +1,4 @@
 
 [Filesystem in_pri_10]
 Device=/dev/sda
+Options=Filesystem:mount_path=/mnt/10
diff --git a/stc/testdata/semi_valid_conf/stc.conf.d/50.conf b/stc/testdata/semi_valid_conf/stc.conf.d/50.conf
index bad4e88..f973184 100644
--- a/stc/testdata/semi_valid_conf/stc.conf.d/50.conf
+++ b/stc/testdata/semi_valid_conf/stc.conf.d/50.conf
@@ -1,3 +1,4 @@
 
 [Filesystem in_pri_50]
 Device=/dev/sda
+Options=Filesystem:mount_path=/mnt/50
diff --git a/stc/testdata/semi_valid_conf/stc.conf.d/50.conf.wrong-extension b/stc/testdata/semi_valid_conf/stc.conf.d/50.conf.wrong-extension
index fae6617..a0f8a5d 100644
--- a/stc/testdata/semi_valid_conf/stc.conf.d/50.conf.wrong-extension
+++ b/stc/testdata/semi_valid_conf/stc.conf.d/50.conf.wrong-extension
@@ -3,3 +3,4 @@
 # it will cause a duplicate-error since in_pri_50 already exists
 [Filesystem in_pri_50]
 Device=/dev/sda
+Options=Filesystem:mount_path=/mnt/50;
diff --git a/stc/testdata/semi_valid_conf/stc.conf.d/90.conf b/stc/testdata/semi_valid_conf/stc.conf.d/90.conf
index 15a3de1..a2baa63 100644
--- a/stc/testdata/semi_valid_conf/stc.conf.d/90.conf
+++ b/stc/testdata/semi_valid_conf/stc.conf.d/90.conf
@@ -1,7 +1,9 @@
 
 [Filesystem in_pri_90]
 Device=/dev/sda
+Options=Filesystem:mount_path=/mnt/90
 
 # this is for testing that we replace the previous entry (see also 91.conf)
 [Filesystem multiple_instances_same_id]
 Device=/dev/multiple_instances_same_id/first
+Options=Filesystem:mount_path=/mnt/m_first
diff --git a/stc/testdata/semi_valid_conf/stc.conf.d/91.conf b/stc/testdata/semi_valid_conf/stc.conf.d/91.conf
index a97db0f..0532a4b 100644
--- a/stc/testdata/semi_valid_conf/stc.conf.d/91.conf
+++ b/stc/testdata/semi_valid_conf/stc.conf.d/91.conf
@@ -1,3 +1,4 @@
 # this is for testing that we replace the previous entry (see also 90.conf)
 [Filesystem multiple_instances_same_id]
 Device=/dev/multiple_instances_same_id/second
+Options=Filesystem:mount_path=/mnt/m_second