Skip to content

Commit

Permalink
add is-owner label (konflux-workspaces#300)
Browse files Browse the repository at this point in the history
When requesting workspaces through our server, we will now label them
with the label `internal.workspaces.konflux-ci.dev/is-owner`.  This
label will either be set to `true` or `false` depending on whether the
requesting user is an owner or not.

This label is set for all operations - get, list, create, and update.

Signed-off-by: Andy Sadler <ansadler@redhat.com>
  • Loading branch information
sadlerap authored and filariow committed Oct 4, 2024
1 parent 37ebe2b commit fd10eb6
Show file tree
Hide file tree
Showing 11 changed files with 257 additions and 57 deletions.
4 changes: 4 additions & 0 deletions server/api/v1alpha1/workspace_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
38 changes: 38 additions & 0 deletions server/persistence/mutate/owner.go
Original file line number Diff line number Diff line change
@@ -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"
}
}
6 changes: 6 additions & 0 deletions server/persistence/readclient/readclient_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down Expand Up @@ -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
}
Expand Down
92 changes: 80 additions & 12 deletions server/persistence/readclient/readclient_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
Expand Down Expand Up @@ -111,7 +110,8 @@ var _ = Describe("List", func() {
Name: "hit",
Namespace: "hit",
Labels: map[string]string{
"hit": "hit",
"hit": "hit",
restworkspacesv1alpha1.LabelIsOwner: "false",
},
},
}),
Expand Down Expand Up @@ -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",
},
},
}),
Expand All @@ -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())
Expand All @@ -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)

Expand All @@ -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)

Expand Down
4 changes: 4 additions & 0 deletions server/persistence/readclient/readclient_read.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit fd10eb6

Please sign in to comment.