Skip to content

fix(v2): validate segment references when creating flags#4749

Merged
kodiakhq[bot] merged 1 commit into
v2from
fix/v2-segment-validation
Sep 17, 2025
Merged

fix(v2): validate segment references when creating flags#4749
kodiakhq[bot] merged 1 commit into
v2from
fix/v2-segment-validation

Conversation

@markphelps

Copy link
Copy Markdown
Collaborator

Summary

This PR fixes issue #4748 where creating a flag resource with non-existent segment references via the v2 API would result in an inconsistent state. The flag would be created successfully but would cause snapshot creation errors and UI issues.

Changes

  • Modified internal/storage/environments/fs/flipt/flags.go: Added validation in PutResource method to check that all referenced segments exist before saving a flag
  • Added tests in internal/storage/environments/fs/flipt/flags_test.go: Comprehensive test coverage for the new validation logic including:
    • Test for rejecting flags with non-existent segments in rules
    • Test for rejecting flags with non-existent segments in rollouts
    • Test for accepting flags with valid existing segments

Problem Details

Previously, when creating a flag like this with a non-existent segment "buzz":

{
    "key": "flag1",
    "rollouts": [{
        "type": "SEGMENT_ROLLOUT_TYPE",
        "segment": {
            "segments": ["buzz"]
        }
    }]
}

The flag would be created but would cause:

  • Snapshot creation errors: flag default/flag1 rule 1 references unknown segment "buzz"
  • Evaluation failures
  • UI display issues

Solution

The fix validates all segment references during flag creation/update:

  1. Collects all existing segments in the namespace
  2. Checks all segments referenced in rules
  3. Checks all segments referenced in rollouts
  4. Returns an error if any segment doesn't exist

This ensures data consistency and prevents the creation of invalid flag configurations.

Fixes #4748

When creating a flag resource via the v2 API with non-existent segment
references, the flag was created but would lead to snapshot creation
errors and inconsistent state.

This fix adds validation during flag creation to ensure all referenced
segments exist in both rules and rollouts before persisting the flag.

- Added segment existence validation in PutResource for flags
- Check segments referenced in both rules and rollouts
- Added comprehensive test coverage for the validation logic
- Tests verify rejection of invalid segments and acceptance of valid ones

Fixes #4748

Signed-off-by: Mark Phelps <[email protected]>
@markphelps
markphelps requested a review from a team as a code owner September 16, 2025 18:28
@dosubot dosubot Bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Sep 16, 2025
@dosubot

dosubot Bot commented Sep 16, 2025

Copy link
Copy Markdown

Related Documentation

Checked 3 published document(s). No updates required.

You have 3 draft document(s). Publish docs to keep them always up-to-date

How did I do? Any feedback?  Join Discord

@markphelps
markphelps requested a review from erka September 16, 2025 18:32
@codecov

codecov Bot commented Sep 16, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.67%. Comparing base (7eb5367) to head (b3b7b5a).
⚠️ Report is 1 commits behind head on v2.

Additional details and impacted files
@@            Coverage Diff             @@
##               v2    #4749      +/-   ##
==========================================
+ Coverage   62.44%   62.67%   +0.22%     
==========================================
  Files         132      132              
  Lines       15576    15594      +18     
==========================================
+ Hits         9727     9773      +46     
+ Misses       5165     5139      -26     
+ Partials      684      682       -2     
Flag Coverage Δ
integrationtests 36.51% <33.33%> (-0.01%) ⬇️
unittests 52.89% <100.00%> (+0.31%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@erka erka left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

@erka erka added the v2 Flipt v2 label Sep 17, 2025
@erka erka added the automerge Used by Kodiak bot to automerge PRs label Sep 17, 2025
@kodiakhq
kodiakhq Bot merged commit 96c9e8b into v2 Sep 17, 2025
44 of 45 checks passed
@kodiakhq
kodiakhq Bot deleted the fix/v2-segment-validation branch September 17, 2025 11:42
@github-project-automation github-project-automation Bot moved this to Done in Flipt V2 Sep 17, 2025
@dosubot

dosubot Bot commented Sep 17, 2025

Copy link
Copy Markdown

Documentation Updates

Checked 3 published document(s). No updates required.

You have 3 draft document(s). Publish docs to keep them always up-to-date

How did I do? Any feedback?  Join Discord

ptejasvini pushed a commit to ptejasvini/flipt that referenced this pull request Oct 3, 2025
When creating a flag resource via the v2 API with non-existent segment
references, the flag was created but would lead to snapshot creation
errors and inconsistent state.

This fix adds validation during flag creation to ensure all referenced
segments exist in both rules and rollouts before persisting the flag.

- Added segment existence validation in PutResource for flags
- Check segments referenced in both rules and rollouts
- Added comprehensive test coverage for the validation logic
- Tests verify rejection of invalid segments and acceptance of valid ones

Fixes flipt-io#4748

Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: tejasvini1 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge Used by Kodiak bot to automerge PRs size:L This PR changes 100-499 lines, ignoring generated files. v2 Flipt v2

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Bug (v2): Creating a Flag Resource with an invalid segment leads to inconsistencies

2 participants