[Q1-2024] WPB-4657 disable development API version

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

@@ -0,0 +1,3 @@
+The settings `setDisabledAPIVersions` (brig) and `disabledAPIVersions` (in cannon, cargohold, galley, gundeck, proxy, and spar) are now required.
+The default defined in `charts/<service>/values.yaml` is set to `[ development ]` and disables all development API versions.
+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/templates/configmap.yaml

@@ -323,9 +323,7 @@ data:
       {{- if .setOAuthEnabled }}
       setOAuthEnabled: {{ .setOAuthEnabled }}
       {{- end }}
-      {{- if .setDisabledAPIVersions }}
       setDisabledAPIVersions: {{ .setDisabledAPIVersions }}
-      {{- end }}
       {{- if .setOAuthRefreshTokenExpirationTimeSecs }}
       setOAuthRefreshTokenExpirationTimeSecs: {{ .setOAuthRefreshTokenExpirationTimeSecs }}
       {{- end }}

charts/brig/values.yaml

@@ -107,7 +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.
-    # setDisabledAPIVersions: [ v3 ]
+    setDisabledAPIVersions: [ development ]
     setFederationStrategy: allowNone
     setFederationDomainConfigsUpdateFreq: 10
   smtp:

charts/cannon/templates/configmap.yaml

@@ -19,9 +19,7 @@ data:
       millisecondsBetweenBatches: {{ .Values.config.drainOpts.millisecondsBetweenBatches }}
       minBatchSize: {{ .Values.config.drainOpts.minBatchSize }}
 
-    {{- if .Values.config.disabledAPIVersions }}
     disabledAPIVersions: {{ .Values.config.disabledAPIVersions }}
-    {{- end }}
 
 kind: ConfigMap
 metadata:

charts/cannon/values.yaml

@@ -24,7 +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.
-  # disabledAPIVersions: [ v3 ]
+  disabledAPIVersions: [ development ]
 
 metrics:
   serviceMonitor:

charts/cargohold/templates/configmap.yaml

@@ -56,7 +56,5 @@ data:
       downloadLinkTTL: {{ .downloadLinkTTL }}
       {{- end }}
       federationDomain: {{ .federationDomain }}
-      {{- if .disabledAPIVersions }}
       disabledAPIVersions: {{ .disabledAPIVersions }}
       {{- end }}
-      {{- end }}

charts/cargohold/values.yaml

@@ -32,7 +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.
-    # disabledAPIVersions: [ v3 ]
+    disabledAPIVersions: [ development ]
 
 serviceAccount:
   # When setting this to 'false', either make sure that a service account named

charts/galley/templates/configmap.yaml

@@ -79,10 +79,8 @@ data:
         removal:
           ed25519: "/etc/wire/galley/secrets/removal_ed25519.pem"
         {{- end }}
-      {{- end -}}
-      {{- if .settings.disabledAPIVersions }}
-      disabledAPIVersions: {{ .settings.disabledAPIVersions }}
       {{- end }}
+      disabledAPIVersions: {{ .settings.disabledAPIVersions }}
       {{- if .settings.featureFlags }}
       {{- if .settings.guestLinkTTLSeconds }}
       guestLinkTTLSeconds: {{ .settings.guestLinkTTLSeconds }}

charts/galley/values.yaml

@@ -57,7 +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.
-    # disabledAPIVersions: [ v3 ]
+    disabledAPIVersions: [ development ]
     # 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
     guestLinkTTLSeconds: 31536000

charts/gundeck/templates/configmap.yaml

@@ -60,9 +60,7 @@ data:
       {{- if hasKey . "perNativePushConcurrency" }}
       perNativePushConcurrency: {{ .perNativePushConcurrency }}
       {{- end }}
-      {{- if .disabledAPIVersions }}
       disabledAPIVersions: {{ .disabledAPIVersions }}
