Skip to content

Commit

Permalink
Disable de-federation to avoid running into a scalability issue (#3582)
Browse files Browse the repository at this point in the history
https://wearezeta.atlassian.net/browse/WPB-4668

Co-authored-by: Akshay Mankar <akshay@wire.com>
  • Loading branch information
2 people authored and supersven committed Oct 4, 2023
1 parent 57698c3 commit 99d3f5e
Show file tree
Hide file tree
Showing 51 changed files with 148 additions and 2,079 deletions.
5 changes: 0 additions & 5 deletions changelog.d/1-api-changes/WPB-3611

This file was deleted.

1 change: 1 addition & 0 deletions changelog.d/1-api-changes/WPB-4668-disable-defederation
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove de-federation (to avoid a scalability issue).
1 change: 0 additions & 1 deletion changelog.d/3-bug-fixes/duplicate-member-notifications

This file was deleted.

1 change: 0 additions & 1 deletion changelog.d/6-federation/WPB-3611

This file was deleted.

9 changes: 2 additions & 7 deletions docs/src/understand/configure-federation.md
Original file line number Diff line number Diff line change
Expand Up @@ -457,13 +457,8 @@ the sysadmin:

* [`PUT`](https://staging-nginz-https.zinfra.io/api-internal/swagger-ui/brig/#/brig/put_i_federation_remotes__domain_)

* [`DELETE`](https://staging-nginz-https.zinfra.io/api-internal/swagger-ui/brig/#/brig/delete_i_federation_remotes__domain_)
- **WARNING:** If you delete a connection, all users from that
remote will be removed from local conversations, and all
conversations hosted by that remote will be removed from the local
backend. Connections between local and remote users that are
removed will be archived, and can be re-established should you
decide to add the same backend later.
* **NOTE:** De-federating (`DELETE`) has been removed from the API to
avoid a scalability issue. Watch out for a fix in the changelog!

The `remotes` list looks like this:

Expand Down
4 changes: 4 additions & 0 deletions integration/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
, Cabal
, case-insensitive
, containers
, cql
, cql-io
, cryptonite
, data-default
, directory
Expand Down Expand Up @@ -80,6 +82,8 @@ mkDerivation {
bytestring-conversion
case-insensitive
containers
cql
cql-io
cryptonite
data-default
directory
Expand Down
3 changes: 2 additions & 1 deletion integration/integration.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ library
Test.Brig
Test.Client
Test.Conversation
Test.Defederation
Test.Demo
Test.Federation
Test.Federator
Expand Down Expand Up @@ -143,6 +142,8 @@ library
, bytestring-conversion
, case-insensitive
, containers
, cql
, cql-io
, cryptonite
, data-default
, directory
Expand Down
19 changes: 0 additions & 19 deletions integration/test/API/BrigInternal.hs
Original file line number Diff line number Diff line change
Expand Up @@ -99,25 +99,6 @@ updateFedConn' owndom dom fedConn = do
conn <- make fedConn
submit "PUT" $ addJSON conn req

deleteFedConn :: (HasCallStack, MakesValue owndom) => owndom -> String -> App Response
deleteFedConn owndom dom = do
bindResponse (deleteFedConn' owndom dom) $ \res -> do
res.status `shouldMatchRange` (200, 299)
pure res

deleteFedConn' :: (HasCallStack, MakesValue owndom) => owndom -> String -> App Response
deleteFedConn' owndom dom = do
req <- rawBaseRequest owndom Brig Unversioned ("/i/federation/remotes/" <> dom)
submit "DELETE" req

deleteAllFedConns :: (HasCallStack, MakesValue dom) => dom -> App ()
deleteAllFedConns dom = do
readFedConns dom >>= \resp ->
resp.json %. "remotes"
& asList
>>= traverse (\v -> v %. "domain" & asString)
>>= mapM_ (deleteFedConn dom)

registerOAuthClient :: (HasCallStack, MakesValue user, MakesValue name, MakesValue url) => user -> name -> url -> App Response
registerOAuthClient user name url = do
req <- baseRequest user Brig Unversioned "i/oauth/clients"
Expand Down
9 changes: 0 additions & 9 deletions integration/test/API/GalleyInternal.hs
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,3 @@ getFederationStatus user domains =
submit
"GET"
$ req & addJSONObject ["domains" .= domainList]

deleteFederationDomain ::
( HasCallStack
) =>
String ->
App Response
deleteFederationDomain domain = do
req <- rawBaseRequest OwnDomain Galley Unversioned $ joinHttpPath ["i", "federation", domain]
submit "DELETE" req
42 changes: 5 additions & 37 deletions integration/test/SetupHelpers.hs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
{-# OPTIONS_GHC -Wno-incomplete-uni-patterns #-}

module SetupHelpers where

import API.Brig qualified as Brig
Expand All @@ -9,7 +11,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.V1 (nextUUID)
import Data.UUID.V4 (nextRandom)
import GHC.Stack
Expand Down Expand Up @@ -75,14 +76,6 @@ getAllConvs u = do
resp.json
result %. "found" & asList

resetFedConns :: (HasCallStack, MakesValue owndom) => owndom -> App ()
resetFedConns owndom = do
bindResponse (Internal.readFedConns owndom) $ \resp -> do
rdoms :: [String] <- do
rawlist <- resp.json %. "remotes" & asList
(asString . (%. "domain")) `mapM` rawlist
Internal.deleteFedConn' owndom `mapM_` rdoms

randomId :: HasCallStack => App String
randomId = liftIO (show <$> nextRandom)

Expand All @@ -95,40 +88,15 @@ 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
{ 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
withFederatingBackendsAllowDynamic :: HasCallStack => ((String, String, String) -> App a) -> App a
withFederatingBackendsAllowDynamic k = do
let setFederationConfig =
setField "optSettings.setFederationStrategy" "allowDynamic"
>=> removeField "optSettings.setFederationDomainConfigs"
>=> setField "optSettings.setFederationDomainConfigsUpdateFreq" (Aeson.Number 1)
startDynamicBackends
[ def {brigCfg = setFederationConfig},
def {brigCfg = setFederationConfig},
def {brigCfg = setFederationConfig}
]
$ \dynDomains -> do
domains@[domainA, domainB, domainC] <- pure dynDomains
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
$ \[domainA, domainB, domainC] ->
k (domainA, domainB, domainC)
69 changes: 14 additions & 55 deletions integration/test/Test/Brig.hs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import API.Brig qualified as Public
import API.BrigInternal qualified as Internal
import API.Common qualified as API
import API.GalleyInternal qualified as Internal
import Control.Concurrent (threadDelay)
import Data.Aeson qualified as Aeson
import Data.Aeson.Types hiding ((.=))
import Data.Set qualified as Set
Expand All @@ -29,14 +30,7 @@ testSearchContactForExternalUsers = do
testCrudFederationRemotes :: HasCallStack => App ()
testCrudFederationRemotes = do
otherDomain <- asString OtherDomain
let overrides =
def
{ brigCfg =
setField
"optSettings.setFederationDomainConfigs"
[object ["domain" .= otherDomain, "search_policy" .= "full_search"]]
}
withModifiedBackend overrides $ \ownDomain -> do
withModifiedBackend def $ \ownDomain -> do
let parseFedConns :: HasCallStack => Response -> App [Value]
parseFedConns resp =
-- Pick out the list of federation domain configs
Expand All @@ -45,74 +39,40 @@ testCrudFederationRemotes = do
-- Enforce that the values are objects and not something else
>>= traverse (fmap Object . asObject)

addOnce :: (MakesValue fedConn, Ord fedConn2, ToJSON fedConn2, MakesValue fedConn2, HasCallStack) => fedConn -> [fedConn2] -> App ()
addOnce fedConn want = do
addTest :: (MakesValue fedConn, Ord fedConn2, ToJSON fedConn2, MakesValue fedConn2, HasCallStack) => fedConn -> [fedConn2] -> App ()
addTest fedConn want = do
bindResponse (Internal.createFedConn ownDomain fedConn) $ \res -> do
addFailureContext ("res = " <> show res) $ res.status `shouldMatchInt` 200
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
addFailureContext ("res = " <> show res) $ res.status `shouldMatchInt` 200
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
updateTest :: (MakesValue fedConn, Ord fedConn2, ToJSON fedConn2, MakesValue fedConn2, HasCallStack) => String -> fedConn -> [fedConn2] -> App ()
updateTest domain fedConn want = do
bindResponse (Internal.updateFedConn ownDomain domain fedConn) $ \res -> do
addFailureContext ("res = " <> show res) $ res.status `shouldMatchInt` 200
res2 <- parseFedConns =<< Internal.readFedConns ownDomain
sort res2 `shouldMatch` sort want

updateFail :: (MakesValue fedConn, HasCallStack) => String -> fedConn -> App ()
updateFail domain fedConn = do
bindResponse (Internal.updateFedConn' ownDomain domain fedConn) $ \res -> do
addFailureContext ("res = " <> show res) $ res.status `shouldMatchInt` 533

dom1 :: String <- (<> ".example.com") . UUID.toString <$> liftIO UUID.nextRandom
dom2 :: String <- (<> ".example.com") . UUID.toString <$> liftIO UUID.nextRandom

let remote1, remote1', remote1'' :: Internal.FedConn
let 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"

remote1J <- make remote1
remote1J' <- make remote1'

resetFedConns ownDomain
liftIO $ threadDelay 5_000_000
cfgRemotes <- parseFedConns =<< Internal.readFedConns ownDomain
cfgRemotes `shouldMatch` [cfgRemotesExpect]
cfgRemotes `shouldMatch` ([] @Value)
-- 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"})
-- updated.
addTest cfgRemotesExpect [cfgRemotesExpect]
-- create
addOnce remote1 $ (remote1J : cfgRemotes)
addOnce remote1 $ (remote1J : cfgRemotes) -- idempotency
addTest remote1 [cfgRemotesExpect, remote1]
addTest remote1 [cfgRemotesExpect, remote1] -- idempotency
-- update
updateOnce (Internal.domain remote1) remote1' (remote1J' : cfgRemotes)
updateFail (Internal.domain remote1) remote1''
-- delete
deleteOnce (Internal.domain remote1) cfgRemotes
deleteOnce (Internal.domain remote1) cfgRemotes -- idempotency
updateTest (Internal.domain remote1) remote1' [cfgRemotesExpect, remote1']

testCrudOAuthClient :: HasCallStack => App ()
testCrudOAuthClient = do
Expand Down Expand Up @@ -185,7 +145,6 @@ testRemoteUserSearch :: HasCallStack => App ()
testRemoteUserSearch = do
let overrides =
setField "optSettings.setFederationStrategy" "allowDynamic"
>=> removeField "optSettings.setFederationDomainConfigs"
>=> setField "optSettings.setFederationDomainConfigsUpdateFreq" (Aeson.Number 1)
startDynamicBackends [def {brigCfg = overrides}, def {brigCfg = overrides}] $ \dynDomains -> do
domains@[d1, d2] <- pure dynDomains
Expand Down
Loading

0 comments on commit 99d3f5e

Please sign in to comment.