From 892c56fa301918efa9ac7524ae27d0a006456f00 Mon Sep 17 00:00:00 2001 From: Chris Wesdorp Date: Mon, 29 Jul 2019 10:50:25 +0200 Subject: [PATCH 1/7] Fix flexibility in the database configuration 1) vcap, 2) database url, 3) custom runtime settings --- README.md | 2 +- lib/database_config.py | 22 ++++++- lib/datadog.py | 5 ++ lib/metrics.py | 8 +++ start.py | 12 ++-- tests/usecase/test_db_config_options.py | 85 +++++++++++++++++++++++++ 6 files changed, 127 insertions(+), 7 deletions(-) create mode 100644 tests/usecase/test_db_config_options.py diff --git a/README.md b/README.md index f27a0fbf..04590090 100644 --- a/README.md +++ b/README.md @@ -54,7 +54,7 @@ In our trial we found the service `elephantsql` which offered the free `turtle` cf bind-service -Note that not all databases are automatically picked up by the buildpack. If `cf push` returns an error like `Could not parse database credentials`, you need to set the `DATABASE_URL` variable manually using the details included in the service. +Note that not all databases are automatically picked up by the buildpack. If `cf push` returns an error like `Could not parse database credentials`, you need to set the `DATABASE_URL` variable manually or set database [Mendix custom runtime variables](https://docs.mendix.com/refguide/custom-settings) to configure a database. Note these need to be prefixed with `MXRUNTIME_`. Now we need to push the application once more. diff --git a/lib/database_config.py b/lib/database_config.py index 273ef6e0..dbfba81b 100644 --- a/lib/database_config.py +++ b/lib/database_config.py @@ -9,15 +9,33 @@ def get_database_config(development_mode=False): + # the following options are validated to get database credentials + # 1) existence of custom runtime settings Database.... values + # 2) VCAP with database credentials + # 3) existence of DATABASE_URL + # + # In case we find MXRUNTIME_Database.... values we don't interfere and + # return nothing. VCAP or DATABASE_URL return m2ee configuration if any( [x.startswith("MXRUNTIME_Database") for x in list(os.environ.keys())] ): + logging.debug( + "Detected database configuration using custom runtime settings." + ) return None factory = DatabaseConfigurationFactory() configuration = factory.get_instance() - return configuration.get_m2ee_configuration() + if configuration: + m2ee_config = configuration.get_m2ee_configuration() + if m2ee_config is not None and "DatabaseType" in m2ee_config: + return m2ee_config + + raise Exception( + "Can't find database configuration from environment variables. " + "Check README for supported configuration options." + ) class DatabaseConfigurationFactory: @@ -38,7 +56,7 @@ def get_instance(self): # fallback to original configuration url = self.get_database_uri_from_vcap(self.vcap_services) - if url is None: + if url is None and "DATABASE_URL" in os.environ: url = os.environ["DATABASE_URL"] if url is not None: diff --git a/lib/datadog.py b/lib/datadog.py index 3a88d56e..85002a19 100644 --- a/lib/datadog.py +++ b/lib/datadog.py @@ -252,6 +252,11 @@ def _set_up_postgres(): "DatabaseHost", ): if k not in dbconfig: + logger.warn( + "Skipping database configuration for DataDog because " + "configuration is not found. See database_config.py " + "for details" + ) return if dbconfig["DatabaseType"] != "PostgreSQL": return diff --git a/lib/metrics.py b/lib/metrics.py index abc7624f..af3a28dc 100644 --- a/lib/metrics.py +++ b/lib/metrics.py @@ -231,6 +231,7 @@ def _get_database_storage(self): def _get_database_mutations(self): conn = self._get_db_conn() db_config = database_config.get_database_config() + with conn.cursor() as cursor: cursor.execute( "SELECT xact_commit, " @@ -327,12 +328,19 @@ def _get_db_conn(self): self.db = None if not self.db: + # get_database config may return None or empty db_config = database_config.get_database_config() + if db_config is None or "DatabaseType" not in db_config: + raise Exception( + "Database not set as VCAP or DATABASE_URL. Check " + "documentation to see supported configuration options." + ) if db_config["DatabaseType"] != "PostgreSQL": raise Exception( "Metrics only supports postgresql, not %s" % db_config["DatabaseType"] ) + host_and_port = db_config["DatabaseHost"].split(":") host = host_and_port[0] if len(host_and_port) > 1: diff --git a/start.py b/start.py index 0f0dd2f2..b806e984 100755 --- a/start.py +++ b/start.py @@ -667,11 +667,15 @@ def set_runtime_config(metadata, mxruntime_config, vcap_data, m2ee): buildpackutil.mkdir_p(os.path.join(os.getcwd(), "model", "resources")) mxruntime_config.update(app_config) - mxruntime_config.update( - database_config.get_database_config( - development_mode=is_development_mode() - ) + + # db configuration might be None, database should then be set up with + # MXRUNTIME_Database... custom runtime settings. + runtime_db_config = database_config.get_database_config( + development_mode=is_development_mode() ) + if runtime_db_config is not None: + mxruntime_config.update(runtime_db_config) + mxruntime_config.update(get_filestore_config(m2ee)) mxruntime_config.update(get_certificate_authorities()) mxruntime_config.update(get_client_certificates()) diff --git a/tests/usecase/test_db_config_options.py b/tests/usecase/test_db_config_options.py new file mode 100644 index 00000000..529caca3 --- /dev/null +++ b/tests/usecase/test_db_config_options.py @@ -0,0 +1,85 @@ +import logging +import os +from nose import with_setup +from database_config import get_database_config + + +class TestDatabaseConfigOptions: + + def clean_env(self): + """ + Setting different environment variables for test in the same process + can lead to flaky tests. + """ + if "DATABASE_URL" in os.environ.keys(): + del os.environ["DATABASE_URL"] + + for key in filter(lambda x: x.startswith("MXRUNTIME_Database"), list(os.environ.keys())): + del os.environ[key] + + + def test_no_setup(self): + self.clean_env(); + try: + config = get_database_config() + assert config is None + except Exception as e: + assert "Can't find database configuration" in str(e) + + + def test_mx_runtime_db_config(self): + """ + Test is MXRUNTIME variables are set up no database configuration is returned + based on DATABASE_URL or VCAP_SERVICES + """ + self.clean_env(); + os.environ["MXRUNTIME_DatabaseType"] = "PostgreSQL" + os.environ[ + "MXRUNTIME_DatabaseJdbcUrl" + ] = "postgres://username:password@rdsbroker-testfree-nonprod-1-eu-west-1.asdbjasdg.eu-west-1.rds.amazonaws.com:5432/testdatabase" # noqa E501 + + config = get_database_config() + assert config is None + + + def test_database_url(self): + self.clean_env(); + os.environ["DATABASE_URL"] = "jdbc:postgres://user:secret@host/database" + + config = get_database_config() + assert config is not None + + + def test_vcap(self): + self.clean_env(); + os.environ["VCAP_SERVICES"] = """ +{ + "rds-testfree": [ + { + "binding_name": null, + "credentials": { + "db_name": "dbuajsdhkasdhaks", + "host": "rdsbroker-testfree-nonprod-1-eu-west-1.asdbjasdg.eu-west-1.rds.amazonaws.com", + "password": "na8nanlayaona0--anbs", + "uri": "postgres://ua98s7ananla:na8nanlayaona0--anbs@rdsbroker-testfree-nonprod-1-eu-west-1.asdbjasdg.eu-west-1.rds.amazonaws.com:5432/dbuajsdhkasdhaks", + "username": "ua98s7ananla" + }, + "instance_name": "ops-432a659e.test.foo.io-database", + "label": "rds-testfree", + "name": "ops-432a659e.test.foo.io-database", + "plan": "shared-psql-testfree", + "provider": null, + "syslog_drain_url": null, + "tags": [ + "database", + "RDS", + "postgresql" + ], + "volume_mounts": [] + } + ] +} + """ # noqa + + config = get_database_config() + assert config is not None From a2b59f44546f59323f336c9268bc1f3329e87b55 Mon Sep 17 00:00:00 2001 From: Chris Wesdorp Date: Tue, 30 Jul 2019 17:22:15 +0200 Subject: [PATCH 2/7] Update version number to 3.6 --- start.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/start.py b/start.py index b806e984..863807f7 100755 --- a/start.py +++ b/start.py @@ -29,7 +29,7 @@ from nginx import get_path_config, gen_htpasswd # noqa: E402 from buildpackutil import i_am_primary_instance # noqa: E402 -BUILDPACK_VERSION = "3.5.0" +BUILDPACK_VERSION = "3.6.0" logger.setLevel(buildpackutil.get_buildpack_loglevel()) From 98dd82d24937ede4a2012a2bc9b475bffffa3744 Mon Sep 17 00:00:00 2001 From: Chris Wesdorp Date: Tue, 30 Jul 2019 17:35:20 +0200 Subject: [PATCH 3/7] make lint work for test case --- tests/usecase/test_db_config_options.py | 28 ++++++++++++------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/usecase/test_db_config_options.py b/tests/usecase/test_db_config_options.py index 529caca3..e4e389aa 100644 --- a/tests/usecase/test_db_config_options.py +++ b/tests/usecase/test_db_config_options.py @@ -1,11 +1,8 @@ -import logging import os -from nose import with_setup from database_config import get_database_config class TestDatabaseConfigOptions: - def clean_env(self): """ Setting different environment variables for test in the same process @@ -14,25 +11,26 @@ def clean_env(self): if "DATABASE_URL" in os.environ.keys(): del os.environ["DATABASE_URL"] - for key in filter(lambda x: x.startswith("MXRUNTIME_Database"), list(os.environ.keys())): + for key in filter( + lambda x: x.startswith("MXRUNTIME_Database"), + list(os.environ.keys()), + ): del os.environ[key] - def test_no_setup(self): - self.clean_env(); + self.clean_env() try: config = get_database_config() assert config is None except Exception as e: assert "Can't find database configuration" in str(e) - def test_mx_runtime_db_config(self): """ Test is MXRUNTIME variables are set up no database configuration is returned based on DATABASE_URL or VCAP_SERVICES """ - self.clean_env(); + self.clean_env() os.environ["MXRUNTIME_DatabaseType"] = "PostgreSQL" os.environ[ "MXRUNTIME_DatabaseJdbcUrl" @@ -41,18 +39,20 @@ def test_mx_runtime_db_config(self): config = get_database_config() assert config is None - def test_database_url(self): - self.clean_env(); - os.environ["DATABASE_URL"] = "jdbc:postgres://user:secret@host/database" + self.clean_env() + os.environ[ + "DATABASE_URL" + ] = "jdbc:postgres://user:secret@host/database" config = get_database_config() assert config is not None - def test_vcap(self): - self.clean_env(); - os.environ["VCAP_SERVICES"] = """ + self.clean_env() + os.environ[ + "VCAP_SERVICES" + ] = """ { "rds-testfree": [ { From 27ef623760bb6285ad48164fa37e5750b72cd800 Mon Sep 17 00:00:00 2001 From: Chris Wesdorp Date: Wed, 31 Jul 2019 15:20:58 +0200 Subject: [PATCH 4/7] changes on review feedback --- README.md | 11 ++++++++--- lib/database_config.py | 20 +++++++++++--------- lib/metrics.py | 2 +- tests/usecase/test_db_config_options.py | 10 ++++------ 4 files changed, 24 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 04590090..3372ed7d 100644 --- a/README.md +++ b/README.md @@ -54,7 +54,12 @@ In our trial we found the service `elephantsql` which offered the free `turtle` cf bind-service -Note that not all databases are automatically picked up by the buildpack. If `cf push` returns an error like `Could not parse database credentials`, you need to set the `DATABASE_URL` variable manually or set database [Mendix custom runtime variables](https://docs.mendix.com/refguide/custom-settings) to configure a database. Note these need to be prefixed with `MXRUNTIME_`. +Note that not all databases are automatically picked up by the buildpack. If `cf push` returns an error like `Could not parse database credentials`, you need to set the `DATABASE_URL` variable manually or set database [Mendix custom runtime variables](https://docs.mendix.com/refguide/custom-settings) to configure a database. Note these variables need to be prefixed with `MXRUNTIME_`, as per example: + + cf set-env MXRUNTIME_DatabaseType PostgreSQL + cf set-env MXRUNTIME_DatabaseJdbcUrl postgres://host/databasename + cf set-env MXRUNTIME_DatabaseUsername user + cf set-env MXRUNTIME_DatabasePassword password Now we need to push the application once more. @@ -405,8 +410,8 @@ cf restart Contributing ==== -Make sure your code complies with PEP8. Make sure your code is styled using [Black](https://github.com/psf/black). -We enforce this using `flake8` and `black` in our travis CI. +Make sure your code complies with PEP8. Make sure your code is styled using [Black](https://github.com/psf/black). +We enforce this using `flake8` and `black` in our travis CI. This simplest way to use these tools is by installing them as a plugin for your editor; for example in Vim, one can auto-format files with `black` on writing out a buffer, and it will also display `flake8` errors. diff --git a/lib/database_config.py b/lib/database_config.py index dbfba81b..234026fc 100644 --- a/lib/database_config.py +++ b/lib/database_config.py @@ -9,13 +9,15 @@ def get_database_config(development_mode=False): - # the following options are validated to get database credentials - # 1) existence of custom runtime settings Database.... values - # 2) VCAP with database credentials - # 3) existence of DATABASE_URL - # - # In case we find MXRUNTIME_Database.... values we don't interfere and - # return nothing. VCAP or DATABASE_URL return m2ee configuration + """ + the following options are validated to get database credentials + 1) existence of custom runtime settings Database.... values + 2) VCAP with database credentials + 3) existence of DATABASE_URL env var + + In case we find MXRUNTIME_Database.... values we don't interfere and + return nothing. VCAP or DATABASE_URL return m2ee configuration + """ if any( [x.startswith("MXRUNTIME_Database") for x in list(os.environ.keys())] ): @@ -29,10 +31,10 @@ def get_database_config(development_mode=False): if configuration: m2ee_config = configuration.get_m2ee_configuration() - if m2ee_config is not None and "DatabaseType" in m2ee_config: + if m2ee_config and "DatabaseType" in m2ee_config: return m2ee_config - raise Exception( + raise RuntimeError( "Can't find database configuration from environment variables. " "Check README for supported configuration options." ) diff --git a/lib/metrics.py b/lib/metrics.py index af3a28dc..274fc767 100644 --- a/lib/metrics.py +++ b/lib/metrics.py @@ -330,7 +330,7 @@ def _get_db_conn(self): if not self.db: # get_database config may return None or empty db_config = database_config.get_database_config() - if db_config is None or "DatabaseType" not in db_config: + if not db_config or "DatabaseType" not in db_config: raise Exception( "Database not set as VCAP or DATABASE_URL. Check " "documentation to see supported configuration options." diff --git a/tests/usecase/test_db_config_options.py b/tests/usecase/test_db_config_options.py index e4e389aa..b74aa375 100644 --- a/tests/usecase/test_db_config_options.py +++ b/tests/usecase/test_db_config_options.py @@ -1,8 +1,9 @@ import os +import unittest from database_config import get_database_config -class TestDatabaseConfigOptions: +class TestDatabaseConfigOptions(unittest.TestCase): def clean_env(self): """ Setting different environment variables for test in the same process @@ -19,11 +20,8 @@ def clean_env(self): def test_no_setup(self): self.clean_env() - try: - config = get_database_config() - assert config is None - except Exception as e: - assert "Can't find database configuration" in str(e) + with self.assertRaises(RuntimeError): + get_database_config() def test_mx_runtime_db_config(self): """ From 4edf2eb54cf675eae83c1fec583a14e034db4449 Mon Sep 17 00:00:00 2001 From: Chris Wesdorp Date: Wed, 31 Jul 2019 15:30:45 +0200 Subject: [PATCH 5/7] Update test_db_config_options to check at least on entry in returned config --- tests/usecase/test_db_config_options.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/usecase/test_db_config_options.py b/tests/usecase/test_db_config_options.py index b74aa375..f608a2b7 100644 --- a/tests/usecase/test_db_config_options.py +++ b/tests/usecase/test_db_config_options.py @@ -35,7 +35,7 @@ def test_mx_runtime_db_config(self): ] = "postgres://username:password@rdsbroker-testfree-nonprod-1-eu-west-1.asdbjasdg.eu-west-1.rds.amazonaws.com:5432/testdatabase" # noqa E501 config = get_database_config() - assert config is None + assert not config def test_database_url(self): self.clean_env() @@ -44,7 +44,8 @@ def test_database_url(self): ] = "jdbc:postgres://user:secret@host/database" config = get_database_config() - assert config is not None + assert config + assert config["DatabaseType"] == "PostgreSQL" def test_vcap(self): self.clean_env() @@ -80,4 +81,5 @@ def test_vcap(self): """ # noqa config = get_database_config() - assert config is not None + assert config + assert config["DatabaseType"] == "PostgreSQL" From 3c1335f0bef34c3b2e5da2b8a89abb97a3852e1c Mon Sep 17 00:00:00 2001 From: Chris Wesdorp Date: Wed, 31 Jul 2019 16:28:47 +0200 Subject: [PATCH 6/7] Change exception to ValueError as suggested, changing all to domain error will follow later in a different branch --- lib/metrics.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/metrics.py b/lib/metrics.py index 274fc767..05918546 100644 --- a/lib/metrics.py +++ b/lib/metrics.py @@ -331,7 +331,7 @@ def _get_db_conn(self): # get_database config may return None or empty db_config = database_config.get_database_config() if not db_config or "DatabaseType" not in db_config: - raise Exception( + raise ValueError( "Database not set as VCAP or DATABASE_URL. Check " "documentation to see supported configuration options." ) From 5ac55edb8e7e800470b69ade93b9f0c331c5ffe8 Mon Sep 17 00:00:00 2001 From: Chris Wesdorp Date: Wed, 31 Jul 2019 16:31:07 +0200 Subject: [PATCH 7/7] change tests on None --- lib/database_config.py | 4 ++-- start.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/database_config.py b/lib/database_config.py index 234026fc..99b9c390 100644 --- a/lib/database_config.py +++ b/lib/database_config.py @@ -58,10 +58,10 @@ def get_instance(self): # fallback to original configuration url = self.get_database_uri_from_vcap(self.vcap_services) - if url is None and "DATABASE_URL" in os.environ: + if not url and "DATABASE_URL" in os.environ: url = os.environ["DATABASE_URL"] - if url is not None: + if url: return UrlDatabaseConfiguration(url) return None diff --git a/start.py b/start.py index 863807f7..f126f9a0 100755 --- a/start.py +++ b/start.py @@ -673,7 +673,7 @@ def set_runtime_config(metadata, mxruntime_config, vcap_data, m2ee): runtime_db_config = database_config.get_database_config( development_mode=is_development_mode() ) - if runtime_db_config is not None: + if runtime_db_config: mxruntime_config.update(runtime_db_config) mxruntime_config.update(get_filestore_config(m2ee))