drop MemberClusterService interface & rename the implementation

pkg/application/service/factory/service_factory.go

@@ -8,7 +8,6 @@ import (
 	"github.com/codeready-toolchain/registration-service/pkg/configuration"
 	"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"
 )
@@ -51,10 +50,6 @@ func (s *ServiceFactory) defaultServiceContextProducer() servicecontext.ServiceC
 	}
 }
 
-func (s *ServiceFactory) MemberClusterService() service.MemberClusterService {
-	return clusterservice.NewMemberClusterService(s.getContext().Client(), s.getContext().Services().SignupService())
-}
-
 func (s *ServiceFactory) SignupService() service.SignupService {
 	return s.signupServiceFunc(s.signupServiceOptions...)
 }

pkg/application/service/services.go

@@ -2,7 +2,6 @@ package service
 
 import (
 	toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"
-	"github.com/codeready-toolchain/registration-service/pkg/proxy/access"
 	"github.com/codeready-toolchain/registration-service/pkg/signup"
 	"github.com/gin-gonic/gin"
 )
@@ -20,12 +19,7 @@ type VerificationService interface {
 	VerifyActivationCode(ctx *gin.Context, userID, username, code string) error
 }
 
-type MemberClusterService interface {
-	GetClusterAccess(userID, username, workspace, proxyPluginName string, publicViewerEnabled bool) (*access.ClusterAccess, error)
-}
-
 type Services interface {
 	SignupService() SignupService
 	VerificationService() VerificationService
-	MemberClusterService() MemberClusterService
 }

pkg/proxy/service/cluster_service.go → pkg/proxy/members.go

@@ -1,4 +1,4 @@
-package service
+package proxy
 
 import (
 	"context"
@@ -20,29 +20,24 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 )
 
