Hide service connector secrets as internal implementation details (#3770)

* WIP

* Hide service connector secrets from users

* Add secret expansion option to service connector API calls and endpoints

* Renamed hidden to internal in the secrets table

* Fix secret serialization and other minor bugs

* Fix linter and spelling errors and add DB migration file

* Validate connector models in the REST zen store and fix unit tests

* Fix unit tests

* Fix remaining unit tests

* Fix linter errors

Author: stefannica

.typos.toml

@@ -36,6 +36,7 @@ arange = "arange"
 cachable = "cachable"
 OT = "OT"
 cll = "cll"
+ser = "ser"
 
 [default]
 locale = "en-us"

src/zenml/cli/service_connectors.py

@@ -899,14 +899,7 @@ def register_service_connector(
 
         # Prepare the rest of the variables to fall through to the
         # non-interactive configuration case
-        parsed_args = connector_model.configuration
-        parsed_args.update(
-            {
-                k: s.get_secret_value()
-                for k, s in connector_model.secrets.items()
-                if s is not None
-            }
-        )
+        parsed_args = connector_model.configuration.plain
         auto_configure = False
         no_verify = False
         expiration_seconds = connector_model.expiration_seconds
@@ -1137,7 +1130,7 @@ def describe_service_connector(
         try:
             connector = client.get_service_connector(
                 name_id_or_prefix=name_id_or_prefix,
-                load_secrets=True,
+                expand_secrets=show_secrets,
             )
         except KeyError as err:
             cli_utils.error(str(err))
@@ -1385,7 +1378,7 @@ def update_service_connector(
         connector = client.get_service_connector(
             name_id_or_prefix,
             allow_name_prefix_match=False,
-            load_secrets=True,
+            expand_secrets=True,
         )
     except KeyError as e:
         cli_utils.error(str(e))
@@ -1445,7 +1438,7 @@ def update_service_connector(
                 default=False,
             )
 
-        existing_config = connector.full_configuration
+        existing_config = connector.configuration.plain
 
         if confirm:
             # Here we reconfigure the connector or update the existing
@@ -1582,7 +1575,7 @@ def update_service_connector(
         # Non-interactive configuration
 
         # Apply the configuration from the command line arguments
-        config_dict = connector.full_configuration
+        config_dict = connector.configuration.plain
         config_dict.update(parsed_args)
 
         if not resource_type and not connector.is_multi_type:

src/zenml/cli/stack.py

@@ -1783,14 +1783,10 @@ def _get_service_connector_info(
     answers = {}
     for req_field in required_fields:
         if connector_details:
-            if conf_value := connector_details.configuration.get(
+            if conf_value := connector_details.configuration.get_plain(
                 req_field, None
             ):
                 answers[req_field] = conf_value
-            elif secret_value := connector_details.secrets.get(
-                req_field, None
-            ):
-                answers[req_field] = secret_value.get_secret_value()
         if req_field not in answers:
             answers[req_field] = Prompt.ask(
                 f"Please enter value for `{req_field}`:",

src/zenml/cli/utils.py

@@ -1814,7 +1814,7 @@ def print_service_connector_configuration(
 
     console.print(rich_table)
 
-    if len(connector.configuration) == 0 and len(connector.secrets) == 0:
+    if len(connector.configuration) == 0:
         declare("No configuration options are set for this connector.")
 
     else:
@@ -1826,8 +1826,8 @@ def print_service_connector_configuration(
         rich_table.add_column("PROPERTY")
         rich_table.add_column("VALUE", overflow="fold")
 
-        config = connector.configuration.copy()
-        secrets = connector.secrets.copy()
+        config = connector.configuration.non_secrets
+        secrets = connector.configuration.secrets
         for key, value in secrets.items():
             if not show_secrets:
                 config[key] = "[HIDDEN]"

src/zenml/client.py

@@ -5500,17 +5500,18 @@ def get_service_connector(
         self,
         name_id_or_prefix: Union[str, UUID],
         allow_name_prefix_match: bool = True,
-        load_secrets: bool = False,
         hydrate: bool = True,
+        expand_secrets: bool = False,
     ) -> ServiceConnectorResponse:
         """Fetches a registered service connector.
 
         Args:
             name_id_or_prefix: The id of the service connector to fetch.
             allow_name_prefix_match: If True, allow matching by name prefix.
-            load_secrets: If True, load the secrets for the service connector.
             hydrate: Flag deciding whether to hydrate the output model(s)
                 by including metadata fields in the response.
+            expand_secrets: If True, expand the secrets for the service
+                connector.
 
         Returns:
             The registered service connector.
@@ -5521,25 +5522,9 @@ def get_service_connector(
             name_id_or_prefix=name_id_or_prefix,
             allow_name_prefix_match=allow_name_prefix_match,
             hydrate=hydrate,
+            expand_secrets=expand_secrets,
         )
 
-        if load_secrets and connector.secret_id:
-            client = Client()
-            try:
-                secret = client.get_secret(
-                    name_id_or_prefix=connector.secret_id,
-                    allow_partial_id_match=False,
-                    allow_partial_name_match=False,
-                )
-            except KeyError as err:
-                logger.error(
-                    "Unable to retrieve secret values associated with "
-                    f"service connector '{connector.name}': {err}"
-                )
-            else:
-                # Add secret values to connector configuration
-                connector.secrets.update(secret.values)
-
         return connector
 
     def list_service_connectors(
@@ -5558,8 +5543,8 @@ def list_service_connectors(
         resource_id: Optional[str] = None,
         user: Optional[Union[UUID, str]] = None,
         labels: Optional[Dict[str, Optional[str]]] = None,
-        secret_id: Optional[Union[str, UUID]] = None,
         hydrate: bool = False,
+        expand_secrets: bool = False,
     ) -> Page[ServiceConnectorResponse]:
         """Lists all registered service connectors.
 
@@ -5580,10 +5565,10 @@ def list_service_connectors(
             user: Filter by user name/ID.
             name: The name of the service connector to filter by.
             labels: The labels of the service connector to filter by.
-            secret_id: Filter by the id of the secret that is referenced by the
-                service connector.
             hydrate: Flag deciding whether to hydrate the output model(s)
                 by including metadata fields in the response.
+            expand_secrets: If True, expand the secrets for the service
+                connectors.
 
         Returns:
             A page of service connectors.
@@ -5603,11 +5588,11 @@ def list_service_connectors(
             created=created,
             updated=updated,
             labels=labels,
-            secret_id=secret_id,
         )
         return self.zen_store.list_service_connectors(
             filter_model=connector_filter_model,
             hydrate=hydrate,
+            expand_secrets=expand_secrets,
         )
 
     def update_service_connector(
@@ -5691,7 +5676,9 @@ def update_service_connector(
         connector_model = self.get_service_connector(
             name_id_or_prefix,
             allow_name_prefix_match=False,
-            load_secrets=True,
+            # We need the existing secrets only if a new configuration is not
+            # provided.
+            expand_secrets=configuration is None,
         )
 
         connector_instance: Optional[ServiceConnector] = None
@@ -5736,23 +5723,16 @@ def update_service_connector(
         )
 
         # Validate and configure the resources
-        if configuration is not None:
+        connector_update.validate_and_configure_resources(
+            connector_type=connector,
+            resource_types=resource_types,
+            resource_id=resource_id,
             # The supplied configuration is a drop-in replacement for the
-            # existing configuration and secrets
-            connector_update.validate_and_configure_resources(
-                connector_type=connector,
-                resource_types=resource_types,
-                resource_id=resource_id,
-                configuration=configuration,
-            )
-        else:
-            connector_update.validate_and_configure_resources(
-                connector_type=connector,
-                resource_types=resource_types,
-                resource_id=resource_id,
-                configuration=connector_model.configuration,
-                secrets=connector_model.secrets,
-            )
+            # existing configuration
+            configuration=configuration
+            if configuration is not None
+            else connector_model.configuration,
+        )
 
         # Add the labels
         if labels is not None:
@@ -5912,6 +5892,12 @@ def verify_service_connector(
                 list_resources=list_resources,
             )
         else:
+            # Get the service connector model, with full secrets
+            service_connector = self.get_service_connector(
+                name_id_or_prefix=name_id_or_prefix,
+                allow_name_prefix_match=False,
+                expand_secrets=True,
+            )
             connector_instance = (
                 service_connector_registry.instantiate_connector(
                     model=service_connector
@@ -6034,6 +6020,12 @@ def get_service_connector_client(
                 # server-side implementation may not be able to do so
                 connector_client.verify()
         else:
+            # Get the service connector model, with full secrets
+            service_connector = self.get_service_connector(
+                name_id_or_prefix=name_id_or_prefix,
+                allow_name_prefix_match=False,
+                expand_secrets=True,
+            )
             connector_instance = (
                 service_connector_registry.instantiate_connector(
                     model=service_connector

src/zenml/integrations/aws/container_registries/aws_container_registry.py

@@ -81,7 +81,9 @@ def _get_ecr_client(self) -> BaseClient:
         """
         if self.connector:
             try:
-                model = Client().get_service_connector(self.connector)
+                model = Client().get_service_connector(
+                    self.connector, expand_secrets=True
+                )
                 connector = service_connector_registry.instantiate_connector(
                     model=model
                 )

src/zenml/models/__init__.py

@@ -286,6 +286,7 @@
     ServiceAccountResponse,
 )
 from zenml.models.v2.core.service_connector import (
+    ServiceConnectorConfiguration,
     ServiceConnectorRequest,
     ServiceConnectorUpdate,
     ServiceConnectorFilter,
@@ -739,6 +740,7 @@
     "ServiceAccountUpdate",
     "ServiceAccountRequest",
     "ServiceAccountResponse",
+    "ServiceConnectorConfiguration",
     "ServiceConnectorRequest",
     "ServiceConnectorUpdate",
     "ServiceConnectorFilter",