From 5b1abbb3a89f42ed891b6ce9be0114a2f4368d27 Mon Sep 17 00:00:00 2001
From: David Hammer <dhammer@mailbox.org>
Date: Fri, 1 Oct 2021 11:16:52 +0200
Subject: [PATCH] Load calibration_client secrets from very secret JSON file

---
 README.md                      | 17 ++++++++++++
 src/calng/AgipdCorrection.py   | 10 +++++--
 src/calng/calcat_utils.py      | 49 +++++++++++-----------------------
 src/tests/test_calcat_utils.py | 22 +++------------
 4 files changed, 44 insertions(+), 54 deletions(-)

diff --git a/README.md b/README.md
index 4a09a144..5933bd80 100644
--- a/README.md
+++ b/README.md
@@ -1,3 +1,20 @@
 # calng
 
 calng is a collection of Karabo devices to perform online processing of 2D X-ray detector data at runtime. It is the successor of the calPy package.
+
+# CalCat secrets and deployment
+Correction devices each run their own `calibration_client.CalibrationClient`, so they need to have credentials for CalCat.
+They expect to be able to load these from a JSON file; by default, this will be in `$KARABO/var/data/calibration-client-secrets.json` (`var/data` is CWD of Karabo devices).
+The file should look something like:
+
+```json
+{
+	"base_url": "https://in.xfel.eu/test_calibration",
+	"client_id": "[sort of secret]",
+	"client_secret": "[actual secret]",
+	"user_email": "[eh, not that secret]",
+	"caldb_store_path": "/gpfs/exfel/d/cal/caldb_store"
+}
+```
+
+For deployment, you'll want `/calibration` instead of `/test_calibration` and the caldb store as seen from ONC will be `/common/cal/caldb_store`.
diff --git a/src/calng/AgipdCorrection.py b/src/calng/AgipdCorrection.py
index a9d9e3a4..22b50581 100644
--- a/src/calng/AgipdCorrection.py
+++ b/src/calng/AgipdCorrection.py
@@ -1,3 +1,4 @@
+import pathlib
 import timeit
 
 import numpy as np
@@ -160,7 +161,10 @@ class AgipdCorrection(BaseCorrection):
 
     def __init__(self, config):
         super().__init__(config)
-        self.calibration_constant_manager = AgipdCalcatFriend(self)
+        # TODO: consider putting this initialization in base class
+        self.calibration_constant_manager = AgipdCalcatFriend(
+            self, pathlib.Path.cwd() / "calibration-client-secrets.json"
+        )
         # TODO: different gpu runner for fixed gain mode
         self.gain_mode = AgipdGainMode[config.get("gainMode")]
         self.bad_pixel_mask_value = eval(config.get("corrections.badPixelMaskValue"))
@@ -316,7 +320,9 @@ class AgipdCorrection(BaseCorrection):
         self.flush_constants()
         for constant in AgipdConstants:
             try:
-                metadata, data = self.calibration_constant_manager.get_constant_version(constant)
+                metadata, data = self.calibration_constant_manager.get_constant_version(
+                    constant
+                )
             except Exception as e:
                 self.log.WARN(f"Failed getting {constant} with {e}")
             else:
diff --git a/src/calng/calcat_utils.py b/src/calng/calcat_utils.py
index 2d1997cd..5d4c8c87 100644
--- a/src/calng/calcat_utils.py
+++ b/src/calng/calcat_utils.py
@@ -1,6 +1,7 @@
 import copy
 import enum
 import functools
+import json
 import pathlib
 import threading
 import typing
@@ -101,34 +102,8 @@ class BaseCalcatFriend:
         AgipdCalcatFriend.dark_condition.
         """
 
-        # Settings needed to use calibration client and find constants
         (
             NODE_ELEMENT(schema).key(prefix).commit(),
-            NODE_ELEMENT(schema).key(f"{prefix}.calCat").commit(),
-            STRING_ELEMENT(schema)
-            .key(f"{prefix}.calCat.baseUrl")
-            .assignmentMandatory()
-            .commit(),
-            STRING_ELEMENT(schema)
-            .key(f"{prefix}.calCat.clientId")
-            .assignmentMandatory()
-            .commit(),
-            STRING_ELEMENT(schema)
-            .key(f"{prefix}.calCat.clientSecret")
-            .assignmentMandatory()
-            .commit(),
-            STRING_ELEMENT(schema)
-            .key(f"{prefix}.calCat.userEmail")
-            .assignmentOptional()
-            .defaultValue("")
-            .commit(),
-            STRING_ELEMENT(schema)
-            .key(f"{prefix}.calCat.caldbStore")
-            .displayedName("Location of caldb_store")
-            .assignmentOptional()
-            .defaultValue("/gpfs/exfel/d/cal/caldb_store")
-            .options("/gpfs/exfel/d/cal/caldb_store,/common/cal/caldb_store")
-            .commit(),
         )
 
         # Parameters which any detector would probably have (extend this in subclass)
@@ -174,21 +149,27 @@ class BaseCalcatFriend:
     def __init__(
         self,
         device,
+        secrets_fn: pathlib.Path,
         prefix="constantParameters",
     ):
         self.device = device
         self.prefix = prefix
-        # TODO: can constants be accessed from ONC?
-        self.caldb_store = pathlib.Path(self._get("calCat.caldbStore"))
+
+        if not secrets_fn.is_file():
+            self.device.log.WARN(f"Missing CalCat secrets file (expected {secrets_fn})")
+        with secrets_fn.open("r") as fd:
+            calcat_secrets = json.load(fd)
+
+        self.caldb_store = pathlib.Path(calcat_secrets["caldb_store_path"])
         if not self.caldb_store.is_dir():
             raise ValueError(f"caldb_store location '{self.caldb_store}' is not dir")
 
-        # TODO: secret / token management
-        base_url = self._get("calCat.baseUrl")
+        self.device.log.INFO(f"Connecting to CalCat at {calcat_secrets['base_url']}")
+        base_url = calcat_secrets["base_url"]
         self.client = calibration_client.CalibrationClient(
-            client_id=self._get("calCat.clientId"),
-            client_secret=self._get("calCat.clientSecret"),
-            user_email=self._get("calCat.userEmail"),
+            client_id=calcat_secrets["client_id"],
+            client_secret=calcat_secrets["client_secret"],
+            user_email=calcat_secrets["user_email"],
             base_api_url=f"{base_url}/api/",
             token_url=f"{base_url}/oauth/token",
             refresh_url=f"{base_url}/oauth/token",
@@ -196,7 +177,7 @@ class BaseCalcatFriend:
             scope="public",
             session_token=None,
         )
-        self.device.log.INFO("calibration_client initialized")
+        self.device.log.INFO("CalCat connection established")
 
     def _get(self, key):
         """Helper to get value from attached device schema"""
diff --git a/src/tests/test_calcat_utils.py b/src/tests/test_calcat_utils.py
index 36f2b322..c0c69fbe 100644
--- a/src/tests/test_calcat_utils.py
+++ b/src/tests/test_calcat_utils.py
@@ -6,8 +6,7 @@ from karabo.bound import Hash, Schema
 
 # TODO: secrets management
 _test_dir = pathlib.Path(__file__).absolute().parent
-with (_test_dir / "calibration-client-secrets.txt").open("r") as fd:
-    base_url, client_id, client_secret, user_email = fd.read().splitlines()
+_test_calcat_secrets_fn = _test_dir / "calibration-client-secrets.json"
 
 
 class DummyLogger:
@@ -46,7 +45,7 @@ class DummyAgipdDevice:
         self.schema = config
         self.calibration_constant_manager = calcat_utils.AgipdCalcatFriend(
             self,
-            "constantParameters",
+            _test_calcat_secrets_fn,
         )
 
     def get(self, key):
@@ -62,7 +61,7 @@ class DummyDsscDevice:
     @staticmethod
     def expectedParameters(expected):
         # super(DummyDsscDevice, DummyDsscDevice).expectedParameters(expected)
-        calcat_utils.DsscCalcatFriend.add_schema(expected, "constantParameters")
+        calcat_utils.DsscCalcatFriend.add_schema(expected)
 
     def __init__(self, config):
         self.log = DummyLogger()
@@ -70,7 +69,7 @@ class DummyDsscDevice:
         self.schema = config
         self.calibration_constant_manager = calcat_utils.DsscCalcatFriend(
             self,
-            "constantParameters",
+            _test_calcat_secrets_fn,
         )
 
     def get(self, key):
@@ -83,12 +82,6 @@ DummyDsscDevice.expectedParameters(DummyDsscDevice.device_class_schema)
 def test_agipd_constants_and_caching_and_async():
     # def test_agipd_constants():
     conf = Hash()
-    conf["constantParameters.calCat.baseUrl"] = base_url
-    conf["constantParameters.calCat.clientId"] = client_id
-    conf["constantParameters.calCat.clientSecret"] = client_secret
-    conf["constantParameters.calCat.userEmail"] = user_email
-    conf["constantParameters.calCat.caldbStore"] = "/gpfs/exfel/d/cal/caldb_store"
-
     conf["constantParameters.detectorType"] = "AGIPD-Type"
     conf["constantParameters.detectorName"] = "SPB_DET_AGIPD1M-1"
     conf["constantParameters.karaboDa"] = "AGIPD00"
@@ -145,13 +138,6 @@ def test_agipd_constants_and_caching_and_async():
 
 def test_dssc_constants():
     conf = Hash()
-
-    conf["constantParameters.calCat.baseUrl"] = base_url
-    conf["constantParameters.calCat.clientId"] = client_id
-    conf["constantParameters.calCat.clientSecret"] = client_secret
-    conf["constantParameters.calCat.userEmail"] = user_email
-    conf["constantParameters.calCat.caldbStore"] = "/gpfs/exfel/d/cal/caldb_store"
-
     conf["constantParameters.detectorType"] = "DSSC-Type"
     conf["constantParameters.detectorName"] = "SCS_DET_DSSC1M-1"
     conf["constantParameters.karaboDa"] = "DSSC00"
-- 
GitLab