Merge lp:~silverdrake11/landscape-charm/add_store_password into lp:landscape-charm

[Kevin Nasto]

Added missing store password

Manual testing instructions:
charmcraft pack
juju deploy ./bundle.yaml
juju ssh landscape-server/0
sudo cat /etc/landscape/service.conf

Take a look at the stores password in the service.conf. Then run:
juju config landscape-server db_schema_password=helloworld

And then look at it again:
juju ssh landscape-server/0
sudo cat /etc/landscape/service.conf

[Mitch Burton (mitchburton)]

review: Needs Fixing

[lp:~silverdrake11/landscape-charm/add_store_password]

437. By Kevin Nasto on 2023-03-14

Changes to variable naming/desc and added test cases

[lp:~silverdrake11/landscape-charm/add_store_password]

438. By Kevin Nasto on 2023-03-14

Fixed code in on_config_changed

[Mitch Burton (mitchburton)]

+1 LGTM

review: Approve

=== modified file 'config.yaml'
--- config.yaml	2023-03-10 23:21:07 +0000
+++ config.yaml	2023-03-14 21:53:56 +0000
@@ -145,21 +145,30 @@
     default:
     description: |
       Optionally specify the host instead of getting it from the postgres charm
-  db_password:
+  db_landscape_password:
     type: string
     default:
-    description: Optionally specify db password
+    description: |
+      Password for landscape user which does normal read/write operations
   db_port:
     type: string
     default:
     description: |
       Optionally specify the db port instead of getting it from the postgres
       charm. Falls back to the default postgres port
-  db_user:
-    type: string
-    default:
-    description: |
-      Optionally specify the postgres user instead of getting it from the charm
+  db_schema_user:
+    type: string
+    default:
+    description: |
+      Database admin user to perform schema checks and migrations. If not
+      provided, the value from the postgres charm is used
+  db_schema_password:
+    type: string
+    default:
+    description: |
+      Password used by database admin to perform schema checks and 
+      migrations. If not set, postgres charm value, followed by
+      db_landscape_password is used.
   deployment_mode:
     type: string
     default: standalone

=== modified file 'src/charm.py'
--- src/charm.py	2023-03-13 21:37:24 +0000
+++ src/charm.py	2023-03-14 21:53:56 +0000
@@ -204,18 +204,21 @@
         self._bootstrap_account()
 
         config_host = self.model.config.get("db_host")
-        config_password = self.model.config.get("db_password")
+        schema_password = self.model.config.get("db_schema_password")
+        landscape_password = self.model.config.get("db_landscape_password")
         config_port = self.model.config.get("db_port")
-        config_user = self.model.config.get("db_user")
+        config_user = self.model.config.get("db_schema_user")
         db_kargs = {}
         if config_host:
             db_kargs["host"] = config_host
-        if config_password:
-            db_kargs["password"] = config_password
+        if schema_password:
+            db_kargs["schema_password"] = schema_password
         if config_port:
             db_kargs["port"] = config_port
         if config_user:
             db_kargs["user"] = config_user
+        if landscape_password:
+            db_kargs["password"] = landscape_password
         if db_kargs:
             update_db_conf(**db_kargs)
             if self._migrate_schema_bootstrap():
@@ -375,12 +378,14 @@
         else:
             host = master["host"]
 
-        config_password = self.model.config.get("db_password")
-        if config_password:
-            password = config_password
+        landscape_password = self.model.config.get("db_landscape_password")
+        if landscape_password:
+            password = landscape_password
         else:
             password = master["password"]
 
+        schema_password = self.model.config.get("db_schema_password")
+
         config_port = self.model.config.get("db_port")
         if config_port:
             port = config_port
@@ -389,13 +394,14 @@
         if not port:
             port = DEFAULT_POSTGRES_PORT  # Fall back to postgres default port if still not set
 
-        config_user = self.model.config.get("db_user")
+        config_user = self.model.config.get("db_schema_user")
         if config_user:
             user = config_user
         else:
             user = unit_data["user"]
 
-        update_db_conf(host=host, port=port, user=user, password=password)
+        update_db_conf(host=host, port=port, user=user, password=password,
+                       schema_password=schema_password)
 
         if not self._migrate_schema_bootstrap():
             return

=== modified file 'src/settings_files.py'
--- src/settings_files.py	2023-03-10 23:21:07 +0000
+++ src/settings_files.py	2023-03-14 21:53:56 +0000
@@ -161,7 +161,8 @@
             "Unable to decode b64-encoded SSL certificate")
 
 
-def update_db_conf(host=None, password=None, port=DEFAULT_POSTGRES_PORT, user=None):
+def update_db_conf(host=None, password=None, schema_password=None, port=DEFAULT_POSTGRES_PORT,
+                   user=None):
     """Postgres specific settings override"""
     to_update = defaultdict(dict)
     if host:  # Note that host is required if port is changed
@@ -169,6 +170,8 @@
     if password:
         to_update["stores"]["password"] = password
         to_update["schema"]["store_password"] = password
+    if schema_password:  # Overrides password
+        to_update["schema"]["store_password"] = schema_password
     if user:
         to_update["schema"]["store_user"] = user
     if to_update:

=== modified file 'tests/test_charm.py'
--- tests/test_charm.py	2023-03-13 21:37:24 +0000
+++ tests/test_charm.py	2023-03-14 21:53:56 +0000
@@ -333,8 +333,8 @@
             {
                 "db_host": "hello",
                 "db_port": "world",
-                "db_user": "test",
-                "db_password": "test_pass",
+                "db_schema_user": "test",
+                "db_landscape_password": "test_pass",
             }
         )
         mock_event = Mock()
@@ -368,6 +368,51 @@
             }
         )
 
+    def test_db_manual_configs_password(self):
+        '''
+        Test specifying both passwords in the juju config
+        '''
+        self.harness.disable_hooks()
+        self.harness.update_config(
+            {
+                "db_host": "hello",
+                "db_port": "world",
+                "db_schema_user": "test",
+                "db_landscape_password": "test_pass",
+                "db_schema_password": "schema_pass",
+            }
+        )
+        mock_event = Mock()
+        mock_event.relation.data = {
+            mock_event.unit: {
+                "allowed-units": self.harness.charm.unit.name,
+                "master": "host=1.2.3.4 password=testpass",
+                "host": "1.2.3.4",
+                "port": "5678",
+                "user": "testuser",
+                "password": "testpass",
+            },
+        }
+
+        with patch("charm.check_call") as check_call_mock:
+            with patch(
+                "settings_files.update_service_conf"
+            ) as update_service_conf_mock:
+                self.harness.charm._db_relation_changed(mock_event)
+
+        update_service_conf_mock.assert_called_once_with(
+            {
+                "stores": {
+                    "host": "hello:world",
+                    "password": "test_pass",
+                },
+                "schema": {
+                    "store_user": "test",
+                    "store_password": "schema_pass",
+                },
+            }
+        )
+
     def test_db_manual_configs_used_partial(self):
         """
         Test that if some of the manual configs are provided, the rest are