-
Notifications
You must be signed in to change notification settings - Fork 25
Add ADR 32 about breaking up Core project #826
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
justindbaur
wants to merge
2
commits into
main
Choose a base branch
from
add-adr-about-breaking-up-core
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+206
β0
Draft
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,206 @@ | ||
| --- | ||
| adr: "0032" | ||
| status: "Proposed" | ||
| date: 2026-06-25 | ||
| tags: [server] | ||
| --- | ||
|
|
||
| # 0032 - Break up the Core project | ||
|
|
||
| <AdrTable frontMatter={frontMatter}></AdrTable> | ||
|
|
||
| ## Context and problem statement | ||
|
|
||
| The `Core` project in the `server` repository has grown into a catch-all library that almost every | ||
| service depends on. This creates two significant problems: | ||
|
|
||
| - **Unowned code and unbounded dependencies** β while much of `Core` is owned by specific teams, a | ||
| significant portion has accumulated without clear ownership, making it a dumping ground for shared | ||
| utilities and dependencies that don't have a better home. | ||
| - **Limited independent deployability** β because every service depends on `Core`, any change to | ||
| `Core` technically constitutes a change to every service. This makes it hard to reason about the | ||
| blast radius of a change and undermines the ability to deploy services independently with | ||
| confidence. | ||
|
|
||
| `GlobalSettings` compounds these problems. It is a single configuration class that houses settings | ||
| for every feature and service, meaning all of `Core`'s consumers must load and be aware of the | ||
| entire settings surface even when they only need a small slice of it. As features are extracted from | ||
| `Core`, they should define their own strongly-typed options classes rather than growing | ||
| `GlobalSettings` further. | ||
|
|
||
| Not all of what currently lives in `Core` needs to move into feature-scoped libraries. A number of | ||
| cross-cutting concerns β feature flag evaluation, version info endpoints, security middleware, and | ||
| caching β are being built into the server SDK. As those SDK packages mature, the corresponding code | ||
| in `Core` can be removed and replaced with an SDK dependency, further shrinking `Core`'s footprint. | ||
|
|
||
| Other Bitwarden repositories have already moved toward a feature-scoped library model. The | ||
| `sdk-internal` repository was built on this pattern from the start and follows it fully. The | ||
| `clients` repository has historically had a large `libs/common` package, which is still being | ||
| gradually decomposed into feature-scoped libraries. These precedents validate the approach for | ||
| `server` as well. | ||
|
|
||
| ## Considered options | ||
|
|
||
| - **Keep `Core` as-is** β No structural changes. `Core` continues to grow as a shared monolith. | ||
| Ownership and deployment problems persist. | ||
| - **Break `Core` into feature-scoped libraries** β New code is placed in dedicated, feature-scoped | ||
| projects. Platform-level utilities (push notifications, mailing, database foundations) are | ||
| extracted first as shared dependencies. Existing code is migrated gradually and opportunistically. | ||
|
|
||
| ## Decision outcome | ||
|
|
||
| Chosen option: **Break `Core` into feature-scoped libraries**. | ||
|
|
||
| New code belonging to a specific feature or domain should live in its own dedicated project rather | ||
| than in `Core`. Platform-level utilities that many features depend on β such as push notifications, | ||
| mailing, and database foundations β should also be extracted into their own projects, as these | ||
| represent cross-cutting infrastructure rather than feature logic. | ||
|
|
||
| In conjunction with [ADR 0031](./0031-adopt-minimal-apis.md), a single library can cover a feature | ||
| end-to-end: repositories, settings, services, and endpoints all in one `.csproj`. There is no | ||
| requirement to separate endpoints into their own project. The goal is feature cohesion, not a | ||
| mandated split between endpoint code and business logic. | ||
|
|
||
| Feature libraries live under `src/Libraries/[Feature]`. If a library later graduates into its own | ||
| deployable container, it moves to `src/Services/[Name]`. This extends the `src/Libraries` directory | ||
| structure introduced in [ADR 0031](./0031-adopt-minimal-apis.md). | ||
|
|
||
| ``` | ||
| src/ | ||
| Libraries/ | ||
| Mailer/ # settings, services, repositories, endpoints for the Mailer feature | ||
| Push/ | ||
| Vault/ | ||
| ... | ||
| Services/ | ||
| Api/ # composes libraries into a deployable service | ||
| Identity/ | ||
| Notifications/ | ||
| ... | ||
| ``` | ||
|
|
||
| Libraries and services under `bitwarden_license/` follow the exact same layout, rooted there instead | ||
| of `src/`: | ||
|
|
||
| ``` | ||
| bitwarden_license/ | ||
| Libraries/ | ||
| SecretsManager/ # settings, services, repositories, endpoints for Secrets Manager | ||
| ... | ||
| Services/ | ||
| Scim/ # composes libraries into a deployable service | ||
| ... | ||
| ``` | ||
|
|
||
| Libraries use the root namespace `Bit.[Feature]` and an assembly name of `[Feature]`. Services use | ||
| the root namespace `Bit.Services.[Name]` and an assembly name of `[Name]`. This applies primarily to | ||
| net new code; when migrating existing code out of `Core`, retaining the existing namespace is | ||
| acceptable if it keeps breaking changes to a minimum. | ||
|
|
||
| The `Core` project will remain during the migration period and code should be moved out | ||
| opportunistically, when a team is already working in that area, rather than in a dedicated | ||
| large-scale migration effort. The long-term goal is to eliminate `Core` entirely. As the migration | ||
| progresses, teams will negotiate the boundaries that should exist between them and create the | ||
| libraries needed to share code across those boundaries. | ||
|
|
||
| ### Positive consequences | ||
|
|
||
| - Clear ownership β teams own their feature project's `.csproj` and control their own dependencies | ||
| - A change to a feature library only affects services that actually depend on it, restoring the | ||
| ability to reason about and deploy services independently | ||
| - Aligns `server` with the patterns already established in `clients` and `sdk-internal` | ||
| - Complements [ADR 0031](./0031-adopt-minimal-apis.md), which establishes feature-scoped endpoint | ||
| libraries for minimal API endpoints | ||
|
|
||
| ### Negative consequences | ||
|
|
||
| - `Core` and feature-scoped libraries will coexist for a long time, requiring developers to know | ||
| where to place new code and where to look for existing code | ||
| - Extracting code from `Core` carries a risk of introducing circular dependencies if the dependency | ||
| graph is not carefully considered during a migration | ||
| - Without an aggressive timeline, the migration may stall and the benefits will be slow to | ||
| materialize | ||
|
|
||
| ### Plan | ||
|
|
||
| - New features should not add code to `Core`; they should create or extend a feature-scoped project | ||
| - New libraries should generally sit at a lower level than `Core` and should not depend on it; if a | ||
| new library needs something that currently lives in `Core`, that is a signal that the dependency | ||
| itself should be extracted into its own library first | ||
| - Platform-level utilities (push notifications, mailing, database foundations) should be prioritized | ||
| for extraction as independent projects, since many features will depend on them | ||
| - Cross-cutting concerns already being built into the server SDK β feature flag evaluation, version | ||
| info endpoints, security middleware, and caching β should be adopted from the SDK as those | ||
| packages become available, allowing the corresponding `Core` code to be deleted rather than | ||
| migrated | ||
| - Feature libraries should define their own strongly-typed options classes rather than adding | ||
| properties to `GlobalSettings`; as features are extracted, their `GlobalSettings` entries should | ||
| migrate alongside them | ||
|
|
||
| **Before:** | ||
|
|
||
| ```csharp | ||
| // Core/Settings/GlobalSettings.cs | ||
| public class GlobalSettings : IGlobalSettings | ||
| { | ||
| public virtual MailerSettings Mailer { get; set; } = new MailerSettings(); | ||
|
|
||
| public class MailerSettings | ||
| { | ||
| public string ReplyToEmail { get; set; } | ||
| public string SmtpHost { get; set; } | ||
| // ... | ||
| } | ||
| } | ||
|
|
||
| // Consuming service | ||
| public class Mailer(GlobalSettings globalSettings) : IMailer | ||
| { | ||
| public void Send() | ||
| { | ||
| var host = globalSettings.Mailer.SmtpHost; | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| **After:** | ||
|
|
||
| The feature library declares its options type and consumes it via `IOptions<T>`. It does not bind | ||
| configuration itself β that is the host service's responsibility, since the host owns its service | ||
| settings and knows which configuration sections map to which feature options. | ||
|
|
||
| ```csharp | ||
| // Libraries/Mailer/MailerSettings.cs | ||
| public class MailerSettings | ||
| { | ||
| public string ReplyToEmail { get; set; } | ||
| public string SmtpHost { get; set; } | ||
| // ... | ||
| } | ||
|
|
||
| // Libraries/Mailer/ServiceCollectionExtensions.cs | ||
| public static IServiceCollection AddMailers(this IServiceCollection services) | ||
| { | ||
| services.TryAddSingleton<IMailer, Mailer>(); | ||
| return services; | ||
| } | ||
|
|
||
| // Libraries/Mailer/Mailer.cs | ||
| internal class Mailer(IOptions<MailerSettings> settings) : IMailer | ||
| { | ||
| public void Send() | ||
| { | ||
| var host = settings.Value.SmtpHost; | ||
| } | ||
| } | ||
|
|
||
| // Services/Api/Program.cs | ||
| builder.Services.Configure<MailerSettings>(builder.Configuration.GetSection("Mailer")); | ||
| builder.Services.AddMailers(); | ||
| ``` | ||
|
|
||
| - Existing code in `Core` should be moved out opportunistically when a team is already working in | ||
| that area β not as a standalone task | ||
| - A guide will be written documenting the conventions for creating a new feature library and the | ||
| expected project structure, similar to the `ENDPOINT_LIBRARY.md` described in | ||
| [ADR 0031](./0031-adopt-minimal-apis.md) | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we consider how bitwarden licenses libraries and services will look? I think we will start to see more of those. Are they under a different top namespace, or do libraries and services follow a prefix? (Currently libraries loosely use a
Commercialprefix.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should look the exact same as this layout just starting at
bitwarden_license. I don't really see a need for a different namespace. If things are feature scoped I don't think things should collide.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably. I think it's worth including an example or two though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done