Refactor customization opt out flag (#569)

* Refactor customizable flag in catalog

* Adjust guides

* temporarily skip api bc check

* remove comment

* remove one unnecessary function

* Make flag naming declarative

* improve functions pipeability

* more destructuring

* format specification

* fix typo

* Add missing assert

* revert skipping api bc check

guides/check_definition.schema.json

@@ -37,7 +37,7 @@
         "$ref": "#/definitions/Fact"
       }
     },
-    "customizable": {
+    "customization_disabled": {
       "type": "boolean"
     },
     "values": {
@@ -91,7 +91,7 @@
         "name": {
           "type": "string"
         },
-        "customizable": {
+        "customization_disabled": {
           "type": "boolean"
         },
         "default": {

guides/specification.md

@@ -87,11 +87,11 @@ facts:
     gatherer: corosync.conf
     argument: totem.token
 
-customizable: true
+customization_disabled: true
 
 values:
   - name: expected_token_timeout
-    customizable: true
+    customization_disabled: true
     default: 5000
     conditions:
       - value: 30000
@@ -112,19 +112,19 @@ expectations:
 
 Following are listed the top level properties of a Check definition yaml.
 
-| Key            | Required/Not Required | Details                   |
-| -------------- | --------------------- | ------------------------- |
-| `id`           | required              | [see more](#id)           |
-| `name`         | required              | [see more](#name)         |
-| `group`        | required              | [see more](#group)        |
-| `description`  | required              | [see more](#description)  |
-| `remediation`  | required              | [see more](#remediation)  |
-| `severity`     | not required          | [see more](#severity)     |
-| `metadata`     | not required          | [see more](#metadata)     |
-| `facts`        | required              | [see more](#facts)        |
-| `customizable` | not required          | [see more](#customizable) |
-| `values`       | not required          | [see more](#values)       |
-| `expectations` | required              | [see more](#expectations) |
+| Key                      | Required/Not Required | Details                            |
+| ------------------------ | --------------------- | ---------------------------------- |
+| `id`                     | required              | [see more](#id)                    |
+| `name`                   | required              | [see more](#name)                  |
+| `group`                  | required              | [see more](#group)                 |
+| `description`            | required              | [see more](#description)           |
+| `remediation`            | required              | [see more](#remediation)           |
+| `severity`               | not required          | [see more](#severity)              |
+| `metadata`               | not required          | [see more](#metadata)              |
+| `facts`                  | required              | [see more](#facts)                 |
+| `customization_disabled` | not required          | [see more](#disable-customization) |
+| `values`                 | not required          | [see more](#values)                |
+| `expectations`           | required              | [see more](#expectations)          |
 
 ---
 
@@ -352,21 +352,24 @@ facts:
 
 Finally, gathered facts, are used in Check's [Expectations](#expectations) to determine whether expected conditions are met for the best practice to be adhered.
 
-## Customizable
+## Disable Customization
 
-Users can modify a check's [expected values](#values) to adapt to specific system and environmental configurations.
+Users can modify a check's [expected values](#values) to accommodate specific system and environmental configurations.
 
-Built-in checks are considered **customizable** by **default**, so the `customizable` flag disables customization for a particular check.
+By default, built-in checks are **customizable**. The `customization_disabled` flag provides a way to **disable** customizability when needed.  
 
-The customizability flag can be set globally for a check and|or for [specific values](#customizable-values).
-When customization is globally disabled for a check, marked with `customizable: false`, it overrides any value-specific customizability settings. If global customization is not disabled, the customizability of individual values is then considered.
-
-To explicitly mark a check as non-customizable, set the `customizable`  key to `false`:
+To disable customization for a check the following bit of specification is required:
 
 ```yaml
-customizable: false
+customization_disabled: true
 ```
 
+Opting out from customizability at the root of a check's specification makes all the values of the given check not customizable.
+
+### Notes:  
+- Setting `customization_disabled: false` has no real effect as by default a check is customizable
+- The `customization_disabled` flag can be also applied to [specific values](#customizable-values)
+
 ## Values
 
 Values are named variables that may evaluate differently based on the execution context and are used with Facts for _Contextual_ [Expectations](#expectations) Evaluation.
@@ -482,18 +485,16 @@ All the _resolved_ declared values would be registered in the [`values`](#values
 
 ### Customizable Values
 
-In addition to the global-level [customizability](#customizable), individual check values are also **customizable** by **default**. To provide finer control, a `boolean` customizable entry can be defined on a per-value basis.
+A check's [expected values](#values) are customizable by default, and to provide finer control to the global-level [customizability opt-out](#disable-customization) it is possible to opt-out customizability on a per-value basis.
 
 ```yaml
 values:
   - name: non_customizable_check_value
-    customizable: false
+    customization_disabled: true
     default: 5000
 ```
 
-Setting **customizable**: `false` for a specific value prevents the modification of the default value.
-
-Note that if global customizability is set to false, it takes precedence over any value-level customizable settings, disabling customizability for all associated values.
+Setting **customization_disabled**: `false` for a specific value prevents the modification of the default value.
 
 ## Expectations
 
@@ -760,15 +761,15 @@ Scopes are namespaced and access to items in the scope is name based.
 
 Examples of entries in the scope. What is actually available during the execution depends on the scenario. Find the updated values in the reference column link.
 
-| name | Type | Reference | Applicable
-| ---- | -----| --------- | -----------
-| `env.target_type` | one of `cluster`, `host` | No enum available | All
-| `env.provider` | one of `azure`, `aws`, `gcp`,`kvm`,`nutanix`, `vmware`, `unknown` | [Providers](https://github.com/trento-project/web/blob/main/lib/trento/enums/provider.ex) | All
-| `env.cluster_type` | one of `hana_scale_up`, `hana_scale_out`, `ascs_ers`, `unknown` | [Cluster types](https://github.com/trento-project/web/blob/main/lib/trento/clusters/enums/cluster_type.ex) | `target_type` is `cluster`
-| `env.hana_scenario` | one of `performance_optimized`, `cost_optimized`, `unknown` | [Hana Scale Up Scenario](https://github.com/trento-project/web/blob/main/lib/trento/clusters/enums/hana_scenario.ex) | `cluster_type` is `hana_scale_up`
-| `env.architecture_type` | one of `classic`, `angi` | [Architecture types](https://github.com/trento-project/web/blob/main/lib/trento/clusters/enums/hana_architecture_type.ex) | `cluster_type` is one of `hana_scale_up`, `hana_scale_out`
-| `env.ensa_version` | one of `ensa1`, `ensa2`, `mixed_versions` | [ENSA version](https://github.com/trento-project/web/blob/main/lib/trento/clusters/enums/cluster_ensa_version.ex) | `cluster_type` is `ascs_ers`
-| `env.filesystem_type` | one of `resource_managed`, `simple_mount`, `mixed_fs_types` | [Filesystem type](https://github.com/trento-project/web/blob/main/lib/trento/clusters/enums/filesystem_type.ex) | `cluster_type` is `ascs_ers`
+| name                    | Type                                                              | Reference                                                                                                                 | Applicable                                                 |
+| ----------------------- | ----------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------- |
+| `env.target_type`       | one of `cluster`, `host`                                          | No enum available                                                                                                         | All                                                        |
+| `env.provider`          | one of `azure`, `aws`, `gcp`,`kvm`,`nutanix`, `vmware`, `unknown` | [Providers](https://github.com/trento-project/web/blob/main/lib/trento/enums/provider.ex)                                 | All                                                        |
+| `env.cluster_type`      | one of `hana_scale_up`, `hana_scale_out`, `ascs_ers`, `unknown`   | [Cluster types](https://github.com/trento-project/web/blob/main/lib/trento/clusters/enums/cluster_type.ex)                | `target_type` is `cluster`                                 |
+| `env.hana_scenario`     | one of `performance_optimized`, `cost_optimized`, `unknown`       | [Hana Scale Up Scenario](https://github.com/trento-project/web/blob/main/lib/trento/clusters/enums/hana_scenario.ex)      | `cluster_type` is `hana_scale_up`                          |
+| `env.architecture_type` | one of `classic`, `angi`                                          | [Architecture types](https://github.com/trento-project/web/blob/main/lib/trento/clusters/enums/hana_architecture_type.ex) | `cluster_type` is one of `hana_scale_up`, `hana_scale_out` |
+| `env.ensa_version`      | one of `ensa1`, `ensa2`, `mixed_versions`                         | [ENSA version](https://github.com/trento-project/web/blob/main/lib/trento/clusters/enums/cluster_ensa_version.ex)         | `cluster_type` is `ascs_ers`                               |
+| `env.filesystem_type`   | one of `resource_managed`, `simple_mount`, `mixed_fs_types`       | [Filesystem type](https://github.com/trento-project/web/blob/main/lib/trento/clusters/enums/filesystem_type.ex)           | `cluster_type` is `ascs_ers`                               |
 
 #### **facts**
 
@@ -813,10 +814,10 @@ values:
 ```
 
 Available entries in scope
-| name | Resolved to  
-| ------------------------------- | -------------------------------------------------------
-| `values.expected_token_timeout` | `5000`, `30000`, `20000` based on the conditions
-| `values.another_variable_value` | `blue`, `red` based on the conditions
+| name                            | Resolved to                                      |
+| ------------------------------- | ------------------------------------------------ |
+| `values.expected_token_timeout` | `5000`, `30000`, `20000` based on the conditions |
+| `values.another_variable_value` | `blue`, `red` based on the conditions            |
 
 ## Best practices and conventions
 

lib/wanda/catalog.ex

@@ -79,21 +79,34 @@ defmodule Wanda.Catalog do
     |> Enum.map(&map_to_selectable_check(&1, available_customizations))
   end
 
-  defp map_to_selectable_check(%Check{} = check, available_customizations) do
+  defp map_to_selectable_check(
+         %Check{
+           id: id,
+           name: name,
+           group: group,
+           description: description,
+           values: values,
+           customization_disabled: customization_globally_disabled?
+         },
+         available_customizations
+       ) do
     mapped_values =
-      check.id
+      id
       |> find_custom_values(available_customizations)
-      |> map_values(check.values)
+      |> map_selectable_check_values(values, customization_globally_disabled?)
+
+    customizable_check? =
+      detect_check_customizability(mapped_values, not customization_globally_disabled?)
 
     %SelectableCheck{
-      id: check.id,
-      name: check.name,
-      group: check.group,
-      description: check.description,
+      id: id,
+      name: name,
+      group: group,
+      description: description,
       values: mapped_values,
-      customizable: check.customizable,
+      customizable: customizable_check?,
       customized:
-        check.customizable &&
+        customizable_check? &&
           Enum.any?(mapped_values, &(&1.customizable && Map.has_key?(&1, :custom_value)))
     }
   end
@@ -108,16 +121,15 @@ defmodule Wanda.Catalog do
     end)
   end
 
-  defp map_values(custom_values, check_values) do
+  defp map_selectable_check_values(custom_values, check_values, customization_globally_disabled?) do
     Enum.map(check_values, fn %Value{
                                 name: value_name,
-                                customizable: customizable,
                                 default: default_value
-                              } ->
+                              } = value ->
       %{
-        name: value_name,
-        customizable: customizable
+        name: value_name
       }
+      |> add_value_customizability(value, customization_globally_disabled?)
       |> maybe_add_current_value(default_value)
       |> maybe_add_customization(custom_values)
     end)
@@ -225,8 +237,7 @@ defmodule Wanda.Catalog do
        facts: Enum.map(facts, &map_fact/1),
        values: mapped_values,
        expectations: Enum.map(expectations, &map_expectation/1),
-       customizable:
-         detect_check_customizability(mapped_values, Map.get(check, "customizable", true))
+       customization_disabled: Map.get(check, "customization_disabled", false)
      }}
   end
 
@@ -299,22 +310,33 @@ defmodule Wanda.Catalog do
       name: name,
       default: default,
       conditions: conditions,
-      customizable: detect_value_customizability(value)
+      customization_disabled: Map.get(value, "customization_disabled", false)
     }
   end
 
-  defp detect_value_customizability(%{"customizable" => false}), do: false
+  defp add_value_customizability(
+         mapped_value,
+         current_value,
+         customization_globally_disabled?
+       ) do
+    Map.put(
+      mapped_value,
+      :customizable,
+      can_customize_value?(current_value, customization_globally_disabled?)
+    )
+  end
+
+  defp can_customize_value?(_current_value, true = _customization_globally_disabled?), do: false
+  defp can_customize_value?(%{customization_disabled: true}, _), do: false
 
-  defp detect_value_customizability(%{"default" => default_value}),
+  defp can_customize_value?(%{default: default_value}, _),
     do: not (is_list(default_value) or is_map(default_value))
 
-  defp detect_value_customizability(_), do: true
-
   defp detect_check_customizability([] = _values, _root_customizability), do: false
 
   defp detect_check_customizability(values, root_customizability) do
     has_at_least_one_customizable_value? =
-      Enum.any?(values, fn %Value{customizable: customizable} -> customizable end)
+      Enum.any?(values, fn %{customizable: customizable} -> customizable end)
 
     root_customizability and has_at_least_one_customizable_value?
   end

lib/wanda/catalog/check.ex

@@ -20,14 +20,14 @@ defmodule Wanda.Catalog.Check do
     :values,
     :expectations,
     :when,
-    :customizable
+    :customization_disabled
   ]
 
   @type t :: %__MODULE__{
           id: String.t(),
           name: String.t(),
           group: String.t(),
-          customizable: boolean(),
+          customization_disabled: boolean(),
           description: String.t(),
           remediation: String.t(),
           metadata: map(),

lib/wanda/catalog/value.ex

@@ -6,12 +6,12 @@ defmodule Wanda.Catalog.Value do
   alias Wanda.Catalog.Condition
 
   @derive Jason.Encoder
-  defstruct [:name, :default, :conditions, :customizable]
+  defstruct [:name, :default, :conditions, :customization_disabled]
 
   @type t :: %__MODULE__{
           name: String.t(),
           default: boolean() | number() | String.t(),
           conditions: [Condition.t()],
-          customizable: boolean()
+          customization_disabled: boolean()
         }
 end

lib/wanda/checks_customizations.ex

@@ -48,9 +48,9 @@ defmodule Wanda.ChecksCustomizations do
     end
   end
 
-  defp determine_customizability(%Check{customizable: true}), do: {:ok, :customizable}
+  defp determine_customizability(%Check{customization_disabled: false}), do: {:ok, :customizable}
 
-  defp determine_customizability(%Check{customizable: false}),
+  defp determine_customizability(%Check{customization_disabled: true}),
     do: {:error, :check_not_customizable}
 
   defp validate_incoming_custom_values([], _), do: {:error, :invalid_custom_values}
@@ -62,8 +62,8 @@ defmodule Wanda.ChecksCustomizations do
     can_customize? =
       Enum.all?(custom_values, fn %{name: customized_name, value: customized_value} ->
         case validate_value_exists_in_check(customized_name, values) do
-          {:ok, %Value{customizable: customizable, default: default_value}} ->
-            customizable and match_value_type(default_value, customized_value)
+          {:ok, %Value{customization_disabled: customization_disabled, default: default_value}} ->
+            not customization_disabled and match_value_type(default_value, customized_value)
 
           {:error, :value_not_found} ->
             false

lib/wanda_web/controllers/v1/catalog_json.ex

@@ -56,7 +56,7 @@ defmodule WandaWeb.V1.CatalogJSON do
   end
 
   def adapt_values_customizability(values) do
-    Enum.map(values, &(&1 |> Map.from_struct() |> Map.drop([:customizable])))
+    Enum.map(values, &(&1 |> Map.from_struct() |> Map.drop([:customization_disabled])))
   end
 
   defp selectable_check(%SelectableCheck{

lib/wanda_web/controllers/v3/catalog_json.ex

@@ -17,7 +17,7 @@ defmodule WandaWeb.V3.CatalogJSON do
         values: values,
         expectations: expectations,
         when: when_expression,
-        customizable: customizable
+        customization_disabled: customization_disabled
       }) do
     %{
       id: id,
@@ -32,7 +32,7 @@ defmodule WandaWeb.V3.CatalogJSON do
       expectations: expectations,
       when: when_expression,
       premium: false,
-      customizable: customizable
+      customization_disabled: customization_disabled
     }
   end
 end

lib/wanda_web/schemas/v3/catalog/check.ex

@@ -75,12 +75,12 @@ defmodule WandaWeb.Schemas.V3.Catalog.Check do
                   required: [:value, :expression]
                 }
               },
-              customizable: %Schema{
+              customization_disabled: %Schema{
                 type: :boolean,
                 description: "Whether the value is customizable or not"
               }
             },
-            required: [:name, :default, :conditions, :customizable]
+            required: [:name, :default, :conditions, :customization_disabled]
           }
         },
         expectations: %Schema{
@@ -124,7 +124,7 @@ defmodule WandaWeb.Schemas.V3.Catalog.Check do
           description: "Check is Premium or not",
           deprecated: true
         },
-        customizable: %Schema{
+        customization_disabled: %Schema{
           type: :boolean,
           description: "Whether the check is customizable or not"
         }
@@ -142,7 +142,7 @@ defmodule WandaWeb.Schemas.V3.Catalog.Check do
         :expectations,
         :when,
         :premium,
-        :customizable
+        :customization_disabled
       ]
     },
     struct?: false