[WPB-5883] Feature flag for a limited event fanout (#3797)

* Introduce the feature flag

This commit implements no business logic around the flag, but merely
sets up the very basics needed to use the flag.

* Document the feature flag

* Guard member deleted event fanout

* Test: Limited event fanout

This extends an existing test case that deletes a team member, but now
explicitly enabling the limited event fanout feature flag.

* Test: future-port a test from a branch from July 14, 2023

* Fix the team event fanout for the unlimited case

* Test: getting and setting the feature flag

* fix linter

* Add a changelog

* Fix more linting

* Move a test within a module

* Fix a galley-types unit test

* Fix a galley-integration test

* Make a notification push transient

* Revert the change to the billing team update notification

* Reuse a notification assertion helper

---------

Co-authored-by: Stefan Berthold <stefan.berthold@wire.com>

cassandra-schema.cql

@@ -1200,6 +1200,7 @@ CREATE TABLE galley_test.team_features (
     guest_links_lock_status int,
     guest_links_status int,
     legalhold_status int,
+    limited_event_fanout_status int,
     mls_allowed_ciphersuites set<int>,
     mls_default_ciphersuite int,
     mls_default_protocol int,

changelog.d/2-features/WPB-5883

@@ -0,0 +1 @@
+Introduce a feature flag that controls whether the limited event fanout should be used when a team member is deleted

charts/galley/templates/configmap.yaml

@@ -141,5 +141,9 @@ data:
         mlsMigration:
           {{- toYaml .settings.featureFlags.mlsMigration | nindent 10 }}
         {{- end }}
+        {{- if .settings.featureFlags.limitedEventFanout }}
+        limitedEventFanout:
+          {{- toYaml .settings.featureFlags.limitedEventFanout | nindent 10 }}
+        {{- end }}
       {{- end }}
   {{- end }}

charts/galley/values.yaml

@@ -132,6 +132,9 @@ config:
             usersThreshold: 100
             clientsThreshold: 100
           lockStatus: locked
+      limitedEventFanout:
+        defaults:
+          status: disabled
 
   aws:
     region: "eu-west-1"

docs/src/developer/reference/config-options.md

@@ -425,7 +425,7 @@ federator:
     clientPrivateKey: client-key.pem
 ```
 
-### Outlook calalendar integration
+### Outlook calendar integration
 
 This feature setting only applies to the Outlook Calendar extension for Wire. As it is an external service, it should only be configured through this feature flag and otherwise ignored by the backend.
 
@@ -450,6 +450,23 @@ config:
     GuestLinkTTLSeconds: 604800
 ```
 
+### Limited Event Fanout
+
+To maintain compatibility with clients and their versions that do not implement
+the limited event fanout when a team member is deleted, the limited event fanout
+flag is used. Its default value `disabled` means that the old-style full event
+fanout will take place when a team member is deleted. Set the flag to `enabled`
+to send team events only to team owners and administrators.
+
+Example configuration:
+
+```yaml
+# galley.yaml
+limitedEventFanout:
+  defaults:
+    status: disabled
+```
+
 ## Settings in brig
 
 Some features (as of the time of writing this: only

hack/helm_vars/wire-server/values.yaml.gotmpl

@@ -235,6 +235,9 @@ galley:
               usersThreshold: 100
               clientsThreshold: 50
             lockStatus: locked
+        limitedEventFanout:
+            defaults:
+            status: disabled
     journal:
       endpoint: http://fake-aws-sqs:4568
       queueName: integration-team-events.fifo

integration/integration.cabal

@@ -119,6 +119,7 @@ library
     Test.Demo
     Test.Errors
     Test.ExternalPartner
+    Test.FeatureFlags
     Test.Federation
     Test.Federator
     Test.MessageTimer

integration/test/Notifications.hs

@@ -139,3 +139,9 @@ assertLeaveNotification fromUser conv user client leaver =
             isNotifFromUser fromUser
           ]
       )
+
+assertConvUserDeletedNotif :: MakesValue leaverId => WebSocket -> leaverId -> App ()
+assertConvUserDeletedNotif ws leaverId = do
+  n <- awaitMatch isConvLeaveNotif ws
+  nPayload n %. "data.qualified_user_ids.0" `shouldMatch` leaverId
+  nPayload n %. "data.reason" `shouldMatch` "user-deleted"

integration/test/Test/Conversation.hs

@@ -660,36 +660,94 @@ testDeleteTeamConversationWithUnreachableRemoteMembers = do
       notif <- awaitNotification bob bobClient noValue isConvDeleteNotif
       assertNotification notif
 
-testDeleteTeamMember :: HasCallStack => App ()
-testDeleteTeamMember = do
-  (alice, team, [alex]) <- createTeam OwnDomain 2
+testDeleteTeamMemberLimitedEventFanout :: HasCallStack => App ()
+testDeleteTeamMemberLimitedEventFanout = do
+  -- Alex will get removed from the team
+  (alice, team, [alex, alison]) <- createTeam OwnDomain 3
+  ana <- createTeamMemberWithRole alice team "admin"
   [amy, bob] <- for [OwnDomain, OtherDomain] $ flip randomUser def
   forM_ [amy, bob] $ connectTwoUsers alice
-  [aliceId, alexId, amyId, bobId] <-
-    forM [alice, alex, amy, bob] (%. "qualified_id")
-  let nc = (defProteus {qualifiedUsers = [alexId, amyId, bobId], team = Just team})
+  [aliceId, alexId, amyId, alisonId, anaId, bobId] <- do
+    forM [alice, alex, amy, alison, ana, bob] (%. "qualified_id")
+  let nc =
+        ( defProteus
+            { qualifiedUsers =
+                [alexId, amyId, alisonId, anaId, bobId],
+              team = Just team
+            }
+        )
+  conv <- postConversation alice nc >>= getJSON 201
+  memsBefore <- getMembers team aliceId
+
+  -- Only the team admins will get the team-level event about Alex being removed
+  -- from the team
+  setTeamFeatureStatus OwnDomain team "limitedEventFanout" "enabled"
+
+  withWebSockets [alice, amy, bob, alison, ana] $
+    \[wsAlice, wsAmy, wsBob, wsAlison, wsAna] -> do
+      void $ deleteTeamMember team alice alex >>= getBody 202
+
+      memsAfter <- getMembers team aliceId
+      memsAfter `shouldNotMatch` memsBefore
+
+      assertConvUserDeletedNotif wsAmy alexId
+      assertConvUserDeletedNotif wsAlison alexId
+
+      alexUId <- alex %. "id"
+      do
+        n <- awaitMatch isTeamMemberLeaveNotif wsAlice
+        nPayload n %. "data.user" `shouldMatch` alexUId
+        assertConvUserDeletedNotif wsAlice alexId
+      do
+        n <- awaitMatch isTeamMemberLeaveNotif wsAna
+        nPayload n %. "data.user" `shouldMatch` alexUId
+        assertConvUserDeletedNotif wsAna alexId
+      do
+        bindResponse (getConversation bob conv) $ \resp -> do
+          resp.status `shouldMatchInt` 200
+          mems <- resp.json %. "members.others" & asList
+          memIds <- forM mems (%. "qualified_id")
+          memIds `shouldMatchSet` [aliceId, alisonId, amyId, anaId]
+        assertConvUserDeletedNotif wsBob alexId
+  where
+    getMembers tid usr = bindResponse (getTeamMembers usr tid) $ \resp -> do
+      resp.status `shouldMatchInt` 200
+      ms <- resp.json %. "members" & asList
+      forM ms $ (%. "user")
+
+-- The test relies on the default value for the 'limitedEventFanout' flag, which
+-- is disabled by default. The counterpart test
+-- 'testDeleteTeamMemberLimitedEventFanout' enables the flag and tests the
+-- limited fanout.
+testDeleteTeamMemberFullEventFanout :: HasCallStack => App ()
+testDeleteTeamMemberFullEventFanout = do
+  (alice, team, [alex, alison]) <- createTeam OwnDomain 3
+  [amy, bob] <- for [OwnDomain, OtherDomain] $ flip randomUser def
+  forM_ [amy, bob] $ connectTwoUsers alice
+  [aliceId, alexId, alisonId, amyId, bobId] <-
+    forM [alice, alex, alison, amy, bob] (%. "qualified_id")
+  let nc = (defProteus {qualifiedUsers = [alexId, alisonId, amyId, bobId], team = Just team})
   conv <- postConversation alice nc >>= getJSON 201
-  withWebSockets [alice, amy, bob] $ \[wsAlice, wsAmy, wsBob] -> do
+  withWebSockets [alice, alison, amy, bob] $ \[wsAlice, wsAlison, wsAmy, wsBob] -> do
     void $ deleteTeamMember team alice alex >>= getBody 202
-    assertConvLeaveNotif wsAmy alexId
+    alexUId <- alex %. "id"
     do
       n <- awaitMatch isTeamMemberLeaveNotif wsAlice
-      alexUId <- alex %. "id"
       nPayload n %. "data.user" `shouldMatch` alexUId
-      assertConvLeaveNotif wsAlice alexId
+    do
+      t <- awaitMatch isTeamMemberLeaveNotif wsAlison
+      nPayload t %. "data.user" `shouldMatch` alexUId
+      assertConvUserDeletedNotif wsAlison alexId
+
+    assertConvUserDeletedNotif wsAmy alexId
+
     do
       bindResponse (getConversation bob conv) $ \resp -> do
         resp.status `shouldMatchInt` 200
         mems <- resp.json %. "members.others" & asList
         memIds <- forM mems (%. "qualified_id")
-        memIds `shouldMatchSet` [aliceId, amyId]
-      assertConvLeaveNotif wsBob alexId
-  where
-    assertConvLeaveNotif :: MakesValue leaverId => WebSocket -> leaverId -> App ()
-    assertConvLeaveNotif ws leaverId = do
-      n <- awaitMatch isConvLeaveNotif ws
-      nPayload n %. "data.qualified_user_ids.0" `shouldMatch` leaverId
-      nPayload n %. "data.reason" `shouldMatch` "user-deleted"
+        memIds `shouldMatchSet` [aliceId, alisonId, amyId]
+      assertConvUserDeletedNotif wsBob alexId
 
 testLeaveConversationSuccess :: HasCallStack => App ()
 testLeaveConversationSuccess = do

integration/test/Test/FeatureFlags.hs

@@ -0,0 +1,35 @@
+-- This file is part of the Wire Server implementation.
+--
+-- Copyright (C) 2023 Wire Swiss GmbH <opensource@wire.com>
+--
+-- This program is free software: you can redistribute it and/or modify it under
+-- the terms of the GNU Affero General Public License as published by the Free
+-- Software Foundation, either version 3 of the License, or (at your option) any
+-- later version.
+--
+-- This program is distributed in the hope that it will be useful, but WITHOUT
+-- ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+-- FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+-- details.
+--
+-- You should have received a copy of the GNU Affero General Public License along
+-- with this program. If not, see <https://www.gnu.org/licenses/>.
+
+module Test.FeatureFlags where
+
+import API.GalleyInternal
+import SetupHelpers
+import Testlib.Prelude
+
+testLimitedEventFanout :: HasCallStack => App ()
+testLimitedEventFanout = do
+  let featureName = "limitedEventFanout"
+  (_alice, team, _) <- createTeam OwnDomain 1
+  -- getTeamFeatureStatus OwnDomain team "limitedEventFanout" "enabled"
+  bindResponse (getTeamFeature OwnDomain featureName team) $ \resp -> do
+    resp.status `shouldMatchInt` 200
+    resp.json %. "status" `shouldMatch` "disabled"
+  setTeamFeatureStatus OwnDomain team featureName "enabled"
+  bindResponse (getTeamFeature OwnDomain featureName team) $ \resp -> do
+    resp.status `shouldMatchInt` 200
+    resp.json %. "status" `shouldMatch` "enabled"

libs/galley-types/src/Galley/Types/Teams.hs

@@ -43,6 +43,7 @@ module Galley.Types.Teams
     flagMlsE2EId,
     flagMlsMigration,
     flagEnforceFileDownloadLocation,
+    flagLimitedEventFanout,
     Defaults (..),
     ImplicitLockStatus (..),
     unImplicitLockStatus,
@@ -167,7 +168,8 @@ data FeatureFlags = FeatureFlags
     _flagOutlookCalIntegration :: !(Defaults (WithStatus OutlookCalIntegrationConfig)),
     _flagMlsE2EId :: !(Defaults (WithStatus MlsE2EIdConfig)),
     _flagMlsMigration :: !(Defaults (WithStatus MlsMigrationConfig)),
-    _flagEnforceFileDownloadLocation :: !(Defaults (WithStatus EnforceFileDownloadLocationConfig))
+    _flagEnforceFileDownloadLocation :: !(Defaults (WithStatus EnforceFileDownloadLocationConfig)),
+    _flagLimitedEventFanout :: !(Defaults (ImplicitLockStatus LimitedEventFanoutConfig))
   }
   deriving (Eq, Show, Generic)
 
