diff --git a/server/api/v1alpha1/workspace_types.go b/server/api/v1alpha1/workspace_types.go index 06186a77..511e2d5d 100644 --- a/server/api/v1alpha1/workspace_types.go +++ b/server/api/v1alpha1/workspace_types.go @@ -17,6 +17,7 @@ limitations under the License. package v1alpha1 import ( + workspacesv1alpha1 "github.com/konflux-workspaces/workspaces/operator/api/v1alpha1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -27,6 +28,9 @@ const ( WorkspaceVisibilityCommunity WorkspaceVisibility = "community" // WorkspaceVisibilityPrivate Private value for Workspaces visibility WorkspaceVisibilityPrivate WorkspaceVisibility = "private" + + // LabelInternalDomain if the requesting user is the owner of the workspace + LabelIsOwner string = workspacesv1alpha1.LabelInternalDomain + "is-owner" ) // WorkspaceSpec defines the desired state of Workspace diff --git a/server/persistence/mapper/internalworkspace_to_workspace_test.go b/server/persistence/mapper/internalworkspace_to_workspace_test.go index 15bc7591..d2ec8c4b 100644 --- a/server/persistence/mapper/internalworkspace_to_workspace_test.go +++ b/server/persistence/mapper/internalworkspace_to_workspace_test.go @@ -91,9 +91,11 @@ func validateMappedWorkspace(w *restworkspacesv1alpha1.Workspace, from workspace Expect(w).ToNot(BeNil()) Expect(w.GetName()).To(Equal(from.Spec.DisplayName)) Expect(w.GetNamespace()).To(Equal(from.Status.Owner.Username)) - Expect(w.GetLabels()).To(HaveKey("expected-label")) - Expect(w.GetLabels()["expected-label"]).To(Equal("not-empty")) - Expect(w.GetLabels()).NotTo(HaveKey(workspacesv1alpha1.LabelInternalDomain + "not-expected-label")) + Expect(w.GetLabels()).To(And( + HaveKeyWithValue("expected-label", "not-empty"), + Not(HaveKey(restworkspacesv1alpha1.LabelIsOwner)), + Not(HaveKey(workspacesv1alpha1.LabelInternalDomain+"not-expected-label")), + )) Expect(w.Generation).To(Equal(int64(1))) Expect(w.Spec).ToNot(BeNil()) Expect(w.Status).ToNot(BeNil()) diff --git a/server/persistence/mutate/owner.go b/server/persistence/mutate/owner.go new file mode 100644 index 00000000..09b4161b --- /dev/null +++ b/server/persistence/mutate/owner.go @@ -0,0 +1,38 @@ +/* +Copyright 2024 The Workspaces Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mutate + +import restworkspacesv1alpha1 "github.com/konflux-workspaces/workspaces/server/api/v1alpha1" + +// Applies the is-owner internal label to a workspace +func ApplyIsOwnerLabel(workspace *restworkspacesv1alpha1.Workspace, owner string) { + // do nothing on an empty workspace + if workspace == nil { + return + } + + if workspace.Labels == nil { + workspace.Labels = map[string]string{} + } + + switch workspace.Namespace { + case owner: + workspace.Labels[restworkspacesv1alpha1.LabelIsOwner] = "true" + default: + workspace.Labels[restworkspacesv1alpha1.LabelIsOwner] = "false" + } +} diff --git a/server/persistence/readclient/readclient_list.go b/server/persistence/readclient/readclient_list.go index aedfc014..69f259e3 100644 --- a/server/persistence/readclient/readclient_list.go +++ b/server/persistence/readclient/readclient_list.go @@ -12,6 +12,7 @@ import ( workspacesv1alpha1 "github.com/konflux-workspaces/workspaces/operator/api/v1alpha1" restworkspacesv1alpha1 "github.com/konflux-workspaces/workspaces/server/api/v1alpha1" "github.com/konflux-workspaces/workspaces/server/core/workspace" + "github.com/konflux-workspaces/workspaces/server/persistence/mutate" ) var _ workspace.WorkspaceLister = &ReadClient{} @@ -50,6 +51,11 @@ func (c *ReadClient) ListUserWorkspaces( // filter by namespace filterByNamespace(ww, listOpts.Namespace) + // apply is-owner label + for i := range ww.Items { + mutate.ApplyIsOwnerLabel(&ww.Items[i], user) + } + ww.DeepCopyInto(objs) return nil } diff --git a/server/persistence/readclient/readclient_list_test.go b/server/persistence/readclient/readclient_list_test.go index 9893660c..8bd0958e 100644 --- a/server/persistence/readclient/readclient_list_test.go +++ b/server/persistence/readclient/readclient_list_test.go @@ -25,6 +25,7 @@ var _ = Describe("List", func() { var frc *mocks.MockFakeIWReadClient var mp *mocks.MockFakeIWMapper var rc *readclient.ReadClient + user := "user" BeforeEach(func() { ctx = context.Background() @@ -40,13 +41,11 @@ var _ = Describe("List", func() { DescribeTable("Filter by label", func(unfilteredInternalWorkspaces []workspacesv1alpha1.InternalWorkspace, expectedObjectMetas []metav1.ObjectMeta) { // given - user := "user" - // internalLabel := workspacesv1alpha1.LabelInternalDomain + "whatever" // internal client expected to be called once. // It returns no error so we can test the filtering by label. frc.EXPECT(). - ListAsUser(gomock.Any(), gomock.Any(), gomock.Any()). + ListAsUser(ctx, user, gomock.Any()). DoAndReturn(func(_ context.Context, _ string, iww *workspacesv1alpha1.InternalWorkspaceList) error { iww.Items = unfilteredInternalWorkspaces return nil @@ -111,7 +110,8 @@ var _ = Describe("List", func() { Name: "hit", Namespace: "hit", Labels: map[string]string{ - "hit": "hit", + "hit": "hit", + restworkspacesv1alpha1.LabelIsOwner: "false", }, }, }), @@ -148,14 +148,36 @@ var _ = Describe("List", func() { Name: "hit-1", Namespace: "hit-1", Labels: map[string]string{ - "hit": "hit", + "hit": "hit", + restworkspacesv1alpha1.LabelIsOwner: "false", }, }, { Name: "hit-2", Namespace: "hit-2", Labels: map[string]string{ - "hit": "hit", + "hit": "hit", + restworkspacesv1alpha1.LabelIsOwner: "false", + }, + }, + }), + Entry("one owned workspace", []workspacesv1alpha1.InternalWorkspace{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "hit", + Namespace: user, + Labels: map[string]string{ + "hit": "hit", + }, + }, + }, + }, []metav1.ObjectMeta{ + { + Name: "hit", + Namespace: user, + Labels: map[string]string{ + "hit": "hit", + restworkspacesv1alpha1.LabelIsOwner: "true", }, }, }), @@ -164,12 +186,12 @@ var _ = Describe("List", func() { DescribeTable("InternalClient returns an error", func(rerr error, expectedErrorFunc func(error) bool) { // given frc.EXPECT(). - ListAsUser(gomock.Any(), gomock.Any(), gomock.Any()). + ListAsUser(ctx, user, gomock.Any()). Return(rerr). Times(1) // when - err := rc.ListUserWorkspaces(ctx, "", nil) + err := rc.ListUserWorkspaces(ctx, user, nil) // then Expect(err).To(HaveOccurred()) @@ -178,15 +200,62 @@ var _ = Describe("List", func() { Entry("unauthorized -> internal error", iwclient.ErrUnauthorized, kerrors.IsInternalError), ) + It("should pass along owned workspaces", func() { + wslist := restworkspacesv1alpha1.WorkspaceList{} + frc.EXPECT(). + ListAsUser(ctx, user, gomock.Any()). + Do(func(_ context.Context, user string, ws *workspacesv1alpha1.InternalWorkspaceList) { + ws.Items = []workspacesv1alpha1.InternalWorkspace{ + { + Spec: workspacesv1alpha1.InternalWorkspaceSpec{ + DisplayName: "default", + }, + Status: workspacesv1alpha1.InternalWorkspaceStatus{ + Owner: workspacesv1alpha1.UserInfoStatus{ + Username: user, + }, + }, + }, + } + }). + Return(nil). + Times(1) + + mp.EXPECT(). + InternalWorkspaceListToWorkspaceList(gomock.Any()). + DoAndReturn(func(_ *workspacesv1alpha1.InternalWorkspaceList) (*restworkspacesv1alpha1.WorkspaceList, error) { + return &restworkspacesv1alpha1.WorkspaceList{ + Items: []restworkspacesv1alpha1.Workspace{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Namespace: user, + }, + }, + }, + }, nil + }). + Times(1) + + // when + err := rc.ListUserWorkspaces(ctx, user, &wslist) + + // then + Expect(err).NotTo(HaveOccurred()) + Expect(wslist.Items).To(HaveLen(1)) + Expect(wslist.Items[0].GetName()).To(Equal("default")) + Expect(wslist.Items[0].GetNamespace()).To(Equal(user)) + Expect(wslist.Items[0].GetLabels()).To(HaveKeyWithValue(restworkspacesv1alpha1.LabelIsOwner, "true")) + }) + It("handles mapper error and returns InternalError", func() { // given merr := fmt.Errorf("mapper error") - user := "user" // internal client expected to be called once. // It returns no error so we can test the filtering by label. frc.EXPECT(). - ListAsUser(gomock.Any(), gomock.Any(), gomock.Any()). + ListAsUser(ctx, user, gomock.Any()). Return(nil). Times(1) @@ -208,13 +277,12 @@ var _ = Describe("List", func() { Describe("ListOptions are mapped", func() { It("returns an error if labels with reserved domain are used", func() { // given - user := "user" internalLabel := workspacesv1alpha1.LabelInternalDomain + "whatever" // internal client expected to be called once. // It returns no error so we can test the filtering by label. frc.EXPECT(). - ListAsUser(gomock.Any(), gomock.Any(), gomock.Any()). + ListAsUser(ctx, user, gomock.Any()). Return(nil). Times(1) diff --git a/server/persistence/readclient/readclient_read.go b/server/persistence/readclient/readclient_read.go index efcf1362..58d33c3f 100644 --- a/server/persistence/readclient/readclient_read.go +++ b/server/persistence/readclient/readclient_read.go @@ -9,6 +9,7 @@ import ( "github.com/konflux-workspaces/workspaces/server/core/workspace" "github.com/konflux-workspaces/workspaces/server/log" "github.com/konflux-workspaces/workspaces/server/persistence/iwclient" + "github.com/konflux-workspaces/workspaces/server/persistence/mutate" workspacesv1alpha1 "github.com/konflux-workspaces/workspaces/operator/api/v1alpha1" restworkspacesv1alpha1 "github.com/konflux-workspaces/workspaces/server/api/v1alpha1" @@ -39,6 +40,9 @@ func (c *ReadClient) ReadUserWorkspace( return kerrors.NewInternalError(err) } + // apply is-owner label + mutate.ApplyIsOwnerLabel(r, user) + // return workspace r.DeepCopyInto(obj) return nil diff --git a/server/persistence/readclient/readclient_read_test.go b/server/persistence/readclient/readclient_read_test.go index 75f58d6a..10eabb22 100644 --- a/server/persistence/readclient/readclient_read_test.go +++ b/server/persistence/readclient/readclient_read_test.go @@ -34,47 +34,90 @@ var _ = Describe("Read", func() { rc = readclient.New(frc, mp) }) - // happy path - It("returns a copy of the mapped value", func() { - // given - - // internal client expected to be called once. - // It returns no error so we can test the mapper invocation. - frc.EXPECT(). - GetAsUser(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). - Return(nil). - Times(1) - - // mapper expects to be called once. - // It returns a valid workspace so we can test handler's result. - mappedWorkspace := restworkspacesv1alpha1.Workspace{ - ObjectMeta: metav1.ObjectMeta{ - Name: "workspace", - Namespace: "owner", - }, - Spec: restworkspacesv1alpha1.WorkspaceSpec{ - Visibility: restworkspacesv1alpha1.WorkspaceVisibilityCommunity, - }, - Status: restworkspacesv1alpha1.WorkspaceStatus{ - Space: &restworkspacesv1alpha1.SpaceInfo{ - Name: "space", + Describe("valid request", func() { + // happy path + It("returns a copy of the mapped value", func() { + // given + + // internal client expected to be called once. + // It returns no error so we can test the mapper invocation. + frc.EXPECT(). + GetAsUser(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil). + Times(1) + + // mapper expects to be called once. + // It returns a valid workspace so we can test handler's result. + mappedWorkspace := restworkspacesv1alpha1.Workspace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "workspace", + Namespace: "owner", }, - }, - } - mp.EXPECT(). - InternalWorkspaceToWorkspace(gomock.Any()). - Return(&mappedWorkspace, nil). - Times(1) - - // when - returnedWorkspace := restworkspacesv1alpha1.Workspace{} - err := rc.ReadUserWorkspace(ctx, "", "", "", &returnedWorkspace) - - // then - Expect(err).NotTo(HaveOccurred()) - Expect(returnedWorkspace).To(Equal(mappedWorkspace)) - // test data is deepcopied - Expect(returnedWorkspace).NotTo(BeIdenticalTo(mappedWorkspace)) + Spec: restworkspacesv1alpha1.WorkspaceSpec{ + Visibility: restworkspacesv1alpha1.WorkspaceVisibilityCommunity, + }, + Status: restworkspacesv1alpha1.WorkspaceStatus{ + Space: &restworkspacesv1alpha1.SpaceInfo{ + Name: "space", + }, + }, + } + mp.EXPECT(). + InternalWorkspaceToWorkspace(gomock.Any()). + Return(&mappedWorkspace, nil). + Times(1) + + // when + returnedWorkspace := restworkspacesv1alpha1.Workspace{} + err := rc.ReadUserWorkspace(ctx, "", "", "", &returnedWorkspace) + + // then + Expect(err).NotTo(HaveOccurred()) + Expect(returnedWorkspace).To(Equal(mappedWorkspace)) + // test data is deepcopied + Expect(returnedWorkspace).NotTo(BeIdenticalTo(mappedWorkspace)) + }) + + DescribeTable("should set the is-owner label on owned workspaces", func(owner, is_owned string) { + // internal client expected to be called once. + // It returns no error so we can test the mapper invocation. + frc.EXPECT(). + GetAsUser(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil). + Times(1) + + // mapper expects to be called once. + // It returns a valid workspace so we can test handler's result. + mappedWorkspace := restworkspacesv1alpha1.Workspace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "workspace", + Namespace: "owner", + }, + Spec: restworkspacesv1alpha1.WorkspaceSpec{ + Visibility: restworkspacesv1alpha1.WorkspaceVisibilityCommunity, + }, + Status: restworkspacesv1alpha1.WorkspaceStatus{ + Space: &restworkspacesv1alpha1.SpaceInfo{ + Name: "space", + }, + }, + } + mp.EXPECT(). + InternalWorkspaceToWorkspace(gomock.Any()). + Return(&mappedWorkspace, nil). + Times(1) + + // when + returnedWorkspace := restworkspacesv1alpha1.Workspace{} + err := rc.ReadUserWorkspace(ctx, owner, "", "", &returnedWorkspace) + + // then + Expect(err).NotTo(HaveOccurred()) + Expect(returnedWorkspace.Labels).To(HaveKeyWithValue(restworkspacesv1alpha1.LabelIsOwner, is_owned)) + }, + Entry("non-owner", "another", "false"), + Entry("owner", "owner", "true"), + ) }) // error handling diff --git a/server/persistence/writeclient/writeclient_create.go b/server/persistence/writeclient/writeclient_create.go index 7808c20c..8e2a1c00 100644 --- a/server/persistence/writeclient/writeclient_create.go +++ b/server/persistence/writeclient/writeclient_create.go @@ -8,6 +8,7 @@ import ( "github.com/konflux-workspaces/workspaces/server/core/workspace" "github.com/konflux-workspaces/workspaces/server/log" "github.com/konflux-workspaces/workspaces/server/persistence/mapper" + "github.com/konflux-workspaces/workspaces/server/persistence/mutate" restworkspacesv1alpha1 "github.com/konflux-workspaces/workspaces/server/api/v1alpha1" ) @@ -42,6 +43,9 @@ func (c *WriteClient) CreateUserWorkspace(ctx context.Context, user string, work return err } + // apply the is-owner label + mutate.ApplyIsOwnerLabel(w, user) + // fill return value w.DeepCopyInto(workspace) return nil diff --git a/server/persistence/writeclient/writeclient_create_test.go b/server/persistence/writeclient/writeclient_create_test.go index cb2d59ec..13289272 100644 --- a/server/persistence/writeclient/writeclient_create_test.go +++ b/server/persistence/writeclient/writeclient_create_test.go @@ -85,7 +85,7 @@ var _ = Describe("WriteclientCreate", func() { }) }) - When("creating a community workspace", func() { + When("creating a private workspace", func() { It("should create workspaces using the given client", func() { // given workspace.Spec.Visibility = restworkspacesv1alpha1.WorkspaceVisibilityPrivate @@ -98,4 +98,19 @@ var _ = Describe("WriteclientCreate", func() { validateCreatedInternalWorkspace(&workspace, workspacesv1alpha1.InternalWorkspaceVisibilityPrivate) }) }) + + When("creating an owned workspace", func() { + It("should have the is-owner label", func() { + // given + workspace.Spec.Visibility = restworkspacesv1alpha1.WorkspaceVisibilityPrivate + + // when + err := cli.CreateUserWorkspace(ctx, "owner", &workspace) + + // then + Expect(err).NotTo(HaveOccurred()) + Expect(workspace.Labels).To(HaveKeyWithValue(restworkspacesv1alpha1.LabelIsOwner, "true")) + validateCreatedInternalWorkspace(&workspace, workspacesv1alpha1.InternalWorkspaceVisibilityPrivate) + }) + }) }) diff --git a/server/persistence/writeclient/writeclient_update.go b/server/persistence/writeclient/writeclient_update.go index 116b2360..b6ac3610 100644 --- a/server/persistence/writeclient/writeclient_update.go +++ b/server/persistence/writeclient/writeclient_update.go @@ -10,6 +10,7 @@ import ( "github.com/konflux-workspaces/workspaces/server/log" "github.com/konflux-workspaces/workspaces/server/persistence/iwclient" "github.com/konflux-workspaces/workspaces/server/persistence/mapper" + "github.com/konflux-workspaces/workspaces/server/persistence/mutate" workspacesv1alpha1 "github.com/konflux-workspaces/workspaces/operator/api/v1alpha1" restworkspacesv1alpha1 "github.com/konflux-workspaces/workspaces/server/api/v1alpha1" @@ -48,5 +49,18 @@ func (c *WriteClient) UpdateUserWorkspace(ctx context.Context, user string, work // update the InternalWorkspace ciw.Spec.Visibility = iw.Spec.Visibility log.FromContext(ctx).Debug("updating user workspace", "workspace", iw, "user", user) - return cli.Update(ctx, &ciw, opts...) + err = cli.Update(ctx, &ciw, opts...) + if err != nil { + return err + } + + ws, err := mapper.Default.InternalWorkspaceToWorkspace(&ciw) + if err != nil { + return err + } + + mutate.ApplyIsOwnerLabel(ws, user) + + ws.DeepCopyInto(workspace) + return nil } diff --git a/server/persistence/writeclient/writeclient_update_test.go b/server/persistence/writeclient/writeclient_update_test.go index 5f72ab23..b6436481 100644 --- a/server/persistence/writeclient/writeclient_update_test.go +++ b/server/persistence/writeclient/writeclient_update_test.go @@ -177,6 +177,8 @@ var _ = Describe("WriteclientUpdate", func() { // then Expect(err).NotTo(HaveOccurred()) + + Expect(w.Labels).To(HaveKeyWithValue(restworkspacesv1alpha1.LabelIsOwner, "true")) }) }) })