[Secrets] Avoid auto-adding internal project secrets + model monitori…

…ng access-key changes (#1487)

mlrun/api/api/endpoints/functions.py

@@ -18,6 +18,8 @@
 import mlrun.api.utils.singletons.project_member
 from mlrun.api.api import deps
 from mlrun.api.api.utils import get_run_db_instance, log_and_raise
+from mlrun.api.crud.secrets import Secrets
+from mlrun.api.schemas import SecretProviderName, SecretsData
 from mlrun.api.utils.singletons.k8s import get_k8s
 from mlrun.builder import build_runtime
 from mlrun.config import config
@@ -430,15 +432,11 @@ def _build_function(
                     if fn.spec.track_models:
                         logger.info("Tracking enabled, initializing model monitoring")
                         _init_serving_function_stream_args(fn=fn)
-                        model_monitoring_access_key = _get_project_secret(
-                            fn.metadata.project, "MODEL_MONITORING_ACCESS_KEY"
+                        model_monitoring_access_key = _process_model_monitoring_secret(
+                            db_session,
+                            fn.metadata.project,
+                            "MODEL_MONITORING_ACCESS_KEY",
                         )
-                        # TODO: if model monitoring is not defined we should try to grab the project owner access key
-                        #  as well
-                        if not model_monitoring_access_key:
-                            raise MLRunRuntimeError(
-                                "MODEL_MONITORING_ACCESS_KEY not found in project secrets"
-                            )
 
                         _create_model_monitoring_stream(project=fn.metadata.project)
                         mlrun.api.crud.ModelEndpoints().deploy_monitoring_functions(
@@ -616,14 +614,55 @@ def _get_function_env_var(fn: ServingRuntime, var_name: str):
     return None
 
 
-def _get_project_secret(project_name: str, secret_key: str):
+def _process_model_monitoring_secret(db_session, project_name: str, secret_key: str):
+    # The expected result of this method is an access-key placed in an internal project-secret.
+    # If the user provided an access-key as the "regular" secret_key, then we delete this secret and move contents
+    # to the internal secret instead. Else, if the internal secret already contained a value, keep it. Last option
+    # (which is the recommended option for users) is to retrieve a new access-key from the project owner and use it.
     logger.info(
         "Getting project secret", project_name=project_name, namespace=config.namespace
     )
-    secret_value = mlrun.api.crud.Secrets().get_secret(
-        project_name,
-        mlrun.api.schemas.SecretProviderName.kubernetes,
-        secret_key,
-        allow_secrets_from_k8s=True,
+
+    provider = SecretProviderName.kubernetes
+    secret_value = Secrets().get_secret(
+        project_name, provider, secret_key, allow_secrets_from_k8s=True,
     )
+    user_provided_key = secret_value is not None
+    internal_key_name = Secrets().generate_model_monitoring_secret_key(secret_key)
+
+    if not user_provided_key:
+        secret_value = Secrets().get_secret(
+            project_name,
+            provider,
+            internal_key_name,
+            allow_secrets_from_k8s=True,
+            allow_internal_secrets=True,
+        )
+        if not secret_value:
+            import mlrun.api.utils.singletons.project_member
+
+            project_owner = mlrun.api.utils.singletons.project_member.get_project_member().get_project_owner(
+                db_session, project_name
+            )
+
+            secret_value = project_owner.session
+            if not secret_value:
+                raise MLRunRuntimeError(
+                    f"No model monitoring access key. Failed to generate one for owner of project {project_name}",
+                )
+
+            logger.info(
+                "Filling model monitoring access-key from project owner",
+                project_name=project_name,
+                project_owner=project_owner.username,
+            )
+
+    secrets = SecretsData(provider=provider, secrets={internal_key_name: secret_value})
+    Secrets().store_secrets(project_name, secrets, allow_internal_secrets=True)
+    if user_provided_key:
+        logger.info(
+            "Deleting user-provided access-key - replaced with an internal secret"
+        )
+        Secrets().delete_secret(project_name, provider, secret_key)
+
     return secret_value

mlrun/api/crud/model_endpoints.py

@@ -10,9 +10,11 @@
 from v3io_frames.errors import CreateError
 
 import mlrun.api.api.utils
+import mlrun.api.utils.singletons.k8s
 import mlrun.datastore.store_resources
 from mlrun.api.api.endpoints.functions import _build_function
 from mlrun.api.api.utils import _submit_run, get_run_db_instance
+from mlrun.api.crud.secrets import Secrets
 from mlrun.api.schemas import (
     Features,
     Metric,
@@ -627,7 +629,13 @@ def deploy_model_monitoring_stream_processing(
             stream_path=stream_path, name="monitoring_stream_trigger"
         )
 
-        fn.set_env("MODEL_MONITORING_ACCESS_KEY", model_monitoring_access_key)
+        fn.set_env_from_secret(
+            "MODEL_MONITORING_ACCESS_KEY",
+            mlrun.api.utils.singletons.k8s.get_k8s().get_project_secret_name(project),
+            Secrets().generate_model_monitoring_secret_key(
+                "MODEL_MONITORING_ACCESS_KEY"
+            ),
+        )
         fn.metadata.credentials.access_key = model_monitoring_access_key
         fn.set_env("MODEL_MONITORING_PARAMETERS", json.dumps({"project": project}))
 
@@ -667,7 +675,13 @@ def deploy_model_monitoring_batch_processing(
 
         fn.apply(mlrun.mount_v3io())
 
-        fn.set_env("MODEL_MONITORING_ACCESS_KEY", model_monitoring_access_key)
+        fn.set_env_from_secret(
+            "MODEL_MONITORING_ACCESS_KEY",
+            mlrun.api.utils.singletons.k8s.get_k8s().get_project_secret_name(project),
+            Secrets().generate_model_monitoring_secret_key(
+                "MODEL_MONITORING_ACCESS_KEY"
+            ),
+        )
 
         # Needs to be a member of the project and have access to project data path
         fn.metadata.credentials.access_key = model_monitoring_access_key

mlrun/api/crud/secrets.py

@@ -28,6 +28,9 @@ def _generate_schedule_secret_key(self, schedule_name: str):
     def generate_schedule_key_map_secret_key(self):
         return f"{self.key_map_secrets_key_prefix}schedules"
 
+    def generate_model_monitoring_secret_key(self, key):
+        return f"{self.internal_secrets_key_prefix}model-monitoring.{key}"
+
     @staticmethod
     def validate_secret_key_regex(key: str, raise_on_failure: bool = True) -> bool:
         return mlrun.utils.helpers.verify_field_regex(

mlrun/runtimes/pod.py

@@ -479,6 +479,9 @@ def _add_azure_vault_params_to_spec(self, k8s_secret_name=None):
     def _add_project_k8s_secrets_to_spec(
         self, secrets, runobj=None, project=None, encode_key_names=True
     ):
+        # Needs to happen here to avoid circular dependencies
+        from mlrun.api.crud.secrets import Secrets
+
         # the secrets param may be an empty dictionary (asking for all secrets of that project) -
         # it's a different case than None (not asking for project secrets at all).
         if (
@@ -494,7 +497,11 @@ def _add_project_k8s_secrets_to_spec(
 
         secret_name = self._get_k8s().get_project_secret_name(project_name)
         existing_secret_keys = (
-            self._get_k8s().get_project_secret_keys(project_name) or []
+            Secrets()
+            .list_secret_keys(
+                project_name, mlrun.api.schemas.SecretProviderName.kubernetes
+            )
+            .secret_keys
         )
 
         # If no secrets were passed or auto-adding all secrets, we need all existing keys

tests/api/conftest.py

@@ -85,12 +85,17 @@ def get_project_secret_data(self, project, secret_keys=None, namespace=""):
             if (secret_keys and key in secret_keys) or not secret_keys
         }
 
-    def get_expected_env_variables_from_secrets(self, project, encode_key_names=True):
+    def get_expected_env_variables_from_secrets(
+        self, project, encode_key_names=True, include_internal=False
+    ):
         expected_env_from_secrets = {}
         secret_name = mlrun.api.utils.singletons.k8s.get_k8s().get_project_secret_name(
             project
         )
         for key in self.project_secrets_map.get(project, {}):
+            if key.startswith("mlrun.") and not include_internal:
+                continue
+
             env_variable_name = (
                 SecretsStore.k8s_env_variable_name_for_secret(key)
                 if encode_key_names

tests/api/runtimes/test_kubejob.py

@@ -193,13 +193,10 @@ def test_run_with_mounts(self, db: Session, client: TestClient):
         self._assert_pvc_mount_configured(pvc_name, pvc_mount_path, volume_name)
 
     def test_run_with_k8s_secrets(self, db: Session, k8s_secrets_mock: K8sSecretsMock):
-        secret_keys = ["secret1", "secret2", "secret3"]
+        secret_keys = ["secret1", "secret2", "secret3", "mlrun.internal_secret"]
         secrets = {key: "some-secret-value" for key in secret_keys}
 
         k8s_secrets_mock.store_project_secrets(self.project, secrets)
-        expected_env_from_secrets = k8s_secrets_mock.get_expected_env_variables_from_secrets(
-            self.project
-        )
 
         runtime = self._generate_runtime()
 
@@ -213,11 +210,27 @@ def test_run_with_k8s_secrets(self, db: Session, k8s_secrets_mock: K8sSecretsMoc
 
         self._execute_run(runtime, runspec=task)
 
+        # We don't expect the internal secret to be visible - the user cannot mount it to the function
+        # even if specifically asking for it in with_secrets()
+        expected_env_from_secrets = k8s_secrets_mock.get_expected_env_variables_from_secrets(
+            self.project, include_internal=False
+        )
+
         self._assert_pod_creation_config(
             expected_secrets=secret_source,
             expected_env_from_secrets=expected_env_from_secrets,
         )
 
+        # Now do the same with auto-mounting of project-secrets, validate internal secret is not visible
+        runtime = self._generate_runtime()
+        task = self._generate_task()
+        task.metadata.project = self.project
+
+        self._execute_run(runtime, runspec=task)
+        self._assert_pod_creation_config(
+            expected_env_from_secrets=expected_env_from_secrets,
+        )
+
     def test_run_with_vault_secrets(self, db: Session, client: TestClient):
         self._mock_vault_functionality()
         runtime = self._generate_runtime()