Fix FerretDB tenant provisioning on 0.9.0

.github/workflows/check.yml

@@ -64,7 +64,7 @@ jobs:
         run: bash .github/cisetup.sh
       - name: Install
         run: task setup
-      - name: Security regression tests
+      - name: Regression tests
         run: poetry run python3 -m unittest discover -s tests -p 'test_*.py' -v
       - name: BuildAndLoad
         run: task build-and-load
@@ -78,20 +78,28 @@ jobs:
         continue-on-error: false
       - name: Reject new integration failures
         run: |
-          # The legacy runner writes failures without returning nonzero.
-          # These two tests also fail on unmodified 0.9.0 (run 38893750250).
+          # The legacy runner does not propagate failure. Only the exact original
+          # 0.9.0 test scripts (3ee0a79; reproduced in run 38893750250) may retain
+          # their known warning. An updated test must pass, so both exceptions
+          # automatically disappear once the two independent fixes are merged.
           unexpected_failures=0
           if [ -f _failed.txt ]; then
             while IFS= read -r failed_test; do
               case "$failed_test" in
-                tests/kind/ferretdb_test.ipy|tests/kind/runtimes_preloader_test.ipy)
-                  echo "::warning::Pre-existing 0.9.0 integration failure: $failed_test"
+                tests/kind/ferretdb_test.ipy)
+                  baseline_blob=90e70ffe1ae1b126e5bd651f32a6a6240b4def07
                   ;;
-                *)
-                  echo "::error::Integration regression: $failed_test"
-                  unexpected_failures=1
+                tests/kind/runtimes_preloader_test.ipy)
+                  baseline_blob=afbee5e8f49945f4737c507de36c731e1da6faee
                   ;;
+                *) baseline_blob= ;;
               esac
+              if [ -n "$baseline_blob" ] && [ "$(git hash-object -- "$failed_test")" = "$baseline_blob" ]; then
+                echo "::warning::Pre-existing 0.9.0 integration failure in unchanged baseline test: $failed_test"
+              else
+                echo "::error::Integration regression: $failed_test"
+                unexpected_failures=1
+              fi
             done < _failed.txt
           fi
           exit "$unexpected_failures"

openserverless/ferretdb.py

@@ -160,10 +160,11 @@ def _add_mdb_user_metadata(user_metadata, data):
 
             mdb_url = f"mongodb://{auth}@{mdb_pod_name}.{mdb_service_name}.{mdb_ns}.svc.cluster.local:27017/{database}?connectTimeoutMS=60000&authMechanism=PLAIN"
             user_metadata.add_metadata("MONGODB_URL",mdb_url)
-        return None
+            return True
+        return False
     except Exception as e:
         logging.error(f"failed to build mongodb_url for {data['database']}: {e}")
-        return None
+        return False
 
 def create_db_user(ucfg: UserConfig, user_metadata: UserMetadata):
     database = ucfg.get('mongodb.database')
@@ -178,16 +179,23 @@ def create_db_user(ucfg: UserConfig, user_metadata: UserMetadata):
         data["password"]=ucfg.get('mongodb.password')
         data["mode"]="create"
 
-        path_to_pgpass = postgres.render_postgres_script(f"{namespace}_ferretdb","pgpass_tpl.properties",data)
-        path_to_mdb_script = postgres.render_postgres_script(f"{namespace}_ferretdb","postgres_manage_user_tpl.sql",data)
-        pod_name = util.get_pod_name("{.items[?(@.metadata.labels.app == 'openserverless-postgres')].metadata.name}")      
+        if not util.validate_namespace(namespace):
+            raise ValueError("Invalid FerretDB namespace")
+        if not util.validate_database_name(data["database"]) or not util.validate_database_name(data["username"]):
+            raise ValueError("Invalid FerretDB database or username")
+        # The renderer needs the Kubernetes namespace as a safe file prefix;
+        # the PostgreSQL names retain their existing _ferretdb suffix.
+        path_to_pgpass = postgres.render_postgres_script(namespace,"pgpass_tpl.properties",data)
+        path_to_mdb_script = postgres.render_postgres_script(namespace,"postgres_manage_user_tpl.sql",data)
+        pod_name = util.get_pod_name_by_selector("app=openserverless-postgres","{.items[?(@.metadata.labels.replicationRole == 'primary')].metadata.name}")
 
         if(pod_name):
             res = postgres.exec_psql_command(pod_name,path_to_mdb_script,path_to_pgpass)
 
             if res:
