Merge ~andersson123/autopkgtest-cloud:configparser-read-refactor into autopkgtest-cloud:master

[Tim Andersson]

[Tim Andersson (andersson123)]

Amended now, unit tests are passing :)

[Skia (hyask)]

Looking good, only a few adjustments here and there.

review: Needs Fixing

[Tim Andersson (andersson123)]

amended and ready for re-review.

[Skia (hyask)]

review: Needs Fixing

[Tim Andersson (andersson123)]

amended and ready for re-review

[Skia (hyask)]

review: Needs Fixing

[Tim Andersson (andersson123)]

mb, I thought that syntax was correct, ready for re-review

[Tim Andersson (andersson123)]

hm, I need to fix a merge conflict too

[Tim Andersson (andersson123)]

merge conflict amended, ready for re-review

[Skia (hyask)]

All good! :-)
Thanks

review: Approve

diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/copy-swift-results b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/copy-swift-results
index ba8a40e..9484539 100755
--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/copy-swift-results
+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/copy-swift-results
@@ -48,7 +48,8 @@ releases = "noble mantic jammy focal lunar"
 
 
 conf = configparser.ConfigParser()
-conf.read(creds_file)
+with open(creds_file, "r") as f:
+    conf.read_file(f)
 
 swift_creds = {
     "old": {
diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker/worker b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker/worker
index 6424ef4..3511171 100755
--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker/worker
+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker/worker
@@ -1529,7 +1529,8 @@ def main():
         },
         allow_no_value=True,
     )
