mk-oracle: Restructure oracle_id and improve ruleset help texts

Move oracle_id from connection options to the instances section.
Add SID and TNS alias fields to oracle_id.
Update ruleset titles and help texts to clarify the relationship between default
settings (applied to all databases) and per-database overrides in the
instance list.

Change-Id: I4557d3883b5e6fe0e023ba289ad2e9c9dc36c093

Author: AnvarU

cmk/plugins/oracle/bakery/mk_oracle_unified.py

@@ -92,8 +92,10 @@ class Config:
 
 
 class GuiOracleIdentificationConf(BaseModel):
-    service_name: str
+    service_name: str | None = None
     instance_name: str | None = None
+    sid: str | None = None
+    alias: str | None = None
 
 
 class GuiConnectionConf(BaseModel):
@@ -102,7 +104,6 @@ class GuiConnectionConf(BaseModel):
     timeout: int | None = None
     tns_admin: str | None = None
     oracle_local_registry: str | None = None
-    oracle_id: GuiOracleIdentificationConf | None = None
 
 
 class GuiDiscoveryConf(BaseModel):
@@ -144,8 +145,7 @@ class GuiInstanceAdditionalOptionsConf(BaseModel):
 
 
 class GuiInstanceConf(BaseModel):
-    service_name: str | None
-    instance_name: str | None
+    oracle_id: GuiOracleIdentificationConf
     auth: GuiAuthConf | None = None
     connection: GuiConnectionConf | None = None
 
@@ -189,8 +189,6 @@ class OracleConnection(BaseModel):
     timeout: int | None = None
     tns_admin: str | None = None
     oracle_local_registry: str | None = None
-    service_name: str | None = None
-    instance: str | None = None
 
 
 class OracleInstanceAdditionalOptions(BaseModel):
@@ -201,6 +199,8 @@ class OracleInstanceAdditionalOptions(BaseModel):
 class OracleInstance(BaseModel):
     service_name: str | None = None
     instance_name: str | None = None
+    sid: str | None = None
+    alias: str | None = None
     authentication: OracleAuth | None = None
     connection: OracleConnection | None = None
 
@@ -288,20 +288,12 @@ def _get_oracle_connection(conn: GuiConnectionConf | None) -> OracleConnection |
     if conn is None:
         return None
 
-    service_name = None
-    instance = None
-    if oracle_id := conn.oracle_id:
-        service_name = oracle_id.service_name
-        instance = oracle_id.instance_name
-
     return OracleConnection(
         hostname=conn.host,
         port=conn.port,
         timeout=conn.timeout,
         tns_admin=conn.tns_admin,
         oracle_local_registry=conn.oracle_local_registry,
-        service_name=service_name,
-        instance=instance,
     )
 
 
