design: feature-gated write-mode#678
Open
cdoan1 wants to merge 3 commits into
Open
Conversation
Add design document for extending the marker system to support feature-set-specific write-mode overrides. This enables gradual rollout of customer control over sensitive fields by allowing different write modes (mutable/immutable/service-set) per feature set (Default/TechPreview/DevPreview). Addresses use cases like making etcd config service-set for production but mutable for TechPreview customers. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
📝 WalkthroughWalkthroughThis PR adds a design document for feature-gate-aware write-mode overrides, including marker syntax, ChangesDesign Proposal
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/design/feature-gated-write-mode-design.md (1)
1-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReframe this as an ADR.
docs/design/**is supposed to use ADR format, but this reads more like a proposal summary. Please add explicit Context / Decision / Consequences sections so the chosen syntax and rollout trade-offs are easy to review and maintain.As per path instructions,
docs/design/**: Review design decisions for ADR format compliance. Ensure decisions are well-justified and align with the regional architecture goals.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/design/feature-gated-write-mode-design.md` around lines 1 - 9, Reframe the document headed by Feature-Gated Write-Mode Control into ADR format instead of a proposal summary. Update the top-level structure in this design doc to include explicit Context, Decision, and Consequences sections, and move the current problem statement and review details into the appropriate ADR sections. Keep the description centered on the chosen syntax and rollout trade-offs so the decision is easy to review and maintain.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/design/feature-gated-write-mode-design.md`:
- Around line 110-119: The parser loop that builds gatedModes from
writeModeGatedPattern currently overwrites earlier entries when the same
featureSet appears more than once, which makes comment order affect the result.
Update the parsing logic around the gatedModes map population to detect
duplicate featureSet keys and return a parse error instead of assigning the
later WriteMode, then only assign meta.GatedWriteModes when the collected
entries are unique.
- Around line 174-201: The proposed validator test matrix only covers Default
and TechPreview, so it does not verify the documented base-mode fallback for an
unspecified feature set. Update TestValidator_GatedWriteMode in the
feature-gated write mode design tests to include a DevPreview or equivalent
non-overridden FeatureSet case for spec.etcd, and assert the expected write
behavior there. Keep the existing ServiceSet/Default and TechPreview coverage,
but add this fallback case so the validator logic cannot accidentally inherit
the wrong override.
---
Nitpick comments:
In `@docs/design/feature-gated-write-mode-design.md`:
- Around line 1-9: Reframe the document headed by Feature-Gated Write-Mode
Control into ADR format instead of a proposal summary. Update the top-level
structure in this design doc to include explicit Context, Decision, and
Consequences sections, and move the current problem statement and review details
into the appropriate ADR sections. Keep the description centered on the chosen
syntax and rollout trade-offs so the decision is easy to review and maintain.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: dd287bc9-fcda-48f9-b10c-256d2f737a0b
📒 Files selected for processing (1)
docs/design/feature-gated-write-mode-design.md
cdoan1
pushed a commit
to cdoan1/hyperfleet-api-codegen
that referenced
this pull request
Jul 9, 2026
Incorporated feedback from ROSAENG-61570 Jira comments and Slack discussion:
**Key Changes:**
1. **Marker Syntax** - Changed from bracket syntax to OpenShift multi-argument pattern:
- Before: `+hyperfleet:write-mode[FeatureSetName]=mutable`
- After: `+hyperfleet:validation:FeatureGateAwareWriteMode:featureGate="GateName",writeMode="mutable"`
- Follows existing patterns: FeatureGateAwareEnum, FeatureGateAwareXValidation
2. **Primary Use Case** - Clarified that write-mode control works independently of feature gating:
- GA fields (no FeatureGate marker) can use it for customer-tier control
- Example: Standard customers get immutable, Premium customers get mutable
- Not limited to fields behind feature gates
3. **Data Model** - Changed from map to slice:
- Before: `GatedWriteModes map[string]WriteMode`
- After: `FeatureGateAwareWriteModes []FeatureGateWriteMode`
- Preserves marker order, supports empty-string default
4. **Validation Logic** - Updated to check customer's enabled feature gates:
- Request needs `EnabledFeatureGates []string` field
- Validation checks if specific gate is enabled for customer
- Priority: specific gate match > default ("") > base WriteMode
**New Examples:**
- GA field with customer-tier control (no FeatureGate required)
- Progressive write-mode rollout for gated fields
- Multiple gate-based overrides
**Feedback Source:**
- Jira: https://redhat.atlassian.net/browse/ROSAENG-61570
- Slack: https://redhat-internal.slack.com/archives/C0A819QK7M5/p1783585582976459
- PR reference: openshift-online/rosa-hyperfleet#678
**Open Questions:**
- Marker namespace choice
- Multiple-gate priority behavior
- Customer gate enablement mechanism
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Major revisions based on Slack/Jira feedback: - Change marker syntax from bracket notation to follow OpenShift conventions: +hyperfleet:validation:FeatureGateAwareWriteMode - Clarify primary use case: GA fields with customer-tier control, not just gated fields - Update data model to use list of FeatureGateWriteMode instead of map - Add validation logic for checking customer's enabled feature gates - Include detailed feedback section documenting design evolution - Add more comprehensive examples covering GA and gated fields References OpenShift patterns: FeatureGateAwareEnum and FeatureGateAwareXValidation for multi-argument marker syntax. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/design/feature-gated-write-mode-design.md (1)
183-189: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep the generated overrides ordered end-to-end.
map[string]stringdrops declaration order and conflicts with the earlier list-basedFeatureGateAwareWriteModesmodel. That makes precedence depend on serialization rather than the marker order the spec promises.Suggested shape
- GatedWriteModes map[string]string // NEW + GatedWriteModes []FeatureGateWriteMode🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/design/feature-gated-write-mode-design.md` around lines 183 - 189, The new GatedWriteModes field on templateField should preserve marker order instead of using a map, because map[string]string loses declaration order and breaks the intended precedence from FeatureGateAwareWriteModes. Update the templateField shape and any related generation logic to use an ordered list or slice-based structure, and make sure the code that builds and consumes GatedWriteModes in the feature-gated write mode flow keeps overrides in the same end-to-end order as the source markers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/design/feature-gated-write-mode-design.md`:
- Around line 401-403: Define a deterministic precedence rule for multiple
enabled gates in the feature-gated write-mode design, instead of selecting the
first specific match in source order. Update the multi-gate behavior guidance in
the design doc to state an explicit priority based on write-mode specificity or
permissiveness, and reference the write-mode selection logic consistently so
reordering annotations cannot change the outcome.
- Around line 414-417: The customer gate enablement section currently presents
an explicit Platform API request parameter as a viable option, but gate
membership must be derived server-side from trusted auth or subscription state.
Update the design in the “Customer gate enablement” discussion to remove or
clearly reject Option C, and emphasize using the request auth/authorization
context or subscription lookup as the source of truth in the relevant design
section.
---
Outside diff comments:
In `@docs/design/feature-gated-write-mode-design.md`:
- Around line 183-189: The new GatedWriteModes field on templateField should
preserve marker order instead of using a map, because map[string]string loses
declaration order and breaks the intended precedence from
FeatureGateAwareWriteModes. Update the templateField shape and any related
generation logic to use an ordered list or slice-based structure, and make sure
the code that builds and consumes GatedWriteModes in the feature-gated write
mode flow keeps overrides in the same end-to-end order as the source markers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2a82349b-ef75-4318-ad35-1fcbf80d027e
📒 Files selected for processing (1)
docs/design/feature-gated-write-mode-design.md
Comment on lines
+401
to
+403
| 2. **Multiple gate behavior**: If a customer has multiple gates enabled, which write-mode wins? | ||
| - **Recommendation**: First specific match in marker order (earliest in source code) | ||
| - **Alternative**: Most permissive (mutable > immutable > service-set) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Pin down multi-gate precedence.
“First specific match in source order” makes the effective write-mode change when annotations are reordered or reformatted. Please define a deterministic precedence rule instead of relying on comment order.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/feature-gated-write-mode-design.md` around lines 401 - 403,
Define a deterministic precedence rule for multiple enabled gates in the
feature-gated write-mode design, instead of selecting the first specific match
in source order. Update the multi-gate behavior guidance in the design doc to
state an explicit priority based on write-mode specificity or permissiveness,
and reference the write-mode selection logic consistently so reordering
annotations cannot change the outcome.
Comment on lines
+414
to
+417
| 6. **Customer gate enablement**: How do we determine which gates are enabled for a customer at runtime? | ||
| - **Option A**: Lookup from database based on subscription tier | ||
| - **Option B**: Include in request authentication/authorization context | ||
| - **Option C**: Explicit parameter in Platform API request |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '390,440p' docs/design/feature-gated-write-mode-design.mdRepository: openshift-online/rosa-hyperfleet
Length of output: 3096
🏁 Script executed:
sed -n '404,430p' docs/design/feature-gated-write-mode-design.mdRepository: openshift-online/rosa-hyperfleet
Length of output: 1846
🏁 Script executed:
rg -n "Option C|gate membership|subscription tier|authentication/authorization context|request parameter" docs/design/feature-gated-write-mode-design.mdRepository: openshift-online/rosa-hyperfleet
Length of output: 524
🏁 Script executed:
sed -n '368,420p' docs/design/feature-gated-write-mode-design.mdRepository: openshift-online/rosa-hyperfleet
Length of output: 2622
Keep gate membership server-side
Option C should not be used as-is: if enabled gates come from a request parameter, callers can self-assert entitlement and bypass write-mode checks. Derive gate membership from trusted auth/subscription state instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/feature-gated-write-mode-design.md` around lines 414 - 417, The
customer gate enablement section currently presents an explicit Platform API
request parameter as a viable option, but gate membership must be derived
server-side from trusted auth or subscription state. Update the design in the
“Customer gate enablement” discussion to remove or clearly reject Option C, and
emphasize using the request auth/authorization context or subscription lookup as
the source of truth in the relevant design section.
lucasponce
reviewed
Jul 10, 2026
| // Override: mutable when MyPremiumFeature gate is enabled | ||
| // +hyperfleet:write-mode=immutable | ||
| // +hyperfleet:validation:FeatureGateAwareWriteMode:featureGate="",writeMode="immutable" | ||
| // +hyperfleet:validation:FeatureGateAwareWriteMode:featureGate="MyPremiumFeature",writeMode="mutable" |
There was a problem hiding this comment.
featureGate is a tag/label/flag that is linked with the AWS account level, right?
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
doc design for feature-gated write-mode from just visibility
Summary by CodeRabbit