in-memory resource client: support expression selector (#551)

* support expression selector

* reuse label selector func

* tests

* changelog

* Update pkg/api/v1/clients/memory/resource_client_test.go

Co-authored-by: David Jumani <[email protected]>

* only use labelSelector if not nil

* add issue link

* benchmark tests

---------

Co-authored-by: David Jumani <[email protected]>

changelog/v0.34.3/mem-client-expression-selector.yaml

@@ -0,0 +1,5 @@
+changelog:
+  - type: FIX
+    issueLink: https://github.com/solo-io/solo-kit/issues/553
+    description: >-
+      Add support for ExpressionSelector in memory ResourceClient.

pkg/api/v1/clients/kube/resource_client.go

@@ -7,11 +7,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/solo-io/solo-kit/pkg/utils/specutils"
-
-	"github.com/solo-io/solo-kit/pkg/utils/kubeutils"
-	"k8s.io/apimachinery/pkg/types"
-
 	"github.com/solo-io/go-utils/stringutils"
 	"github.com/solo-io/solo-kit/pkg/api/shared"
 	"github.com/solo-io/solo-kit/pkg/api/v1/clients"
@@ -20,12 +15,14 @@ import (
 	v1 "github.com/solo-io/solo-kit/pkg/api/v1/clients/kube/crd/solo.io/v1"
 	"github.com/solo-io/solo-kit/pkg/api/v1/resources"
 	"github.com/solo-io/solo-kit/pkg/errors"
+	"github.com/solo-io/solo-kit/pkg/utils/kubeutils"
+	"github.com/solo-io/solo-kit/pkg/utils/specutils"
 	"go.opencensus.io/stats"
 	"go.opencensus.io/stats/view"
 	"go.opencensus.io/tag"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/types"
 )
 
 var (
@@ -298,7 +295,7 @@ func (rc *ResourceClient) List(namespace string, opts clients.ListOpts) (resourc
 		return nil, err
 	}
 
-	labelSelector, err := rc.getLabelSelector(opts)
+	labelSelector, err := kubeutils.ToLabelSelector(opts)
 	if err != nil {
 		return nil, errors.Wrapf(err, "parsing label selector")
 	}
@@ -458,16 +455,6 @@ func (rc *ResourceClient) Watch(namespace string, opts clients.WatchOpts) (<-cha
 	return resourcesChan, errs, nil
 }
 
-func (rc *ResourceClient) getLabelSelector(listOpts clients.ListOpts) (labels.Selector, error) {
-	// https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#set-based-requirement
-	if listOpts.ExpressionSelector != "" {
-		return labels.Parse(listOpts.ExpressionSelector)
-	}
-
-	// https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#equality-based-requirement
-	return labels.SelectorFromSet(listOpts.Selector), nil
-}
-
 // Checks whether the group version kind of the given resource matches that of the client's underlying CRD:
 func (rc *ResourceClient) matchesClientGVK(resource v1.Resource) bool {
 	return resource.GroupVersionKind().String() == rc.crd.GroupVersionKind().String()

pkg/api/v1/clients/memory/resource_client.go

@@ -12,6 +12,7 @@ import (
 	"github.com/solo-io/solo-kit/pkg/api/v1/clients"
 	"github.com/solo-io/solo-kit/pkg/api/v1/resources"
 	"github.com/solo-io/solo-kit/pkg/errors"
+	"github.com/solo-io/solo-kit/pkg/utils/kubeutils"
 	"k8s.io/apimachinery/pkg/labels"
 )
 
@@ -197,9 +198,19 @@ func (rc *ResourceClient) Delete(namespace, name string, opts clients.DeleteOpts
 func (rc *ResourceClient) List(namespace string, opts clients.ListOpts) (resources.ResourceList, error) {
 	opts = opts.WithDefaults()
 	cachedResources := rc.cache.List(rc.Prefix(namespace))
+
+	var labelSelector labels.Selector
+	var err error
+	if kubeutils.HasSelector(opts) {
+		labelSelector, err = kubeutils.ToLabelSelector(opts)
+		if err != nil {
+			return nil, errors.Wrapf(err, "parsing label selector")
+		}
+	}
+
 	var resourceList resources.ResourceList
 	for _, resource := range cachedResources {
-		if labels.SelectorFromSet(opts.Selector).Matches(labels.Set(resource.GetMetadata().Labels)) {
+		if labelSelector == nil || labelSelector.Matches(labels.Set(resource.GetMetadata().Labels)) {
 			clone := resources.Clone(resource)
 			resourceList = append(resourceList, clone)
 		}
@@ -220,8 +231,9 @@ func (rc *ResourceClient) Watch(namespace string, opts clients.WatchOpts) (<-cha
 	errs := make(chan error)
 	updateResourceList := func() {
 		list, err := rc.List(namespace, clients.ListOpts{
-			Ctx:      opts.Ctx,
-			Selector: opts.Selector,
+			Ctx:                opts.Ctx,
+			Selector:           opts.Selector,
+			ExpressionSelector: opts.ExpressionSelector,
 		})
 		if err != nil {
 			errs <- err

pkg/api/v1/clients/memory/resource_client_test.go

@@ -8,6 +8,7 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"github.com/onsi/gomega/gmeasure"
 	"github.com/solo-io/solo-kit/pkg/api/v1/clients"
 	. "github.com/solo-io/solo-kit/pkg/api/v1/clients/memory"
 	"github.com/solo-io/solo-kit/pkg/api/v1/resources"
@@ -62,33 +63,273 @@ var _ = Describe("Base", func() {
 		Expect(listret[0]).NotTo(BeIdenticalTo(listret2[0]))
 	})
 
+	Context("listing resources", func() {
+		var (
+			obj1 *v1.MockResource
+			obj2 *v1.MockResource
+			obj3 *v1.MockResource
+			obj4 *v1.MockResource
+			obj5 *v1.MockResource
+		)
+
+		BeforeEach(func() {
+			obj1 = &v1.MockResource{
+				Metadata: &core.Metadata{
+					Name:      "name1",
+					Namespace: "ns1",
+					Labels: map[string]string{
+						"key": "val1",
+					},
+				},
+			}
+			obj2 = &v1.MockResource{
+				Metadata: &core.Metadata{
+					Name:      "name2",
+					Namespace: "ns2",
+					Labels: map[string]string{
+						"key": "val2",
+					},
+				},
+			}
+			obj3 = &v1.MockResource{
+				Metadata: &core.Metadata{
+					Name:      "name3",
+					Namespace: "ns1",
+					Labels: map[string]string{
+						"key": "val2",
+					},
+				},
+			}
+			obj4 = &v1.MockResource{
+				Metadata: &core.Metadata{
+					Name:      "name4",
+					Namespace: "ns2",
+					Labels: map[string]string{
+						"key": "val3",
+					},
+				},
+			}
+			obj5 = &v1.MockResource{
+				Metadata: &core.Metadata{
+					Name:      "name5",
+					Namespace: "ns1",
+					Labels: map[string]string{
+						"key": "val3",
+					},
+				},
+			}
+
+			_, err := client.Write(obj1, clients.WriteOpts{})
+			Expect(err).NotTo(HaveOccurred())
+			_, err = client.Write(obj2, clients.WriteOpts{})
+			Expect(err).NotTo(HaveOccurred())
+			_, err = client.Write(obj3, clients.WriteOpts{})
+			Expect(err).NotTo(HaveOccurred())
+			_, err = client.Write(obj4, clients.WriteOpts{})
+			Expect(err).NotTo(HaveOccurred())
+			_, err = client.Write(obj5, clients.WriteOpts{})
+			Expect(err).NotTo(HaveOccurred())
+		})
+
+		It("lists all resources when empty namespace is provided", func() {
+			resources, err := client.List("", clients.ListOpts{})
+			Expect(err).NotTo(HaveOccurred())
+
+			// resources are sorted by namespace, then name
+			expectedResourceNames := []string{
+				"name1", "name3", "name5", // ns1
+				"name2", "name4", // ns2
+			}
+			Expect(resources).To(HaveLen(len(expectedResourceNames)))
+			for i, r := range resources {
+				Expect(r.GetMetadata().GetName()).To(Equal(expectedResourceNames[i]))
+			}
+		})
+
+		It("lists resources in a given namespace", func() {
+			resources, err := client.List("ns2", clients.ListOpts{})
+			Expect(err).NotTo(HaveOccurred())
+
+			expectedResourceNames := []string{
+				"name2", "name4",
+			}
+			Expect(resources).To(HaveLen(len(expectedResourceNames)))
+			for i, r := range resources {
+				Expect(r.GetMetadata().GetName()).To(Equal(expectedResourceNames[i]))
+			}
+		})
+
+		It("returns empty list if namespace is invalid", func() {
+			resources, err := client.List("invalid-namespace", clients.ListOpts{})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(resources).To(HaveLen(0))
+		})
+
+		It("returns resources matching the given selector, across all namespaces", func() {
+			resources, err := client.List("", clients.ListOpts{
+				Selector: map[string]string{
+					"key": "val2",
+				},
+			})
+			Expect(err).NotTo(HaveOccurred())
+
+			// resources are sorted by namespace, then name
+			expectedResourceNames := []string{
+				"name3", "name2",
+			}
+			Expect(resources).To(HaveLen(len(expectedResourceNames)))
+			for i, r := range resources {
+				Expect(r.GetMetadata().GetName()).To(Equal(expectedResourceNames[i]))
+			}
+		})
+
+		It("returns resources matching the given selector, in given namespace", func() {
+			resources, err := client.List("ns2", clients.ListOpts{
+				Selector: map[string]string{
+					"key": "val2",
+				},
+			})
+			Expect(err).NotTo(HaveOccurred())
+
+			Expect(resources).To(HaveLen(1))
+			Expect(resources[0].GetMetadata().GetName()).To(Equal("name2"))
+		})
+
+		It("returns resources matching the given expression selector, across all namespaces", func() {
+			resources, err := client.List("", clients.ListOpts{
+				ExpressionSelector: "key in (val1,val3)",
+			})
+			Expect(err).NotTo(HaveOccurred())
+
+			// resources are sorted by namespace, then name
+			expectedResourceNames := []string{
+				"name1", "name5", "name4",
+			}
+			Expect(resources).To(HaveLen(len(expectedResourceNames)))
+			for i, r := range resources {
+				Expect(r.GetMetadata().GetName()).To(Equal(expectedResourceNames[i]))
+			}
+		})
+
+		It("returns resources matching the given expression selector, in given namespace", func() {
+			resources, err := client.List("ns2", clients.ListOpts{
+				ExpressionSelector: "key in (val1,val3)",
+			})
+			Expect(err).NotTo(HaveOccurred())
+
+			Expect(resources).To(HaveLen(1))
+			Expect(resources[0].GetMetadata().GetName()).To(Equal("name4"))
+		})
+
+		It("when both selector and expression selector are provided, uses expression selector", func() {
+			resources, err := client.List("ns2", clients.ListOpts{
+				Selector: map[string]string{
+					"key": "val2",
+				},
+				ExpressionSelector: "key in (val1,val3)",
+			})
+			Expect(err).NotTo(HaveOccurred())
+
+			Expect(resources).To(HaveLen(1))
+			Expect(resources[0].GetMetadata().GetName()).To(Equal("name4"))
+		})
+	})
+
 	Context("Benchmarks", func() {
-		Measure("it should perform list efficiently", func(b Benchmarker) {
-			const numobjs = 10000
+		const numobjs = 10000
 
+		BeforeEach(func() {
 			for i := 0; i < numobjs; i++ {
+				isEven := i%2 == 0
 				obj := &v1.MockResource{
 					Metadata: &core.Metadata{
 						Namespace: "ns",
 						Name:      fmt.Sprintf("n-%v", numobjs-i),
+						Labels: map[string]string{
+							"even": fmt.Sprintf("%v", isEven),
+						},
 					},
 					Data: strings.Repeat("123", 1000) + fmt.Sprintf("test-%v", i),
 				}
 				client.Write(obj, clients.WriteOpts{})
 			}
+		})
+
+		It("should perform list efficiently", Serial, func() {
+			experiment := gmeasure.NewExperiment("list resources with no selector")
+			AddReportEntry(experiment.Name, experiment)
+
 			l := clients.ListOpts{}
 			var output resources.ResourceList
 			var err error
-			runtime := b.Time("runtime", func() {
-				output, err = client.List("ns", l)
-			})
-			Expect(err).NotTo(HaveOccurred())
-			Expect(output).To(HaveLen(numobjs))
-			Expect(output[0].GetMetadata().Name).To(Equal("n-1"))
 
-			Expect(runtime.Seconds()).Should(BeNumerically("<", 0.5), "List() shouldn't take too long.")
-		}, 10)
+			experiment.Sample(func(idx int) {
+				experiment.MeasureDuration("list-resources", func() {
+					output, err = client.List("ns", l)
+				})
 
+				Expect(err).NotTo(HaveOccurred())
+				Expect(output).To(HaveLen(numobjs))
+				Expect(output[0].GetMetadata().Name).To(Equal("n-1"))
+			}, gmeasure.SamplingConfig{N: 10, Duration: 10 * time.Second})
+
+			stats := experiment.GetStats("list-resources")
+			medianDuration := stats.DurationFor(gmeasure.StatMedian)
+
+			Expect(medianDuration).To(BeNumerically("<", 500*time.Millisecond))
+		})
+
+		It("should perform list efficiently, with equality-based selector", Serial, func() {
+			experiment := gmeasure.NewExperiment("list resources with equality selector")
+			AddReportEntry(experiment.Name, experiment)
+
+			l := clients.ListOpts{
+				Selector: map[string]string{
+					"even": "true",
+				},
+			}
+			var output resources.ResourceList
+			var err error
+
+			experiment.Sample(func(idx int) {
+				experiment.MeasureDuration("list-resources-with-selector", func() {
+					output, err = client.List("ns", l)
+				})
+
+				Expect(err).NotTo(HaveOccurred())
+				Expect(output).To(HaveLen(numobjs / 2))
+			}, gmeasure.SamplingConfig{N: 10, Duration: 10 * time.Second})
+
+			stats := experiment.GetStats("list-resources-with-selector")
+			medianDuration := stats.DurationFor(gmeasure.StatMedian)
+
+			Expect(medianDuration).To(BeNumerically("<", 500*time.Millisecond))
+		})
+
+		It("should perform list efficiently, with set-based selector", Serial, func() {
+			experiment := gmeasure.NewExperiment("list resources with set selector")
+			AddReportEntry(experiment.Name, experiment)
+
+			l := clients.ListOpts{
+				ExpressionSelector: "even in (true,false)",
+			}
+			var output resources.ResourceList
+			var err error
+
+			experiment.Sample(func(idx int) {
+				experiment.MeasureDuration("list-resources-with-set-selector", func() {
+					output, err = client.List("ns", l)
+				})
+
+				Expect(err).NotTo(HaveOccurred())
+				Expect(output).To(HaveLen(numobjs))
+			}, gmeasure.SamplingConfig{N: 10, Duration: 10 * time.Second})
+
+			stats := experiment.GetStats("list-resources-with-set-selector")
+			medianDuration := stats.DurationFor(gmeasure.StatMedian)
+
+			Expect(medianDuration).To(BeNumerically("<", 500*time.Millisecond))
+		})
 	})
 
 })