@@ -374,16 +366,21 @@ def _get_oracle_instances(instances: list[GuiInstanceConf] | None) -> list[Oracl
 
     result: list[OracleInstance] = []
     for instance in instances:
+        oracle_id = instance.oracle_id
         if (
-            not instance.service_name
-            and not instance.instance_name
+            oracle_id.service_name is None
+            and oracle_id.instance_name is None
+            and oracle_id.sid is None
+            and oracle_id.alias is None
             and instance.auth is None
             and instance.connection is None
         ):
             continue
         oracle_instance = OracleInstance(
-            service_name=instance.service_name,
-            instance_name=instance.instance_name,
+            service_name=oracle_id.service_name,
+            instance_name=oracle_id.instance_name,
+            sid=oracle_id.sid,
+            alias=oracle_id.alias,
             authentication=_get_oracle_authentication(instance.auth),
             connection=_get_oracle_connection(instance.connection),
         )

cmk/plugins/oracle/rulesets/mk_oracle_unified.py

@@ -238,15 +238,48 @@ def _oracle_id() -> Dictionary:
             "service_name": DictElement(
                 parameter_form=String(
                     title=Title("Service Name"),
-                    help_text=Help("Oracle Service Name of the database instance."),
+                    help_text=Help(
+                        "The Oracle service name used to connect to the database. "
+                        "A service name typically represents a database and can map to "
+                        "one or more instances in a RAC environment."
+                    ),
                 ),
-                required=True,
+                required=False,
             ),
             "instance_name": DictElement(
                 parameter_form=String(
-                    title=Title("Instance Name(SID)"),
+                    title=Title("Instance Name"),
                     help_text=Help(
-                        "Oracle Instance Name of the database instance. May be the same as SID."
+                        "The Oracle instance name (ORACLE_SID running instance) to connect to. "
+                        "Use this to target a specific instance when multiple instances serve "
+                        "the same service name, e.g. in Oracle RAC configurations. "
+                        "If not set, the connection is made to the service name without "
+                        "specifying a particular instance."
+                    ),
+                ),
+                required=False,
+            ),
+            "sid": DictElement(
+                parameter_form=String(
+                    title=Title("SID"),
+                    help_text=Help(
+                        "The Oracle System Identifier (SID) that identifies "
+                        "a specific database instance on the host. "
+                        "Use SID-based connections for older Oracle configurations or "
+                        "when connecting to a database that does not use service names. "
+                        "If both a service name and SID are specified, "
+                        "the service name takes precedence."
+                    ),
+                ),
+                required=False,
+            ),
+            "alias": DictElement(
+                parameter_form=String(
+                    title=Title("TNS Alias"),
+                    help_text=Help(
+                        "A TNS alias as defined in the <tt>tnsnames.ora</tt> file. "
+                        "Requires a properly configured <tt>tnsnames.ora</tt> file accessible "
+                        "via the TNS_ADMIN directory path."
                     ),
                 ),
                 required=False,
@@ -255,7 +288,7 @@ def _oracle_id() -> Dictionary:
     )
 
 
-def _connection_options(*, with_service_name: bool) -> Dictionary:
+def _connection_options() -> Dictionary:
     base: dict[str, DictElement[str] | DictElement[int]] = {
         "host": DictElement(
             parameter_form=String(
@@ -304,16 +337,9 @@ def _connection_options(*, with_service_name: bool) -> Dictionary:
             required=False,
         ),
     }
-
-    extension: dict[str, DictElement[_NamedOption]] = {
-        "oracle_id": DictElement(
-            parameter_form=_oracle_id(),
-            required=False,
-        ),
-    }
     return Dictionary(
         title=Title("Connection options"),
-        elements=base | extension if with_service_name else base,
+        elements=base,
     )
 
 
@@ -510,15 +536,20 @@ def _endpoint(
             required=is_main_entry,
         ),
         "connection": DictElement(
-            parameter_form=_connection_options(with_service_name=is_main_entry),
+            parameter_form=_connection_options(),
             required=is_main_entry,
         ),
     }
 
 
 def _main() -> Dictionary:
     return Dictionary(
-        title=Title("Default configuration"),
+        title=Title("Default settings"),
+        help_text=Help(
+            "These settings apply to all monitored Oracle databases by default. "
+            "They define the authentication, connection, and data collection options "
+            "used unless overridden for a specific database in the instance list below."
+        ),
         elements={
             **_endpoint(is_main_entry=True),
             "cache_age": DictElement(
@@ -543,27 +574,20 @@ def _main() -> Dictionary:
 
 def _instances() -> List[_NamedOption]:
     return List(
-        title=Title("Database instances"),
+        title=Title("Databases to monitor"),
         help_text=Help(
-            "Define Oracle database instances to monitor to overwrite default settings."
+            "Define the Oracle databases you want to monitor. Each entry must include "
+            "an Oracle database identifier (service name, instance name, SID, or TNS alias). "
+            "Authentication and connection options from the default settings are used "
+            "automatically, but can be overridden per database if needed."
         ),
-        add_element_label=Label("Add new database instance"),
+        add_element_label=Label("Add new database"),
         element_template=Dictionary(
-            title=Title("Database instance"),
+            title=Title("Database"),
             elements={
-                "service_name": DictElement(
-                    parameter_form=String(
-                        title=Title("Service name"),
-                        help_text=Help("Oracle Service Name"),
-                    ),
-                    required=False,
-                ),
-                "instance_name": DictElement(
-                    parameter_form=String(
-                        title=Title("Oracle instance name"),
-                        help_text=Help("Oracle Instance Name, may be the same as SID."),
-                    ),
-                    required=False,
+                "oracle_id": DictElement(
+                    parameter_form=_oracle_id(),
+                    required=True,
                 ),
                 **_endpoint(is_main_entry=False),
             },

tests/unit/cmk/plugins/oracle/bakery/test_oracle_unified.py

@@ -115,7 +115,6 @@ def _combine(files: Sequence[Plugin], yaml_lines: Sequence[str]) -> Sequence[Plu
             port=None,
             timeout=None,
             tns_admin=None,
-            oracle_id=None,
         ),
         options=None,
         cache_age=None,
@@ -157,7 +156,6 @@ def _combine(files: Sequence[Plugin], yaml_lines: Sequence[str]) -> Sequence[Plu
             port=1521,
             timeout=10,
             tns_admin="/etc/oracle/tns",
-            oracle_id=None,
         ),
         options=GuiAdditionalOptionsConf(
             max_connections=10,
@@ -195,14 +193,15 @@ def _combine(files: Sequence[Plugin], yaml_lines: Sequence[str]) -> Sequence[Plu
     ),
     instances=[
         GuiInstanceConf(
-            service_name="Service_Name_1",
-            instance_name=None,
-            auth=None,
-            connection=None,
+            oracle_id=GuiOracleIdentificationConf(
+                service_name="Service_Name_1",
+            ),
         ),
         GuiInstanceConf(
-            service_name="Service_Name_2",
-            instance_name="Instance_Name_2",
+            oracle_id=GuiOracleIdentificationConf(
+                service_name="Service_Name_2",
+                instance_name="Instance_Name_2",
+            ),
             auth=GuiAuthConf(
                 auth_type=(
                     OracleAuthType.STANDARD,
@@ -218,14 +217,10 @@ def _combine(files: Sequence[Plugin], yaml_lines: Sequence[str]) -> Sequence[Plu
                 port=1522,
                 timeout=20,
                 tns_admin="/etc/oracle/tns2",
-                oracle_id=None,
             ),
         ),
         GuiInstanceConf(
-            service_name=None,
-            instance_name=None,
-            auth=None,
-            connection=None,
+            oracle_id=GuiOracleIdentificationConf(),
         ),
     ],
 )
@@ -296,9 +291,6 @@ def _combine(files: Sequence[Plugin], yaml_lines: Sequence[str]) -> Sequence[Plu
             timeout=None,
             tns_admin="some_tns_admin",
             oracle_local_registry="some_registry",
-            oracle_id=GuiOracleIdentificationConf(
-                service_name="some_name", instance_name="some_instance"
-            ),
         ),
         options=None,
         cache_age=None,
@@ -339,10 +331,8 @@ def _combine(files: Sequence[Plugin], yaml_lines: Sequence[str]) -> Sequence[Plu
     "    cache_age: 600",
     "    connection:",
     "      hostname: localhost",
-    "      instance: some_instance",
     "      oracle_local_registry: some_registry",
     "      port: 1521",
-    "      service_name: some_name",
     "      tns_admin: some_tns_admin",
     "    sections:",
     "    - instance:",
@@ -376,10 +366,9 @@ def _combine(files: Sequence[Plugin], yaml_lines: Sequence[str]) -> Sequence[Plu
     ),
     instances=[
         GuiInstanceConf(
-            service_name="SIDONLY",
-            instance_name=None,
-            auth=None,
-            connection=None,
+            oracle_id=GuiOracleIdentificationConf(
+                service_name="SIDONLY",
+            ),
         ),
     ],
 )
@@ -430,14 +419,14 @@ def _combine(files: Sequence[Plugin], yaml_lines: Sequence[str]) -> Sequence[Plu
     ),
     instances=[
         GuiInstanceConf(
-            service_name=None,
-            instance_name="SID_A",
-            auth=None,
-            connection=None,
+            oracle_id=GuiOracleIdentificationConf(
+                instance_name="SID_A",
+            ),
         ),
         GuiInstanceConf(
-            service_name=None,
-            instance_name="SID_B",
+            oracle_id=GuiOracleIdentificationConf(
+                instance_name="SID_B",
+            ),
             auth=GuiAuthConf(
                 auth_type=(
                     OracleAuthType.STANDARD,