feat: Add feature flag support to option schema#91
Conversation
First pass with claude.
Add feature flag support to option schema. The option schema that applications generate will reference a definition that is spliced in when the schema file is processed by sentry-options. This is the first step to having feature flags stored as structured data in the options file. Part of the alternative approach to #84
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
thanks claude...
The namespace schema can include non-feature values.
sentry-options-validation/src/lib.rs
Outdated
| "features.organizations:fury-mode": { | ||
| "$ref": "#/definitions/Feature" | ||
| } |
There was a problem hiding this comment.
When we compile options schema for an application, each feature flag will be included with a $ref. This lets us validate feature flag names, and the flag values.
kenzoengineer
left a comment
kenzoengineer
left a comment
There was a problem hiding this comment.
lgtm, small call-out: old flagpole uses feature.* but this system expected features.*
I'll get those prefixes aligned. |
Having the prefixes aligned makes working with existing data much simpler.
| "properties": { | ||
| "$ref": { | ||
| "type": "string", | ||
| "pattern": "#/definitions/Feature" |
There was a problem hiding this comment.
Unanchored $ref pattern allows invalid reference values
Low Severity
The pattern for the $ref property is "#/definitions/Feature" without ^ and $ anchors. JSON Schema's pattern keyword does substring matching, so values like "#/definitions/FeatureXYZ" or "foo#/definitions/Feature" would pass meta-schema validation but fail at runtime when the $ref can't be resolved. The version pattern on line 6 correctly uses "^[0-9]+\\.[0-9]+$" — the same anchoring is needed here.
sentry-options-validation/src/lib.rs
Outdated
| } | ||
| } | ||
|
|
||
| // Inject additionalProperties: false to reject unknown options |
There was a problem hiding this comment.
We had to do this too, not the most clean but necessary for being more strict
There was a problem hiding this comment.
I see we have another copy of this logic higher up in the function after I merged main in. I'll remove the redundant block.
| ] | ||
| } | ||
| }, | ||
| "additionalProperties": false |
There was a problem hiding this comment.
will removal of this be a problem? i don't think it will but just double checking
There was a problem hiding this comment.
It should be ok, as all properties should either be feature.* or !feature.*. I'll add it back just to be safe.
|
linking this PR to https://linear.app/getsentry/issue/DI-1686/flagpole-support |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
While all properties should be `feature` or ! `feature` this doesn't cause test failures.
I don't think we get any value from having feature names in both the feature config and the option key. Instead we could just use the option name as the feature name.
3 participants
Add feature flag support to option schema. The option schema that applications generate will reference a definition that is spliced in when the schema file is processed by sentry-options.
This is the first step to having feature flags stored as structured data in the options file.
Part of the alternative approach to #84