ROSAENG-62089: doc: Add feature-gated write-mode design doc and update api-management - #154
Open
cdoan1 wants to merge 1 commit into
Open
ROSAENG-62089: doc: Add feature-gated write-mode design doc and update api-management#154cdoan1 wants to merge 1 commit into
cdoan1 wants to merge 1 commit into
Conversation
Move the feature-gated write-mode design from rosa-hyperfleet PR #678 to this repo alongside the api-management spec it extends. Update api-management.md to reflect gate-aware write-mode overrides in the FieldMeta data model, validation loop, and marker documentation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cdoan1 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
Contributor
WalkthroughThe API management documentation now defines feature-gate-aware write-mode overrides. A companion design proposal specifies marker syntax, field metadata, runtime effective-mode resolution, examples, migration guidance, trade-offs, and open questions. ChangesFeature-Gated Write-Mode Control
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 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: 7
🤖 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/api/feature-gated-write-mode-design.md`:
- Line 27: Update the “Example 1 — Customer-tier based control” heading to
hyphenate the compound adjective as “customer-tier-based,” preserving the rest
of the heading.
- Line 380: Remove the internal generation prompt comment from the published
design document, leaving the surrounding design content unchanged.
- Around line 153-165: Update featureGateAwareWriteModePattern and the
extraction logic in MarkerScanner.extractMarkers to handle empty featureGate
values consistently with the data model: either reject empty gate names during
marker parsing or explicitly treat them as the default gate when matching at
runtime. Ensure the chosen behavior prevents an empty-gate override from being
silently treated as a normal feature gate.
- Around line 67-73: Resolve the HyperFleetEtcdConfig stage contradiction by
choosing a TechPreview gate or revising the documented Default-customer
behavior, then apply that choice consistently in
docs/api/feature-gated-write-mode-design.md lines 67-73, 240-245, and 260-269:
update the EtcdSpec gate annotation, test matrix, and progressive-rollout
example accordingly.
- Around line 100-114: Standardize the gate-aware write-mode metadata contract
by selecting one canonical FieldMeta field name and representation at
docs/api/feature-gated-write-mode-design.md:100-114, then update every
corresponding JSON example at
docs/api/feature-gated-write-mode-design.md:121-140 and registry/runtime example
at docs/api/api-management.md:217-224 to use it consistently.
- Around line 218-234: Update the validation Request and IsFeatureGateEnabled
flow so enabled feature gates are derived from the customer’s authorized feature
set and entitlement lookup rather than a client-supplied EnabledFeatureGates
field. Keep any request-level snapshot internal, populate it only after
authorization succeeds, and ensure gate checks use that authorized snapshot.
- Around line 180-187: The templateField.GatedWriteModes field must preserve
override order for first-match generation and consistent serialization. Replace
the map type with the ordered slice representation used elsewhere in the design,
and update any related generation or serialization code to consume that ordered
form without changing existing semantics.
🪄 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: Repository: openshift-online/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7b18051a-6c1c-4825-b816-a5bf351435b4
📒 Files selected for processing (2)
docs/api/api-management.mddocs/api/feature-gated-write-mode-design.md
|
|
||
| **Scenario**: A field is **GA** (no `+openshift:enable:FeatureGate` marker) but we want to give specific customers the ability to mutate a field that is otherwise immutable or service-set. | ||
|
|
||
| **Example 1 — Customer-tier based control**: |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Hyphenate “customer-tier-based.”
This user-facing heading should use the compound adjective form.
🧰 Tools
🪛 LanguageTool
[grammar] ~27-~27: Use a hyphen to join words.
Context: ...ervice-set. Example 1 — Customer-tier based control: - **Standard customers...
(QB_NEW_EN_HYPHEN)
🤖 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/api/feature-gated-write-mode-design.md` at line 27, Update the “Example
1 — Customer-tier based control” heading to hyphenate the compound adjective as
“customer-tier-based,” preserving the rest of the heading.
Source: Linters/SAST tools
Comment on lines
+67
to
+73
| // Gated field with different write-modes per feature set | ||
| // Default: service-set (platform-managed) | ||
| // TechPreview+: mutable (customer-controlled) | ||
| // +hyperfleet:write-mode=service-set | ||
| // +hyperfleet:validation:FeatureGateAwareWriteMode:featureGate="HyperFleetEtcdConfig",writeMode="mutable" | ||
| // +openshift:enable:FeatureGate=HyperFleetEtcdConfig | ||
| Etcd *EtcdSpec `json:"etcd,omitempty"` |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Resolve the HyperFleetEtcdConfig stage contradiction.
The examples and tests require this gate to distinguish Default from TechPreview customers, but docs/api/api-management.md defines it as GA, which makes it available to Default customers.
docs/api/feature-gated-write-mode-design.md#L67-L73: use a TechPreview gate or revise the stated Default behavior.docs/api/feature-gated-write-mode-design.md#L240-L245: align the test matrix with the selected gate stage.docs/api/feature-gated-write-mode-design.md#L260-L269: align the progressive-rollout example with the same stage.
📍 Affects 1 file
docs/api/feature-gated-write-mode-design.md#L67-L73(this comment)docs/api/feature-gated-write-mode-design.md#L240-L245docs/api/feature-gated-write-mode-design.md#L260-L269
🤖 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/api/feature-gated-write-mode-design.md` around lines 67 - 73, Resolve
the HyperFleetEtcdConfig stage contradiction by choosing a TechPreview gate or
revising the documented Default-customer behavior, then apply that choice
consistently in docs/api/feature-gated-write-mode-design.md lines 67-73,
240-245, and 260-269: update the EtcdSpec gate annotation, test matrix, and
progressive-rollout example accordingly.
Comment on lines
+100
to
+114
| **Proposed FieldMeta**: | ||
|
|
||
| ```go | ||
| type FieldMeta struct { | ||
| FieldPath string | ||
| WriteMode WriteMode // Base mode (fallback) | ||
| FeatureGate string // Gate required for visibility | ||
| Hidden bool | ||
| FeatureGateAwareWriteModes []FeatureGateWriteMode `json:"featureGateAwareWriteModes,omitempty"` | ||
| } | ||
|
|
||
| type FeatureGateWriteMode struct { | ||
| FeatureGate string // Gate name; empty string = default (no gates enabled) | ||
| WriteMode WriteMode | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Standardize the gate-aware write-mode metadata contract.
The documents use different field names for the same registry data, which can produce incompatible generated metadata and runtime consumers.
docs/api/feature-gated-write-mode-design.md#L100-L114: choose the canonicalFieldMetafield name and representation.docs/api/feature-gated-write-mode-design.md#L121-L140: update JSON examples to use that canonical name.docs/api/api-management.md#L217-L224: update the registry/runtime example to the same name.
📍 Affects 2 files
docs/api/feature-gated-write-mode-design.md#L100-L114(this comment)docs/api/feature-gated-write-mode-design.md#L121-L140docs/api/api-management.md#L217-L224
🤖 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/api/feature-gated-write-mode-design.md` around lines 100 - 114,
Standardize the gate-aware write-mode metadata contract by selecting one
canonical FieldMeta field name and representation at
docs/api/feature-gated-write-mode-design.md:100-114, then update every
corresponding JSON example at
docs/api/feature-gated-write-mode-design.md:121-140 and registry/runtime example
at docs/api/api-management.md:217-224 to use it consistently.
Comment on lines
+153
to
+165
| var featureGateAwareWriteModePattern = regexp.MustCompile( | ||
| `\+hyperfleet:validation:FeatureGateAwareWriteMode:featureGate="([^"]*)",writeMode="(mutable|immutable|service-set)"`, | ||
| ) | ||
|
|
||
| func (s *MarkerScanner) extractMarkers(field *ast.Field, fieldPath string) *FieldMeta { | ||
| // ... existing code ... | ||
|
|
||
| var gatedModes []FeatureGateWriteMode | ||
| for _, match := range featureGateAwareWriteModePattern.FindAllStringSubmatch(comments, -1) { | ||
| gatedModes = append(gatedModes, FeatureGateWriteMode{ | ||
| FeatureGate: match[1], | ||
| WriteMode: WriteMode(match[2]), | ||
| }) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Define or reject empty featureGate overrides.
The regex accepts featureGate="", while the data model says an empty gate means “default.” The runtime algorithm only checks whether a gate is enabled, so an empty-gate override will not reliably match. Either reject empty gate names or implement explicit fallback semantics.
🤖 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/api/feature-gated-write-mode-design.md` around lines 153 - 165, Update
featureGateAwareWriteModePattern and the extraction logic in
MarkerScanner.extractMarkers to handle empty featureGate values consistently
with the data model: either reject empty gate names during marker parsing or
explicitly treat them as the default gate when matching at runtime. Ensure the
chosen behavior prevents an empty-gate override from being silently treated as a
normal feature gate.
Comment on lines
+180
to
+187
| ```go | ||
| type templateField struct { | ||
| FieldPath string | ||
| WriteMode string | ||
| FeatureGate string | ||
| Hidden bool | ||
| GatedWriteModes map[string]string | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
rg -n 'GatedWriteModes|FeatureGateAwareWriteModes|map\[string\]string' docs pkgRepository: openshift-online/rosa-hyperfleet-api
Length of output: 10590
Keep the overrides ordered. GatedWriteModes map[string]string breaks the first-match semantics used elsewhere in this design because map iteration is unordered; use an ordered slice here too so generation and serialization stay consistent.
🤖 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/api/feature-gated-write-mode-design.md` around lines 180 - 187, The
templateField.GatedWriteModes field must preserve override order for first-match
generation and consistent serialization. Replace the map type with the ordered
slice representation used elsewhere in the design, and update any related
generation or serialization code to consume that ordered form without changing
existing semantics.
Comment on lines
+218
to
+234
| The validation Request needs a new method: | ||
|
|
||
| ```go | ||
| type Request struct { | ||
| // ... existing fields ... | ||
| EnabledFeatureGates []string | ||
| } | ||
|
|
||
| func (r *Request) IsFeatureGateEnabled(gateName string) bool { | ||
| for _, gate := range r.EnabledFeatureGates { | ||
| if gate == gateName { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } | ||
| ``` |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== feature-gated-write-mode-design.md (relevant ranges) ==\n'
wc -l docs/api/feature-gated-write-mode-design.md
sed -n '200,250p' docs/api/feature-gated-write-mode-design.md
printf '\n---\n'
sed -n '350,385p' docs/api/feature-gated-write-mode-design.md
printf '\n== api-management.md (relevant search) ==\n'
rg -n "feature gate|entitlement|authorization context|customer’s feature set|feature set|gates" docs/api/api-management.md
printf '\n---\n'
sed -n '1,260p' docs/api/api-management.mdRepository: openshift-online/rosa-hyperfleet-api
Length of output: 18173
Derive feature gates from authorization context, not request data.
EnabledFeatureGates should come from the customer’s feature set and entitlement lookup, not from a client-supplied request field. Keep any request snapshot internal and populate it only after authorization.
🤖 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/api/feature-gated-write-mode-design.md` around lines 218 - 234, Update
the validation Request and IsFeatureGateEnabled flow so enabled feature gates
are derived from the customer’s authorized feature set and entitlement lookup
rather than a client-supplied EnabledFeatureGates field. Keep any request-level
snapshot internal, populate it only after authorization succeeds, and ensure
gate checks use that authorized snapshot.
|
|
||
| --- | ||
|
|
||
| <!-- Generation prompt: review rosa-hyperfleet#678, adapt for rosa-hyperfleet-api docs/api/, cross-reference api-management.md, apply style rules (tables for structured field lists, ASCII art for digraphs, bullet points in tables, relative paths from docs/). --> |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the generation prompt from the published design document.
This is an internal review artifact rather than product documentation.
🤖 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/api/feature-gated-write-mode-design.md` at line 380, Remove the internal
generation prompt comment from the published design document, leaving the
surrounding design content unchanged.
13 tasks
cdoan1
changed the title
openshift-ci-robot
added
the
jira/valid-reference
Collaborator
|
@cdoan1: This pull request references ROSAENG-62089 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
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.
Move the feature-gated write-mode design from rosa-hyperfleet PR #678 to this repo alongside the api-management spec it extends. Update api-management.md to reflect gate-aware write-mode overrides in the FieldMeta data model, validation loop, and marker documentation.
Description
Type of Change
Testing
make test)Checklist
Summary by CodeRabbit