@@ -221,6 +223,7 @@ instance FromJSON FeatureFlags where
       <*> (fromMaybe (Defaults (defFeatureStatus @MlsE2EIdConfig)) <$> (obj .:? "mlsE2EId"))
       <*> (fromMaybe (Defaults (defFeatureStatus @MlsMigrationConfig)) <$> (obj .:? "mlsMigration"))
       <*> (fromMaybe (Defaults (defFeatureStatus @EnforceFileDownloadLocationConfig)) <$> (obj .:? "enforceFileDownloadLocation"))
+      <*> withImplicitLockStatusOrDefault obj "limitedEventFanout"
     where
       withImplicitLockStatusOrDefault :: forall cfg. (IsFeatureConfig cfg, Schema.ToSchema cfg) => Object -> Key -> A.Parser (Defaults (ImplicitLockStatus cfg))
       withImplicitLockStatusOrDefault obj fieldName = fromMaybe (Defaults (ImplicitLockStatus (defFeatureStatus @cfg))) <$> obj .:? fieldName
@@ -245,6 +248,7 @@ instance ToJSON FeatureFlags where
         mlsE2EId
         mlsMigration
         enforceFileDownloadLocation
+        teamMemberDeletedLimitedEventFanout
       ) =
       object
         [ "sso" .= sso,
@@ -263,7 +267,8 @@ instance ToJSON FeatureFlags where
           "outlookCalIntegration" .= outlookCalIntegration,
           "mlsE2EId" .= mlsE2EId,
           "mlsMigration" .= mlsMigration,
-          "enforceFileDownloadLocation" .= enforceFileDownloadLocation
+          "enforceFileDownloadLocation" .= enforceFileDownloadLocation,
+          "limitedEventFanout" .= teamMemberDeletedLimitedEventFanout
         ]
 
 instance FromJSON FeatureSSO where

