Adapt change logging for both PanDomainAuthSettingsRefresher and Publ…

…icSettings
guardian · Sep 11, 2024 · e33dd44 · e33dd44
1 parent 9fcb4ce
commit e33dd44
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 18 deletions.
diff --git a/...domain-auth-core/src/main/scala/com/gu/pandomainauth/PanDomainAuthSettingsRefresher.scala b/...domain-auth-core/src/main/scala/com/gu/pandomainauth/PanDomainAuthSettingsRefresher.scala
@@ -39,12 +39,7 @@ class PanDomainAuthSettingsRefresher(
   private val settingsRefresher = new Settings.Refresher[PanDomainAuthSettings](
     new Settings.Loader(s3BucketLoader, settingsFileKey),
     PanDomainAuthSettings.apply,
-    (o, n) => {
-      for (change <- CryptoConf.Change.compare(o.signingAndVerification, n.signingAndVerification)) {
-        val message = s"PanDomainAuthSettings have changed for $domain: ${change.summary}"
-        if (change.isBreakingChange) logger.warn(message) else logger.info(message)
-      }
-    },
+    _.signingAndVerification,
     scheduler
   )
   settingsRefresher.start(1)

diff --git a/pan-domain-auth-verification/src/main/scala/com/gu/pandomainauth/PublicSettings.scala b/pan-domain-auth-verification/src/main/scala/com/gu/pandomainauth/PublicSettings.scala
@@ -25,12 +25,7 @@ class PublicSettings(loader: Settings.Loader, scheduler: ScheduledExecutorServic
   private val settingsRefresher = new Settings.Refresher[Verification](
     loader,
     CryptoConf.SettingsReader(_).verificationConf,
-    (o, n) => {
-//      for (change <- CryptoConf.Change.compare(o, n)) {
-//        val message = s"PanDomainAuthSettings have changed for $domain: ${change.summary}"
-//        if (change.isBreakingChange) logger.warn(message) else logger.info(message)
-//      }
-    },
+    identity,
     scheduler
   )
 

diff --git a/pan-domain-auth-verification/src/main/scala/com/gu/pandomainauth/Settings.scala b/pan-domain-auth-verification/src/main/scala/com/gu/pandomainauth/Settings.scala
@@ -2,6 +2,8 @@ package com.gu.pandomainauth
 
 import com.amazonaws.util.IOUtils
 import com.gu.pandomainauth.SettingsFailure.SettingsResult
+import com.gu.pandomainauth.service.CryptoConf
+import com.gu.pandomainauth.service.CryptoConf.Verification
 import org.slf4j.{Logger, LoggerFactory}
 
 import java.io.ByteArrayInputStream
@@ -83,7 +85,7 @@ object Settings {
   class Refresher[A](
     loader: Settings.Loader,
     settingsParser: Map[String, String] => SettingsResult[A],
-    changeMonitor: (A, A) => Unit,
+    verificationIn: A => Verification,
     scheduler: ScheduledExecutorService = Executors.newScheduledThreadPool(1)
   ) {
     // This is deliberately designed to throw an exception during construction if we cannot immediately read the settings
@@ -101,9 +103,9 @@ object Settings {
     private def refresh(): Unit = loadAndParseSettings() match {
       case Right(newSettings) =>
         val oldSettings = store.getAndSet(newSettings)
-        if (oldSettings != newSettings) {
-          logger.info("Updated pan-domain settings")
-          changeMonitor(oldSettings, newSettings)
+        for (change <- CryptoConf.Change.compare(verificationIn(oldSettings), verificationIn(newSettings))) {
+          val message = s"Panda settings changed: ${change.summary}"
+          if (change.isBreakingChange) logger.warn(message) else logger.info(message)
         }
       case Left(err) =>
         logger.error("Failed to update pan-domain settings for $domain")

diff --git a/pan-domain-auth-verification/src/main/scala/com/gu/pandomainauth/service/CryptoConf.scala b/pan-domain-auth-verification/src/main/scala/com/gu/pandomainauth/service/CryptoConf.scala
@@ -80,9 +80,9 @@ object CryptoConf {
   }
 
   object Change {
-    def compare(oldConf: SigningAndVerification, newConf: SigningAndVerification): Option[CryptoConf.Change] =
+    def compare(oldConf: Verification, newConf: Verification): Option[CryptoConf.Change] =
       Option.when(newConf != oldConf)(Change(
-        activeKey = Option.when(newConf.activeKeyPair != oldConf.activeKeyPair)(ActiveKey(
+        activeKey = Option.when(newConf.activePublicKey != oldConf.activePublicKey)(ActiveKey(
           toleratingOldKey = newConf.acceptsActiveKeyFrom(oldConf),
           newKeyAlreadyAccepted = oldConf.acceptsActiveKeyFrom(newConf)
         )),