Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

replace methods of the CRTRestClient with the cached client #462

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions pkg/informers/service/informer_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ 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/resources"
"github.com/codeready-toolchain/toolchain-common/pkg/hash"

"k8s.io/apimachinery/pkg/labels"
Expand Down Expand Up @@ -53,7 +52,7 @@ func (s *ServiceImpl) GetSpace(name string) (*toolchainv1alpha1.Space, error) {

func (s *ServiceImpl) GetToolchainStatus() (*toolchainv1alpha1.ToolchainStatus, error) {
status := &toolchainv1alpha1.ToolchainStatus{}
namespacedName := types.NamespacedName{Name: resources.ToolchainStatusName, Namespace: s.namespace}
namespacedName := types.NamespacedName{Name: "toolchain-status", Namespace: s.namespace}
err := s.client.Get(context.TODO(), namespacedName, status)
return status, err
}
Expand Down
32 changes: 5 additions & 27 deletions pkg/kubeclient/banneduser.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,10 @@

import (
"context"
"fmt"

crtapi "github.com/codeready-toolchain/api/api/v1alpha1"
"github.com/codeready-toolchain/toolchain-common/pkg/hash"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/dynamic"
)

const (
bannedUserResourcePlural = "bannedusers"
"sigs.k8s.io/controller-runtime/pkg/client"
)

type bannedUserClient struct {
Expand Down Expand Up @@ -50,24 +42,10 @@

// 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) {

intf, err := dynamic.NewForConfig(&c.cfg)
if err != nil {
bannedUsers := &crtapi.BannedUserList{}
if err := c.client.List(context.TODO(), bannedUsers, client.InNamespace(c.ns),
client.MatchingLabels{labelKey: labelValue}); err != nil {

Check warning on line 47 in pkg/kubeclient/banneduser.go

View check run for this annotation

Codecov / codecov/patch

pkg/kubeclient/banneduser.go#L45-L47

Added lines #L45 - L47 were not covered by tests
return nil, err
}

r := schema.GroupVersionResource{Group: "toolchain.dev.openshift.com", Version: "v1alpha1", Resource: bannedUserResourcePlural}
listOptions := metav1.ListOptions{
LabelSelector: fmt.Sprintf("%s=%s", labelKey, labelValue),
}

list, err := intf.Resource(r).Namespace(c.ns).List(context.TODO(), listOptions)
if err != nil {
return nil, err
}

result := &crtapi.BannedUserList{}

err = c.crtClient.scheme.Convert(list, result, nil)
return result, err
return bannedUsers, nil

Check warning on line 50 in pkg/kubeclient/banneduser.go

View check run for this annotation

Codecov / codecov/patch

pkg/kubeclient/banneduser.go#L50

Added line #L50 was not covered by tests
}
116 changes: 32 additions & 84 deletions pkg/kubeclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
"sigs.k8s.io/controller-runtime/pkg/client"

"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/serializer"
"k8s.io/client-go/rest"
)

type CRTClient interface {
Expand All @@ -24,62 +22,28 @@
}

// NewCRTRESTClient creates a new REST client for managing Codeready Toolchain resources via the Kubernetes API
func NewCRTRESTClient(cfg *rest.Config, client client.Client, namespace string) (CRTClient, error) {
func NewCRTRESTClient(client client.Client, namespace string) (CRTClient, error) {
scheme := runtime.NewScheme()
err := crtapi.SchemeBuilder.AddToScheme(scheme)
if err != nil {
return nil, err
}
crtapi.SchemeBuilder.Register(getRegisterObject()...)

config := *cfg
config.GroupVersion = &crtapi.GroupVersion
config.APIPath = "/apis"
config.ContentType = runtime.ContentTypeJSON
config.NegotiatedSerializer = serializer.WithoutConversionCodecFactory{CodecFactory: serializer.NewCodecFactory(scheme)}
config.UserAgent = rest.DefaultKubernetesUserAgent()

restClient, err := rest.RESTClientFor(&config)
if err != nil {
return nil, err
}

crtRESTClient := &CRTRESTClient{
RestClient: restClient,
Client: client,
Config: config,
NS: namespace,
Scheme: scheme,
Client: client,
NS: namespace,
Scheme: scheme,
}

crtRESTClient.v1Alpha1 = &V1Alpha1REST{client: crtRESTClient}
return crtRESTClient, nil
}

func getRegisterObject() []runtime.Object {
return []runtime.Object{
&crtapi.UserSignup{},
&crtapi.UserSignupList{},
&crtapi.MasterUserRecord{},
&crtapi.MasterUserRecordList{},
&crtapi.BannedUser{},
&crtapi.BannedUserList{},
&crtapi.ToolchainStatus{},
&crtapi.ToolchainStatusList{},
&crtapi.Space{},
&crtapi.SpaceList{},
&crtapi.SpaceBinding{},
&crtapi.SpaceBindingList{},
}
}

type CRTRESTClient struct {
RestClient rest.Interface
Client client.Client
NS string
Config rest.Config
Scheme *runtime.Scheme
v1Alpha1 *V1Alpha1REST
Client client.Client
NS string
Scheme *runtime.Scheme
v1Alpha1 *V1Alpha1REST
}

func (c *CRTRESTClient) V1Alpha1() V1Alpha1 {
Expand All @@ -94,11 +58,9 @@
func (c *V1Alpha1REST) UserSignups() UserSignupInterface {
return &userSignupClient{
crtClient: crtClient{
restClient: c.client.RestClient,
client: c.client.Client,
ns: c.client.NS,
cfg: c.client.Config,
scheme: c.client.Scheme,
client: c.client.Client,
ns: c.client.NS,
scheme: c.client.Scheme,
},
}
}
Expand All @@ -107,11 +69,9 @@
func (c *V1Alpha1REST) MasterUserRecords() MasterUserRecordInterface {
return &masterUserRecordClient{
crtClient: crtClient{
restClient: c.client.RestClient,
client: c.client.Client,
ns: c.client.NS,
cfg: c.client.Config,
scheme: c.client.Scheme,
client: c.client.Client,
ns: c.client.NS,
scheme: c.client.Scheme,
},
}
}
Expand All @@ -120,11 +80,9 @@
func (c *V1Alpha1REST) BannedUsers() BannedUserInterface {
return &bannedUserClient{
crtClient: crtClient{
restClient: c.client.RestClient,
client: c.client.Client,
ns: c.client.NS,
cfg: c.client.Config,
scheme: c.client.Scheme,
client: c.client.Client,
ns: c.client.NS,
scheme: c.client.Scheme,
},
}
}
Expand All @@ -133,11 +91,9 @@
func (c *V1Alpha1REST) ToolchainStatuses() ToolchainStatusInterface {
return &toolchainStatusClient{
crtClient: crtClient{
restClient: c.client.RestClient,
client: c.client.Client,
ns: c.client.NS,
cfg: c.client.Config,
scheme: c.client.Scheme,
client: c.client.Client,
ns: c.client.NS,
scheme: c.client.Scheme,
},
}
}
Expand All @@ -146,11 +102,9 @@
func (c *V1Alpha1REST) SocialEvents() SocialEventInterface {
return &socialeventClient{
crtClient: crtClient{
restClient: c.client.RestClient,
client: c.client.Client,
ns: c.client.NS,
cfg: c.client.Config,
scheme: c.client.Scheme,
client: c.client.Client,
ns: c.client.NS,
scheme: c.client.Scheme,

Check warning on line 107 in pkg/kubeclient/client.go

View check run for this annotation

Codecov / codecov/patch

pkg/kubeclient/client.go#L105-L107

Added lines #L105 - L107 were not covered by tests
},
}
}
Expand All @@ -159,11 +113,9 @@
func (c *V1Alpha1REST) Spaces() SpaceInterface {
return &spaceClient{
crtClient: crtClient{
restClient: c.client.RestClient,
client: c.client.Client,
ns: c.client.NS,
cfg: c.client.Config,
scheme: c.client.Scheme,
client: c.client.Client,
ns: c.client.NS,
scheme: c.client.Scheme,

Check warning on line 118 in pkg/kubeclient/client.go

View check run for this annotation

Codecov / codecov/patch

pkg/kubeclient/client.go#L116-L118

Added lines #L116 - L118 were not covered by tests
},
}
}
Expand All @@ -172,19 +124,15 @@
func (c *V1Alpha1REST) SpaceBindings() SpaceBindingInterface {
return &spaceBindingClient{
crtClient: crtClient{
restClient: c.client.RestClient,
client: c.client.Client,
ns: c.client.NS,
cfg: c.client.Config,
scheme: c.client.Scheme,
client: c.client.Client,
ns: c.client.NS,
scheme: c.client.Scheme,

Check warning on line 129 in pkg/kubeclient/client.go

View check run for this annotation

Codecov / codecov/patch

pkg/kubeclient/client.go#L127-L129

Added lines #L127 - L129 were not covered by tests
},
}
}

type crtClient struct {
restClient rest.Interface
client client.Client
ns string
cfg rest.Config
scheme *runtime.Scheme
client client.Client
ns string
scheme *runtime.Scheme
}
4 changes: 1 addition & 3 deletions pkg/kubeclient/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import (
"testing"

"github.com/codeready-toolchain/toolchain-common/pkg/test"
"k8s.io/client-go/rest"

"github.com/stretchr/testify/require"

"github.com/codeready-toolchain/registration-service/pkg/kubeclient"
Expand All @@ -17,7 +15,7 @@ const (

func TestNewClient(t *testing.T) {
// Try creating a new client with an empty config
client, err := kubeclient.NewCRTRESTClient(&rest.Config{}, test.NewFakeClient(t), TestNamespace)
client, err := kubeclient.NewCRTRESTClient(test.NewFakeClient(t), TestNamespace)

// Check that there are no errors, and the clients are returned
require.NoError(t, err)
Expand Down
13 changes: 5 additions & 8 deletions pkg/kubeclient/mur.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"context"

crtapi "github.com/codeready-toolchain/api/api/v1alpha1"
"github.com/codeready-toolchain/registration-service/pkg/kubeclient/resources"
"k8s.io/apimachinery/pkg/types"
)

type masterUserRecordClient struct {
Expand All @@ -18,11 +18,8 @@
// 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{}
err := c.restClient.Get().
Namespace(c.ns).
Resource(resources.MurResourcePlural).
Name(name).
Do(context.TODO()).
Into(result)
return result, err
if err := c.client.Get(context.TODO(), types.NamespacedName{Namespace: c.ns, Name: name}, result); err != nil {
return nil, err

Check warning on line 22 in pkg/kubeclient/mur.go

View check run for this annotation

Codecov / codecov/patch

pkg/kubeclient/mur.go#L21-L22

Added lines #L21 - L22 were not covered by tests
}
return result, nil

Check warning on line 24 in pkg/kubeclient/mur.go

View check run for this annotation

Codecov / codecov/patch

pkg/kubeclient/mur.go#L24

Added line #L24 was not covered by tests
}
13 changes: 0 additions & 13 deletions pkg/kubeclient/resources/names.go

This file was deleted.

34 changes: 7 additions & 27 deletions pkg/kubeclient/signup.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@
"regexp"

crtapi "github.com/codeready-toolchain/api/api/v1alpha1"
"github.com/codeready-toolchain/registration-service/pkg/kubeclient/resources"
"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"
)

Expand All @@ -32,48 +32,28 @@
// If not found then NotFound error returned
func (c *userSignupClient) Get(name string) (*crtapi.UserSignup, error) {
result := &crtapi.UserSignup{}
err := c.restClient.Get().
Namespace(c.ns).
Resource(resources.UserSignupResourcePlural).
Name(name).
Do(context.TODO()).
Into(result)
if err != nil {
if err := c.client.Get(context.TODO(), types.NamespacedName{Namespace: c.ns, Name: name}, result); err != nil {

Check warning on line 35 in pkg/kubeclient/signup.go

View check run for this annotation

Codecov / codecov/patch

pkg/kubeclient/signup.go#L35

Added line #L35 was not covered by tests
return nil, err
}
return result, err
return result, nil

Check warning on line 38 in pkg/kubeclient/signup.go

View check run for this annotation

Codecov / codecov/patch

pkg/kubeclient/signup.go#L38

Added line #L38 was not covered by tests
}

// 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) {
result := &crtapi.UserSignup{}
err := c.restClient.Post().
Namespace(c.ns).
Resource(resources.UserSignupResourcePlural).
Body(obj).
Do(context.TODO()).
Into(result)
if err != nil {
if err := c.client.Create(context.TODO(), obj); err != nil {

Check warning on line 44 in pkg/kubeclient/signup.go

View check run for this annotation

Codecov / codecov/patch

pkg/kubeclient/signup.go#L44

Added line #L44 was not covered by tests
return nil, err
}
return result, err
return obj, nil

Check warning on line 47 in pkg/kubeclient/signup.go

View check run for this annotation

Codecov / codecov/patch

pkg/kubeclient/signup.go#L47

Added line #L47 was not covered by tests
}

// 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) {
result := &crtapi.UserSignup{}
err := c.restClient.Put().
Namespace(c.ns).
Resource(resources.UserSignupResourcePlural).
Name(obj.Name).
Body(obj).
Do(context.TODO()).
Into(result)
err := c.client.Update(context.TODO(), obj)

Check warning on line 52 in pkg/kubeclient/signup.go

View check run for this annotation

Codecov / codecov/patch

pkg/kubeclient/signup.go#L52

Added line #L52 was not covered by tests
if err != nil {
return nil, err
}
return result, nil
return obj, nil

Check warning on line 56 in pkg/kubeclient/signup.go

View check run for this annotation

Codecov / codecov/patch

pkg/kubeclient/signup.go#L56

Added line #L56 was not covered by tests
}

// ListActiveSignupsByPhoneNumberOrHash will return a list of non-deactivated UserSignups that have a phone number hash
Expand Down
Loading
Loading