mgr/cephadm/nvmeof: Rewrite NVMEoF fields validation.

Fixes https://tracker.ceph.com/issues/69176

Signed-off-by: Gil Bregman <gbregman@il.ibm.com>

diff --git a/src/python-common/ceph/deployment/service_spec.py b/src/python-common/ceph/deployment/service_spec.py
index 316f9567b208b306d33b2e46c95ac006f41d7d88..e779356ae0cc08cf61de083d73d72b2272eb57ca 100644 (file)
--- a/src/python-common/ceph/deployment/service_spec.py
+++ b/src/python-common/ceph/deployment/service_spec.py
@@ -25,7 +25,9 @@ from typing import (
 import yaml
 
 from ceph.deployment.hostspec import HostSpec, SpecValidationError, assert_valid_host
-from ceph.deployment.utils import unwrap_ipv6, valid_addr
+from ceph.deployment.utils import unwrap_ipv6, valid_addr, verify_non_negative_int
+from ceph.deployment.utils import verify_positive_int, verify_non_negative_number
+from ceph.deployment.utils import verify_boolean, verify_enum
 from ceph.utils import is_hex
 
 ServiceSpecT = TypeVar('ServiceSpecT', bound='ServiceSpec')
@@ -1545,6 +1547,7 @@ class NvmeofServiceSpec(ServiceSpec):
         if not self.pool:
             raise SpecValidationError('Cannot add NVMEOF: No Pool specified')
 
+        verify_boolean(self.enable_auth, "Enable authentication")
         if self.enable_auth:
             if not all([self.server_key, self.server_cert, self.client_key,
                         self.client_cert, self.root_ca_cert]):
@@ -1559,142 +1562,64 @@ class NvmeofServiceSpec(ServiceSpec):
         if self.transports not in ['tcp']:
             raise SpecValidationError('Invalid transport. Valid values are tcp')
 
-        if self.log_level:
-            if self.log_level.lower() not in ['debug',
-                                              'info',
-                                              'warning',
-                                              'error',
-                                              'critical']:
-                raise SpecValidationError(
-                    'Invalid log level. Valid values are: debug, info, warning, error, critial')
-
-        if self.spdk_log_level:
-            if self.spdk_log_level.lower() not in ['debug',
-                                                   'info',
-                                                   'warning',
-                                                   'error',
-                                                   'notice']:
-                raise SpecValidationError(
-                    'Invalid SPDK log level. Valid values are: '
-                    'DEBUG, INFO, WARNING, ERROR, NOTICE')
-
-        if self.spdk_protocol_log_level:
-            if self.spdk_protocol_log_level.lower() not in ['debug',
-                                                            'info',
-                                                            'warning',
-                                                            'error',
-                                                            'notice']:
-                raise SpecValidationError(
-                    'Invalid SPDK protocol log level. Valid values are: '
-                    'DEBUG, INFO, WARNING, ERROR, NOTICE')
+        verify_enum(self.log_level, "log level", ['debug', 'info', 'warning', 'error', 'critical'])
+        verify_enum(self.spdk_log_level, "SPDK log level",
+                    ['DEBUG', 'INFO', 'WARNING', 'ERROR', 'NOTICE'])
+        verify_enum(self.spdk_protocol_log_level, "SPDK protocol log level",
+                    ['DEBUG', 'INFO', 'WARNING', 'ERROR', 'NOTICE'])
+        verify_positive_int(self.bdevs_per_cluster, "Bdevs per cluster")
+        if self.bdevs_per_cluster is not None and self.bdevs_per_cluster < 1:
+            raise SpecValidationError("Bdevs per cluster should be at least 1")
 
+        verify_non_negative_number(self.spdk_ping_interval_in_seconds, "SPDK ping interval")
         if (
-            self.spdk_ping_interval_in_seconds
+            self.spdk_ping_interval_in_seconds is not None
             and self.spdk_ping_interval_in_seconds < 1.0
         ):
             raise SpecValidationError("SPDK ping interval should be at least 1 second")
 
+        verify_non_negative_int(self.allowed_consecutive_spdk_ping_failures,
+                                "Allowed consecutive SPDK ping failures")
         if (
-            self.allowed_consecutive_spdk_ping_failures
+            self.allowed_consecutive_spdk_ping_failures is not None
             and self.allowed_consecutive_spdk_ping_failures < 1
         ):
             raise SpecValidationError("Allowed consecutive SPDK ping failures should be at least 1")
 
-        if (
-            self.state_update_interval_sec
-            and self.state_update_interval_sec < 0
-        ):
-            raise SpecValidationError("State update interval can't be negative")
-
-        if (
-            self.omap_file_lock_duration
-            and self.omap_file_lock_duration < 0
-        ):
-            raise SpecValidationError("OMAP file lock duration can't be negative")
-
-        if (
-            self.omap_file_lock_retries
-            and self.omap_file_lock_retries < 0
-        ):
-            raise SpecValidationError("OMAP file lock retries can't be negative")
-
-        if (
-            self.omap_file_update_reloads
-            and self.omap_file_update_reloads < 0
-        ):
-            raise SpecValidationError("OMAP file reloads can't be negative")
-
-        if (
-            self.spdk_timeout
-            and self.spdk_timeout < 0.0
-        ):
-            raise SpecValidationError("SPDK timeout can't be negative")
-
-        if (
-            self.conn_retries
-            and self.conn_retries < 0
-        ):
-            raise SpecValidationError("Connection retries can't be negative")
-
-        if (
-            self.max_log_file_size_in_mb
-            and self.max_log_file_size_in_mb < 0
-        ):
-            raise SpecValidationError("Log file size can't be negative")
-
-        if (
-            self.max_log_files_count
-            and self.max_log_files_count < 0
-        ):
-            raise SpecValidationError("Log files count can't be negative")
-
-        if (
-            self.max_log_directory_backups
-            and self.max_log_directory_backups < 0
-        ):
-            raise SpecValidationError("Log file directory backups can't be negative")
-
-        if (self.max_hosts_per_namespace and self.max_hosts_per_namespace < 0):
-            raise SpecValidationError("Max hosts per namespace can't be negative")
-
-        if (self.max_namespaces_with_netmask and self.max_namespaces_with_netmask < 0):
-            raise SpecValidationError("Max namespaces with netmask can't be negative")
-
-        if not isinstance(self.max_subsystems, int):
-            raise SpecValidationError("Max subsystems must be an integer")
-
-        if self.max_subsystems <= 0:
-            raise SpecValidationError("Max subsystems must be greater than zero")
-
-        if not isinstance(self.max_namespaces, int):
-            raise SpecValidationError("Max namespaces must be an integer")
-
-        if self.max_namespaces <= 0:
-            raise SpecValidationError("Max namespaces must be greater than zero")
-
-        if not isinstance(self.max_namespaces_per_subsystem, int):
-            raise SpecValidationError("Max namespaces per subsystem must be an integer")
-
-        if self.max_namespaces_per_subsystem <= 0:
-            raise SpecValidationError("Max namespaces per subsystem must be greater than zero")
-
-        if not isinstance(self.max_hosts_per_subsystem, int):
-            raise SpecValidationError("Max hosts per subsystem must be an integer")
-
-        if self.max_hosts_per_subsystem <= 0:
-            raise SpecValidationError("Max hosts per subsystem must be greater than zero")
-
-        if (
-            self.monitor_timeout
-            and self.monitor_timeout < 0.0
-        ):
-            raise SpecValidationError("Monitor timeout can't be negative")
-
-        if self.port and self.port < 0:
-            raise SpecValidationError("Port can't be negative")
-
-        if self.discovery_port and self.discovery_port < 0:
-            raise SpecValidationError("Discovery port can't be negative")
+        verify_non_negative_int(self.state_update_interval_sec, "State update interval")
+        verify_non_negative_int(self.rebalance_period_sec, "Rebalance period")
+        verify_non_negative_int(self.max_gws_in_grp, "Max gateways in group")
+        verify_non_negative_int(self.max_ns_to_change_lb_grp,
+                                "Max namespaces to change load balancing group")
+        verify_non_negative_int(self.omap_file_lock_duration, "OMAP file lock duration")
+        verify_non_negative_number(self.omap_file_lock_retry_sleep_interval,
+                                   "OMAP file lock sleep interval")
+        verify_non_negative_int(self.omap_file_lock_retries, "OMAP file lock retries")
+        verify_non_negative_int(self.omap_file_update_reloads, "OMAP file reloads")
+        verify_non_negative_number(self.spdk_timeout, "SPDK timeout")
+        verify_non_negative_int(self.max_log_file_size_in_mb, "Log file size")
+        verify_non_negative_int(self.max_log_files_count, "Log files count")
+        verify_non_negative_int(self.max_log_directory_backups, "Log file directory backups")
+        verify_non_negative_int(self.max_hosts_per_namespace, "Max hosts per namespace")
+        verify_non_negative_int(self.max_namespaces_with_netmask, "Max namespaces with netmask")
+        verify_positive_int(self.max_subsystems, "Max subsystems")
+        verify_positive_int(self.max_namespaces, "Max namespaces")
+        verify_positive_int(self.max_namespaces_per_subsystem, "Max namespaces per subsystem")
+        verify_positive_int(self.max_hosts_per_subsystem, "Max hosts per subsystem")
+        verify_non_negative_number(self.monitor_timeout, "Monitor timeout")
+        verify_non_negative_int(self.port, "Port")
+        verify_non_negative_int(self.discovery_port, "Discovery port")
+        verify_non_negative_int(self.prometheus_port, "Prometheus port")
+        verify_non_negative_int(self.prometheus_stats_interval, "Prometheus stats interval")
+        verify_boolean(self.state_update_notify, "State update notify")
+        verify_boolean(self.enable_spdk_discovery_controller, "Enable SPDK discovery controller")
+        verify_boolean(self.enable_key_encryption, "Enable key encryption")
+        verify_boolean(self.enable_prometheus_exporter, "Enable Prometheus exporter")
+        verify_boolean(self.verify_nqns, "Verify NQNs")
+        verify_boolean(self.log_files_enabled, "Log files enabled")
+        verify_boolean(self.log_files_rotation_enabled, "Log files rotation enabled")
+        verify_boolean(self.verbose_log_messages, "Verbose log messages")
+        verify_boolean(self.enable_monitor_client, "Enable monitor client")
 
 
 yaml.add_representer(NvmeofServiceSpec, ServiceSpec.yaml_representer)

diff --git a/src/python-common/ceph/deployment/utils.py b/src/python-common/ceph/deployment/utils.py
index f800e3738970c87cd7f15a40e7897607e0b2803b..758eddc9412409c27309d92b6347f1035e51c1a6 100644 (file)
--- a/src/python-common/ceph/deployment/utils.py
+++ b/src/python-common/ceph/deployment/utils.py
@@ -1,7 +1,9 @@
 import ipaddress
 import socket
-from typing import Tuple, Optional
+from typing import Tuple, Optional, Any
 from urllib.parse import urlparse
+from ceph.deployment.hostspec import SpecValidationError
+from numbers import Number
 
 
 def unwrap_ipv6(address):
@@ -100,3 +102,50 @@ def valid_addr(addr: str) -> Tuple[bool, str]:
     if addr[0].isalpha() and '.' in addr:
         return _dns_lookup(addr, port)
     return _ip_lookup(addr, port)
+
+
+def verify_numeric(field: Any, field_name: str) -> None:
+    if field is not None:
+        if not isinstance(field, Number) or isinstance(field, bool):
+            raise SpecValidationError(f"{field_name} must be a number")
+
+
+def verify_non_negative_int(field: Any, field_name: str) -> None:
+    verify_numeric(field, field_name)
+    if field is not None:
+        if not isinstance(field, int) or isinstance(field, bool):
+            raise SpecValidationError(f"{field_name} must be an integer")
+        if field < 0:
+            raise SpecValidationError(f"{field_name} can't be negative")
+
+
+def verify_positive_int(field: Any, field_name: str) -> None:
+    verify_non_negative_int(field, field_name)
+    if field is not None and field <= 0:
+        raise SpecValidationError(f"{field_name} must be greater than zero")
+
+
+def verify_non_negative_number(field: Any, field_name: str) -> None:
+    verify_numeric(field, field_name)
+    if field is not None:
+        if field < 0.0:
+            raise SpecValidationError(f"{field_name} can't be negative")
+
+
+def verify_boolean(field: Any, field_name: str) -> None:
+    if field is not None:
+        if not isinstance(field, bool):
+            raise SpecValidationError(f"{field_name} must be a boolean")
+
+
+def verify_enum(field: Any, field_name: str, allowed: list) -> None:
+    if field:
+        allowed_lower = []
+        if not isinstance(field, str):
+            raise SpecValidationError(f"{field_name} must be a string")
+        for val in allowed:
+            assert isinstance(val, str)
+            allowed_lower.append(val.lower())
+        if field.lower() not in allowed_lower:
+            raise SpecValidationError(
+                           f'Invalid {field_name}. Valid values are: {", ".join(allowed)}')