tls: accept the domain and its type as named parameters too

Rejecting 'domain=' and 'type=' meant the identity of a row was the one
thing that could not be written the way every other column is, which is
hard to justify to someone who just read the column list.

Take everything as 'column=value', keep the first two arguments as a
shorthand for the domain and its type, and refuse only the combination
of the two, which is the single ambiguous case.  Whatever is left out is
then asked for, in one place instead of one per spelling.

This folds tls_db_domain() into tls_db_params(), so db_show and
db_delete go through the same parser and now report the columns they do
not take.  'id' stays refused: the database generates it.

Author: dariusstefan

docs/modules/tls.md

@@ -20,10 +20,16 @@ as paths to files.  A TLS domain is identified by its name and its type
 ```
 opensips-cli -x tls db_delete a.example.org server
 ```
-The commands addressing a single domain ask for the type when it is not given.
-`db_add` and `db_show` default to `server`, while `db_update` and `db_delete`
-have no default and keep asking until one is given, so that they cannot change
-a different domain than the intended one.
+The domain and its type may also be given by name, in which case they can
+appear anywhere among the other columns:
+```
+opensips-cli -x tls db_delete domain=a.example.org type=server
+```
+Giving one of them both ways at once is an error.  The commands addressing a
+single domain ask for whatever is left out; `db_add` and `db_show` default the
+type to `server`, while `db_update` and `db_delete` have no default and keep
+asking until one is given, so that they cannot change a different domain than
+the intended one.
 
 `db_add` and `db_update` take the remaining `tls_mgm` columns as `column=value`
 arguments, in any order and after the domain and the type:

opensipscli/modules/tls.py

@@ -38,8 +38,7 @@
 TLS_CALIST_COL = "ca_list"
 TLS_DH_COL = "dh_params"
 
