WPB-4657 Disabling development versions

changelog.d/5-internal/WPB-4657

@@ -0,0 +1 @@
+The development API version is now disabled by default

charts/brig/templates/configmap.yaml

@@ -332,5 +332,8 @@ data:
       {{- if .setOAuthMaxActiveRefreshTokens }}
       setOAuthMaxActiveRefreshTokens: {{ .setOAuthMaxActiveRefreshTokens }}
       {{- end }}
+      {{- if .setEnableDevAPI }}
+      setEnableDevAPI: {{ .setEnableDevAPI }}
+      {{- end }}
     {{- end }}
   {{- end }}

charts/cannon/templates/configmap.yaml

@@ -23,6 +23,10 @@ data:
     disabledAPIVersions: {{ .Values.config.disabledAPIVersions }}
     {{- end }}
 
+    {{- if .Values.config.enableDevAPI }}
+    enableDevAPI: {{ .Values.config.enableDevAPI }}
+    {{- end }}
+
 kind: ConfigMap
 metadata:
   name: cannon

charts/cargohold/templates/configmap.yaml

@@ -59,4 +59,7 @@ data:
       {{- if .disabledAPIVersions }}
       disabledAPIVersions: {{ .disabledAPIVersions }}
       {{- end }}
+      {{- if .enableDevAPI }}
+      enableDevAPI: {{ .enableDevAPI }}
+      {{- end }}
       {{- end }}

charts/galley/templates/configmap.yaml

@@ -87,6 +87,9 @@ data:
       {{- if .settings.guestLinkTTLSeconds }}
       guestLinkTTLSeconds: {{ .settings.guestLinkTTLSeconds }}
       {{- end }}
+      {{- if .settings.enableDevAPI }}
+      enableDevAPI: {{ .settings.enableDevAPI }}
+      {{- end }}
       featureFlags:
         sso: {{ .settings.featureFlags.sso }}
         legalhold: {{ .settings.featureFlags.legalhold }}

charts/gundeck/templates/configmap.yaml

@@ -63,7 +63,6 @@ data:
       {{- if .disabledAPIVersions }}
       disabledAPIVersions: {{ .disabledAPIVersions }}
       {{- end }}
-      # disabledAPIVersions: [ 2 ]
       maxConcurrentNativePushes:
         soft: {{ .maxConcurrentNativePushes.soft }}
         {{- if hasKey .maxConcurrentNativePushes "hard" }}
@@ -72,5 +71,8 @@ data:
         # hard: 30  # more than this number of threads will not be allowed
       maxPayloadLoadSize : {{ .maxPayloadLoadSize }}
       internalPageSize : {{ .internalPageSize }}
+      {{- if .enableDevAPI }}
+      enableDevAPI: {{ .enableDevAPI }}
+      {{- end }}
 
   {{- end }}

charts/proxy/templates/configmap.yaml

@@ -10,6 +10,9 @@ data:
     {{- if .Values.config.disabledAPIVersions }}
     disabledAPIVersions: {{ .Values.config.disabledAPIVersions }}
     {{- end }}
+    {{- if .Values.config.enableDevAPI }}
+    enableDevAPI: {{ .Values.config.enableDevAPI }}
+    {{- end }}
     host: 0.0.0.0
     port: {{ .Values.service.internalPort }}
     httpPoolSize: 1000

charts/spar/templates/configmap.yaml

@@ -39,6 +39,9 @@ data:
     {{- if .disabledAPIVersions }}
     disabledAPIVersions: {{ .disabledAPIVersions }}
     {{- end }}
+    {{- if .enableDevAPI }}
+    enableDevAPI: {{ .enableDevAPI }}
+    {{- end }}
 
     saml:
       version:     SAML2.0

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

