Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

federator: rename Brig -> Service and add galley #1570

Merged
merged 5 commits into from
Jun 7, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions charts/federator/templates/configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ data:
host: brig
port: 8080

galley:
host: brig
port: 8080

{{- with .Values.config }}

logNetStrings: True # log using netstrings encoding:
Expand Down
4 changes: 3 additions & 1 deletion charts/federator/templates/tests/configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,6 @@ data:
brig:
host: brig
port: 8080

galley:
host: brig
port: 8080
4 changes: 2 additions & 2 deletions services/federator/federator.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ cabal-version: 1.12
--
-- see: https://github.com/sol/hpack
--
-- hash: 9838302126363c5c21222de184bbf831d238744bb2871b425aadee55a917b341
-- hash: b7d470767dc3f1475b43b3914d7aa4572f00de52e59a396331bf20062d1e71fb

name: federator
version: 1.0.0
Expand All @@ -19,14 +19,14 @@ build-type: Simple
library
exposed-modules:
Federator.App
Federator.Brig
Federator.Discovery
Federator.Env
Federator.ExternalServer
Federator.InternalServer
Federator.Options
Federator.Remote
Federator.Run
Federator.Service
Federator.Utils.PolysemyServerError
Federator.Validation
other-modules:
Expand Down
3 changes: 3 additions & 0 deletions services/federator/federator.integration.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ federatorExternal:
brig:
host: 0.0.0.0
port: 8082
galley:
host: 0.0.0.0
port: 8085

logLevel: Debug
logNetStrings: false
Expand Down
5 changes: 2 additions & 3 deletions services/federator/src/Federator/Env.hs
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,15 @@ import Federator.Options (RunSettings)
import Network.DNS.Resolver (Resolver)
import qualified Network.HTTP.Client as HTTP
import qualified System.Logger.Class as LC
import Util.Options
import Wire.API.Federation.GRPC.Types

data Env = Env
{ _metrics :: Metrics,
_applog :: LC.Logger,
_requestId :: RequestId,
_dnsResolver :: Resolver,
_runSettings :: RunSettings,
_brig :: RPC.Request,
_brigEndpoint :: Endpoint,
_brig :: Component -> RPC.Request,
_httpManager :: HTTP.Manager
}

