FCP-1434: Explicit wildcard support for snapshot cache #29

mohit-pal-kohli · 2025-11-18T22:25:03Z

Part 1 of implementing explicit wildcard support for xds-control-plane.

Context

We leverage the Snapshot Cache in go-control-plane, where delta for a node ID snapshot is proliferated to Envoys with that node ID when there is a change. This is acceptable in the current state where Fabric Proxy declares the destinations (clusters) it needs at start time in YAML.

The introduction of ODCDS makes the current state unpalatable, even though it is functional. The issue is that we do not want a mass proliferation of new clusters to all Envoys with node ID X when one pod with that node ID X requests a resource. Not solving this could create significant load on our control plane, especially for Fabric Proxy users with hundreds or thousands of destinations.

To solve this, we should add support for explicit wildcard support for control plane. By default, a snapshot resource will be considered wildcard and proliferated to all Envoys with the relevant node ID. By contrast, we will allow certain resources to not be wildcard, allowing ODCDS clusters to only be proliferated on demand.

Implementation details

This PR aims to make changes in a way that are fully backward compatible for Delta, SoTW, and legacyWildcardEnabled.

Exported functions

NewSnapshotWithExplicitWildcard: creates a Snapshot where each resource explicitly specifies whether it should be returned as part of wildcard requests.

This allows the snapshot to contain wildcard metadata for every resource. E.g.

cache.NewSnapshotWithExplicitWildcard("v1", map[rsrc.Type][]cache.SnapshotResource{
    rsrc.ClusterType: {
        {Resource: types.ResourceWithTTL{Resource: serviceA}, Wildcard: true},
        {Resource: types.ResourceWithTTL{Resource: serviceB}, Wildcard: true},
        {Resource: types.ResourceWithTTL{Resource: analytics}, Wildcard: false},  // On-demand
        {Resource: types.ResourceWithTTL{Resource: logging}, Wildcard: false},    // On-demand
    },
})

The following are public accessors for snapshots to specifically get resources marked as wildcard:

GetWildcardResources
GetWildcardResourcesAndTTL

…added. Clean up tests

mohit-pal-kohli · 2025-11-20T23:05:55Z

pkg/cache/v3/simple.go

The nil check above prevents sending empty responses when filtering returns no matching
resources for non-wildcard subscriptions, preserving the xDS "watch stays open" behavior.

This should likely be changed to "if non-wildcard" rather than "if has resources"

mohit-pal-kohli · 2025-11-20T23:08:25Z

pkg/cache/v3/simple_test.go

