From: Gil Bregman Date: Wed, 11 Dec 2024 21:48:21 +0000 (+0200) Subject: mgr/cephadm/nvmeof: Rewrite NVMEoF fields validation. X-Git-Tag: v20.0.0~548^2 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=31283c0b2ab2d12de0b002131d5921568f9f0a6e;p=ceph.git mgr/cephadm/nvmeof: Rewrite NVMEoF fields validation. Fixes https://tracker.ceph.com/issues/69176 Signed-off-by: Gil Bregman --- diff --git a/src/python-common/ceph/deployment/service_spec.py b/src/python-common/ceph/deployment/service_spec.py index 316f9567b208b..e779356ae0cc0 100644 --- 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 f800e3738970c..758eddc941240 100644 --- 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)}')