Skip to content

Commit

Permalink
Fix federationStrategy 'allowAll' (#3588)
Browse files Browse the repository at this point in the history
* hack/{helmfile,helm_vars}: Remove unnecessary hacks

* backend-notification-pusher: Watch RabbitMQ queues to push notifications

Instead of watching brig's federation configs internal endpoint.

* background-notification-pusher: Better function name

* Move federated search tests to new integration suite

The tests require search policy to be set, it is easier to test with dynamic backends.

* Simplify integration tests

Now that allowAll is fixed and is the default for all test backends, we can
write tests in a simpler way.

* background-worker: Do not run federation domains config sync thread

* background-worker: Make remotesRefreshInterval configurable

Defaults to 5 min in the helm chart. For integration tests the value is set to
1s.
  • Loading branch information
akshaymankar authored Sep 26, 2023
1 parent 1c6f746 commit d835b97
Show file tree
Hide file tree
Showing 22 changed files with 156 additions and 330 deletions.
1 change: 1 addition & 0 deletions changelog.d/5-internal/background-worker-nosync
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
background-worker: Get list of domains from RabbitMQ instead of brig for pushing backend notifications
8 changes: 0 additions & 8 deletions charts/background-worker/templates/configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,6 @@ data:
host: federator
port: 8080
galley:
host: galley
port: 8080
brig:
host: brig
port: 8080
rabbitmq:
{{toYaml .rabbitmq | indent 6 }}
backendNotificationPusher:
Expand Down
1 change: 1 addition & 0 deletions charts/background-worker/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ config:
backendNotificationPusher:
pushBackoffMinWait: 10000 # in microseconds, so 10ms
pushBackoffMaxWait: 300000000 # microseconds, so 300s
remotesRefreshInterval: 300000000 # microseconds, so 300s

serviceAccount:
# When setting this to 'false', either make sure that a service account named
Expand Down
16 changes: 1 addition & 15 deletions hack/helm_vars/wire-server/values.yaml.gotmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -321,6 +306,7 @@ background-worker:
backendNotificationPusher:
pushBackoffMinWait: 1000 # 1ms
pushBackoffMaxWait: 500000 # 0.5s
remotesRefreshInterval: 1000000 # 1s
secrets:
rabbitmq:
username: {{ .Values.rabbitmqUsername }}
Expand Down
16 changes: 0 additions & 16 deletions hack/helmfile.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand All @@ -151,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'
14 changes: 14 additions & 0 deletions integration/test/API/Brig.hs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@ getUser user target = do
joinHttpPath ["users", domain, uid]
submit "GET" req

getUserByHandle :: (HasCallStack, MakesValue user, MakesValue domain) => user -> domain -> String -> App Response
getUserByHandle user domain handle = do
domainStr <- asString domain
req <-
baseRequest user Brig Versioned $
joinHttpPath ["users", "by-handle", domainStr, handle]
submit "GET" req

getClient ::
(HasCallStack, MakesValue user, MakesValue client) =>
user ->
Expand All @@ -39,6 +47,12 @@ deleteUser user = do
submit "DELETE" $
req & addJSONObject ["password" .= defPassword]

putHandle :: (HasCallStack, MakesValue user) => user -> String -> App Response
putHandle user handle = do
req <- baseRequest user Brig Versioned "/self/handle"
submit "PUT" $
req & addJSONObject ["handle" .= handle]

data AddClient = AddClient
{ ctype :: String,
internal :: Bool,
Expand Down
8 changes: 8 additions & 0 deletions integration/test/API/Common.hs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@ randomEmail = liftIO $ do
chars = mkArray $ ['A' .. 'Z'] <> ['a' .. 'z'] <> ['0' .. '9']
pick = (chars !) <$> randomRIO (Array.bounds chars)

randomHandle :: App String
randomHandle = liftIO $ do
n <- randomRIO (50, 256)
replicateM n pick
where
chars = mkArray $ ['a' .. 'z'] <> ['0' .. '9'] <> "_-."
pick = (chars !) <$> randomRIO (Array.bounds chars)

randomHex :: Int -> App String
randomHex n = liftIO $ replicateM n pick
where
Expand Down
7 changes: 0 additions & 7 deletions integration/test/SetupHelpers.hs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import API.Brig qualified as Brig
import API.BrigInternal qualified as Internal
import API.Common
import API.Galley
import Control.Concurrent (threadDelay)
import Control.Monad.Reader
import Data.Aeson hiding ((.=))
import Data.Aeson.Types qualified as Aeson
Expand All @@ -17,12 +16,6 @@ import Data.UUID.V4 (nextRandom)
import GHC.Stack
import Testlib.Prelude

-- | `n` should be 2 x `setFederationDomainConfigsUpdateFreq` in the config
connectAllDomainsAndWaitToSync :: HasCallStack => Int -> [String] -> App ()
connectAllDomainsAndWaitToSync n domains = 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

randomUser :: (HasCallStack, MakesValue domain) => domain -> Internal.CreateUser -> App Value
randomUser domain cu = bindResponse (Internal.createUser domain cu) $ \resp -> do
resp.status `shouldMatchInt` 201
Expand Down
34 changes: 26 additions & 8 deletions integration/test/Test/Brig.hs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
{-# OPTIONS_GHC -Wno-incomplete-uni-patterns #-}

module Test.Brig where

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
import Data.String.Conversions
Expand Down Expand Up @@ -143,18 +144,35 @@ testSwagger = do

testRemoteUserSearch :: HasCallStack => App ()
testRemoteUserSearch = do
let overrides =
setField "optSettings.setFederationStrategy" "allowDynamic"
>=> setField "optSettings.setFederationDomainConfigsUpdateFreq" (Aeson.Number 1)
startDynamicBackends [def {brigCfg = overrides}, def {brigCfg = overrides}] $ \dynDomains -> do
domains@[d1, d2] <- pure dynDomains
connectAllDomainsAndWaitToSync 1 domains
[u1, u2] <- createAndConnectUsers [d1, d2]
startDynamicBackends [def, def] $ \[d1, d2] -> do
void $ Internal.createFedConn d2 (Internal.FedConn d1 "full_search")

u1 <- randomUser d1 def
u2 <- randomUser d2 def
Internal.refreshIndex d2
uidD2 <- objId u2

bindResponse (Public.searchContacts u1 (u2 %. "name") d2) $ \resp -> do
resp.status `shouldMatchInt` 200
docs <- resp.json %. "documents" >>= asList
case docs of
[] -> assertFailure "Expected a non empty result, but got an empty one"
doc : _ -> doc %. "id" `shouldMatch` uidD2

testRemoteUserSearchExactHandle :: HasCallStack => App ()
testRemoteUserSearchExactHandle = do
startDynamicBackends [def, def] $ \[d1, d2] -> do
void $ Internal.createFedConn d2 (Internal.FedConn d1 "exact_handle_search")

u1 <- randomUser d1 def
u2 <- randomUser d2 def
u2Handle <- API.randomHandle
bindResponse (Public.putHandle u2 u2Handle) $ assertSuccess
Internal.refreshIndex d2

bindResponse (Public.searchContacts u1 u2Handle d2) $ \resp -> do
resp.status `shouldMatchInt` 200
docs <- resp.json %. "documents" >>= asList
case docs of
[] -> assertFailure "Expected a non empty result, but got an empty one"
doc : _ -> objQid doc `shouldMatch` objQid u2
Loading

0 comments on commit d835b97

Please sign in to comment.