From c11f3a834280d3fa418637c99ce0f4a01ca9c912 Mon Sep 17 00:00:00 2001 From: Matous Jobanek Date: Thu, 3 Oct 2024 13:23:51 +0200 Subject: [PATCH] drop CRTClient abstraction (#473) --- cmd/main.go | 5 +- .../service/context/service_context.go | 6 +- .../service/factory/service_factory.go | 22 +-- pkg/kubeclient/banneduser.go | 51 ------- pkg/kubeclient/client.go | 138 ------------------ pkg/kubeclient/client_test.go | 28 ---- pkg/kubeclient/mur.go | 25 ---- pkg/kubeclient/signup.go | 108 -------------- pkg/kubeclient/socialevent.go | 25 ---- pkg/kubeclient/space.go | 25 ---- pkg/kubeclient/spacebinding.go | 29 ---- pkg/kubeclient/toolchainstatus.go | 26 ---- pkg/middleware/jwt_middleware_test.go | 2 +- pkg/middleware/promhttp_middleware_test.go | 2 +- pkg/namespaced/client.go | 22 +++ pkg/server/in_cluster_application.go | 16 +- pkg/server/server_test.go | 2 +- pkg/signup/service/resource_provider.go | 25 ---- pkg/signup/service/signup_service.go | 58 +++++--- pkg/signup/service/signup_service_test.go | 98 ++----------- .../service/verification_service.go | 9 +- .../service/verification_service_test.go | 1 + test/fake/mockable_application.go | 5 +- test/fake/proxy.go | 15 +- test/testsuite.go | 4 +- test/util/application.go | 4 +- test/util/service_context.go | 13 +- 27 files changed, 109 insertions(+), 655 deletions(-) delete mode 100644 pkg/kubeclient/banneduser.go delete mode 100644 pkg/kubeclient/client.go delete mode 100644 pkg/kubeclient/client_test.go delete mode 100644 pkg/kubeclient/mur.go delete mode 100644 pkg/kubeclient/signup.go delete mode 100644 pkg/kubeclient/socialevent.go delete mode 100644 pkg/kubeclient/space.go delete mode 100644 pkg/kubeclient/spacebinding.go delete mode 100644 pkg/kubeclient/toolchainstatus.go create mode 100644 pkg/namespaced/client.go diff --git a/cmd/main.go b/cmd/main.go index 534f163e..202273b5 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -84,10 +84,7 @@ func main() { } } - app, err := server.NewInClusterApplication(cl, configuration.Namespace()) - if err != nil { - panic(err.Error()) - } + app := server.NewInClusterApplication(cl, configuration.Namespace()) // Initialize toolchain cluster cache service // let's cache the member clusters before we start the services, // this will speed up the first request diff --git a/pkg/application/service/context/service_context.go b/pkg/application/service/context/service_context.go index 3f47b5ed..cf1929c3 100644 --- a/pkg/application/service/context/service_context.go +++ b/pkg/application/service/context/service_context.go @@ -2,14 +2,12 @@ package context import ( "github.com/codeready-toolchain/registration-service/pkg/application/service" - "github.com/codeready-toolchain/registration-service/pkg/kubeclient" - "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/codeready-toolchain/registration-service/pkg/namespaced" ) type ServiceContextProducer func() ServiceContext type ServiceContext interface { - CRTClient() kubeclient.CRTClient - Client() client.Client + Client() namespaced.Client Services() service.Services } diff --git a/pkg/application/service/factory/service_factory.go b/pkg/application/service/factory/service_factory.go index ccf6947b..6860b9c1 100644 --- a/pkg/application/service/factory/service_factory.go +++ b/pkg/application/service/factory/service_factory.go @@ -7,42 +7,30 @@ import ( servicecontext "github.com/codeready-toolchain/registration-service/pkg/application/service/context" "github.com/codeready-toolchain/registration-service/pkg/configuration" informerservice "github.com/codeready-toolchain/registration-service/pkg/informers/service" - "github.com/codeready-toolchain/registration-service/pkg/kubeclient" "github.com/codeready-toolchain/registration-service/pkg/log" + "github.com/codeready-toolchain/registration-service/pkg/namespaced" clusterservice "github.com/codeready-toolchain/registration-service/pkg/proxy/service" signupservice "github.com/codeready-toolchain/registration-service/pkg/signup/service" verificationservice "github.com/codeready-toolchain/registration-service/pkg/verification/service" - "sigs.k8s.io/controller-runtime/pkg/client" ) type serviceContextImpl struct { - kubeClient kubeclient.CRTClient - client client.Client - services service.Services + client namespaced.Client + services service.Services } type ServiceContextOption = func(ctx *serviceContextImpl) -func CRTClientOption(kubeClient kubeclient.CRTClient) ServiceContextOption { - return func(ctx *serviceContextImpl) { - ctx.kubeClient = kubeClient - } -} - -func InformerOption(client client.Client) ServiceContextOption { +func NamespacedClientOption(client namespaced.Client) ServiceContextOption { return func(ctx *serviceContextImpl) { ctx.client = client } } -func (s *serviceContextImpl) Client() client.Client { +func (s *serviceContextImpl) Client() namespaced.Client { return s.client } -func (s *serviceContextImpl) CRTClient() kubeclient.CRTClient { - return s.kubeClient -} - func (s *serviceContextImpl) Services() service.Services { return s.services } diff --git a/pkg/kubeclient/banneduser.go b/pkg/kubeclient/banneduser.go deleted file mode 100644 index e34733c2..00000000 --- a/pkg/kubeclient/banneduser.go +++ /dev/null @@ -1,51 +0,0 @@ -package kubeclient - -import ( - "context" - - crtapi "github.com/codeready-toolchain/api/api/v1alpha1" - "github.com/codeready-toolchain/toolchain-common/pkg/hash" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -type bannedUserClient struct { - crtClient -} - -type BannedUserInterface interface { - ListByEmail(email string) (*crtapi.BannedUserList, error) - ListByPhoneNumberOrHash(phoneNumberOrHash string) (*crtapi.BannedUserList, error) -} - -func (c *bannedUserClient) ListByEmail(email string) (*crtapi.BannedUserList, error) { - return c.listByLabelForHashedValue(crtapi.BannedUserEmailHashLabelKey, email) -} - -// ListByPhoneNumberOrHash will return a list of BannedUsers that have a phone number hash label value matching -// the provided value. If the value provided is an actual phone number, then the hash will be calculated and then -// used to query the BannedUsers, otherwise if the hash value has been provided, then that value will be used -// directly for the query. -func (c *bannedUserClient) ListByPhoneNumberOrHash(phoneNumberOrHash string) (*crtapi.BannedUserList, error) { - if md5Matcher.Match([]byte(phoneNumberOrHash)) { - return c.listByLabel(crtapi.BannedUserPhoneNumberHashLabelKey, phoneNumberOrHash) - } - - // Default to searching for a hash of the specified value - return c.listByLabelForHashedValue(crtapi.BannedUserPhoneNumberHashLabelKey, phoneNumberOrHash) -} - -// listByLabelForHashedValue returns a BannedUserList containing any BannedUser resources that have a label matching -// the hash of the specified value -func (c *bannedUserClient) listByLabelForHashedValue(labelKey, valueToHash string) (*crtapi.BannedUserList, error) { - return c.listByLabel(labelKey, hash.EncodeString(valueToHash)) -} - -// listByLabel returns a BannedUserList containing any BannedUser resources that have a label matching the specified label -func (c *bannedUserClient) listByLabel(labelKey, labelValue string) (*crtapi.BannedUserList, error) { - bannedUsers := &crtapi.BannedUserList{} - if err := c.client.List(context.TODO(), bannedUsers, client.InNamespace(c.ns), - client.MatchingLabels{labelKey: labelValue}); err != nil { - return nil, err - } - return bannedUsers, nil -} diff --git a/pkg/kubeclient/client.go b/pkg/kubeclient/client.go deleted file mode 100644 index eb6f345d..00000000 --- a/pkg/kubeclient/client.go +++ /dev/null @@ -1,138 +0,0 @@ -package kubeclient - -import ( - crtapi "github.com/codeready-toolchain/api/api/v1alpha1" - "sigs.k8s.io/controller-runtime/pkg/client" - - "k8s.io/apimachinery/pkg/runtime" -) - -type CRTClient interface { - V1Alpha1() V1Alpha1 -} - -type V1Alpha1 interface { - UserSignups() UserSignupInterface - MasterUserRecords() MasterUserRecordInterface - BannedUsers() BannedUserInterface - ToolchainStatuses() ToolchainStatusInterface - SocialEvents() SocialEventInterface - Spaces() SpaceInterface - SpaceBindings() SpaceBindingInterface -} - -// NewCRTRESTClient creates a new REST client for managing Codeready Toolchain resources via the Kubernetes API -func NewCRTRESTClient(client client.Client, namespace string) (CRTClient, error) { - scheme := runtime.NewScheme() - err := crtapi.SchemeBuilder.AddToScheme(scheme) - if err != nil { - return nil, err - } - - crtRESTClient := &CRTRESTClient{ - Client: client, - NS: namespace, - Scheme: scheme, - } - - crtRESTClient.v1Alpha1 = &V1Alpha1REST{client: crtRESTClient} - return crtRESTClient, nil -} - -type CRTRESTClient struct { - Client client.Client - NS string - Scheme *runtime.Scheme - v1Alpha1 *V1Alpha1REST -} - -func (c *CRTRESTClient) V1Alpha1() V1Alpha1 { - return c.v1Alpha1 -} - -type V1Alpha1REST struct { - client *CRTRESTClient -} - -// UserSignups returns an interface which may be used to perform CRUD operations for UserSignup resources -func (c *V1Alpha1REST) UserSignups() UserSignupInterface { - return &userSignupClient{ - crtClient: crtClient{ - client: c.client.Client, - ns: c.client.NS, - scheme: c.client.Scheme, - }, - } -} - -// MasterUserRecords returns an interface which may be used to perform CRUD operations for MasterUserRecord resources -func (c *V1Alpha1REST) MasterUserRecords() MasterUserRecordInterface { - return &masterUserRecordClient{ - crtClient: crtClient{ - client: c.client.Client, - ns: c.client.NS, - scheme: c.client.Scheme, - }, - } -} - -// BannedUsers returns an interface which may be used to perform query operations on BannedUser resources -func (c *V1Alpha1REST) BannedUsers() BannedUserInterface { - return &bannedUserClient{ - crtClient: crtClient{ - client: c.client.Client, - ns: c.client.NS, - scheme: c.client.Scheme, - }, - } -} - -// ToolchainStatuses returns an interface which may be used to perform query operations on ToolchainStatus resources -func (c *V1Alpha1REST) ToolchainStatuses() ToolchainStatusInterface { - return &toolchainStatusClient{ - crtClient: crtClient{ - client: c.client.Client, - ns: c.client.NS, - scheme: c.client.Scheme, - }, - } -} - -// SocialEvents returns an interface which may be used to perform CRUD operations for SocialEvent resources -func (c *V1Alpha1REST) SocialEvents() SocialEventInterface { - return &socialeventClient{ - crtClient: crtClient{ - client: c.client.Client, - ns: c.client.NS, - scheme: c.client.Scheme, - }, - } -} - -// Spaces returns an interface which may be used to perform CRUD operations for Space resources -func (c *V1Alpha1REST) Spaces() SpaceInterface { - return &spaceClient{ - crtClient: crtClient{ - client: c.client.Client, - ns: c.client.NS, - scheme: c.client.Scheme, - }, - } -} - -// SpaceBindings returns an interface which may be used to perform CRUD operations for SpaceBinding resources -func (c *V1Alpha1REST) SpaceBindings() SpaceBindingInterface { - return &spaceBindingClient{ - crtClient: crtClient{ - client: c.client.Client, - ns: c.client.NS, - scheme: c.client.Scheme, - }, - } -} - -type crtClient struct { - client client.Client - ns string - scheme *runtime.Scheme -} diff --git a/pkg/kubeclient/client_test.go b/pkg/kubeclient/client_test.go deleted file mode 100644 index 8528e5e3..00000000 --- a/pkg/kubeclient/client_test.go +++ /dev/null @@ -1,28 +0,0 @@ -package kubeclient_test - -import ( - "testing" - - "github.com/codeready-toolchain/toolchain-common/pkg/test" - "github.com/stretchr/testify/require" - - "github.com/codeready-toolchain/registration-service/pkg/kubeclient" -) - -const ( - TestNamespace = "test-namespace-name" -) - -func TestNewClient(t *testing.T) { - // Try creating a new client with an empty config - client, err := kubeclient.NewCRTRESTClient(test.NewFakeClient(t), TestNamespace) - - // Check that there are no errors, and the clients are returned - require.NoError(t, err) - require.NotNil(t, client) - require.NotNil(t, client.V1Alpha1()) - require.NotNil(t, client.V1Alpha1().UserSignups()) - require.NotNil(t, client.V1Alpha1().MasterUserRecords()) - require.NotNil(t, client.V1Alpha1().BannedUsers()) - require.NotNil(t, client.V1Alpha1().ToolchainStatuses()) -} diff --git a/pkg/kubeclient/mur.go b/pkg/kubeclient/mur.go deleted file mode 100644 index 098b77af..00000000 --- a/pkg/kubeclient/mur.go +++ /dev/null @@ -1,25 +0,0 @@ -package kubeclient - -import ( - "context" - - crtapi "github.com/codeready-toolchain/api/api/v1alpha1" - "k8s.io/apimachinery/pkg/types" -) - -type masterUserRecordClient struct { - crtClient -} - -type MasterUserRecordInterface interface { - Get(name string) (*crtapi.MasterUserRecord, error) -} - -// Get returns the MasterUserRecord with the specified name, or an error if something went wrong while attempting to retrieve it -func (c *masterUserRecordClient) Get(name string) (*crtapi.MasterUserRecord, error) { - result := &crtapi.MasterUserRecord{} - if err := c.client.Get(context.TODO(), types.NamespacedName{Namespace: c.ns, Name: name}, result); err != nil { - return nil, err - } - return result, nil -} diff --git a/pkg/kubeclient/signup.go b/pkg/kubeclient/signup.go deleted file mode 100644 index 069f5959..00000000 --- a/pkg/kubeclient/signup.go +++ /dev/null @@ -1,108 +0,0 @@ -package kubeclient - -import ( - "context" - "regexp" - - crtapi "github.com/codeready-toolchain/api/api/v1alpha1" - "github.com/codeready-toolchain/toolchain-common/pkg/hash" - "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/selection" - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -var ( - md5Matcher = regexp.MustCompile("(?i)[a-f0-9]{32}$") -) - -type userSignupClient struct { - crtClient -} - -// UserSignupInterface is the interface for user signup. -type UserSignupInterface interface { - Get(name string) (*crtapi.UserSignup, error) - Create(obj *crtapi.UserSignup) (*crtapi.UserSignup, error) - Update(obj *crtapi.UserSignup) (*crtapi.UserSignup, error) - ListActiveSignupsByPhoneNumberOrHash(phoneNumberOrHash string) ([]*crtapi.UserSignup, error) -} - -// Get returns the UserSignup with the specified name, or an error if something went wrong while attempting to retrieve it -// If not found then NotFound error returned -func (c *userSignupClient) Get(name string) (*crtapi.UserSignup, error) { - result := &crtapi.UserSignup{} - if err := c.client.Get(context.TODO(), types.NamespacedName{Namespace: c.ns, Name: name}, result); err != nil { - return nil, err - } - return result, nil -} - -// Create creates a new UserSignup resource in the cluster, and returns the resulting UserSignup that was created, or -// an error if something went wrong -func (c *userSignupClient) Create(obj *crtapi.UserSignup) (*crtapi.UserSignup, error) { - if err := c.client.Create(context.TODO(), obj); err != nil { - return nil, err - } - return obj, nil -} - -// Update will update an existing UserSignup resource in the cluster, returning an error if something went wrong -func (c *userSignupClient) Update(obj *crtapi.UserSignup) (*crtapi.UserSignup, error) { - err := c.client.Update(context.TODO(), obj) - if err != nil { - return nil, err - } - return obj, nil -} - -// ListActiveSignupsByPhoneNumberOrHash will return a list of non-deactivated UserSignups that have a phone number hash -// label value matching the provided value. If the value provided is an actual phone number, then the hash will be -// calculated and then used to query the UserSignups, otherwise if the hash value has been provided, then that value -// will be used directly for the query. -func (c *userSignupClient) ListActiveSignupsByPhoneNumberOrHash(phoneNumberOrHash string) ([]*crtapi.UserSignup, error) { - if md5Matcher.Match([]byte(phoneNumberOrHash)) { - return c.listActiveSignupsByLabel(crtapi.BannedUserPhoneNumberHashLabelKey, phoneNumberOrHash) - } - - // Default to searching for a hash of the specified value - return c.listActiveSignupsByLabelForHashedValue(crtapi.BannedUserPhoneNumberHashLabelKey, phoneNumberOrHash) -} - -// listActiveSignupsByLabelForHashedValue returns an array of UserSignups containing any non-deactivated UserSignup resources -// that have a label matching the md5 hash of the specified value -func (c *userSignupClient) listActiveSignupsByLabelForHashedValue(labelKey, valueToHash string) ([]*crtapi.UserSignup, error) { - return c.listActiveSignupsByLabel(labelKey, hash.EncodeString(valueToHash)) -} - -// listActiveSignupsByLabel returns an array of UserSignups containing any non-deactivated UserSignup resources that have a -// label matching the specified label -func (c *userSignupClient) listActiveSignupsByLabel(labelKey, labelValue string) ([]*crtapi.UserSignup, error) { - - // TODO add unit tests checking that the label selection works as expected. Right now, it's not possible to do that thanks to the great abstraction and multiple level of layers, mocks, and services. - selector := labels.NewSelector() - stateRequirement, err := labels.NewRequirement(crtapi.UserSignupStateLabelKey, selection.NotIn, []string{crtapi.UserSignupStateLabelValueDeactivated, crtapi.UserSignupStateLabelValueNotReady}) - if err != nil { - return nil, err - } - selector = selector.Add(*stateRequirement) - labelRequirement, err := labels.NewRequirement(labelKey, selection.Equals, []string{labelValue}) - if err != nil { - return nil, err - } - selector = selector.Add(*labelRequirement) - - userSignups := &crtapi.UserSignupList{} - err = c.client.List(context.TODO(), userSignups, client.InNamespace(c.ns), client.MatchingLabelsSelector{Selector: selector}) - if err != nil { - return nil, err - } - - result := make([]*crtapi.UserSignup, len(userSignups.Items)) - - for i := range userSignups.Items { - result[i] = &userSignups.Items[i] - } - - return result, err -} diff --git a/pkg/kubeclient/socialevent.go b/pkg/kubeclient/socialevent.go deleted file mode 100644 index 346a04f7..00000000 --- a/pkg/kubeclient/socialevent.go +++ /dev/null @@ -1,25 +0,0 @@ -package kubeclient - -import ( - "context" - - crtapi "github.com/codeready-toolchain/api/api/v1alpha1" - "k8s.io/apimachinery/pkg/types" -) - -type socialeventClient struct { - crtClient -} - -type SocialEventInterface interface { - Get(name string) (*crtapi.SocialEvent, error) -} - -// Get returns the SocialEvent with the specified name, or an error if something went wrong while attempting to retrieve it -func (c *socialeventClient) Get(name string) (*crtapi.SocialEvent, error) { - result := &crtapi.SocialEvent{} - if err := c.client.Get(context.TODO(), types.NamespacedName{Namespace: c.ns, Name: name}, result); err != nil { - return nil, err - } - return result, nil -} diff --git a/pkg/kubeclient/space.go b/pkg/kubeclient/space.go deleted file mode 100644 index 79745a99..00000000 --- a/pkg/kubeclient/space.go +++ /dev/null @@ -1,25 +0,0 @@ -package kubeclient - -import ( - "context" - - crtapi "github.com/codeready-toolchain/api/api/v1alpha1" - "k8s.io/apimachinery/pkg/types" -) - -type spaceClient struct { - crtClient -} - -type SpaceInterface interface { - Get(name string) (*crtapi.Space, error) -} - -// List returns the Spaces that match for the provided selector, or an error if something went wrong while attempting to retrieve it -func (c *spaceClient) Get(name string) (*crtapi.Space, error) { - result := &crtapi.Space{} - if err := c.client.Get(context.TODO(), types.NamespacedName{Namespace: c.ns, Name: name}, result); err != nil { - return nil, err - } - return result, nil -} diff --git a/pkg/kubeclient/spacebinding.go b/pkg/kubeclient/spacebinding.go deleted file mode 100644 index f32b60da..00000000 --- a/pkg/kubeclient/spacebinding.go +++ /dev/null @@ -1,29 +0,0 @@ -package kubeclient - -import ( - "context" - - crtapi "github.com/codeready-toolchain/api/api/v1alpha1" - "k8s.io/apimachinery/pkg/labels" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -type spaceBindingClient struct { - crtClient -} - -type SpaceBindingInterface interface { - ListSpaceBindings(reqs ...labels.Requirement) ([]crtapi.SpaceBinding, error) -} - -// List returns the SpaceBindings that match for the provided selector, or an error if something went wrong while attempting to retrieve it -func (c *spaceBindingClient) ListSpaceBindings(reqs ...labels.Requirement) ([]crtapi.SpaceBinding, error) { - - selector := labels.NewSelector().Add(reqs...) - - result := &crtapi.SpaceBindingList{} - err := c.client.List(context.TODO(), result, client.InNamespace(c.ns), - client.MatchingLabelsSelector{Selector: selector}) - - return result.Items, err -} diff --git a/pkg/kubeclient/toolchainstatus.go b/pkg/kubeclient/toolchainstatus.go deleted file mode 100644 index 0c591899..00000000 --- a/pkg/kubeclient/toolchainstatus.go +++ /dev/null @@ -1,26 +0,0 @@ -package kubeclient - -import ( - "context" - - crtapi "github.com/codeready-toolchain/api/api/v1alpha1" - "k8s.io/apimachinery/pkg/types" -) - -type toolchainStatusClient struct { - crtClient -} - -type ToolchainStatusInterface interface { - Get() (*crtapi.ToolchainStatus, error) -} - -// Get returns the ToolchainStatus with the "toolchain-status" name, or an error if something went wrong while attempting to retrieve it -// If not found then NotFound error returned -func (c *toolchainStatusClient) Get() (*crtapi.ToolchainStatus, error) { - result := &crtapi.ToolchainStatus{} - if err := c.client.Get(context.TODO(), types.NamespacedName{Namespace: c.ns, Name: "toolchain-status"}, result); err != nil { - return nil, err - } - return result, nil -} diff --git a/pkg/middleware/jwt_middleware_test.go b/pkg/middleware/jwt_middleware_test.go index 15290bf8..98d6262c 100644 --- a/pkg/middleware/jwt_middleware_test.go +++ b/pkg/middleware/jwt_middleware_test.go @@ -75,7 +75,7 @@ func (s *TestAuthMiddlewareSuite) TestAuthMiddlewareService() { keysEndpointURL := tokengenerator.NewKeyServer().URL // create server - srv := server.New(fake.NewMockableApplication(nil)) + srv := server.New(fake.NewMockableApplication()) // set the key service url in the config s.SetConfig(testconfig.RegistrationService(). diff --git a/pkg/middleware/promhttp_middleware_test.go b/pkg/middleware/promhttp_middleware_test.go index f94b3813..fd716caa 100644 --- a/pkg/middleware/promhttp_middleware_test.go +++ b/pkg/middleware/promhttp_middleware_test.go @@ -33,7 +33,7 @@ func TestPromHTTPMiddlewareSuite(t *testing.T) { func (s *PromHTTPMiddlewareSuite) TestPromHTTPMiddleware() { // given tokengenerator := authsupport.NewTokenManager() - srv := server.New(fake.NewMockableApplication(nil)) + srv := server.New(fake.NewMockableApplication()) keysEndpointURL := tokengenerator.NewKeyServer().URL s.SetConfig(testconfig.RegistrationService(). Environment(configuration.UnitTestsEnvironment). diff --git a/pkg/namespaced/client.go b/pkg/namespaced/client.go new file mode 100644 index 00000000..659140fe --- /dev/null +++ b/pkg/namespaced/client.go @@ -0,0 +1,22 @@ +package namespaced + +import ( + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func NewClient(client client.Client, namespace string) Client { + return Client{Client: client, Namespace: namespace} +} + +type Client struct { + client.Client + Namespace string +} + +func (c Client) NamespacedName(name string) types.NamespacedName { + return types.NamespacedName{ + Namespace: c.Namespace, + Name: name, + } +} diff --git a/pkg/server/in_cluster_application.go b/pkg/server/in_cluster_application.go index 903a973d..60ba20c8 100644 --- a/pkg/server/in_cluster_application.go +++ b/pkg/server/in_cluster_application.go @@ -4,7 +4,7 @@ import ( "github.com/codeready-toolchain/registration-service/pkg/application" "github.com/codeready-toolchain/registration-service/pkg/application/service" "github.com/codeready-toolchain/registration-service/pkg/application/service/factory" - "github.com/codeready-toolchain/registration-service/pkg/kubeclient" + "github.com/codeready-toolchain/registration-service/pkg/namespaced" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -12,18 +12,12 @@ import ( // application type is intended to run inside a Kubernetes cluster, where it makes use of the rest.InClusterConfig() // function to determine which Kubernetes configuration to use to create the REST client that interacts with the // Kubernetes service endpoints. -func NewInClusterApplication(client client.Client, namespace string, options ...factory.Option) (application.Application, error) { - kubeClient, err := kubeclient.NewCRTRESTClient(client, namespace) - if err != nil { - return nil, err - } - +func NewInClusterApplication(client client.Client, namespace string, options ...factory.Option) application.Application { return &InClusterApplication{ serviceFactory: factory.NewServiceFactory(append(options, - factory.WithServiceContextOptions(factory.CRTClientOption(kubeClient), - factory.InformerOption(client), - ))...), - }, nil + factory.WithServiceContextOptions( + factory.NamespacedClientOption(namespaced.NewClient(client, namespace))))...), + } } type InClusterApplication struct { diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go index 8ab5559e..dc12f33a 100644 --- a/pkg/server/server_test.go +++ b/pkg/server/server_test.go @@ -34,7 +34,7 @@ const ( func (s *TestServerSuite) TestServer() { // We're using the example config for the configuration here as the // specific config params do not matter for testing the routes setup. - srv := server.New(fake.NewMockableApplication(nil)) + srv := server.New(fake.NewMockableApplication()) fake.MockKeycloakCertsCall(s.T()) // Setting up the routes. diff --git a/pkg/signup/service/resource_provider.go b/pkg/signup/service/resource_provider.go index 21f12f75..f33963e7 100644 --- a/pkg/signup/service/resource_provider.go +++ b/pkg/signup/service/resource_provider.go @@ -2,7 +2,6 @@ package service import ( toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" - "github.com/codeready-toolchain/registration-service/pkg/kubeclient" "k8s.io/apimachinery/pkg/labels" ) @@ -13,27 +12,3 @@ type ResourceProvider interface { GetSpace(name string) (*toolchainv1alpha1.Space, error) ListSpaceBindings(reqs ...labels.Requirement) ([]toolchainv1alpha1.SpaceBinding, error) } - -type CrtClientProvider struct { - Cl kubeclient.CRTClient -} - -func (p CrtClientProvider) GetMasterUserRecord(name string) (*toolchainv1alpha1.MasterUserRecord, error) { - return p.Cl.V1Alpha1().MasterUserRecords().Get(name) -} - -func (p CrtClientProvider) GetToolchainStatus() (*toolchainv1alpha1.ToolchainStatus, error) { - return p.Cl.V1Alpha1().ToolchainStatuses().Get() -} - -func (p CrtClientProvider) GetUserSignup(name string) (*toolchainv1alpha1.UserSignup, error) { - return p.Cl.V1Alpha1().UserSignups().Get(name) -} - -func (p CrtClientProvider) GetSpace(name string) (*toolchainv1alpha1.Space, error) { - return p.Cl.V1Alpha1().Spaces().Get(name) -} - -func (p CrtClientProvider) ListSpaceBindings(reqs ...labels.Requirement) ([]toolchainv1alpha1.SpaceBinding, error) { - return p.Cl.V1Alpha1().SpaceBindings().ListSpaceBindings(reqs...) -} diff --git a/pkg/signup/service/signup_service.go b/pkg/signup/service/signup_service.go index 98370c08..d2a4e249 100644 --- a/pkg/signup/service/signup_service.go +++ b/pkg/signup/service/signup_service.go @@ -1,6 +1,7 @@ package service import ( + gocontext "context" "fmt" "hash/crc32" "regexp" @@ -15,7 +16,9 @@ import ( "github.com/codeready-toolchain/registration-service/pkg/configuration" "github.com/codeready-toolchain/registration-service/pkg/context" "github.com/codeready-toolchain/registration-service/pkg/errors" + infservice "github.com/codeready-toolchain/registration-service/pkg/informers/service" "github.com/codeready-toolchain/registration-service/pkg/log" + "github.com/codeready-toolchain/registration-service/pkg/namespaced" "github.com/codeready-toolchain/registration-service/pkg/signup" "github.com/codeready-toolchain/registration-service/pkg/verification/captcha" "github.com/codeready-toolchain/toolchain-common/pkg/condition" @@ -29,6 +32,7 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/selection" + "sigs.k8s.io/controller-runtime/pkg/client" ) const ( @@ -46,6 +50,7 @@ var annotationsToRetain = []string{ // ServiceImpl represents the implementation of the signup service. type ServiceImpl struct { // nolint:revive base.BaseService + namespaced.Client defaultProvider ResourceProvider CaptchaChecker captcha.Assessor } @@ -57,8 +62,9 @@ func NewSignupService(context servicecontext.ServiceContext, opts ...SignupServi s := &ServiceImpl{ BaseService: base.NewBaseService(context), - defaultProvider: CrtClientProvider{context.CRTClient()}, + defaultProvider: infservice.NewInformerService(context.Client().Client, context.Client().Namespace), CaptchaChecker: captcha.Helper{}, + Client: context.Client(), } for _, opt := range opts { @@ -90,8 +96,9 @@ func (s *ServiceImpl) newUserSignup(ctx *gin.Context) (*toolchainv1alpha1.UserSi emailHash := hash.EncodeString(userEmail) // Query BannedUsers to check the user has not been banned - bannedUsers, err := s.CRTClient().V1Alpha1().BannedUsers().ListByEmail(userEmail) - if err != nil { + bannedUsers := &toolchainv1alpha1.BannedUserList{} + if err := s.List(gocontext.TODO(), bannedUsers, client.InNamespace(s.Namespace), + client.MatchingLabels{toolchainv1alpha1.BannedUserEmailHashLabelKey: hash.EncodeString(userEmail)}); err != nil { return nil, err } @@ -285,13 +292,12 @@ func (s *ServiceImpl) Signup(ctx *gin.Context) (*toolchainv1alpha1.UserSignup, e encodedUserID := EncodeUserIdentifier(ctx.GetString(context.SubKey)) // Retrieve UserSignup resource from the host cluster - userSignup, err := s.CRTClient().V1Alpha1().UserSignups().Get(encodedUserID) - if err != nil { + userSignup := &toolchainv1alpha1.UserSignup{} + if err := s.Get(gocontext.TODO(), s.NamespacedName(encodedUserID), userSignup); err != nil { if apierrors.IsNotFound(err) { // The UserSignup could not be located by its encoded UserID, attempt to load it using its encoded PreferredUsername instead encodedUsername := EncodeUserIdentifier(ctx.GetString(context.UsernameKey)) - userSignup, err = s.CRTClient().V1Alpha1().UserSignups().Get(encodedUsername) - if err != nil { + if err := s.Get(gocontext.TODO(), s.NamespacedName(encodedUsername), userSignup); err != nil { if apierrors.IsNotFound(err) { // New Signup log.WithValues(map[string]interface{}{"encoded_user_id": encodedUserID}).Info(ctx, "user not found, creating a new one") @@ -323,7 +329,7 @@ func (s *ServiceImpl) createUserSignup(ctx *gin.Context) (*toolchainv1alpha1.Use return nil, err } - return s.CRTClient().V1Alpha1().UserSignups().Create(userSignup) + return userSignup, s.Create(gocontext.TODO(), userSignup) } // reactivateUserSignup reactivates the deactivated UserSignup resource with the specified username and userID @@ -349,7 +355,7 @@ func (s *ServiceImpl) reactivateUserSignup(ctx *gin.Context, existing *toolchain existing.Labels = newUserSignup.Labels existing.Spec = newUserSignup.Spec - return s.CRTClient().V1Alpha1().UserSignups().Update(existing) + return existing, s.Update(gocontext.TODO(), existing) } // GetSignup returns Signup resource which represents the corresponding K8s UserSignup @@ -589,33 +595,49 @@ func (s *ServiceImpl) DoGetUserSignupFromIdentifier(provider ResourceProvider, u // UpdateUserSignup is used to update the provided UserSignup resource, and returning the updated resource func (s *ServiceImpl) UpdateUserSignup(userSignup *toolchainv1alpha1.UserSignup) (*toolchainv1alpha1.UserSignup, error) { - userSignup, err := s.CRTClient().V1Alpha1().UserSignups().Update(userSignup) - if err != nil { + if err := s.Update(gocontext.TODO(), userSignup); err != nil { return nil, err } return userSignup, nil } +var ( + md5Matcher = regexp.MustCompile("(?i)[a-f0-9]{32}$") +) + // PhoneNumberAlreadyInUse checks if the phone number has been banned. If so, return -// an internal server error. If not, check if an active (non-deactivated) UserSignup with a different userID and username +// an internal server error. If not, check if an approved UserSignup with a different userID and username // and email address exists. If so, return an internal server error. Otherwise, return without error. // Either the actual phone number, or the md5 hash of the phone number may be provided here. func (s *ServiceImpl) PhoneNumberAlreadyInUse(userID, username, phoneNumberOrHash string) error { - bannedUserList, err := s.CRTClient().V1Alpha1().BannedUsers().ListByPhoneNumberOrHash(phoneNumberOrHash) - if err != nil { + labelValue := hash.EncodeString(phoneNumberOrHash) + if md5Matcher.Match([]byte(phoneNumberOrHash)) { + labelValue = phoneNumberOrHash + } + + bannedUserList := &toolchainv1alpha1.BannedUserList{} + if err := s.List(gocontext.TODO(), bannedUserList, client.InNamespace(s.Namespace), + client.MatchingLabels{toolchainv1alpha1.BannedUserPhoneNumberHashLabelKey: labelValue}); err != nil { return errors.NewInternalError(err, "failed listing banned users") } + if len(bannedUserList.Items) > 0 { return errors.NewForbiddenError("cannot re-register with phone number", "phone number already in use") } - userSignups, err := s.CRTClient().V1Alpha1().UserSignups().ListActiveSignupsByPhoneNumberOrHash(phoneNumberOrHash) - if err != nil { + labelSelector := client.MatchingLabels{ + toolchainv1alpha1.UserSignupStateLabelKey: toolchainv1alpha1.UserSignupStateLabelValueApproved, + toolchainv1alpha1.BannedUserPhoneNumberHashLabelKey: labelValue, + } + userSignups := &toolchainv1alpha1.UserSignupList{} + if err := s.List(gocontext.TODO(), userSignups, client.InNamespace(s.Namespace), labelSelector); err != nil { return errors.NewInternalError(err, "failed listing userSignups") } - for _, signup := range userSignups { - if signup.Spec.IdentityClaims.Sub != userID && signup.Spec.IdentityClaims.PreferredUsername != username && !states.Deactivated(signup) { // nolint:gosec + + for _, signup := range userSignups.Items { + userSignup := signup // drop with go 1.22 + if userSignup.Spec.IdentityClaims.Sub != userID && userSignup.Spec.IdentityClaims.PreferredUsername != username && !states.Deactivated(&userSignup) { return errors.NewForbiddenError("cannot re-register with phone number", "phone number already in use") } diff --git a/pkg/signup/service/signup_service_test.go b/pkg/signup/service/signup_service_test.go index a568cd28..7268ad1a 100644 --- a/pkg/signup/service/signup_service_test.go +++ b/pkg/signup/service/signup_service_test.go @@ -17,7 +17,6 @@ import ( "github.com/codeready-toolchain/registration-service/pkg/context" errors2 "github.com/codeready-toolchain/registration-service/pkg/errors" infservice "github.com/codeready-toolchain/registration-service/pkg/informers/service" - "github.com/codeready-toolchain/registration-service/pkg/kubeclient" "github.com/codeready-toolchain/registration-service/pkg/signup/service" "github.com/codeready-toolchain/registration-service/pkg/util" "github.com/codeready-toolchain/registration-service/test" @@ -617,6 +616,7 @@ func (s *TestSignupServiceSuite) TestPhoneNumberAlreadyInUseUserSignup() { Labels: map[string]string{ toolchainv1alpha1.UserSignupUserEmailHashLabelKey: "a7b1b413c1cbddbcd19a51222ef8e20a", toolchainv1alpha1.UserSignupUserPhoneHashLabelKey: "fd276563a8232d16620da8ec85d0575f", + toolchainv1alpha1.UserSignupStateLabelKey: toolchainv1alpha1.UserSignupStateLabelValueApproved, }, }, } @@ -1432,28 +1432,14 @@ func (s *TestSignupServiceSuite) TestGetDefaultUserNamespace() { space := s.newSpace("dave") fakeClient := commontest.NewFakeClient(s.T(), space) - crtClient, err := kubeclient.NewCRTRESTClient(fakeClient, commontest.HostOperatorNs) - require.NoError(s.T(), err) - crtClientProvider := service.CrtClientProvider{Cl: crtClient} + inf := infservice.NewInformerService(fakeClient, commontest.HostOperatorNs) // when - targetCluster, defaultUserNamespace := service.GetDefaultUserTarget(crtClientProvider, "dave", "dave") + targetCluster, defaultUserNamespace := service.GetDefaultUserTarget(inf, "dave", "dave") // then assert.Equal(s.T(), "dave-dev", defaultUserNamespace) assert.Equal(s.T(), "member-123", targetCluster) - - s.T().Run("informer", func(t *testing.T) { - // given - inf := infservice.NewInformerService(fakeClient, commontest.HostOperatorNs) - - // when - targetCluster, defaultUserNamespace := service.GetDefaultUserTarget(inf, "dave", "dave") - - // then - assert.Equal(t, "dave-dev", defaultUserNamespace) - assert.Equal(s.T(), "member-123", targetCluster) - }) } // TestGetDefaultUserNamespaceFromFirstUnownedSpace tests that the default user namespace is returned even if the only accessible Space was not created as the home space. @@ -1471,28 +1457,14 @@ func (s *TestSignupServiceSuite) TestGetDefaultUserNamespaceFromFirstUnownedSpac spaceCindingC := s.newSpaceBinding("userB", spaceC.Name) fakeClient := commontest.NewFakeClient(s.T(), space, spacebindingB, spaceC, spaceCindingC) - crtClient, err := kubeclient.NewCRTRESTClient(fakeClient, commontest.HostOperatorNs) - require.NoError(s.T(), err) - crtClientProvider := service.CrtClientProvider{Cl: crtClient} + inf := infservice.NewInformerService(fakeClient, commontest.HostOperatorNs) // when - targetCluster, defaultUserNamespace := service.GetDefaultUserTarget(crtClientProvider, "", "userB") + targetCluster, defaultUserNamespace := service.GetDefaultUserTarget(inf, "", "userB") // then assert.Equal(s.T(), "userA-dev", defaultUserNamespace) assert.Equal(s.T(), "member-123", targetCluster) - - s.T().Run("informer", func(t *testing.T) { - // given - inf := infservice.NewInformerService(fakeClient, commontest.HostOperatorNs) - - // when - targetCluster, defaultUserNamespace := service.GetDefaultUserTarget(inf, "", "userB") - - // then - assert.Equal(t, "userA-dev", defaultUserNamespace) - assert.Equal(s.T(), "member-123", targetCluster) - }) } // TestGetDefaultUserNamespaceMultiSpace tests that the home Space created for the user is prioritized when there are multiple spaces @@ -1510,29 +1482,15 @@ func (s *TestSignupServiceSuite) TestGetDefaultUserNamespaceMultiSpace() { spacebinding2 := s.newSpaceBinding("userB", space2.Name) fakeClient := commontest.NewFakeClient(s.T(), space1, space2, spacebinding1, spacebinding2) - crtClient, err := kubeclient.NewCRTRESTClient(fakeClient, commontest.HostOperatorNs) - require.NoError(s.T(), err) - crtClientProvider := service.CrtClientProvider{Cl: crtClient} + inf := infservice.NewInformerService(fakeClient, commontest.HostOperatorNs) // when - // get default namespace for userB - targetCluster, defaultUserNamespace := service.GetDefaultUserTarget(crtClientProvider, "userB", "userB") + targetCluster, defaultUserNamespace := service.GetDefaultUserTarget(inf, "userB", "userB") // then assert.Equal(s.T(), "userB-dev", defaultUserNamespace) // space2 is prioritized over space1 because it was created by the userB assert.Equal(s.T(), "member-123", targetCluster) - s.T().Run("informer", func(t *testing.T) { - // given - inf := infservice.NewInformerService(fakeClient, commontest.HostOperatorNs) - - // when - targetCluster, defaultUserNamespace := service.GetDefaultUserTarget(inf, "userB", "userB") - - // then - assert.Equal(t, "userB-dev", defaultUserNamespace) - assert.Equal(s.T(), "member-123", targetCluster) - }) } func (s *TestSignupServiceSuite) TestGetDefaultUserNamespaceFailNoHomeSpaceNoSpaceBinding() { @@ -1541,62 +1499,28 @@ func (s *TestSignupServiceSuite) TestGetDefaultUserNamespaceFailNoHomeSpaceNoSpa space := s.newSpace("dave") fakeClient := commontest.NewFakeClient(s.T(), space) - crtClient, err := kubeclient.NewCRTRESTClient(fakeClient, commontest.HostOperatorNs) - require.NoError(s.T(), err) - crtClientProvider := service.CrtClientProvider{Cl: crtClient} + inf := infservice.NewInformerService(fakeClient, commontest.HostOperatorNs) // when - targetCluster, defaultUserNamespace := service.GetDefaultUserTarget(crtClientProvider, "", "dave") + targetCluster, defaultUserNamespace := service.GetDefaultUserTarget(inf, "", "dave") // then assert.Empty(s.T(), defaultUserNamespace) assert.Empty(s.T(), targetCluster) - - s.T().Run("informer", func(t *testing.T) { - // given - fakeClient.MockList = func(ctx gocontext.Context, list client.ObjectList, opts ...client.ListOption) error { - if _, ok := list.(*toolchainv1alpha1.SpaceBindingList); ok { - return apierrors.NewInternalError(fmt.Errorf("something went wrong")) - } - return fakeClient.Client.List(ctx, list, opts...) - } - inf := infservice.NewInformerService(fakeClient, commontest.HostOperatorNs) - - // when - targetCluster, defaultUserNamespace := service.GetDefaultUserTarget(inf, "", "dave") - - // then - assert.Empty(t, defaultUserNamespace) - assert.Empty(s.T(), targetCluster) - }) } func (s *TestSignupServiceSuite) TestGetDefaultUserNamespaceFailNoSpace() { // given s.ServiceConfiguration(true, "", 5) fakeClient := commontest.NewFakeClient(s.T()) - crtClient, err := kubeclient.NewCRTRESTClient(fakeClient, commontest.HostOperatorNs) - require.NoError(s.T(), err) - crtClientProvider := service.CrtClientProvider{Cl: crtClient} + inf := infservice.NewInformerService(fakeClient, commontest.HostOperatorNs) // when - targetCluster, defaultUserNamespace := service.GetDefaultUserTarget(crtClientProvider, "dave", "dave") + targetCluster, defaultUserNamespace := service.GetDefaultUserTarget(inf, "dave", "dave") // then assert.Empty(s.T(), defaultUserNamespace) assert.Empty(s.T(), targetCluster) - - s.T().Run("informer", func(t *testing.T) { - // given - inf := infservice.NewInformerService(fakeClient, commontest.HostOperatorNs) - - // when - targetCluster, defaultUserNamespace := service.GetDefaultUserTarget(inf, "dave", "dave") - - // then - assert.Empty(t, defaultUserNamespace) - assert.Empty(s.T(), targetCluster) - }) } func (s *TestSignupServiceSuite) TestGetUserSignup() { diff --git a/pkg/verification/service/verification_service.go b/pkg/verification/service/verification_service.go index 84bb1952..7fdd3919 100644 --- a/pkg/verification/service/verification_service.go +++ b/pkg/verification/service/verification_service.go @@ -1,6 +1,7 @@ package service import ( + gocontext "context" "crypto/rand" "errors" "fmt" @@ -8,8 +9,8 @@ import ( "strconv" "time" + "github.com/codeready-toolchain/registration-service/pkg/namespaced" signuppkg "github.com/codeready-toolchain/registration-service/pkg/signup" - "github.com/codeready-toolchain/registration-service/pkg/verification/sender" toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" @@ -37,6 +38,7 @@ const ( // ServiceImpl represents the implementation of the verification service. type ServiceImpl struct { // nolint:revive base.BaseService + namespaced.Client HTTPClient *http.Client NotificationService sender.NotificationSender } @@ -47,6 +49,7 @@ type VerificationServiceOption func(svc *ServiceImpl) func NewVerificationService(context servicecontext.ServiceContext, opts ...VerificationServiceOption) service.VerificationService { s := &ServiceImpl{ BaseService: base.NewBaseService(context), + Client: context.Client(), } for _, opt := range opts { @@ -444,8 +447,8 @@ func (s *ServiceImpl) VerifyActivationCode(ctx *gin.Context, userID, username, c annotationValues[toolchainv1alpha1.UserVerificationAttemptsAnnotationKey] = strconv.Itoa(attemptsMade) // look-up the SocialEvent - event, err := s.CRTClient().V1Alpha1().SocialEvents().Get(code) - if err != nil { + event := &toolchainv1alpha1.SocialEvent{} + if err := s.Get(gocontext.TODO(), s.NamespacedName(code), event); err != nil { if apierrors.IsNotFound(err) { // a SocialEvent was not found for the provided code return crterrors.NewForbiddenError("invalid code", "the provided code is invalid") diff --git a/pkg/verification/service/verification_service_test.go b/pkg/verification/service/verification_service_test.go index 65bd524f..ad4486c2 100644 --- a/pkg/verification/service/verification_service_test.go +++ b/pkg/verification/service/verification_service_test.go @@ -521,6 +521,7 @@ func (s *TestVerificationServiceSuite) TestInitVerificationFailsWhenPhoneNumberI Namespace: configuration.Namespace(), Labels: map[string]string{ toolchainv1alpha1.UserSignupUserPhoneHashLabelKey: phoneHash, + toolchainv1alpha1.UserSignupStateLabelKey: toolchainv1alpha1.UserSignupStateLabelValueApproved, }, }, Spec: toolchainv1alpha1.UserSignupSpec{ diff --git a/test/fake/mockable_application.go b/test/fake/mockable_application.go index d1ddf1d9..62d94fd1 100644 --- a/test/fake/mockable_application.go +++ b/test/fake/mockable_application.go @@ -3,12 +3,9 @@ package fake import ( "github.com/codeready-toolchain/registration-service/pkg/application/service" "github.com/codeready-toolchain/registration-service/pkg/application/service/factory" - "github.com/codeready-toolchain/registration-service/pkg/kubeclient" ) -func NewMockableApplication(crtClient kubeclient.CRTClient, - options ...factory.Option) *MockableApplication { - options = append(options, factory.WithServiceContextOptions(factory.CRTClientOption(crtClient))) +func NewMockableApplication(options ...factory.Option) *MockableApplication { return &MockableApplication{ serviceFactory: factory.NewServiceFactory(options...), } diff --git a/test/fake/proxy.go b/test/fake/proxy.go index c3ebacbc..8853f4e9 100644 --- a/test/fake/proxy.go +++ b/test/fake/proxy.go @@ -3,11 +3,10 @@ package fake import ( toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" "github.com/codeready-toolchain/registration-service/pkg/application/service" - "github.com/codeready-toolchain/registration-service/pkg/kubeclient" + "github.com/codeready-toolchain/registration-service/pkg/namespaced" "github.com/codeready-toolchain/registration-service/pkg/proxy/access" "github.com/codeready-toolchain/registration-service/pkg/signup" "github.com/gin-gonic/gin" - "sigs.k8s.io/controller-runtime/pkg/client" ) // This whole service abstraction is such a huge pain. We have to get rid of it!!! @@ -126,16 +125,12 @@ func (m *SignupService) PhoneNumberAlreadyInUse(_, _, _ string) error { } type MemberClusterServiceContext struct { - CrtClient kubeclient.CRTClient - Svcs service.Services + NamespacedClient namespaced.Client + Svcs service.Services } -func (sc MemberClusterServiceContext) CRTClient() kubeclient.CRTClient { - return sc.CrtClient -} - -func (sc MemberClusterServiceContext) Client() client.Client { - panic("shouldn't need informer in mock member cluster service") +func (sc MemberClusterServiceContext) Client() namespaced.Client { + return sc.NamespacedClient } func (sc MemberClusterServiceContext) Services() service.Services { diff --git a/test/testsuite.go b/test/testsuite.go index 9f6a35bb..30d85691 100644 --- a/test/testsuite.go +++ b/test/testsuite.go @@ -43,12 +43,12 @@ func (s *UnitTestSuite) SetupTest() { func (s *UnitTestSuite) SetupDefaultApplication() { // initialize the toolchainconfig cache s.DefaultConfig() - s.Application = fake.NewMockableApplication(nil, s.factoryOptions...) + s.Application = fake.NewMockableApplication(s.factoryOptions...) } func (s *UnitTestSuite) OverrideApplicationDefault(opts ...testconfig.ToolchainConfigOption) { s.SetConfig(opts...) - s.Application = fake.NewMockableApplication(nil, s.factoryOptions...) + s.Application = fake.NewMockableApplication(s.factoryOptions...) } func (s *UnitTestSuite) SetConfig(opts ...testconfig.ToolchainConfigOption) configuration.RegistrationServiceConfig { diff --git a/test/util/application.go b/test/util/application.go index 2ffdf116..54cfed82 100644 --- a/test/util/application.go +++ b/test/util/application.go @@ -7,7 +7,6 @@ import ( "github.com/codeready-toolchain/registration-service/pkg/application/service/factory" "github.com/codeready-toolchain/registration-service/pkg/server" commontest "github.com/codeready-toolchain/toolchain-common/pkg/test" - "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/runtime" ) @@ -18,7 +17,6 @@ func PrepareInClusterApp(t *testing.T, objects ...runtime.Object) (*commontest.F func PrepareInClusterAppWithOption(t *testing.T, option factory.Option, objects ...runtime.Object) (*commontest.FakeClient, application.Application) { fakeClient := commontest.NewFakeClient(t, objects...) - app, err := server.NewInClusterApplication(fakeClient, commontest.HostOperatorNs, option) - require.NoError(t, err) + app := server.NewInClusterApplication(fakeClient, commontest.HostOperatorNs, option) return fakeClient, app } diff --git a/test/util/service_context.go b/test/util/service_context.go index 41135a61..7847c4f8 100644 --- a/test/util/service_context.go +++ b/test/util/service_context.go @@ -3,21 +3,16 @@ package util import ( "testing" - "github.com/codeready-toolchain/registration-service/pkg/kubeclient" + "github.com/codeready-toolchain/registration-service/pkg/namespaced" "github.com/codeready-toolchain/registration-service/pkg/server" "github.com/codeready-toolchain/registration-service/test/fake" commontest "github.com/codeready-toolchain/toolchain-common/pkg/test" - "github.com/stretchr/testify/require" "sigs.k8s.io/controller-runtime/pkg/client" ) -func NewMemberClusterServiceContext(t *testing.T, cl client.Client) fake.MemberClusterServiceContext { - crtClient, err := kubeclient.NewCRTRESTClient(cl, commontest.HostOperatorNs) - require.NoError(t, err) - application, err := server.NewInClusterApplication(cl, commontest.HostOperatorNs) - require.NoError(t, err) +func NewMemberClusterServiceContext(_ *testing.T, cl client.Client) fake.MemberClusterServiceContext { return fake.MemberClusterServiceContext{ - CrtClient: crtClient, - Svcs: application, + NamespacedClient: namespaced.NewClient(cl, commontest.HostOperatorNs), + Svcs: server.NewInClusterApplication(cl, commontest.HostOperatorNs), } }