WPB-4657 disable development API version

changelog.d/0-release-notes/WPB-4657

@@ -0,0 +1,4 @@
+The development API is now disabled by default.
+The old structure of the server config is compatible with this change. So it is safe to deploy this without changing the server config.
+However, if you want to disable the development API explicitly by adding the `development` keyword to the list of disabled APIs, this has to be done during or after the deployment of the services, not before.
+For more information see <https://docs.wire.com/developer/reference/config-options.html#disabling-api-versions>

changelog.d/5-internal/WPB-4657

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

charts/brig/values.yaml

@@ -107,6 +107,7 @@ config:
     setOAuthMaxActiveRefreshTokens: 10
     # Disable one ore more API versions. Please make sure the configuration value is the same in all these charts:
     # brig, cannon, cargohold, galley, gundeck, proxy, spar.
+    # The default value if not set is `["development"]` which disables the development API versions.
     # setDisabledAPIVersions: [ v3 ]
     setFederationStrategy: allowNone
     setFederationDomainConfigsUpdateFreq: 10

charts/cannon/values.yaml

@@ -24,6 +24,7 @@ config:
 
   # Disable one ore more API versions. Please make sure the configuration value is the same in all these charts:
   # brig, cannon, cargohold, galley, gundeck, proxy, spar.
+  # The default value if not set is `["development"]` which disables the development API versions.
   # disabledAPIVersions: [ v3 ]
 
 metrics:

charts/cargohold/values.yaml

@@ -32,6 +32,7 @@ config:
     downloadLinkTTL: 300 # Seconds
     # Disable one ore more API versions. Please make sure the configuration value is the same in all these charts:
     # brig, cannon, cargohold, galley, gundeck, proxy, spar.
+    # The default value if not set is `["development"]` which disables the development API versions.
     # disabledAPIVersions: [ v3 ]
 
 serviceAccount:

charts/galley/values.yaml

@@ -57,6 +57,7 @@ config:
     multiIngress: null
     # Disable one ore more API versions. Please make sure the configuration value is the same in all these charts:
     # brig, cannon, cargohold, galley, gundeck, proxy, spar.
+    # The default value if not set is `["development"]` which disables the development API versions.
     # disabledAPIVersions: [ v3 ]
     # The lifetime of a conversation guest link in seconds. Must be a value 0 < x <= 31536000 (365 days)
     # Default is 31536000 (365 days) if not set

charts/gundeck/values.yaml

@@ -44,6 +44,7 @@ config:
     soft: 1000
   # Disable one ore more API versions. Please make sure the configuration value is the same in all these charts:
   # brig, cannon, cargohold, galley, gundeck, proxy, spar.
+  # The default value if not set is `["development"]` which disables the development API versions.
   # disabledAPIVersions: [ v3 ]
 
   # Maximum number of bytes loaded into memory when fetching (referenced) payloads.

charts/proxy/values.yaml

@@ -21,6 +21,7 @@ config:
   proxy: {}
   # Disable one ore more API versions. Please make sure the configuration value is the same in all these charts:
   # brig, cannon, cargohold, galley, gundeck, proxy, spar.
+  # The default value if not set is `["development"]` which disables the development API versions.
   # disabledAPIVersions: [ v3 ]
 
 podSecurityContext:

charts/spar/values.yaml

@@ -34,6 +34,7 @@ config:
   proxy: {}
   # Disable one ore more API versions. Please make sure the configuration value is the same in all these charts:
   # brig, cannon, cargohold, galley, gundeck, proxy, spar.
+  # The default value if not set is `["development"]` which disables the development API versions.
   # disabledAPIVersions: [ v3 ]
 
 podSecurityContext:

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

@@ -591,10 +591,6 @@ See {ref}`configure-federation-strategy-in-brig` (since [PR#3260](https://github
 
 ### API Versioning
 
-#### `setEnableDevelopmentVersions`
-
-This options determines whether development versions should be enabled. If set to `False`, all development versions are removed from the `supported` field of the `/api-version` endpoint. Note that they are still listed in the `development` field, and continue to work normally.
-
 ### OAuth
 
 For more information on OAuth please refer to <https://docs.wire.com/developer/reference/oauth.html>.
@@ -654,10 +650,9 @@ It is possible to disable one ore more API versions. When an API version is disa
 
 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 ]
 
@@ -680,7 +675,13 @@ config.disabledAPIVersions: [ v3 ]
 config.disabledAPIVersions: [ v3 ]
 ```
 
-The default setting is that no API version is disabled.
+The development API version(s) can be disabled either explicitly or by adding the `development` keyword to the list of disabled API versions. E.g.:
+
+```yaml
+config.disabledAPIVersions: [ v3, development ]
+```
+
+The default setting (in case the value is not present in the server configuration) is that all development versions are disabled while all other supported versions are enabled. To enable all versions including the development version set the value to be empty: `[]`.
 
 ## Settings in cargohold
 

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

@@ -95,6 +95,7 @@ brig:
       setFederationDomain: integration.example.com
       setFederationStrategy: allowAll
       setFederationDomainConfigsUpdateFreq: 10
+      setDisabledAPIVersions: []
       set2FACodeGenerationDelaySecs: 5
       setNonceTtlSecs: 300
       setDpopMaxSkewSecs: 1
@@ -169,6 +170,7 @@ cannon:
     limits:
       memory: 512Mi
   drainTimeout: 0
+  disabledAPIVersions: []
 cargohold:
   replicaCount: 1
   imagePullPolicy: {{ .Values.imagePullPolicy }}
@@ -184,6 +186,7 @@ cargohold:
     settings:
       # See helmfile for the real value
       federationDomain: integration.example.com
+      disabledAPIVersions: []
   secrets:
     awsKeyId: dummykey
     awsSecretKey: dummysecret
@@ -218,6 +221,8 @@ galley:
       conversationCodeURI: https://kube-staging-nginz-https.zinfra.io/conversation-join/
       # See helmfile for the real value
       federationDomain: integration.example.com
+      disabledAPIVersions: []
+
       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
+    disabledAPIVersions: []
   secrets:
     awsKeyId: dummykey
     awsSecretKey: dummysecret
@@ -341,6 +347,8 @@ proxy:
               giphy      = "..."
               spotify    = "Basic ..."
        }
+  config:
+    disabledAPIVersions: []
 spar:
   replicaCount: 1
   imagePullPolicy: {{ .Values.imagePullPolicy }}
@@ -368,6 +376,7 @@ spar:
     - type: ContactSupport
       company: Example Company
       email: email:backend+spar@wire.com
+    disabledAPIVersions: []
   tests:
     {{- if .Values.uploadXml }}
     config:

integration/integration.cabal

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

integration/test/API/Brig.hs

@@ -398,11 +398,17 @@ replaceKeyPackages cid suites kps = do
       & addQueryParams [("ciphersuites", intercalate "," (map (.code) suites))]
       & addJSONObject ["key_packages" .= map (T.decodeUtf8 . Base64.encode) kps]
 
-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
+-- | https://staging-nginz-https.zinfra.io/v6/api/swagger-ui/#/default/get_self
+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 = getSelfWithVersion Versioned $ object ["domain" .= domain, "id" .= uid]
 
 getUserSupportedProtocols ::
   (HasCallStack, MakesValue user, MakesValue target) =>

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
 
@@ -117,18 +117,18 @@ testDynamicBackend = do
         resp.json %. "setRestrictUserCreation" `shouldMatch` False
 
     -- user created in own domain should not be found in dynamic backend
-    bindResponse (BrigP.getSelf dynDomain uid) $ \resp -> do
+    bindResponse (BrigP.getSelf' dynDomain uid) $ \resp -> do
       resp.status `shouldMatchInt` 404
 
     -- 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
 
     -- the d1 user should not be found in the own domain
-    bindResponse (BrigP.getSelf ownDomain uidD1) $ \resp -> do
+    bindResponse (BrigP.getSelf' ownDomain uidD1) $ \resp -> do
       resp.status `shouldMatchInt` 404
 
 testStartMultipleDynamicBackends :: HasCallStack => App ()

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,109 @@
+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
+
+testVersionDisabledDevNotAdvertised :: App ()
+testVersionDisabledDevNotAdvertised = withModifiedBackend
+  def {brigCfg = setField "optSettings.setDisabledAPIVersions" ["development"]}
+  $ \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
+
+testVersionDevDisabledPerDefault :: App ()
+testVersionDevDisabledPerDefault = withModifiedBackend
+  def {brigCfg = removeField "optSettings.setDisabledAPIVersions"}
+  $ \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