Skip to content

Commit

Permalink
Merge remote-tracking branch 'upstream/master' into deprecate-tc-spec…
Browse files Browse the repository at this point in the history
…-fields-contd2
  • Loading branch information
metlos committed Oct 9, 2024
2 parents aebef38 + 4797d02 commit 64cad5d
Show file tree
Hide file tree
Showing 16 changed files with 97 additions and 271 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/publish-operators-for-e2e-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ jobs:
# Is executed only for comment events - in that case the pull_request field is empty
- name: Send Github API Request to get PR data
id: request
uses: octokit/request-action@v2.3.1
uses: octokit/request-action@v2.4.0
if: ${{ github.event.pull_request == '' }}
with:
route: ${{ github.event.issue.pull_request.url }}
Expand Down
4 changes: 2 additions & 2 deletions 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 Expand Up @@ -111,7 +111,7 @@ func NewServiceFactory(options ...Option) *ServiceFactory {

if f.signupServiceFunc == nil {
f.signupServiceFunc = func(_ ...signupservice.SignupServiceOption) service.SignupService {
return signupservice.NewSignupService(f.getContext(), f.signupServiceOptions...)
return signupservice.NewSignupService(f.getContext().Client(), f.signupServiceOptions...)
}
}

Expand Down
3 changes: 0 additions & 3 deletions pkg/application/service/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,7 @@ import (
type InformerService interface {
GetMasterUserRecord(name string) (*toolchainv1alpha1.MasterUserRecord, error)
GetSpace(name string) (*toolchainv1alpha1.Space, error)
GetToolchainStatus() (*toolchainv1alpha1.ToolchainStatus, error)
GetUserSignup(name string) (*toolchainv1alpha1.UserSignup, 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)
}
Expand Down
21 changes: 0 additions & 21 deletions pkg/informers/service/informer_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand All @@ -50,20 +43,6 @@ func (s *ServiceImpl) GetSpace(name string) (*toolchainv1alpha1.Space, error) {
return space, err
}

func (s *ServiceImpl) GetToolchainStatus() (*toolchainv1alpha1.ToolchainStatus, error) {
status := &toolchainv1alpha1.ToolchainStatus{}
namespacedName := types.NamespacedName{Name: "toolchain-status", Namespace: s.namespace}
err := s.client.Get(context.TODO(), namespacedName, status)
return status, err
}

func (s *ServiceImpl) GetUserSignup(name string) (*toolchainv1alpha1.UserSignup, error) {
signup := &toolchainv1alpha1.UserSignup{}
namespacedName := types.NamespacedName{Name: name, Namespace: s.namespace}
err := s.client.Get(context.TODO(), namespacedName, signup)
return signup, err
}

func (s *ServiceImpl) ListSpaceBindings(reqs ...labels.Requirement) ([]toolchainv1alpha1.SpaceBinding, error) {
selector := labels.NewSelector()
for i := range reqs {
Expand Down
67 changes: 0 additions & 67 deletions pkg/informers/service/informer_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,73 +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("toolchainstatuses", func(t *testing.T) {
t.Run("not found", func(t *testing.T) {
// given
client := test.NewFakeClient(t)
svc := service.NewInformerService(client, test.HostOperatorNs)

// when
val, err := svc.GetToolchainStatus()

// then
assert.Empty(t, val)
assert.EqualError(t, err, "toolchainstatuses.toolchain.dev.openshift.com \"toolchain-status\" not found")
})

t.Run("found", func(t *testing.T) {
// when
val, err := svc.GetToolchainStatus()

// then
require.NotNil(t, val)
require.NoError(t, err)
assert.Equal(t, status.Status, val.Status)
})
})

t.Run("usersignups", func(t *testing.T) {
t.Run("not found", func(t *testing.T) {
// when
val, err := svc.GetUserSignup("unknown")

// then
assert.Empty(t, val)
assert.EqualError(t, err, "usersignups.toolchain.dev.openshift.com \"unknown\" not found")
})

t.Run("found", func(t *testing.T) {
// when
val, err := svc.GetUserSignup("johnUserSignup")

// then
require.NotNil(t, val)
require.NoError(t, err)
assert.Equal(t, signupJohn.Spec, val.Spec)
})
})

t.Run("bannedusers", func(t *testing.T) {
t.Run("not found", func(t *testing.T) {
// when
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
14 changes: 0 additions & 14 deletions pkg/signup/service/resource_provider.go

This file was deleted.

Loading

0 comments on commit 64cad5d

Please sign in to comment.