design: feature-gated write-mode - #678
Closed
cdoan1 wants to merge 5 commits into
Closed
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>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThis PR adds a design document for feature-gate-aware write-mode overrides, covering marker syntax, metadata changes, implementation phases, runtime validation, examples, compatibility, and rollout guidance. No code or exported entities change. ChangesFeature-gated write-mode design
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
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?
typeid
requested changes
Jul 14, 2026
Remove Slack/Jira references per reviewer request, remove redundant featureGate="" entries, add restrictive override example, and remove incorporated feedback/next-steps sections. Co-Authored-By: Claude Opus 4.6 <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
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/design/feature-gated-write-mode-design.md (2)
107-110: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject empty feature-gate names for override entries. The parser allows
featureGate="", butIsFeatureGateEnabledonly matches a real gate name, so that override never applies in normal requests.WriteModealready provides the fallback, so require a non-empty gate and drop the empty-string convention.🤖 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 107 - 110, Update FeatureGateWriteMode to require a non-empty FeatureGate for override entries, removing the empty-string-as-default convention from its documentation and validation/parsing behavior. Preserve WriteMode as the fallback for requests without a matching feature gate, and ensure IsFeatureGateEnabled only processes real gate names.
176-185: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPreserve override order in the registry data model.
FieldMeta.FeatureGateAwareWriteModesis ordered andvalidateWriteMode()uses the first matching gate, sotemplateField.GatedWriteModes map[string]stringwould drop ordering and duplicates and can change which override wins. Use an ordered slice or another ordered representation end-to-end.🤖 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 176 - 185, Replace templateField.GatedWriteModes with an ordered representation and propagate that representation through template generation and validation, preserving the original order and duplicate entries from FieldMeta.FeatureGateAwareWriteModes so validateWriteMode() continues using the first matching gate.
🤖 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`:
- Line 4: Remove the Jira link metadata near the document header and delete the
Jira entries in the References section around the referenced locations. Ensure
no Jira references remain in the design document while preserving all other
documentation content.
---
Outside diff comments:
In `@docs/design/feature-gated-write-mode-design.md`:
- Around line 107-110: Update FeatureGateWriteMode to require a non-empty
FeatureGate for override entries, removing the empty-string-as-default
convention from its documentation and validation/parsing behavior. Preserve
WriteMode as the fallback for requests without a matching feature gate, and
ensure IsFeatureGateEnabled only processes real gate names.
- Around line 176-185: Replace templateField.GatedWriteModes with an ordered
representation and propagate that representation through template generation and
validation, preserving the original order and duplicate entries from
FieldMeta.FeatureGateAwareWriteModes so validateWriteMode() continues using the
first matching gate.
🪄 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: 1583c08e-3ae5-453f-a6a4-7b49a8672c18
📒 Files selected for processing (1)
docs/design/feature-gated-write-mode-design.md
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Collaborator
Author
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
3 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