libs/galley-types/test/unit/Test/Galley/Types.hs

@@ -100,6 +100,7 @@ instance Arbitrary FeatureFlags where
       <*> arbitrary
       <*> arbitrary
       <*> arbitrary
+      <*> fmap (fmap unlocked) arbitrary
     where
       unlocked :: ImplicitLockStatus a -> ImplicitLockStatus a
       unlocked = ImplicitLockStatus . Public.setLockStatus Public.LockStatusUnlocked . _unImplicitLockStatus

libs/wire-api/src/Wire/API/Routes/Internal/Galley.hs

@@ -165,6 +165,10 @@ type IFeatureAPI =
     :<|> IFeatureStatusPutWithDesc '[] '() EnforceFileDownloadLocationConfig "<p><b>Custom feature: only supported for some decidated on-prem systems.</b></p>"
     :<|> IFeatureStatusPatchWithDesc '[] '() EnforceFileDownloadLocationConfig "<p><b>Custom feature: only supported for some decidated on-prem systems.</b></p>"
     :<|> IFeatureStatusLockStatusPutWithDesc EnforceFileDownloadLocationConfig "<p><b>Custom feature: only supported for some decidated on-prem systems.</b></p>"
+    -- LimitedEventFanoutConfig
+    :<|> IFeatureStatusGet LimitedEventFanoutConfig
+    :<|> IFeatureStatusPut '[] '() LimitedEventFanoutConfig
+    :<|> IFeatureStatusPatch '[] '() LimitedEventFanoutConfig
     -- all feature configs
     :<|> Named
            "feature-configs-internal"

