WPB-23792 public setting for nomad profiles#5077
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new public system setting to advertise “nomad profiles” support via Brig’s system settings endpoint, and updates integration tests + configuration docs/templates accordingly.
Changes:
- Extend Brig settings/config (
optSettings) with an optionalsetNomadProfilesflag and expose it via/system/settings/unauthorized. - Update Brig’s system settings handlers and wire-api schema to include the new public field.
- Move/replace old Brig integration tests with new
integration/testcoverage and update cabal/module lists.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| services/brig/test/integration/Run.hs | Removes the old SystemSettings integration tests from Brig’s integration runner. |
| services/brig/test/integration/API/SystemSettings.hs | Deletes legacy SystemSettings test module. |
| services/brig/src/Brig/Options.hs | Adds nomadProfiles :: Maybe Bool to Brig settings (configurable via setNomadProfiles). |
| services/brig/src/Brig/API/Public.hs | Extends system settings responses and refactors handlers to reuse internal construction. |
| services/brig/brig.cabal | Drops removed test module from the integration executable. |
| libs/wire-api/src/Wire/API/SystemSettings.hs | Adds nomadProfiles to the public schema and renames the Haskell field for restrict-user-creation. |
| integration/test/Test/SystemSettings.hs | Adds new integration tests validating public/internal system settings behavior. |
| integration/test/API/Brig.hs | Adds API helpers for the system settings endpoints. |
| integration/integration.cabal | Registers the new Test.SystemSettings module. |
| docs/src/developer/reference/config-options.md | Documents the new setNomadProfiles config option. |
| charts/brig/templates/configmap.yaml | Renders setNomadProfiles into brig.yaml when configured. |
| changelog.d/2-features/WPB-23792 | Changelog entry for the new public setting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| -- fall back to their default behaviour. | ||
| -- * 'Just True' – nomad profiles are explicitly supported/enabled. | ||
| -- * 'Just False' – nomad profiles are explicitly not supported/disabled. | ||
| nomadProfiles :: !(Maybe Bool) |
There was a problem hiding this comment.
Can we have a dedicated data-type for this? Or are least a newtype?
There was a problem hiding this comment.
What would we gain from that?
There was a problem hiding this comment.
Today, that would avoid ambiguity/the necessity to go through this comment.
Eventually, I think it will help with refactorings.
There was a problem hiding this comment.
That comment was a suggestion from copilot. I actually regret have accepted it.
It's just a setting for clients. It's optional because it is not needed in 99% of the environments.
True means that nomad profiles are enabled, that's all.
I would not introduce a type for it, we can still do that if we think it has become useful later.
This is following the same pattern as other settings.
https://wearezeta.atlassian.net/browse/WPB-23792
Checklist
changelog.d