From 9e176957f3731133d108c3c2258bef2b22a3a297 Mon Sep 17 00:00:00 2001 From: Rafaela Soares <119665479+rsoaresd@users.noreply.github.com> Date: Wed, 24 Jul 2024 09:55:18 +0100 Subject: [PATCH] ban funcs from toolchain-common (#52) Co-authored-by: Matous Jobanek --- go.mod | 4 ++-- go.sum | 8 ++++---- pkg/cmd/ban.go | 50 +++++++-------------------------------------- pkg/cmd/ban_test.go | 39 +++++++++++++++++++++++++++++++++++ 4 files changed, 52 insertions(+), 49 deletions(-) diff --git a/go.mod b/go.mod index af5f3b3..3295c8b 100644 --- a/go.mod +++ b/go.mod @@ -3,8 +3,8 @@ module github.com/kubesaw/ksctl go 1.20 require ( - github.com/codeready-toolchain/api v0.0.0-20240708122235-0af5a9a178bb - github.com/codeready-toolchain/toolchain-common v0.0.0-20240716065433-8604fe46b96a + github.com/codeready-toolchain/api v0.0.0-20240717145630-bb67a632867a + github.com/codeready-toolchain/toolchain-common v0.0.0-20240719075302-e22862787165 github.com/ghodss/yaml v1.0.0 github.com/mitchellh/go-homedir v1.1.0 // using latest commit from 'github.com/openshift/api branch release-4.12' diff --git a/go.sum b/go.sum index 7a8cb7f..92cd63c 100644 --- a/go.sum +++ b/go.sum @@ -133,10 +133,10 @@ github.com/cockroachdb/datadriven v0.0.0-20190809214429-80d97fb3cbaa/go.mod h1:z github.com/cockroachdb/datadriven v0.0.0-20200714090401-bf6692d28da5/go.mod h1:h6jFvWxBdQXxjopDMZyH2UVceIRfR84bdzbkoKrsWNo= github.com/cockroachdb/errors v1.2.4/go.mod h1:rQD95gz6FARkaKkQXUksEje/d9a6wBJoCr5oaCLELYA= github.com/cockroachdb/logtags v0.0.0-20190617123548-eb05cc24525f/go.mod h1:i/u985jwjWRlyHXQbwatDASoW0RMlZ/3i9yJHE2xLkI= -github.com/codeready-toolchain/api v0.0.0-20240708122235-0af5a9a178bb h1:Wc9CMsv0ODZv9dM5qF3OI0mFDO95YNIXV/8oRvoz8aE= -github.com/codeready-toolchain/api v0.0.0-20240708122235-0af5a9a178bb/go.mod h1:ie9p4LenCCS0LsnbWp6/xwpFDdCWYE0KWzUO6Sk1g0E= -github.com/codeready-toolchain/toolchain-common v0.0.0-20240716065433-8604fe46b96a h1:HcaJtZCLfYkWZCxIa3iTvq3zgn711JGqPLkunBTfGSc= -github.com/codeready-toolchain/toolchain-common v0.0.0-20240716065433-8604fe46b96a/go.mod h1:8M9k7w2VSyRKSK6P08Jo2ddW3uyGgxCcSitnYa3HK9o= +github.com/codeready-toolchain/api v0.0.0-20240717145630-bb67a632867a h1:La7GOCysmkU+4vnN8lDzXFJwJiA1LWZ9YkX/yQXYnpw= +github.com/codeready-toolchain/api v0.0.0-20240717145630-bb67a632867a/go.mod h1:ie9p4LenCCS0LsnbWp6/xwpFDdCWYE0KWzUO6Sk1g0E= +github.com/codeready-toolchain/toolchain-common v0.0.0-20240719075302-e22862787165 h1:isSKa0hIpSXfrQWWtuxJNXmBcOeobBflPzCvJuMkpQY= +github.com/codeready-toolchain/toolchain-common v0.0.0-20240719075302-e22862787165/go.mod h1:LGk4lAYtvYPXgopPW65aoa7c5KiKq6Ssuytgckc31Wk= github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk= github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE= github.com/coreos/etcd v3.3.13+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE= diff --git a/pkg/cmd/ban.go b/pkg/cmd/ban.go index deb4954..4697061 100644 --- a/pkg/cmd/ban.go +++ b/pkg/cmd/ban.go @@ -2,17 +2,15 @@ package cmd import ( "context" - "fmt" toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" + "github.com/codeready-toolchain/toolchain-common/pkg/banneduser" "github.com/kubesaw/ksctl/pkg/client" "github.com/kubesaw/ksctl/pkg/configuration" clicontext "github.com/kubesaw/ksctl/pkg/context" "github.com/kubesaw/ksctl/pkg/ioutils" "github.com/spf13/cobra" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - runtimeclient "sigs.k8s.io/controller-runtime/pkg/client" ) func NewBanCmd() *cobra.Command { @@ -70,25 +68,23 @@ func CreateBannedUser(ctx *clicontext.CommandContext, userSignupName string, con return err } - bannedUser, err := newBannedUser(userSignup, ksctlConfig.Name) + bannedUser, err := banneduser.NewBannedUser(userSignup, ksctlConfig.Name) if err != nil { return err } - emailHashLabelMatch := runtimeclient.MatchingLabels(map[string]string{ - toolchainv1alpha1.BannedUserEmailHashLabelKey: bannedUser.Labels[toolchainv1alpha1.BannedUserEmailHashLabelKey], - }) - bannedUsers := &toolchainv1alpha1.BannedUserList{} - if err := cl.List(context.TODO(), bannedUsers, emailHashLabelMatch, runtimeclient.InNamespace(cfg.OperatorNamespace)); err != nil { + alreadyBannedUser, err := banneduser.GetBannedUser(ctx, bannedUser.Labels[toolchainv1alpha1.BannedUserEmailHashLabelKey], cl, cfg.OperatorNamespace) + if err != nil { return err } if err := ctx.PrintObject(userSignup, "UserSignup to be banned"); err != nil { return err } - if len(bannedUsers.Items) > 0 { + + if alreadyBannedUser != nil { ctx.Println("The user was already banned - there is a BannedUser resource with the same labels already present") - return ctx.PrintObject(&bannedUsers.Items[0], "BannedUser resource") + return ctx.PrintObject(alreadyBannedUser, "BannedUser resource") } if shouldCreate, err := confirm(userSignup, bannedUser); !shouldCreate || err != nil { @@ -102,35 +98,3 @@ func CreateBannedUser(ctx *clicontext.CommandContext, userSignupName string, con ctx.Printlnf("\nUserSignup has been banned by creating BannedUser resource with name " + bannedUser.Name) return nil } - -func newBannedUser(userSignup *toolchainv1alpha1.UserSignup, bannedBy string) (*toolchainv1alpha1.BannedUser, error) { - var emailHashLbl, phoneHashLbl string - var exists bool - - if userSignup.Spec.IdentityClaims.Email == "" { - return nil, fmt.Errorf("the UserSignup doesn't have email set") - } - - if emailHashLbl, exists = userSignup.Labels[toolchainv1alpha1.UserSignupUserEmailHashLabelKey]; !exists { - return nil, fmt.Errorf("the UserSignup doesn't have the label '%s' set", toolchainv1alpha1.UserSignupUserEmailHashLabelKey) - } - - bannedUser := &toolchainv1alpha1.BannedUser{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: userSignup.Namespace, - GenerateName: "banneduser-", - Labels: map[string]string{ - toolchainv1alpha1.BannedUserEmailHashLabelKey: emailHashLbl, - BannedByLabel: bannedBy, - }, - }, - Spec: toolchainv1alpha1.BannedUserSpec{ - Email: userSignup.Spec.IdentityClaims.Email, - }, - } - - if phoneHashLbl, exists = userSignup.Labels[toolchainv1alpha1.UserSignupUserPhoneHashLabelKey]; exists { - bannedUser.Labels[toolchainv1alpha1.BannedUserPhoneNumberHashLabelKey] = phoneHashLbl - } - return bannedUser, nil -} diff --git a/pkg/cmd/ban_test.go b/pkg/cmd/ban_test.go index 244676b..d3b59a5 100644 --- a/pkg/cmd/ban_test.go +++ b/pkg/cmd/ban_test.go @@ -2,6 +2,7 @@ package cmd_test import ( "context" + "errors" "fmt" "testing" @@ -13,6 +14,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "sigs.k8s.io/controller-runtime/pkg/client" runtimeclient "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -205,6 +207,43 @@ func TestCreateBannedUser(t *testing.T) { require.EqualError(t, err, "some error") AssertNoBannedUser(t, fakeClient, userSignup) }) + + t.Run("GetBannedUser call fails", func(t *testing.T) { + //given + userSignup := NewUserSignup() + newClient, fakeClient := NewFakeClients(t, userSignup) + term := NewFakeTerminal() + ctx := clicontext.NewCommandContext(term, newClient) + + fakeClient.MockList = func(ctx context.Context, list runtimeclient.ObjectList, opts ...client.ListOption) error { + return errors.New("something went wrong listing the banned users") + } + + // when + err := cmd.CreateBannedUser(ctx, userSignup.Name, func(signup *toolchainv1alpha1.UserSignup, bannedUser *toolchainv1alpha1.BannedUser) (bool, error) { + return true, nil + }) + + // then + require.Error(t, err, "something went wrong listing the banned users") + }) + t.Run("NewBannedUser call fails", func(t *testing.T) { + //given + userSignup := NewUserSignup() + userSignup.Labels = nil + newClient, fakeClient := NewFakeClients(t, userSignup) + term := NewFakeTerminal() + ctx := clicontext.NewCommandContext(term, newClient) + + // when + err := cmd.CreateBannedUser(ctx, userSignup.Name, func(signup *toolchainv1alpha1.UserSignup, bannedUser *toolchainv1alpha1.BannedUser) (bool, error) { + return true, nil + }) + + // then + require.Error(t, err, "userSignup doesn't have UserSignupUserEmailHashLabelKey") + AssertNoBannedUser(t, fakeClient, userSignup) + }) } func TestCreateBannedUserLacksPermissions(t *testing.T) {