@@ -648,16 +648,44 @@ optSettings:
   setOAuthMaxActiveRefreshTokens: 10
 ```
 
+#### Enable Development API
+
+To enable the development API version(s), set the following flags to true for brig, cannon, cargohold, galley, gundeck, proxy, spar. This setting should be consistent across all services.
+
+```yaml
+# brig's values.yaml
+config.optSettings.setEnableDevAPI: true
+
+# cannon's values.yaml
+config.enableDevAPI: true
+
+# cargohold's values.yaml
+config.settings.enableDevAPI: true
+
+# galley's values.yaml
+config.settings.enableDevAPI: true
+
+# gundecks' values.yaml
+config.enableDevAPI: true
+
+# proxy's values.yaml
+config.enableDevAPI: true
+
+# spar's values.yaml
+config.enableDevAPI: true
+```
+
+Per default the development API version(s) are disabled. Disabling an API version explicitly (as described below) takes precedence over enabled development API versions.
+
 #### Disabling API versions
 
 It is possible to disable one ore more API versions. When an API version is disabled it won't be advertised on the `GET /api-version` endpoint, neither in the `supported`, nor in the `development` section. Requests made to any endpoint of a disabled API version will result in the same error response as a request made to an API version that does not exist.
 
 Each of the services brig, cannon, cargohold, galley, gundeck, proxy, spar should to be configured with the same set of disable API versions in each service's values.yaml config files.
 
-
 For example to disable API version v3, you need to configure:
 
-```
+```yaml
 # brig's values.yaml
 config.optSettings.setDisabledAPIVersions: [ v3 ]
 

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

@@ -105,6 +105,7 @@ brig:
       setOAuthEnabled: true
       setOAuthRefreshTokenExpirationTimeSecs: 14515200 # 24 weeks
       setOAuthMaxActiveRefreshTokens: 10
+      setEnableDevAPI: true
     aws:
       sesEndpoint: http://fake-aws-ses:4569
       sqsEndpoint: http://fake-aws-sqs:4568
@@ -169,6 +170,8 @@ cannon:
     limits:
       memory: 512Mi
   drainTimeout: 0
+  config:
+    enableDevAPI: true
 cargohold:
   replicaCount: 1
   imagePullPolicy: {{ .Values.imagePullPolicy }}
@@ -184,6 +187,7 @@ cargohold:
     settings:
       # See helmfile for the real value
       federationDomain: integration.example.com
+    enableDevAPI: true
   secrets:
     awsKeyId: dummykey
     awsSecretKey: dummysecret
@@ -218,6 +222,7 @@ galley:
       conversationCodeURI: https://kube-staging-nginz-https.zinfra.io/conversation-join/
       # See helmfile for the real value
       federationDomain: integration.example.com
+      enableDevAPI: true
       featureFlags:
         sso: disabled-by-default  # this needs to be the default; tests can enable it when needed.
         legalhold: whitelist-teams-and-implicit-consent
@@ -293,6 +298,7 @@ gundeck:
     setMaxConcurrentNativePushes:
       hard: 30
       soft: 10
+    enableDevAPI: true
   secrets:
     awsKeyId: dummykey
     awsSecretKey: dummysecret
@@ -332,6 +338,8 @@ nginz:
 proxy:
   replicaCount: 1
   imagePullPolicy: {{ .Values.imagePullPolicy }}
+  config:
+    enableDevAPI: true
   secrets:
     proxy_config: |-
       secrets {
@@ -368,6 +376,7 @@ spar:
     - type: ContactSupport
       company: Example Company
       email: email:backend+spar@wire.com
+    enableDevAPI: true
   tests:
     {{- if .Values.uploadXml }}
     config:

integration/integration.cabal

@@ -138,6 +138,7 @@ library
     Test.Services
     Test.Swagger
     Test.User
+    Test.Version
     Testlib.App
     Testlib.Assertions
     Testlib.Cannon

integration/test/API/Brig.hs

@@ -394,18 +394,16 @@ replaceKeyPackages cid suites kps = do
       & addJSONObject ["key_packages" .= map (T.decodeUtf8 . Base64.encode) kps]
 
 -- | https://staging-nginz-https.zinfra.io/v6/api/swagger-ui/#/default/get_self
-getSelf :: (HasCallStack, MakesValue caller) => caller -> App Response
-getSelf caller = do
-  req <- baseRequest caller Brig Versioned "/self"
-  submit "GET" req
+getSelf :: (HasCallStack, MakesValue user) => user -> App Response
+getSelf = getSelfWithVersion Versioned
+
+getSelfWithVersion :: (HasCallStack, MakesValue user) => Versioned -> user -> App Response
+getSelfWithVersion v user = baseRequest user Brig v "/self" >>= submit "GET"
 
 -- | https://staging-nginz-https.zinfra.io/v6/api/swagger-ui/#/default/get_self
 -- this is a low-level version of `getSelf` for testing some error conditions.
 getSelf' :: HasCallStack => String -> String -> App Response
-getSelf' domain uid = do
-  let user = object ["domain" .= domain, "id" .= uid]
-  req <- baseRequest user Brig Versioned "/self"
-  submit "GET" req
+getSelf' domain uid = getSelfWithVersion Versioned $ object ["domain" .= domain, "id" .= uid]
 
 data PutSelf = PutSelf
   { accent :: Maybe Int,

integration/test/Test/Demo.hs

@@ -105,7 +105,7 @@ testDynamicBackend = do
   ownDomain <- objDomain OwnDomain
   user <- randomUser OwnDomain def
   uid <- objId user
-  bindResponse (BrigP.getSelf' ownDomain uid) $ \resp -> do
+  bindResponse (BrigP.getSelf user) $ \resp -> do
     resp.status `shouldMatchInt` 200
     (resp.json %. "id") `shouldMatch` objId user
 
@@ -123,7 +123,7 @@ testDynamicBackend = do
     -- now create a user in the dynamic backend
     userD1 <- randomUser dynDomain def
     uidD1 <- objId userD1
-    bindResponse (BrigP.getSelf' dynDomain uidD1) $ \resp -> do
+    bindResponse (BrigP.getSelf userD1) $ \resp -> do
       resp.status `shouldMatchInt` 200
       (resp.json %. "id") `shouldMatch` objId userD1
 

integration/test/Test/Swagger.hs

@@ -23,13 +23,6 @@ testSwagger = do
       dev <- resp.json %. "development" & asSetOf asIntegral
       pure $ sup <> dev
     assertBool ("unexpected actually existing versions: " <> show actualVersions) $
-      -- make sure nobody has added a new version without adding it to `existingVersions`.
-      -- make sure nobody has added a new version without adding it to `existingVersions`.
-      -- ("subset" because blocked versions like v3 are not actually existing, but still
-      -- ("subset" because blocked versions like v3 are not actually existing, but still
-      -- documented.)
-      -- documented.)
-
       -- make sure nobody has added a new version without adding it to `existingVersions`.
       -- ("subset" because blocked versions like v3 are not actually existing, but still
       -- documented.)

integration/test/Test/Version.hs

@@ -0,0 +1,97 @@
+module Test.Version where
+
+import API.Brig
+import qualified Data.Set as Set
+import SetupHelpers
+import Testlib.Prelude
+
+newtype Versioned' = Versioned' Versioned
+
+-- | This instance is used to generate tests for some of the versions. (Not checking all of them for time efficiency reasons)
+instance HasTests x => HasTests (Versioned' -> x) where
+  mkTests m n s f x =
+    mkTests m (n <> "[version=unversioned]") s f (x (Versioned' Unversioned))
+      <> mkTests m (n <> "[version=versioned]") s f (x (Versioned' Versioned))
+      <> mkTests m (n <> "[version=v1]") s f (x (Versioned' (ExplicitVersion 1)))
+      <> mkTests m (n <> "[version=v3]") s f (x (Versioned' (ExplicitVersion 3)))
+      <> mkTests m (n <> "[version=v6]") s f (x (Versioned' (ExplicitVersion 6)))
+
+testVersion :: Versioned' -> App ()
+testVersion (Versioned' v) = withModifiedBackend
+  def {brigCfg = setField "optSettings.setDisabledAPIVersions" ([] :: [String])}
+  $ \dom ->
+    bindResponse (baseRequest dom Brig v "/api-version" >>= submit "GET") $ \resp -> do
+      resp.status `shouldMatchInt` 200
+      dev <- resp.json %. "development" & asSet
+      supported <- resp.json %. "supported" & asSet
+      domain <- resp.json %. "domain" & asString
+      federation <- resp.json %. "federation" & asBool
+
+      -- currently there is only one development version
+      -- it is however theoretically possible to have multiple development versions
+      length dev `shouldMatchInt` 1
+      domain `shouldMatch` dom
+      federation `shouldMatch` True
+
+      unless (null (Set.intersection supported dev)) $
+        assertFailure "development and supported versions should not overlap"
+
+testVersionUnsupported :: App ()
+testVersionUnsupported = bindResponse (baseRequest OwnDomain Brig (ExplicitVersion 500) "/api-version" >>= submit "GET") $
+  \resp -> do
+    resp.status `shouldMatchInt` 404
+    resp.json %. "label" `shouldMatch` "unsupported-version"
+
+testVersionDisabled :: App ()
+testVersionDisabled = withModifiedBackend
+  def {brigCfg = setField "optSettings.setDisabledAPIVersions" ["v2"]}
+  $ \domain -> do
+    do
+      user <- randomUser OwnDomain def
+      void $ getSelfWithVersion (ExplicitVersion 2) user >>= getJSON 200
+
+    do
+      user <- randomUser domain def
+      bindResponse (getSelfWithVersion (ExplicitVersion 2) user) $ \resp -> do
+        resp.status `shouldMatchInt` 404
+        resp.json %. "label" `shouldMatch` "unsupported-version"
+
+      void $ getSelfWithVersion (ExplicitVersion 1) user >>= getJSON 200
+      void $ getSelfWithVersion (ExplicitVersion 3) user >>= getJSON 200
+      void $ getSelfWithVersion (ExplicitVersion 4) user >>= getJSON 200
+      void $ getSelfWithVersion (ExplicitVersion 5) user >>= getJSON 200
+      void $ getSelfWithVersion (ExplicitVersion 6) user >>= getJSON 200
+      void $ getSelfWithVersion Unversioned user >>= getJSON 200
+
+testVersionDisabledNotAdvertised :: App ()
+testVersionDisabledNotAdvertised = do
+  allVersions <- bindResponse (baseRequest OwnDomain Brig Versioned "/api-version" >>= submit "GET") $ \resp ->
+    (<>)
+      <$> (resp.json %. "development" & asList >>= traverse asInt)
+      <*> (resp.json %. "supported" & asList >>= traverse asInt)
+  forM_ allVersions testWithVersion
+  where
+    testWithVersion :: Int -> App ()
+    testWithVersion v = withModifiedBackend
+      def {brigCfg = setField "optSettings.setDisabledAPIVersions" ["v" <> show v]}
+      $ \domain -> do
+        bindResponse (getAPIVersion domain) $ \resp -> do
+          resp.status `shouldMatchInt` 200
+          dev <- resp.json %. "development" & asList >>= traverse asInt
+          supported <- resp.json %. "supported" & asList >>= traverse asInt
+
+          assertBool "supported versions should not be empty" $ not (null supported)
+          assertBool "the disabled version should not be propagated as dev version" $ v `notElem` dev
+          assertBool "the disabled version should not be propagated as supported version" $ v `notElem` supported
+
+testVersionDevDisabledPerDefault :: App ()
+testVersionDevDisabledPerDefault = withModifiedBackend
+  def {brigCfg = removeField "optSettings.setEnableDevAPI"}
+  $ \domain -> do
+    bindResponse (getAPIVersion domain) $ \resp -> do
+      resp.status `shouldMatchInt` 200
+      dev <- resp.json %. "development" & asList
+      supported <- resp.json %. "supported" & asList
+
+      assertBool "supported versions should not be empty" $ not (null supported)
+      assertBool "development versions should be empty" $ null dev

integration/test/Testlib/JSON.hs

@@ -131,6 +131,9 @@ asListOf :: HasCallStack => (Value -> App b) -> MakesValue a => a -> App [b]
 asListOf makeElem x =
   asList x >>= mapM makeElem
 
+asSet :: HasCallStack => MakesValue a => a -> App (Set.Set Value)
+asSet = fmap Set.fromList . asList
+
 asSetOf :: (HasCallStack, Ord b) => (Value -> App b) -> MakesValue a => a -> App (Set.Set b)
 asSetOf makeElem x = Set.fromList <$> asListOf makeElem x
 

libs/wire-api/src/Wire/API/Routes/Version.hs

@@ -1,3 +1,4 @@
+{-# LANGUAGE DeriveAnyClass #-}
 {-# LANGUAGE GeneralizedNewtypeDeriving #-}
 {-# LANGUAGE StandaloneKindSignatures #-}
 {-# LANGUAGE TemplateHaskell #-}
@@ -34,6 +35,7 @@ module Wire.API.Routes.Version
     supportedVersions,
     isDevelopmentVersion,
     developmentVersions,
+    toDisabledVersions,
 
     -- * Servant combinators
     Until,
@@ -55,6 +57,7 @@ import Data.ByteString.Lazy qualified as LBS
 import Data.Domain
 import Data.OpenApi qualified as S
 import Data.Schema
+import Data.Set qualified as Set
 import Data.Singletons.Base.TH
 import Data.Text qualified as Text
 import Data.Text.Encoding as Text
@@ -64,7 +67,7 @@ import Servant
 import Servant.API.Extended.RawM
 import Wire.API.Deprecated
 import Wire.API.Routes.MultiVerb
-import Wire.API.Routes.Named
+import Wire.API.Routes.Named hiding (unnamed)
 import Wire.API.VersionInfo
 import Wire.Arbitrary (Arbitrary, GenericUniform (GenericUniform))
 
@@ -204,6 +207,9 @@ isDevelopmentVersion _ = True
 developmentVersions :: [Version]
 developmentVersions = filter isDevelopmentVersion supportedVersions
 
+toDisabledVersions :: Maybe Bool -> Set Version
+toDisabledVersions mEnableDevAPI = Set.fromList $ maybe developmentVersions (const []) mEnableDevAPI
+
 -- Version-aware swagger generation
 
 $(promoteOrdInstances [''Version])