-type Option func(f *ServiceImpl)
-
-// ServiceImpl represents the implementation of the member cluster service.
-type ServiceImpl struct { // nolint:revive
+// MemberClusters is a type that helps with retrieving access to a specific member cluster
+type MemberClusters struct { // nolint:revive
 	namespaced.Client
 	SignupService  service.SignupService
 	GetMembersFunc cluster.GetMemberClustersFunc
 }
 
-// NewMemberClusterService creates a service object for performing toolchain cluster related activities.
-func NewMemberClusterService(client namespaced.Client, signupService service.SignupService, options ...Option) service.MemberClusterService {
-	si := &ServiceImpl{
+// NewMemberClusters creates an instance of the MemberClusters type
+func NewMemberClusters(client namespaced.Client, signupService service.SignupService, getMembersFunc cluster.GetMemberClustersFunc) *MemberClusters {
+	si := &MemberClusters{
 		Client:         client,
 		SignupService:  signupService,
-		GetMembersFunc: cluster.GetMemberClusters,
-	}
-	for _, o := range options {
-		o(si)
+		GetMembersFunc: getMembersFunc,
 	}
 	return si
 }
 
-func (s *ServiceImpl) GetClusterAccess(userID, username, workspace, proxyPluginName string, publicViewerEnabled bool) (*access.ClusterAccess, error) {
+func (s *MemberClusters) GetClusterAccess(userID, username, workspace, proxyPluginName string, publicViewerEnabled bool) (*access.ClusterAccess, error) {
 	// if workspace is not provided then return the default space access
 	if workspace == "" {
 		return s.getClusterAccessForDefaultWorkspace(userID, username, proxyPluginName)
@@ -52,7 +47,7 @@ func (s *ServiceImpl) GetClusterAccess(userID, username, workspace, proxyPluginN
 }
 
 // getSpaceAccess retrieves space access for an user
-func (s *ServiceImpl) getSpaceAccess(userID, username, workspace, proxyPluginName string, publicViewerEnabled bool) (*access.ClusterAccess, error) {
+func (s *MemberClusters) getSpaceAccess(userID, username, workspace, proxyPluginName string, publicViewerEnabled bool) (*access.ClusterAccess, error) {
 	// retrieve the user's complaint name
 	complaintUserName, err := s.getUserSignupComplaintName(userID, username, publicViewerEnabled)
 	if err != nil {
@@ -70,7 +65,7 @@ func (s *ServiceImpl) getSpaceAccess(userID, username, workspace, proxyPluginNam
 	return s.accessForSpace(space, complaintUserName, proxyPluginName)
 }
 
-func (s *ServiceImpl) getUserSignupComplaintName(userID, username string, publicViewerEnabled bool) (string, error) {
+func (s *MemberClusters) getUserSignupComplaintName(userID, username string, publicViewerEnabled bool) (string, error) {
 	// if PublicViewer is enabled and the requested user is the PublicViewer, than no lookup is required
 	if publicViewerEnabled && username == toolchainv1alpha1.KubesawAuthenticatedUsername {
 		return username, nil
@@ -86,7 +81,7 @@ func (s *ServiceImpl) getUserSignupComplaintName(userID, username string, public
 }
 
 // getClusterAccessForDefaultWorkspace retrieves the cluster for the user's default workspace
-func (s *ServiceImpl) getClusterAccessForDefaultWorkspace(userID, username, proxyPluginName string) (*access.ClusterAccess, error) {
+func (s *MemberClusters) getClusterAccessForDefaultWorkspace(userID, username, proxyPluginName string) (*access.ClusterAccess, error) {
 	// retrieve the UserSignup from cache
 	userSignup, err := s.getSignupFromInformerForProvisionedUser(userID, username)
 	if err != nil {
@@ -97,7 +92,7 @@ func (s *ServiceImpl) getClusterAccessForDefaultWorkspace(userID, username, prox
 	return s.accessForCluster(userSignup.APIEndpoint, userSignup.ClusterName, userSignup.CompliantUsername, proxyPluginName)
 }
 
-func (s *ServiceImpl) getSignupFromInformerForProvisionedUser(userID, username string) (*signup.Signup, error) {
+func (s *MemberClusters) 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.SignupService.GetSignup(nil, userID, username, false)
@@ -115,7 +110,7 @@ func (s *ServiceImpl) getSignupFromInformerForProvisionedUser(userID, username s
 	return userSignup, nil
 }
 
-func (s *ServiceImpl) accessForSpace(space *toolchainv1alpha1.Space, username, proxyPluginName string) (*access.ClusterAccess, error) {
+func (s *MemberClusters) accessForSpace(space *toolchainv1alpha1.Space, username, proxyPluginName string) (*access.ClusterAccess, error) {
 	// Get the target member
 	members := s.GetMembersFunc()
 	if len(members) == 0 {
@@ -138,7 +133,7 @@ func (s *ServiceImpl) accessForSpace(space *toolchainv1alpha1.Space, username, p
 	return nil, errs.New(errMsg)
 }
 
-func (s *ServiceImpl) accessForCluster(apiEndpoint, clusterName, username, proxyPluginName string) (*access.ClusterAccess, error) {
+func (s *MemberClusters) accessForCluster(apiEndpoint, clusterName, username, proxyPluginName string) (*access.ClusterAccess, error) {
 	// Get the target member
 	members := s.GetMembersFunc()
 	if len(members) == 0 {
@@ -161,7 +156,7 @@ func (s *ServiceImpl) accessForCluster(apiEndpoint, clusterName, username, proxy
 	return nil, errs.New("no member cluster found for the user")
 }
 
-func (s *ServiceImpl) getMemberURL(proxyPluginName string, member *cluster.CachedToolchainCluster) (*url.URL, error) {
+func (s *MemberClusters) getMemberURL(proxyPluginName string, member *cluster.CachedToolchainCluster) (*url.URL, error) {
 	if member == nil {
 		return nil, errs.New("nil member provided")
 	}

pkg/proxy/service/cluster_service_test.go → pkg/proxy/members_test.go

@@ -1,4 +1,4 @@
-package service_test
+package proxy_test
 
 import (
 	"context"
@@ -8,8 +8,8 @@ import (
 	"testing"
 
 	"github.com/codeready-toolchain/registration-service/pkg/namespaced"
+	"github.com/codeready-toolchain/registration-service/pkg/proxy"
 	"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"
 	"github.com/codeready-toolchain/registration-service/test"
 	"github.com/codeready-toolchain/registration-service/test/fake"
@@ -29,15 +29,15 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-type TestClusterServiceSuite struct {
+type TestMemberClustersSuite struct {
 	test.UnitTestSuite
 }
 
-func TestRunClusterServiceSuite(t *testing.T) {
-	suite.Run(t, &TestClusterServiceSuite{test.UnitTestSuite{}})
+func TestRunMemberClustersSuite(t *testing.T) {
+	suite.Run(t, &TestMemberClustersSuite{test.UnitTestSuite{}})
 }
 
-func (s *TestClusterServiceSuite) TestGetClusterAccess() {
+func (s *TestMemberClustersSuite) TestGetClusterAccess() {
 	// given
 	sc := fake.NewSignupService(fake.Signup("123-noise", &signup.Signup{
 		CompliantUsername: "noise1",
@@ -88,7 +88,7 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() {
 		fake.NewSpace("unknown-cluster", "unknown-cluster", "unknown-cluster"),
 		pp)
 	nsClient := namespaced.NewClient(fakeClient, commontest.HostOperatorNs)
-	svc := service.NewMemberClusterService(nsClient, sc)
+	svc := proxy.NewMemberClusters(nsClient, sc, commoncluster.GetMemberClusters)
 
 	tt := map[string]struct {
 		publicViewerEnabled bool
@@ -166,7 +166,7 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() {
 						fakeClient.MockGet = nil
 					}()
 					nsClient := namespaced.NewClient(fakeClient, commontest.HostOperatorNs)
-					svc := service.NewMemberClusterService(nsClient, sc)
+					svc := proxy.NewMemberClusters(nsClient, sc, commoncluster.GetMemberClusters)
 
 					// when
 					_, err := svc.GetClusterAccess("789-ready", "", "smith2", "", publicViewerEnabled)
@@ -187,13 +187,9 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() {
 
 			s.Run("no member cluster found", func() {
 				s.Run("no member clusters", func() {
-					svc := service.NewMemberClusterService(nsClient, sc,
-						func(si *service.ServiceImpl) {
-							si.GetMembersFunc = func(_ ...commoncluster.Condition) []*commoncluster.CachedToolchainCluster {
-								return []*commoncluster.CachedToolchainCluster{}
-							}
-						},
-					)
+					svc := proxy.NewMemberClusters(nsClient, sc, func(_ ...commoncluster.Condition) []*commoncluster.CachedToolchainCluster {
+						return []*commoncluster.CachedToolchainCluster{}
+					})
 					s.Run("default workspace case", func() {
 						// when
 						_, err := svc.GetClusterAccess("789-ready", "", "", "", publicViewerEnabled)
@@ -212,14 +208,9 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() {
 				})
 
 				s.Run("no member cluster with the given URL", func() {
-					svc := service.NewMemberClusterService(nsClient, sc,
-						func(si *service.ServiceImpl) {
-							si.GetMembersFunc = func(_ ...commoncluster.Condition) []*commoncluster.CachedToolchainCluster {
-								return s.memberClusters()
-							}
-						},
-					)
-
+					svc := proxy.NewMemberClusters(nsClient, sc, func(_ ...commoncluster.Condition) []*commoncluster.CachedToolchainCluster {
+						return s.memberClusters()
+					})
 					s.Run("default workspace case", func() {
 						// when
 						_, err := svc.GetClusterAccess("012-ready-unknown-cluster", "", "", "", publicViewerEnabled)
@@ -270,14 +261,9 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() {
 					},
 				}
 
-				svc := service.NewMemberClusterService(nsClient, sc,
-					func(si *service.ServiceImpl) {
-						si.GetMembersFunc = func(_ ...commoncluster.Condition) []*commoncluster.CachedToolchainCluster {
-							return memberArray
-						}
-					},
-				)
-
+				svc := proxy.NewMemberClusters(nsClient, sc, func(_ ...commoncluster.Condition) []*commoncluster.CachedToolchainCluster {
+					return memberArray
+				})
 				s.Run("verify cluster access with route", func() {
 					memberClient.MockGet = func(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
 						route, ok := obj.(*routev1.Route)
@@ -434,14 +420,9 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() {
 		})
 
 		s.Run("get workspace by name", func() {
-			svc := service.NewMemberClusterService(nsClient, sc,
-				func(si *service.ServiceImpl) {
-					si.GetMembersFunc = func(_ ...commoncluster.Condition) []*commoncluster.CachedToolchainCluster {
-						return s.memberClusters()
-					}
-				},
-			)
-
+			svc := proxy.NewMemberClusters(nsClient, sc, func(_ ...commoncluster.Condition) []*commoncluster.CachedToolchainCluster {
+				return s.memberClusters()
+			})
 			s.Run("public-viewer is disabled", func() {
 				// when
 				ca, err := svc.GetClusterAccess("", toolchainv1alpha1.KubesawAuthenticatedUsername, "smith2", "", false)
@@ -486,14 +467,14 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() {
 	})
 }
 
-func (s *TestClusterServiceSuite) assertClusterAccess(expected, actual *access.ClusterAccess) {
+func (s *TestMemberClustersSuite) assertClusterAccess(expected, actual *access.ClusterAccess) {
 	require.NotNil(s.T(), expected)
 	require.NotNil(s.T(), actual)
 	assert.Equal(s.T(), expected.APIURL(), actual.APIURL())
 	assert.Equal(s.T(), expected.ImpersonatorToken(), actual.ImpersonatorToken())
 }
 
-func (s *TestClusterServiceSuite) memberClusters() []*commoncluster.CachedToolchainCluster {
+func (s *TestMemberClustersSuite) memberClusters() []*commoncluster.CachedToolchainCluster {
 	cls := make([]*commoncluster.CachedToolchainCluster, 0, 3)
 	for i := 0; i < 3; i++ {
 		clusterName := fmt.Sprintf("member-%d", i)

pkg/proxy/proxy.go

@@ -18,6 +18,7 @@ import (
 
 	toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"
 	"github.com/codeready-toolchain/registration-service/pkg/application"
+	"github.com/codeready-toolchain/registration-service/pkg/application/service"
 	"github.com/codeready-toolchain/registration-service/pkg/auth"
 	"github.com/codeready-toolchain/registration-service/pkg/configuration"
 	"github.com/codeready-toolchain/registration-service/pkg/context"
@@ -64,7 +65,7 @@ func authorizationEndpointTarget() string {
 
 type Proxy struct {
 	namespaced.Client
-	app            application.Application
+	signupService  service.SignupService
 	tokenParser    *auth.TokenParser
 	spaceLister    *handlers.SpaceLister
 	metrics        *metrics.ProxyMetrics
@@ -81,7 +82,7 @@ func NewProxy(nsClient namespaced.Client, app application.Application, proxyMetr
 	spaceLister := handlers.NewSpaceLister(nsClient, app, proxyMetrics)
 	return &Proxy{
 		Client:         nsClient,
-		app:            app,
+		signupService:  app.SignupService(),
 		tokenParser:    tokenParser,
 		spaceLister:    spaceLister,
 		metrics:        proxyMetrics,
@@ -296,7 +297,8 @@ func (p *Proxy) processRequest(ctx echo.Context) (string, *access.ClusterAccess,
 // processHomeWorkspaceRequest process an HTTP Request targeting the user's home workspace.
 func (p *Proxy) processHomeWorkspaceRequest(ctx echo.Context, userID, username, proxyPluginName string) (*access.ClusterAccess, error) {
 	// retrieves the ClusterAccess for the user and their home workspace
-	cluster, err := p.app.MemberClusterService().GetClusterAccess(userID, username, "", proxyPluginName, false)
+	members := NewMemberClusters(p.Client, p.signupService, p.getMembersFunc)
+	cluster, err := members.GetClusterAccess(userID, username, "", proxyPluginName, false)
 	if err != nil {
 		return nil, crterrors.NewInternalError(errs.New("unable to get target cluster"), err.Error())
 	}
@@ -374,7 +376,7 @@ func (p *Proxy) checkUserIsProvisioned(ctx echo.Context, userID, username string
 	//
 	// UserSignup complete status is not checked, since it might cause the proxy blocking the request
 	// and returning an error when quick transitions from ready to provisioning are happening.
-	userSignup, err := p.app.SignupService().GetSignup(nil, userID, username, false)
+	userSignup, err := p.signupService.GetSignup(nil, userID, username, false)
 	if err != nil {
 		return err
 	}
@@ -410,16 +412,17 @@ func (p *Proxy) getClusterAccess(ctx echo.Context, userID, username, proxyPlugin
 // this function returns an error.
 func (p *Proxy) getClusterAccessAsUserOrPublicViewer(ctx echo.Context, userID, username, proxyPluginName string, workspace *toolchainv1alpha1.Workspace) (*access.ClusterAccess, error) {
 	// retrieve the requesting user's UserSignup
-	userSignup, err := p.app.SignupService().GetSignup(nil, userID, username, false)
+	userSignup, err := p.signupService.GetSignup(nil, userID, username, false)
 	if err != nil {
 		log.Error(nil, err, fmt.Sprintf("error retrieving user signup for userID '%s' and username '%s'", userID, username))
 		return nil, crterrors.NewInternalError(errs.New("unable to get user info"), "error retrieving user")
 	}
 
 	// proceed as PublicViewer if the feature is enabled and userSignup is nil
 	publicViewerEnabled := context.IsPublicViewerEnabled(ctx)
+	members := NewMemberClusters(p.Client, p.signupService, p.getMembersFunc)
 	if publicViewerEnabled && !userHasDirectAccess(userSignup, workspace) {
-		return p.app.MemberClusterService().GetClusterAccess(
+		return members.GetClusterAccess(
 			toolchainv1alpha1.KubesawAuthenticatedUsername,
 			toolchainv1alpha1.KubesawAuthenticatedUsername,
 			workspace.Name,
@@ -428,7 +431,7 @@ func (p *Proxy) getClusterAccessAsUserOrPublicViewer(ctx echo.Context, userID, u
 	}
 
 	// otherwise retrieve the ClusterAccess for the cluster hosting the workspace and the given user.
-	return p.app.MemberClusterService().GetClusterAccess(userID, username, workspace.Name, proxyPluginName, publicViewerEnabled)
+	return members.GetClusterAccess(userID, username, workspace.Name, proxyPluginName, publicViewerEnabled)
 }
 
 // userHasDirectAccess checks if an UserSignup has access to a workspace.