docs(bh-rfc-6): DogTags (SKU Feature Entitlements) BED-7094 #2226
Conversation
WalkthroughAdds an RFC (bh-rfc-6.md) defining "Dogtags" — a SKU-based feature entitlements design with a provider abstraction, startup-time provider injection and caching, fail-fast provider load, public service interfaces, an API endpoint, and guidance for extending and testing flags. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as "API /api/v2/dog-tags"
participant Service as "Dogtags Service"
participant Provider
participant Startup as "Startup (load/cache)"
Startup->>Provider: load entitlements at startup (fail-fast on error)
Startup->>Service: inject provider + cached entitlements
Client->>API: GET /api/v2/dog-tags
API->>Service: request flattened dog-tag payload
Service->>Provider: query flag values (GetFlagAsBool/String/Int)
Provider-->>Service: return values (from cache/source)
Service-->>API: assemble dot-notated JSON
API-->>Client: respond with flat dog-tags JSON
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @rfc/bh-rfc-6.md:
- Line 14: Replace the compound adjective "SKU level configuration" with the
hyphenated form "SKU-level configuration" wherever that phrase appears
(specifically the instance that describes configuration as a modifier of
"configuration" in the sentence starting "Dogtags is a provider-based
system...") so the compound adjective is correctly hyphenated.
🧹 Nitpick comments (4)
rfc/bh-rfc-6.md (4)
31-36: Clarify provider failure modes and startup behavior.The document states that "changes require a restart" and adopts a fail-fast approach, but doesn't detail which provider errors are fatal vs. recoverable. For example: does a provider unable to connect to a database fail startup, or does it fall back to defaults? Are there partial-load scenarios where some flags fail but others succeed?
Consider adding a subsection to Section 3 (Considerations) or Section 4 (Architecture) that explicitly covers provider failure modes and their consequences. For example:
### 3.5 Provider Failure Modes - **Configuration Error** (missing or invalid config file): Fails startup immediately - **Network/Connectivity Error** (database unreachable): Fails startup immediately - **Missing Flag in Provider** (flag not in provider but in code): Service returns defaultThis prevents ambiguity during implementation and testing.
51-72: Clarify Provider error semantics and relationship to Service defaults.Lines 51–55 show that
Providermethods return errors, and line 58 says "providers return errors when a key isn't found." However, it's not explicit whether a missing key returns an error or a zero value. Additionally, line 62 says the service "falls back to the default defined in the SKU spec" if a provider doesn't have a flag, but it's unclear whether this means the provider returns an error or behaves differently.Consider adding an explicit example showing the provider error contract:
// Provider missing a key returns ErrFlagNotFound or similar value, err := provider.GetFlagAsBool("unknown_flag") // err is non-nil; service catches it and uses default // Provider with a key returns the value value, err := provider.GetFlagAsBool("existing_flag") // err is nil; service uses provider's valueThis removes ambiguity for implementers.
31-32: Document thread-safety and concurrency guarantees.The design caches entitlements at startup (lines 31–32) for reuse across the application lifecycle. However, there's no mention of whether the cached map or individual flag values are safe for concurrent access by multiple goroutines. For a system handling HTTP requests concurrently, this is important to clarify.
Consider adding a note such as:
### 3.6 Thread Safety The cached entitlements are immutable after startup and safe for concurrent read access.Alternatively, if mutations are possible post-startup, specify the synchronization strategy (e.g., RWMutex).
76-87: Document API error handling and authorization.The endpoint example (lines 76–87) shows a successful response structure, but doesn't cover error cases: What is the HTTP status code if the client is unauthorized? If the dogtags service fails to initialize? If a flag value is missing (which shouldn't happen, but documenting the contract is useful)?
Additionally, is there authentication/authorization required for this endpoint, or is it public?
Consider adding a paragraph documenting:
### 4.3.1 Error Responses - **401 Unauthorized**: Client lacks permission to access dogtags - **500 Internal Server Error**: Dogtags service initialization failed - **200 OK with partial data**: If a flag fails to serialize, omit it from the response (or include an error field)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
rfc/bh-rfc-6.md
🧰 Additional context used
🪛 LanguageTool
rfc/bh-rfc-6.md
[grammar] ~14-~14: Use a hyphen to join words.
Context: ...ure entitlements. It abstracts where SKU level configuration comes from, allowing...
(QB_NEW_EN_HYPHEN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: run-tests
- GitHub Check: run-analysis
- GitHub Check: build-ui
rfc/bh-rfc-6.md
Outdated
|
|
||
| ### 4.3 API Endpoint | ||
|
|
||
| Dogtags are exposed via `GET /api/v2/dog-tags`: |
There was a problem hiding this comment.
Can there be a callout that the response here should always maintain a flat map?
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @rfc/bh-rfc-6.md:
- Line 14: Update the sentence that currently reads "SKU level configuration" to
use the hyphenated form "SKU-level configuration" so the compound modifier
correctly hyphenates when modifying the noun; locate this phrase in the Dogtags
description paragraph (the line starting "Dogtags is a provider-based system for
managing feature entitlements...") and replace the unhyphenated phrase with
"SKU-level configuration".
🧹 Nitpick comments (3)
rfc/bh-rfc-6.md (3)
46-58: Clarify provider error-handling semantics.The Provider interface description states that "Providers return errors when a key isn't found," but doesn't document whether other error types are possible (e.g., initialization errors, I/O failures) or how the service should handle them. Consider clarifying the error contract.
For example, you could add:
type Provider interface { // GetFlagAsBool retrieves a boolean flag. // Returns an error if the key is not found; other errors indicate // a provider-level failure (e.g., I/O, invalid state). GetFlagAsBool(key string) (bool, error) GetFlagAsString(key string) (string, error) GetFlagAsInt(key string) (int64, error) }Or document the error contract separately in the Considerations section.
76-87: Enhance API response documentation for clarity.The API endpoint section describes a flat, dot-notated JSON structure but could be more explicit about the response envelope. Clarify whether:
- The
"data"wrapper is always present- Clients should expect all flagged keys in every response or only a subset
- The response includes only active flags or all known flags
Consider adding:
The endpoint returns a flat map of all available SKU flags with their current values. Keys use dot notation for namespacing (e.g., `privilege_zones.tier_limit`), but this is purely a naming convention. All flags defined in `sku_flags.go` are included in the response, including those using default values.
122-127: Expand testing guidance with initialization details.The testing section recommends using
NoopProviderbut the example code is incomplete. Clarify:
- The exact signature and parameters of
NewDefaultService()- Whether consumers need to inject a
NoopProviderexplicitly or if it's the default- Whether and how to mock providers for testing specific flag values
Consider expanding the example:
// Testing with defaults (CE behavior) dogtagsService := dogtags.NewDefaultService() // uses NoopProvider internally enabled := dogtagsService.GetFlagAsBool(dogtags.MY_FEATURE) // enabled is false (the default) // Testing with specific values mockProvider := &mockDogtags.Provider{ BoolFlags: map[string]bool{ "my_feature.enabled": true, }, } dogtagsService := dogtags.NewService(mockProvider)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
rfc/bh-rfc-6.md
🧰 Additional context used
🪛 LanguageTool
rfc/bh-rfc-6.md
[grammar] ~14-~14: Use a hyphen to join words.
Context: ...ure entitlements. It abstracts where SKU level configuration comes from, allowing...
(QB_NEW_EN_HYPHEN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: run-analysis
- GitHub Check: build-ui
- GitHub Check: run-tests
|
|
||
| ## 1. Overview | ||
|
|
||
| Dogtags is a provider-based system for managing feature entitlements. It abstracts where SKU level configuration comes from, allowing the same application code to work whether entitlements are sourced from a license file, a database, or some future backend we haven't thought of yet. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and read the relevant section
if [ -f "rfc/bh-rfc-6.md" ]; then
echo "=== File exists ==="
echo "=== Line 14 context (lines 10-20) ==="
sed -n '10,20p' rfc/bh-rfc-6.md | cat -n
echo ""
echo "=== Search for all 'SKU level' instances ==="
rg "SKU level" rfc/bh-rfc-6.md -n
else
echo "File not found"
fiRepository: SpecterOps/BloodHound
Length of output: 1204
Fix hyphenation in "SKU-level configuration".
The phrase should use a hyphen when modifying a noun.
Suggested change
- Dogtags is a provider-based system for managing feature entitlements. It abstracts where SKU level configuration comes from, allowing the same application code to work whether entitlements are sourced from a license file, a database, or some future backend we haven't thought of yet.
+ Dogtags is a provider-based system for managing feature entitlements. It abstracts where SKU-level configuration comes from, allowing the same application code to work whether entitlements are sourced from a license file, a database, or some future backend we haven't thought of yet.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Dogtags is a provider-based system for managing feature entitlements. It abstracts where SKU level configuration comes from, allowing the same application code to work whether entitlements are sourced from a license file, a database, or some future backend we haven't thought of yet. | |
| Dogtags is a provider-based system for managing feature entitlements. It abstracts where SKU-level configuration comes from, allowing the same application code to work whether entitlements are sourced from a license file, a database, or some future backend we haven't thought of yet. |
🧰 Tools
🪛 LanguageTool
[grammar] ~14-~14: Use a hyphen to join words.
Context: ...ure entitlements. It abstracts where SKU level configuration comes from, allowing...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
In @rfc/bh-rfc-6.md at line 14, Update the sentence that currently reads "SKU
level configuration" to use the hyphenated form "SKU-level configuration" so the
compound modifier correctly hyphenates when modifying the noun; locate this
phrase in the Dogtags description paragraph (the line starting "Dogtags is a
provider-based system for managing feature entitlements...") and replace the
unhyphenated phrase with "SKU-level configuration".
Description
DogTags RFC, to be paired with #2147
Motivation and Context
BED-7094
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.