-      {{- end }}
       # disabledAPIVersions: [ 2 ]
       maxConcurrentNativePushes:
         soft: {{ .maxConcurrentNativePushes.soft }}

charts/gundeck/values.yaml

@@ -44,7 +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.
-  # disabledAPIVersions: [ v3 ]
+  disabledAPIVersions: [ development ]
 
   # Maximum number of bytes loaded into memory when fetching (referenced) payloads.
   # Gundeck will return a truncated page if the whole page's payload sizes would exceed this limit in total.

charts/proxy/templates/configmap.yaml

@@ -7,9 +7,7 @@ data:
     logFormat: {{ .Values.config.logFormat }}
     logLevel: {{ .Values.config.logLevel }}
     logNetStrings: {{ .Values.config.logNetStrings }}
-    {{- if .Values.config.disabledAPIVersions }}
     disabledAPIVersions: {{ .Values.config.disabledAPIVersions }}
-    {{- end }}
     host: 0.0.0.0
     port: {{ .Values.service.internalPort }}
     httpPoolSize: 1000

charts/proxy/values.yaml

@@ -21,7 +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.
-  # disabledAPIVersions: [ v3 ]
+  disabledAPIVersions: [ development ]
 
 podSecurityContext:
   allowPrivilegeEscalation: false

charts/spar/templates/configmap.yaml

@@ -36,9 +36,7 @@ data:
 
     maxScimTokens: {{ .maxScimTokens }}
 
-    {{- if .disabledAPIVersions }}
     disabledAPIVersions: {{ .disabledAPIVersions }}
-    {{- end }}
 
     saml:
       version:     SAML2.0

charts/spar/values.yaml

@@ -34,7 +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.
-  # disabledAPIVersions: [ v3 ]
+  disabledAPIVersions: [ development ]
 
 podSecurityContext:
   allowPrivilegeEscalation: false

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 ]
 
@@ -671,7 +666,7 @@ config.settings.disabledAPIVersions: [ v3 ]
 config.settings.disabledAPIVersions: [ v3 ]
 
 # gundecks' values.yaml
-config.disabledAPIVersions: [ v3 ]
+config.settings.disabledAPIVersions: [ v3 ]
 
 # proxy's values.yaml
 config.disabledAPIVersions: [ v3 ]
@@ -680,7 +675,15 @@ 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 ]
+```
+
+This setting is required to be present for all the services (brig, cannon, cargohold, galley, gundeck, proxy, and spar).
+
+The default value (provided under `charts/<service>/values.yaml`) is `[ development ]` and disables the development versions. To enable all versions including the development versions 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,8 @@ cannon:
     limits:
       memory: 512Mi
   drainTimeout: 0
+  config:
+    disabledAPIVersions: []
 cargohold:
   replicaCount: 1
   imagePullPolicy: {{ .Values.imagePullPolicy }}
@@ -184,6 +187,7 @@ cargohold:
     settings:
       # See helmfile for the real value
       federationDomain: integration.example.com
+      disabledAPIVersions: []
   secrets:
     awsKeyId: dummykey
     awsSecretKey: dummysecret
@@ -218,6 +222,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
@@ -289,6 +295,7 @@ gundeck:
       queueName: integration-gundeck-events
       sqsEndpoint: http://fake-aws-sqs:4568
       snsEndpoint: http://fake-aws-sns:4575
+    disabledAPIVersions: []
     bulkPush: true
     setMaxConcurrentNativePushes:
       hard: 30
@@ -341,6 +348,8 @@ proxy:
               giphy      = "..."
               spotify    = "Basic ..."
        }
+  config:
+    disabledAPIVersions: []
 spar:
   replicaCount: 1
   imagePullPolicy: {{ .Values.imagePullPolicy }}
@@ -368,6 +377,7 @@ spar:
     - type: ContactSupport
       company: Example Company
       email: email:backend+spar@wire.com
+    disabledAPIVersions: []
   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/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
+
+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