diff --git a/changelog.d/5-internal/play-with-version-types b/changelog.d/5-internal/play-with-version-types new file mode 100644 index 00000000000..df6311ba8b5 --- /dev/null +++ b/changelog.d/5-internal/play-with-version-types @@ -0,0 +1 @@ +Introduce VersionNumber newtype (see `/libs/wire-api/src/Wire/API/Routes/Version.hs` for explanation) \ No newline at end of file diff --git a/libs/wire-api/default.nix b/libs/wire-api/default.nix index 8b7f20896ec..e502149190a 100644 --- a/libs/wire-api/default.nix +++ b/libs/wire-api/default.nix @@ -43,6 +43,8 @@ , hex , hostname-validate , hscim +, hspec +, hspec-wai , http-api-data , http-media , http-types @@ -87,6 +89,7 @@ , tagged , tasty , tasty-expected-failure +, tasty-hspec , tasty-hunit , tasty-quickcheck , text @@ -223,6 +226,9 @@ mkDerivation { filepath hex hscim + hspec + hspec-wai + http-types imports iso3166-country-codes iso639 @@ -238,11 +244,13 @@ mkDerivation { saml2-web-sso schema-profunctor servant + servant-server servant-swagger-ui string-conversions swagger2 tasty tasty-expected-failure + tasty-hspec tasty-hunit tasty-quickcheck text @@ -253,6 +261,7 @@ mkDerivation { uri-bytestring uuid vector + wai wire-message-proto-lens ]; license = lib.licenses.agpl3Only; diff --git a/libs/wire-api/src/Wire/API/Routes/Version.hs b/libs/wire-api/src/Wire/API/Routes/Version.hs index 364de313d53..87327d699d9 100644 --- a/libs/wire-api/src/Wire/API/Routes/Version.hs +++ b/libs/wire-api/src/Wire/API/Routes/Version.hs @@ -1,3 +1,4 @@ +{-# LANGUAGE GeneralizedNewtypeDeriving #-} {-# LANGUAGE StandaloneKindSignatures #-} {-# LANGUAGE TemplateHaskell #-} -- This file is part of the Wire Server implementation. @@ -28,11 +29,9 @@ module Wire.API.Routes.Version -- * Version Version (..), + VersionNumber (..), supportedVersions, developmentVersions, - readVersionNumber, - mkVersion, - toPathComponent, -- * Servant combinators Until, @@ -40,11 +39,13 @@ module Wire.API.Routes.Version ) where +import Control.Error (note) import Control.Lens ((?~)) import Data.Aeson (FromJSON, ToJSON (..)) import qualified Data.Aeson as Aeson import Data.Bifunctor -import Data.ByteString.Conversion (ToByteString (builder)) +import qualified Data.Binary.Builder as Builder +import Data.ByteString.Conversion (ToByteString (builder), toByteString') import qualified Data.ByteString.Lazy as LBS import Data.Domain import Data.Schema @@ -57,52 +58,87 @@ import Servant import Servant.Swagger import Wire.API.Routes.Named import Wire.API.VersionInfo +import Wire.Arbitrary (Arbitrary, GenericUniform (GenericUniform)) --- | Version of the public API. Check the documentation in the *docs* directory --- for a checklist when adding a new version. +-- | Version of the public API. Serializes to `"v"`. See 'VersionNumber' below for one +-- that serializes to ``. See `/libs/wire-api/test/unit/Test/Wire/API/Routes/Version.hs` +-- for serialization rules. +-- +-- If you add or remove versions from this type, make sure 'versionInt', 'supportedVersions', +-- and 'developmentVersions' stay in sync; everything else here should keep working without +-- change. See also documentation in the *docs* directory. -- https://docs.wire.com/developer/developer/api-versioning.html#version-bump-checklist data Version = V0 | V1 | V2 | V3 | V4 - deriving stock (Eq, Ord, Bounded, Enum, Show) + deriving stock (Eq, Ord, Bounded, Enum, Show, Generic) deriving (FromJSON, ToJSON) via (Schema Version) + deriving (Arbitrary) via (GenericUniform Version) + +-- | Manual enumeration of version integrals (the `` in the constructor `V`). +-- +-- This is not the same as 'fromEnum': we will remove unsupported versions in the future, +-- which will cause `` and `fromEnum V` to diverge. `Enum` should not be understood as +-- a bijection between meaningful integers and versions, but merely as a convenient way to say +-- `allVersions = [minBound..]`. +versionInt :: Integral i => Version -> i +versionInt V0 = 0 +versionInt V1 = 1 +versionInt V2 = 2 +versionInt V3 = 3 +versionInt V4 = 4 + +supportedVersions :: [Version] +supportedVersions = [minBound .. V4] + +developmentVersions :: [Version] +developmentVersions = [V4] + +---------------------------------------------------------------------- + +versionText :: Version -> Text +versionText = ("v" <>) . toUrlPiece . versionInt @Int + +versionByteString :: Version -> ByteString +versionByteString = ("v" <>) . toByteString' . versionInt @Int instance ToSchema Version where - schema = - enum @Integer "Version" . mconcat $ - [ element 0 V0, - element 1 V1, - element 2 V2, - element 3 V3, - element 4 V4 - ] - -mkVersion :: Integer -> Maybe Version -mkVersion n = case Aeson.fromJSON (Aeson.Number (fromIntegral n)) of - Aeson.Error _ -> Nothing - Aeson.Success v -> pure v + schema = enum @Text "Version" . mconcat $ (\v -> element (versionText v) v) <$> [minBound ..] instance FromHttpApiData Version where - parseHeader = first Text.pack . Aeson.eitherDecode . LBS.fromStrict - parseUrlPiece = parseHeader . Text.encodeUtf8 + parseQueryParam v = note ("Unknown version: " <> v) $ + getAlt $ + flip foldMap [minBound ..] $ \s -> + guard (versionText s == v) $> s instance ToHttpApiData Version where - toHeader = LBS.toStrict . Aeson.encode - toUrlPiece = Text.decodeUtf8 . toHeader + toHeader = versionByteString + toUrlPiece = versionText instance ToByteString Version where - builder = toEncodedUrlPiece + builder = Builder.fromByteString . versionByteString + +-- | Wrapper around 'Version' that serializes to integers ``, as needed in +-- eg. `VersionInfo`. See `/libs/wire-api/test/unit/Test/Wire/API/Routes/Version.hs` for +-- serialization rules. +newtype VersionNumber = VersionNumber {fromVersionNumber :: Version} + deriving stock (Eq, Ord, Show, Generic) + deriving newtype (Bounded, Enum) + deriving (FromJSON, ToJSON) via (Schema VersionNumber) + deriving (Arbitrary) via (GenericUniform Version) + +instance ToSchema VersionNumber where + schema = + enum @Integer "VersionNumber" . mconcat $ (\v -> element (versionInt v) (VersionNumber v)) <$> [minBound ..] --- | `Version` as it appears in an URL path --- --- >>> toPathComponent V1 --- "v1" -toPathComponent :: Version -> ByteString -toPathComponent v = "v" <> toHeader v +instance FromHttpApiData VersionNumber where + parseHeader = first Text.pack . Aeson.eitherDecode . LBS.fromStrict + parseUrlPiece = parseHeader . Text.encodeUtf8 -supportedVersions :: [Version] -supportedVersions = [minBound .. maxBound] +instance ToHttpApiData VersionNumber where + toHeader = LBS.toStrict . Aeson.encode + toUrlPiece = Text.decodeUtf8 . toHeader -developmentVersions :: [Version] -developmentVersions = [V4] +instance ToByteString VersionNumber where + builder = toEncodedUrlPiece -- | Information related to the public API version. -- @@ -111,8 +147,8 @@ developmentVersions = [V4] -- backend, in order to decide how to form request paths, and how to deal with -- federated backends and qualified user IDs. data VersionInfo = VersionInfo - { vinfoSupported :: [Version], - vinfoDevelopment :: [Version], + { vinfoSupported :: [VersionNumber], + vinfoDevelopment :: [VersionNumber], vinfoFederation :: Bool, vinfoDomain :: Domain } @@ -130,7 +166,7 @@ instance ToSchema VersionInfo where example :: VersionInfo example = VersionInfo - { vinfoSupported = supportedVersions, + { vinfoSupported = VersionNumber <$> supportedVersions, vinfoDevelopment = [maxBound], vinfoFederation = False, vinfoDomain = Domain "example.com" diff --git a/libs/wire-api/src/Wire/API/Routes/Version/Wai.hs b/libs/wire-api/src/Wire/API/Routes/Version/Wai.hs index 545acdeae4d..50429098358 100644 --- a/libs/wire-api/src/Wire/API/Routes/Version/Wai.hs +++ b/libs/wire-api/src/Wire/API/Routes/Version/Wai.hs @@ -17,36 +17,50 @@ module Wire.API.Routes.Version.Wai where +import Control.Monad.Except (throwError) import Data.ByteString.Conversion -import qualified Data.Text.Lazy as LText +import Data.EitherR (fmapL) +import Data.String.Conversions (cs) +import qualified Data.Text as T import Imports import qualified Network.HTTP.Types as HTTP import Network.Wai import Network.Wai.Middleware.Rewrite import Network.Wai.Utilities.Error import Network.Wai.Utilities.Response +import Web.HttpApiData (parseUrlPiece, toUrlPiece) import Wire.API.Routes.Version -- | Strip off version prefix. Return 404 if the version is not supported. versionMiddleware :: Set Version -> Middleware versionMiddleware disabledAPIVersions app req k = case parseVersion (removeVersionHeader req) of - Nothing -> app req k - Just (req', n) -> case mkVersion n of - Just v | v `notElem` disabledAPIVersions -> app (addVersionHeader v req') k - _ -> - k $ - errorRs' $ - mkError HTTP.status404 "unsupported-version" $ - "Version " <> LText.pack (show n) <> " is not supported" - -parseVersion :: Request -> Maybe (Request, Integer) + Right (req', v) -> + if v `elem` disabledAPIVersions + then err (toUrlPiece v) + else app (addVersionHeader v req') k + Left (BadVersion v) -> err v + Left NoVersion -> app req k + where + err :: Text -> IO ResponseReceived + err v = + k . errorRs' . mkError HTTP.status404 "unsupported-version" $ + "Version " <> cs v <> " is not supported" + +data ParseVersionError = NoVersion | BadVersion Text + +parseVersion :: Request -> Either ParseVersionError (Request, Version) parseVersion req = do (version, pinfo) <- case pathInfo req of - [] -> Nothing + [] -> throwError NoVersion (x : xs) -> pure (x, xs) - n <- readVersionNumber version + unless (looksLikeVersion version) $ + throwError NoVersion + n <- fmapL (const $ BadVersion version) $ parseUrlPiece version pure (rewriteRequestPure (\(_, q) _ -> (pinfo, q)) req, n) +looksLikeVersion :: Text -> Bool +looksLikeVersion version = case T.splitAt 1 version of (h, t) -> h == "v" && T.all isDigit t + removeVersionHeader :: Request -> Request removeVersionHeader req = req diff --git a/libs/wire-api/src/Wire/API/VersionInfo.hs b/libs/wire-api/src/Wire/API/VersionInfo.hs index 7809b0411f0..a50b3407cd2 100644 --- a/libs/wire-api/src/Wire/API/VersionInfo.hs +++ b/libs/wire-api/src/Wire/API/VersionInfo.hs @@ -20,7 +20,6 @@ module Wire.API.VersionInfo vinfoObjectSchema, -- * Version utilities - readVersionNumber, versionHeader, VersionHeader, @@ -36,8 +35,6 @@ import qualified Data.CaseInsensitive as CI import Data.Metrics.Servant import Data.Schema import Data.Singletons -import qualified Data.Text as Text -import qualified Data.Text.Read as Text import GHC.TypeLits import Imports import qualified Network.Wai as Wai @@ -51,13 +48,6 @@ import Wire.API.Routes.ClientAlgebra vinfoObjectSchema :: ValueSchema NamedSwaggerDoc v -> ObjectSchema SwaggerDoc [v] vinfoObjectSchema sch = field "supported" (array sch) -readVersionNumber :: Text -> Maybe Integer -readVersionNumber v = do - ('v', rest) <- Text.uncons v - case Text.decimal rest of - Right (n, "") -> pure n - _ -> Nothing - type VersionHeader = "X-Wire-API-Version" versionHeader :: CI.CI ByteString diff --git a/libs/wire-api/test/unit/Main.hs b/libs/wire-api/test/unit/Main.hs index 7c3c3249c7e..3a0b5eb11a2 100644 --- a/libs/wire-api/test/unit/Main.hs +++ b/libs/wire-api/test/unit/Main.hs @@ -21,6 +21,7 @@ module Main where import Imports +import System.IO.Unsafe (unsafePerformIO) import Test.Tasty import qualified Test.Wire.API.Call.Config as Call.Config import qualified Test.Wire.API.Conversation as Conversation @@ -33,6 +34,7 @@ import qualified Test.Wire.API.Roundtrip.HttpApiData as Roundtrip.HttpApiData import qualified Test.Wire.API.Roundtrip.MLS as Roundtrip.MLS import qualified Test.Wire.API.Routes as Routes import qualified Test.Wire.API.Routes.Version as Routes.Version +import qualified Test.Wire.API.Routes.Version.Wai as Routes.Version.Wai import qualified Test.Wire.API.Swagger as Swagger import qualified Test.Wire.API.Team.Export as Team.Export import qualified Test.Wire.API.Team.Member as Team.Member @@ -63,5 +65,6 @@ main = Conversation.tests, MLS.tests, Routes.Version.tests, + unsafePerformIO Routes.Version.Wai.tests, RawJson.tests ] diff --git a/libs/wire-api/test/unit/Test/Wire/API/Roundtrip/Aeson.hs b/libs/wire-api/test/unit/Test/Wire/API/Roundtrip/Aeson.hs index 3586bb0a0b3..20348877470 100644 --- a/libs/wire-api/test/unit/Test/Wire/API/Roundtrip/Aeson.hs +++ b/libs/wire-api/test/unit/Test/Wire/API/Roundtrip/Aeson.hs @@ -47,6 +47,7 @@ import qualified Wire.API.Provider.Service as Provider.Service import qualified Wire.API.Provider.Service.Tag as Provider.Service.Tag import qualified Wire.API.Push.Token as Push.Token import qualified Wire.API.Routes.Internal.Galley.TeamsIntra as TeamsIntra +import qualified Wire.API.Routes.Version as Routes.Version import qualified Wire.API.SystemSettings as SystemSettings import qualified Wire.API.Team as Team import qualified Wire.API.Team.Conversation as Team.Conversation @@ -316,6 +317,8 @@ tests = testRoundTrip @User.Search.TeamContact, testRoundTrip @(Wrapped.Wrapped "some_int" Int), testRoundTrip @Conversation.Action.SomeConversationAction, + testRoundTrip @Routes.Version.Version, + testRoundTrip @Routes.Version.VersionNumber, testRoundTrip @TeamsIntra.GuardLegalholdPolicyConflicts, testRoundTrip @TeamsIntra.TeamStatus, testRoundTrip @TeamsIntra.TeamStatusUpdate, diff --git a/libs/wire-api/test/unit/Test/Wire/API/Roundtrip/HttpApiData.hs b/libs/wire-api/test/unit/Test/Wire/API/Roundtrip/HttpApiData.hs index 5ebf76a08e7..00ef20df73f 100644 --- a/libs/wire-api/test/unit/Test/Wire/API/Roundtrip/HttpApiData.hs +++ b/libs/wire-api/test/unit/Test/Wire/API/Roundtrip/HttpApiData.hs @@ -22,6 +22,7 @@ import Servant.API import qualified Test.Tasty as T import Test.Tasty.QuickCheck (Arbitrary, counterexample, testProperty, (===)) import Type.Reflection (typeRep) +import qualified Wire.API.Routes.Version import qualified Wire.API.User import qualified Wire.API.User.Search import qualified Wire.Arbitrary as Arbitrary () @@ -30,7 +31,9 @@ tests :: T.TestTree tests = T.localOption (T.Timeout (60 * 1000000) "60s") . T.testGroup "HttpApiData roundtrip tests" $ [ testRoundTrip @Wire.API.User.InvitationCode, - testRoundTrip @Wire.API.User.Search.PagingState + testRoundTrip @Wire.API.User.Search.PagingState, + testRoundTrip @Wire.API.Routes.Version.Version, + testRoundTrip @Wire.API.Routes.Version.VersionNumber ] testRoundTrip :: diff --git a/libs/wire-api/test/unit/Test/Wire/API/Routes/Version.hs b/libs/wire-api/test/unit/Test/Wire/API/Routes/Version.hs index 9af4812e89f..6195c711f5c 100644 --- a/libs/wire-api/test/unit/Test/Wire/API/Routes/Version.hs +++ b/libs/wire-api/test/unit/Test/Wire/API/Routes/Version.hs @@ -1,22 +1,45 @@ module Test.Wire.API.Routes.Version where +import Data.Aeson as Aeson +import Data.Binary.Builder +import Data.ByteString.Conversion +import Data.String.Conversions (cs) import Imports -import qualified Test.Tasty as T +import Servant.API +import Test.Tasty import Test.Tasty.HUnit import Wire.API.Routes.Version -tests :: T.TestTree +{-# ANN tests ("HLint: ignore Functor law" :: String) #-} +tests :: TestTree tests = - T.testGroup "Version" $ - [ T.testGroup - "toPathComponent" - [testCase "serialise different versions" testToPathComponent] + testGroup + "Version always has the shape V; serializations of Version and VersionNumber are `v`, ``, resp. is non-negative." + [ testCase "Version, show, 'v' prefix" $ do + nub (toLower . head . show <$> allVersions) @=? ['v'], + testCase "Version, show, int suffix" $ do + let expected = show $ (read @Int) . tail . show <$> allVersions + assertBool expected (isJust (Aeson.decode @[Int] (cs expected))), + testGroup "Version: all serializations are the same as `show`, up to string type" $ do + [ testCase "toByteString'" $ fmap toLower . show <$> allVersions @=? cs . toByteString' <$> allVersions, + testCase "encode" $ fmap toLower (show (show <$> allVersions)) @=? cs (encode allVersions), -- (`encode @Version` has extra double-quotes) + testCase "toUrlPiece" $ fmap toLower . show <$> allVersions @=? cs . toUrlPiece <$> allVersions, + testCase "toEncodedUrlPiece" $ fmap toLower . show <$> allVersions @=? cs . toLazyByteString . toEncodedUrlPiece <$> allVersions, + testCase "toHeader" $ fmap toLower . show <$> allVersions @=? cs . toHeader <$> allVersions, + testCase "toQueryParam" $ fmap toLower . show <$> allVersions @=? cs . toQueryParam <$> allVersions + ], + testGroup "VersionNumber: all serializations are the same as `tail . show . fromVersionNumber`, up to string type" $ + [ testCase "toByteString'" $ tail . show . fromVersionNumber <$> allVersionNumbers @=? cs . toByteString' <$> allVersionNumbers, + testCase "encode" $ tail . show . fromVersionNumber <$> allVersionNumbers @=? cs . encode <$> allVersionNumbers, + testCase "toUrlPiece" $ tail . show . fromVersionNumber <$> allVersionNumbers @=? cs . toUrlPiece <$> allVersionNumbers, + testCase "toEncodedUrlPiece" $ tail . show . fromVersionNumber <$> allVersionNumbers @=? cs . toLazyByteString . toEncodedUrlPiece <$> allVersionNumbers, + testCase "toHeader" $ tail . show . fromVersionNumber <$> allVersionNumbers @=? cs . toHeader <$> allVersionNumbers, + testCase "toQueryParam" $ tail . show . fromVersionNumber <$> allVersionNumbers @=? cs . toQueryParam <$> allVersionNumbers + ] ] -testToPathComponent :: Assertion -testToPathComponent = do - "v0" @=? toPathComponent V0 - "v1" @=? toPathComponent V1 - "v2" @=? toPathComponent V2 - "v3" @=? toPathComponent V3 - "v4" @=? toPathComponent V4 +allVersions :: [Version] +allVersions = [minBound ..] + +allVersionNumbers :: [VersionNumber] +allVersionNumbers = [minBound ..] diff --git a/libs/wire-api/test/unit/Test/Wire/API/Routes/Version/Wai.hs b/libs/wire-api/test/unit/Test/Wire/API/Routes/Version/Wai.hs new file mode 100644 index 00000000000..e5107205da0 --- /dev/null +++ b/libs/wire-api/test/unit/Test/Wire/API/Routes/Version/Wai.hs @@ -0,0 +1,67 @@ +module Test.Wire.API.Routes.Version.Wai where + +import Data.Proxy +import qualified Data.Set as Set +import Data.String.Conversions +import Data.Text as T +import Imports +import Network.HTTP.Types.Status (status200, status400) +import Network.Wai +import Servant.API +import Servant.Server +import Test.Hspec +import Test.Hspec.Wai +import Test.Hspec.Wai.Matcher +import Test.Tasty +import Test.Tasty.Hspec +import Wire.API.Routes.Version +import Wire.API.Routes.Version.Wai + +implicitVersion :: Text +implicitVersion = "0" + +sndGoodVersion :: Text +sndGoodVersion = "2" + +disabledVersion :: Text +disabledVersion = T.filter isDigit . cs $ toHeader disabledVersionTyped + +disabledVersionTyped :: Version +disabledVersionTyped = V3 + +unknownVersion :: Text +unknownVersion = "100" + +tests :: IO TestTree +tests = + testSpec "versionMiddleware" . with testApp $ do + mkTest Nothing Nothing ("good", 200) + mkTest Nothing (Just implicitVersion) ("mismatch: (Nothing,Just 0)", 400) + mkTest Nothing (Just sndGoodVersion) ("mismatch: (Nothing,Just 2)", 400) + mkTest (Just implicitVersion) (Just implicitVersion) ("good", 200) + mkTest (Just sndGoodVersion) (Just sndGoodVersion) ("good", 200) + mkTest (Just disabledVersion) (Just disabledVersion) (errmsg disabledVersion, 404) + mkTest (Just unknownVersion) (Just unknownVersion) (errmsg unknownVersion, 404) + where + errmsg v = "{\"code\":404,\"label\":\"unsupported-version\",\"message\":\"Version v" <> cs v <> " is not supported\"}" + +mkTest :: Maybe Text -> Maybe Text -> (LByteString, Int) -> SpecWith (st, Application) +mkTest mv1 mv2 (msg, status) = + it ("GET " <> cs path <> " => " <> show (msg, status)) $ do + get path `shouldRespondWith` ResponseMatcher status [] (bodyEquals msg) + where + path :: ByteString + path = cs $ maybe "" ("/v" <>) mv1 <> "/check-version" <> maybe "" ("?version=" <>) mv2 + +type TestAPI = "check-version" :> QueryParam "version" Int :> Raw + +testApp :: IO Application +testApp = pure $ versionMiddleware (Set.singleton disabledVersionTyped) (serve (Proxy @TestAPI) testHandler) + +testHandler :: Server TestAPI +testHandler mVersionNumber = Tagged $ \req cont -> + cont $ + let headerVersion = lookup "X-Wire-API-Version" (requestHeaders req) + in if headerVersion == (cs . show <$> mVersionNumber) + then responseLBS status200 [] "good" + else responseLBS status400 [] (cs $ "mismatch: " <> show (headerVersion, mVersionNumber)) diff --git a/libs/wire-api/wire-api.cabal b/libs/wire-api/wire-api.cabal index 8571f041ddf..2428be72dcb 100644 --- a/libs/wire-api/wire-api.cabal +++ b/libs/wire-api/wire-api.cabal @@ -660,6 +660,7 @@ test-suite wire-api-tests Test.Wire.API.Roundtrip.MLS Test.Wire.API.Routes Test.Wire.API.Routes.Version + Test.Wire.API.Routes.Version.Wai Test.Wire.API.Swagger Test.Wire.API.Team.Export Test.Wire.API.Team.Member @@ -734,6 +735,9 @@ test-suite wire-api-tests , filepath , hex , hscim + , hspec + , hspec-wai + , http-types , imports , iso3166-country-codes , iso639 @@ -749,11 +753,13 @@ test-suite wire-api-tests , saml2-web-sso , schema-profunctor , servant + , servant-server , servant-swagger-ui , string-conversions , swagger2 , tasty , tasty-expected-failure + , tasty-hspec , tasty-hunit , tasty-quickcheck , text @@ -764,6 +770,7 @@ test-suite wire-api-tests , uri-bytestring , uuid , vector + , wai , wire-api , wire-message-proto-lens diff --git a/nix/wire-server.nix b/nix/wire-server.nix index 8fc1b22edd1..0077c4dac96 100644 --- a/nix/wire-server.nix +++ b/nix/wire-server.nix @@ -373,7 +373,7 @@ in pkgs.netcat pkgs.niv (pkgs.python3.withPackages - (ps: with ps; [ pyyaml ])) + (ps: with ps; [ pyyaml requests ])) pkgs.rsync pkgs.wget pkgs.yq diff --git a/services/brig/src/Brig/API/Public/Swagger.hs b/services/brig/src/Brig/API/Public/Swagger.hs index 0e75164817a..514a44c47fa 100644 --- a/services/brig/src/Brig/API/Public/Swagger.hs +++ b/services/brig/src/Brig/API/Public/Swagger.hs @@ -60,7 +60,7 @@ pregenSwagger :: Version -> Q Exp pregenSwagger v = embedLazyByteString =<< makeRelativeToProject - ("docs/swagger-v" <> T.unpack (toUrlPiece v) <> ".json") + ("docs/swagger-v" <> T.unpack (toUrlPiece (VersionNumber v)) <> ".json") swaggerPregenUIServer :: LByteString -> Server SwaggerDocsAPIBase swaggerPregenUIServer = diff --git a/services/brig/src/Brig/Effects/GalleyProvider/RPC.hs b/services/brig/src/Brig/Effects/GalleyProvider/RPC.hs index 65ee53fead4..f028516bf38 100644 --- a/services/brig/src/Brig/Effects/GalleyProvider/RPC.hs +++ b/services/brig/src/Brig/Effects/GalleyProvider/RPC.hs @@ -99,7 +99,7 @@ createSelfConv u = do void $ ServiceRPC.request @'Galley POST req where req = - paths ["v" <> toHeader (maxBound :: Version), "conversations", "self"] + paths [toHeader (maxBound :: Version), "conversations", "self"] . zUser u . expect2xx @@ -125,7 +125,7 @@ getConv usr lcnv = do where req = paths - [ "v" <> toHeader (maxBound :: Version), + [ toHeader (maxBound :: Version), "conversations", toByteString' (tDomain lcnv), toByteString' (tUnqualified lcnv) @@ -155,7 +155,7 @@ getTeamConv usr tid cnv = do where req = paths - [ "v" <> toHeader (maxBound :: Version), + [ toHeader (maxBound :: Version), "teams", toByteString' tid, "conversations", diff --git a/services/brig/src/Brig/Version.hs b/services/brig/src/Brig/Version.hs index acf7603d76b..f3d6a5ca89e 100644 --- a/services/brig/src/Brig/Version.hs +++ b/services/brig/src/Brig/Version.hs @@ -40,8 +40,8 @@ versionAPI = Named $ do | otherwise = Set.difference allVersions devVersions pure $ VersionInfo - { vinfoSupported = toList supported, - vinfoDevelopment = toList devVersions, + { vinfoSupported = VersionNumber <$> toList supported, + vinfoDevelopment = VersionNumber <$> toList devVersions, vinfoFederation = isJust fed, vinfoDomain = dom } diff --git a/services/brig/test/integration/API/Version.hs b/services/brig/test/integration/API/Version.hs index 07e52638ad3..1315d356ec9 100644 --- a/services/brig/test/integration/API/Version.hs +++ b/services/brig/test/integration/API/Version.hs @@ -53,7 +53,7 @@ testVersion brig = do =<< get (brig . path "/api-version") vinfoSupported vinfo) @?= supportedVersions \\ developmentVersions testVersionV1 :: Brig -> Http () testVersionV1 brig = do @@ -62,7 +62,7 @@ testVersionV1 brig = do =<< get (apiVersion "v1" . brig . path "api-version") vinfoSupported vinfo) @?= supportedVersions \\ developmentVersions testDevVersion :: Opts -> Brig -> Http () testDevVersion opts brig = withSettingsOverrides @@ -73,7 +73,7 @@ testDevVersion opts brig = withSettingsOverrides =<< get (brig . path "/api-version") vinfoSupported vinfo) @?= supportedVersions testUnsupportedVersion :: Brig -> Http () testUnsupportedVersion brig = do @@ -127,7 +127,7 @@ testDisabledVersionIsUnsupported opts brig = do testVersionDisabledSupportedVersion :: Opts -> Brig -> Http () testVersionDisabledSupportedVersion opts brig = do vinfo <- getVersionInfo brig - liftIO $ filter (== V2) (vinfoSupported vinfo) @?= [V2] + liftIO $ filter (== VersionNumber V2) (vinfoSupported vinfo) @?= [VersionNumber V2] disabledVersionIsNotAdvertised opts brig V2 testVersionDisabledDevelopmentVersion :: Opts -> Brig -> Http () @@ -135,7 +135,7 @@ testVersionDisabledDevelopmentVersion opts brig = do vinfo <- getVersionInfo brig for_ (listToMaybe (vinfoDevelopment vinfo)) $ \devVersion -> do liftIO $ filter (== devVersion) (vinfoDevelopment vinfo) @?= [devVersion] - disabledVersionIsNotAdvertised opts brig devVersion + disabledVersionIsNotAdvertised opts brig (fromVersionNumber devVersion) disabledVersionIsNotAdvertised :: Opts -> Brig -> Version -> Http () disabledVersionIsNotAdvertised opts brig version = @@ -147,8 +147,8 @@ disabledVersionIsNotAdvertised opts brig version = ) $ do vinfo <- getVersionInfo brig - liftIO $ filter (== version) (vinfoSupported vinfo) @?= [] - liftIO $ filter (== version) (vinfoDevelopment vinfo) @?= [] + liftIO $ filter (== VersionNumber version) (vinfoSupported vinfo) @?= [] + liftIO $ filter (== VersionNumber version) (vinfoDevelopment vinfo) @?= [] getVersionInfo :: (MonadIO m, MonadCatch m, MonadHttp m, HasCallStack) => diff --git a/services/brig/test/integration/Main.hs b/services/brig/test/integration/Main.hs index 90b64986f4a..ad1d6a02d69 100644 --- a/services/brig/test/integration/Main.hs +++ b/services/brig/test/integration/Main.hs @@ -192,7 +192,7 @@ runTests iConf brigOpts otherArgs = do mkVersionedRequest endpoint = addPrefix . mkRequest endpoint addPrefix :: Request -> Request - addPrefix r = r {HTTP.path = "v" <> toHeader latestVersion <> "/" <> removeSlash (HTTP.path r)} + addPrefix r = r {HTTP.path = toHeader latestVersion <> "/" <> removeSlash (HTTP.path r)} where removeSlash s = case B8.uncons s of Just ('/', s') -> s' diff --git a/services/cargohold/test/integration/TestSetup.hs b/services/cargohold/test/integration/TestSetup.hs index 7b494caa43a..759c760a698 100644 --- a/services/cargohold/test/integration/TestSetup.hs +++ b/services/cargohold/test/integration/TestSetup.hs @@ -115,7 +115,7 @@ unversioned r = viewCargohold :: TestM Cargohold viewCargohold = fmap - (apiVersion ("v" <> toHeader latestVersion) .) + (apiVersion (toHeader latestVersion) .) viewUnversionedCargohold where latestVersion :: Version diff --git a/services/galley/test/integration/API/Util.hs b/services/galley/test/integration/API/Util.hs index ade69830f11..77bb1f65a70 100644 --- a/services/galley/test/integration/API/Util.hs +++ b/services/galley/test/integration/API/Util.hs @@ -150,7 +150,7 @@ addPrefix :: Request -> Request addPrefix = addPrefixAtVersion maxBound addPrefixAtVersion :: Version -> Request -> Request -addPrefixAtVersion v r = r {HTTP.path = "v" <> toHeader v <> "/" <> removeSlash (HTTP.path r)} +addPrefixAtVersion v r = r {HTTP.path = toHeader v <> "/" <> removeSlash (HTTP.path r)} where removeSlash s = case B8.uncons s of Just ('/', s') -> s' diff --git a/services/run-services b/services/run-services index 9fdbf894e6e..1cbd415f36f 100755 --- a/services/run-services +++ b/services/run-services @@ -10,17 +10,17 @@ import shutil import socket import subprocess import yaml -import urllib.request -import urllib.error import sys import tempfile import time import traceback import threading +import requests @dataclass class SpawnFailException(Exception): failed_instances: object + failing_status_responses: object class Colors: GREEN = "\x1b[38;5;10m" @@ -107,6 +107,18 @@ class Nginz: if shutil.which("nginx") is None: raise Exception("nginx not found") +@dataclass +class StatusResponse: + http_response: object = None + good_status: list[int] = None + + def __bool__(self): + if self.http_response is None: + return False + else: + return self.http_response.status_code in self.good_status + + @dataclass(frozen=True) class Instance: service: Service @@ -122,9 +134,9 @@ class Instance: if not self.service.check_status: return True try: - with urllib.request.urlopen(f"http://localhost:{self.port}/i/status") as resp: - return resp.status in [200, 204] - except urllib.error.URLError: + resp = requests.get(f"http://localhost:{self.port}/i/status") + return StatusResponse(resp, [200, 204]) + except Exception as e: return False def spawn(self, service_map, environment, suffix, domain, backend_name): @@ -323,6 +335,9 @@ def start_backend(services, suffix, domain, backend_name): # check instances to_be_checked = [instance for instance in instances if instance.exception is None] + + failing_status_responses = {} + start_time = time.time() while to_be_checked: if time.time() - start_time >= 5: @@ -333,8 +348,10 @@ def start_backend(services, suffix, domain, backend_name): to_be_checked_again = set() for instance in to_be_checked: try: - if not instance.check_status(): + status = instance.check_status() + if not status: to_be_checked_again.add(instance) + failing_status_responses[instance] = status except Exception as e: failed_instances.append(replace(instance, exception=e)) @@ -346,7 +363,7 @@ def start_backend(services, suffix, domain, backend_name): if failed_instances: cleanup_instances(instances) - raise SpawnFailException(failed_instances) + raise SpawnFailException(failed_instances, failing_status_responses) return instances @@ -439,5 +456,10 @@ if __name__ == '__main__': for instance in e.failed_instances: print(f"{instance.service.name} at port {instance.port}" + (f" ({instance.exception})" if instance.exception else "")) + + for instance, status in e.failing_status_responses.items(): + if isinstance(status, StatusResponse): + print(f"{instance.service.name} responded with status " + + "{status.http_response.status_code} and body:\n " + status.http_response.text) finally: cleanup_instances(instances) diff --git a/tools/stern/src/Stern/Intra.hs b/tools/stern/src/Stern/Intra.hs index fe9429062da..6c80801e33c 100644 --- a/tools/stern/src/Stern/Intra.hs +++ b/tools/stern/src/Stern/Intra.hs @@ -89,6 +89,7 @@ import Imports import Network.HTTP.Types.Method import Network.HTTP.Types.Status hiding (statusCode) import Network.Wai.Utilities (Error (..), mkError) +import Servant.API (toUrlPiece) import Stern.App import Stern.Types import System.Logger.Class hiding (Error, name, (.=)) @@ -120,10 +121,10 @@ backendApiVersion :: Version backendApiVersion = V2 path :: ByteString -> Request -> Request -path = Bilge.path . ((toPathComponent backendApiVersion <> "/") <>) +path = Bilge.path . ((cs (toUrlPiece backendApiVersion) <> "/") <>) paths :: [ByteString] -> Request -> Request -paths = Bilge.paths . (toPathComponent backendApiVersion :) +paths = Bilge.paths . (cs (toUrlPiece backendApiVersion) :) -------------------------------------------------------------------------------