From 9823fd0faa9d125e4efde275895428f34a7d8e52 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Tue, 22 Aug 2023 14:18:53 +0200 Subject: [PATCH 01/61] nit-pick. --- .../background-worker/src/Wire/BackendNotificationPusher.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/background-worker/src/Wire/BackendNotificationPusher.hs b/services/background-worker/src/Wire/BackendNotificationPusher.hs index f52f165dbbd..88d64463357 100644 --- a/services/background-worker/src/Wire/BackendNotificationPusher.hs +++ b/services/background-worker/src/Wire/BackendNotificationPusher.hs @@ -154,7 +154,7 @@ startPusher consumersRef chan = do ensureConsumers :: IORef (Map Domain (Q.ConsumerTag, MVar ())) -> Q.Channel -> [Domain] -> AppT IO () ensureConsumers consumers chan domains = do - keys' <- Set.fromList . Map.keys <$> readIORef consumers + keys' <- Map.keysSet <$> readIORef consumers let domains' = Set.fromList domains droppedDomains = Set.difference keys' domains' -- Loop over all of the new domains. We can check for existing consumers and add new ones. From a347a753b1b0d3712129ce978779da20a2d76e91 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Tue, 22 Aug 2023 14:27:09 +0200 Subject: [PATCH 02/61] nit-pick. --- .../src/Wire/BackendNotificationPusher.hs | 25 +++++-------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/services/background-worker/src/Wire/BackendNotificationPusher.hs b/services/background-worker/src/Wire/BackendNotificationPusher.hs index 88d64463357..c71cccfa6a8 100644 --- a/services/background-worker/src/Wire/BackendNotificationPusher.hs +++ b/services/background-worker/src/Wire/BackendNotificationPusher.hs @@ -123,7 +123,7 @@ startPusher consumersRef chan = do throwM e -- If this thread is cancelled, catch the exception, kill the consumers, and carry on. - -- FUTUREWORK?: + -- -- If this throws an exception on the Chan / in the forever loop, the exception will -- bubble all the way up and kill the pod. Kubernetes should restart the pod automatically. flip @@ -132,25 +132,12 @@ startPusher consumersRef chan = do Handler $ cleanup @SomeAsyncException ] $ do - -- Get an initial set of domains from the sync thread - -- The Chan that we will be waiting on isn't initialised with a - -- value until the domain update loop runs the callback for the - -- first time. - initRemotes <- liftIO $ readIORef env.remoteDomains - -- Get an initial set of consumers for the domains pulled from the IORef - -- so that we aren't just sitting around not doing anything for a bit at - -- the start. - ensureConsumers consumersRef chan $ domain <$> initRemotes.remotes - -- Wait for updates to the domains, this is where the bulk of the action - -- is going to take place + consumers <- newIORef mempty + BackendNotificationPusherOpts {..} <- asks (.backendNotificationPusher) forever $ do - -- Wait for a new set of domains. This is a blocking action - -- so we will only move past here when we get a new set of domains. - -- It is a bit nicer than having another timeout value, as Brig is - -- already providing one in the domain update message. - chanRemotes <- liftIO $ readChan env.remoteDomainsChan - -- Make new consumers for the new domains, clean up old ones from the consumer map. - ensureConsumers consumersRef chan $ domain <$> chanRemotes.remotes + remoteDomains <- getRemoteDomains + ensureConsumers consumers chan remoteDomains + threadDelay (1_000_000 * remotesRefreshInterval) ensureConsumers :: IORef (Map Domain (Q.ConsumerTag, MVar ())) -> Q.Channel -> [Domain] -> AppT IO () ensureConsumers consumers chan domains = do From 88f202f362b41aac9b9780034344012f3015f30f Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Tue, 22 Aug 2023 15:27:45 +0200 Subject: [PATCH 03/61] Call `getRemoteDomains` in a loop (again). (To pull remotes from rabbitMQ, allowing us to quit talking to brig about this.) --- .../background-worker/src/Wire/BackendNotificationPusher.hs | 4 +--- .../background-worker/src/Wire/BackgroundWorker/Options.hs | 4 +++- .../test/Test/Wire/BackendNotificationPusherSpec.hs | 4 ++-- services/background-worker/test/Test/Wire/Util.hs | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/services/background-worker/src/Wire/BackendNotificationPusher.hs b/services/background-worker/src/Wire/BackendNotificationPusher.hs index c71cccfa6a8..e473d7320ae 100644 --- a/services/background-worker/src/Wire/BackendNotificationPusher.hs +++ b/services/background-worker/src/Wire/BackendNotificationPusher.hs @@ -21,7 +21,6 @@ import System.Logger.Class qualified as Log import UnliftIO import Wire.API.Federation.BackendNotifications import Wire.API.Federation.Client -import Wire.API.Routes.FederationDomainConfig import Wire.BackgroundWorker.Env import Wire.BackgroundWorker.Options import Wire.BackgroundWorker.Util @@ -133,11 +132,10 @@ startPusher consumersRef chan = do ] $ do consumers <- newIORef mempty - BackendNotificationPusherOpts {..} <- asks (.backendNotificationPusher) forever $ do remoteDomains <- getRemoteDomains ensureConsumers consumers chan remoteDomains - threadDelay (1_000_000 * remotesRefreshInterval) + threadDelay (round $ 1_000_000 * fromMaybe 60 env.backendNotificationsConfig.remotesRefreshInterval) ensureConsumers :: IORef (Map Domain (Q.ConsumerTag, MVar ())) -> Q.Channel -> [Domain] -> AppT IO () ensureConsumers consumers chan domains = do diff --git a/services/background-worker/src/Wire/BackgroundWorker/Options.hs b/services/background-worker/src/Wire/BackgroundWorker/Options.hs index 7cac93318db..e012e251bc5 100644 --- a/services/background-worker/src/Wire/BackgroundWorker/Options.hs +++ b/services/background-worker/src/Wire/BackgroundWorker/Options.hs @@ -31,7 +31,9 @@ data BackendNotificationsConfig = BackendNotificationsConfig -- | Upper limit on amount of time (in microseconds) to wait before retrying -- any notification. This exists to ensure that exponential back-off doesn't -- cause wait times to be very big. - pushBackoffMaxWait :: Int + pushBackoffMaxWait :: Int, + -- | Number of seconds between two calls to `getRemoteDomains`. + remotesRefreshInterval :: Maybe Double } deriving (Show, Generic) diff --git a/services/background-worker/test/Test/Wire/BackendNotificationPusherSpec.hs b/services/background-worker/test/Test/Wire/BackendNotificationPusherSpec.hs index d7a275cf285..f4433423c9d 100644 --- a/services/background-worker/test/Test/Wire/BackendNotificationPusherSpec.hs +++ b/services/background-worker/test/Test/Wire/BackendNotificationPusherSpec.hs @@ -232,7 +232,7 @@ spec = do defederationTimeout = responseTimeoutNone galley = Endpoint "localhost" 8085 brig = Endpoint "localhost" 8082 - backendNotificationsConfig = BackendNotificationsConfig 1000 500000 + backendNotificationsConfig = BackendNotificationsConfig 1000 500000 (Just 0.5) backendNotificationMetrics <- mkBackendNotificationMetrics domains <- runAppT Env {..} getRemoteDomains @@ -255,7 +255,7 @@ spec = do defederationTimeout = responseTimeoutNone galley = Endpoint "localhost" 8085 brig = Endpoint "localhost" 8082 - backendNotificationsConfig = BackendNotificationsConfig 1000 500000 + backendNotificationsConfig = BackendNotificationsConfig 1000 500000 (Just 0.5) backendNotificationMetrics <- mkBackendNotificationMetrics domainsThread <- async $ runAppT Env {..} getRemoteDomains diff --git a/services/background-worker/test/Test/Wire/Util.hs b/services/background-worker/test/Test/Wire/Util.hs index d80121fdc69..3fc3c4ad8c4 100644 --- a/services/background-worker/test/Test/Wire/Util.hs +++ b/services/background-worker/test/Test/Wire/Util.hs @@ -30,7 +30,7 @@ testEnv = do galley = Endpoint "localhost" 8085 brig = Endpoint "localhost" 8082 defederationTimeout = responseTimeoutNone - backendNotificationsConfig = BackendNotificationsConfig 1000 500000 + backendNotificationsConfig = BackendNotificationsConfig 1000 500000 (Just 0.5) pure Env {..} runTestAppT :: AppT IO a -> Int -> IO a From 8752761bd598f93626116e9bd4691f377d4c8ff5 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Tue, 22 Aug 2023 15:29:29 +0200 Subject: [PATCH 04/61] Remove dead code. --- services/background-worker/src/Wire/Defederation.hs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/services/background-worker/src/Wire/Defederation.hs b/services/background-worker/src/Wire/Defederation.hs index e8da0e9366b..e51b7a7cd97 100644 --- a/services/background-worker/src/Wire/Defederation.hs +++ b/services/background-worker/src/Wire/Defederation.hs @@ -7,7 +7,6 @@ import Control.Monad.Catch import Control.Retry import Data.Aeson qualified as A import Data.ByteString.Conversion -import Data.Domain import Data.Text (unpack) import Data.Text.Encoding import Imports @@ -21,7 +20,6 @@ import System.Logger.Class qualified as Log import Util.Options import Util.Options qualified as O import Wire.API.Federation.BackendNotifications -import Wire.API.Routes.FederationDomainConfig qualified as Fed import Wire.BackgroundWorker.Env import Wire.BackgroundWorker.Util @@ -59,11 +57,6 @@ mkBrigEnv = do <$> asks httpManager <*> pure (BaseUrl Http (unpack brigHost) (fromIntegral brigPort) "") -getRemoteDomains :: AppT IO [Domain] -getRemoteDomains = do - ref <- asks remoteDomains - fmap Fed.domain . Fed.remotes <$> readIORef ref - callGalleyDelete :: ( MonadReader Env m, MonadMask m, From 3d4ffc1845a97c04e0128fe95dd8f9dbba68d91c Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Tue, 22 Aug 2023 15:45:45 +0200 Subject: [PATCH 05/61] Don't call brig from background-worker. (This was only needed to get federation remote configs, but we get those from rabbitMQ now, and without bugs in allowAll.) --- .../background-worker/background-worker.cabal | 2 -- .../src/Wire/BackgroundWorker.hs | 4 +--- .../src/Wire/BackgroundWorker/Env.hs | 19 ++----------------- .../src/Wire/BackgroundWorker/Options.hs | 1 - .../src/Wire/Defederation.hs | 10 ---------- .../Wire/BackendNotificationPusherSpec.hs | 8 -------- .../background-worker/test/Test/Wire/Util.hs | 5 ----- 7 files changed, 3 insertions(+), 46 deletions(-) diff --git a/services/background-worker/background-worker.cabal b/services/background-worker/background-worker.cabal index 2dc7c897f45..6c1d712efae 100644 --- a/services/background-worker/background-worker.cabal +++ b/services/background-worker/background-worker.cabal @@ -31,7 +31,6 @@ library aeson , amqp , async - , base , bilge , bytestring-conversion , containers @@ -56,7 +55,6 @@ library , types-common , unliftio , wai-utilities - , wire-api , wire-api-federation default-extensions: diff --git a/services/background-worker/src/Wire/BackgroundWorker.hs b/services/background-worker/src/Wire/BackgroundWorker.hs index 117b135aa6b..368365a56f0 100644 --- a/services/background-worker/src/Wire/BackgroundWorker.hs +++ b/services/background-worker/src/Wire/BackgroundWorker.hs @@ -2,7 +2,6 @@ module Wire.BackgroundWorker where -import Control.Concurrent.Async (cancel) import Data.Domain import Data.Map.Strict qualified as Map import Data.Metrics.Servant qualified as Metrics @@ -22,14 +21,13 @@ import Wire.Defederation as Defederation -- FUTUREWORK: Start an http service with status and metrics endpoints run :: Opts -> IO () run opts = do - (env, syncThread) <- mkEnv opts + env <- mkEnv opts (defedChanRef, defedConsumerRef) <- runAppT env $ Defederation.startWorker opts.rabbitmq (notifChanRef, notifConsumersRef) <- runAppT env $ BackendNotificationPusher.startWorker opts.rabbitmq let -- cleanup will run in a new thread when the signal is caught, so we need to use IORefs and -- specific exception types to message threads to clean up l = logger env cleanup = do - cancel syncThread -- Cancel the consumers and wait for them to finish their processing step. -- Defederation thread Log.info (logger env) $ Log.msg (Log.val "Cancelling the defederation thread") diff --git a/services/background-worker/src/Wire/BackgroundWorker/Env.hs b/services/background-worker/src/Wire/BackgroundWorker/Env.hs index 86a5b99ed57..2c9205c3762 100644 --- a/services/background-worker/src/Wire/BackgroundWorker/Env.hs +++ b/services/background-worker/src/Wire/BackgroundWorker/Env.hs @@ -3,8 +3,6 @@ module Wire.BackgroundWorker.Env where -import Control.Concurrent.Async -import Control.Concurrent.Chan import Control.Monad.Base import Control.Monad.Catch import Control.Monad.Trans.Control @@ -24,8 +22,6 @@ import System.Logger qualified as Log import System.Logger.Class (Logger, MonadLogger (..)) import System.Logger.Extended qualified as Log import Util.Options -import Wire.API.FederationUpdate -import Wire.API.Routes.FederationDomainConfig import Wire.BackgroundWorker.Options type IsWorking = Bool @@ -45,10 +41,7 @@ data Env = Env federatorInternal :: Endpoint, httpManager :: Manager, galley :: Endpoint, - brig :: Endpoint, defederationTimeout :: ResponseTimeout, - remoteDomains :: IORef FederationDomainConfigs, - remoteDomainsChan :: Chan FederationDomainConfigs, backendNotificationMetrics :: BackendNotificationMetrics, -- This is needed so that the defederation worker can push -- connection-removed notifications into the notifications channels. @@ -71,12 +64,11 @@ mkBackendNotificationMetrics = <*> register (vector "targetDomain" $ counter $ Prometheus.Info "wire_backend_notifications_errors" "Number of errors that occurred while pushing notifications") <*> register (vector "targetDomain" $ gauge $ Prometheus.Info "wire_backend_notifications_stuck_queues" "Set to 1 when pushing notifications is stuck") -mkEnv :: Opts -> IO (Env, Async ()) +mkEnv :: Opts -> IO Env mkEnv opts = do http2Manager <- initHttp2Manager logger <- Log.mkLogger opts.logLevel Nothing opts.logFormat httpManager <- newManager defaultManagerSettings - remoteDomainsChan <- newChan let federatorInternal = opts.federatorInternal galley = opts.galley defederationTimeout = @@ -84,14 +76,7 @@ mkEnv opts = do responseTimeoutNone (\t -> responseTimeoutMicro $ 1000000 * t) -- seconds to microseconds opts.defederationTimeout - brig = opts.brig rabbitmqVHost = opts.rabbitmq.vHost - callback = - SyncFedDomainConfigsCallback - { fromFedUpdateCallback = \_old new -> do - writeChan remoteDomainsChan new - } - (remoteDomains, syncThread) <- syncFedDomainConfigs brig logger callback rabbitmqAdminClient <- mkRabbitMqAdminClientEnv opts.rabbitmq statuses <- newIORef $ @@ -103,7 +88,7 @@ mkEnv opts = do backendNotificationMetrics <- mkBackendNotificationMetrics notificationChannel <- mkRabbitMqChannelMVar logger $ demoteOpts opts.rabbitmq let backendNotificationsConfig = opts.backendNotificationPusher - pure (Env {..}, syncThread) + pure Env {..} initHttp2Manager :: IO Http2Manager initHttp2Manager = do diff --git a/services/background-worker/src/Wire/BackgroundWorker/Options.hs b/services/background-worker/src/Wire/BackgroundWorker/Options.hs index e012e251bc5..d9e82f714d3 100644 --- a/services/background-worker/src/Wire/BackgroundWorker/Options.hs +++ b/services/background-worker/src/Wire/BackgroundWorker/Options.hs @@ -13,7 +13,6 @@ data Opts = Opts federatorInternal :: !Endpoint, rabbitmq :: !RabbitMqAdminOpts, galley :: !Endpoint, - brig :: !Endpoint, -- | Seconds, Nothing for no timeout defederationTimeout :: Maybe Int, backendNotificationPusher :: BackendNotificationsConfig diff --git a/services/background-worker/src/Wire/Defederation.hs b/services/background-worker/src/Wire/Defederation.hs index e51b7a7cd97..df2d5360e32 100644 --- a/services/background-worker/src/Wire/Defederation.hs +++ b/services/background-worker/src/Wire/Defederation.hs @@ -7,7 +7,6 @@ import Control.Monad.Catch import Control.Retry import Data.Aeson qualified as A import Data.ByteString.Conversion -import Data.Text (unpack) import Data.Text.Encoding import Imports import Network.AMQP qualified as Q @@ -15,9 +14,7 @@ import Network.AMQP.Extended import Network.AMQP.Lifted qualified as QL import Network.HTTP.Client import Network.HTTP.Types -import Servant.Client (BaseUrl (..), ClientEnv, Scheme (Http), mkClientEnv) import System.Logger.Class qualified as Log -import Util.Options import Util.Options qualified as O import Wire.API.Federation.BackendNotifications import Wire.BackgroundWorker.Env @@ -50,13 +47,6 @@ deleteFederationDomainInner' go (msg, envelope) = do Log.msg (Log.val "Failed to delete federation domain") . Log.field "error" err -mkBrigEnv :: AppT IO ClientEnv -mkBrigEnv = do - Endpoint brigHost brigPort <- asks brig - mkClientEnv - <$> asks httpManager - <*> pure (BaseUrl Http (unpack brigHost) (fromIntegral brigPort) "") - callGalleyDelete :: ( MonadReader Env m, MonadMask m, diff --git a/services/background-worker/test/Test/Wire/BackendNotificationPusherSpec.hs b/services/background-worker/test/Test/Wire/BackendNotificationPusherSpec.hs index f4433423c9d..e9186f0cb87 100644 --- a/services/background-worker/test/Test/Wire/BackendNotificationPusherSpec.hs +++ b/services/background-worker/test/Test/Wire/BackendNotificationPusherSpec.hs @@ -4,7 +4,6 @@ module Test.Wire.BackendNotificationPusherSpec where -import Control.Concurrent.Chan import Control.Exception import Control.Monad.Trans.Except import Data.Aeson qualified as Aeson @@ -42,7 +41,6 @@ import Wire.API.Federation.API.Brig import Wire.API.Federation.API.Common import Wire.API.Federation.BackendNotifications import Wire.API.RawJson -import Wire.API.Routes.FederationDomainConfig import Wire.BackendNotificationPusher import Wire.BackgroundWorker.Env import Wire.BackgroundWorker.Options @@ -220,8 +218,6 @@ spec = do ] logger <- Logger.new Logger.defSettings httpManager <- newManager defaultManagerSettings - remoteDomains <- newIORef defFederationDomainConfigs - remoteDomainsChan <- newChan notificationChannel <- newEmptyMVar let federatorInternal = Endpoint "localhost" 8097 http2Manager = undefined @@ -231,7 +227,6 @@ spec = do rabbitmqVHost = "test-vhost" defederationTimeout = responseTimeoutNone galley = Endpoint "localhost" 8085 - brig = Endpoint "localhost" 8082 backendNotificationsConfig = BackendNotificationsConfig 1000 500000 (Just 0.5) backendNotificationMetrics <- mkBackendNotificationMetrics @@ -243,8 +238,6 @@ spec = do mockAdmin <- newMockRabbitMqAdmin True ["backend-notifications.foo.example"] logger <- Logger.new Logger.defSettings httpManager <- newManager defaultManagerSettings - remoteDomains <- newIORef defFederationDomainConfigs - remoteDomainsChan <- newChan notificationChannel <- newEmptyMVar let federatorInternal = Endpoint "localhost" 8097 http2Manager = undefined @@ -254,7 +247,6 @@ spec = do rabbitmqVHost = "test-vhost" defederationTimeout = responseTimeoutNone galley = Endpoint "localhost" 8085 - brig = Endpoint "localhost" 8082 backendNotificationsConfig = BackendNotificationsConfig 1000 500000 (Just 0.5) backendNotificationMetrics <- mkBackendNotificationMetrics domainsThread <- async $ runAppT Env {..} getRemoteDomains diff --git a/services/background-worker/test/Test/Wire/Util.hs b/services/background-worker/test/Test/Wire/Util.hs index 3fc3c4ad8c4..8a16729c588 100644 --- a/services/background-worker/test/Test/Wire/Util.hs +++ b/services/background-worker/test/Test/Wire/Util.hs @@ -2,12 +2,10 @@ module Test.Wire.Util where -import Control.Concurrent.Chan import Imports import Network.HTTP.Client import System.Logger.Class qualified as Logger import Util.Options (Endpoint (..)) -import Wire.API.Routes.FederationDomainConfig import Wire.BackgroundWorker.Env hiding (federatorInternal, galley) import Wire.BackgroundWorker.Env qualified as E import Wire.BackgroundWorker.Options @@ -20,15 +18,12 @@ testEnv = do statuses <- newIORef mempty backendNotificationMetrics <- mkBackendNotificationMetrics httpManager <- newManager defaultManagerSettings - remoteDomains <- newIORef defFederationDomainConfigs - remoteDomainsChan <- newChan notificationChannel <- newEmptyMVar let federatorInternal = Endpoint "localhost" 0 rabbitmqAdminClient = undefined rabbitmqVHost = undefined metrics = undefined galley = Endpoint "localhost" 8085 - brig = Endpoint "localhost" 8082 defederationTimeout = responseTimeoutNone backendNotificationsConfig = BackendNotificationsConfig 1000 500000 (Just 0.5) pure Env {..} From 39987538310f378d80139470dfea19186730314f Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Tue, 22 Aug 2023 15:48:34 +0200 Subject: [PATCH 06/61] Changelog. --- changelog.d/3-bug-fixes/WPB-3796 | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/3-bug-fixes/WPB-3796 diff --git a/changelog.d/3-bug-fixes/WPB-3796 b/changelog.d/3-bug-fixes/WPB-3796 new file mode 100644 index 00000000000..76392180c03 --- /dev/null +++ b/changelog.d/3-bug-fixes/WPB-3796 @@ -0,0 +1 @@ +[background-worker] Get federation remote domains from rabbitMQ, not from brig (this fixes a bug in federation policy `allowAll`) From 06e2f3f3e3666726d4e958557c71ebff9644e95c Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Tue, 22 Aug 2023 15:51:24 +0200 Subject: [PATCH 07/61] make sanitize-pr --- services/background-worker/default.nix | 2 -- 1 file changed, 2 deletions(-) diff --git a/services/background-worker/default.nix b/services/background-worker/default.nix index de2217e45b6..945e4101bb2 100644 --- a/services/background-worker/default.nix +++ b/services/background-worker/default.nix @@ -56,7 +56,6 @@ mkDerivation { aeson amqp async - base bilge bytestring-conversion containers @@ -81,7 +80,6 @@ mkDerivation { types-common unliftio wai-utilities - wire-api wire-api-federation ]; executableHaskellDepends = [ HsOpenSSL imports types-common ]; From 90477cae56ff1b8365a4de5ccf70ca9ddef6c5cc Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Thu, 24 Aug 2023 11:43:21 +0200 Subject: [PATCH 08/61] Make config value required, make it milliseconds and Int. --- .../background-worker/src/Wire/BackendNotificationPusher.hs | 2 +- .../background-worker/src/Wire/BackgroundWorker/Options.hs | 6 ++++-- .../test/Test/Wire/BackendNotificationPusherSpec.hs | 4 ++-- services/background-worker/test/Test/Wire/Util.hs | 2 +- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/services/background-worker/src/Wire/BackendNotificationPusher.hs b/services/background-worker/src/Wire/BackendNotificationPusher.hs index e473d7320ae..0f69b61f4dc 100644 --- a/services/background-worker/src/Wire/BackendNotificationPusher.hs +++ b/services/background-worker/src/Wire/BackendNotificationPusher.hs @@ -135,7 +135,7 @@ startPusher consumersRef chan = do forever $ do remoteDomains <- getRemoteDomains ensureConsumers consumers chan remoteDomains - threadDelay (round $ 1_000_000 * fromMaybe 60 env.backendNotificationsConfig.remotesRefreshInterval) + threadDelay (1_000 * env.backendNotificationsConfig.remotesRefreshInterval) ensureConsumers :: IORef (Map Domain (Q.ConsumerTag, MVar ())) -> Q.Channel -> [Domain] -> AppT IO () ensureConsumers consumers chan domains = do diff --git a/services/background-worker/src/Wire/BackgroundWorker/Options.hs b/services/background-worker/src/Wire/BackgroundWorker/Options.hs index d9e82f714d3..20cc116d164 100644 --- a/services/background-worker/src/Wire/BackgroundWorker/Options.hs +++ b/services/background-worker/src/Wire/BackgroundWorker/Options.hs @@ -31,8 +31,10 @@ data BackendNotificationsConfig = BackendNotificationsConfig -- any notification. This exists to ensure that exponential back-off doesn't -- cause wait times to be very big. pushBackoffMaxWait :: Int, - -- | Number of seconds between two calls to `getRemoteDomains`. - remotesRefreshInterval :: Maybe Double + -- | Number of milliseconds between two calls to `getRemoteDomains`. (good + -- defaults are 3 seconds, or 30 seconds, but for testing we sometimes set + -- it below 1 second.) + remotesRefreshInterval :: Int } deriving (Show, Generic) diff --git a/services/background-worker/test/Test/Wire/BackendNotificationPusherSpec.hs b/services/background-worker/test/Test/Wire/BackendNotificationPusherSpec.hs index e9186f0cb87..66477fc2865 100644 --- a/services/background-worker/test/Test/Wire/BackendNotificationPusherSpec.hs +++ b/services/background-worker/test/Test/Wire/BackendNotificationPusherSpec.hs @@ -227,7 +227,7 @@ spec = do rabbitmqVHost = "test-vhost" defederationTimeout = responseTimeoutNone galley = Endpoint "localhost" 8085 - backendNotificationsConfig = BackendNotificationsConfig 1000 500000 (Just 0.5) + backendNotificationsConfig = BackendNotificationsConfig 1000 500000 500 backendNotificationMetrics <- mkBackendNotificationMetrics domains <- runAppT Env {..} getRemoteDomains @@ -247,7 +247,7 @@ spec = do rabbitmqVHost = "test-vhost" defederationTimeout = responseTimeoutNone galley = Endpoint "localhost" 8085 - backendNotificationsConfig = BackendNotificationsConfig 1000 500000 (Just 0.5) + backendNotificationsConfig = BackendNotificationsConfig 1000 500000 500 backendNotificationMetrics <- mkBackendNotificationMetrics domainsThread <- async $ runAppT Env {..} getRemoteDomains diff --git a/services/background-worker/test/Test/Wire/Util.hs b/services/background-worker/test/Test/Wire/Util.hs index 8a16729c588..eac2f85c6ea 100644 --- a/services/background-worker/test/Test/Wire/Util.hs +++ b/services/background-worker/test/Test/Wire/Util.hs @@ -25,7 +25,7 @@ testEnv = do metrics = undefined galley = Endpoint "localhost" 8085 defederationTimeout = responseTimeoutNone - backendNotificationsConfig = BackendNotificationsConfig 1000 500000 (Just 0.5) + backendNotificationsConfig = BackendNotificationsConfig 1000 500000 500 pure Env {..} runTestAppT :: AppT IO a -> Int -> IO a From 21c5770fc18fe7dbe11066ffb8a838fd803a5445 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Thu, 24 Aug 2023 11:49:50 +0200 Subject: [PATCH 09/61] rename. --- .../background-worker/src/Wire/BackendNotificationPusher.hs | 2 +- services/background-worker/src/Wire/BackgroundWorker/Options.hs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/services/background-worker/src/Wire/BackendNotificationPusher.hs b/services/background-worker/src/Wire/BackendNotificationPusher.hs index 0f69b61f4dc..950e7bd1a61 100644 --- a/services/background-worker/src/Wire/BackendNotificationPusher.hs +++ b/services/background-worker/src/Wire/BackendNotificationPusher.hs @@ -135,7 +135,7 @@ startPusher consumersRef chan = do forever $ do remoteDomains <- getRemoteDomains ensureConsumers consumers chan remoteDomains - threadDelay (1_000 * env.backendNotificationsConfig.remotesRefreshInterval) + threadDelay (1_000 * env.backendNotificationsConfig.remotesRefreshIntervalMs) ensureConsumers :: IORef (Map Domain (Q.ConsumerTag, MVar ())) -> Q.Channel -> [Domain] -> AppT IO () ensureConsumers consumers chan domains = do diff --git a/services/background-worker/src/Wire/BackgroundWorker/Options.hs b/services/background-worker/src/Wire/BackgroundWorker/Options.hs index 20cc116d164..30324961174 100644 --- a/services/background-worker/src/Wire/BackgroundWorker/Options.hs +++ b/services/background-worker/src/Wire/BackgroundWorker/Options.hs @@ -34,7 +34,7 @@ data BackendNotificationsConfig = BackendNotificationsConfig -- | Number of milliseconds between two calls to `getRemoteDomains`. (good -- defaults are 3 seconds, or 30 seconds, but for testing we sometimes set -- it below 1 second.) - remotesRefreshInterval :: Int + remotesRefreshIntervalMs :: Int } deriving (Show, Generic) From ff59df17fecbbbcffe4d65e8f65fc7fb582bc034 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Thu, 24 Aug 2023 12:12:44 +0200 Subject: [PATCH 10/61] Fixups --- changelog.d/0-release-notes/WPB-3796 | 1 + charts/background-worker/values.yaml | 1 + hack/helm_vars/wire-server/values.yaml.gotmpl | 1 + services/background-worker/background-worker.integration.yaml | 1 + 4 files changed, 4 insertions(+) create mode 100644 changelog.d/0-release-notes/WPB-3796 diff --git a/changelog.d/0-release-notes/WPB-3796 b/changelog.d/0-release-notes/WPB-3796 new file mode 100644 index 00000000000..ed53000d5ae --- /dev/null +++ b/changelog.d/0-release-notes/WPB-3796 @@ -0,0 +1 @@ +New config value `background-worker.backendNotificationPusher.remotesRefreshIntervalMs`. If in doubt, set to 30000 ms. (This controls the delay between backend-to-backend notification queue update pulls from rabbitMQ to background-worker.) diff --git a/charts/background-worker/values.yaml b/charts/background-worker/values.yaml index fcae0115bfc..48559c8b2c1 100644 --- a/charts/background-worker/values.yaml +++ b/charts/background-worker/values.yaml @@ -26,6 +26,7 @@ config: backendNotificationPusher: pushBackoffMinWait: 10000 # in microseconds, so 10ms pushBackoffMaxWait: 300000000 # microseconds, so 300s + remotesRefreshIntervalMs: 30000 serviceAccount: # When setting this to 'false', either make sure that a service account named diff --git a/hack/helm_vars/wire-server/values.yaml.gotmpl b/hack/helm_vars/wire-server/values.yaml.gotmpl index 4bd06d953fe..78f1eb6159a 100644 --- a/hack/helm_vars/wire-server/values.yaml.gotmpl +++ b/hack/helm_vars/wire-server/values.yaml.gotmpl @@ -321,6 +321,7 @@ background-worker: backendNotificationPusher: pushBackoffMinWait: 1000 # 1ms pushBackoffMaxWait: 500000 # 0.5s + remotesRefreshIntervalMs: 500 # 0.5s secrets: rabbitmq: username: {{ .Values.rabbitmqUsername }} diff --git a/services/background-worker/background-worker.integration.yaml b/services/background-worker/background-worker.integration.yaml index 9762cc70825..6aae7d20040 100644 --- a/services/background-worker/background-worker.integration.yaml +++ b/services/background-worker/background-worker.integration.yaml @@ -25,3 +25,4 @@ rabbitmq: backendNotificationPusher: pushBackoffMinWait: 1000 pushBackoffMaxWait: 1000000 + remotesRefreshIntervalMs: 500 From 3e2a4c3ae630d9e6aa1ca023035ebbd49c2cc2b1 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Thu, 24 Aug 2023 12:50:57 +0200 Subject: [PATCH 11/61] Fix bug about ensuring rabbitMQ queues and namespaces. (from before this PR) --- .../src/Wire/API/Federation/BackendNotifications.hs | 13 +++++++++---- .../src/Wire/BackendNotificationPusher.hs | 2 +- services/background-worker/src/Wire/Defederation.hs | 4 ++-- services/brig/src/Brig/API/Internal.hs | 4 ++-- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/libs/wire-api-federation/src/Wire/API/Federation/BackendNotifications.hs b/libs/wire-api-federation/src/Wire/API/Federation/BackendNotifications.hs index 3560e2c5e44..3c56ae9964c 100644 --- a/libs/wire-api-federation/src/Wire/API/Federation/BackendNotifications.hs +++ b/libs/wire-api-federation/src/Wire/API/Federation/BackendNotifications.hs @@ -92,14 +92,19 @@ type DefederationDomain = Domain defederationQueue :: Text defederationQueue = "delete-federation" +-- | Create a queue under routingKey (`backend-notifications`). If you want to do something +-- else use `ensureTLQueue`. +ensureB2BQueue :: Q.Channel -> Text -> IO () +ensureB2BQueue chan = ensureTLQueue chan . routingKey + -- | If you ever change this function and modify -- queue parameters, know that it will start failing in the -- next release! So be prepared to write migrations. -ensureQueue :: Q.Channel -> Text -> IO () -ensureQueue chan queue = do +ensureTLQueue :: Q.Channel -> Text -> IO () +ensureTLQueue chan queue = do let opts = Q.QueueOpts - { Q.queueName = routingKey queue, + { Q.queueName = queue, Q.queuePassive = False, Q.queueDurable = True, Q.queueExclusive = False, @@ -169,7 +174,7 @@ instance (KnownComponent c) => RunClient (FedQueueClient c) where -- Empty string means default exchange exchange = "" liftIO $ do - ensureQueue env.channel env.targetDomain._domainText + ensureB2BQueue env.channel env.targetDomain._domainText void $ Q.publishMsg env.channel exchange (routingKey env.targetDomain._domainText) msg pure $ Response diff --git a/services/background-worker/src/Wire/BackendNotificationPusher.hs b/services/background-worker/src/Wire/BackendNotificationPusher.hs index 950e7bd1a61..ea376034b64 100644 --- a/services/background-worker/src/Wire/BackendNotificationPusher.hs +++ b/services/background-worker/src/Wire/BackendNotificationPusher.hs @@ -31,7 +31,7 @@ startPushingNotifications :: Domain -> AppT IO Q.ConsumerTag startPushingNotifications runningFlag chan domain = do - lift $ ensureQueue chan domain._domainText + lift $ ensureB2BQueue chan domain._domainText QL.consumeMsgs chan (routingKey domain._domainText) Q.Ack (void . pushNotification runningFlag domain) pushNotification :: RabbitMQEnvelope e => MVar () -> Domain -> (Q.Message, e) -> AppT IO (Async ()) diff --git a/services/background-worker/src/Wire/Defederation.hs b/services/background-worker/src/Wire/Defederation.hs index df2d5360e32..490c035ac8f 100644 --- a/services/background-worker/src/Wire/Defederation.hs +++ b/services/background-worker/src/Wire/Defederation.hs @@ -22,8 +22,8 @@ import Wire.BackgroundWorker.Util deleteFederationDomain :: MVar () -> Q.Channel -> AppT IO Q.ConsumerTag deleteFederationDomain runningFlag chan = do - lift $ ensureQueue chan defederationQueue - QL.consumeMsgs chan (routingKey defederationQueue) Q.Ack $ deleteFederationDomainInner runningFlag + lift $ ensureTLQueue chan defederationQueue + QL.consumeMsgs chan defederationQueue Q.Ack $ deleteFederationDomainInner runningFlag x3 :: RetryPolicy x3 = limitRetries 3 <> exponentialBackoff 100000 diff --git a/services/brig/src/Brig/API/Internal.hs b/services/brig/src/Brig/API/Internal.hs index cd2393132ee..29783b67d13 100644 --- a/services/brig/src/Brig/API/Internal.hs +++ b/services/brig/src/Brig/API/Internal.hs @@ -347,7 +347,7 @@ deleteFederationRemote dom = do remoteDomains <- fmap domain . remotes <$> getFederationRemotes for_ (env ^. rabbitmqChannel) $ \chan -> liftIO . withMVar chan $ \chan' -> do -- ensureQueue uses routingKey internally - ensureQueue chan' defederationQueue + ensureTLQueue chan' defederationQueue void $ Q.publishMsg chan' "" queue $ Q.newMsg @@ -362,7 +362,7 @@ deleteFederationRemote dom = do -- clean up their conversations and notify clients. -- Just to be safe! for_ (filter (/= dom) remoteDomains) $ \remoteDomain -> do - ensureQueue chan' $ domainText remoteDomain + ensureB2BQueue chan' $ domainText remoteDomain liftIO $ enqueue chan' ownDomain remoteDomain Q.Persistent . void From 07042e7c0d1e97250f44dfb1be85df0636c5836e Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Thu, 24 Aug 2023 13:57:23 +0200 Subject: [PATCH 12/61] Fix integration tests. --- integration/test/Test/Conversation.hs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration/test/Test/Conversation.hs b/integration/test/Test/Conversation.hs index 3d38b417307..868fe47a403 100644 --- a/integration/test/Test/Conversation.hs +++ b/integration/test/Test/Conversation.hs @@ -216,7 +216,7 @@ testDefederationGroupConversation = do -- assert conversation deleted from domainA retryT $ bindResponse (getConversation uA convId) $ \r -> - r.status `shouldMatchInt` 404 + r.status `shouldMatchInt` 422 -- assert federation.delete event is sent twice void $ @@ -295,7 +295,7 @@ testDefederationOneOnOne = do -- assert conversation deleted eventually retryT $ bindResponse (getConversation user convId) $ \r -> - r.status `shouldMatchInt` 404 + r.status `shouldMatchInt` 422 -- assert federation.delete event is sent twice void $ awaitNMatches 2 3 (\n -> nPayload n %. "type" `isEqual` "federation.delete") ws From 7265f2dcf67ad046fc6368f06033a93ef1a96831 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Thu, 24 Aug 2023 14:00:43 +0200 Subject: [PATCH 13/61] FUTUREWORK. --- .../src/Wire/API/Federation/BackendNotifications.hs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libs/wire-api-federation/src/Wire/API/Federation/BackendNotifications.hs b/libs/wire-api-federation/src/Wire/API/Federation/BackendNotifications.hs index 3c56ae9964c..221e763ca74 100644 --- a/libs/wire-api-federation/src/Wire/API/Federation/BackendNotifications.hs +++ b/libs/wire-api-federation/src/Wire/API/Federation/BackendNotifications.hs @@ -89,6 +89,8 @@ routingKey t = "backend-notifications." <> t -- they are stored in Rabbit. type DefederationDomain = Domain +-- | FUTUREWORK: This really doesn't belong here in a module about pushing backend-to-backend +-- notifications. Maybe "Wire.API.Federation.Defederate"? (Same for `ensureTLQueue`.) defederationQueue :: Text defederationQueue = "delete-federation" From cc820b826d9b2c8c93b61df18a8bcf10c0f0216e Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Thu, 24 Aug 2023 14:06:11 +0200 Subject: [PATCH 14/61] TODO --- changelog.d/0-release-notes/WPB-3796 | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/changelog.d/0-release-notes/WPB-3796 b/changelog.d/0-release-notes/WPB-3796 index ed53000d5ae..92be27047f1 100644 --- a/changelog.d/0-release-notes/WPB-3796 +++ b/changelog.d/0-release-notes/WPB-3796 @@ -1 +1,5 @@ New config value `background-worker.backendNotificationPusher.remotesRefreshIntervalMs`. If in doubt, set to 30000 ms. (This controls the delay between backend-to-backend notification queue update pulls from rabbitMQ to background-worker.) + +Remote federation domains in the config file are no longer supported (deprecated since Charts...). Before deploying this release, make sure you have completed the migration steps from [the CHANGELOG of that release](...). + +If you have enabled federation and therefore rabbitMQ before this upgrade, you may need to remove all queues called `backend-notifications.delete-federation` manually, or this release won't start. From 3252b1606b2b290b1d2b356d9ddfe840a61ff799 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 25 Aug 2023 09:42:37 +0200 Subject: [PATCH 15/61] Move BackgroundNotifications module to a more suitlable place. --- .../{Federation/BackendNotifications.hs => BackgroundWorker.hs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename libs/wire-api-federation/src/Wire/API/{Federation/BackendNotifications.hs => BackgroundWorker.hs} (100%) diff --git a/libs/wire-api-federation/src/Wire/API/Federation/BackendNotifications.hs b/libs/wire-api-federation/src/Wire/API/BackgroundWorker.hs similarity index 100% rename from libs/wire-api-federation/src/Wire/API/Federation/BackendNotifications.hs rename to libs/wire-api-federation/src/Wire/API/BackgroundWorker.hs From a6230d38729efd93d806c7dcc79dbae5144e6606 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 25 Aug 2023 09:45:52 +0200 Subject: [PATCH 16/61] Move BackgroundNotifications module (cleanup). --- libs/wire-api-federation/src/Wire/API/BackgroundWorker.hs | 6 ++---- libs/wire-api-federation/src/Wire/API/Federation/API.hs | 2 +- libs/wire-api-federation/wire-api-federation.cabal | 2 +- .../background-worker/src/Wire/BackendNotificationPusher.hs | 2 +- services/background-worker/src/Wire/Defederation.hs | 2 +- .../test/Test/Wire/BackendNotificationPusherSpec.hs | 2 +- .../background-worker/test/Test/Wire/DefederationSpec.hs | 2 +- services/brig/src/Brig/API/Internal.hs | 2 +- services/brig/src/Brig/Federation/Client.hs | 2 +- .../src/Galley/Effects/BackendNotificationQueueAccess.hs | 2 +- .../galley/src/Galley/Intra/BackendNotificationQueue.hs | 2 +- 11 files changed, 12 insertions(+), 14 deletions(-) diff --git a/libs/wire-api-federation/src/Wire/API/BackgroundWorker.hs b/libs/wire-api-federation/src/Wire/API/BackgroundWorker.hs index 221e763ca74..8d6ec98f305 100644 --- a/libs/wire-api-federation/src/Wire/API/BackgroundWorker.hs +++ b/libs/wire-api-federation/src/Wire/API/BackgroundWorker.hs @@ -1,7 +1,7 @@ {-# LANGUAGE GeneralizedNewtypeDeriving #-} {-# LANGUAGE RecordWildCards #-} -module Wire.API.Federation.BackendNotifications where +module Wire.API.BackgroundWorker where import Control.Exception import Control.Monad.Except @@ -84,13 +84,11 @@ enqueue channel originDomain targetDomain deliveryMode (FedQueueClient action) = routingKey :: Text -> Text routingKey t = "backend-notifications." <> t --- Shared values for both brig and background worker so they are +-- | Shared values for both brig and background worker so they are -- kept in sync about what types they are expecting and where -- they are stored in Rabbit. type DefederationDomain = Domain --- | FUTUREWORK: This really doesn't belong here in a module about pushing backend-to-backend --- notifications. Maybe "Wire.API.Federation.Defederate"? (Same for `ensureTLQueue`.) defederationQueue :: Text defederationQueue = "delete-federation" diff --git a/libs/wire-api-federation/src/Wire/API/Federation/API.hs b/libs/wire-api-federation/src/Wire/API/Federation/API.hs index f344a80ced2..6312013ab3c 100644 --- a/libs/wire-api-federation/src/Wire/API/Federation/API.hs +++ b/libs/wire-api-federation/src/Wire/API/Federation/API.hs @@ -42,7 +42,7 @@ import Servant.Client.Core import Wire.API.Federation.API.Brig import Wire.API.Federation.API.Cargohold import Wire.API.Federation.API.Galley -import Wire.API.Federation.BackendNotifications +import Wire.API.BackgroundWorker import Wire.API.Federation.Client import Wire.API.MakesFederatedCall import Wire.API.Routes.Named diff --git a/libs/wire-api-federation/wire-api-federation.cabal b/libs/wire-api-federation/wire-api-federation.cabal index 53862a2f92f..41a29e9d2c5 100644 --- a/libs/wire-api-federation/wire-api-federation.cabal +++ b/libs/wire-api-federation/wire-api-federation.cabal @@ -16,12 +16,12 @@ build-type: Simple library -- cabal-fmt: expand src exposed-modules: + Wire.API.BackgroundWorker Wire.API.Federation.API Wire.API.Federation.API.Brig Wire.API.Federation.API.Cargohold Wire.API.Federation.API.Common Wire.API.Federation.API.Galley - Wire.API.Federation.BackendNotifications Wire.API.Federation.Client Wire.API.Federation.Component Wire.API.Federation.Domain diff --git a/services/background-worker/src/Wire/BackendNotificationPusher.hs b/services/background-worker/src/Wire/BackendNotificationPusher.hs index ea376034b64..4ccb212795b 100644 --- a/services/background-worker/src/Wire/BackendNotificationPusher.hs +++ b/services/background-worker/src/Wire/BackendNotificationPusher.hs @@ -19,7 +19,7 @@ import Network.RabbitMqAdmin import Prometheus import System.Logger.Class qualified as Log import UnliftIO -import Wire.API.Federation.BackendNotifications +import Wire.API.BackgroundWorker import Wire.API.Federation.Client import Wire.BackgroundWorker.Env import Wire.BackgroundWorker.Options diff --git a/services/background-worker/src/Wire/Defederation.hs b/services/background-worker/src/Wire/Defederation.hs index 490c035ac8f..4686a49d0a1 100644 --- a/services/background-worker/src/Wire/Defederation.hs +++ b/services/background-worker/src/Wire/Defederation.hs @@ -16,7 +16,7 @@ import Network.HTTP.Client import Network.HTTP.Types import System.Logger.Class qualified as Log import Util.Options qualified as O -import Wire.API.Federation.BackendNotifications +import Wire.API.BackgroundWorker import Wire.BackgroundWorker.Env import Wire.BackgroundWorker.Util diff --git a/services/background-worker/test/Test/Wire/BackendNotificationPusherSpec.hs b/services/background-worker/test/Test/Wire/BackendNotificationPusherSpec.hs index 66477fc2865..7e2a6cf61c6 100644 --- a/services/background-worker/test/Test/Wire/BackendNotificationPusherSpec.hs +++ b/services/background-worker/test/Test/Wire/BackendNotificationPusherSpec.hs @@ -39,7 +39,7 @@ import Util.Options import Wire.API.Federation.API import Wire.API.Federation.API.Brig import Wire.API.Federation.API.Common -import Wire.API.Federation.BackendNotifications +import Wire.API.BackgroundWorker import Wire.API.RawJson import Wire.BackendNotificationPusher import Wire.BackgroundWorker.Env diff --git a/services/background-worker/test/Test/Wire/DefederationSpec.hs b/services/background-worker/test/Test/Wire/DefederationSpec.hs index 8707414d442..793291ff768 100644 --- a/services/background-worker/test/Test/Wire/DefederationSpec.hs +++ b/services/background-worker/test/Test/Wire/DefederationSpec.hs @@ -9,7 +9,7 @@ import Test.HUnit.Lang import Test.Hspec import Test.Wire.Util import Wire.API.Federation.API.Common -import Wire.API.Federation.BackendNotifications +import Wire.API.BackgroundWorker import Wire.BackgroundWorker.Util import Wire.Defederation diff --git a/services/brig/src/Brig/API/Internal.hs b/services/brig/src/Brig/API/Internal.hs index 29783b67d13..e8dd24e299b 100644 --- a/services/brig/src/Brig/API/Internal.hs +++ b/services/brig/src/Brig/API/Internal.hs @@ -87,7 +87,7 @@ import Wire.API.Connection import Wire.API.Error import Wire.API.Error.Brig qualified as E import Wire.API.Federation.API -import Wire.API.Federation.BackendNotifications +import Wire.API.BackgroundWorker import Wire.API.Federation.Error (FederationError (..)) import Wire.API.MLS.Credential import Wire.API.MLS.KeyPackage diff --git a/services/brig/src/Brig/Federation/Client.hs b/services/brig/src/Brig/Federation/Client.hs index e824b4f53e9..bd7d319dd78 100644 --- a/services/brig/src/Brig/Federation/Client.hs +++ b/services/brig/src/Brig/Federation/Client.hs @@ -41,7 +41,7 @@ import Network.AMQP qualified as Q import System.Logger.Class qualified as Log import Wire.API.Federation.API import Wire.API.Federation.API.Brig as FederatedBrig -import Wire.API.Federation.BackendNotifications +import Wire.API.BackgroundWorker import Wire.API.Federation.Client import Wire.API.Federation.Error import Wire.API.User diff --git a/services/galley/src/Galley/Effects/BackendNotificationQueueAccess.hs b/services/galley/src/Galley/Effects/BackendNotificationQueueAccess.hs index c7ecdfcb771..69bd752882e 100644 --- a/services/galley/src/Galley/Effects/BackendNotificationQueueAccess.hs +++ b/services/galley/src/Galley/Effects/BackendNotificationQueueAccess.hs @@ -6,7 +6,7 @@ import Data.Qualified import Imports import Network.AMQP qualified as Q import Polysemy -import Wire.API.Federation.BackendNotifications +import Wire.API.BackgroundWorker import Wire.API.Federation.Component import Wire.API.Federation.Error diff --git a/services/galley/src/Galley/Intra/BackendNotificationQueue.hs b/services/galley/src/Galley/Intra/BackendNotificationQueue.hs index fb2e02605fc..60a43b97070 100644 --- a/services/galley/src/Galley/Intra/BackendNotificationQueue.hs +++ b/services/galley/src/Galley/Intra/BackendNotificationQueue.hs @@ -17,7 +17,7 @@ import Polysemy import Polysemy.Input import System.Logger.Class qualified as Log import UnliftIO.Timeout (timeout) -import Wire.API.Federation.BackendNotifications +import Wire.API.BackgroundWorker import Wire.API.Federation.Error interpretBackendNotificationQueueAccess :: From de0cabdf45bd4110c70e39a322277389e76763cf Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 25 Aug 2023 10:01:54 +0200 Subject: [PATCH 17/61] Clean up ensureQueue mess. --- .../src/Wire/API/BackgroundWorker.hs | 12 ++++++------ .../src/Wire/BackendNotificationPusher.hs | 4 ++-- services/background-worker/src/Wire/Defederation.hs | 2 +- services/brig/src/Brig/API/Internal.hs | 10 +++------- 4 files changed, 12 insertions(+), 16 deletions(-) diff --git a/libs/wire-api-federation/src/Wire/API/BackgroundWorker.hs b/libs/wire-api-federation/src/Wire/API/BackgroundWorker.hs index 8d6ec98f305..9e646422110 100644 --- a/libs/wire-api-federation/src/Wire/API/BackgroundWorker.hs +++ b/libs/wire-api-federation/src/Wire/API/BackgroundWorker.hs @@ -93,15 +93,15 @@ defederationQueue :: Text defederationQueue = "delete-federation" -- | Create a queue under routingKey (`backend-notifications`). If you want to do something --- else use `ensureTLQueue`. -ensureB2BQueue :: Q.Channel -> Text -> IO () -ensureB2BQueue chan = ensureTLQueue chan . routingKey +-- else use `ensureQueue`. +ensureBackendNotificationsQueue :: Q.Channel -> Text -> IO () +ensureBackendNotificationsQueue chan = ensureQueue chan . routingKey -- | If you ever change this function and modify -- queue parameters, know that it will start failing in the -- next release! So be prepared to write migrations. -ensureTLQueue :: Q.Channel -> Text -> IO () -ensureTLQueue chan queue = do +ensureQueue :: Q.Channel -> Text -> IO () +ensureQueue chan queue = do let opts = Q.QueueOpts { Q.queueName = queue, @@ -174,7 +174,7 @@ instance (KnownComponent c) => RunClient (FedQueueClient c) where -- Empty string means default exchange exchange = "" liftIO $ do - ensureB2BQueue env.channel env.targetDomain._domainText + ensureBackendNotificationsQueue env.channel env.targetDomain._domainText void $ Q.publishMsg env.channel exchange (routingKey env.targetDomain._domainText) msg pure $ Response diff --git a/services/background-worker/src/Wire/BackendNotificationPusher.hs b/services/background-worker/src/Wire/BackendNotificationPusher.hs index 4ccb212795b..0cf04439841 100644 --- a/services/background-worker/src/Wire/BackendNotificationPusher.hs +++ b/services/background-worker/src/Wire/BackendNotificationPusher.hs @@ -31,7 +31,7 @@ startPushingNotifications :: Domain -> AppT IO Q.ConsumerTag startPushingNotifications runningFlag chan domain = do - lift $ ensureB2BQueue chan domain._domainText + lift $ ensureBackendNotificationsQueue chan domain._domainText QL.consumeMsgs chan (routingKey domain._domainText) Q.Ack (void . pushNotification runningFlag domain) pushNotification :: RabbitMQEnvelope e => MVar () -> Domain -> (Q.Message, e) -> AppT IO (Async ()) @@ -135,7 +135,7 @@ startPusher consumersRef chan = do forever $ do remoteDomains <- getRemoteDomains ensureConsumers consumers chan remoteDomains - threadDelay (1_000 * env.backendNotificationsConfig.remotesRefreshIntervalMs) + threadDelay env.backendNotificationsConfig.remotesRefreshInterval ensureConsumers :: IORef (Map Domain (Q.ConsumerTag, MVar ())) -> Q.Channel -> [Domain] -> AppT IO () ensureConsumers consumers chan domains = do diff --git a/services/background-worker/src/Wire/Defederation.hs b/services/background-worker/src/Wire/Defederation.hs index 4686a49d0a1..67d141be841 100644 --- a/services/background-worker/src/Wire/Defederation.hs +++ b/services/background-worker/src/Wire/Defederation.hs @@ -22,7 +22,7 @@ import Wire.BackgroundWorker.Util deleteFederationDomain :: MVar () -> Q.Channel -> AppT IO Q.ConsumerTag deleteFederationDomain runningFlag chan = do - lift $ ensureTLQueue chan defederationQueue + lift $ ensureQueue chan defederationQueue QL.consumeMsgs chan defederationQueue Q.Ack $ deleteFederationDomainInner runningFlag x3 :: RetryPolicy diff --git a/services/brig/src/Brig/API/Internal.hs b/services/brig/src/Brig/API/Internal.hs index e8dd24e299b..bacf1af5f4c 100644 --- a/services/brig/src/Brig/API/Internal.hs +++ b/services/brig/src/Brig/API/Internal.hs @@ -346,10 +346,9 @@ deleteFederationRemote dom = do ownDomain <- viewFederationDomain remoteDomains <- fmap domain . remotes <$> getFederationRemotes for_ (env ^. rabbitmqChannel) $ \chan -> liftIO . withMVar chan $ \chan' -> do - -- ensureQueue uses routingKey internally - ensureTLQueue chan' defederationQueue + ensureQueue chan' defederationQueue void $ - Q.publishMsg chan' "" queue $ + Q.publishMsg chan' "" defederationQueue $ Q.newMsg { -- Check that this message type is compatible with what -- background worker is expecting @@ -362,7 +361,7 @@ deleteFederationRemote dom = do -- clean up their conversations and notify clients. -- Just to be safe! for_ (filter (/= dom) remoteDomains) $ \remoteDomain -> do - ensureB2BQueue chan' $ domainText remoteDomain + ensureBackendNotificationsQueue chan' $ domainText remoteDomain liftIO $ enqueue chan' ownDomain remoteDomain Q.Persistent . void @@ -373,9 +372,6 @@ deleteFederationRemote dom = do -- domain. num <- Q.deleteQueue chan' . routingKey $ domainText dom Lg.info (env ^. applog) $ Log.msg @String "Dropped Notifications" . Log.field "domain" (domainText dom) . Log.field "count" (show num) - where - -- Ensure that this is kept in sync with background worker - queue = routingKey defederationQueue -- | Remove one-on-one conversations for the given remote domain. This is called from Galley as -- part of the defederation process, and should not be called during the initial domain removal From 127728291fcd743ae22382e684a423a6db5b6028 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 25 Aug 2023 10:02:19 +0200 Subject: [PATCH 18/61] Cleanup `remotesRefreshInterval` unit inconsistency. --- charts/background-worker/values.yaml | 2 +- hack/helm_vars/wire-server/values.yaml.gotmpl | 2 +- services/background-worker/background-worker.integration.yaml | 2 +- .../background-worker/src/Wire/BackgroundWorker/Options.hs | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/charts/background-worker/values.yaml b/charts/background-worker/values.yaml index 48559c8b2c1..e85973290c1 100644 --- a/charts/background-worker/values.yaml +++ b/charts/background-worker/values.yaml @@ -26,7 +26,7 @@ config: backendNotificationPusher: pushBackoffMinWait: 10000 # in microseconds, so 10ms pushBackoffMaxWait: 300000000 # microseconds, so 300s - remotesRefreshIntervalMs: 30000 + remotesRefreshInterval: 30000000 # microseconds, so 3s serviceAccount: # When setting this to 'false', either make sure that a service account named diff --git a/hack/helm_vars/wire-server/values.yaml.gotmpl b/hack/helm_vars/wire-server/values.yaml.gotmpl index 78f1eb6159a..844fff9d9c5 100644 --- a/hack/helm_vars/wire-server/values.yaml.gotmpl +++ b/hack/helm_vars/wire-server/values.yaml.gotmpl @@ -321,7 +321,7 @@ background-worker: backendNotificationPusher: pushBackoffMinWait: 1000 # 1ms pushBackoffMaxWait: 500000 # 0.5s - remotesRefreshIntervalMs: 500 # 0.5s + remotesRefreshInterval: 500000 # 0.5s secrets: rabbitmq: username: {{ .Values.rabbitmqUsername }} diff --git a/services/background-worker/background-worker.integration.yaml b/services/background-worker/background-worker.integration.yaml index 6aae7d20040..5fbdac7dfe1 100644 --- a/services/background-worker/background-worker.integration.yaml +++ b/services/background-worker/background-worker.integration.yaml @@ -25,4 +25,4 @@ rabbitmq: backendNotificationPusher: pushBackoffMinWait: 1000 pushBackoffMaxWait: 1000000 - remotesRefreshIntervalMs: 500 + remotesRefreshInterval: 500000 diff --git a/services/background-worker/src/Wire/BackgroundWorker/Options.hs b/services/background-worker/src/Wire/BackgroundWorker/Options.hs index 30324961174..e35c89f1e70 100644 --- a/services/background-worker/src/Wire/BackgroundWorker/Options.hs +++ b/services/background-worker/src/Wire/BackgroundWorker/Options.hs @@ -31,10 +31,10 @@ data BackendNotificationsConfig = BackendNotificationsConfig -- any notification. This exists to ensure that exponential back-off doesn't -- cause wait times to be very big. pushBackoffMaxWait :: Int, - -- | Number of milliseconds between two calls to `getRemoteDomains`. (good + -- | Number of microseconds between two calls to `getRemoteDomains`. (good -- defaults are 3 seconds, or 30 seconds, but for testing we sometimes set -- it below 1 second.) - remotesRefreshIntervalMs :: Int + remotesRefreshInterval :: Int } deriving (Show, Generic) From 883c063b8481c851ce17e64a80d21cc45692eeb8 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 25 Aug 2023 10:02:35 +0200 Subject: [PATCH 19/61] make sanitize-pr --- libs/wire-api-federation/src/Wire/API/Federation/API.hs | 2 +- .../test/Test/Wire/BackendNotificationPusherSpec.hs | 2 +- services/background-worker/test/Test/Wire/DefederationSpec.hs | 2 +- services/brig/src/Brig/API/Internal.hs | 2 +- services/brig/src/Brig/Federation/Client.hs | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/libs/wire-api-federation/src/Wire/API/Federation/API.hs b/libs/wire-api-federation/src/Wire/API/Federation/API.hs index 6312013ab3c..c08aa45d0ed 100644 --- a/libs/wire-api-federation/src/Wire/API/Federation/API.hs +++ b/libs/wire-api-federation/src/Wire/API/Federation/API.hs @@ -39,10 +39,10 @@ import GHC.TypeLits import Imports import Servant.Client import Servant.Client.Core +import Wire.API.BackgroundWorker import Wire.API.Federation.API.Brig import Wire.API.Federation.API.Cargohold import Wire.API.Federation.API.Galley -import Wire.API.BackgroundWorker import Wire.API.Federation.Client import Wire.API.MakesFederatedCall import Wire.API.Routes.Named diff --git a/services/background-worker/test/Test/Wire/BackendNotificationPusherSpec.hs b/services/background-worker/test/Test/Wire/BackendNotificationPusherSpec.hs index 7e2a6cf61c6..2623ca7e05a 100644 --- a/services/background-worker/test/Test/Wire/BackendNotificationPusherSpec.hs +++ b/services/background-worker/test/Test/Wire/BackendNotificationPusherSpec.hs @@ -36,10 +36,10 @@ import Test.QuickCheck import Test.Wire.Util import UnliftIO.Async import Util.Options +import Wire.API.BackgroundWorker import Wire.API.Federation.API import Wire.API.Federation.API.Brig import Wire.API.Federation.API.Common -import Wire.API.BackgroundWorker import Wire.API.RawJson import Wire.BackendNotificationPusher import Wire.BackgroundWorker.Env diff --git a/services/background-worker/test/Test/Wire/DefederationSpec.hs b/services/background-worker/test/Test/Wire/DefederationSpec.hs index 793291ff768..343933d4eb5 100644 --- a/services/background-worker/test/Test/Wire/DefederationSpec.hs +++ b/services/background-worker/test/Test/Wire/DefederationSpec.hs @@ -8,8 +8,8 @@ import Network.AMQP qualified as Q import Test.HUnit.Lang import Test.Hspec import Test.Wire.Util -import Wire.API.Federation.API.Common import Wire.API.BackgroundWorker +import Wire.API.Federation.API.Common import Wire.BackgroundWorker.Util import Wire.Defederation diff --git a/services/brig/src/Brig/API/Internal.hs b/services/brig/src/Brig/API/Internal.hs index bacf1af5f4c..9b0df62b31c 100644 --- a/services/brig/src/Brig/API/Internal.hs +++ b/services/brig/src/Brig/API/Internal.hs @@ -83,11 +83,11 @@ import System.Logger qualified as Lg import System.Logger.Class qualified as Log import System.Random (randomRIO) import UnliftIO.Async +import Wire.API.BackgroundWorker import Wire.API.Connection import Wire.API.Error import Wire.API.Error.Brig qualified as E import Wire.API.Federation.API -import Wire.API.BackgroundWorker import Wire.API.Federation.Error (FederationError (..)) import Wire.API.MLS.Credential import Wire.API.MLS.KeyPackage diff --git a/services/brig/src/Brig/Federation/Client.hs b/services/brig/src/Brig/Federation/Client.hs index bd7d319dd78..693407ae8bc 100644 --- a/services/brig/src/Brig/Federation/Client.hs +++ b/services/brig/src/Brig/Federation/Client.hs @@ -39,9 +39,9 @@ import Data.Time.Units import Imports import Network.AMQP qualified as Q import System.Logger.Class qualified as Log +import Wire.API.BackgroundWorker import Wire.API.Federation.API import Wire.API.Federation.API.Brig as FederatedBrig -import Wire.API.BackgroundWorker import Wire.API.Federation.Client import Wire.API.Federation.Error import Wire.API.User From f76a1c6ed1825a0893dcf20bbc1cb3c2e430ce90 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 25 Aug 2023 11:43:10 +0200 Subject: [PATCH 20/61] Remove seemless-migration-to-database-remotes hack. --- charts/brig/templates/configmap.yaml | 6 -- docs/src/understand/configure-federation.md | 8 ++- hack/helm_vars/wire-server/values.yaml.gotmpl | 15 ----- hack/helmfile.yaml | 8 --- integration/test/SetupHelpers.hs | 1 - integration/test/Test/Brig.hs | 7 ++- integration/test/Test/Conversation.hs | 7 --- integration/test/Test/Defederation.hs | 1 - integration/test/Testlib/ModService.hs | 1 - integration/test/Testlib/Types.hs | 1 - services/brig/brig.integration.yaml | 3 - services/brig/src/Brig/API/Internal.hs | 60 +------------------ services/brig/src/Brig/Options.hs | 8 --- 13 files changed, 11 insertions(+), 115 deletions(-) diff --git a/charts/brig/templates/configmap.yaml b/charts/brig/templates/configmap.yaml index 2ed6eb6833a..a08d359f31c 100644 --- a/charts/brig/templates/configmap.yaml +++ b/charts/brig/templates/configmap.yaml @@ -253,12 +253,6 @@ data: {{- if .setFederationDomainConfigsUpdateFreq }} setFederationDomainConfigsUpdateFreq: {{ .setFederationDomainConfigsUpdateFreq }} {{- end }} - {{- if .setFederationDomainConfigs }} - # 'setFederationDomainConfigs' is deprecated as of https://github.com/wireapp/wire-server/pull/3260. See - # https://docs.wire.com/understand/federation/backend-communication.html#configuring-remote-connections - # for details. - setFederationDomainConfigs: {{ toYaml .setFederationDomainConfigs | nindent 8 }} - {{- end }} {{- if .setSearchSameTeamOnly }} setSearchSameTeamOnly: {{ .setSearchSameTeamOnly }} {{- end }} diff --git a/docs/src/understand/configure-federation.md b/docs/src/understand/configure-federation.md index 3477a10242a..98fa2401a62 100644 --- a/docs/src/understand/configure-federation.md +++ b/docs/src/understand/configure-federation.md @@ -506,9 +506,11 @@ search policy if you create an entry. #### If your instance has been federating before You only need to read this section if your instance has been -federating with other instances prior to -[PR#3260](https://github.com/wireapp/wire-server/pull/3260), and you -are upgrading to the release containing that PR. +federating with other instances prior to [Chart Release +4.36.0](https://github.com/wireapp/wire-server/releases/tag/v2023-08-11), +and you are upgrading to that PR. + +Ignore this section if your release is more recent than 4.38.0.. From now on the federation policy set in the federator config under `federationStrategy` is ignored. Instead, the federation strategy is diff --git a/hack/helm_vars/wire-server/values.yaml.gotmpl b/hack/helm_vars/wire-server/values.yaml.gotmpl index 844fff9d9c5..83b6302fa0e 100644 --- a/hack/helm_vars/wire-server/values.yaml.gotmpl +++ b/hack/helm_vars/wire-server/values.yaml.gotmpl @@ -77,21 +77,6 @@ brig: setMaxConvSize: 16 # See helmfile for the real value setFederationDomain: integration.example.com - setFederationDomainConfigs: - # 'setFederationDomainConfigs' is deprecated as of https://github.com/wireapp/wire-server/pull/3260. See - # https://docs.wire.com/understand/federation/backend-communication.html#configuring-remote-connections - # for details. - - domain: integration.example.com - search_policy: full_search - - domain: federation-test-helper.{{ .Release.Namespace }}.svc.cluster.local - search_policy: full_search - # Remove these after fixing https://wearezeta.atlassian.net/browse/WPB-3796 - - domain: dyn-backend-1 - search_policy: full_search - - domain: dyn-backend-2 - search_policy: full_search - - domain: dyn-backend-3 - search_policy: full_search setFederationStrategy: allowAll setFederationDomainConfigsUpdateFreq: 10 set2FACodeGenerationDelaySecs: 5 diff --git a/hack/helmfile.yaml b/hack/helmfile.yaml index b40cd73d623..3cc832502e2 100644 --- a/hack/helmfile.yaml +++ b/hack/helmfile.yaml @@ -127,14 +127,6 @@ releases: value: {{ .Values.federationDomain1 }} - name: cargohold.config.settings.federationDomain value: {{ .Values.federationDomain1 }} - - name: brig.config.optSettings.setFederationDomainConfigs[0].domain - value: {{ .Values.federationDomain2 }} - - name: brig.config.optSettings.setFederationDomainConfigs[2].domain - value: {{ .Values.dynBackendDomain1 }} - - name: brig.config.optSettings.setFederationDomainConfigs[3].domain - value: {{ .Values.dynBackendDomain2 }} - - name: brig.config.optSettings.setFederationDomainConfigs[4].domain - value: {{ .Values.dynBackendDomain3 }} needs: - 'databases-ephemeral' diff --git a/integration/test/SetupHelpers.hs b/integration/test/SetupHelpers.hs index f6ba6f46768..1bdd4a988fd 100644 --- a/integration/test/SetupHelpers.hs +++ b/integration/test/SetupHelpers.hs @@ -117,7 +117,6 @@ withFederatingBackendsAllowDynamic :: HasCallStack => Int -> ((String, String, S withFederatingBackendsAllowDynamic n k = do let setFederationConfig = setField "optSettings.setFederationStrategy" "allowDynamic" - >=> removeField "optSettings.setFederationDomainConfigs" >=> setField "optSettings.setFederationDomainConfigsUpdateFreq" (Aeson.Number 1) startDynamicBackends [ def {dbBrig = setFederationConfig}, diff --git a/integration/test/Test/Brig.hs b/integration/test/Test/Brig.hs index 3380d8400ca..c18a6a22e6d 100644 --- a/integration/test/Test/Brig.hs +++ b/integration/test/Test/Brig.hs @@ -29,12 +29,14 @@ testSearchContactForExternalUsers = do testCrudFederationRemotes :: HasCallStack => App () testCrudFederationRemotes = do otherDomain <- asString OtherDomain - let overrides = + withModifiedService Brig overrides $ \_ -> do + do + TODO ( setField "optSettings.setFederationDomainConfigs" [object ["domain" .= otherDomain, "search_policy" .= "full_search"]] ) - withModifiedService Brig overrides $ \_ -> do + let parseFedConns :: HasCallStack => Response -> App [Value] parseFedConns resp = -- Pick out the list of federation domain configs @@ -183,7 +185,6 @@ testRemoteUserSearch :: HasCallStack => App () testRemoteUserSearch = do let overrides = setField "optSettings.setFederationStrategy" "allowDynamic" - >=> removeField "optSettings.setFederationDomainConfigs" >=> setField "optSettings.setFederationDomainConfigsUpdateFreq" (Aeson.Number 1) startDynamicBackends [def {dbBrig = overrides}, def {dbBrig = overrides}] $ \dynDomains -> do domains@[d1, d2] <- pure dynDomains diff --git a/integration/test/Test/Conversation.hs b/integration/test/Test/Conversation.hs index 868fe47a403..18be50fd295 100644 --- a/integration/test/Test/Conversation.hs +++ b/integration/test/Test/Conversation.hs @@ -59,7 +59,6 @@ testDynamicBackendsFullyConnectedWhenAllowDynamic :: HasCallStack => App () testDynamicBackendsFullyConnectedWhenAllowDynamic = do let overrides = setField "optSettings.setFederationStrategy" "allowDynamic" - >=> removeField "optSettings.setFederationDomainConfigs" >=> setField "optSettings.setFederationDomainConfigsUpdateFreq" (Aeson.Number 1) startDynamicBackends [ def {dbBrig = overrides}, @@ -88,7 +87,6 @@ testDynamicBackendsNotFullyConnected = do def { dbBrig = setField "optSettings.setFederationStrategy" "allowDynamic" - >=> removeField "optSettings.setFederationDomainConfigs" >=> setField "optSettings.setFederationDomainConfigsUpdateFreq" (Aeson.Number 1) } startDynamicBackends [overrides, overrides, overrides] $ @@ -137,7 +135,6 @@ testCreateConversationFullyConnected :: HasCallStack => App () testCreateConversationFullyConnected = do let setFederationConfig = setField "optSettings.setFederationStrategy" "allowDynamic" - >=> removeField "optSettings.setFederationDomainConfigs" >=> setField "optSettings.setFederationDomainConfigsUpdateFreq" (Aeson.Number 1) startDynamicBackends [ def {dbBrig = setFederationConfig}, @@ -155,7 +152,6 @@ testCreateConversationNonFullyConnected :: HasCallStack => App () testCreateConversationNonFullyConnected = do let setFederationConfig = setField "optSettings.setFederationStrategy" "allowDynamic" - >=> removeField "optSettings.setFederationDomainConfigs" >=> setField "optSettings.setFederationDomainConfigsUpdateFreq" (Aeson.Number 1) startDynamicBackends [ def {dbBrig = setFederationConfig}, @@ -178,7 +174,6 @@ testDefederationGroupConversation :: HasCallStack => App () testDefederationGroupConversation = do let setFederationConfig = setField "optSettings.setFederationStrategy" "allowDynamic" - >=> removeField "optSettings.setFederationDomainConfigs" >=> setField "optSettings.setFederationDomainConfigsUpdateFreq" (Aeson.Number 1) startDynamicBackends [ def {dbBrig = setFederationConfig}, @@ -244,7 +239,6 @@ testDefederationOneOnOne :: HasCallStack => App () testDefederationOneOnOne = do let setFederationConfig = setField "optSettings.setFederationStrategy" "allowDynamic" - >=> removeField "optSettings.setFederationDomainConfigs" >=> setField "optSettings.setFederationDomainConfigsUpdateFreq" (Aeson.Number 1) startDynamicBackends [ def {dbBrig = setFederationConfig}, @@ -414,7 +408,6 @@ testAddingUserNonFullyConnectedFederation = do def { dbBrig = setField "optSettings.setFederationStrategy" "allowDynamic" - >=> removeField "optSettings.setFederationDomainConfigs" } startDynamicBackends [overrides] $ \[dynBackend] -> do own <- asString OwnDomain diff --git a/integration/test/Test/Defederation.hs b/integration/test/Test/Defederation.hs index 73399d96280..99bd854aa95 100644 --- a/integration/test/Test/Defederation.hs +++ b/integration/test/Test/Defederation.hs @@ -30,7 +30,6 @@ testDefederationNonFullyConnectedGraph :: HasCallStack => App () testDefederationNonFullyConnectedGraph = do let setFederationConfig = setField "optSettings.setFederationStrategy" "allowDynamic" - >=> removeField "optSettings.setFederationDomainConfigs" >=> setField "optSettings.setFederationDomainConfigsUpdateFreq" (Aeson.Number 1) startDynamicBackends [ def {dbBrig = setFederationConfig}, diff --git a/integration/test/Testlib/ModService.hs b/integration/test/Testlib/ModService.hs index 44606a6f238..25111078daa 100644 --- a/integration/test/Testlib/ModService.hs +++ b/integration/test/Testlib/ModService.hs @@ -197,7 +197,6 @@ startDynamicBackend resource staticPorts beOverrides = do \case Brig -> setField "optSettings.setFederationDomain" resource.berDomain - >=> setField "optSettings.setFederationDomainConfigs" ([] :: [Value]) >=> setField "federatorInternal.port" resource.berFederatorInternal >=> setField "federatorInternal.host" ("127.0.0.1" :: String) >=> setField "rabbitmq.vHost" resource.berVHost diff --git a/integration/test/Testlib/Types.hs b/integration/test/Testlib/Types.hs index 25ed9c640ed..d5cf7343143 100644 --- a/integration/test/Testlib/Types.hs +++ b/integration/test/Testlib/Types.hs @@ -246,7 +246,6 @@ defaultServiceOverridesToMap = ([minBound .. maxBound] <&> (,pure)) & Map.fromLi -- def -- { dbBrig = -- setField "optSettings.setFederationStrategy" "allowDynamic" --- >=> removeField "optSettings.setFederationDomainConfigs" -- } -- withOverrides overrides defaultServiceOverridesToMap` withOverrides :: ServiceOverrides -> Map.Map Service (Value -> App Value) -> Map.Map Service (Value -> App Value) diff --git a/services/brig/brig.integration.yaml b/services/brig/brig.integration.yaml index 6114e56fa76..448d24b69aa 100644 --- a/services/brig/brig.integration.yaml +++ b/services/brig/brig.integration.yaml @@ -191,9 +191,6 @@ optSettings: setFeatureFlags: # see #RefConfigOptions in `/docs/reference` setFederationDomainConfigsUpdateFreq: 1 setFederationStrategy: allowAll - setFederationDomainConfigs: - - domain: example.com - search_policy: full_search set2FACodeGenerationDelaySecs: 5 setNonceTtlSecs: 5 setDpopMaxSkewSecs: 1 diff --git a/services/brig/src/Brig/API/Internal.hs b/services/brig/src/Brig/API/Internal.hs index 9b0df62b31c..3035a4cc73f 100644 --- a/services/brig/src/Brig/API/Internal.hs +++ b/services/brig/src/Brig/API/Internal.hs @@ -230,7 +230,6 @@ federationRemotesAPI = addFederationRemote :: FederationDomainConfig -> ExceptT Brig.API.Error.Error (AppT r) () addFederationRemote fedDomConf = do - assertNoDivergingDomainInConfigFiles fedDomConf result <- lift . wrapClient $ Data.addFederationRemote fedDomConf case result of Data.AddFederationRemoteSuccess -> pure () @@ -239,57 +238,13 @@ addFederationRemote fedDomConf = do "Maximum number of remote backends reached. If you need to create more connections, \ \please contact wire.com." --- | Compile config file list into a map indexed by domains. Use this to make sure the config --- file is consistent (ie., no two entries for the same domain). -remotesMapFromCfgFile :: AppT r (Map Domain FederationDomainConfig) -remotesMapFromCfgFile = do - cfg <- asks (fromMaybe [] . setFederationDomainConfigs . view settings) - let dict = [(domain cnf, cnf) | cnf <- cfg] - merge c c' = - if c == c' - then c - else error $ "error in config file: conflicting parameters on domain: " <> show (c, c') - pure $ Map.fromListWith merge dict - --- | Return the config file list. Use this to make sure the config file is consistent (ie., --- no two entries for the same domain). Based on `remotesMapFromCfgFile`. -remotesListFromCfgFile :: AppT r [FederationDomainConfig] -remotesListFromCfgFile = Map.elems <$> remotesMapFromCfgFile - --- | If remote domain is registered in config file, the version that can be added to the --- database must be the same. -assertNoDivergingDomainInConfigFiles :: FederationDomainConfig -> ExceptT Brig.API.Error.Error (AppT r) () -assertNoDivergingDomainInConfigFiles fedComConf = do - cfg <- lift remotesMapFromCfgFile - let diverges = case Map.lookup (domain fedComConf) cfg of - Nothing -> False - Just fedComConf' -> fedComConf' /= fedComConf - when diverges $ do - throwError . fedError . FederationUnexpectedError $ - "keeping track of remote domains in the brig config file is deprecated, but as long as we \ - \do that, adding a domain with different settings than in the config file is nto allowed. want " - <> ( "Just " - <> cs (show fedComConf) - <> "or Nothing, " - ) - <> ( "got " - <> cs (show (Map.lookup (domain fedComConf) cfg)) - ) - getFederationRemotes :: ExceptT Brig.API.Error.Error (AppT r) FederationDomainConfigs getFederationRemotes = lift $ do - -- FUTUREWORK: we should solely rely on `db` in the future for remote domains; merging - -- remote domains from `cfg` is just for providing an easier, more robust migration path. - -- See - -- https://docs.wire.com/understand/federation/backend-communication.html#configuring-remote-connections, - -- http://docs.wire.com/developer/developer/federation-design-aspects.html#configuring-remote-connections-dev-perspective db <- wrapClient Data.getFederationRemotes - (ms :: Maybe FederationStrategy, mf :: [FederationDomainConfig], mu :: Maybe Int) <- do + (ms :: Maybe FederationStrategy, mu :: Maybe Int) <- do cfg <- ask - domcfgs <- remotesListFromCfgFile -- (it's not very elegant to prove the env twice here, but this code is transitory.) pure ( setFederationStrategy (cfg ^. settings), - domcfgs, setFederationDomainConfigsUpdateFreq (cfg ^. settings) ) @@ -303,14 +258,13 @@ getFederationRemotes = lift $ do defFederationDomainConfigs & maybe id (\v cfg -> cfg {strategy = v}) ms - & (\cfg -> cfg {remotes = nub $ db <> mf}) + & (\cfg -> cfg {remotes = db}) & maybe id (\v cfg -> cfg {updateInterval = min 1 v}) mu & pure updateFederationRemote :: Domain -> FederationDomainConfig -> ExceptT Brig.API.Error.Error (AppT r) () updateFederationRemote dom fedcfg = do assertDomainIsNotUpdated dom fedcfg - assertNoDomainsFromConfigFiles dom (lift . wrapClient . Data.updateFederationRemote $ fedcfg) >>= \case True -> pure () False -> @@ -323,15 +277,6 @@ assertDomainIsNotUpdated dom fedcfg = do throwError . fedError . FederationUnexpectedError . cs $ "federation domain of a given peer cannot be changed from " <> show (domain fedcfg) <> " to " <> show dom <> "." --- | FUTUREWORK: should go away in the future; see 'getFederationRemotes'. -assertNoDomainsFromConfigFiles :: Domain -> ExceptT Brig.API.Error.Error (AppT r) () -assertNoDomainsFromConfigFiles dom = do - cfg <- asks (fromMaybe [] . setFederationDomainConfigs . view settings) - when (dom `elem` (domain <$> cfg)) $ do - throwError . fedError . FederationUnexpectedError $ - "keeping track of remote domains in the brig config file is deprecated, but as long as we \ - \do that, removing or updating items listed in the config file is not allowed." - -- | Remove the entry from the database if present (or do nothing if not). This responds with -- 533 if the entry was also present in the config file, but only *after* it has removed the -- entry from cassandra. @@ -341,7 +286,6 @@ assertNoDomainsFromConfigFiles dom = do deleteFederationRemote :: Domain -> ExceptT Brig.API.Error.Error (AppT r) () deleteFederationRemote dom = do lift . wrapClient . Data.deleteFederationRemote $ dom - assertNoDomainsFromConfigFiles dom env <- ask ownDomain <- viewFederationDomain remoteDomains <- fmap domain . remotes <$> getFederationRemotes diff --git a/services/brig/src/Brig/Options.hs b/services/brig/src/Brig/Options.hs index 6ca9938851b..d86e2298916 100644 --- a/services/brig/src/Brig/Options.hs +++ b/services/brig/src/Brig/Options.hs @@ -552,13 +552,6 @@ data Settings = Settings -- | See https://docs.wire.com/understand/federation/backend-communication.html#configuring-remote-connections -- default: AllowNone setFederationStrategy :: !(Maybe FederationStrategy), - -- | 'setFederationDomainConfigs' is introduced in - -- https://github.com/wireapp/wire-server/pull/3260 for the sole purpose of transitioning - -- to dynamic federation remote configuration. See - -- https://docs.wire.com/understand/federation/backend-communication.html#configuring-remote-connections - -- for details. - -- default: [] - setFederationDomainConfigs :: !(Maybe [FederationDomainConfig]), -- | In seconds. Default: 10 seconds. Values <1 are silently replaced by 1. See -- https://docs.wire.com/understand/federation/backend-communication.html#configuring-remote-connections setFederationDomainConfigsUpdateFreq :: !(Maybe Int), @@ -913,7 +906,6 @@ Lens.makeLensesFor ("setSqsThrottleMillis", "sqsThrottleMillis"), ("setSftStaticUrl", "sftStaticUrl"), ("setSftListAllServers", "sftListAllServers"), - ("setFederationDomainConfigs", "federationDomainConfigs"), ("setEnableDevelopmentVersions", "enableDevelopmentVersions"), ("setRestrictUserCreation", "restrictUserCreation"), ("setEnableMLS", "enableMLS"), From 815d15deee3f329faad7ad131ee218d831878a83 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 25 Aug 2023 11:43:35 +0200 Subject: [PATCH 21/61] Update changelog. --- changelog.d/0-release-notes/WPB-3796 | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/changelog.d/0-release-notes/WPB-3796 b/changelog.d/0-release-notes/WPB-3796 index 92be27047f1..fd8d7268e82 100644 --- a/changelog.d/0-release-notes/WPB-3796 +++ b/changelog.d/0-release-notes/WPB-3796 @@ -1,5 +1,15 @@ -New config value `background-worker.backendNotificationPusher.remotesRefreshIntervalMs`. If in doubt, set to 30000 ms. (This controls the delay between backend-to-backend notification queue update pulls from rabbitMQ to background-worker.) +New config value +`background-worker.backendNotificationPusher.remotesRefreshIntervalMs`. +If in doubt, set to 30000 ms. (This controls the delay between +backend-to-backend notification queue update pulls from rabbitMQ to +background-worker.) -Remote federation domains in the config file are no longer supported (deprecated since Charts...). Before deploying this release, make sure you have completed the migration steps from [the CHANGELOG of that release](...). +If you have enabled federation and therefore rabbitMQ before this +upgrade, you should remove all queues called +`backend-notifications.delete-federation` manually. -If you have enabled federation and therefore rabbitMQ before this upgrade, you may need to remove all queues called `backend-notifications.delete-federation` manually, or this release won't start. +Remote federation domains in the config file are no longer supported. +Before deploying this release, make sure you have completed the +migration steps from [the CHANGELOG of Chart Release 4.36.0 (search +for "remote connections can be configured via an internal REST +API")](https://github.com/wireapp/wire-server/releases/tag/v2023-08-11). From 4ce60fc060a60e295d7afd523c85e9c5c6689d64 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 25 Aug 2023 12:17:22 +0200 Subject: [PATCH 22/61] Fixup --- hack/helmfile.yaml | 8 -------- integration/test/SetupHelpers.hs | 11 ----------- integration/test/Testlib/ModService.hs | 1 - integration/test/Testlib/Types.hs | 1 - 4 files changed, 21 deletions(-) diff --git a/hack/helmfile.yaml b/hack/helmfile.yaml index 3cc832502e2..878cb016f50 100644 --- a/hack/helmfile.yaml +++ b/hack/helmfile.yaml @@ -143,13 +143,5 @@ releases: value: {{ .Values.federationDomain2 }} - name: cargohold.config.settings.federationDomain value: {{ .Values.federationDomain2 }} - - name: brig.config.optSettings.setFederationDomainConfigs[0].domain - value: {{ .Values.federationDomain1 }} - - name: brig.config.optSettings.setFederationDomainConfigs[2].domain - value: {{ .Values.dynBackendDomain1 }} - - name: brig.config.optSettings.setFederationDomainConfigs[3].domain - value: {{ .Values.dynBackendDomain2 }} - - name: brig.config.optSettings.setFederationDomainConfigs[4].domain - value: {{ .Values.dynBackendDomain3 }} needs: - 'databases-ephemeral' diff --git a/integration/test/SetupHelpers.hs b/integration/test/SetupHelpers.hs index 0b0e75838c3..9a3053686ed 100644 --- a/integration/test/SetupHelpers.hs +++ b/integration/test/SetupHelpers.hs @@ -92,17 +92,6 @@ randomUserId domain = do uid <- randomId pure $ object ["id" .= uid, "domain" .= d] -addFullSearchFor :: [String] -> Value -> App Value -addFullSearchFor domains val = - modifyField - "optSettings.setFederationDomainConfigs" - ( \configs -> do - cfg <- assertJust "" configs - xs <- cfg & asList - pure (xs <> [object ["domain" .= domain, "search_policy" .= "full_search"] | domain <- domains]) - ) - val - fullSearchWithAll :: ServiceOverrides fullSearchWithAll = def diff --git a/integration/test/Testlib/ModService.hs b/integration/test/Testlib/ModService.hs index 9a0b30cdd08..152dc13ed5f 100644 --- a/integration/test/Testlib/ModService.hs +++ b/integration/test/Testlib/ModService.hs @@ -174,7 +174,6 @@ startDynamicBackend resource beOverrides = do \case Brig -> setField "optSettings.setFederationDomain" resource.berDomain - >=> setField "optSettings.setFederationDomainConfigs" ([] :: [Value]) >=> setField "federatorInternal.port" resource.berFederatorInternal >=> setField "federatorInternal.host" ("127.0.0.1" :: String) >=> setField "rabbitmq.vHost" resource.berVHost diff --git a/integration/test/Testlib/Types.hs b/integration/test/Testlib/Types.hs index cb033f5665f..121234a6340 100644 --- a/integration/test/Testlib/Types.hs +++ b/integration/test/Testlib/Types.hs @@ -249,7 +249,6 @@ defaultServiceOverridesToMap = ([minBound .. maxBound] <&> (,pure)) & Map.fromLi -- def -- { dbBrig = -- setField "optSettings.setFederationStrategy" "allowDynamic" --- >=> removeField "optSettings.setFederationDomainConfigs" -- } -- withOverrides overrides defaultServiceOverridesToMap` withOverrides :: ServiceOverrides -> Map.Map Service (Value -> App Value) -> Map.Map Service (Value -> App Value) From cb6184c76cb6ac6cc6d14fb88062063415d14f13 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 28 Aug 2023 12:24:18 +0200 Subject: [PATCH 23/61] ... --- integration/test/Testlib/Types.hs | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/integration/test/Testlib/Types.hs b/integration/test/Testlib/Types.hs index 121234a6340..d6a2f590f1f 100644 --- a/integration/test/Testlib/Types.hs +++ b/integration/test/Testlib/Types.hs @@ -240,34 +240,6 @@ defaultServiceOverrides = federatorInternalCfg = pure } -defaultServiceOverridesToMap :: Map.Map Service (Value -> App Value) -defaultServiceOverridesToMap = ([minBound .. maxBound] <&> (,pure)) & Map.fromList - --- | Overrides the service configurations with the given overrides. --- e.g. --- `let overrides = --- def --- { dbBrig = --- setField "optSettings.setFederationStrategy" "allowDynamic" --- } --- withOverrides overrides defaultServiceOverridesToMap` -withOverrides :: ServiceOverrides -> Map.Map Service (Value -> App Value) -> Map.Map Service (Value -> App Value) -withOverrides overrides = - Map.mapWithKey - ( \svr f -> - case svr of - Brig -> f >=> overrides.dbBrig - Cannon -> f >=> overrides.dbCannon - Cargohold -> f >=> overrides.dbCargohold - Galley -> f >=> overrides.dbGalley - Gundeck -> f >=> overrides.dbGundeck - Nginz -> f >=> overrides.dbNginz - Spar -> f >=> overrides.dbSpar - BackgroundWorker -> f >=> overrides.dbBackgroundWorker - Stern -> f >=> overrides.dbStern - FederatorInternal -> f - ) - lookupConfigOverride :: ServiceOverrides -> Service -> (Value -> App Value) lookupConfigOverride overrides = \case Brig -> overrides.brigCfg From f3440fdf33218b6808ac4b84c8ef7b3720756bb4 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 28 Aug 2023 12:27:04 +0200 Subject: [PATCH 24/61] ... --- integration/test/Testlib/ModService.hs | 52 +++++++++++++------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/integration/test/Testlib/ModService.hs b/integration/test/Testlib/ModService.hs index c96a867ca4a..9890e8c765c 100644 --- a/integration/test/Testlib/ModService.hs +++ b/integration/test/Testlib/ModService.hs @@ -171,32 +171,32 @@ startDynamicBackend resource beOverrides = do setFederationSettings :: ServiceOverrides setFederationSettings = - \case - Brig -> - setField "optSettings.setFederationDomain" resource.berDomain - >=> setField "federatorInternal.port" resource.berFederatorInternal - >=> setField "federatorInternal.host" ("127.0.0.1" :: String) - >=> setField "rabbitmq.vHost" resource.berVHost - Cargohold -> - setField "settings.federationDomain" resource.berDomain - >=> setField "federator.host" ("127.0.0.1" :: String) - >=> setField "federator.port" resource.berFederatorInternal - Galley -> - setField "settings.federationDomain" resource.berDomain - >=> setField "settings.featureFlags.classifiedDomains.config.domains" [resource.berDomain] - >=> setField "federator.host" ("127.0.0.1" :: String) - >=> setField "federator.port" resource.berFederatorInternal - >=> setField "rabbitmq.vHost" resource.berVHost - Gundeck -> setField "settings.federationDomain" resource.berDomain - BackgroundWorker -> - setField "federatorInternal.port" resource.berFederatorInternal - >=> setField "federatorInternal.host" ("127.0.0.1" :: String) - >=> setField "rabbitmq.vHost" resource.berVHost - FederatorInternal -> - setField "federatorInternal.port" resource.berFederatorInternal - >=> setField "federatorExternal.port" resource.berFederatorExternal - >=> setField "optSettings.setFederationDomain" resource.berDomain - _ -> pure + def + { brigCfg = + setField "optSettings.setFederationDomain" resource.berDomain + >=> setField "federatorInternal.port" resource.berFederatorInternal + >=> setField "federatorInternal.host" ("127.0.0.1" :: String) + >=> setField "rabbitmq.vHost" resource.berVHost, + cargoholdCfg = + setField "settings.federationDomain" resource.berDomain + >=> setField "federator.host" ("127.0.0.1" :: String) + >=> setField "federator.port" resource.berFederatorInternal, + galleyCfg = + setField "settings.federationDomain" resource.berDomain + >=> setField "settings.featureFlags.classifiedDomains.config.domains" [resource.berDomain] + >=> setField "federator.host" ("127.0.0.1" :: String) + >=> setField "federator.port" resource.berFederatorInternal + >=> setField "rabbitmq.vHost" resource.berVHost, + gundeckCfg = setField "settings.federationDomain" resource.berDomain, + backgroundWorkerCfg = + setField "federatorInternal.port" resource.berFederatorInternal + >=> setField "federatorInternal.host" ("127.0.0.1" :: String) + >=> setField "rabbitmq.vHost" resource.berVHost, + federatorInternalCfg = + setField "federatorInternal.port" resource.berFederatorInternal + >=> setField "federatorExternal.port" resource.berFederatorExternal + >=> setField "optSettings.setFederationDomain" resource.berDomain + } setKeyspace :: ServiceOverrides setKeyspace = From 15c0cef4303316d9964ac439f3ae74e491fe44b1 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 28 Aug 2023 15:56:34 +0200 Subject: [PATCH 25/61] ... --- integration/test/SetupHelpers.hs | 11 ----------- integration/test/Test/Conversation.hs | 8 ++------ integration/test/Test/Demo.hs | 5 +++-- integration/test/Testlib/RunServices.hs | 5 +---- 4 files changed, 6 insertions(+), 23 deletions(-) diff --git a/integration/test/SetupHelpers.hs b/integration/test/SetupHelpers.hs index 9a3053686ed..896910ecd68 100644 --- a/integration/test/SetupHelpers.hs +++ b/integration/test/SetupHelpers.hs @@ -9,7 +9,6 @@ import Data.Aeson hiding ((.=)) import Data.Aeson.Types qualified as Aeson import Data.Default import Data.Function -import Data.List qualified as List import Data.UUID.V4 (nextRandom) import GHC.Stack import Testlib.Prelude @@ -92,16 +91,6 @@ randomUserId domain = do uid <- randomId pure $ object ["id" .= uid, "domain" .= d] -fullSearchWithAll :: ServiceOverrides -fullSearchWithAll = - def - { brigCfg = \val -> do - ownDomain <- asString =<< val %. "optSettings.setFederationDomain" - env <- ask - let remoteDomains = List.delete ownDomain $ [env.domain1, env.domain2] <> env.dynamicDomains - addFullSearchFor remoteDomains val - } - withFederatingBackendsAllowDynamic :: HasCallStack => Int -> ((String, String, String) -> App a) -> App a withFederatingBackendsAllowDynamic n k = do let setFederationConfig = diff --git a/integration/test/Test/Conversation.hs b/integration/test/Test/Conversation.hs index 88313593d2d..c15d1f997f3 100644 --- a/integration/test/Test/Conversation.hs +++ b/integration/test/Test/Conversation.hs @@ -20,9 +20,7 @@ testDynamicBackendsFullyConnectedWhenAllowAll :: HasCallStack => App () testDynamicBackendsFullyConnectedWhenAllowAll = do let overrides = def {brigCfg = setField "optSettings.setFederationStrategy" "allowAll"} - <> fullSearchWithAll - startDynamicBackends [overrides, overrides, overrides] $ \dynDomains -> do - [domainA, domainB, domainC] <- pure dynDomains + startDynamicBackends [overrides, overrides, overrides] $ \[domainA, domainB, domainC] -> do uidA <- randomUser domainA def {team = True} uidB <- randomUser domainA def {team = True} uidC <- randomUser domainA def {team = True} @@ -338,7 +336,7 @@ testConvWithUnreachableRemoteUsers :: HasCallStack => App () testConvWithUnreachableRemoteUsers = do let overrides = def {brigCfg = setField "optSettings.setFederationStrategy" "allowAll"} - <> fullSearchWithAll + ([alice, alex, bob, charlie, dylan], domains) <- startDynamicBackends [overrides, overrides] $ \domains -> do own <- make OwnDomain & asString @@ -359,7 +357,6 @@ testAddReachableWithUnreachableRemoteUsers :: HasCallStack => App () testAddReachableWithUnreachableRemoteUsers = do let overrides = def {brigCfg = setField "optSettings.setFederationStrategy" "allowAll"} - <> fullSearchWithAll ([alex, bob], conv, domains) <- startDynamicBackends [overrides, overrides] $ \domains -> do own <- make OwnDomain & asString @@ -385,7 +382,6 @@ testAddUnreachable :: HasCallStack => App () testAddUnreachable = do let overrides = def {brigCfg = setField "optSettings.setFederationStrategy" "allowAll"} - <> fullSearchWithAll ([alex, charlie], [charlieDomain, dylanDomain], conv) <- startDynamicBackends [overrides, overrides] $ \domains -> do own <- make OwnDomain & asString diff --git a/integration/test/Test/Demo.hs b/integration/test/Test/Demo.hs index 69d3cf1b0c4..a61c8d1950f 100644 --- a/integration/test/Test/Demo.hs +++ b/integration/test/Test/Demo.hs @@ -1,3 +1,5 @@ +{-# OPTIONS_GHC -Wno-incomplete-uni-patterns #-} + -- | This module is meant to show how Testlib can be used module Test.Demo where @@ -173,8 +175,7 @@ testIndependentESIndices = do testDynamicBackendsFederation :: HasCallStack => App () testDynamicBackendsFederation = do - startDynamicBackends [def <> fullSearchWithAll, def <> fullSearchWithAll] $ \dynDomains -> do - [aDynDomain, anotherDynDomain] <- pure dynDomains + startDynamicBackends [def, def] $ \[aDynDomain, anotherDynDomain] -> do u1 <- randomUser aDynDomain def u2 <- randomUser anotherDynDomain def uid2 <- objId u2 diff --git a/integration/test/Testlib/RunServices.hs b/integration/test/Testlib/RunServices.hs index 5f2b5a3eb34..48a3c6d2103 100644 --- a/integration/test/Testlib/RunServices.hs +++ b/integration/test/Testlib/RunServices.hs @@ -4,7 +4,6 @@ module Testlib.RunServices where import Control.Concurrent import Control.Monad.Codensity (lowerCodensity) -import SetupHelpers import System.Directory import System.Environment (getArgs) import System.Exit (exitWith) @@ -63,9 +62,7 @@ main = do _modifyEnv <- traverseConcurrentlyCodensity ( \resource -> - -- We add the 'fullSerachWithAll' overrrides is a hack to get - -- around https://wearezeta.atlassian.net/browse/WPB-3796 - startDynamicBackend resource fullSearchWithAll + startDynamicBackend resource def ) [backendA, backendB] liftIO run From 2200da8c4a2292d46ebdeb7966921398f487af63 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 4 Sep 2023 14:05:54 +0200 Subject: [PATCH 26/61] ... --- .../brig/test/integration/API/Federation.hs | 59 ++++++++++--------- 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/services/brig/test/integration/API/Federation.hs b/services/brig/test/integration/API/Federation.hs index c58d022c901..0c1ac2b48b4 100644 --- a/services/brig/test/integration/API/Federation.hs +++ b/services/brig/test/integration/API/Federation.hs @@ -83,9 +83,13 @@ tests m opts brig cannon fedBrigClient = test m "POST /federation/api-version : 200" (testAPIVersion brig fedBrigClient) ] -allowFullSearch :: Domain -> Opt.Opts -> Opt.Opts -allowFullSearch domain opts = - opts & Opt.optionSettings . Opt.federationDomainConfigs ?~ [FD.FederationDomainConfig domain FullSearch] +setSearchPolicy :: Domain -> FederatedUserSearchPolicy -> WaiTest.Session () +setSearchPolicy domain policy = do + put (path "/i/federation/remotes" . Bilge.json (FD.FederationDomainConfig domain policy)) + !!! const 200 === statusCode + +allowFullSearch :: Domain -> WaiTest.Session () +allowFullSearch domain = setSearchPolicy domain FullSearch testSearchSuccess :: Opt.Opts -> Brig -> Http () testSearchSuccess opts brig = do @@ -95,7 +99,8 @@ testSearchSuccess opts brig = do let quid = userQualifiedId user let domain = Domain "example.com" - searchResponse <- withSettingsOverrides (allowFullSearch domain opts) $ do + searchResponse <- withSettingsOverrides opts $ do + allowFullSearch domain runWaiTestFedClient domain $ createWaiTestFedClient @"search-users" @'Brig $ SearchRequest (fromHandle handle) @@ -112,7 +117,8 @@ testFulltextSearchSuccess opts brig = do let quid = userQualifiedId user let domain = Domain "example.com" - searchResponse <- withSettingsOverrides (allowFullSearch domain opts) $ do + searchResponse <- withSettingsOverrides opts $ do + allowFullSearch domain runWaiTestFedClient domain $ createWaiTestFedClient @"search-users" @'Brig $ SearchRequest (fromName $ userDisplayName user) @@ -139,7 +145,8 @@ testFulltextSearchMultipleUsers opts brig = do let domain = Domain "example.com" - searchResponse <- withSettingsOverrides (allowFullSearch domain opts) $ do + searchResponse <- withSettingsOverrides opts $ do + allowFullSearch domain runWaiTestFedClient domain $ createWaiTestFedClient @"search-users" @'Brig $ SearchRequest (fromHandle handle) @@ -152,7 +159,8 @@ testSearchNotFound :: Opt.Opts -> Http () testSearchNotFound opts = do let domain = Domain "example.com" - searchResponse <- withSettingsOverrides (allowFullSearch domain opts) $ do + searchResponse <- withSettingsOverrides opts $ do + allowFullSearch domain runWaiTestFedClient domain $ createWaiTestFedClient @"search-users" @'Brig $ SearchRequest "this-handle-should-not-exist" @@ -163,7 +171,8 @@ testSearchNotFoundEmpty :: Opt.Opts -> Http () testSearchNotFoundEmpty opts = do let domain = Domain "example.com" - searchResponse <- withSettingsOverrides (allowFullSearch domain opts) $ do + searchResponse <- withSettingsOverrides opts $ do + allowFullSearch domain runWaiTestFedClient domain $ createWaiTestFedClient @"search-users" @'Brig $ SearchRequest "this-handle-should-not-exist" @@ -181,14 +190,6 @@ testSearchRestrictions opts brig = do let quid = userQualifiedId user refreshIndex brig - let opts' = - opts - & Opt.optionSettings . Opt.federationDomainConfigs - ?~ [ FD.FederationDomainConfig domainNoSearch NoSearch, - FD.FederationDomainConfig domainExactHandle ExactHandleSearch, - FD.FederationDomainConfig domainFullSearch FullSearch - ] - let expectSearch :: HasCallStack => Domain -> Either Handle Name -> Maybe (Qualified UserId) -> FederatedUserSearchPolicy -> WaiTest.Session () expectSearch domain handleOrName mExpectedUser expectedSearchPolicy = do let squery = either fromHandle fromName handleOrName @@ -205,7 +206,11 @@ testSearchRestrictions opts brig = do assertEqual "Unexpected search result" (maybeToList mExpectedUser) (contactQualifiedId <$> S.contacts searchResponse) assertEqual "Unexpected search result" expectedSearchPolicy (S.searchPolicy searchResponse) - withSettingsOverrides opts' $ do + withSettingsOverrides opts $ do + setSearchPolicy domainNoSearch NoSearch + setSearchPolicy domainExactHandle ExactHandleSearch + setSearchPolicy domainFullSearch FullSearch + expectSearch domainNoSearch (Left handle) Nothing NoSearch expectSearch domainExactHandle (Left handle) (Just quid) ExactHandleSearch expectSearch domainExactHandle (Right (userDisplayName user)) Nothing ExactHandleSearch @@ -225,21 +230,17 @@ testGetUserByHandleRestrictions opts brig = do let quid = userQualifiedId user refreshIndex brig - let opts' = - opts - & Opt.optionSettings . Opt.federationDomainConfigs - ?~ [ FD.FederationDomainConfig domainNoSearch NoSearch, - FD.FederationDomainConfig domainExactHandle ExactHandleSearch, - FD.FederationDomainConfig domainFullSearch FullSearch - ] - let expectSearch domain expectedUser = do maybeUserProfile <- runWaiTestFedClient domain $ createWaiTestFedClient @"get-user-by-handle" @'Brig handle liftIO $ assertEqual "Unexpected search result" expectedUser (profileQualifiedId <$> maybeUserProfile) - withSettingsOverrides opts' $ do + withSettingsOverrides opts $ do + setSearchPolicy domainNoSearch NoSearch + setSearchPolicy domainExactHandle ExactHandleSearch + setSearchPolicy domainFullSearch FullSearch + expectSearch domainNoSearch Nothing expectSearch domainExactHandle (Just quid) expectSearch domainFullSearch (Just quid) @@ -251,7 +252,8 @@ testGetUserByHandleSuccess opts brig = do let quid = userQualifiedId user let domain = Domain "example.com" - maybeProfile <- withSettingsOverrides (allowFullSearch domain opts) $ do + maybeProfile <- withSettingsOverrides opts $ do + allowFullSearch domain runWaiTestFedClient domain $ createWaiTestFedClient @"get-user-by-handle" @'Brig $ handle @@ -268,7 +270,8 @@ testGetUserByHandleNotFound opts = do hdl <- randomHandle let domain = Domain "example.com" - maybeProfile <- withSettingsOverrides (allowFullSearch domain opts) $ do + maybeProfile <- withSettingsOverrides opts $ do + allowFullSearch domain runWaiTestFedClient domain $ createWaiTestFedClient @"get-user-by-handle" @'Brig $ Handle hdl From 97856cdc1a8c1e6cd9e9d020d9380f476f48b1a6 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 4 Sep 2023 14:38:00 +0200 Subject: [PATCH 27/61] Fixup --- .../brig/test/integration/API/Federation.hs | 55 ++++++++++--------- 1 file changed, 28 insertions(+), 27 deletions(-) diff --git a/services/brig/test/integration/API/Federation.hs b/services/brig/test/integration/API/Federation.hs index 0c1ac2b48b4..f6d7b6f65a1 100644 --- a/services/brig/test/integration/API/Federation.hs +++ b/services/brig/test/integration/API/Federation.hs @@ -65,12 +65,12 @@ tests m opts brig cannon fedBrigClient = [ test m "POST /federation/search-users : Found" (testSearchSuccess opts brig), test m "POST /federation/search-users : Found (fulltext)" (testFulltextSearchSuccess opts brig), test m "POST /federation/search-users : Found (multiple users)" (testFulltextSearchMultipleUsers opts brig), - test m "POST /federation/search-users : NotFound" (testSearchNotFound opts), - test m "POST /federation/search-users : Empty Input - NotFound" (testSearchNotFoundEmpty opts), + test m "POST /federation/search-users : NotFound" (testSearchNotFound opts brig), + test m "POST /federation/search-users : Empty Input - NotFound" (testSearchNotFoundEmpty opts brig), test m "POST /federation/search-users : configured restrictions" (testSearchRestrictions opts brig), test m "POST /federation/get-user-by-handle : configured restrictions" (testGetUserByHandleRestrictions opts brig), test m "POST /federation/get-user-by-handle : Found" (testGetUserByHandleSuccess opts brig), - test m "POST /federation/get-user-by-handle : NotFound" (testGetUserByHandleNotFound opts), + test m "POST /federation/get-user-by-handle : NotFound" (testGetUserByHandleNotFound opts brig), test m "POST /federation/get-users-by-ids : 200 all found" (testGetUsersByIdsSuccess brig fedBrigClient), test m "POST /federation/get-users-by-ids : 200 partially found" (testGetUsersByIdsPartial brig fedBrigClient), test m "POST /federation/get-users-by-ids : 200 none found" (testGetUsersByIdsNoneFound fedBrigClient), @@ -83,13 +83,14 @@ tests m opts brig cannon fedBrigClient = test m "POST /federation/api-version : 200" (testAPIVersion brig fedBrigClient) ] -setSearchPolicy :: Domain -> FederatedUserSearchPolicy -> WaiTest.Session () -setSearchPolicy domain policy = do - put (path "/i/federation/remotes" . Bilge.json (FD.FederationDomainConfig domain policy)) +setSearchPolicy :: Brig -> Domain -> FederatedUserSearchPolicy -> WaiTest.Session () +setSearchPolicy brig domain policy = do + let req = brig . path "/i/federation/remotes" . Bilge.json (FD.FederationDomainConfig domain policy) + post req !!! const 200 === statusCode -allowFullSearch :: Domain -> WaiTest.Session () -allowFullSearch domain = setSearchPolicy domain FullSearch +allowFullSearch :: Brig -> Domain -> WaiTest.Session () +allowFullSearch brig domain = setSearchPolicy brig domain FullSearch testSearchSuccess :: Opt.Opts -> Brig -> Http () testSearchSuccess opts brig = do @@ -100,7 +101,7 @@ testSearchSuccess opts brig = do let domain = Domain "example.com" searchResponse <- withSettingsOverrides opts $ do - allowFullSearch domain + allowFullSearch brig domain runWaiTestFedClient domain $ createWaiTestFedClient @"search-users" @'Brig $ SearchRequest (fromHandle handle) @@ -118,7 +119,7 @@ testFulltextSearchSuccess opts brig = do let domain = Domain "example.com" searchResponse <- withSettingsOverrides opts $ do - allowFullSearch domain + allowFullSearch brig domain runWaiTestFedClient domain $ createWaiTestFedClient @"search-users" @'Brig $ SearchRequest (fromName $ userDisplayName user) @@ -146,7 +147,7 @@ testFulltextSearchMultipleUsers opts brig = do let domain = Domain "example.com" searchResponse <- withSettingsOverrides opts $ do - allowFullSearch domain + allowFullSearch brig domain runWaiTestFedClient domain $ createWaiTestFedClient @"search-users" @'Brig $ SearchRequest (fromHandle handle) @@ -155,24 +156,24 @@ testFulltextSearchMultipleUsers opts brig = do let contacts = contactQualifiedId <$> S.contacts searchResponse assertEqual "should find both users" (sort [quid, userQualifiedId identityThief]) (sort contacts) -testSearchNotFound :: Opt.Opts -> Http () -testSearchNotFound opts = do +testSearchNotFound :: Opt.Opts -> Brig -> Http () +testSearchNotFound opts brig = do let domain = Domain "example.com" searchResponse <- withSettingsOverrides opts $ do - allowFullSearch domain + allowFullSearch brig domain runWaiTestFedClient domain $ createWaiTestFedClient @"search-users" @'Brig $ SearchRequest "this-handle-should-not-exist" liftIO $ assertEqual "should return empty array of users" [] (S.contacts searchResponse) -testSearchNotFoundEmpty :: Opt.Opts -> Http () -testSearchNotFoundEmpty opts = do +testSearchNotFoundEmpty :: Opt.Opts -> Brig -> Http () +testSearchNotFoundEmpty opts brig = do let domain = Domain "example.com" searchResponse <- withSettingsOverrides opts $ do - allowFullSearch domain + allowFullSearch brig domain runWaiTestFedClient domain $ createWaiTestFedClient @"search-users" @'Brig $ SearchRequest "this-handle-should-not-exist" @@ -207,9 +208,9 @@ testSearchRestrictions opts brig = do assertEqual "Unexpected search result" expectedSearchPolicy (S.searchPolicy searchResponse) withSettingsOverrides opts $ do - setSearchPolicy domainNoSearch NoSearch - setSearchPolicy domainExactHandle ExactHandleSearch - setSearchPolicy domainFullSearch FullSearch + setSearchPolicy brig domainNoSearch NoSearch + setSearchPolicy brig domainExactHandle ExactHandleSearch + setSearchPolicy brig domainFullSearch FullSearch expectSearch domainNoSearch (Left handle) Nothing NoSearch expectSearch domainExactHandle (Left handle) (Just quid) ExactHandleSearch @@ -237,9 +238,9 @@ testGetUserByHandleRestrictions opts brig = do liftIO $ assertEqual "Unexpected search result" expectedUser (profileQualifiedId <$> maybeUserProfile) withSettingsOverrides opts $ do - setSearchPolicy domainNoSearch NoSearch - setSearchPolicy domainExactHandle ExactHandleSearch - setSearchPolicy domainFullSearch FullSearch + setSearchPolicy brig domainNoSearch NoSearch + setSearchPolicy brig domainExactHandle ExactHandleSearch + setSearchPolicy brig domainFullSearch FullSearch expectSearch domainNoSearch Nothing expectSearch domainExactHandle (Just quid) @@ -253,7 +254,7 @@ testGetUserByHandleSuccess opts brig = do let domain = Domain "example.com" maybeProfile <- withSettingsOverrides opts $ do - allowFullSearch domain + allowFullSearch brig domain runWaiTestFedClient domain $ createWaiTestFedClient @"get-user-by-handle" @'Brig $ handle @@ -265,13 +266,13 @@ testGetUserByHandleSuccess opts brig = do assertEqual "should return correct user Id" quid (profileQualifiedId profile) assertEqual "should not have email address" Nothing (profileEmail profile) -testGetUserByHandleNotFound :: Opt.Opts -> Http () -testGetUserByHandleNotFound opts = do +testGetUserByHandleNotFound :: Opt.Opts -> Brig -> Http () +testGetUserByHandleNotFound opts brig = do hdl <- randomHandle let domain = Domain "example.com" maybeProfile <- withSettingsOverrides opts $ do - allowFullSearch domain + allowFullSearch brig domain runWaiTestFedClient domain $ createWaiTestFedClient @"get-user-by-handle" @'Brig $ Handle hdl From d456a7cdb4192ee0e95d1e3771f2d63ca15df4ec Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 4 Sep 2023 15:01:19 +0200 Subject: [PATCH 28/61] Fixup --- integration/test/Test/Brig.hs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/integration/test/Test/Brig.hs b/integration/test/Test/Brig.hs index eedca3010b0..d55a6dada9e 100644 --- a/integration/test/Test/Brig.hs +++ b/integration/test/Test/Brig.hs @@ -29,15 +29,8 @@ testSearchContactForExternalUsers = do testCrudFederationRemotes :: HasCallStack => App () testCrudFederationRemotes = do otherDomain <- asString OtherDomain - let -- TODO - overrides = - def - { brigCfg = - setField - "optSettings.setFederationDomainConfigs" - [object ["domain" .= otherDomain, "search_policy" .= "full_search"]] - } - withModifiedBackend overrides $ \ownDomain -> do + withModifiedBackend def $ \ownDomain -> do + void $ Internal.createFedConn ownDomain "full_search" let parseFedConns :: HasCallStack => Response -> App [Value] parseFedConns resp = -- Pick out the list of federation domain configs From b9afef668d0258c35d5c24e37640d9a37a81724e Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 4 Sep 2023 15:01:45 +0200 Subject: [PATCH 29/61] Fixup --- integration/test/SetupHelpers.hs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/integration/test/SetupHelpers.hs b/integration/test/SetupHelpers.hs index 896910ecd68..7a60f6756ff 100644 --- a/integration/test/SetupHelpers.hs +++ b/integration/test/SetupHelpers.hs @@ -106,3 +106,9 @@ withFederatingBackendsAllowDynamic n k = do sequence_ [Internal.createFedConn x (Internal.FedConn y "full_search") | x <- domains, y <- domains, x /= y] liftIO $ threadDelay (n * 1000 * 1000) -- wait for federation status to be updated k (domainA, domainB, domainC) + +setSearchPolicy :: (MakesValue dom, MakesValue policy) => dom -> policy -> App () +setSearchPolicy dom policy = _ dom policy + +allowFullSearch :: MakesValue dom => dom -> App () +allowFullSearch dom = setSearchPolicy dom "full_search" From eb7ee5080ce5fb678879db4330504a60544922b5 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 4 Sep 2023 17:18:15 +0200 Subject: [PATCH 30/61] Fixup --- integration/test/SetupHelpers.hs | 8 +----- integration/test/Test/Brig.hs | 45 +++++++++++--------------------- 2 files changed, 16 insertions(+), 37 deletions(-) diff --git a/integration/test/SetupHelpers.hs b/integration/test/SetupHelpers.hs index 7a60f6756ff..34c9df8e962 100644 --- a/integration/test/SetupHelpers.hs +++ b/integration/test/SetupHelpers.hs @@ -79,7 +79,7 @@ resetFedConns owndom = do rdoms :: [String] <- do rawlist <- resp.json %. "remotes" & asList (asString . (%. "domain")) `mapM` rawlist - Internal.deleteFedConn' owndom `mapM_` rdoms + Internal.deleteFedConn owndom `mapM_` rdoms randomId :: HasCallStack => App String randomId = do @@ -106,9 +106,3 @@ withFederatingBackendsAllowDynamic n k = do sequence_ [Internal.createFedConn x (Internal.FedConn y "full_search") | x <- domains, y <- domains, x /= y] liftIO $ threadDelay (n * 1000 * 1000) -- wait for federation status to be updated k (domainA, domainB, domainC) - -setSearchPolicy :: (MakesValue dom, MakesValue policy) => dom -> policy -> App () -setSearchPolicy dom policy = _ dom policy - -allowFullSearch :: MakesValue dom => dom -> App () -allowFullSearch dom = setSearchPolicy dom "full_search" diff --git a/integration/test/Test/Brig.hs b/integration/test/Test/Brig.hs index d55a6dada9e..9df96a2c885 100644 --- a/integration/test/Test/Brig.hs +++ b/integration/test/Test/Brig.hs @@ -30,7 +30,7 @@ testCrudFederationRemotes :: HasCallStack => App () testCrudFederationRemotes = do otherDomain <- asString OtherDomain withModifiedBackend def $ \ownDomain -> do - void $ Internal.createFedConn ownDomain "full_search" + void $ Internal.createFedConn ownDomain (Internal.FedConn otherDomain "full_search") let parseFedConns :: HasCallStack => Response -> App [Value] parseFedConns resp = -- Pick out the list of federation domain configs @@ -46,11 +46,6 @@ testCrudFederationRemotes = do res2 <- parseFedConns =<< Internal.readFedConns ownDomain sort res2 `shouldMatch` sort want - addFail :: HasCallStack => MakesValue fedConn => fedConn -> App () - addFail fedConn = do - bindResponse (Internal.createFedConn' ownDomain fedConn) $ \res -> do - addFailureContext ("res = " <> show res) $ res.status `shouldMatchInt` 533 - deleteOnce :: (Ord fedConn, ToJSON fedConn, MakesValue fedConn) => String -> [fedConn] -> App () deleteOnce domain want = do bindResponse (Internal.deleteFedConn ownDomain domain) $ \res -> do @@ -58,11 +53,6 @@ testCrudFederationRemotes = do res2 <- parseFedConns =<< Internal.readFedConns ownDomain sort res2 `shouldMatch` sort want - deleteFail :: HasCallStack => String -> App () - deleteFail del = do - bindResponse (Internal.deleteFedConn' ownDomain del) $ \res -> do - addFailureContext ("res = " <> show res) $ res.status `shouldMatchInt` 533 - updateOnce :: (MakesValue fedConn, Ord fedConn2, ToJSON fedConn2, MakesValue fedConn2, HasCallStack) => String -> fedConn -> [fedConn2] -> App () updateOnce domain fedConn want = do bindResponse (Internal.updateFedConn ownDomain domain fedConn) $ \res -> do @@ -80,33 +70,28 @@ testCrudFederationRemotes = do let remote1, remote1', remote1'' :: Internal.FedConn remote1 = Internal.FedConn dom1 "no_search" - remote1' = remote1 {Internal.searchStrategy = "full_search"} - remote1'' = remote1 {Internal.domain = dom2} - - cfgRemotesExpect :: Internal.FedConn - cfgRemotesExpect = Internal.FedConn (cs otherDomain) "full_search" + remote1' = Internal.FedConn dom1 "full_search" + remote1'' = Internal.FedConn dom2 "exact_handle_search" remote1J <- make remote1 remote1J' <- make remote1' + remote1J'' <- make remote1'' resetFedConns ownDomain - cfgRemotes <- parseFedConns =<< Internal.readFedConns ownDomain - cfgRemotes `shouldMatch` [cfgRemotesExpect] - -- entries present in the config file can be idempotently added if identical, but cannot be - -- updated, deleted or updated. - addOnce cfgRemotesExpect [cfgRemotesExpect] - addFail (cfgRemotesExpect {Internal.searchStrategy = "no_search"}) - deleteFail (Internal.domain cfgRemotesExpect) - updateFail (Internal.domain cfgRemotesExpect) (cfgRemotesExpect {Internal.searchStrategy = "no_search"}) -- create - addOnce remote1 $ (remote1J : cfgRemotes) - addOnce remote1 $ (remote1J : cfgRemotes) -- idempotency + addOnce remote1 [remote1J] + addOnce remote1 [remote1J] -- idempotency + addOnce remote1'' [remote1J, remote1J''] -- update - updateOnce (Internal.domain remote1) remote1' (remote1J' : cfgRemotes) - updateFail (Internal.domain remote1) remote1'' + updateOnce (Internal.domain remote1) remote1' [remote1J', remote1J''] + updateOnce (Internal.domain remote1) remote1' [remote1J', remote1J''] -- idempotency + updateFail (Internal.domain remote1) remote1'' -- existing entries can't change domain -- delete - deleteOnce (Internal.domain remote1) cfgRemotes - deleteOnce (Internal.domain remote1) cfgRemotes -- idempotency + deleteOnce (Internal.domain remote1) [remote1J''] + deleteOnce (Internal.domain remote1) [remote1J''] -- idempotency + -- reset + resetFedConns ownDomain + deleteOnce (Internal.domain remote1) ([] :: [Internal.FedConn]) testCrudOAuthClient :: HasCallStack => App () testCrudOAuthClient = do From dc70e1c0c41aa24b94e7dde6cc27032311c03b5e Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 4 Sep 2023 17:31:14 +0200 Subject: [PATCH 31/61] Fixup --- integration/test/Test/Defederation.hs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/integration/test/Test/Defederation.hs b/integration/test/Test/Defederation.hs index 769017f65bf..68fe1ec2a96 100644 --- a/integration/test/Test/Defederation.hs +++ b/integration/test/Test/Defederation.hs @@ -13,8 +13,7 @@ testDefederationRemoteNotifications :: HasCallStack => App () testDefederationRemoteNotifications = do let remoteDomain = "example.example.com" -- Setup federation between OtherDomain and the remote domain - bindResponse (createFedConn OtherDomain $ object ["domain" .= remoteDomain, "search_policy" .= "full_search"]) $ \resp -> - resp.status `shouldMatchInt` 200 + void $ createFedConn OtherDomain (FedConn remoteDomain "full_search") -- Setup a remote user we can get notifications for. user <- randomUser OtherDomain def From 1e7d00c9338a5ea7bdf6249366a2e6ff449a5c16 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 4 Sep 2023 17:31:25 +0200 Subject: [PATCH 32/61] Fix two tests. The 404 seems more reasonable since the convo should be gone completely. Also, defederating from an unknown domain shouldn't trigger any events, no? --- integration/test/Test/Conversation.hs | 2 +- integration/test/Test/Defederation.hs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/integration/test/Test/Conversation.hs b/integration/test/Test/Conversation.hs index c15d1f997f3..8f4c1eb48e1 100644 --- a/integration/test/Test/Conversation.hs +++ b/integration/test/Test/Conversation.hs @@ -210,7 +210,7 @@ testDefederationGroupConversation = do -- assert conversation deleted from domainA retryT $ bindResponse (getConversation uA convId) $ \r -> - r.status `shouldMatchInt` 422 + r.status `shouldMatchInt` 404 -- assert federation.delete event is sent twice void $ diff --git a/integration/test/Test/Defederation.hs b/integration/test/Test/Defederation.hs index 68fe1ec2a96..7ee9942fcb5 100644 --- a/integration/test/Test/Defederation.hs +++ b/integration/test/Test/Defederation.hs @@ -20,10 +20,10 @@ testDefederationRemoteNotifications = do withWebSocket user $ \ws -> do -- Defederate from a domain that doesn't exist. This won't do anything to the databases - -- But it will send out notifications that we can wait on. + -- and it won't send any bogus notifications. -- Begin the whole process at Brig, the same as an operator would. void $ deleteFedConn OwnDomain remoteDomain - void $ awaitNMatches 2 3 (\n -> nPayload n %. "type" `isEqual` "federation.connectionRemoved") ws + void $ awaitNMatches 0 3 (\_ -> pure True) ws testDefederationNonFullyConnectedGraph :: HasCallStack => App () testDefederationNonFullyConnectedGraph = do From fddd9c9926358ef9f1bec7042428090f6db377e0 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Tue, 5 Sep 2023 12:37:21 +0200 Subject: [PATCH 33/61] stash --- services/brig/src/Brig/Federation/Client.hs | 6 ++++-- services/brig/src/Brig/User/API/Handle.hs | 3 +++ services/brig/test/integration/Federation/End2end.hs | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/services/brig/src/Brig/Federation/Client.hs b/services/brig/src/Brig/Federation/Client.hs index 693407ae8bc..e656f13a94b 100644 --- a/services/brig/src/Brig/Federation/Client.hs +++ b/services/brig/src/Brig/Federation/Client.hs @@ -26,7 +26,7 @@ import Brig.App import Control.Lens import Control.Monad import Control.Monad.Catch (MonadMask, throwM) -import Control.Monad.Trans.Except (ExceptT (..), throwE) +import Control.Monad.Trans.Except (ExceptT (..), runExceptT, throwE) import Control.Retry import Control.Timeout import Data.Domain @@ -58,7 +58,9 @@ getUserHandleInfo :: ExceptT FederationError m (Maybe UserProfile) getUserHandleInfo (tUntagged -> Qualified handle domain) = do lift $ Log.info $ Log.msg $ T.pack "Brig-federation: handle lookup call on remote backend" - runBrigFederatorClient domain $ fedClient @'Brig @"get-user-by-handle" handle + x <- runExceptT (runBrigFederatorClient domain $ fedClient @'Brig @"get-user-by-handle" handle) + lift $ Log.err $ Log.msg $ cs @_ @ByteString ("F.getUserHandleInfo: " <> show x) + either throwE pure x getUsersByIds :: ( MonadReader Env m, diff --git a/services/brig/src/Brig/User/API/Handle.hs b/services/brig/src/Brig/User/API/Handle.hs index a554b61f0f6..6af72e28810 100644 --- a/services/brig/src/Brig/User/API/Handle.hs +++ b/services/brig/src/Brig/User/API/Handle.hs @@ -50,7 +50,9 @@ getHandleInfo :: Qualified Handle -> (Handler r) (Maybe Public.UserProfile) getHandleInfo self handle = do + Log.err $ Log.msg (Log.val . cs $ show (self, handle)) lself <- qualifyLocal self + Log.err $ Log.msg (Log.val . cs $ show lself) foldQualified lself (getLocalHandleInfo lself . tUnqualified) @@ -59,6 +61,7 @@ getHandleInfo self handle = do getRemoteHandleInfo :: Remote Handle -> (Handler r) (Maybe Public.UserProfile) getRemoteHandleInfo handle = do + Log.err $ Log.msg (Log.val (cs $ "getRemoteHandleInfo: " <> show handle)) lift . Log.info $ Log.msg (Log.val "getHandleInfo - remote lookup") . Log.field "domain" (show (tDomain handle)) diff --git a/services/brig/test/integration/Federation/End2end.hs b/services/brig/test/integration/Federation/End2end.hs index 4f9a117016c..f3f2e977b16 100644 --- a/services/brig/test/integration/Federation/End2end.hs +++ b/services/brig/test/integration/Federation/End2end.hs @@ -129,7 +129,7 @@ testHandleLookup brig brigTwo = do (handle, userBrigTwo) <- createUserWithHandle brigTwo -- Get result from brig two for comparison let domain = qDomain $ userQualifiedId userBrigTwo - resultViaBrigTwo <- getUserInfoFromHandle brigTwo domain handle + resultViaBrigTwo <- getUserInfoFromHandle brigTwo domain handle -- this crashes with 404 handle not found. with "no such end-point" on v4, and with "no handle" on v1 -- query the local-namespace brig for a user sitting on the other backend -- (which will exercise the network traffic via two federators to the remote brig) From 91718c1d3c7427c53ca91b9033a8b9ea26b946f9 Mon Sep 17 00:00:00 2001 From: Igor Ranieri Date: Tue, 5 Sep 2023 14:56:13 +0200 Subject: [PATCH 34/61] Added origin domain to e2e config, added search policy to failing test. --- services/brig/test/integration/API/Federation.hs | 10 ---------- .../brig/test/integration/Federation/End2end.hs | 12 ++++++------ services/brig/test/integration/Main.hs | 4 +++- services/brig/test/integration/Util.hs | 13 +++++++++++++ 4 files changed, 22 insertions(+), 17 deletions(-) diff --git a/services/brig/test/integration/API/Federation.hs b/services/brig/test/integration/API/Federation.hs index f6d7b6f65a1..5693104cf28 100644 --- a/services/brig/test/integration/API/Federation.hs +++ b/services/brig/test/integration/API/Federation.hs @@ -49,7 +49,6 @@ import Wire.API.Federation.API.Brig qualified as FedBrig import Wire.API.Federation.API.Brig qualified as S import Wire.API.Federation.Component import Wire.API.Federation.Version -import Wire.API.Routes.FederationDomainConfig as FD import Wire.API.User import Wire.API.User.Client import Wire.API.User.Client.Prekey @@ -83,15 +82,6 @@ tests m opts brig cannon fedBrigClient = test m "POST /federation/api-version : 200" (testAPIVersion brig fedBrigClient) ] -setSearchPolicy :: Brig -> Domain -> FederatedUserSearchPolicy -> WaiTest.Session () -setSearchPolicy brig domain policy = do - let req = brig . path "/i/federation/remotes" . Bilge.json (FD.FederationDomainConfig domain policy) - post req - !!! const 200 === statusCode - -allowFullSearch :: Brig -> Domain -> WaiTest.Session () -allowFullSearch brig domain = setSearchPolicy brig domain FullSearch - testSearchSuccess :: Opt.Opts -> Brig -> Http () testSearchSuccess opts brig = do (handle, user) <- createUserWithHandle brig diff --git a/services/brig/test/integration/Federation/End2end.hs b/services/brig/test/integration/Federation/End2end.hs index f3f2e977b16..b12908628bf 100644 --- a/services/brig/test/integration/Federation/End2end.hs +++ b/services/brig/test/integration/Federation/End2end.hs @@ -23,7 +23,6 @@ import API.User.Util import Bilge import Bilge.Assert ((!!!), ( + Domain -> Manager -> Brig -> Galley -> @@ -91,11 +90,11 @@ spec :: CargoHold -> Cannon -> IO TestTree -spec _brigOpts mg brig galley cargohold cannon _federator brigTwo galleyTwo cargoholdTwo cannonTwo = +spec originDomain mg brig galley cargohold cannon _federator brigTwo galleyTwo cargoholdTwo cannonTwo = pure $ testGroup "federation-end2end-user" - [ test mg "lookup user by qualified handle on remote backend" $ testHandleLookup brig brigTwo, + [ test mg "lookup user by qualified handle on remote backend" $ testHandleLookup originDomain brig brigTwo, test mg "search users on remote backend" $ testSearchUsers brig brigTwo, test mg "get users by ids on multiple backends" $ testGetUsersById brig brigTwo, test mg "claim client prekey" $ testClaimPrekeySuccess brig brigTwo, @@ -122,13 +121,14 @@ spec _brigOpts mg brig galley cargohold cannon _federator brigTwo galleyTwo carg -- | brig | http2 |federator| http2 |federator| http | brig | -- | +-------->+ +------->+ +--------->+ | -- +------+ +-+-------+ +---------+ +------+ -testHandleLookup :: Brig -> Brig -> Http () -testHandleLookup brig brigTwo = do +testHandleLookup :: Domain -> Brig -> Brig -> Http () +testHandleLookup originDomain brig brigTwo = do -- Create a user on the "other side" using an internal brig endpoint from a -- second brig instance in backendTwo (in another namespace in kubernetes) (handle, userBrigTwo) <- createUserWithHandle brigTwo -- Get result from brig two for comparison let domain = qDomain $ userQualifiedId userBrigTwo + allowFullSearch brigTwo originDomain resultViaBrigTwo <- getUserInfoFromHandle brigTwo domain handle -- this crashes with 404 handle not found. with "no such end-point" on v4, and with "no handle" on v1 -- query the local-namespace brig for a user sitting on the other backend diff --git a/services/brig/test/integration/Main.hs b/services/brig/test/integration/Main.hs index dcdef16cd5a..f5f79cc599e 100644 --- a/services/brig/test/integration/Main.hs +++ b/services/brig/test/integration/Main.hs @@ -46,6 +46,7 @@ import Cassandra.Util (defInitCassandra) import Control.Lens import Data.Aeson import Data.ByteString.Char8 qualified as B8 +import Data.Domain import Data.Metrics.Test (pathsConsistencyCheck) import Data.Metrics.WaiRoute (treeToPaths) import Data.Text.Encoding (encodeUtf8) @@ -106,6 +107,7 @@ data Config = Config -- external provider provider :: Provider.Config, -- for federation + originDomain :: Domain, backendTwo :: BackendConf } deriving (Show, Generic) @@ -155,7 +157,7 @@ runTests iConf brigOpts otherArgs = do createIndex <- Index.Create.spec brigOpts browseTeam <- TeamUserSearch.tests brigOpts mg g b userPendingActivation <- UserPendingActivation.tests brigOpts mg db b g s - federationEnd2End <- Federation.End2end.spec brigOpts mg b g ch c f brigTwo galleyTwo ch2 cannonTwo + federationEnd2End <- Federation.End2end.spec iConf.originDomain mg b g ch c f brigTwo galleyTwo ch2 cannonTwo federationEndpoints <- API.Federation.tests mg brigOpts b c fedBrigClient internalApi <- API.Internal.tests brigOpts mg db b (brig iConf) gd g diff --git a/services/brig/test/integration/Util.hs b/services/brig/test/integration/Util.hs index 6aafab8a991..2d83b2b4ef3 100644 --- a/services/brig/test/integration/Util.hs +++ b/services/brig/test/integration/Util.hs @@ -108,6 +108,7 @@ import Wire.API.Conversation.Role (roleNameWireAdmin) import Wire.API.Federation.API import Wire.API.Federation.Domain import Wire.API.Internal.Notification +import Wire.API.Routes.FederationDomainConfig qualified as FD import Wire.API.Routes.MultiTablePaging import Wire.API.Team.Member hiding (userId) import Wire.API.User hiding (AccountStatus (..)) @@ -118,6 +119,7 @@ import Wire.API.User.Auth.LegalHold import Wire.API.User.Auth.Sso import Wire.API.User.Client import Wire.API.User.Client.Prekey +import Wire.API.User.Search import Wire.API.VersionInfo type Brig = Request -> Request @@ -1338,3 +1340,14 @@ assertElem :: (HasCallStack, Eq a, Show a) => String -> a -> [a] -> Assertion assertElem msg x xs = unless (x `elem` xs) $ assertFailure (msg <> "\nExpected to find: \n" <> show x <> "\nin:\n" <> show xs) + +-------------------------- + +setSearchPolicy :: (MonadHttp m, MonadCatch m, MonadIO m) => Brig -> Domain -> FederatedUserSearchPolicy -> m () +setSearchPolicy brig domain policy = do + let req = brig . path "/i/federation/remotes" . Bilge.json (FD.FederationDomainConfig domain policy) + post req + !!! const 200 === statusCode + +allowFullSearch :: (MonadHttp m, MonadCatch m, MonadIO m) => Brig -> Domain -> m () +allowFullSearch brig domain = setSearchPolicy brig domain FullSearch From 195c51c5f566431aa1a68311712490d67b59c91b Mon Sep 17 00:00:00 2001 From: Igor Ranieri Date: Tue, 5 Sep 2023 15:59:05 +0200 Subject: [PATCH 35/61] Restored test assertion. --- integration/test/Test/Conversation.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/test/Test/Conversation.hs b/integration/test/Test/Conversation.hs index 945867da0a9..c03041d5c5e 100644 --- a/integration/test/Test/Conversation.hs +++ b/integration/test/Test/Conversation.hs @@ -289,7 +289,7 @@ testDefederationOneOnOne = do -- assert conversation deleted eventually retryT $ bindResponse (getConversation user convId) $ \r -> - r.status `shouldMatchInt` 422 + r.status `shouldMatchInt` 404 -- assert federation.delete event is sent twice void $ awaitNMatches 2 3 (\n -> nPayload n %. "type" `isEqual` "federation.delete") ws From 24ce4f96366b07c2c64fbc603f879058c5271b2d Mon Sep 17 00:00:00 2001 From: Igor Ranieri Date: Tue, 5 Sep 2023 16:18:37 +0200 Subject: [PATCH 36/61] Bumped timeout for offline BE notification --- integration/test/Test/Federation.hs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/integration/test/Test/Federation.hs b/integration/test/Test/Federation.hs index a16b3e9077f..f29f03e8d87 100644 --- a/integration/test/Test/Federation.hs +++ b/integration/test/Test/Federation.hs @@ -93,14 +93,14 @@ testNotificationsForOfflineBackends = do isDelUserLeaveUpConvNotif = allPreds [isConvLeaveNotif, isNotifConv upBackendConv, isNotifForUser delUser] do - newMsgNotif <- awaitNotification otherUser otherClient noValue 1 isNewMessageNotif + newMsgNotif <- awaitNotification otherUser otherClient noValue 2 isNewMessageNotif newMsgNotif %. "payload.0.qualified_conversation" `shouldMatch` objQidObject upBackendConv newMsgNotif %. "payload.0.data.text" `shouldMatchBase64` "success message for other user" - void $ awaitNotification otherUser otherClient (Just newMsgNotif) 1 isOtherUser2LeaveUpConvNotif - void $ awaitNotification otherUser otherClient (Just newMsgNotif) 1 isDelUserLeaveUpConvNotif + void $ awaitNotification otherUser otherClient (Just newMsgNotif) 2 isOtherUser2LeaveUpConvNotif + void $ awaitNotification otherUser otherClient (Just newMsgNotif) 2 isDelUserLeaveUpConvNotif - delUserDeletedNotif <- nPayload $ awaitNotification otherUser otherClient (Just newMsgNotif) 1 isDeleteUserNotif + delUserDeletedNotif <- nPayload $ awaitNotification otherUser otherClient (Just newMsgNotif) 2 isDeleteUserNotif objQid delUserDeletedNotif `shouldMatch` objQid delUser runCodensity (startDynamicBackend downBackend mempty) $ \_ -> do From b244bf04661a3d65d651901c20512bf694f543f9 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Tue, 5 Sep 2023 16:33:48 +0200 Subject: [PATCH 37/61] Better names, better types. --- .../src/Wire/API/BackgroundWorker.hs | 12 ++++++------ .../src/Wire/BackendNotificationPusher.hs | 4 ++-- services/brig/src/Brig/API/Internal.hs | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/libs/wire-api-federation/src/Wire/API/BackgroundWorker.hs b/libs/wire-api-federation/src/Wire/API/BackgroundWorker.hs index 9e646422110..93219b373b4 100644 --- a/libs/wire-api-federation/src/Wire/API/BackgroundWorker.hs +++ b/libs/wire-api-federation/src/Wire/API/BackgroundWorker.hs @@ -81,8 +81,8 @@ enqueue :: Q.Channel -> Domain -> Domain -> Q.DeliveryMode -> FedQueueClient c ( enqueue channel originDomain targetDomain deliveryMode (FedQueueClient action) = runReaderT action FedQueueEnv {..} -routingKey :: Text -> Text -routingKey t = "backend-notifications." <> t +backendNotificationQueueName :: Domain -> Text +backendNotificationQueueName (Domain t) = "backend-notifications." <> t -- | Shared values for both brig and background worker so they are -- kept in sync about what types they are expecting and where @@ -94,8 +94,8 @@ defederationQueue = "delete-federation" -- | Create a queue under routingKey (`backend-notifications`). If you want to do something -- else use `ensureQueue`. -ensureBackendNotificationsQueue :: Q.Channel -> Text -> IO () -ensureBackendNotificationsQueue chan = ensureQueue chan . routingKey +ensureBackendNotificationsQueue :: Q.Channel -> Domain -> IO () +ensureBackendNotificationsQueue chan = ensureQueue chan . backendNotificationQueueName -- | If you ever change this function and modify -- queue parameters, know that it will start failing in the @@ -174,8 +174,8 @@ instance (KnownComponent c) => RunClient (FedQueueClient c) where -- Empty string means default exchange exchange = "" liftIO $ do - ensureBackendNotificationsQueue env.channel env.targetDomain._domainText - void $ Q.publishMsg env.channel exchange (routingKey env.targetDomain._domainText) msg + ensureBackendNotificationsQueue env.channel env.targetDomain + void $ Q.publishMsg env.channel exchange (backendNotificationQueueName env.targetDomain) msg pure $ Response { responseHttpVersion = http20, diff --git a/services/background-worker/src/Wire/BackendNotificationPusher.hs b/services/background-worker/src/Wire/BackendNotificationPusher.hs index 0cf04439841..8b0ddc3a946 100644 --- a/services/background-worker/src/Wire/BackendNotificationPusher.hs +++ b/services/background-worker/src/Wire/BackendNotificationPusher.hs @@ -31,8 +31,8 @@ startPushingNotifications :: Domain -> AppT IO Q.ConsumerTag startPushingNotifications runningFlag chan domain = do - lift $ ensureBackendNotificationsQueue chan domain._domainText - QL.consumeMsgs chan (routingKey domain._domainText) Q.Ack (void . pushNotification runningFlag domain) + lift $ ensureBackendNotificationsQueue chan domain + QL.consumeMsgs chan (backendNotificationQueueName domain) Q.Ack (void . pushNotification runningFlag domain) pushNotification :: RabbitMQEnvelope e => MVar () -> Domain -> (Q.Message, e) -> AppT IO (Async ()) pushNotification runningFlag targetDomain (msg, envelope) = do diff --git a/services/brig/src/Brig/API/Internal.hs b/services/brig/src/Brig/API/Internal.hs index 3035a4cc73f..818c49bf5b8 100644 --- a/services/brig/src/Brig/API/Internal.hs +++ b/services/brig/src/Brig/API/Internal.hs @@ -305,7 +305,7 @@ deleteFederationRemote dom = do -- clean up their conversations and notify clients. -- Just to be safe! for_ (filter (/= dom) remoteDomains) $ \remoteDomain -> do - ensureBackendNotificationsQueue chan' $ domainText remoteDomain + ensureBackendNotificationsQueue chan' $ remoteDomain liftIO $ enqueue chan' ownDomain remoteDomain Q.Persistent . void @@ -314,7 +314,7 @@ deleteFederationRemote dom = do -- This will also drop all of the messages in the queue -- as we will no longer be able to communicate with this -- domain. - num <- Q.deleteQueue chan' . routingKey $ domainText dom + num <- Q.deleteQueue chan' $ backendNotificationQueueName dom Lg.info (env ^. applog) $ Log.msg @String "Dropped Notifications" . Log.field "domain" (domainText dom) . Log.field "count" (show num) -- | Remove one-on-one conversations for the given remote domain. This is called from Galley as From 8c8da684b196b6d763c8202b2f40fc1567fa4a86 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Tue, 5 Sep 2023 16:41:39 +0200 Subject: [PATCH 38/61] Cleanup. --- services/brig/test/integration/Util.hs | 2 -- 1 file changed, 2 deletions(-) diff --git a/services/brig/test/integration/Util.hs b/services/brig/test/integration/Util.hs index 2d83b2b4ef3..cccbbdc52ca 100644 --- a/services/brig/test/integration/Util.hs +++ b/services/brig/test/integration/Util.hs @@ -1341,8 +1341,6 @@ assertElem msg x xs = unless (x `elem` xs) $ assertFailure (msg <> "\nExpected to find: \n" <> show x <> "\nin:\n" <> show xs) --------------------------- - setSearchPolicy :: (MonadHttp m, MonadCatch m, MonadIO m) => Brig -> Domain -> FederatedUserSearchPolicy -> m () setSearchPolicy brig domain policy = do let req = brig . path "/i/federation/remotes" . Bilge.json (FD.FederationDomainConfig domain policy) From 12d11cb077dbbf68f0b0dc7121292379001392a9 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Tue, 5 Sep 2023 16:44:11 +0200 Subject: [PATCH 39/61] Cleanup. --- services/brig/test/integration/Federation/End2end.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/brig/test/integration/Federation/End2end.hs b/services/brig/test/integration/Federation/End2end.hs index b12908628bf..8ce7e74d521 100644 --- a/services/brig/test/integration/Federation/End2end.hs +++ b/services/brig/test/integration/Federation/End2end.hs @@ -129,7 +129,7 @@ testHandleLookup originDomain brig brigTwo = do -- Get result from brig two for comparison let domain = qDomain $ userQualifiedId userBrigTwo allowFullSearch brigTwo originDomain - resultViaBrigTwo <- getUserInfoFromHandle brigTwo domain handle -- this crashes with 404 handle not found. with "no such end-point" on v4, and with "no handle" on v1 + resultViaBrigTwo <- getUserInfoFromHandle brigTwo domain handle -- query the local-namespace brig for a user sitting on the other backend -- (which will exercise the network traffic via two federators to the remote brig) From b854f6d90917fb00dc7642271ce74520ef260596 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Tue, 5 Sep 2023 16:48:19 +0200 Subject: [PATCH 40/61] Cleanup. --- services/brig/src/Brig/User/API/Handle.hs | 3 --- 1 file changed, 3 deletions(-) diff --git a/services/brig/src/Brig/User/API/Handle.hs b/services/brig/src/Brig/User/API/Handle.hs index 6af72e28810..a554b61f0f6 100644 --- a/services/brig/src/Brig/User/API/Handle.hs +++ b/services/brig/src/Brig/User/API/Handle.hs @@ -50,9 +50,7 @@ getHandleInfo :: Qualified Handle -> (Handler r) (Maybe Public.UserProfile) getHandleInfo self handle = do - Log.err $ Log.msg (Log.val . cs $ show (self, handle)) lself <- qualifyLocal self - Log.err $ Log.msg (Log.val . cs $ show lself) foldQualified lself (getLocalHandleInfo lself . tUnqualified) @@ -61,7 +59,6 @@ getHandleInfo self handle = do getRemoteHandleInfo :: Remote Handle -> (Handler r) (Maybe Public.UserProfile) getRemoteHandleInfo handle = do - Log.err $ Log.msg (Log.val (cs $ "getRemoteHandleInfo: " <> show handle)) lift . Log.info $ Log.msg (Log.val "getHandleInfo - remote lookup") . Log.field "domain" (show (tDomain handle)) From d716699648f9dd89eb3a74c4c1e312198bf29fd8 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Tue, 5 Sep 2023 16:50:09 +0200 Subject: [PATCH 41/61] Cleanup. --- services/brig/src/Brig/Federation/Client.hs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/services/brig/src/Brig/Federation/Client.hs b/services/brig/src/Brig/Federation/Client.hs index e656f13a94b..693407ae8bc 100644 --- a/services/brig/src/Brig/Federation/Client.hs +++ b/services/brig/src/Brig/Federation/Client.hs @@ -26,7 +26,7 @@ import Brig.App import Control.Lens import Control.Monad import Control.Monad.Catch (MonadMask, throwM) -import Control.Monad.Trans.Except (ExceptT (..), runExceptT, throwE) +import Control.Monad.Trans.Except (ExceptT (..), throwE) import Control.Retry import Control.Timeout import Data.Domain @@ -58,9 +58,7 @@ getUserHandleInfo :: ExceptT FederationError m (Maybe UserProfile) getUserHandleInfo (tUntagged -> Qualified handle domain) = do lift $ Log.info $ Log.msg $ T.pack "Brig-federation: handle lookup call on remote backend" - x <- runExceptT (runBrigFederatorClient domain $ fedClient @'Brig @"get-user-by-handle" handle) - lift $ Log.err $ Log.msg $ cs @_ @ByteString ("F.getUserHandleInfo: " <> show x) - either throwE pure x + runBrigFederatorClient domain $ fedClient @'Brig @"get-user-by-handle" handle getUsersByIds :: ( MonadReader Env m, From 055f55dfdff03f1b7b3b37791019d63ff508ccc7 Mon Sep 17 00:00:00 2001 From: Igor Ranieri Date: Wed, 6 Sep 2023 09:32:42 +0200 Subject: [PATCH 42/61] hi ci From 3ac378bd348147b0de928302fef95d631211720b Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Wed, 6 Sep 2023 14:39:07 +0200 Subject: [PATCH 43/61] Fix: AllowAll has default search policy FullSearch. --- changelog.d/1-api-changes/WPB-3796 | 1 + services/brig/src/Brig/API/Federation.hs | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 changelog.d/1-api-changes/WPB-3796 diff --git a/changelog.d/1-api-changes/WPB-3796 b/changelog.d/1-api-changes/WPB-3796 new file mode 100644 index 00000000000..ee95fee0b71 --- /dev/null +++ b/changelog.d/1-api-changes/WPB-3796 @@ -0,0 +1 @@ +Change default search policy for `"allowAll"` to `"full_search"`. (Reminder: `"allowAll"` is for testing only, and not supported for production use!) diff --git a/services/brig/src/Brig/API/Federation.hs b/services/brig/src/Brig/API/Federation.hs index c8da1a5e702..0d424c929f8 100644 --- a/services/brig/src/Brig/API/Federation.hs +++ b/services/brig/src/Brig/API/Federation.hs @@ -242,4 +242,8 @@ lookupSearchPolicy :: Domain -> (Handler r) FederatedUserSearchPolicy lookupSearchPolicy domain = do domainConfigs <- getFederationRemotes let mConfig = find ((== domain) . FD.domain) (domainConfigs.remotes) - pure $ maybe NoSearch FD.cfgSearchPolicy mConfig + defaultPolicy = case domainConfigs.strategy of + AllowNone -> NoSearch + AllowDynamic -> NoSearch + AllowAll -> FullSearch + pure $ maybe defaultPolicy FD.cfgSearchPolicy mConfig From 29e342903f9d2b0ac61effbefa96cb2a48cb9e30 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Wed, 6 Sep 2023 16:53:42 +0200 Subject: [PATCH 44/61] Revert "Fix: AllowAll has default search policy FullSearch." This reverts commit 3ac378bd348147b0de928302fef95d631211720b. --- changelog.d/1-api-changes/WPB-3796 | 1 - services/brig/src/Brig/API/Federation.hs | 6 +----- 2 files changed, 1 insertion(+), 6 deletions(-) delete mode 100644 changelog.d/1-api-changes/WPB-3796 diff --git a/changelog.d/1-api-changes/WPB-3796 b/changelog.d/1-api-changes/WPB-3796 deleted file mode 100644 index ee95fee0b71..00000000000 --- a/changelog.d/1-api-changes/WPB-3796 +++ /dev/null @@ -1 +0,0 @@ -Change default search policy for `"allowAll"` to `"full_search"`. (Reminder: `"allowAll"` is for testing only, and not supported for production use!) diff --git a/services/brig/src/Brig/API/Federation.hs b/services/brig/src/Brig/API/Federation.hs index 0d424c929f8..c8da1a5e702 100644 --- a/services/brig/src/Brig/API/Federation.hs +++ b/services/brig/src/Brig/API/Federation.hs @@ -242,8 +242,4 @@ lookupSearchPolicy :: Domain -> (Handler r) FederatedUserSearchPolicy lookupSearchPolicy domain = do domainConfigs <- getFederationRemotes let mConfig = find ((== domain) . FD.domain) (domainConfigs.remotes) - defaultPolicy = case domainConfigs.strategy of - AllowNone -> NoSearch - AllowDynamic -> NoSearch - AllowAll -> FullSearch - pure $ maybe defaultPolicy FD.cfgSearchPolicy mConfig + pure $ maybe NoSearch FD.cfgSearchPolicy mConfig From b2460369f18ca727afb97fe4b276d72811a9a4b4 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Thu, 7 Sep 2023 11:44:52 +0200 Subject: [PATCH 45/61] Fix federator integration tests. All green now? --- .../test/integration/Test/Federator/IngressSpec.hs | 10 +++++++++- .../test/integration/Test/Federator/InwardSpec.hs | 14 ++++++++++++-- .../test/integration/Test/Federator/Util.hs | 11 +++++++++++ 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/services/federator/test/integration/Test/Federator/IngressSpec.hs b/services/federator/test/integration/Test/Federator/IngressSpec.hs index a93a4fa78bf..bb5bb8d852a 100644 --- a/services/federator/test/integration/Test/Federator/IngressSpec.hs +++ b/services/federator/test/integration/Test/Federator/IngressSpec.hs @@ -17,7 +17,7 @@ module Test.Federator.IngressSpec where -import Control.Lens (view) +import Control.Lens (to, view, (^.)) import Control.Monad.Catch (throwM) import Control.Monad.Codensity import Data.Aeson qualified as Aeson @@ -46,6 +46,7 @@ import Wire.API.Federation.Client import Wire.API.Federation.Component import Wire.API.Federation.Domain import Wire.API.User +import Wire.API.User.Search (FederatedUserSearchPolicy (..)) import Wire.Network.DNS.SRV -- | This module contains tests for the interface between federator and ingress. Ingress is @@ -56,6 +57,13 @@ spec env = do it "should be accessible using http2 and forward to the local brig" $ runTestFederator env $ do brig <- view teBrig <$> ask + let domain = + -- FUTUREWORK: we need to come up with a more + -- parallelisable way to test this. as of now, this + -- comes from the + env ^. teTstOpts . to originDomain + setSearchPolicyFor brig (Domain domain) FullSearch + user <- randomUser brig hdl <- randomHandle _ <- putHandle brig (userId user) hdl diff --git a/services/federator/test/integration/Test/Federator/InwardSpec.hs b/services/federator/test/integration/Test/Federator/InwardSpec.hs index ae267dd67e8..8a9e34e311e 100644 --- a/services/federator/test/integration/Test/Federator/InwardSpec.hs +++ b/services/federator/test/integration/Test/Federator/InwardSpec.hs @@ -22,12 +22,13 @@ module Test.Federator.InwardSpec where import Bilge import Bilge.Assert -import Control.Lens (view) +import Control.Lens (to, view, (^.)) import Data.Aeson import Data.Aeson.Types qualified as Aeson import Data.ByteString qualified as BS import Data.ByteString.Conversion (toByteString') import Data.ByteString.Lazy qualified as LBS +import Data.Domain import Data.Handle import Data.LegalHold (UserLegalHoldStatus (UserLegalHoldNoConsent)) import Data.Text.Encoding @@ -41,7 +42,9 @@ import Test.QuickCheck (arbitrary, generate) import Util.Options (Endpoint (Endpoint)) import Wire.API.Federation.API.Cargohold import Wire.API.Federation.Domain +import Wire.API.Routes.FederationDomainConfig import Wire.API.User +import Wire.API.User.Search (FederatedUserSearchPolicy (..)) -- FUTUREWORK(federation): move these tests to brig-integration (benefit: avoid duplicating all of the brig helper code) -- FUTUREWORK(fisx): better yet, reorganize integration tests (or at least the helpers) so @@ -70,6 +73,13 @@ spec env = it "should be able to call brig" $ runTestFederator env $ do brig <- view teBrig <$> ask + let domain = + -- FUTUREWORK: we need to come up with a more + -- parallelisable way to test this. as of now, this + -- comes from the + env ^. teTstOpts . to originDomain + setSearchPolicyFor brig (Domain domain) ExactHandleSearch + user <- randomUser brig hdl <- randomHandle _ <- putHandle brig (userId user) hdl @@ -77,7 +87,7 @@ spec env = let expectedProfile = (publicProfile user UserLegalHoldNoConsent) {profileHandle = Just (Handle hdl)} bdy <- responseJsonError - =<< inwardCall "/federation/brig/get-user-by-handle" (encode hdl) + =<< inwardCallWithOriginDomain (cs domain) "/federation/brig/get-user-by-handle" (encode hdl) Request @@ -347,3 +350,11 @@ assertNoError = runError >=> \case Left err -> embed @IO . assertFailure $ "Unexpected error: " <> show err Right x -> pure x + +setSearchPolicyFor :: BrigReq -> Domain -> FederatedUserSearchPolicy -> (Functor m, MonadHttp m) => m () +setSearchPolicyFor brig domain policy = + void $ + post $ + brig + . paths ["/i/federation/remotes"] + . Bilge.json (FederationDomainConfig domain policy) From d47e4df587a1382fa26a769902e1ecbc20df6b15 Mon Sep 17 00:00:00 2001 From: Igor Ranieri Date: Thu, 7 Sep 2023 13:18:59 +0200 Subject: [PATCH 46/61] hi ci From f32375d630847475a1676f92e219088e55120db7 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Thu, 7 Sep 2023 13:27:45 +0200 Subject: [PATCH 47/61] Fixup --- services/federator/test/integration/Test/Federator/InwardSpec.hs | 1 - 1 file changed, 1 deletion(-) diff --git a/services/federator/test/integration/Test/Federator/InwardSpec.hs b/services/federator/test/integration/Test/Federator/InwardSpec.hs index 8a9e34e311e..17cd5d46fa6 100644 --- a/services/federator/test/integration/Test/Federator/InwardSpec.hs +++ b/services/federator/test/integration/Test/Federator/InwardSpec.hs @@ -42,7 +42,6 @@ import Test.QuickCheck (arbitrary, generate) import Util.Options (Endpoint (Endpoint)) import Wire.API.Federation.API.Cargohold import Wire.API.Federation.Domain -import Wire.API.Routes.FederationDomainConfig import Wire.API.User import Wire.API.User.Search (FederatedUserSearchPolicy (..)) From 2a86f023456cb0f00640a235b54166dabda19fb3 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Thu, 7 Sep 2023 14:13:19 +0200 Subject: [PATCH 48/61] Fixup --- services/brig/test/integration/API/Federation.hs | 1 - 1 file changed, 1 deletion(-) diff --git a/services/brig/test/integration/API/Federation.hs b/services/brig/test/integration/API/Federation.hs index 67f89f8542e..57719a42722 100644 --- a/services/brig/test/integration/API/Federation.hs +++ b/services/brig/test/integration/API/Federation.hs @@ -24,7 +24,6 @@ import Bilge hiding (head) import Bilge.Assert import Brig.Options qualified as Opt import Control.Arrow (Arrow (first), (&&&)) -import Control.Lens ((?~)) import Data.Aeson import Data.Domain (Domain (Domain)) import Data.Handle (Handle (..)) From b0f3e4d512d2d6adae631d783fa43502a285394f Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Thu, 7 Sep 2023 17:29:03 +0200 Subject: [PATCH 49/61] hi ci From 4f246bf5e1fede7556987a0e873d360bec513971 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 8 Sep 2023 11:02:54 +0200 Subject: [PATCH 50/61] Re-align brig-integration config file on ci. --- charts/brig/templates/tests/configmap.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/charts/brig/templates/tests/configmap.yaml b/charts/brig/templates/tests/configmap.yaml index 56667e55ed3..4ccf6773513 100644 --- a/charts/brig/templates/tests/configmap.yaml +++ b/charts/brig/templates/tests/configmap.yaml @@ -87,3 +87,5 @@ data: federatorExternal: host: federator.{{ .Release.Namespace }}-fed2.svc.cluster.local port: 8081 + + originDomain: 6da818ea-4e26-11ee-9422-9b252c5bbf2b.example.com From 4d6e6da82f84fb8bb65f57ab5870404f2f9d4993 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 8 Sep 2023 11:09:18 +0200 Subject: [PATCH 51/61] Increase time to wait for events in /integration. --- integration/test/Test/Federation.hs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration/test/Test/Federation.hs b/integration/test/Test/Federation.hs index f29f03e8d87..8b28e1d9115 100644 --- a/integration/test/Test/Federation.hs +++ b/integration/test/Test/Federation.hs @@ -97,8 +97,8 @@ testNotificationsForOfflineBackends = do newMsgNotif %. "payload.0.qualified_conversation" `shouldMatch` objQidObject upBackendConv newMsgNotif %. "payload.0.data.text" `shouldMatchBase64` "success message for other user" - void $ awaitNotification otherUser otherClient (Just newMsgNotif) 2 isOtherUser2LeaveUpConvNotif - void $ awaitNotification otherUser otherClient (Just newMsgNotif) 2 isDelUserLeaveUpConvNotif + void $ awaitNotification otherUser otherClient (Just newMsgNotif) 10 isOtherUser2LeaveUpConvNotif + void $ awaitNotification otherUser otherClient (Just newMsgNotif) 10 isDelUserLeaveUpConvNotif delUserDeletedNotif <- nPayload $ awaitNotification otherUser otherClient (Just newMsgNotif) 2 isDeleteUserNotif objQid delUserDeletedNotif `shouldMatch` objQid delUser From 06e21e20f871976dc1d5f996bbce533a7b77e9a1 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 8 Sep 2023 13:54:01 +0200 Subject: [PATCH 52/61] Fixup? --- charts/brig/templates/tests/configmap.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/charts/brig/templates/tests/configmap.yaml b/charts/brig/templates/tests/configmap.yaml index 4ccf6773513..966b57bc48a 100644 --- a/charts/brig/templates/tests/configmap.yaml +++ b/charts/brig/templates/tests/configmap.yaml @@ -58,6 +58,8 @@ data: cert: /etc/wire/integration-secrets/provider-publiccert.pem botHost: https://brig-integration + originDomain: 612c6a18-4e3e-11ee-a205-97507f092421.example.com + backendTwo: brig: host: brig.{{ .Release.Namespace }}-fed2.svc.cluster.local From a6927babe5988247720d7fc15c92a986d7dbf583 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Sat, 9 Sep 2023 22:03:09 +0200 Subject: [PATCH 53/61] Fixup! --- services/brig/test/integration/Federation/End2end.hs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/services/brig/test/integration/Federation/End2end.hs b/services/brig/test/integration/Federation/End2end.hs index efcd18c8e3b..ca36e740471 100644 --- a/services/brig/test/integration/Federation/End2end.hs +++ b/services/brig/test/integration/Federation/End2end.hs @@ -122,13 +122,14 @@ testHandleLookup originDomain brig brigTwo = do -- second brig instance in backendTwo (in another namespace in kubernetes) (handle, userBrigTwo) <- createUserWithHandle brigTwo -- Get result from brig two for comparison - let domain = qDomain $ userQualifiedId userBrigTwo + let backendTwoDomain = qDomain $ userQualifiedId userBrigTwo allowFullSearch brigTwo originDomain - resultViaBrigTwo <- getUserInfoFromHandle brigTwo domain handle + resultViaBrigTwo <- getUserInfoFromHandle brigTwo backendTwoDomain handle -- query the local-namespace brig for a user sitting on the other backend -- (which will exercise the network traffic via two federators to the remote brig) - resultViaBrigOne <- getUserInfoFromHandle brig domain handle + allowFullSearch brig backendTwoDomain + resultViaBrigOne <- getUserInfoFromHandle brig backendTwoDomain handle liftIO $ assertEqual "remote handle lookup via federator should work in the happy case" (profileQualifiedId resultViaBrigOne) (userQualifiedId userBrigTwo) liftIO $ assertEqual "querying brig1 or brig2 about the same user should give same result" resultViaBrigTwo resultViaBrigOne From 7c8bf45b151e3bab31be95f5fbeed141910fdd30 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Sun, 10 Sep 2023 18:04:35 +0200 Subject: [PATCH 54/61] *sigh* --- services/brig/test/integration/Federation/End2end.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/brig/test/integration/Federation/End2end.hs b/services/brig/test/integration/Federation/End2end.hs index ca36e740471..7e365239717 100644 --- a/services/brig/test/integration/Federation/End2end.hs +++ b/services/brig/test/integration/Federation/End2end.hs @@ -129,7 +129,7 @@ testHandleLookup originDomain brig brigTwo = do -- query the local-namespace brig for a user sitting on the other backend -- (which will exercise the network traffic via two federators to the remote brig) allowFullSearch brig backendTwoDomain - resultViaBrigOne <- getUserInfoFromHandle brig backendTwoDomain handle + resultViaBrigOne <- getUserInfoFromHandle brig backendTwoDomain handle liftIO $ assertEqual "remote handle lookup via federator should work in the happy case" (profileQualifiedId resultViaBrigOne) (userQualifiedId userBrigTwo) liftIO $ assertEqual "querying brig1 or brig2 about the same user should give same result" resultViaBrigTwo resultViaBrigOne From 975f142a0eb3cbea62b53696e98f6eef0953f3e8 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 11 Sep 2023 09:54:58 +0200 Subject: [PATCH 55/61] ?! --- services/brig/test/integration/Federation/End2end.hs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/services/brig/test/integration/Federation/End2end.hs b/services/brig/test/integration/Federation/End2end.hs index 7e365239717..2e891236bd6 100644 --- a/services/brig/test/integration/Federation/End2end.hs +++ b/services/brig/test/integration/Federation/End2end.hs @@ -124,11 +124,14 @@ testHandleLookup originDomain brig brigTwo = do -- Get result from brig two for comparison let backendTwoDomain = qDomain $ userQualifiedId userBrigTwo allowFullSearch brigTwo originDomain + allowFullSearch brig backendTwoDomain + allowFullSearch brigTwo backendTwoDomain + allowFullSearch brig originDomain + -- TODO: ?! resultViaBrigTwo <- getUserInfoFromHandle brigTwo backendTwoDomain handle -- query the local-namespace brig for a user sitting on the other backend -- (which will exercise the network traffic via two federators to the remote brig) - allowFullSearch brig backendTwoDomain resultViaBrigOne <- getUserInfoFromHandle brig backendTwoDomain handle liftIO $ assertEqual "remote handle lookup via federator should work in the happy case" (profileQualifiedId resultViaBrigOne) (userQualifiedId userBrigTwo) From 5f89f966859defef30992368ba5e83f120be2315 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 11 Sep 2023 10:49:27 +0200 Subject: [PATCH 56/61] ... --- .../test/integration/Federation/End2end.hs | 14 +++++-------- services/brig/test/integration/Util.hs | 20 +++++++++++++++++++ 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/services/brig/test/integration/Federation/End2end.hs b/services/brig/test/integration/Federation/End2end.hs index 2e891236bd6..b63f408e50a 100644 --- a/services/brig/test/integration/Federation/End2end.hs +++ b/services/brig/test/integration/Federation/End2end.hs @@ -123,19 +123,15 @@ testHandleLookup originDomain brig brigTwo = do (handle, userBrigTwo) <- createUserWithHandle brigTwo -- Get result from brig two for comparison let backendTwoDomain = qDomain $ userQualifiedId userBrigTwo - allowFullSearch brigTwo originDomain - allowFullSearch brig backendTwoDomain - allowFullSearch brigTwo backendTwoDomain - allowFullSearch brig originDomain - -- TODO: ?! - resultViaBrigTwo <- getUserInfoFromHandle brigTwo backendTwoDomain handle + mbResultViaBrigTwo <- getUserInfoFromHandle' brigTwo backendTwoDomain handle -- query the local-namespace brig for a user sitting on the other backend -- (which will exercise the network traffic via two federators to the remote brig) - resultViaBrigOne <- getUserInfoFromHandle brig backendTwoDomain handle + allowFullSearch brigTwo originDomain + mbResultViaBrigOne <- getUserInfoFromHandle' brig backendTwoDomain handle - liftIO $ assertEqual "remote handle lookup via federator should work in the happy case" (profileQualifiedId resultViaBrigOne) (userQualifiedId userBrigTwo) - liftIO $ assertEqual "querying brig1 or brig2 about the same user should give same result" resultViaBrigTwo resultViaBrigOne + liftIO $ assertEqual "remote handle lookup via federator should work in the happy case" (profileQualifiedId <$> mbResultViaBrigOne) (Just $ userQualifiedId userBrigTwo) + liftIO $ assertEqual "querying brig1 or brig2 about the same user should give same result" mbResultViaBrigTwo mbResultViaBrigOne testSearchUsers :: Brig -> Brig -> Http () testSearchUsers brig brigTwo = do diff --git a/services/brig/test/integration/Util.hs b/services/brig/test/integration/Util.hs index cccbbdc52ca..550b4907c5b 100644 --- a/services/brig/test/integration/Util.hs +++ b/services/brig/test/integration/Util.hs @@ -649,6 +649,26 @@ getUserInfoFromHandle brig domain handle = do . expect2xx ) +getUserInfoFromHandle' :: + (MonadIO m, MonadCatch m, MonadHttp m, HasCallStack) => + Brig -> + Domain -> + Handle -> + m (Maybe UserProfile) +getUserInfoFromHandle' brig domain handle = do + u <- randomId + resp <- + get + ( apiVersion "v1" + . brig + . paths ["users", "by-handle", toByteString' (domainText domain), toByteString' handle] + . zUser u + ) + case HTTP.statusCode $ responseStatus resp of + 200 -> responseJsonError resp + 404 -> pure Nothing + bad -> error $ "unexpected: " <> show bad + addClient :: (MonadHttp m, HasCallStack) => Brig -> From 23b71308250f36852e4f0f0542ff107b3d466fb9 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 11 Sep 2023 11:50:48 +0200 Subject: [PATCH 57/61] Makefile: clean up federation_remotes before `make ci-fast`. --- Makefile | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index b2706f460a2..5f47141ff25 100644 --- a/Makefile +++ b/Makefile @@ -95,7 +95,7 @@ endif # # If you want to pass arguments to the test-suite call cabal-run-integration.sh directly. .PHONY: ci-fast -ci-fast: c db-migrate +ci-fast: c db-migrate cqlsh_clear_federation_remotes ifeq ("$(package)", "all") ./hack/bin/cabal-run-integration.sh all ./hack/bin/cabal-run-integration.sh integration @@ -270,6 +270,18 @@ cqlsh: @echo "make sure you have ./deploy/dockerephemeral/run.sh running in another window!" docker exec -it $(CASSANDRA_CONTAINER) /usr/bin/cqlsh +.PHONY: cqlsh_clear_federation_remotes +cqlsh_clear_federation_remotes: + $(eval CASSANDRA_CONTAINER := $(shell docker ps | grep '/cassandra:' | perl -ne '/^(\S+)\s/ && print $$1')) + @echo "make sure you have ./deploy/dockerephemeral/run.sh running in another window!" + for ts in brig_test{,2} brig_dyn1_test brig_dyn2_test brig_dyn3_test; do echo "truncate $ts.federation_remotes if exists" ; done + echo "truncate brig_test.federation_remotes; \ + truncate brig_test2.federation_remotes; \ + truncate brig_test_dyn_1.federation_remotes; \ + truncate brig_test_dyn_2.federation_remotes; \ + truncate brig_test_dyn_3.federation_remotes;" | \ + docker exec -i $(CASSANDRA_CONTAINER) /usr/bin/cqlsh + .PHONY: db-reset-package db-reset-package: @echo "Deprecated! Please use 'db-reset' instead" From 4c346684a71e5f8e2d53239ae23706a5228feb3b Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 11 Sep 2023 12:15:12 +0200 Subject: [PATCH 58/61] Fix one more. --- .../brig/test/integration/Federation/End2end.hs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/services/brig/test/integration/Federation/End2end.hs b/services/brig/test/integration/Federation/End2end.hs index b63f408e50a..cf1d19f89eb 100644 --- a/services/brig/test/integration/Federation/End2end.hs +++ b/services/brig/test/integration/Federation/End2end.hs @@ -58,6 +58,7 @@ import Wire.API.Routes.MultiTablePaging import Wire.API.User hiding (assetKey) import Wire.API.User.Client import Wire.API.User.Client.Prekey +import Wire.API.User.Search (FederatedUserSearchPolicy (FullSearch)) -- NOTE: These federation tests require deploying two sets of (some) services -- This might be best left to a kubernetes setup. @@ -91,7 +92,7 @@ spec originDomain mg brig galley cargohold cannon _federator brigTwo galleyTwo c testGroup "federation-end2end-user" [ test mg "lookup user by qualified handle on remote backend" $ testHandleLookup originDomain brig brigTwo, - test mg "search users on remote backend" $ testSearchUsers brig brigTwo, + test mg "search users on remote backend" $ testSearchUsers originDomain brig brigTwo, test mg "get users by ids on multiple backends" $ testGetUsersById brig brigTwo, test mg "claim client prekey" $ testClaimPrekeySuccess brig brigTwo, test mg "claim prekey bundle" $ testClaimPrekeyBundleSuccess brig brigTwo, @@ -133,8 +134,8 @@ testHandleLookup originDomain brig brigTwo = do liftIO $ assertEqual "remote handle lookup via federator should work in the happy case" (profileQualifiedId <$> mbResultViaBrigOne) (Just $ userQualifiedId userBrigTwo) liftIO $ assertEqual "querying brig1 or brig2 about the same user should give same result" mbResultViaBrigTwo mbResultViaBrigOne -testSearchUsers :: Brig -> Brig -> Http () -testSearchUsers brig brigTwo = do +testSearchUsers :: Domain -> Brig -> Brig -> Http () +testSearchUsers originDomain brig brigTwo = do -- Create a user on the "other side" using an internal brig endpoint from a -- second brig instance in backendTwo (in another namespace in kubernetes) (handle, userBrigTwo) <- createUserWithHandle brigTwo @@ -142,13 +143,14 @@ testSearchUsers brig brigTwo = do searcher <- userId <$> randomUser brig let expectedUserId = userQualifiedId userBrigTwo searchTerm = fromHandle handle - domain = qDomain expectedUserId + domainTwo = qDomain expectedUserId liftIO $ putStrLn "search for user on brigTwo (directly)..." - assertCanFindWithDomain brigTwo searcher expectedUserId searchTerm domain + assertCanFindWithDomain brigTwo searcher expectedUserId searchTerm domainTwo -- exercises multi-backend network traffic liftIO $ putStrLn "search for user on brigOne via federators to remote brig..." - assertCanFindWithDomain brig searcher expectedUserId searchTerm domain + setSearchPolicy brigTwo originDomain FullSearch + assertCanFindWithDomain brig searcher expectedUserId searchTerm domainTwo testGetUsersById :: Brig -> Brig -> Http () testGetUsersById brig1 brig2 = do From 5f74ad8611efbb8c9b6f1adfd033d78b54c05ddb Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 11 Sep 2023 13:05:41 +0200 Subject: [PATCH 59/61] Nit-pick. --- services/brig/test/integration/Federation/End2end.hs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/brig/test/integration/Federation/End2end.hs b/services/brig/test/integration/Federation/End2end.hs index cf1d19f89eb..c9a2650160a 100644 --- a/services/brig/test/integration/Federation/End2end.hs +++ b/services/brig/test/integration/Federation/End2end.hs @@ -131,8 +131,8 @@ testHandleLookup originDomain brig brigTwo = do allowFullSearch brigTwo originDomain mbResultViaBrigOne <- getUserInfoFromHandle' brig backendTwoDomain handle - liftIO $ assertEqual "remote handle lookup via federator should work in the happy case" (profileQualifiedId <$> mbResultViaBrigOne) (Just $ userQualifiedId userBrigTwo) - liftIO $ assertEqual "querying brig1 or brig2 about the same user should give same result" mbResultViaBrigTwo mbResultViaBrigOne + liftIO $ assertEqual "remote handle lookup via federator should work in the happy case" (Just $ userQualifiedId userBrigTwo) (profileQualifiedId <$> mbResultViaBrigOne) + liftIO $ assertEqual "querying brig1 or brig2 about the same user should give same result" mbResultViaBrigOne mbResultViaBrigTwo testSearchUsers :: Domain -> Brig -> Brig -> Http () testSearchUsers originDomain brig brigTwo = do From e65ef0ba373e6e0a816e48fa379efe917b4e63cc Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Tue, 12 Sep 2023 13:54:05 +0200 Subject: [PATCH 60/61] ... --- services/brig/test/integration/Federation/End2end.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/brig/test/integration/Federation/End2end.hs b/services/brig/test/integration/Federation/End2end.hs index c9a2650160a..c19ecadc35a 100644 --- a/services/brig/test/integration/Federation/End2end.hs +++ b/services/brig/test/integration/Federation/End2end.hs @@ -131,8 +131,8 @@ testHandleLookup originDomain brig brigTwo = do allowFullSearch brigTwo originDomain mbResultViaBrigOne <- getUserInfoFromHandle' brig backendTwoDomain handle - liftIO $ assertEqual "remote handle lookup via federator should work in the happy case" (Just $ userQualifiedId userBrigTwo) (profileQualifiedId <$> mbResultViaBrigOne) liftIO $ assertEqual "querying brig1 or brig2 about the same user should give same result" mbResultViaBrigOne mbResultViaBrigTwo + liftIO $ assertEqual "remote handle lookup via federator should work in the happy case" (Just $ userQualifiedId userBrigTwo) (profileQualifiedId <$> mbResultViaBrigOne) testSearchUsers :: Domain -> Brig -> Brig -> Http () testSearchUsers originDomain brig brigTwo = do From 43dd731e83f9902ede612f9259bf6695d12fb464 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Wed, 13 Sep 2023 10:46:24 +0200 Subject: [PATCH 61/61] stash --- .../templates/integration-integration.yaml | 4 +++ docs/src/developer/developer/how-to.md | 28 ++++++++++++++++--- hack/bin/integration-test.sh | 2 +- integration/test/Test/Federation.hs | 2 +- services/brig/src/Brig/User/API/Handle.hs | 2 +- 5 files changed, 31 insertions(+), 7 deletions(-) diff --git a/charts/integration/templates/integration-integration.yaml b/charts/integration/templates/integration-integration.yaml index f4ad967a975..dc30a869406 100644 --- a/charts/integration/templates/integration-integration.yaml +++ b/charts/integration/templates/integration-integration.yaml @@ -207,3 +207,7 @@ spec: secretKeyRef: name: brig key: rabbitmqPassword + - name: TEST_INCLUDE + value: "" + - name: TASTY_PATTERN + value: "(!/turn/&&!/user.auth.cookies.limit/)&&/search users on remote backend/" diff --git a/docs/src/developer/developer/how-to.md b/docs/src/developer/developer/how-to.md index 6c7cc0e1cb2..2d47e9857b0 100644 --- a/docs/src/developer/developer/how-to.md +++ b/docs/src/developer/developer/how-to.md @@ -108,9 +108,7 @@ make kube-integration-setup #### Deploy your local code to a kind cluster -This can be useful to get quicker feedback while working on multi-backend code or configuration (e.g. helm charts) than to wait an hour for CI. This allows you to test code without uploading it to github and waiting an hour for CI. - -FUTUREWORK: this process is in development (update this section after it's confirmed to work): +This can be useful to get quicker feedback while working on multi-backend code or configuration (e.g. helm charts), or integration tests. It saves you uploading everything to github and waiting an hour for CI. ##### Run tests in kind @@ -119,11 +117,33 @@ FUTUREWORK: this process is in development (update this section after it's confi *Note:* First time all the images need to be uploaded. When working on one service it can be selectively uploaded using `make kind-upload-image-` - (e.g. `make kind-upload-image-brig`). + (e.g. `make kind-upload-image-{brig,integration}`). 2. Install wire-server using `make kind-integration-setup`. 3. Run tests using `make kind-integration-test`. 4. Run end2end integration tests: `make kind-integration-e2e`. +If you want to run only a particular test suite, [edit this line](https://github.com/wireapp/wire-server/blob/2dfc268f233cc7b42a75243f2fd0071b81aef65b/hack/bin/integration-test.sh#L14). + +If you want to run only a particular test *case*, you can add env variables like `TASTY_PATTERN` or `TEST_INCLUDE` to [this block](https://github.com/wireapp/wire-server/blob/2dfc268f233cc7b42a75243f2fd0071b81aef65b/charts/integration/templates/integration-integration.yaml#L192-L209). + +To shut everything down (sometimes RAM / CPU use stays high indefinitely otherwise): + +1. `make kind-delete` + +---------------------------------------------------------------------- + +make kind-integration-setup +make kind-integration-test + + + +[needs kind] make ci-safe package=integration TEST_INCLUDE=Federation.testNotificationsForOfflineBackends + + +[fixed?] make ci-safe package=brig TASTY_PATTERN='(!/turn/&&!/user.auth.cookies.limit/)&&/search users on remote backend/' +[fixed?] make ci-safe package=brig TASTY_PATTERN='(!/turn/&&!/user.auth.cookies.limit/)&&/lookup user by qualified handle on remote backend/' + + #### Deploy your local code to a kubernetes cluster diff --git a/hack/bin/integration-test.sh b/hack/bin/integration-test.sh index 841a3172fd8..b3cc5bdee9b 100755 --- a/hack/bin/integration-test.sh +++ b/hack/bin/integration-test.sh @@ -11,7 +11,7 @@ UPLOAD_LOGS=${UPLOAD_LOGS:-0} echo "Running integration tests on wire-server with parallelism=${HELM_PARALLELISM} ..." CHART=wire-server -tests=(integration stern galley cargohold gundeck federator spar brig) +tests=(integration) # brig cleanup() { if (( CLEANUP_LOCAL_FILES > 0 )); then diff --git a/integration/test/Test/Federation.hs b/integration/test/Test/Federation.hs index 8b28e1d9115..f81e9ec8b86 100644 --- a/integration/test/Test/Federation.hs +++ b/integration/test/Test/Federation.hs @@ -97,7 +97,7 @@ testNotificationsForOfflineBackends = do newMsgNotif %. "payload.0.qualified_conversation" `shouldMatch` objQidObject upBackendConv newMsgNotif %. "payload.0.data.text" `shouldMatchBase64` "success message for other user" - void $ awaitNotification otherUser otherClient (Just newMsgNotif) 10 isOtherUser2LeaveUpConvNotif + void $ awaitNotification otherUser otherClient (Just newMsgNotif) 10 isOtherUser2LeaveUpConvNotif -- TODO: this is still not enough. something else wrong, then? but why only sometimes? void $ awaitNotification otherUser otherClient (Just newMsgNotif) 10 isDelUserLeaveUpConvNotif delUserDeletedNotif <- nPayload $ awaitNotification otherUser otherClient (Just newMsgNotif) 2 isDeleteUserNotif diff --git a/services/brig/src/Brig/User/API/Handle.hs b/services/brig/src/Brig/User/API/Handle.hs index a554b61f0f6..908667f67d4 100644 --- a/services/brig/src/Brig/User/API/Handle.hs +++ b/services/brig/src/Brig/User/API/Handle.hs @@ -53,7 +53,7 @@ getHandleInfo self handle = do lself <- qualifyLocal self foldQualified lself - (getLocalHandleInfo lself . tUnqualified) + (getLocalHandleInfo lself . tUnqualified) -- does it call local or remote? how does local work? getRemoteHandleInfo handle