libs/wire-api/src/Wire/API/Routes/Public/Galley/Feature.hs

@@ -101,6 +101,7 @@ type FeatureAPI =
             '()
             EnforceFileDownloadLocationConfig
             "<p><b>Custom feature: only supported for some decidated on-prem systems.</b></p>"
+    :<|> From 'V5 ::> FeatureStatusGet LimitedEventFanoutConfig
     :<|> AllFeatureConfigsUserGet
     :<|> AllFeatureConfigsTeamGet
     :<|> FeatureConfigDeprecatedGet "The usage of this endpoint was removed in iOS in version 3.101. It is not used by team management, or webapp, and is potentially used by the old Android client as of June 2022" LegalholdConfig

libs/wire-api/src/Wire/API/Team/Feature.hs

@@ -82,6 +82,7 @@ module Wire.API.Team.Feature
     MlsE2EIdConfig (..),
     MlsMigrationConfig (..),
     EnforceFileDownloadLocationConfig (..),
+    LimitedEventFanoutConfig (..),
     AllFeatureConfigs (..),
     unImplicitLockStatus,
     ImplicitLockStatus (..),
@@ -211,6 +212,7 @@ data FeatureSingleton cfg where
     -- all other constructors)
     FeatureSingleton MlsMigrationConfig
   FeatureSingletonEnforceFileDownloadLocationConfig :: FeatureSingleton EnforceFileDownloadLocationConfig