-# the tls_mgm columns that can be provisioned; 'id' is auto-incremented, while
-# 'domain' and 'type' identify the row and are passed as arguments
+# the tls_mgm columns that can be provisioned
 TLS_MGM_COLUMNS = ["match_ip_address", "match_sip_domain", "method",
         "verify_cert", "require_cert", TLS_CERT_COL, TLS_PK_COL,
         "crl_check_all", "crl_dir", TLS_CALIST_COL, "ca_dir", "cipher_list",
@@ -48,8 +47,11 @@
 # columns holding PEM content, which is read from the file they point to
 TLS_PEM_COLUMNS = [TLS_CERT_COL, TLS_PK_COL, TLS_CALIST_COL, TLS_DH_COL]
 
-# columns identifying a row, which are not provisioned as 'column=value'
-TLS_ID_COLUMNS = ["id", TLS_DOMAIN_COL, TLS_TYPE_COL]
+# columns identifying a row, in the order they are accepted as arguments
+TLS_KEY_COLUMNS = [TLS_DOMAIN_COL, TLS_TYPE_COL]
+
+# generated by the database, never provisioned
+TLS_ID_COL = "id"
 
 # as defined by CLIENT_DOMAIN_TYPE/SERVER_DOMAIN_TYPE in tls_mgm/tls_domain.h
 TLS_DOMAIN_TYPES = {"client": 1, "server": 2}
@@ -411,37 +413,6 @@ def tls_db_connect(self):
 
         return db
 
-    def tls_db_domain(self, params, require_type=False):
-        """
-        resolves the (domain, type) pair identifying a tls_mgm row; commands
-        that change an existing row get no default type, so that they cannot
-        pick a different row than the intended one
-        """
-        if len(params) > 0:
-            domain = params[0]
-        else:
-            domain = cfg.read_param(None,
-                    "Please provide the name of the TLS domain")
-            if not domain:
-                logger.error("no TLS domain specified!")
-                return None, None
-
-        if len(params) > 1:
-            dtype = params[1]
-        else:
-            dtype = cfg.read_param(None, "TLS domain type (server/client)",
-                    None if require_type else "server")
-            if not dtype:
-                logger.error("no TLS domain type specified!")
-                return None, None
-
-        if dtype.lower() not in TLS_DOMAIN_TYPES:
-            logger.error("invalid TLS domain type '%s': "
-                    "expected 'server' or 'client'", dtype)
-            return None, None
-
-        return domain, TLS_DOMAIN_TYPES[dtype.lower()]
-
     def tls_db_reload(self):
         """
         makes a running OpenSIPS pick up the tls_mgm changes
@@ -452,21 +423,21 @@ def tls_db_reload(self):
 
     def tls_db_params(self, params, require_type=False):
         """
-        splits the params into the 'column=value' assignments and the
-        (domain, type) pair identifying the row; the value of a PEM column is
-        the path of the file holding it.  The columns are parsed first, so that
-        a bad one is reported without asking for the domain beforehand
+        resolves the (domain, type) pair identifying a tls_mgm row, along with
+        the columns to provision.  Everything is given as 'column=value', with
+        the domain and its type also accepted as the first two arguments; what
+        is left out is asked for.  The value of a PEM column is the path of the
+        file holding it
         """
         cols = {}
         for param in [p for p in params if '=' in p]:
             col, val = param.split('=', 1)
-            if col in TLS_ID_COLUMNS:
-                logger.error("column '%s' identifies the row and cannot be "
-                        "provisioned; the domain and its type are passed as "
-                        "arguments", col)
+            if col == TLS_ID_COL:
+                logger.error("column '%s' is generated by the database",
+                        TLS_ID_COL)
                 return None, None, None
 
-            if col not in TLS_MGM_COLUMNS:
+            if col not in TLS_MGM_COLUMNS and col not in TLS_KEY_COLUMNS:
                 logger.error("unknown %s column '%s'", TLS_MGM_TABLE, col)
                 return None, None, None
 
@@ -486,12 +457,48 @@ def tls_db_params(self, params, require_type=False):
 
             cols[col] = val
 
-        domain, dtype = self.tls_db_domain(
-                [p for p in params if '=' not in p], require_type)
+        # the domain and its type identify the row, they are not provisioned
+        args = [p for p in params if '=' not in p]
+        if len(args) > len(TLS_KEY_COLUMNS):
+            logger.error("too many arguments: expected at most a domain and "
+                    "its type")
+            return None, None, None
+
+        key = {}
+        for i, col in enumerate(TLS_KEY_COLUMNS):
+            if i < len(args):
+                if col in cols:
+                    logger.error("'%s' given both as an argument and as "
+                            "'%s='", col, col)
+                    return None, None, None
+                key[col] = args[i]
+            else:
+                key[col] = cols.pop(col, None)
+
+        domain = key[TLS_DOMAIN_COL]
         if not domain:
+            domain = cfg.read_param(None,
+                    "Please provide the name of the TLS domain")
+            if not domain:
+                logger.error("no TLS domain specified!")
+                return None, None, None
+
+        dtype = key[TLS_TYPE_COL]
+        if not dtype:
+            # commands changing an existing row get no default, so that they
+            # cannot pick a different row than the intended one
+            dtype = cfg.read_param(None, "TLS domain type (server/client)",
+                    None if require_type else "server")
+            if not dtype:
+                logger.error("no TLS domain type specified!")
+                return None, None, None
+
+        if dtype.lower() not in TLS_DOMAIN_TYPES:
+            logger.error("invalid TLS domain type '%s': "
+                    "expected 'server' or 'client'", dtype)
             return None, None, None
 
-        return domain, dtype, cols
+        return domain, TLS_DOMAIN_TYPES[dtype.lower()], cols
 
     def do_db_add(self, params=None, modifiers=None):
         """
@@ -590,7 +597,10 @@ def do_db_show(self, params=None, modifiers=None):
         """
         prints the columns of a TLS domain
         """
-        domain, dtype = self.tls_db_domain(params or [])
+        domain, dtype, cols = self.tls_db_params(params or [])
+        if domain and cols:
+            logger.error("db_show takes no column: '%s'", list(cols)[0])
+            return -1
         if not domain:
             return -1
 
@@ -635,7 +645,10 @@ def do_db_delete(self, params=None, modifiers=None):
         """
         removes a TLS domain from the database
         """
-        domain, dtype = self.tls_db_domain(params or [], True)
+        domain, dtype, cols = self.tls_db_params(params or [], True)
+        if domain and cols:
+            logger.error("db_delete takes no column: '%s'", list(cols)[0])
+            return -1
         if not domain:
             return -1