Skip to content

Commit

Permalink
drop BaseService from ClusterService & use Client instead
Browse files Browse the repository at this point in the history
  • Loading branch information
MatousJobanek committed Oct 4, 2024
1 parent 437da3e commit 6e62189
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 69 deletions.
2 changes: 1 addition & 1 deletion pkg/application/service/factory/service_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 3 additions & 1 deletion pkg/proxy/proxy_community_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
16 changes: 8 additions & 8 deletions pkg/proxy/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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://"):
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -949,7 +949,7 @@ func (s *TestProxySuite) newMemberClusterServiceWithMembers(serverURL string) ap
BearerToken: "clusterSAToken",
},
},
Client: fakeClient,
Client: commontest.NewFakeClient(s.T(), route),
},
}
}
Expand Down
23 changes: 12 additions & 11 deletions pkg/proxy/service/cluster_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {
Expand All @@ -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")
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
Expand Down
31 changes: 7 additions & 24 deletions pkg/proxy/service/cluster_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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{}
Expand All @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
3 changes: 1 addition & 2 deletions pkg/verification/service/verification_service.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package service

import (
gocontext "context"
"crypto/rand"
"errors"
"fmt"
Expand Down Expand Up @@ -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")
Expand Down
12 changes: 4 additions & 8 deletions test/fake/mockable_application.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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()
}

Expand Down
14 changes: 0 additions & 14 deletions test/fake/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

0 comments on commit 6e62189

Please sign in to comment.