@@ -598,10 +608,10 @@ func TestAvertPanicForWatchOnNonExistentSnapshot(t *testing.T) {
 	// Create watch.
 	req := &cache.Request{
 		Node:          &core.Node{Id: "test"},
-		ResourceNames: []string{"rtds"},


Fixed test bug: resource name in request didn't match snapshot ("rtds" vs "one-second").
New filtering logic correctly returns nil for non-existent resources, exposing the mismatch.

mohit-pal-kohli · 2025-11-20T23:23:59Z

pkg/cache/v3/simple_test.go

I'm unsure why these tests needed this mock struct. The test can be accomplished without, though I was hesitant to make an out-of-scope change

valerian-roche

I believe overall the code might need a bit more refactor on the respond vs. createResponse functions, as well as within createDeltaResponse at this stage
The alternative would be to fully redesign as the linear cache but might be out of scope here

valerian-roche · 2025-11-24T16:00:09Z

pkg/cache/v3/snapshot.go

@@ -54,6 +72,47 @@ func NewSnapshot(version string, resources map[resource.Type][]types.Resource) (
 	return &out, nil
 }

+// NewSnapshotWithExplicitWildcard creates a Snapshot where each resource
+// explicitly specifies whether it should be returned as part of wildcard requests.
+func NewSnapshotWithExplicitWildcard(


Minor: given the functional implication, I'm wondering if we should rename this "default" instead
Wildcard is the technical name within the xDS specification, but it does not necessarily mean much in the context of users

valerian-roche · 2025-11-24T16:00:25Z

pkg/cache/v3/snapshot.go

@@ -54,6 +72,47 @@ func NewSnapshot(version string, resources map[resource.Type][]types.Resource) (
 	return &out, nil
 }

+// NewSnapshotWithExplicitWildcard creates a Snapshot where each resource
+// explicitly specifies whether it should be returned as part of wildcard requests.


Can you specify that it is mainly for OdCDS currently?

valerian-roche · 2025-11-24T16:01:22Z

pkg/cache/v3/snapshot.go

+		out.Resources[index] = NewResourcesWithTTL(version, rawResources)
+
+		// Only store wildcard map entry if there are resources of this type
+		// An empty set means no resources are wildcard for this type


What's the functional interpretation of this? Is it that the type uses all resources for wildcard or none?

valerian-roche · 2025-11-24T17:46:13Z

pkg/cache/v3/snapshot.go

+
+		// Only store wildcard map entry if there are resources of this type
+		// An empty set means no resources are wildcard for this type
+		if len(snapshotResources) > 0 {


Is this check needed? If no resources exist it's fine to set an empty set

valerian-roche · 2025-11-24T17:46:59Z

pkg/cache/v3/snapshot.go

+// IsResourceWildcard checks if a specific resource should be returned for wildcard requests.
+// If wildcardMap is nil, all resources are considered wildcard-eligible (backward compatibility).
+// If wildcardMap is not nil, only resources explicitly marked as wildcard are eligible.
+func (s *Snapshot) IsResourceWildcard(typeURL resource.Type, resourceName string) bool {


Minor: maybe ShouldReturnOnWildcard or WatchedByDefault

valerian-roche · 2025-11-24T18:04:22Z

pkg/cache/v3/simple.go

@@ -195,7 +248,7 @@ func (cache *snapshotCache) sendHeartbeats(ctx context.Context, node string) {
 		for id, watch := range info.watches {
 			// Respond with the current version regardless of whether the version has changed.
 			version := snapshot.GetVersion(watch.Request.GetTypeUrl())
-			resources := snapshot.GetResourcesAndTTL(watch.Request.GetTypeUrl())
+			resources := getResourcesAndTTLForSubscription(snapshot, watch.Request.GetTypeUrl(), watch.subscription.SubscribedResources(), watch.subscription.IsWildcard())


Minor: why not just pass in the watch or subscription directly?

valerian-roche · 2025-11-24T20:06:50Z

pkg/cache/v3/simple.go

 			// for instance if the subscription is newly wildcard.
-			for r := range snapshot.GetResources(request.GetTypeUrl()) {
+			for r := range snapshot.GetWildcardResources(request.GetTypeUrl()) {


This must also assess non-wildcard resources as above.
Likely would better be rewritten as always checking "subscribed" and also checking wildcard when applicable (instead of if/else)
Also shows there might be a missing test

valerian-roche · 2025-11-24T20:07:28Z

pkg/cache/v3/simple.go

@@ -485,6 +538,10 @@ func (cache *snapshotCache) cancelWatch(nodeID string, watchID int64) func() {
 // Respond to a watch with the snapshot value. The value channel should have capacity not to block.
 // TODO(kuat) do not respond always, see issue https://github.com/envoyproxy/go-control-plane/issues/46
 func (cache *snapshotCache) respond(ctx context.Context, watch ResponseWatch, resources map[string]types.ResourceWithTTL, version string, heartbeat bool) error {
+	if resources == nil {


This should not exit on initial query for wildcard, even if nothing is to be returned

valerian-roche · 2025-11-24T20:08:00Z

pkg/cache/v3/simple.go

This should likely be changed to "if non-wildcard" rather than "if has resources"

valerian-roche · 2025-11-24T20:08:31Z

pkg/cache/v3/simple.go

Wildcard support should be added here

mohit-pal-kohli added 6 commits November 18, 2025 16:42

FCP-1434: Explicit wildcard support for go control plane

8e2d459

Simplify the get funtionality to minimize code deuplication

9424db5

lint and test fixes

f8df9e1

Add support for ODCDS case where wildcard AND explicit resources are …

0710f23

…added. Clean up tests

Test fixes

5fbb1c0

Fix a few more failing tests

deed9fc

mohit-pal-kohli commented Nov 20, 2025

View reviewed changes

mohit-pal-kohli marked this pull request as ready for review November 20, 2025 23:17

mohit-pal-kohli self-assigned this Nov 20, 2025

mohit-pal-kohli requested review from a team November 20, 2025 23:18

mohit-pal-kohli commented Nov 20, 2025

View reviewed changes

style cleanup

96101d0

valerian-roche reviewed Nov 24, 2025

View reviewed changes

FCP-1434: Explicit wildcard support for snapshot cache #29

Are you sure you want to change the base?

FCP-1434: Explicit wildcard support for snapshot cache #29

Uh oh!

Conversation

mohit-pal-kohli commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Implementation details

Exported functions

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

valerian-roche left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mohit-pal-kohli commented Nov 18, 2025 •

edited

Loading