Merge pull request ceph#56928 from adk3798/grafana-anon-access-dropped

python-common: handle "anonymous_access: false" in to_json of Grafana spec

Reviewed-by: Nizamudeen A <[email protected]>

Author: adk3798

src/python-common/ceph/deployment/service_spec.py

@@ -1494,10 +1494,16 @@ def validate(self) -> None:
                 raise SpecValidationError(
                     'Invalid SPDK log level. Valid values are: DEBUG, INFO, WARNING, ERROR, NOTICE')
 
-        if self.spdk_ping_interval_in_seconds < 1.0:
+        if (
+            self.spdk_ping_interval_in_seconds
+            and self.spdk_ping_interval_in_seconds < 1.0
+        ):
             raise SpecValidationError("SPDK ping interval should be at least 1 second")
 
-        if self.allowed_consecutive_spdk_ping_failures < 1:
+        if (
+            self.allowed_consecutive_spdk_ping_failures
+            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 < 0:
@@ -2032,7 +2038,7 @@ def __init__(self,
                  port: Optional[int] = None,
                  protocol: Optional[str] = 'https',
                  initial_admin_password: Optional[str] = None,
-                 anonymous_access: Optional[bool] = True,
+                 anonymous_access: bool = True,
                  extra_container_args: Optional[GeneralArgList] = None,
                  extra_entrypoint_args: Optional[GeneralArgList] = None,
                  custom_configs: Optional[List[CustomConfig]] = None,
@@ -2067,6 +2073,24 @@ def validate(self) -> None:
                        'be inaccessible.')
             raise SpecValidationError(err_msg)
 
+    def to_json(self) -> "OrderedDict[str, Any]":
+        json_dict = super(GrafanaSpec, self).to_json()
+        if not self.anonymous_access:
+            # This field was added as a boolean that defaults
+            # to True, which makes it get dropped when the user
+            # sets it to False and it is converted to json. This means
+            # the in memory version of the spec will have the option set
+            # correctly, but the persistent version we store in the config-key
+            # store will always drop this option. It's already been backported to
+            # some release versions, or we'd probably just rename it to
+            # no_anonymous_access and default it to False. This block is to
+            # handle this option specially and in the future, we should avoid
+            # boolean fields that default to True.
+            if 'spec' not in json_dict:
+                json_dict['spec'] = {}
+            json_dict['spec']['anonymous_access'] = False
+        return json_dict
+
 
 yaml.add_representer(GrafanaSpec, ServiceSpec.yaml_representer)
 

src/python-common/ceph/tests/test_service_spec.py

@@ -488,6 +488,15 @@ def test_osd_unmanaged():
   privacy_protocol: AES
   snmp_destination: 192.168.1.42:162
   snmp_version: V3
+---
+service_type: grafana
+service_name: grafana
+placement:
+  count: 1
+spec:
+  anonymous_access: false
+  initial_admin_password: password
+  protocol: https
 """.split('---\n'))
 def test_yaml(y):
     data = yaml.safe_load(y)