address comments

Author: zhiyanfoo

pkg/server/config/config.go

@@ -24,14 +24,18 @@ func NewOpts() Opts {
 	}
 }
 
-// LegacyWildcardDeactivated returns whether legacy wildcard mode is deactivated for all resource types
-func (o Opts) LegacyWildcardDeactivated() bool {
-	return o.legacyWildcardDeactivated
-}
-
-// LegacyWildcardDeactivatedTypes returns the set of resource types for which legacy wildcard mode is deactivated
-func (o Opts) LegacyWildcardDeactivatedTypes() map[string]struct{} {
-	return o.legacyWildcardDeactivatedTypes
+// IsLegacyWildcardActive returns whether legacy wildcard mode is active for the given resource type.
+// Returns true if legacy wildcard mode is active, false if it has been deactivated.
+func (o Opts) IsLegacyWildcardActive(typeURL string) bool {
+	if o.legacyWildcardDeactivated {
+		return false
+	}
+	if len(o.legacyWildcardDeactivatedTypes) > 0 {
+		if _, found := o.legacyWildcardDeactivatedTypes[typeURL]; found {
+			return false
+		}
+	}
+	return true
 }
 
 // Each xDS implementation should implement their own functional opts.

pkg/server/stream/v3/subscription.go

@@ -11,10 +11,10 @@ type Subscription struct {
 	// wildcard indicates if the subscription currently has a wildcard watch.
 	wildcard bool
 
-	// allowLegacyWildcard indicates that an empty request should be treated as a wildcard request.
-	// If in the stream at some point subscribes explicitly a resource or a explicitly makes wildcard
-	// request, then subsequent empty requests should not be treated as wildcard and so this
-	// flag will be set to false.
+	// allowLegacyWildcard indicates that the stream never provided any resource
+	// and is de facto wildcard.
+	// As soon as a resource or an explicit subscription to wildcard is provided,
+	// this flag will be set to false
 	allowLegacyWildcard bool
 
 	// subscribedResourceNames provides the resources explicitly requested by the client
@@ -26,17 +26,7 @@ type Subscription struct {
 }
 
 // newSubscription initializes a subscription state.
-func newSubscription(emptyRequest bool, initialResourceVersions map[string]string, opts config.Opts, typeURL string) Subscription {
-	allowLegacyWildcard := emptyRequest
-
-	if opts.LegacyWildcardDeactivated() {
-		allowLegacyWildcard = false
-	} else if typeMap := opts.LegacyWildcardDeactivatedTypes(); len(typeMap) > 0 {
-		if _, found := typeMap[typeURL]; found {
-			allowLegacyWildcard = false
-		}
-	}
-
+func newSubscription(emptyRequest, allowLegacyWildcard bool, initialResourceVersions map[string]string) Subscription {
 	// By default we set the subscription as a wildcard only if the request was empty
 	// and in legacy mode. Later on, outside of this constructor, when we actually
 	// process the request, if the request was non-empty, it may have an
@@ -59,7 +49,7 @@ func newSubscription(emptyRequest bool, initialResourceVersions map[string]strin
 }
 
 func NewSotwSubscription(subscribed []string, opts config.Opts, typeURL string) Subscription {
-	sub := newSubscription(len(subscribed) == 0, nil, opts, typeURL)
+	sub := newSubscription(len(subscribed) == 0, opts.IsLegacyWildcardActive(typeURL), nil)
 	sub.SetResourceSubscription(subscribed)
 	return sub
 }
@@ -110,7 +100,7 @@ func (s *Subscription) SetResourceSubscription(subscribed []string) {
 }
 
 func NewDeltaSubscription(subscribed, unsubscribed []string, initialResourceVersions map[string]string, opts config.Opts, typeURL string) Subscription {
-	sub := newSubscription(len(subscribed) == 0, initialResourceVersions, opts, typeURL)
+	sub := newSubscription(len(subscribed) == 0, opts.IsLegacyWildcardActive(typeURL), initialResourceVersions)
 	sub.UpdateResourceSubscriptions(subscribed, unsubscribed)
 	return sub
 }