From da95fdffd8ab5f9c43687fcaa84cce1f850783fe Mon Sep 17 00:00:00 2001 From: Fabian Fischer Date: Wed, 5 Jan 2022 11:18:36 +0100 Subject: [PATCH] Test and refactor List and Watch --- apiserver/organization/organization.go | 66 ------- apiserver/organization/organization_list.go | 83 +++++++++ .../organization/organization_list_test.go | 165 ++++++++++++++++++ apiserver/organization/organization_test.go | 21 +++ 4 files changed, 269 insertions(+), 66 deletions(-) create mode 100644 apiserver/organization/organization_list.go create mode 100644 apiserver/organization/organization_list_test.go diff --git a/apiserver/organization/organization.go b/apiserver/organization/organization.go index 1183c091..f37027d6 100644 --- a/apiserver/organization/organization.go +++ b/apiserver/organization/organization.go @@ -7,15 +7,10 @@ import ( orgv1 "github.com/appuio/control-api/apis/organization/v1" - corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" - metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/selection" - "k8s.io/apimachinery/pkg/watch" genericregistry "k8s.io/apiserver/pkg/registry/generic" "k8s.io/apiserver/pkg/registry/rest" restbuilder "sigs.k8s.io/apiserver-runtime/pkg/builder/rest" @@ -79,35 +74,6 @@ func (s *organizationStorage) Create(ctx context.Context, obj runtime.Object, cr return org, nil } -var _ rest.Lister = &organizationStorage{} - -func (s organizationStorage) NewList() runtime.Object { - return &orgv1.OrganizationList{} -} - -func (s *organizationStorage) List(ctx context.Context, options *metainternalversion.ListOptions) (runtime.Object, error) { - orgNamspace, err := labels.NewRequirement(orgv1.TypeKey, selection.Equals, []string{orgv1.OrgType}) - if err != nil { - return nil, err - } - options.LabelSelector = options.LabelSelector.Add(*orgNamspace) - namespaces, err := s.namepaces.ListNamespaces(ctx, options) - if err != nil { - return nil, err - } - - res := orgv1.OrganizationList{ - ListMeta: namespaces.ListMeta, - Items: []orgv1.Organization{}, - } - - for _, n := range namespaces.Items { - res.Items = append(res.Items, *orgv1.NewOrganizationFromNS(&n)) - } - - return &res, nil -} - var _ rest.Updater = &organizationStorage{} var _ rest.CreaterUpdater = &organizationStorage{} @@ -162,38 +128,6 @@ func (s *organizationStorage) Delete(ctx context.Context, name string, deleteVal return orgv1.NewOrganizationFromNS(ns), false, convertNamespaceError(err) } -var _ rest.Watcher = &organizationStorage{} - -func (s *organizationStorage) Watch(ctx context.Context, options *metainternalversion.ListOptions) (watch.Interface, error) { - orgNamspace, err := labels.NewRequirement(orgv1.TypeKey, selection.Equals, []string{orgv1.OrgType}) - if err != nil { - return nil, err - } - options.LabelSelector = options.LabelSelector.Add(*orgNamspace) - - nsWatcher, err := s.namepaces.WatchNamespaces(ctx, options) - if err != nil { - return nil, err - } - - return watch.Filter(nsWatcher, func(in watch.Event) (out watch.Event, keep bool) { - if in.Object == nil { - // This should never happen, let downstream deal with it - return in, true - } - ns, ok := in.Object.(*corev1.Namespace) - if !ok { - // We received a non Namespace object - // This is most likely an error so we pass it on - return in, true - } - - in.Object = orgv1.NewOrganizationFromNS(ns) - - return in, true - }), nil -} - func convertNamespaceError(err error) error { groupResource := schema.GroupResource{ Group: orgv1.GroupVersion.Group, diff --git a/apiserver/organization/organization_list.go b/apiserver/organization/organization_list.go new file mode 100644 index 00000000..aaa77c24 --- /dev/null +++ b/apiserver/organization/organization_list.go @@ -0,0 +1,83 @@ +package organization + +import ( + "context" + + orgv1 "github.com/appuio/control-api/apis/organization/v1" + + corev1 "k8s.io/api/core/v1" + metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/selection" + "k8s.io/apimachinery/pkg/watch" + "k8s.io/apiserver/pkg/registry/rest" +) + +var _ rest.Lister = &organizationStorage{} + +func (s organizationStorage) NewList() runtime.Object { + return &orgv1.OrganizationList{} +} + +func (s *organizationStorage) List(ctx context.Context, options *metainternalversion.ListOptions) (runtime.Object, error) { + namespaces, err := s.namepaces.ListNamespaces(ctx, extendNamespaceListOption(options)) + if err != nil { + return nil, convertNamespaceError(err) + } + + res := orgv1.OrganizationList{ + ListMeta: namespaces.ListMeta, + Items: []orgv1.Organization{}, + } + + for _, n := range namespaces.Items { + res.Items = append(res.Items, *orgv1.NewOrganizationFromNS(&n)) + } + + return &res, nil +} + +var _ rest.Watcher = &organizationStorage{} + +func (s *organizationStorage) Watch(ctx context.Context, options *metainternalversion.ListOptions) (watch.Interface, error) { + + nsWatcher, err := s.namepaces.WatchNamespaces(ctx, extendNamespaceListOption(options)) + if err != nil { + return nil, convertNamespaceError(err) + } + + return watch.Filter(nsWatcher, func(in watch.Event) (out watch.Event, keep bool) { + if in.Object == nil { + // This should never happen, let downstream deal with it + return in, true + } + ns, ok := in.Object.(*corev1.Namespace) + if !ok { + // We received a non Namespace object + // This is most likely an error so we pass it on + return in, true + } + + in.Object = orgv1.NewOrganizationFromNS(ns) + + return in, true + }), nil +} + +func extendNamespaceListOption(options *metainternalversion.ListOptions) *metainternalversion.ListOptions { + orgNamspace, err := labels.NewRequirement(orgv1.TypeKey, selection.Equals, []string{orgv1.OrgType}) + if err != nil { + // The input is static. This call will only fail during development. + panic(err) + } + if options == nil { + options = &metainternalversion.ListOptions{} + } + if options.LabelSelector == nil { + options.LabelSelector = labels.NewSelector() + } + options.LabelSelector = options.LabelSelector.Add(*orgNamspace) + + return options +} diff --git a/apiserver/organization/organization_list_test.go b/apiserver/organization/organization_list_test.go new file mode 100644 index 00000000..7fff01ff --- /dev/null +++ b/apiserver/organization/organization_list_test.go @@ -0,0 +1,165 @@ +package organization + +import ( + "context" + "testing" + + mock "github.com/appuio/control-api/apiserver/organization/mock" + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + orgv1 "github.com/appuio/control-api/apis/organization/v1" + + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/watch" +) + +func TestOrganizationStorage_List(t *testing.T) { + tests := map[string]struct { + namespaces *corev1.NamespaceList + namespaceErr error + + organizations *orgv1.OrganizationList + err error + }{ + "GivenList_ThenSucceed": { + namespaces: &corev1.NamespaceList{ + Items: []corev1.Namespace{ + *fooNs, + *barNs, + }, + }, + + organizations: &orgv1.OrganizationList{ + Items: []orgv1.Organization{ + *fooOrg, + *barOrg, + }, + }, + }, + "GivenErrNotFound_ThenErrNotFound": { + namespaceErr: apierrors.NewNotFound(schema.GroupResource{ + Resource: "namepaces", + }, "not-found"), + err: apierrors.NewNotFound(schema.GroupResource{ + Group: orgv1.GroupVersion.Group, + Resource: "organizations", + }, "not-found"), + }, + } + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mnp := mock.NewMocknamespaceProvider(ctrl) + os := organizationStorage{ + namepaces: mnp, + } + + for n, tc := range tests { + t.Run(n, func(t *testing.T) { + mnp.EXPECT(). + ListNamespaces(gomock.Any(), gomock.Any()). + Return(tc.namespaces, tc.namespaceErr). + Times(1) + org, err := os.List(context.TODO(), nil) + if tc.err != nil { + assert.EqualError(t, err, tc.err.Error()) + return + } + require.NoError(t, err) + assert.Equal(t, tc.organizations, org) + }) + } +} + +type testWatcher struct { + events chan watch.Event +} + +func (w testWatcher) Stop() {} + +func (w testWatcher) ResultChan() <-chan watch.Event { + return w.events +} + +func TestOrganizationStorage_Watch(t *testing.T) { + tests := map[string]struct { + namespacesEvents []watch.Event + namespaceErr error + + organizationEvents []watch.Event + err error + }{ + "GivenNsEvents_ThenOrgEvents": { + namespacesEvents: []watch.Event{ + { + Type: watch.Added, + Object: fooNs, + }, + { + Type: watch.Modified, + Object: barNs, + }, + }, + organizationEvents: []watch.Event{ + { + Type: watch.Added, + Object: fooOrg, + }, + { + Type: watch.Modified, + Object: barOrg, + }, + }, + }, + "GivenErrNotFound_ThenErrNotFound": { + namespaceErr: apierrors.NewNotFound(schema.GroupResource{ + Resource: "namepaces", + }, "not-found"), + err: apierrors.NewNotFound(schema.GroupResource{ + Group: orgv1.GroupVersion.Group, + Resource: "organizations", + }, "not-found"), + }, + } + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mnp := mock.NewMocknamespaceProvider(ctrl) + os := organizationStorage{ + namepaces: mnp, + } + + for n, tc := range tests { + t.Run(n, func(t *testing.T) { + nsWatcher := testWatcher{ + events: make(chan watch.Event, len(tc.namespacesEvents)), + } + for _, e := range tc.namespacesEvents { + nsWatcher.events <- e + } + close(nsWatcher.events) + mnp.EXPECT(). + WatchNamespaces(gomock.Any(), gomock.Any()). + Return(nsWatcher, tc.namespaceErr). + Times(1) + orgWatch, err := os.Watch(context.TODO(), nil) + if tc.err != nil { + assert.EqualError(t, err, tc.err.Error()) + return + } + require.NoError(t, err) + orgEvents := []watch.Event{} + for e := range orgWatch.ResultChan() { + orgEvents = append(orgEvents, e) + } + assert.Equal(t, tc.organizationEvents, orgEvents) + }) + } + +} diff --git a/apiserver/organization/organization_test.go b/apiserver/organization/organization_test.go index 14a873d9..ace22f35 100644 --- a/apiserver/organization/organization_test.go +++ b/apiserver/organization/organization_test.go @@ -232,6 +232,27 @@ var ( }, }, } + barOrg = &orgv1.Organization{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + Labels: map[string]string{}, + Annotations: map[string]string{}, + }, + Spec: orgv1.OrganizationSpec{ + DisplayName: "Bar Gmbh.", + }, + } + barNs = &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + Labels: map[string]string{ + orgv1.TypeKey: orgv1.OrgType, + }, + Annotations: map[string]string{ + orgv1.DisplayNameKey: "Bar Gmbh.", + }, + }, + } defaultNs = &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: "default",