Expand Down
14 changes: 7 additions & 7 deletions services/federator/src/Federator/ExternalServer.hs
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ import qualified Data.ByteString.Lazy as LBS
import qualified Data.Text as Text
import qualified Data.Text.Encoding as Text
import Federator.App (Federator, runAppT)
import Federator.Brig (Brig, brigCall, interpretBrig)
import Federator.Env (Env, applog, runSettings)
import Federator.Options (RunSettings)
import Federator.Service (Service, interpretService, serviceCall)
import Federator.Utils.PolysemyServerError (absorbServerError)
import Federator.Validation
import Imports
Expand All @@ -52,13 +52,13 @@ import Wire.API.Federation.GRPC.Types
-- FUTUREWORK(federation): How do we make sure that only legitimate endpoints can be
-- reached, some discussion here:
-- https://wearezeta.atlassian.net/wiki/spaces/CORE/pages/224166764/Limiting+access+to+federation+endpoints
-- Also, see comment in 'Federator.Brig.interpretBrig'
-- Also, see comment in 'Federator.Service.interpretService'
--
-- FUTUREWORK(federation): implement server2server authentication!
-- (current validation only checks parsing and compares to allowList)
--
-- FUTUREWORK: consider using Polysemy.Error also in callLocal to reduce nesting and improve readability.
callLocal :: (Members '[Brig, Embed IO, TinyLog, Polysemy.Reader RunSettings] r) => Request -> Sem r InwardResponse
callLocal :: (Members '[Service, Embed IO, TinyLog, Polysemy.Reader RunSettings] r) => Request -> Sem r InwardResponse
callLocal req@Request {..} = do
Log.debug $
Log.msg ("Inward Request" :: ByteString)
Expand All @@ -68,7 +68,7 @@ callLocal req@Request {..} = do
case validation of
Left err -> pure $ InwardResponseErr err
Right domain -> do
(resStatus, resBody) <- brigCall path body domain
(resStatus, resBody) <- serviceCall component path body domain
pure $ case HTTP.statusCode resStatus of
200 -> InwardResponseBody $ maybe mempty LBS.toStrict resBody
-- TODO: There is a unit test for this, but Akshay has seen the integration
Expand All @@ -78,19 +78,19 @@ callLocal req@Request {..} = do
-- our integration test, let's verify this.
code -> InwardResponseErr $ "Invalid HTTP status from component: " <> Text.pack (show code) <> " " <> Text.decodeUtf8 (HTTP.statusMessage resStatus)

routeToInternal :: (Members '[Brig, Embed IO, Polysemy.Error ServerError, TinyLog, Polysemy.Reader RunSettings] r) => SingleServerT info Inward (Sem r) _
routeToInternal :: (Members '[Service, Embed IO, Polysemy.Error ServerError, TinyLog, Polysemy.Reader RunSettings] r) => SingleServerT info Inward (Sem r) _
routeToInternal = singleService (Mu.method @"call" callLocal)

serveInward :: Env -> Int -> IO ()
serveInward env port = do
runGRpcAppTrans msgProtoBuf port transformer routeToInternal
where
transformer :: Sem '[TinyLog, Embed IO, Polysemy.Error ServerError, Brig, Polysemy.Reader RunSettings, Embed Federator] a -> ServerErrorIO a
transformer :: Sem '[TinyLog, Embed IO, Polysemy.Error ServerError, Service, Polysemy.Reader RunSettings, Embed Federator] a -> ServerErrorIO a
transformer action =
runAppT env
. runM @Federator
. Polysemy.runReader (view runSettings env)
. interpretBrig
. interpretService
. absorbServerError
. embedToMonadIO @Federator
. Log.runTinyLog (view applog env)
Expand Down
2 changes: 2 additions & 0 deletions services/federator/src/Federator/Options.hs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ data Opts = Opts
federatorExternal :: Endpoint,
-- | Host and port of brig
brig :: Endpoint,
-- | Host and port of galley
galley :: Endpoint,
-- | Log level (Debug, Info, etc)
logLevel :: Level,
-- | Use netstrings encoding (see <http://cr.yp.to/proto/netstrings.txt>)
Expand Down
5 changes: 3 additions & 2 deletions services/federator/src/Federator/Run.hs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import qualified System.Logger.Extended as LogExt
import UnliftIO (bracket)
import UnliftIO.Async (async, waitAnyCancel)
import Util.Options
import Wire.API.Federation.GRPC.Types
import qualified Wire.Network.DNS.Helper as DNS

------------------------------------------------------------------------------
Expand Down Expand Up @@ -86,8 +87,8 @@ newEnv o _dnsResolver = do
_applog <- LogExt.mkLogger (Opt.logLevel o) (Opt.logNetStrings o) (Opt.logFormat o)
let _requestId = def
let _runSettings = Opt.optSettings o
let _brig = mkEndpoint (Opt.brig o)
let _brigEndpoint = Opt.brig o
let _brig Brig = mkEndpoint (Opt.brig o)
_brig Galley = mkEndpoint (Opt.galley o)
pcapriotti marked this conversation as resolved.
Show resolved Hide resolved
_httpManager <- initHttpManager
return Env {..}
where
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,8 @@
-- 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 Federator.Brig where
module Federator.Service where

-- Is there is a point in creating an effect for each service?
--
-- FUTUREWORK(federation): Once we authenticate the call, we should send authentication data
-- to brig so brig can do some authorization as required.

Expand All @@ -32,30 +30,31 @@ import Federator.Env (brig)
import Imports
import qualified Network.HTTP.Types as HTTP
import Polysemy
import Wire.API.Federation.GRPC.Types

newtype BrigError = BrigErrorInvalidStatus HTTP.Status
newtype ServiceError = ServiceErrorInvalidStatus HTTP.Status
deriving (Eq, Show)

data Brig m a where
data Service m a where
-- | Returns status and body, 'HTTP.Response' is not nice to work with in tests
BrigCall :: ByteString -> ByteString -> Domain -> Brig m (HTTP.Status, Maybe LByteString)
ServiceCall :: Component -> ByteString -> ByteString -> Domain -> Service m (HTTP.Status, Maybe LByteString)

makeSem ''Brig
makeSem ''Service

-- FUTUREWORK(federation): Do we want to use servant client here? May make
-- everything typed and safe
--
-- FUTUREWORK: Avoid letting the IO errors escape into `Embed Federator` and
-- return them as `Left`
interpretBrig ::
interpretService ::
Member (Embed Federator) r =>
Sem (Brig ': r) a ->
Sem (Service ': r) a ->
Sem r a
interpretBrig = interpret $ \case
BrigCall path body domain -> embed @Federator . liftAppIOToFederator $ do
interpretService = interpret $ \case
ServiceCall component path body domain -> embed @Federator . liftAppIOToFederator $ do
brigReq <- view brig <$> ask
res <-
rpc' "brig" brigReq $
rpc' "brig" (brigReq component) $
RPC.method HTTP.POST
. RPC.path path -- FUTUREWORK(federation): Protect against arbitrary paths
. RPC.body (RPC.RequestBodyBS body)
pcapriotti marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
24 changes: 12 additions & 12 deletions services/federator/test/unit/Test/Federator/ExternalServer.hs
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@
module Test.Federator.ExternalServer where

import Data.Domain (Domain (..))
import Federator.Brig (Brig)
import Federator.ExternalServer (callLocal)
import Federator.Options (FederationStrategy (AllowAll), RunSettings (..))
import Federator.Service (Service)
import Imports
import qualified Network.HTTP.Types as HTTP
import Polysemy (embed, runM)
Expand All @@ -36,7 +36,7 @@ import Test.Tasty (TestTree, testGroup)
import Test.Tasty.HUnit (assertEqual, testCase)
import Wire.API.Federation.GRPC.Types

genMock ''Brig
genMock ''Service

tests :: TestTree
tests =
pcapriotti marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -48,27 +48,27 @@ tests =
requestBrigSuccess :: TestTree
requestBrigSuccess =
testCase "should translate response from brig to 'InwardResponseBody' when response has status 200" $
runM . evalMock @Brig @IO $ do
mockBrigCallReturns @IO (\_ _ _ -> pure (HTTP.ok200, Just "response body"))
runM . evalMock @Service @IO $ do
mockServiceCallReturns @IO (\_ _ _ _ -> pure (HTTP.ok200, Just "response body"))
let request = Request Brig "/users" "\"foo\"" exampleDomain

res :: InwardResponse <- mock @Brig @IO . noLogs . Polysemy.runReader allowAllSettings $ callLocal request
actualCalls <- mockBrigCallCalls @IO
let expectedCall = ("/users", "\"foo\"", aValidDomain)
res :: InwardResponse <- mock @Service @IO . noLogs . Polysemy.runReader allowAllSettings $ callLocal request
actualCalls <- mockServiceCallCalls @IO
let expectedCall = (Brig, "/users", "\"foo\"", aValidDomain)
embed $ assertEqual "one call to brig should be made" [expectedCall] actualCalls
embed $ assertEqual "response should be success with correct body" (InwardResponseBody "response body") res

requestBrigFailure :: TestTree
requestBrigFailure =
testCase "should translate response from brig to 'InwardResponseBody' when response has status 404" $
runM . evalMock @Brig @IO $ do
mockBrigCallReturns @IO (\_ _ _ -> pure (HTTP.notFound404, Just "response body"))
runM . evalMock @Service @IO $ do
mockServiceCallReturns @IO (\_ _ _ _ -> pure (HTTP.notFound404, Just "response body"))
pcapriotti marked this conversation as resolved.
Show resolved Hide resolved
let request = Request Brig "/users" "\"foo\"" exampleDomain

res <- mock @Brig @IO . noLogs . Polysemy.runReader allowAllSettings $ callLocal request
res <- mock @Service @IO . noLogs . Polysemy.runReader allowAllSettings $ callLocal request

actualCalls <- mockBrigCallCalls @IO
let expectedCall = ("/users", "\"foo\"", aValidDomain)
actualCalls <- mockServiceCallCalls @IO
pcapriotti marked this conversation as resolved.
Show resolved Hide resolved
let expectedCall = (Brig, "/users", "\"foo\"", aValidDomain)
embed $ assertEqual "one call to brig should be made" [expectedCall] actualCalls
embed $ assertEqual "response should be success with correct body" (InwardResponseErr "Invalid HTTP status from component: 404 Not Found") res

Expand Down