-                _add_mdb_user_metadata(user_metadata, data)
-                return res
+                if _add_mdb_user_metadata(user_metadata, data):
+                    return res
+                logging.error("FerretDB user created but connection metadata is unavailable")
             else:
                 logging.error("failed to execute SQL script on Postgres DB to enable MongoDB (FerretDB) user")
 
@@ -205,9 +213,13 @@ def delete_db_user(namespace, database):
         data["username"]=f"{namespace}_ferretdb"
         data["mode"]="delete"
 
-        path_to_pgpass = postgres.render_postgres_script(f"{namespace}_ferretdb","pgpass_tpl.properties",data)
-        path_to_mdb_script = postgres.render_postgres_script(f"{namespace}_ferretdb","postgres_manage_user_tpl.sql",data)
-        pod_name = util.get_pod_name("{.items[?(@.metadata.labels.app == 'openserverless-postgres')].metadata.name}")
+        if not util.validate_namespace(namespace):
+            raise ValueError("Invalid FerretDB namespace")
+        if not util.validate_database_name(data["database"]) or not util.validate_database_name(data["username"]):
+            raise ValueError("Invalid FerretDB database or username")
+        path_to_pgpass = postgres.render_postgres_script(namespace,"pgpass_tpl.properties",data)
+        path_to_mdb_script = postgres.render_postgres_script(namespace,"postgres_manage_user_tpl.sql",data)
+        pod_name = util.get_pod_name_by_selector("app=openserverless-postgres","{.items[?(@.metadata.labels.replicationRole == 'primary')].metadata.name}")
 
         if(pod_name):
             res = postgres.exec_psql_command(pod_name,path_to_mdb_script,path_to_pgpass)
@@ -216,4 +228,4 @@ def delete_db_user(namespace, database):
         return None
     except Exception as e:
         logging.error(f"failed to remove Ferretdb database {namespace} authorization id and key: {e}")
-        return None        
+        return None

tests/kind/ferretdb_test.ipy

@@ -21,6 +21,7 @@
 import openserverless.config as cfg
 import openserverless.ferretdb as mdb
 import openserverless.kube as kube
+import openserverless.util as util
 import openserverless.postgres_operator as postgres
 import openserverless.testutil as tu
 from openserverless.user_config import UserConfig
@@ -46,7 +47,22 @@ udata = UserMetadata(ucfg)
 
 assert(mdb.create_db_user(ucfg, udata))
 assert(any(e['key'] == "MONGODB_URL" for e in udata.get_metadata()['metadata']))
+
+# Prove provisioning created the intended database and login grants, rather than merely
+# returning psql output. Keep credentials inside the database pod environment.
+primary = util.get_pod_name_by_selector("app=openserverless-postgres", "{.items[?(@.metadata.labels.replicationRole == 'primary')].metadata.name}")
+assert(primary)
+
+def scalar(query):
+    return kube.kubectl("exec", primary, "--", "sh", "-c",
+        'PGPASSWORD="$POSTGRES_PASSWORD" exec psql --no-psqlrc --username=postgres --dbname=postgres --tuples-only --no-align --set=ON_ERROR_STOP=1 --command "$1"',
+        "ferretdb-probe", query).strip()
+
+database = (ucfg.get('mongodb.database') + '_ferretdb').replace("'", "''")
+username = (ucfg.get('namespace') + '_ferretdb').replace("'", "''")
+assert(scalar(f"SELECT count(*) FROM pg_database d, pg_roles r WHERE d.datname='{database}' AND r.rolname='{username}' AND r.rolcanlogin AND has_database_privilege(r.oid,d.oid,'CONNECT') AND has_database_privilege(r.oid,d.oid,'CREATE')") == '1')
 assert(mdb.delete_db_user(ucfg.get('namespace'),ucfg.get('mongodb.database')))
+assert(scalar(f"SELECT ((EXISTS(SELECT 1 FROM pg_database WHERE datname='{database}')) OR (EXISTS(SELECT 1 FROM pg_roles WHERE rolname='{username}')))::int") == '0')
 
 assert(mdb.delete())
 assert(postgres.delete())

tests/test_ferretdb_provisioning.py