-    cfg.read(args.config)
+    with open(args.config, "r") as f:
+        cfg.read_file(f)
 
     logging.basicConfig(
         level=(args.debug and logging.DEBUG or logging.INFO),
diff --git a/charms/focal/autopkgtest-web/webcontrol/amqp-status-collector b/charms/focal/autopkgtest-web/webcontrol/amqp-status-collector
index a61a949..1169f2e 100755
--- a/charms/focal/autopkgtest-web/webcontrol/amqp-status-collector
+++ b/charms/focal/autopkgtest-web/webcontrol/amqp-status-collector
@@ -2,7 +2,6 @@
 # Pick up running tests, their status and logtail from the "teststatus" fanout
 # queue, and regularly write it into /run/amqp-status-collector/running.json
 
-import configparser
 import json
 import logging
 import os
@@ -11,6 +10,7 @@ import time
 import urllib.parse
 
 import amqplib.client_0_8 as amqp
+from helpers.utils import get_autopkgtest_cloud_conf
 
 exchange_name = "teststatus.fanout"
 running_name = os.path.join(
@@ -26,8 +26,7 @@ last_update = 0
 def amqp_connect():
     """Connect to AMQP server"""
 
-    cp = configparser.ConfigParser()
-    cp.read(os.path.expanduser("~ubuntu/autopkgtest-cloud.conf"))
+    cp = get_autopkgtest_cloud_conf()
     amqp_uri = cp["amqp"]["uri"]
     parts = urllib.parse.urlsplit(amqp_uri, allow_fragments=False)
     amqp_con = amqp.Connection(
diff --git a/charms/focal/autopkgtest-web/webcontrol/browse.cgi b/charms/focal/autopkgtest-web/webcontrol/browse.cgi
index a1c87d9..47eb8a0 100755
--- a/charms/focal/autopkgtest-web/webcontrol/browse.cgi
+++ b/charms/focal/autopkgtest-web/webcontrol/browse.cgi
@@ -2,7 +2,6 @@
 
 """Browse autopkgtest results"""
 
-import configparser
 import json
 import os
 import re
@@ -13,7 +12,12 @@ from wsgiref.handlers import CGIHandler
 import flask
 from helpers.admin import select_abnormally_long_jobs
 from helpers.exceptions import RunningJSONNotFound
-from helpers.utils import get_all_releases, get_supported_releases, setup_key
+from helpers.utils import (
+    get_all_releases,
+    get_autopkgtest_cloud_conf,
+    get_supported_releases,
+    setup_key,
+)
 from werkzeug.middleware.proxy_fix import ProxyFix
 
 # Initialize app
@@ -43,8 +47,7 @@ RUNNING_CACHE = "/run/amqp-status-collector/running.json"
 def init_config():
     global db_con, swift_container_url, INDEXED_PACKAGES_FP
 
-    cp = configparser.ConfigParser()
-    cp.read(os.path.expanduser("~ubuntu/autopkgtest-cloud.conf"))
+    cp = get_autopkgtest_cloud_conf()
 
     db_con = sqlite3.connect(
         "file:%s?mode=ro" % cp["web"]["database_ro"],
diff --git a/charms/focal/autopkgtest-web/webcontrol/cache-amqp b/charms/focal/autopkgtest-web/webcontrol/cache-amqp
index d8cd653..adfd4df 100755
--- a/charms/focal/autopkgtest-web/webcontrol/cache-amqp
+++ b/charms/focal/autopkgtest-web/webcontrol/cache-amqp
@@ -1,7 +1,6 @@
 #!/usr/bin/python3
 
 import argparse
-import configparser
 import json
 import logging
 import os
@@ -14,6 +13,7 @@ import urllib.request
 
 import amqplib.client_0_8 as amqp
 from amqplib.client_0_8.exceptions import AMQPChannelException
+from helpers.utils import get_autopkgtest_cloud_conf
 
 AMQP_CONTEXTS = ["ubuntu", "huge", "ppa", "upstream"]
 
@@ -236,8 +236,7 @@ if __name__ == "__main__":
 
     args = parser.parse_args()
 
-    cp = configparser.ConfigParser()
-    cp.read(os.path.expanduser("~ubuntu/autopkgtest-cloud.conf"))
+    cp = get_autopkgtest_cloud_conf()
 
     logger = logging.getLogger()
     formatter = logging.Formatter(
diff --git a/charms/focal/autopkgtest-web/webcontrol/db-backup b/charms/focal/autopkgtest-web/webcontrol/db-backup
index 08b8513..b03100b 100755
--- a/charms/focal/autopkgtest-web/webcontrol/db-backup
+++ b/charms/focal/autopkgtest-web/webcontrol/db-backup
@@ -5,7 +5,6 @@ and clears up old backups
 """
 
 import atexit
-import configparser
 import datetime
 import gzip
 import hashlib
@@ -17,7 +16,7 @@ import sys
 from pathlib import Path
 
 import swiftclient
-from helpers.utils import init_db
+from helpers.utils import get_autopkgtest_cloud_conf, init_db
 
 DB_PATH = ""
 DB_NAME = ""
@@ -36,8 +35,7 @@ def db_connect() -> sqlite3.Connection:
     global DB_NAME
     global DB_BACKUP_NAME
     global DB_BACKUP_PATH
-    cp = configparser.ConfigParser()
-    cp.read(os.path.expanduser("~ubuntu/autopkgtest-cloud.conf"))
+    cp = get_autopkgtest_cloud_conf()
     DB_PATH = Path(cp["web"]["database"])
     DB_NAME = DB_PATH.name
     DB_BACKUP_NAME = "%s.bak" % DB_NAME
diff --git a/charms/focal/autopkgtest-web/webcontrol/download-all-results b/charms/focal/autopkgtest-web/webcontrol/download-all-results
index 9a8c8d3..157c052 100755
--- a/charms/focal/autopkgtest-web/webcontrol/download-all-results
+++ b/charms/focal/autopkgtest-web/webcontrol/download-all-results
@@ -26,7 +26,7 @@ import urllib.parse
 import amqplib.client_0_8 as amqp
 import swiftclient
 from distro_info import UbuntuDistroInfo
-from helpers.utils import SqliteWriterConfig
+from helpers.utils import SqliteWriterConfig, get_autopkgtest_cloud_conf
 
 LOGGER = logging.getLogger(__name__)
 SWIFT_CREDS_FILE = "/home/ubuntu/public-swift-creds"
@@ -229,8 +229,7 @@ if __name__ == "__main__":
     )
     releases.sort(key=UbuntuDistroInfo().all.index, reverse=True)
 
-    config = configparser.ConfigParser()
-    config.read(os.path.expanduser("~ubuntu/autopkgtest-cloud.conf"))
+    config = get_autopkgtest_cloud_conf()
     amqp_con = amqp_connect()
     db_con = sqlite3.connect(
         "file:%s%s" % (config["web"]["database"], "?mode=ro"), uri=True
diff --git a/charms/focal/autopkgtest-web/webcontrol/download-results b/charms/focal/autopkgtest-web/webcontrol/download-results
index ac19f39..1297a66 100755
--- a/charms/focal/autopkgtest-web/webcontrol/download-results
+++ b/charms/focal/autopkgtest-web/webcontrol/download-results
@@ -1,6 +1,5 @@
 #!/usr/bin/python3
 
-import configparser
 import datetime
 import json
 import logging
@@ -11,7 +10,7 @@ import time
 import urllib.parse
 
 import amqplib.client_0_8 as amqp
-from helpers.utils import SqliteWriterConfig
+from helpers.utils import SqliteWriterConfig, get_autopkgtest_cloud_conf
 
 EXCHANGE_NAME = "testcomplete.fanout"
 amqp_con = None
@@ -20,8 +19,7 @@ amqp_con = None
 def amqp_connect():
     """Connect to AMQP server"""
 
-    cp = configparser.ConfigParser()
-    cp.read(os.path.expanduser("~ubuntu/autopkgtest-cloud.conf"))
+    cp = get_autopkgtest_cloud_conf()
     amqp_uri = cp["amqp"]["uri"]
     parts = urllib.parse.urlsplit(amqp_uri, allow_fragments=False)
     amqp_con = amqp.Connection(
diff --git a/charms/focal/autopkgtest-web/webcontrol/helpers/utils.py b/charms/focal/autopkgtest-web/webcontrol/helpers/utils.py
index 9b007d5..9b95cc8 100644
--- a/charms/focal/autopkgtest-web/webcontrol/helpers/utils.py
+++ b/charms/focal/autopkgtest-web/webcontrol/helpers/utils.py
@@ -1,12 +1,16 @@
 """
 utilities for autopkgtest-web webcontrol
 """
+import configparser
+
 # pylint: disable=protected-access
 import logging
 import os
+import pathlib
 import random
 import sqlite3
 import time
+import typing
 
 # introduced in python3.7, we use 3.8
 from dataclasses import dataclass
@@ -34,6 +38,38 @@ class SqliteWriterConfig:
     retry_time_limit = 120  # seconds
 
 
+def read_config_file(
+    filepath: typing.Union[str, pathlib.Path], cfg_key: str = None
+):
+    """
+    Reads a given config file, whether it be a key=value env file or
+    properly formatted config file
+    :param filepath
+        Path to config file. Can be a string or pathlib.Path
+    :type filepath: ``type[str | pathlib.Path]``
+    :param cfg_key
+        Variable only necessary when parsing a key=value env file.
+        indicates the first key to be used e.g. cfg["cfg_key"]["env_file_value"]
+        This is a configparser "section" name.
+    :type cfg_key ``str``
+    """
+    config = configparser.ConfigParser()
+    if cfg_key is None:
+        with open(filepath, "r") as f:
+            config.read_file(f)
+        return config
+    else:
+        with open(filepath, "r") as fp:
+            config.read_string(
+                ("[%s]\n" % cfg_key) + fp.read().replace('"', "")
+            )  # read_string preserves "" quotes
+        return config
+
+
+def get_autopkgtest_cloud_conf():
+    return read_config_file("/home/ubuntu/autopkgtest-cloud.conf")
+
+
 def get_all_releases():
     udi = distro_info.UbuntuDistroInfo()
     return udi.all
diff --git a/charms/focal/autopkgtest-web/webcontrol/indexed-packages b/charms/focal/autopkgtest-web/webcontrol/indexed-packages
index 82502b1..1122bd2 100755
--- a/charms/focal/autopkgtest-web/webcontrol/indexed-packages
+++ b/charms/focal/autopkgtest-web/webcontrol/indexed-packages
@@ -1,10 +1,10 @@
 #!/usr/bin/python3
 
-import configparser
 import json
-import os
 import sqlite3
 
+from helpers.utils import get_autopkgtest_cloud_conf
+
 
 def srchash(src):
     if src.startswith("lib"):
@@ -14,8 +14,7 @@ def srchash(src):
 
 
 if __name__ == "__main__":
-    cp = configparser.ConfigParser()
-    cp.read(os.path.expanduser("~ubuntu/autopkgtest-cloud.conf"))
+    cp = get_autopkgtest_cloud_conf()
     indexed_packages_fp = cp["web"]["indexed_packages_fp"]
 
     db_con = sqlite3.connect(
diff --git a/charms/focal/autopkgtest-web/webcontrol/private_results/app.py b/charms/focal/autopkgtest-web/webcontrol/private_results/app.py
index 0a793bc..36b1fd0 100644
--- a/charms/focal/autopkgtest-web/webcontrol/private_results/app.py
+++ b/charms/focal/autopkgtest-web/webcontrol/private_results/app.py
@@ -1,5 +1,4 @@
 """Test Result Fetcher Flask App"""
-import configparser
 import logging
 import os
 import sys
@@ -15,7 +14,11 @@ from flask import (
     session,
 )
 from flask_openid import OpenID
-from helpers.utils import setup_key
+from helpers.utils import (
+    get_autopkgtest_cloud_conf,
+    read_config_file,
+    setup_key,
+)
 from request.submit import Submit
 from werkzeug.middleware.proxy_fix import ProxyFix
 
@@ -94,11 +97,11 @@ secret_path = os.path.join(PATH, "secret_key")
 setup_key(app, secret_path)
 oid = OpenID(app, os.path.join(PATH, "openid"), safe_roots=[])
 # Load configuration
-cfg = configparser.ConfigParser()
-cfg.read(os.path.expanduser("~ubuntu/swift-web-credentials.conf"))
+cfg = read_config_file(
+    os.path.expanduser("~ubuntu/swift-web-credentials.conf")
+)
 # The web configuration as well
-cfg_web = configparser.ConfigParser()
-cfg_web.read(os.path.expanduser("~ubuntu/autopkgtest-cloud.conf"))
+cfg_web = get_autopkgtest_cloud_conf()
 # Build swift credentials
 auth_url = cfg.get("swift", "auth_url")
 if "/v2.0" in auth_url:
diff --git a/charms/focal/autopkgtest-web/webcontrol/publish-db b/charms/focal/autopkgtest-web/webcontrol/publish-db
index ef4d1ef..03c8b10 100755
--- a/charms/focal/autopkgtest-web/webcontrol/publish-db
+++ b/charms/focal/autopkgtest-web/webcontrol/publish-db
@@ -6,7 +6,6 @@ This is being used for statistics, and is used by britney and other
 external monitoring tools.
 """
 
-import configparser
 import gzip
 import logging
 import os
@@ -15,6 +14,7 @@ import tempfile
 import urllib.request
 
 import apt_pkg
+from helpers.utils import get_autopkgtest_cloud_conf
 
 config = None
 db_con = None
@@ -185,8 +185,7 @@ if __name__ == "__main__":
     if "DEBUG" in os.environ:
         logging.basicConfig(level=logging.DEBUG)
 
-    config = configparser.ConfigParser()
-    config.read(os.path.expanduser("~ubuntu/autopkgtest-cloud.conf"))
+    config = get_autopkgtest_cloud_conf()
 
     target = config["web"]["database_ro"]
     target_new = "{}.new".format(target)
diff --git a/charms/focal/autopkgtest-web/webcontrol/request/submit.py b/charms/focal/autopkgtest-web/webcontrol/request/submit.py
index d2e5b0d..30caaa5 100644
--- a/charms/focal/autopkgtest-web/webcontrol/request/submit.py
+++ b/charms/focal/autopkgtest-web/webcontrol/request/submit.py
@@ -4,7 +4,6 @@ Author: Martin Pitt <[e-mail]>
 """
 
 import base64
-import configparser
 import json
 import logging
 import os
@@ -26,6 +25,7 @@ from helpers.exceptions import (
     RequestInQueue,
     RequestRunning,
 )
+from helpers.utils import get_autopkgtest_cloud_conf
 
 # Launchpad REST API base
 LP = "https://api.launchpad.net/1.0/"
@@ -56,8 +56,7 @@ RUNNING_FP = "/run/amqp-status-collector/running.json"
 
 class Submit:
     def __init__(self):
-        cp = configparser.ConfigParser()
-        cp.read(os.path.expanduser("~ubuntu/autopkgtest-cloud.conf"))
+        cp = get_autopkgtest_cloud_conf()
 
         # read valid releases and architectures from DB
         self.db_con = sqlite3.connect(
diff --git a/charms/focal/autopkgtest-web/webcontrol/request/tests/test_submit.py b/charms/focal/autopkgtest-web/webcontrol/request/tests/test_submit.py
index 4e9e1f7..5fefef5 100644
--- a/charms/focal/autopkgtest-web/webcontrol/request/tests/test_submit.py
+++ b/charms/focal/autopkgtest-web/webcontrol/request/tests/test_submit.py
@@ -21,9 +21,18 @@ from helpers.exceptions import (
 class SubmitTestBase(TestCase):
     """Common setup of tests of Submit class"""
 
-    @patch("request.submit.configparser.ConfigParser")
     @patch("request.submit.sqlite3")
-    def setUp(self, mock_sqlite, mock_configparser):
+    @patch(
+        "request.submit.get_autopkgtest_cloud_conf",
+        MagicMock(
+            return_value={
+                "amqp": {"uri": "amqp://user:s3kr1t@1.2.3.4"},
+                "web": {"database": "/ignored", "database_ro": "/ignored"},
+                "autopkgtest": {"releases": "testy grumpy"},
+            }
+        ),
+    )
+    def setUp(self, mock_sqlite):
         test_db = sqlite3.connect(":memory:")
         test_db.execute(
             "CREATE TABLE test ("
@@ -44,15 +53,6 @@ class SubmitTestBase(TestCase):
         test_db.commit()
         mock_sqlite.connect.return_value = test_db
 
-        # mock config values
-        cfg = {
-            "amqp": {"uri": "amqp://user:s3kr1t@1.2.3.4"},
-            "web": {"database": "/ignored", "database_ro": "/ignored"},
-            "autopkgtest": {"releases": "testy grumpy"},
-        }
-        mock_configparser.return_value = MagicMock()
-        mock_configparser.return_value.__getitem__.side_effect = cfg.get
-
         self.submit = request.submit.Submit()
         self.submit.releases.add("testy")
 
diff --git a/charms/focal/autopkgtest-web/webcontrol/update-github-jobs b/charms/focal/autopkgtest-web/webcontrol/update-github-jobs
index 9aeb8e7..4308183 100755
--- a/charms/focal/autopkgtest-web/webcontrol/update-github-jobs
+++ b/charms/focal/autopkgtest-web/webcontrol/update-github-jobs
@@ -1,6 +1,5 @@
 #!/usr/bin/python3
 
-import configparser
 import datetime
 import io
 import json
@@ -12,6 +11,7 @@ import tarfile
 from datetime import datetime, timedelta
 
 import swiftclient
+from helpers.utils import get_autopkgtest_cloud_conf, read_config_file
 from request.submit import Submit
 
 PENDING_DIR = "/run/autopkgtest_webcontrol/github-pending"
@@ -216,13 +216,9 @@ if __name__ == "__main__":
         logging.info("%s does not exist, nothing to do", PENDING_DIR)
         sys.exit(0)
 
-    config = configparser.ConfigParser()
-    config.read("/home/ubuntu/autopkgtest-cloud.conf")
+    config = get_autopkgtest_cloud_conf()
     external_url = config["web"]["ExternalURL"]
-    swift_cfg = configparser.ConfigParser()
-    # Nice way of passing an environment-like file to configparser
-    with open(SWIFT_CREDS_FILE, "r") as fp:
-        swift_cfg.read_string("[swift]\n" + fp.read())
+    swift_cfg = read_config_file(filepath=SWIFT_CREDS_FILE, cfg_key="swift")
 
     swift_creds = {
         "authurl": swift_cfg["swift"]["OS_AUTH_URL"],