feat: Normalize selector handling across sync, evaluation, and OFREP services

[Copilot]

Problem

Currently, flagd services have inconsistent handling of selectors:

• Sync Service: Uses selector field from the request body
• Evaluation Service (v1 & v2): Uses Flagd-Selector header
• OFREP Service: Uses Flagd-Selector header

This inconsistency makes the API surface less predictable and can lead to confusion for users.

Solution

This PR normalizes selector handling by updating the sync service to accept the Flagd-Selector header as the primary method, aligning it with the evaluation and OFREP services. The implementation maintains full backward compatibility by falling back to the request body selector when the header is not present.

Changes

Core Implementation

Modified flagd/pkg/service/flag-sync/handler.go:

• Added getSelectorExpression() helper method that checks for Flagd-Selector header (via gRPC metadata) first
• Falls back to request body selector field if header is not present
• Logs a deprecation warning when the request body selector is used
• Applied the new logic to both SyncFlags() and FetchAllFlags() methods

Test Coverage

Added 5 comprehensive test cases in handler_test.go:

• Header-based selector (verifies no deprecation warning)
• Request body selector (verifies backward compatibility and deprecation warning)
• Header precedence (verifies header takes priority when both are provided)
• FetchAllFlags with header selector
• FetchAllFlags with request body selector

All existing tests continue to pass.

Migration Path

The change is fully backward compatible. Users can migrate at their convenience:

Current (deprecated):

req := &syncv1.FetchAllFlagsRequest{
    Selector: "source:my-source",
}
resp, err := client.FetchAllFlags(ctx, req)

Recommended:

md := metadata.New(map[string]string{
    "Flagd-Selector": "source:my-source",
})
ctx := metadata.NewOutgoingContext(context.Background(), md)
req := &syncv1.FetchAllFlagsRequest{}
resp, err := client.FetchAllFlags(ctx, req)

Benefits

• Consistency: All flagd services now use the same Flagd-Selector header pattern
• Backward Compatibility: Existing clients continue to work without modification
• Clear Migration Path: Deprecation warnings guide users to adopt the new approach
• Aligns with Standards: Using headers/metadata for query/filter parameters is a common gRPC pattern

Breaking Changes

None. The request body selector will be removed in a future major version (e.g., v2.0.0).

Related

Addresses #[issue_number]

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[FEATURE] Normalize selector handling across sync, evaluation, and OFREP services</issue_title>
<issue_description>### Requirements

Problem

Currently, we have inconsistent handling of selectors across different flagd services:

• Sync Service (flagd/pkg/service/flag-sync/handler.go:34): Uses req.GetSelector() from the request body
• Evaluation Service: Uses flagd-selector header
• OFREP Service: Uses flagd-selector header

This inconsistency can lead to confusion for users and makes the API surface less predictable.

Proposal

Normalize selector handling across all services by:

1. Preferred approach: Use the flagd-selector header as the primary method for all services
2. Backward compatibility: For the sync service, continue to accept selector in the request body as a fallback for a deprecation period

Implementation Strategy

1. Update sync service to check for flagd-selector header first
2. Fall back to request body selector field if header is not present
3. Log deprecation warning when using request body selector
4. Document the change and migration path
5. Remove request body selector support in a future major version (e.g., v2.0.0)

Benefits

• Consistent API across all flagd services
• Easier to understand and use for developers
• Aligns with common patterns for query/filter parameters in gRPC metadata

Breaking Changes

None initially (backward compatible), but request body selector would be removed in a future major release.

Additional Context

The sync service currently processes the selector at:

selectorExpression := req.GetSelector()
selector := store.NewSelector(selectorExpression)

This should be updated to prioritize metadata/header values while maintaining backward compatibility.</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes #1814

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[netlify]

✅ Deploy Preview for polite-licorice-3db33c canceled.

Name	Link
🔨 Latest commit	fe63d65
🔍 Latest deploy log	https://app.netlify.com/projects/polite-licorice-3db33c/deploys/68fb0c796343dd00088dab31

[aepfli]

/Gemini review

[gemini-code-assist]

Code Review

This pull request normalizes selector handling in the sync service to align with other services, prioritizing the Flagd-Selector header while maintaining backward compatibility with the request body selector. The changes are well-implemented and include comprehensive tests for all scenarios. My review includes a few suggestions to refactor the new logic and test code for improved maintainability and to reduce duplication.

[gemini-code-assist]

This logic to verify that no deprecation warning was logged is duplicated across TestSyncHandler_SelectorHeaderTakesPrecedence and TestSyncHandler_FetchAllFlags_SelectorFromHeader. Consider extracting it into a shared helper function to improve code reuse and maintainability.

For example:

func assertNoDeprecationWarning(t *testing.T, logs *observer.ObservedLogs) {
	t.Helper()
	for _, entry := range logs.All() {
		assert.NotContains(t, entry.Message, "deprecated", "Should not log deprecation warning")
	}
}

You could then call assertNoDeprecationWarning(t, observedLogs) in all relevant tests.

[gemini-code-assist]

This logic to verify the deprecation warning is duplicated in TestSyncHandler_FetchAllFlags_SelectorFromRequestBody. To improve maintainability and reduce code duplication, consider extracting this block into a shared helper function.

For example:

func assertDeprecationWarning(t *testing.T, logs *observer.ObservedLogs) {
	t.Helper()
	
	entries := logs.All()
	require.NotEmpty(t, entries, "Expected at least one log entry")

	found := false
	for _, entry := range entries {
		if entry.Level == zapcore.WarnLevel {
			assert.Contains(t, entry.Message, "deprecated")
			assert.Contains(t, entry.Message, "Flagd-Selector")
			found = true
			break
		}
	}
	assert.True(t, found, "Expected to find deprecation warning in logs")
}

You could then call assertDeprecationWarning(t, observedLogs) here and in the other test.