@@ -0,0 +1,105 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+"""Exercise FerretDB provisioning with the real namespace validator and renderer."""
+from contextlib import ExitStack
+from types import SimpleNamespace
+import unittest
+from unittest.mock import Mock, patch
+from urllib.parse import urlsplit, unquote
+
+from openserverless import ferretdb as mongo, postgres_operator as postgres, util
+
+
+class FerretDBProvisioningTests(unittest.TestCase):
+    def setUp(self):
+        stack=ExitStack();self.addCleanup(stack.close)
+        self.values={'namespace':'fixtureuser','mongodb.database':'fixturedb','mongodb.password':'synthetic'}
+        self.user=SimpleNamespace(get=self.values.get)
+        self.metadata=Mock()
+        stack.enter_context(patch.object(util,'get_postgres_config_data',return_value={}))
+        self.primary=stack.enter_context(patch.object(util,'get_pod_name_by_selector',return_value='postgres-primary'))
+        stack.enter_context(patch.object(util,'get_pod_name',side_effect=AssertionError('ambiguous pod lookup')))
+        self.service=stack.enter_context(patch.object(util,'get_service',return_value={'metadata':{'name':'mongo-svc','namespace':'openserverless'}}))
+        stack.enter_context(patch.object(mongo,'get_ferret_pod_name',return_value='mongo-0'))
+        # Keep the actual renderer and validator; mock only filesystem and Kubernetes I/O.
+        self.spool=stack.enter_context(patch.object(postgres.ntp,'spool_template',side_effect=lambda template,path,data:path))
+        self.chmod=stack.enter_context(patch.object(postgres.os,'chmod'))
+        self.render=stack.enter_context(patch.object(postgres,'render_postgres_script',wraps=postgres.render_postgres_script))
+        self.sql=stack.enter_context(patch.object(postgres,'exec_psql_command',return_value='SQL completed'))
+
+    def assert_primary_and_names(self,mode):
+        self.primary.assert_called_once_with('app=openserverless-postgres',"{.items[?(@.metadata.labels.replicationRole == 'primary')].metadata.name}")
+        self.sql.assert_called_once()
+        self.assertEqual(self.sql.call_args.args[0],'postgres-primary')
+        self.assertEqual(len(self.render.call_args_list),2)
+        for call in self.render.call_args_list:
+            namespace,template,data=call.args
+            self.assertEqual(namespace,'fixtureuser')
+            self.assertEqual(data['database'],'fixturedb_ferretdb')
+            self.assertEqual(data['username'],'fixtureuser_ferretdb')
+            self.assertEqual(data['mode'],mode)
+
+    def test_create_keeps_sql_suffix_and_uses_real_kubernetes_namespace(self):
+        self.assertTrue(mongo.create_db_user(self.user,self.metadata))
+        self.assert_primary_and_names('create')
+        name,uri=self.metadata.add_metadata.call_args.args
+        self.assertEqual(name,'MONGODB_URL')
+        self.assertEqual(unquote(urlsplit(uri).username),'fixtureuser_ferretdb')
+        self.assertEqual(urlsplit(uri).path,'/fixturedb_ferretdb')
+
+    def test_delete_uses_primary_and_the_same_existing_sql_names(self):
+        self.assertTrue(mongo.delete_db_user('fixtureuser','fixturedb'))
+        self.assert_primary_and_names('delete')
+
+    def test_invalid_database_is_rejected_before_rendering_or_sql(self):
+        self.values['mongodb.database']='bad;name'
+        self.assertFalse(mongo.create_db_user(self.user,self.metadata))
+        self.render.assert_not_called();self.sql.assert_not_called()
+
+    def test_invalid_delete_database_is_rejected_before_rendering_or_sql(self):
+        self.assertFalse(mongo.delete_db_user('fixtureuser','bad;name'))
+        self.render.assert_not_called();self.sql.assert_not_called()
+
+    def test_real_namespace_validation_remains_strict(self):
+        self.assertFalse(util.validate_namespace('fixtureuser_ferretdb'))
+        self.values['namespace']='fixture_user'
+        self.assertFalse(mongo.create_db_user(self.user,self.metadata))
+        self.render.assert_not_called()
+        self.sql.assert_not_called()
+
+    def test_invalid_delete_namespace_is_rejected_before_rendering_or_sql(self):
+        self.assertFalse(mongo.delete_db_user('fixture_user','fixturedb'))
+        self.render.assert_not_called();self.sql.assert_not_called()
+
+    def test_failed_sql_does_not_publish_connection_metadata(self):
+        self.sql.return_value=''
+        self.assertFalse(mongo.create_db_user(self.user,self.metadata))
+        self.metadata.add_metadata.assert_not_called()
+
+    def test_missing_service_does_not_report_provisioning_success(self):
+        self.service.return_value=None
+        self.assertFalse(mongo.create_db_user(self.user,self.metadata))
+        self.metadata.add_metadata.assert_not_called()
+
+    def test_metadata_publication_failure_does_not_report_success(self):
+        self.metadata.add_metadata.side_effect=RuntimeError('synthetic metadata failure')
+        self.assertFalse(mongo.create_db_user(self.user,self.metadata))
+
+
+if __name__=='__main__':unittest.main()