diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 965ca64..3dca931 100644 --- a/.github/workflows/check.yml +++ b/.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" diff --git a/openserverless/ferretdb.py b/openserverless/ferretdb.py index 6d911e6..df4f87b 100644 --- a/openserverless/ferretdb.py +++ b/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 \ No newline at end of file + return None diff --git a/tests/kind/ferretdb_test.ipy b/tests/kind/ferretdb_test.ipy index 90e70ff..6b34d4a 100644 --- a/tests/kind/ferretdb_test.ipy +++ b/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()) diff --git a/tests/test_ferretdb_provisioning.py b/tests/test_ferretdb_provisioning.py new file mode 100644 index 0000000..3bda0ee --- /dev/null +++ b/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()