+  FeatureSingletonLimitedEventFanoutConfig :: FeatureSingleton LimitedEventFanoutConfig
 
 class FeatureTrivialConfig cfg where
   trivialConfig :: cfg
@@ -1118,6 +1120,32 @@ instance IsFeatureConfig EnforceFileDownloadLocationConfig where
   featureSingleton = FeatureSingletonEnforceFileDownloadLocationConfig
   objectSchema = field "config" schema
 
+----------------------------------------------------------------------
+-- Guarding the fanout of events when a team member is deleted.
+--
+-- FUTUREWORK: This is a transient flag that is to be removed after about 6
+-- months of its introduction, namely once all clients get a chance to adapt to
+-- a limited event fanout.
+
+data LimitedEventFanoutConfig = LimitedEventFanoutConfig
+  deriving stock (Eq, Show, Generic)
+  deriving (Arbitrary) via (GenericUniform LimitedEventFanoutConfig)
+
+instance RenderableSymbol LimitedEventFanoutConfig where
+  renderSymbol = "LimitedEventFanoutConfig"
+
+instance IsFeatureConfig LimitedEventFanoutConfig where
+  type FeatureSymbol LimitedEventFanoutConfig = "limitedEventFanout"
+  defFeatureStatus = withStatus FeatureStatusEnabled LockStatusUnlocked LimitedEventFanoutConfig FeatureTTLUnlimited
+  featureSingleton = FeatureSingletonLimitedEventFanoutConfig
+  objectSchema = pure LimitedEventFanoutConfig
+
+instance ToSchema LimitedEventFanoutConfig where
+  schema = object "LimitedEventFanoutConfig" objectSchema
+
+instance FeatureTrivialConfig LimitedEventFanoutConfig where
+  trivialConfig = LimitedEventFanoutConfig
+
 ----------------------------------------------------------------------
 -- FeatureStatus
 
@@ -1196,7 +1224,8 @@ data AllFeatureConfigs = AllFeatureConfigs
     afcOutlookCalIntegration :: WithStatus OutlookCalIntegrationConfig,
     afcMlsE2EId :: WithStatus MlsE2EIdConfig,
     afcMlsMigration :: WithStatus MlsMigrationConfig,
-    afcEnforceFileDownloadLocation :: WithStatus EnforceFileDownloadLocationConfig
+    afcEnforceFileDownloadLocation :: WithStatus EnforceFileDownloadLocationConfig,
+    afcLimitedEventFanout :: WithStatus LimitedEventFanoutConfig
   }
   deriving stock (Eq, Show)
   deriving (FromJSON, ToJSON, S.ToSchema) via (Schema AllFeatureConfigs)
@@ -1224,6 +1253,7 @@ instance ToSchema AllFeatureConfigs where
         <*> afcMlsE2EId .= featureField
         <*> afcMlsMigration .= featureField
         <*> afcEnforceFileDownloadLocation .= featureField
+        <*> afcLimitedEventFanout .= featureField
     where
       featureField ::
         forall cfg.
@@ -1253,5 +1283,6 @@ instance Arbitrary AllFeatureConfigs where
       <*> arbitrary
       <*> arbitrary
       <*> arbitrary
+      <*> arbitrary
 
 makeLenses ''ImplicitLockStatus

services/galley/galley.cabal

@@ -276,6 +276,7 @@ library
     Galley.Schema.V88_RemoveMemberClientAndTruncateMLSGroupMemberClient
     Galley.Schema.V89_MlsLockStatus
     Galley.Schema.V90_EnforceFileDownloadLocationConfig
+    Galley.Schema.V91_TeamMemberDeletedLimitedEventFanout
     Galley.Types.Clients
     Galley.Types.ToUserRole
     Galley.Types.UserList