diff --git a/pkg/application/service/factory/service_factory.go b/pkg/application/service/factory/service_factory.go index 905c29d7..67f947ba 100644 --- a/pkg/application/service/factory/service_factory.go +++ b/pkg/application/service/factory/service_factory.go @@ -57,7 +57,7 @@ func (s *ServiceFactory) InformerService() service.InformerService { } func (s *ServiceFactory) MemberClusterService() service.MemberClusterService { - return clusterservice.NewMemberClusterService(s.getContext()) + return clusterservice.NewMemberClusterService(s.getContext().Client(), s.getContext().Services().SignupService()) } func (s *ServiceFactory) SignupService() service.SignupService { diff --git a/pkg/application/service/services.go b/pkg/application/service/services.go index 96cca4cd..4df4130f 100644 --- a/pkg/application/service/services.go +++ b/pkg/application/service/services.go @@ -12,7 +12,6 @@ type InformerService interface { GetMasterUserRecord(name string) (*toolchainv1alpha1.MasterUserRecord, error) GetSpace(name string) (*toolchainv1alpha1.Space, error) ListSpaceBindings(reqs ...labels.Requirement) ([]toolchainv1alpha1.SpaceBinding, error) - GetProxyPluginConfig(name string) (*toolchainv1alpha1.ProxyPlugin, error) GetNSTemplateTier(name string) (*toolchainv1alpha1.NSTemplateTier, error) ListBannedUsersByEmail(email string) ([]toolchainv1alpha1.BannedUser, error) } diff --git a/pkg/informers/service/informer_service.go b/pkg/informers/service/informer_service.go index 424dd665..269d1952 100644 --- a/pkg/informers/service/informer_service.go +++ b/pkg/informers/service/informer_service.go @@ -29,13 +29,6 @@ func NewInformerService(client client.Client, namespace string) service.Informer return si } -func (s *ServiceImpl) GetProxyPluginConfig(name string) (*toolchainv1alpha1.ProxyPlugin, error) { - pluginConfig := &toolchainv1alpha1.ProxyPlugin{} - namespacedName := types.NamespacedName{Name: name, Namespace: s.namespace} - err := s.client.Get(context.TODO(), namespacedName, pluginConfig) - return pluginConfig, err -} - func (s *ServiceImpl) GetMasterUserRecord(name string) (*toolchainv1alpha1.MasterUserRecord, error) { mur := &toolchainv1alpha1.MasterUserRecord{} namespacedName := types.NamespacedName{Name: name, Namespace: s.namespace} diff --git a/pkg/informers/service/informer_service_test.go b/pkg/informers/service/informer_service_test.go index 881c51f7..832bf78e 100644 --- a/pkg/informers/service/informer_service_test.go +++ b/pkg/informers/service/informer_service_test.go @@ -136,27 +136,6 @@ func TestInformerService(t *testing.T) { }) }) - t.Run("proxy configs", func(t *testing.T) { - t.Run("not found", func(t *testing.T) { - // when - val, err := svc.GetProxyPluginConfig("unknown") - - // then - assert.Empty(t, val) - assert.EqualError(t, err, "proxyplugins.toolchain.dev.openshift.com \"unknown\" not found") - }) - - t.Run("found", func(t *testing.T) { - // when - val, err := svc.GetProxyPluginConfig("tekton-results") - - // then - require.NotNil(t, val) - require.NoError(t, err) - assert.Equal(t, pluginTekton.Spec, val.Spec) - }) - }) - t.Run("bannedusers", func(t *testing.T) { t.Run("not found", func(t *testing.T) { // when diff --git a/pkg/proxy/proxy_community_test.go b/pkg/proxy/proxy_community_test.go index 5274bd75..3e59b324 100644 --- a/pkg/proxy/proxy_community_test.go +++ b/pkg/proxy/proxy_community_test.go @@ -9,6 +9,7 @@ import ( appservice "github.com/codeready-toolchain/registration-service/pkg/application/service" "github.com/codeready-toolchain/registration-service/pkg/auth" infservice "github.com/codeready-toolchain/registration-service/pkg/informers/service" + "github.com/codeready-toolchain/registration-service/pkg/namespaced" "github.com/codeready-toolchain/registration-service/pkg/proxy/handlers" "github.com/codeready-toolchain/registration-service/pkg/signup" "github.com/codeready-toolchain/registration-service/test/fake" @@ -144,11 +145,12 @@ func (s *TestProxySuite) checkProxyCommunityOK(fakeApp *fake.ProxyFakeApp, p *Pr // configure informer inf := infservice.NewInformerService(cli, commontest.HostOperatorNs) + nsClient := namespaced.NewClient(cli, commontest.HostOperatorNs) // configure Application fakeApp.Err = nil fakeApp.InformerServiceMock = inf - fakeApp.MemberClusterServiceMock = s.newMemberClusterServiceWithMembers(testServer.URL) + fakeApp.MemberClusterServiceMock = s.newMemberClusterServiceWithMembers(nsClient, signupService, testServer.URL) fakeApp.SignupServiceMock = signupService s.Application.MockSignupService(signupService) diff --git a/pkg/proxy/proxy_test.go b/pkg/proxy/proxy_test.go index 46d8fe13..f803789b 100644 --- a/pkg/proxy/proxy_test.go +++ b/pkg/proxy/proxy_test.go @@ -20,6 +20,7 @@ import ( appservice "github.com/codeready-toolchain/registration-service/pkg/application/service" "github.com/codeready-toolchain/registration-service/pkg/auth" infservice "github.com/codeready-toolchain/registration-service/pkg/informers/service" + "github.com/codeready-toolchain/registration-service/pkg/namespaced" "github.com/codeready-toolchain/registration-service/pkg/proxy/access" "github.com/codeready-toolchain/registration-service/pkg/proxy/handlers" "github.com/codeready-toolchain/registration-service/pkg/proxy/metrics" @@ -850,9 +851,10 @@ func (s *TestProxySuite) checkProxyOK(fakeApp *fake.ProxyFakeApp, p *Proxy) { proxyPlugin, fake.NewBase1NSTemplateTier()) inf := infservice.NewInformerService(fakeClient, commontest.HostOperatorNs) + nsClient := namespaced.NewClient(fakeClient, commontest.HostOperatorNs) s.Application.MockInformerService(inf) - fakeApp.MemberClusterServiceMock = s.newMemberClusterServiceWithMembers(testServer.URL) + fakeApp.MemberClusterServiceMock = s.newMemberClusterServiceWithMembers(nsClient, fakeApp.SignupServiceMock, testServer.URL) fakeApp.InformerServiceMock = inf p.spaceLister = &handlers.SpaceLister{ @@ -900,7 +902,7 @@ type headerToAdd struct { key, value string } -func (s *TestProxySuite) newMemberClusterServiceWithMembers(serverURL string) appservice.MemberClusterService { +func (s *TestProxySuite) newMemberClusterServiceWithMembers(nsClient namespaced.Client, signupService appservice.SignupService, serverURL string) appservice.MemberClusterService { serverHost := serverURL switch { case strings.HasPrefix(serverURL, "http://"): @@ -911,7 +913,7 @@ func (s *TestProxySuite) newMemberClusterServiceWithMembers(serverURL string) ap route := &routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - Namespace: commontest.HostOperatorNs, + Namespace: nsClient.Namespace, Name: "proxy-plugin", }, Spec: routev1.RouteSpec{ @@ -925,11 +927,9 @@ func (s *TestProxySuite) newMemberClusterServiceWithMembers(serverURL string) ap }, }, } - fakeClient := commontest.NewFakeClient(s.T(), route) return service.NewMemberClusterService( - fake.MemberClusterServiceContext{ - Svcs: s.Application, - }, + nsClient, + signupService, func(si *service.ServiceImpl) { si.GetMembersFunc = func(_ ...commoncluster.Condition) []*commoncluster.CachedToolchainCluster { return []*commoncluster.CachedToolchainCluster{ @@ -949,7 +949,7 @@ func (s *TestProxySuite) newMemberClusterServiceWithMembers(serverURL string) ap BearerToken: "clusterSAToken", }, }, - Client: fakeClient, + Client: commontest.NewFakeClient(s.T(), route), }, } } diff --git a/pkg/proxy/service/cluster_service.go b/pkg/proxy/service/cluster_service.go index 29892b63..4785458a 100644 --- a/pkg/proxy/service/cluster_service.go +++ b/pkg/proxy/service/cluster_service.go @@ -7,9 +7,8 @@ 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/application/service/base" - servicecontext "github.com/codeready-toolchain/registration-service/pkg/application/service/context" "github.com/codeready-toolchain/registration-service/pkg/log" + "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/codeready-toolchain/toolchain-common/pkg/cluster" @@ -25,14 +24,16 @@ type Option func(f *ServiceImpl) // ServiceImpl represents the implementation of the member cluster service. type ServiceImpl struct { // nolint:revive - base.BaseService + namespaced.Client + SignupService service.SignupService GetMembersFunc cluster.GetMemberClustersFunc } // NewMemberClusterService creates a service object for performing toolchain cluster related activities. -func NewMemberClusterService(context servicecontext.ServiceContext, options ...Option) service.MemberClusterService { +func NewMemberClusterService(client namespaced.Client, signupService service.SignupService, options ...Option) service.MemberClusterService { si := &ServiceImpl{ - BaseService: base.NewBaseService(context), + Client: client, + SignupService: signupService, GetMembersFunc: cluster.GetMemberClusters, } for _, o := range options { @@ -59,8 +60,8 @@ func (s *ServiceImpl) getSpaceAccess(userID, username, workspace, proxyPluginNam } // look up space - space, err := s.Services().InformerService().GetSpace(workspace) - if err != nil { + space := &toolchainv1alpha1.Space{} + if err := s.Get(context.TODO(), s.NamespacedName(workspace), space); err != nil { // log the actual error but do not return it so that it doesn't reveal information about a space that may not belong to the requestor log.Error(nil, err, "unable to get target cluster for workspace "+workspace) return nil, fmt.Errorf("the requested space is not available") @@ -99,7 +100,7 @@ func (s *ServiceImpl) getClusterAccessForDefaultWorkspace(userID, username, prox func (s *ServiceImpl) getSignupFromInformerForProvisionedUser(userID, username string) (*signup.Signup, error) { // don't check for usersignup complete status, since it might cause the proxy blocking the request // and returning an error when quick transitions from ready to provisioning are happening. - userSignup, err := s.Services().SignupService().GetSignupFromInformer(nil, userID, username, false) + userSignup, err := s.SignupService.GetSignupFromInformer(nil, userID, username, false) if err != nil { return nil, err } @@ -170,8 +171,8 @@ func (s *ServiceImpl) getMemberURL(proxyPluginName string, member *cluster.Cache if member.Client == nil { return nil, errs.New(fmt.Sprintf("client for member %s not set", member.Name)) } - proxyCfg, err := s.Services().InformerService().GetProxyPluginConfig(proxyPluginName) - if err != nil { + proxyCfg := &toolchainv1alpha1.ProxyPlugin{} + if err := s.Get(context.TODO(), s.NamespacedName(proxyPluginName), proxyCfg); err != nil { return nil, errs.New(fmt.Sprintf("unable to get proxy config %s: %s", proxyPluginName, err.Error())) } if proxyCfg.Spec.OpenShiftRouteTargetEndpoint == nil { @@ -185,7 +186,7 @@ func (s *ServiceImpl) getMemberURL(proxyPluginName string, member *cluster.Cache Namespace: routeNamespace, Name: routeName, } - err = member.Client.Get(context.Background(), key, proxyRoute) + err := member.Client.Get(context.Background(), key, proxyRoute) if err != nil { return nil, err } diff --git a/pkg/proxy/service/cluster_service_test.go b/pkg/proxy/service/cluster_service_test.go index aa50a7f7..00fad999 100644 --- a/pkg/proxy/service/cluster_service_test.go +++ b/pkg/proxy/service/cluster_service_test.go @@ -8,6 +8,7 @@ import ( "testing" infservice "github.com/codeready-toolchain/registration-service/pkg/informers/service" + "github.com/codeready-toolchain/registration-service/pkg/namespaced" "github.com/codeready-toolchain/registration-service/pkg/proxy/access" "github.com/codeready-toolchain/registration-service/pkg/proxy/service" "github.com/codeready-toolchain/registration-service/pkg/signup" @@ -88,14 +89,8 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() { fake.NewSpace("smith2", "member-2", "smith2"), fake.NewSpace("unknown-cluster", "unknown-cluster", "unknown-cluster"), pp) - inf := infservice.NewInformerService(fakeClient, commontest.HostOperatorNs) - s.Application.MockInformerService(inf) - - svc := service.NewMemberClusterService( - fake.MemberClusterServiceContext{ - Svcs: s.Application, - }, - ) + nsClient := namespaced.NewClient(fakeClient, commontest.HostOperatorNs) + svc := service.NewMemberClusterService(nsClient, sc) tt := map[string]struct { publicViewerEnabled bool @@ -193,10 +188,7 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() { s.Run("no member cluster found", func() { s.Run("no member clusters", func() { - svc := service.NewMemberClusterService( - fake.MemberClusterServiceContext{ - Svcs: s.Application, - }, + svc := service.NewMemberClusterService(nsClient, sc, func(si *service.ServiceImpl) { si.GetMembersFunc = func(_ ...commoncluster.Condition) []*commoncluster.CachedToolchainCluster { return []*commoncluster.CachedToolchainCluster{} @@ -221,10 +213,7 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() { }) s.Run("no member cluster with the given URL", func() { - svc := service.NewMemberClusterService( - fake.MemberClusterServiceContext{ - Svcs: s.Application, - }, + svc := service.NewMemberClusterService(nsClient, sc, func(si *service.ServiceImpl) { si.GetMembersFunc = func(_ ...commoncluster.Condition) []*commoncluster.CachedToolchainCluster { return s.memberClusters() @@ -282,10 +271,7 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() { }, } - svc := service.NewMemberClusterService( - fake.MemberClusterServiceContext{ - Svcs: s.Application, - }, + svc := service.NewMemberClusterService(nsClient, sc, func(si *service.ServiceImpl) { si.GetMembersFunc = func(_ ...commoncluster.Condition) []*commoncluster.CachedToolchainCluster { return memberArray @@ -449,10 +435,7 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() { }) s.Run("get workspace by name", func() { - svc := service.NewMemberClusterService( - fake.MemberClusterServiceContext{ - Svcs: s.Application, - }, + svc := service.NewMemberClusterService(nsClient, sc, func(si *service.ServiceImpl) { si.GetMembersFunc = func(_ ...commoncluster.Condition) []*commoncluster.CachedToolchainCluster { return s.memberClusters() diff --git a/pkg/verification/service/verification_service.go b/pkg/verification/service/verification_service.go index 7fdd3919..c8ce7831 100644 --- a/pkg/verification/service/verification_service.go +++ b/pkg/verification/service/verification_service.go @@ -1,7 +1,6 @@ package service import ( - gocontext "context" "crypto/rand" "errors" "fmt" @@ -448,7 +447,7 @@ func (s *ServiceImpl) VerifyActivationCode(ctx *gin.Context, userID, username, c // look-up the SocialEvent event := &toolchainv1alpha1.SocialEvent{} - if err := s.Get(gocontext.TODO(), s.NamespacedName(code), event); err != nil { + if err := s.Get(ctx, 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/test/fake/mockable_application.go b/test/fake/mockable_application.go index 62d94fd1..8fed8c2d 100644 --- a/test/fake/mockable_application.go +++ b/test/fake/mockable_application.go @@ -12,11 +12,10 @@ func NewMockableApplication(options ...factory.Option) *MockableApplication { } type MockableApplication struct { - serviceFactory *factory.ServiceFactory - mockInformerService service.InformerService - mockSignupService service.SignupService - mockVerificationService service.VerificationService - mockMemberClusterService service.MemberClusterService + serviceFactory *factory.ServiceFactory + mockInformerService service.InformerService + mockSignupService service.SignupService + mockVerificationService service.VerificationService } func (m *MockableApplication) SignupService() service.SignupService { @@ -38,9 +37,6 @@ func (m *MockableApplication) VerificationService() service.VerificationService } func (m *MockableApplication) MemberClusterService() service.MemberClusterService { - if m.mockMemberClusterService != nil { - return m.mockMemberClusterService - } return m.serviceFactory.MemberClusterService() } diff --git a/test/fake/proxy.go b/test/fake/proxy.go index 8853f4e9..92133d28 100644 --- a/test/fake/proxy.go +++ b/test/fake/proxy.go @@ -3,7 +3,6 @@ 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/namespaced" "github.com/codeready-toolchain/registration-service/pkg/proxy/access" "github.com/codeready-toolchain/registration-service/pkg/signup" "github.com/gin-gonic/gin" @@ -123,16 +122,3 @@ func (m *SignupService) UpdateUserSignup(_ *toolchainv1alpha1.UserSignup) (*tool func (m *SignupService) PhoneNumberAlreadyInUse(_, _, _ string) error { return nil } - -type MemberClusterServiceContext struct { - NamespacedClient namespaced.Client - Svcs service.Services -} - -func (sc MemberClusterServiceContext) Client() namespaced.Client { - return sc.NamespacedClient -} - -func (sc MemberClusterServiceContext) Services() service.Services { - return sc.Svcs -}