Skip to content

WPB-23792 public setting for nomad profiles#5077

Merged
battermann merged 8 commits intodevelopfrom
WPB-23792-provide-wire-server-setting-nomad-profiles-via-unauthenticated-endpoint
Mar 3, 2026
Merged

WPB-23792 public setting for nomad profiles#5077
battermann merged 8 commits intodevelopfrom
WPB-23792-provide-wire-server-setting-nomad-profiles-via-unauthenticated-endpoint

Conversation

@battermann
Copy link
Contributor

@battermann battermann commented Mar 3, 2026

https://wearezeta.atlassian.net/browse/WPB-23792

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

@battermann battermann marked this pull request as ready for review March 3, 2026 11:24
@battermann battermann requested review from a team as code owners March 3, 2026 11:24
@battermann battermann requested a review from Copilot March 3, 2026 11:24
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Mar 3, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 optional setNomadProfiles flag 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/test coverage 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.

battermann and others added 3 commits March 3, 2026 12:32
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have a dedicated data-type for this? Or are least a newtype?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would we gain from that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Today, that would avoid ambiguity/the necessity to go through this comment.

Eventually, I think it will help with refactorings.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@battermann battermann requested a review from blackheaven March 3, 2026 12:52
@battermann battermann merged commit 3ac0a24 into develop Mar 3, 2026
11 checks passed
@battermann battermann deleted the WPB-23792-provide-wire-server-setting-nomad-profiles-via-unauthenticated-endpoint branch March 3, 2026 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants