diff --git a/pkg/builder/example_test.go b/pkg/builder/example_test.go index 21f8500f6b..8339b56087 100644 --- a/pkg/builder/example_test.go +++ b/pkg/builder/example_test.go @@ -77,7 +77,7 @@ func (a *ReplicaSetReconciler) Reconcile(req reconcile.Request) (reconcile.Resul // List the Pods matching the PodTemplate Labels pods := &corev1.PodList{} - err = a.List(context.TODO(), pods, client.InNamespace(req.Namespace), client.MatchingLabels(rs.Spec.Template.Labels)) + err = a.List(context.TODO(), client.InNamespace(req.Namespace).MatchingLabels(rs.Spec.Template.Labels), pods) if err != nil { return reconcile.Result{}, err } diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go index 1a2d71d12f..82441777d8 100644 --- a/pkg/cache/cache_test.go +++ b/pkg/cache/cache_test.go @@ -113,7 +113,7 @@ var _ = Describe("Informer Cache", func() { It("should be able to list objects that haven't been watched previously", func() { By("listing all services in the cluster") listObj := &kcorev1.ServiceList{} - Expect(informerCache.List(context.Background(), listObj)).To(Succeed()) + Expect(informerCache.List(context.Background(), nil, listObj)).To(Succeed()) By("verifying that the returned list contains the Kubernetes service") // NB: kubernetes default service is automatically created in testenv. @@ -143,10 +143,8 @@ var _ = Describe("Informer Cache", func() { By("listing pods with a particular label") // NB: each pod has a "test-label": out := kcorev1.PodList{} - Expect(informerCache.List(context.Background(), &out, - client.InNamespace(testNamespaceTwo), - client.MatchingLabels(map[string]string{"test-label": "test-pod-2"}), - )).To(Succeed()) + Expect(informerCache.List(context.Background(), client.InNamespace(testNamespaceTwo). + MatchingLabels(map[string]string{"test-label": "test-pod-2"}), &out)).To(Succeed()) By("verifying the returned pods have the correct label") Expect(out.Items).NotTo(BeEmpty()) @@ -163,9 +161,8 @@ var _ = Describe("Informer Cache", func() { // NB: each pod has a "test-label": out := kcorev1.PodList{} labels := map[string]string{"test-label": "test-pod-2"} - Expect(informerCache.List(context.Background(), &out, - client.MatchingLabels(labels), - )).To(Succeed()) + Expect(informerCache.List(context.Background(), + client.MatchingLabels(labels), &out)).To(Succeed()) By("verifying multiple pods with the same label in different namespaces are returned") Expect(out.Items).NotTo(BeEmpty()) @@ -180,9 +177,9 @@ var _ = Describe("Informer Cache", func() { It("should be able to list objects by namespace", func() { By("listing pods in test-namespace-1") listObj := &kcorev1.PodList{} - Expect(informerCache.List(context.Background(), listObj, + Expect(informerCache.List(context.Background(), client.InNamespace(testNamespaceOne), - )).To(Succeed()) + listObj)).To(Succeed()) By("verifying that the returned pods are in test-namespace-1") Expect(listObj.Items).NotTo(BeEmpty()) @@ -320,9 +317,9 @@ var _ = Describe("Informer Cache", func() { By("listing Pods with restartPolicyOnFailure") listObj := &kcorev1.PodList{} - Expect(informer.List(context.Background(), listObj, + Expect(informer.List(context.Background(), client.MatchingField("spec.restartPolicy", "OnFailure"), - )).To(Succeed()) + listObj)).To(Succeed()) By("verifying that the returned pods have correct restart policy") Expect(listObj.Items).NotTo(BeEmpty()) diff --git a/pkg/cache/informer_cache.go b/pkg/cache/informer_cache.go index 4e132fa3af..03298baeeb 100644 --- a/pkg/cache/informer_cache.go +++ b/pkg/cache/informer_cache.go @@ -57,7 +57,7 @@ func (ip *informerCache) Get(ctx context.Context, key client.ObjectKey, out runt } // List implements Reader -func (ip *informerCache) List(ctx context.Context, out runtime.Object, opts ...client.ListOptionFunc) error { +func (ip *informerCache) List(ctx context.Context, opts *client.ListOptions, out runtime.Object) error { itemsPtr, err := apimeta.GetItemsPtr(out) if err != nil { return nil @@ -87,7 +87,7 @@ func (ip *informerCache) List(ctx context.Context, out runtime.Object, opts ...c return err } - return cache.Reader.List(ctx, out, opts...) + return cache.Reader.List(ctx, opts, out) } // GetInformerForKind returns the informer for the GroupVersionKind diff --git a/pkg/cache/informertest/fake_cache.go b/pkg/cache/informertest/fake_cache.go index 2f311cf729..87e6aeedcb 100644 --- a/pkg/cache/informertest/fake_cache.go +++ b/pkg/cache/informertest/fake_cache.go @@ -136,6 +136,6 @@ func (c *FakeInformers) Get(ctx context.Context, key client.ObjectKey, obj runti } // List implements Cache -func (c *FakeInformers) List(ctx context.Context, list runtime.Object, opts ...client.ListOptionFunc) error { +func (c *FakeInformers) List(ctx context.Context, opts *client.ListOptions, list runtime.Object) error { return nil } diff --git a/pkg/cache/internal/cache_reader.go b/pkg/cache/internal/cache_reader.go index 281a0fe826..199d0bf835 100644 --- a/pkg/cache/internal/cache_reader.go +++ b/pkg/cache/internal/cache_reader.go @@ -86,26 +86,23 @@ func (c *CacheReader) Get(_ context.Context, key client.ObjectKey, out runtime.O } // List lists items out of the indexer and writes them to out -func (c *CacheReader) List(ctx context.Context, out runtime.Object, opts ...client.ListOptionFunc) error { +func (c *CacheReader) List(ctx context.Context, opts *client.ListOptions, out runtime.Object) error { var objs []interface{} var err error - listOpts := client.ListOptions{} - listOpts.ApplyOptions(opts) - - if listOpts.FieldSelector != nil { + if opts != nil && opts.FieldSelector != nil { // TODO(directxman12): support more complicated field selectors by // combining multiple indicies, GetIndexers, etc - field, val, requiresExact := requiresExactMatch(listOpts.FieldSelector) + field, val, requiresExact := requiresExactMatch(opts.FieldSelector) if !requiresExact { return fmt.Errorf("non-exact field matches are not supported by the cache") } // list all objects by the field selector. If this is namespaced and we have one, ask for the // namespaced index key. Otherwise, ask for the non-namespaced variant by using the fake "all namespaces" // namespace. - objs, err = c.indexer.ByIndex(FieldIndexName(field), KeyToNamespacedKey(listOpts.Namespace, val)) - } else if listOpts.Namespace != "" { - objs, err = c.indexer.ByIndex(cache.NamespaceIndex, listOpts.Namespace) + objs, err = c.indexer.ByIndex(FieldIndexName(field), KeyToNamespacedKey(opts.Namespace, val)) + } else if opts != nil && opts.Namespace != "" { + objs, err = c.indexer.ByIndex(cache.NamespaceIndex, opts.Namespace) } else { objs = c.indexer.List() } @@ -113,8 +110,8 @@ func (c *CacheReader) List(ctx context.Context, out runtime.Object, opts ...clie return err } var labelSel labels.Selector - if listOpts.LabelSelector != nil { - labelSel = listOpts.LabelSelector + if opts != nil && opts.LabelSelector != nil { + labelSel = opts.LabelSelector } outItems, err := c.getListItems(objs, labelSel) diff --git a/pkg/client/client.go b/pkg/client/client.go index a1e1478eae..195d180dda 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -140,19 +140,20 @@ func (c *client) Get(ctx context.Context, key ObjectKey, obj runtime.Object) err } // List implements client.Client -func (c *client) List(ctx context.Context, obj runtime.Object, opts ...ListOptionFunc) error { +func (c *client) List(ctx context.Context, opts *ListOptions, obj runtime.Object) error { r, err := c.cache.getResource(obj) if err != nil { return err } - - listOpts := ListOptions{} - listOpts.ApplyOptions(opts) + namespace := "" + if opts != nil { + namespace = opts.Namespace + } return r.Get(). - NamespaceIfScoped(listOpts.Namespace, r.isNamespaced()). + NamespaceIfScoped(namespace, r.isNamespaced()). Resource(r.resource()). Body(obj). - VersionedParams(listOpts.AsListOptions(), c.paramCodec). + VersionedParams(opts.AsListOptions(), c.paramCodec). Do(). Into(obj) } diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 4a91153f39..072322c8ed 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -763,7 +763,7 @@ var _ = Describe("Client", func() { By("listing all objects of that type in the cluster") deps := &appsv1.DeploymentList{} - Expect(cl.List(context.Background(), deps)).NotTo(HaveOccurred()) + Expect(cl.List(context.Background(), nil, deps)).NotTo(HaveOccurred()) Expect(deps.Items).NotTo(BeEmpty()) hasDep := false @@ -784,7 +784,7 @@ var _ = Describe("Client", func() { By("listing all Deployments in the cluster") deps := &appsv1.DeploymentList{} - Expect(cl.List(context.Background(), deps)).NotTo(HaveOccurred()) + Expect(cl.List(context.Background(), nil, deps)).NotTo(HaveOccurred()) By("validating no Deployments are returned") Expect(deps.Items).To(BeEmpty()) @@ -793,69 +793,59 @@ var _ = Describe("Client", func() { }, serverSideTimeoutSeconds) // TODO(seans): get label selector test working - It("should filter results by label selector", func(done Done) { - By("creating a Deployment with the app=frontend label") - depFrontend := &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "deployment-frontend", - Namespace: ns, - Labels: map[string]string{"app": "frontend"}, - }, - Spec: appsv1.DeploymentSpec{ - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"app": "frontend"}, - }, - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"app": "frontend"}}, - Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "nginx", Image: "nginx"}}}, - }, - }, - } - depFrontend, err := clientset.AppsV1().Deployments(ns).Create(depFrontend) - Expect(err).NotTo(HaveOccurred()) - - By("creating a Deployment with the app=backend label") - depBackend := &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "deployment-backend", - Namespace: ns, - Labels: map[string]string{"app": "backend"}, - }, - Spec: appsv1.DeploymentSpec{ - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"app": "backend"}, - }, - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"app": "backend"}}, - Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "nginx", Image: "nginx"}}}, - }, - }, - } - depBackend, err = clientset.AppsV1().Deployments(ns).Create(depBackend) - Expect(err).NotTo(HaveOccurred()) - - cl, err := client.New(cfg, client.Options{}) - Expect(err).NotTo(HaveOccurred()) - - By("listing all Deployments with label app=backend") - deps := &appsv1.DeploymentList{} - labels := map[string]string{"app": "backend"} - err = cl.List(context.Background(), deps, - client.MatchingLabels(labels), - ) - Expect(err).NotTo(HaveOccurred()) - - By("only the Deployment with the backend label is returned") - Expect(deps.Items).NotTo(BeEmpty()) - Expect(1).To(Equal(len(deps.Items))) - actual := deps.Items[0] - Expect(actual.Name).To(Equal("deployment-backend")) - - deleteDeployment(depFrontend, ns) - deleteDeployment(depBackend, ns) - - close(done) - }, serverSideTimeoutSeconds) + // It("should filter results by label selector", func(done Done) { + // By("creating a Deployment with the app=frontend label") + // depFrontend := &appsv1.Deployment{ + // ObjectMeta: metav1.ObjectMeta{Name: "deployment-frontend", Namespace: ns}, + // Spec: appsv1.DeploymentSpec{ + // Selector: &metav1.LabelSelector{ + // MatchLabels: map[string]string{"app": "frontend"}, + // }, + // Template: corev1.PodTemplateSpec{ + // ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"app": "frontend"}}, + // Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "nginx", Image: "nginx"}}}, + // }, + // }, + // } + // depFrontend, err := clientset.AppsV1().Deployments(ns).Create(depFrontend) + // Expect(err).NotTo(HaveOccurred()) + + // By("creating a Deployment with the app=backend label") + // depBackend := &appsv1.Deployment{ + // ObjectMeta: metav1.ObjectMeta{Name: "deployment-backend", Namespace: ns}, + // Spec: appsv1.DeploymentSpec{ + // Selector: &metav1.LabelSelector{ + // MatchLabels: map[string]string{"app": "backend"}, + // }, + // Template: corev1.PodTemplateSpec{ + // ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"app": "backend"}}, + // Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "nginx", Image: "nginx"}}}, + // }, + // }, + // } + // depBackend, err = clientset.AppsV1().Deployments(ns).Create(depBackend) + // Expect(err).NotTo(HaveOccurred()) + + // cl, err := client.New(cfg, client.Options{}) + // Expect(err).NotTo(HaveOccurred()) + + // By("listing all Deployments with label app=backend") + // deps := &appsv1.DeploymentList{} + // labels := map[string]string{"app": "backend"} + // lo := client.InNamespace(ns).MatchingLabels(labels) + // Expect(cl.List(context.Background(), lo, deps)).NotTo(HaveOccurred()) + + // By("only the Deployment with the backend label is returned") + // Expect(deps.Items).NotTo(BeEmpty()) + // Expect(1).To(Equal(len(deps.Items))) + // actual := deps.Items[0] + // Expect(actual.Name).To(Equal("deployment-backend")) + + // deleteDeployment(depFrontend, ns) + // deleteDeployment(depBackend, ns) + + // close(done) + // }, serverSideTimeoutSeconds) It("should filter results by namespace selector", func(done Done) { By("creating a Deployment in test-namespace-1") @@ -901,8 +891,8 @@ var _ = Describe("Client", func() { By("listing all Deployments in test-namespace-1") deps := &appsv1.DeploymentList{} - err = cl.List(context.Background(), deps, client.InNamespace("test-namespace-1")) - Expect(err).NotTo(HaveOccurred()) + lo := client.InNamespace("test-namespace-1") + Expect(cl.List(context.Background(), lo, deps)).NotTo(HaveOccurred()) By("only the Deployment in test-namespace-1 is returned") Expect(deps.Items).NotTo(BeEmpty()) @@ -956,8 +946,8 @@ var _ = Describe("Client", func() { By("listing all Deployments with field metadata.name=deployment-backend") deps := &appsv1.DeploymentList{} - err = cl.List(context.Background(), deps, client.MatchingField("metadata.name", "deployment-backend")) - Expect(err).NotTo(HaveOccurred()) + lo := client.MatchingField("metadata.name", "deployment-backend") + Expect(cl.List(context.Background(), lo, deps)).NotTo(HaveOccurred()) By("only the Deployment with the backend field is returned") Expect(deps.Items).NotTo(BeEmpty()) @@ -971,101 +961,6 @@ var _ = Describe("Client", func() { close(done) }, serverSideTimeoutSeconds) - It("should filter results by namespace selector and label selector", func(done Done) { - By("creating a Deployment in test-namespace-3 with the app=frontend label") - tns3 := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace-3"}} - _, err := clientset.CoreV1().Namespaces().Create(tns3) - Expect(err).NotTo(HaveOccurred()) - depFrontend3 := &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "deployment-frontend", - Namespace: "test-namespace-3", - Labels: map[string]string{"app": "frontend"}, - }, - Spec: appsv1.DeploymentSpec{ - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"app": "frontend"}, - }, - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"app": "frontend"}}, - Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "nginx", Image: "nginx"}}}, - }, - }, - } - depFrontend3, err = clientset.AppsV1().Deployments("test-namespace-3").Create(depFrontend3) - Expect(err).NotTo(HaveOccurred()) - - By("creating a Deployment in test-namespace-3 with the app=backend label") - depBackend3 := &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "deployment-backend", - Namespace: "test-namespace-3", - Labels: map[string]string{"app": "backend"}, - }, - Spec: appsv1.DeploymentSpec{ - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"app": "backend"}, - }, - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"app": "backend"}}, - Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "nginx", Image: "nginx"}}}, - }, - }, - } - depBackend3, err = clientset.AppsV1().Deployments("test-namespace-3").Create(depBackend3) - Expect(err).NotTo(HaveOccurred()) - - By("creating a Deployment in test-namespace-4 with the app=frontend label") - tns4 := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace-4"}} - _, err = clientset.CoreV1().Namespaces().Create(tns4) - Expect(err).NotTo(HaveOccurred()) - depFrontend4 := &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "deployment-frontend", - Namespace: "test-namespace-4", - Labels: map[string]string{"app": "frontend"}, - }, - Spec: appsv1.DeploymentSpec{ - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"app": "frontend"}, - }, - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"app": "frontend"}}, - Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "nginx", Image: "nginx"}}}, - }, - }, - } - depFrontend4, err = clientset.AppsV1().Deployments("test-namespace-4").Create(depFrontend4) - Expect(err).NotTo(HaveOccurred()) - - cl, err := client.New(cfg, client.Options{}) - Expect(err).NotTo(HaveOccurred()) - - By("listing all Deployments in test-namespace-3 with label app=frontend") - deps := &appsv1.DeploymentList{} - labels := map[string]string{"app": "frontend"} - err = cl.List(context.Background(), deps, - client.InNamespace("test-namespace-3"), - client.MatchingLabels(labels), - ) - Expect(err).NotTo(HaveOccurred()) - - By("only the Deployment in test-namespace-3 with label app=frontend is returned") - Expect(deps.Items).NotTo(BeEmpty()) - Expect(1).To(Equal(len(deps.Items))) - actual := deps.Items[0] - Expect(actual.Name).To(Equal("deployment-frontend")) - Expect(actual.Namespace).To(Equal("test-namespace-3")) - - deleteDeployment(depFrontend3, "test-namespace-3") - deleteDeployment(depBackend3, "test-namespace-3") - deleteDeployment(depFrontend4, "test-namespace-4") - deleteNamespace(tns3) - deleteNamespace(tns4) - - close(done) - }, serverSideTimeoutSeconds) - PIt("should fail if it cannot get a client", func() { }) @@ -1179,30 +1074,19 @@ var _ = Describe("Client", func() { It("should be created from MatchingLabels", func() { labels := map[string]string{"foo": "bar"} - lo := &client.ListOptions{} - client.MatchingLabels(labels)(lo) + lo := client.MatchingLabels(labels) Expect(lo).NotTo(BeNil()) Expect(lo.LabelSelector.String()).To(Equal("foo=bar")) }) It("should be created from MatchingField", func() { - lo := &client.ListOptions{} - client.MatchingField("field1", "bar")(lo) + lo := client.MatchingField("field1", "bar") Expect(lo).NotTo(BeNil()) Expect(lo.FieldSelector.String()).To(Equal("field1=bar")) }) It("should be created from InNamespace", func() { - lo := &client.ListOptions{} - client.InNamespace("test")(lo) - Expect(lo).NotTo(BeNil()) - Expect(lo.Namespace).To(Equal("test")) - }) - - It("should allow pre-built ListOptions", func() { - lo := &client.ListOptions{} - newLo := &client.ListOptions{} - client.UseListOptions(newLo.InNamespace("test"))(lo) + lo := client.InNamespace("test") Expect(lo).NotTo(BeNil()) Expect(lo.Namespace).To(Equal("test")) }) diff --git a/pkg/client/fake/client.go b/pkg/client/fake/client.go index 657d031dca..8597cbde6c 100644 --- a/pkg/client/fake/client.go +++ b/pkg/client/fake/client.go @@ -19,9 +19,7 @@ package fake import ( "context" "encoding/json" - "fmt" "os" - "strings" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" @@ -79,23 +77,10 @@ func (c *fakeClient) Get(ctx context.Context, key client.ObjectKey, obj runtime. return err } -func (c *fakeClient) List(ctx context.Context, obj runtime.Object, opts ...client.ListOptionFunc) error { - gvk, err := apiutil.GVKForObject(obj, scheme.Scheme) - if err != nil { - return err - } - - if !strings.HasSuffix(gvk.Kind, "List") { - return fmt.Errorf("non-list type %T (kind %q) passed as output", obj, gvk) - } - // we need the non-list GVK, so chop off the "List" from the end of the kind - gvk.Kind = gvk.Kind[:len(gvk.Kind)-4] - - listOpts := client.ListOptions{} - listOpts.ApplyOptions(opts) - +func (c *fakeClient) List(ctx context.Context, opts *client.ListOptions, list runtime.Object) error { + gvk := opts.Raw.TypeMeta.GroupVersionKind() gvr, _ := meta.UnsafeGuessKindToResource(gvk) - o, err := c.tracker.List(gvr, gvk, listOpts.Namespace) + o, err := c.tracker.List(gvr, gvk, opts.Namespace) if err != nil { return err } @@ -104,7 +89,7 @@ func (c *fakeClient) List(ctx context.Context, obj runtime.Object, opts ...clien return err } decoder := scheme.Codecs.UniversalDecoder() - _, _, err = decoder.Decode(j, nil, obj) + _, _, err = decoder.Decode(j, nil, list) return err } diff --git a/pkg/client/fake/client_test.go b/pkg/client/fake/client_test.go index 872686c7bf..635c8c1058 100644 --- a/pkg/client/fake/client_test.go +++ b/pkg/client/fake/client_test.go @@ -17,12 +17,15 @@ limitations under the License. package fake import ( + "encoding/json" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -66,11 +69,22 @@ var _ = Describe("Fake client", func() { It("should be able to List", func() { By("Listing all deployments in a namespace") - list := &appsv1.DeploymentList{} - err := cl.List(nil, list, client.InNamespace("ns1")) + list := &metav1.List{} + err := cl.List(nil, &client.ListOptions{ + Namespace: "ns1", + Raw: &metav1.ListOptions{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apps/v1", + Kind: "Deployment", + }, + }, + }, list) Expect(err).To(BeNil()) Expect(list.Items).To(HaveLen(1)) - Expect(list.Items).To(ConsistOf(*dep)) + j, err := json.Marshal(dep) + Expect(err).To(BeNil()) + expectedDep := runtime.RawExtension{Raw: j} + Expect(list.Items).To(ConsistOf(expectedDep)) }) It("should be able to Create", func() { @@ -126,8 +140,16 @@ var _ = Describe("Fake client", func() { Expect(err).To(BeNil()) By("Listing all deployments in the namespace") - list := &appsv1.DeploymentList{} - err = cl.List(nil, list, client.InNamespace("ns1")) + list := &metav1.List{} + err = cl.List(nil, &client.ListOptions{ + Namespace: "ns1", + Raw: &metav1.ListOptions{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apps/v1", + Kind: "Deployment", + }, + }, + }, list) Expect(err).To(BeNil()) Expect(list.Items).To(HaveLen(0)) }) diff --git a/pkg/client/interfaces.go b/pkg/client/interfaces.go index 968a5e6ccf..36d0fce620 100644 --- a/pkg/client/interfaces.go +++ b/pkg/client/interfaces.go @@ -51,7 +51,7 @@ type Reader interface { // List retrieves list of objects for a given namespace and list options. On a // successful call, Items field in the list will be populated with the // result returned from the server. - List(ctx context.Context, list runtime.Object, opts ...ListOptionFunc) error + List(ctx context.Context, opts *ListOptions, list runtime.Object) error } // Writer knows how to create, delete, and update Kubernetes objects. @@ -185,7 +185,7 @@ func PropagationPolicy(p metav1.DeletionPropagation) DeleteOptionFunc { } } -// ListOptions contains options for limiting or filtering results. +// ListOptions contains options for limitting or filtering results. // It's generally a subset of metav1.ListOptions, with support for // pre-parsed selectors (since generally, selectors will be executed // against the cache). @@ -248,20 +248,6 @@ func (o *ListOptions) AsListOptions() *metav1.ListOptions { return o.Raw } -// ApplyOptions executes the given ListOptionFuncs and returns the mutated -// ListOptions. -func (o *ListOptions) ApplyOptions(optFuncs []ListOptionFunc) *ListOptions { - for _, optFunc := range optFuncs { - optFunc(o) - } - return o -} - -// ListOptionFunc is a function that mutates a ListOptions struct. It implements -// the functional options pattern. See -// https://github.com/tmrts/go-patterns/blob/master/idiom/functional-options.md. -type ListOptionFunc func(*ListOptions) - // MatchingLabels is a convenience function that sets the label selector // to match the given labels, and then returns the options. // It mutates the list options. @@ -287,42 +273,20 @@ func (o *ListOptions) InNamespace(ns string) *ListOptions { return o } -// MatchingLabels is a functional option that sets the LabelSelector field of -// a ListOptions struct. -func MatchingLabels(lbls map[string]string) ListOptionFunc { - sel := labels.SelectorFromSet(lbls) - return func(opts *ListOptions) { - opts.LabelSelector = sel - } -} - -// MatchingField is a functional option that sets the FieldSelector field of -// a ListOptions struct. -func MatchingField(name, val string) ListOptionFunc { - sel := fields.SelectorFromSet(fields.Set{name: val}) - return func(opts *ListOptions) { - opts.FieldSelector = sel - } +// MatchingLabels is a convenience function that constructs list options +// to match the given labels. +func MatchingLabels(lbls map[string]string) *ListOptions { + return (&ListOptions{}).MatchingLabels(lbls) } -// InNamespace is a functional option that sets the Namespace field of -// a ListOptions struct. -func InNamespace(ns string) ListOptionFunc { - return func(opts *ListOptions) { - opts.Namespace = ns - } +// MatchingField is a convenience function that constructs list options +// to match the given field. +func MatchingField(name, val string) *ListOptions { + return (&ListOptions{}).MatchingField(name, val) } -// UseListOptions is a functional option that replaces the fields of a -// ListOptions struct with those of a different ListOptions struct. -// -// Example: -// cl.List(ctx, list, client.UseListOptions(lo.InNamespace(ns).MatchingLabels(labels))) -func UseListOptions(newOpts *ListOptions) ListOptionFunc { - return func(opts *ListOptions) { - opts.LabelSelector = newOpts.LabelSelector - opts.FieldSelector = newOpts.FieldSelector - opts.Namespace = newOpts.Namespace - opts.Raw = newOpts.Raw - } +// InNamespace is a convenience function that constructs list +// options to list in the given namespace. +func InNamespace(ns string) *ListOptions { + return (&ListOptions{}).InNamespace(ns) } diff --git a/pkg/webhook/internal/cert/writer/secret_test.go b/pkg/webhook/internal/cert/writer/secret_test.go index c784ec725d..0090f9add1 100644 --- a/pkg/webhook/internal/cert/writer/secret_test.go +++ b/pkg/webhook/internal/cert/writer/secret_test.go @@ -17,11 +17,14 @@ limitations under the License. package writer import ( + "encoding/json" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -35,6 +38,7 @@ var _ = Describe("secretCertWriter", func() { var certWriter CertWriter var sCertWriter *secretCertWriter var secret *corev1.Secret + var expectedSecret runtime.RawExtension BeforeEach(func(done Done) { var err error @@ -101,21 +105,43 @@ var _ = Describe("secretCertWriter", func() { It("should default it and return no error", func() { _, _, err := certWriter.EnsureCert(dnsName, false) Expect(err).NotTo(HaveOccurred()) - list := &corev1.SecretList{} - err = sCertWriter.Client.List(nil, list, client.InNamespace("namespace-bar")) + list := &corev1.List{} + err = sCertWriter.Client.List(nil, &client.ListOptions{ + Namespace: "namespace-bar", + Raw: &metav1.ListOptions{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "Secret", + }, + }, + }, list) Expect(err).NotTo(HaveOccurred()) Expect(list.Items).To(HaveLen(1)) }) }) Context("no existing secret", func() { + BeforeEach(func(done Done) { + j, _ := json.Marshal(secret) + expectedSecret = runtime.RawExtension{Raw: j} + close(done) + }) + It("should create new secrets with certs", func() { _, changed, err := certWriter.EnsureCert(dnsName, false) Expect(err).NotTo(HaveOccurred()) - list := &corev1.SecretList{} - err = sCertWriter.Client.List(nil, list, client.InNamespace("namespace-bar")) + list := &corev1.List{} + err = sCertWriter.Client.List(nil, &client.ListOptions{ + Namespace: "namespace-bar", + Raw: &metav1.ListOptions{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "Secret", + }, + }, + }, list) Expect(err).NotTo(HaveOccurred()) - Expect(list.Items).To(ConsistOf(*secret)) + Expect(list.Items).To(ConsistOf(expectedSecret)) Expect(list.Items).To(HaveLen(1)) Expect(changed).To(BeTrue()) }) @@ -125,6 +151,12 @@ var _ = Describe("secretCertWriter", func() { var oldSecret *corev1.Secret Context("cert is invalid", func() { + BeforeEach(func(done Done) { + j, _ := json.Marshal(secret) + expectedSecret = runtime.RawExtension{Raw: j} + close(done) + }) + Describe("cert in secret is incomplete", func() { BeforeEach(func(done Done) { oldSecret = secret.DeepCopy() @@ -136,10 +168,18 @@ var _ = Describe("secretCertWriter", func() { It("should replace with new certs", func() { _, changed, err := certWriter.EnsureCert(dnsName, false) Expect(err).NotTo(HaveOccurred()) - list := &corev1.SecretList{} - err = sCertWriter.Client.List(nil, list, client.InNamespace("namespace-bar")) + list := &corev1.List{} + err = sCertWriter.Client.List(nil, &client.ListOptions{ + Namespace: "namespace-bar", + Raw: &metav1.ListOptions{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "Secret", + }, + }, + }, list) Expect(err).NotTo(HaveOccurred()) - Expect(list.Items).To(ConsistOf(*secret)) + Expect(list.Items).To(ConsistOf(expectedSecret)) Expect(list.Items).To(HaveLen(1)) Expect(changed).To(BeTrue()) }) @@ -160,10 +200,18 @@ var _ = Describe("secretCertWriter", func() { It("should replace with new certs", func() { _, changed, err := certWriter.EnsureCert(dnsName, false) Expect(err).NotTo(HaveOccurred()) - list := &corev1.SecretList{} - err = sCertWriter.Client.List(nil, list, client.InNamespace("namespace-bar")) + list := &corev1.List{} + err = sCertWriter.Client.List(nil, &client.ListOptions{ + Namespace: "namespace-bar", + Raw: &metav1.ListOptions{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "Secret", + }, + }, + }, list) Expect(err).NotTo(HaveOccurred()) - Expect(list.Items).To(ConsistOf(*secret)) + Expect(list.Items).To(ConsistOf(expectedSecret)) Expect(list.Items).To(HaveLen(1)) Expect(changed).To(BeTrue()) }) @@ -177,6 +225,8 @@ var _ = Describe("secretCertWriter", func() { ServerKeyName: []byte(certs2.Key), ServerCertName: []byte(certs2.Cert), } + j, _ := json.Marshal(oldSecret) + expectedSecret = runtime.RawExtension{Raw: j} sCertWriter.Client = fake.NewFakeClient(oldSecret) close(done) }) @@ -189,17 +239,28 @@ var _ = Describe("secretCertWriter", func() { ServerKeyName: []byte(certs2.Key), ServerCertName: []byte(certs2.Cert), } + j, _ := json.Marshal(oldSecret) + expectedSecret = runtime.RawExtension{Raw: j} + sCertWriter.Client = fake.NewFakeClient(oldSecret) close(done) }) It("should keep the secret", func() { _, changed, err := certWriter.EnsureCert(dnsName, false) Expect(err).NotTo(HaveOccurred()) - list := &corev1.SecretList{} - err = sCertWriter.Client.List(nil, list, client.InNamespace("namespace-bar")) + list := &corev1.List{} + err = sCertWriter.Client.List(nil, &client.ListOptions{ + Namespace: "namespace-bar", + Raw: &metav1.ListOptions{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "Secret", + }, + }, + }, list) Expect(err).NotTo(HaveOccurred()) Expect(list.Items).To(HaveLen(1)) - Expect(list.Items).To(ConsistOf(*oldSecret)) + Expect(list.Items[0]).To(Equal(expectedSecret)) Expect(changed).To(BeFalse()) }) })