feat(ext): serialize isoneof constraint values as YAML lists#6072
Conversation
Signed-off-by: Jon Gaul <[email protected]> # Conflicts: # internal/storage/fs/snapshot_test.go
dosubot
Bot
added
the
size:M
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v2 #6072 +/- ##
==========================================
+ Coverage 61.45% 61.65% +0.20%
==========================================
Files 142 142
Lines 14323 14430 +107
==========================================
+ Hits 8802 8897 +95
- Misses 4783 4790 +7
- Partials 738 743 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: approve
This PR cleanly implements native YAML/JSON list serialization for isoneof/isnotoneof constraint values, bumps the format version to 1.6, and keeps backward compatibility for old stringified-JSON files. Both the CUE and JSON schemas are updated consistently, and the storage layer correctly bumps document versions on rewrite so validation stays in sync. The test suite covers marshaling, unmarshaling, round-trips, old-format normalization, bare YAML special values, and evaluation parity across 1.5 and 1.6 files. The two review points raised in the prior discussion (empty-value validation and JSON schema consistency) are both addressed in the current diff by the custom MarshalJSON/UnmarshalJSON pair and the unconditional list return for isoneof operators in MarshalYAML. No critical, major, or minor issues remain.
🤖 Automated review by the Flipt PR review agent.
- Add v1.6 as a valid document version (CUE + JSON Schema) - Split isoneof/isnotoneof constraint values: string for <1.6, array for 1.6+ - Align JSON Schema with CUE via version-aware root-level allOf - Set namespace documents to latest version on write - Test to verify v1.5 and v1.6 fixtures Signed-off-by: Roman Dmytrenko <[email protected]>
dosubot
Bot
added
size:L
| func (c Constraint) MarshalYAML() (any, error) { | ||
| if c.Operator == "isoneof" || c.Operator == "isnotoneof" { | ||
| var list []any | ||
| if c.Value != "" && json.Unmarshal([]byte(c.Value), &list) == nil && len(list) > 0 { |
There was a problem hiding this comment.
Empty / [] isoneof in a 1.6 doc will fail validation.
When the value is "", "[]", or otherwise not a non-empty JSON array, this falls through to the alias(c) path and serializes value as a string (or omits it via omitempty). But the new 1.6 CUE schema requires value: [...string] (a list, present — not optional), so writing such a constraint to a 1.6 file will fail validation in documentEncoder.Close(), where the string form previously saved fine.
This is reachable if the UI lets a user clear all selections on an isoneof/isnotoneof constraint. Suggest coercing the fallback to an empty list [] for isoneof operators (or making value optional for isoneof in the 1.6 CUE branch), and adding an empty-value case to TestSegmentStorage_PutResource_IsOneOf.
| Description string `yaml:"description,omitempty"` | ||
| } | ||
|
|
||
| func (c Constraint) MarshalYAML() (any, error) { |
There was a problem hiding this comment.
JSON serialization is now inconsistent with the published schema.
This adds MarshalYAML/UnmarshalYAML but no MarshalJSON/UnmarshalJSON, so the JSON format keeps the stringified value. Meanwhile flipt.json now requires arrays for 1.6 isoneof (constraintValueV16) and constraint.value permits arrays. A hand-written 1.6 JSON document that follows the published schema (array value) can't be unmarshaled into Constraint.Value string, and JSON export still emits the string form — contradicting the schema.
If JSON feature files are a supported path in v2, this needs matching JSON (un)marshaling; if at-rest is effectively YAML-only, please note the intent so the schema/behavior drift is deliberate.
…arshal Signed-off-by: Roman Dmytrenko <[email protected]>
|
@erka @markphelps Thank you both for the help polishing this up and thank you for the review and feedback!! |
3 participants
Description:
Editing
isoneof/isnotoneofvalues produces noisy diffs in the resulting PRs. The list of values is stored as a stringified JSON list rather than a native YAML list. This PR adds a customMarshalYAML/UnmarshalYAMLonConstraint, so nowgets stored as
sorted lexicographically. As a result, git diffs are minimal and exactly indicate which elements are added or removed.
Backwards Compatibility
Old JSON-string formatted files are read just fine and don't require updates. The next time the files are edited through the UI, they'll be reformatted into yaml lists.
YAML special values
I don't want to cause Norway problems, and in yaml.v3 I don't think I will.
UnmarshalYAMLcoercesboolandnilfrom hand-written files just in case andTestConstraint_UnmarshalYAML_BareYAMLSpecialValuesconfirms this behavior.