From f23d332653b4a4274bcf1f9ba16bbf6fcbffde66 Mon Sep 17 00:00:00 2001 From: Jakub Kabza Date: Tue, 3 Sep 2019 13:38:56 +0200 Subject: [PATCH] status --- api/v1alpha1/oauth2client_types.go | 18 +- controllers/mocks/HydraClientInterface.go | 101 ++++++++ controllers/oauth2client_controller.go | 13 + ...auth2client_controller_integration_test.go | 241 +++++++++++++----- go.mod | 1 + go.sum | 1 + 6 files changed, 304 insertions(+), 71 deletions(-) create mode 100644 controllers/mocks/HydraClientInterface.go diff --git a/api/v1alpha1/oauth2client_types.go b/api/v1alpha1/oauth2client_types.go index 201dc58..74b5d8e 100644 --- a/api/v1alpha1/oauth2client_types.go +++ b/api/v1alpha1/oauth2client_types.go @@ -20,6 +20,13 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +type StatusCode string + +const ( + StatusRegistrationFailed StatusCode = "CLIENT_REGISTRATION_FAILED" + StatusCreateSecretFailed StatusCode = "SECRET_CREATION_FAILED" +) + // OAuth2ClientSpec defines the desired state of OAuth2Client type OAuth2ClientSpec struct { // +kubebuilder:validation:MaxItems=4 @@ -58,7 +65,16 @@ type OAuth2ClientStatus struct { // ClientID is the id for this client. ClientID *string `json:"clientID,omitempty"` // ObservedGeneration represents the most recent generation observed by the daemon set controller. - ObservedGeneration int64 `json:"observedGeneration,omitempty"` + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + ReconciliationError ReconciliationError +} + +// ReconciliationError represents an error that occurred during the reconciliation process +type ReconciliationError struct { + // Code is the status code of the reconciliation error + Code StatusCode + // Description is the description of the reconciliation error + Description string } // +kubebuilder:object:root=true diff --git a/controllers/mocks/HydraClientInterface.go b/controllers/mocks/HydraClientInterface.go new file mode 100644 index 0000000..f8a2c52 --- /dev/null +++ b/controllers/mocks/HydraClientInterface.go @@ -0,0 +1,101 @@ +// Code generated by mockery v1.0.0. DO NOT EDIT. + +package mocks + +import hydra "github.com/ory/hydra-maester/hydra" +import mock "github.com/stretchr/testify/mock" + +// HydraClientInterface is an autogenerated mock type for the HydraClientInterface type +type HydraClientInterface struct { + mock.Mock +} + +// DeleteOAuth2Client provides a mock function with given fields: id +func (_m *HydraClientInterface) DeleteOAuth2Client(id string) error { + ret := _m.Called(id) + + var r0 error + if rf, ok := ret.Get(0).(func(string) error); ok { + r0 = rf(id) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// GetOAuth2Client provides a mock function with given fields: id +func (_m *HydraClientInterface) GetOAuth2Client(id string) (*hydra.OAuth2ClientJSON, bool, error) { + ret := _m.Called(id) + + var r0 *hydra.OAuth2ClientJSON + if rf, ok := ret.Get(0).(func(string) *hydra.OAuth2ClientJSON); ok { + r0 = rf(id) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*hydra.OAuth2ClientJSON) + } + } + + var r1 bool + if rf, ok := ret.Get(1).(func(string) bool); ok { + r1 = rf(id) + } else { + r1 = ret.Get(1).(bool) + } + + var r2 error + if rf, ok := ret.Get(2).(func(string) error); ok { + r2 = rf(id) + } else { + r2 = ret.Error(2) + } + + return r0, r1, r2 +} + +// PostOAuth2Client provides a mock function with given fields: o +func (_m *HydraClientInterface) PostOAuth2Client(o *hydra.OAuth2ClientJSON) (*hydra.OAuth2ClientJSON, error) { + ret := _m.Called(o) + + var r0 *hydra.OAuth2ClientJSON + if rf, ok := ret.Get(0).(func(*hydra.OAuth2ClientJSON) *hydra.OAuth2ClientJSON); ok { + r0 = rf(o) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*hydra.OAuth2ClientJSON) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(*hydra.OAuth2ClientJSON) error); ok { + r1 = rf(o) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// PutOAuth2Client provides a mock function with given fields: o +func (_m *HydraClientInterface) PutOAuth2Client(o *hydra.OAuth2ClientJSON) (*hydra.OAuth2ClientJSON, error) { + ret := _m.Called(o) + + var r0 *hydra.OAuth2ClientJSON + if rf, ok := ret.Get(0).(func(*hydra.OAuth2ClientJSON) *hydra.OAuth2ClientJSON); ok { + r0 = rf(o) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*hydra.OAuth2ClientJSON) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(*hydra.OAuth2ClientJSON) error); ok { + r1 = rf(o) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} diff --git a/controllers/oauth2client_controller.go b/controllers/oauth2client_controller.go index 310f66d..f71d1b6 100644 --- a/controllers/oauth2client_controller.go +++ b/controllers/oauth2client_controller.go @@ -101,6 +101,15 @@ func (r *OAuth2ClientReconciler) SetupWithManager(mgr ctrl.Manager) error { func (r *OAuth2ClientReconciler) registerOAuth2Client(ctx context.Context, client *hydrav1alpha1.OAuth2Client) error { created, err := r.HydraClient.PostOAuth2Client(client.ToOAuth2ClientJSON()) if err != nil { + client.Status.ObservedGeneration = client.Generation + client.Status.ReconciliationError = hydrav1alpha1.ReconciliationError{ + Code: hydrav1alpha1.StatusRegistrationFailed, + Description: err.Error(), + } + if updateErr := r.Status().Update(ctx, client); updateErr != nil { + return err + } + return err } @@ -121,6 +130,10 @@ func (r *OAuth2ClientReconciler) registerOAuth2Client(ctx context.Context, clien err = r.Create(ctx, &clientSecret) if err != nil { r.Log.Error(err, fmt.Sprintf("error creating secret for client %s/%s ", client.Name, client.Namespace), "oauth2client", "register") + client.Status.ReconciliationError = hydrav1alpha1.ReconciliationError{ + Code: hydrav1alpha1.StatusCreateSecretFailed, + Description: err.Error(), + } } else { client.Status.Secret = &clientSecret.Name } diff --git a/controllers/oauth2client_controller_integration_test.go b/controllers/oauth2client_controller_integration_test.go index 6863767..c177dfa 100644 --- a/controllers/oauth2client_controller_integration_test.go +++ b/controllers/oauth2client_controller_integration_test.go @@ -3,6 +3,8 @@ package controllers_test import ( "context" "fmt" + "github.com/ory/hydra-maester/controllers/mocks" + "github.com/pkg/errors" "time" . "github.com/onsi/ginkgo" @@ -10,6 +12,7 @@ import ( hydrav1alpha1 "github.com/ory/hydra-maester/api/v1alpha1" "github.com/ory/hydra-maester/controllers" "github.com/ory/hydra-maester/hydra" + . "github.com/stretchr/testify/mock" apiv1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -24,20 +27,23 @@ import ( "sigs.k8s.io/controller-runtime/pkg/source" ) -const timeout = time.Second * 5 +const ( + timeout = time.Second * 5 + tstNamespace = "default" + tstScopes = "a b c" +) + +var tstClientID = "testClientID" +var tstSecret = "testSecret" var _ = Describe("OAuth2Client Controller", func() { Context("in a happy-path scenario", func() { - var tstName = "test" - var tstNamespace = "default" - var tstScopes = "a b c" - var tstClientID = "testClientID" - var tstSecret = "testSecret" - - var expectedRequest = reconcile.Request{NamespacedName: types.NamespacedName{Name: tstName, Namespace: tstNamespace}} It("should call create OAuth2 client in Hydra and a Secret", func() { + tstName := "test" + expectedRequest := &reconcile.Request{NamespacedName: types.NamespacedName{Name: tstName, Namespace: tstNamespace}} + s := scheme.Scheme err := hydrav1alpha1.AddToScheme(s) Expect(err).NotTo(HaveOccurred()) @@ -48,12 +54,21 @@ var _ = Describe("OAuth2Client Controller", func() { Expect(err).NotTo(HaveOccurred()) c := mgr.GetClient() - mch := (&mockHydraClient{}). - withSecret(tstSecret). - withClientID(tstClientID) + mch := mocks.HydraClientInterface{} + mch.On("PostOAuth2Client", AnythingOfType("*hydra.OAuth2ClientJSON")).Return(func(o *hydra.OAuth2ClientJSON) *hydra.OAuth2ClientJSON { + return &hydra.OAuth2ClientJSON{ + ClientID: &tstClientID, + Secret: &tstSecret, + Name: o.Name, + GrantTypes: o.GrantTypes, + ResponseTypes: o.ResponseTypes, + Scope: o.Scope, + } + }, func(o *hydra.OAuth2ClientJSON) error { + return nil + }) - recFn, requests := SetupTestReconcile(getAPIReconciler(mgr, mch)) - //_, requests := SetupTestReconcile(getApiReconciler(mgr)) + recFn, requests := SetupTestReconcile(getAPIReconciler(mgr, &mch)) Expect(add(mgr, recFn)).To(Succeed()) @@ -66,7 +81,7 @@ var _ = Describe("OAuth2Client Controller", func() { mgrStopped.Wait() }() - instance := testInstance(tstName, tstNamespace, tstScopes) + instance := testInstance(tstName) err = c.Create(context.TODO(), instance) // The instance object may not be a valid object because it might be missing some required fields. // Please modify the instance object by adding required fields and then remove the following if statement. @@ -76,7 +91,7 @@ var _ = Describe("OAuth2Client Controller", func() { } Expect(err).NotTo(HaveOccurred()) defer c.Delete(context.TODO(), instance) - Eventually(requests, timeout).Should(Receive(Equal(expectedRequest))) + Eventually(requests, timeout).Should(Receive(Equal(*expectedRequest))) //Verify the created CR instance status var retrieved hydrav1alpha1.OAuth2Client @@ -93,6 +108,143 @@ var _ = Describe("OAuth2Client Controller", func() { Expect(err).NotTo(HaveOccurred()) Expect(createdSecret.Data["client_secret"]).To(Equal([]byte(tstSecret))) }) + + It("should call create OAuth2 client in Hydra and update object status if the call failed", func() { + + tstName := "test2" + expectedRequest := &reconcile.Request{NamespacedName: types.NamespacedName{Name: tstName, Namespace: tstNamespace}} + + s := scheme.Scheme + err := hydrav1alpha1.AddToScheme(s) + Expect(err).NotTo(HaveOccurred()) + + // Setup the Manager and Controller. Wrap the Controller Reconcile function so it writes each request to a + // channel when it is finished. + mgr, err := manager.New(cfg, manager.Options{Scheme: s}) + Expect(err).NotTo(HaveOccurred()) + c := mgr.GetClient() + + mch := mocks.HydraClientInterface{} + mch.On("PostOAuth2Client", Anything).Return(nil, errors.New("error")) + + recFn, requests := SetupTestReconcile(getAPIReconciler(mgr, &mch)) + + Expect(add(mgr, recFn)).To(Succeed()) + + //Start the manager and the controller + stopMgr, mgrStopped := StartTestManager(mgr) + + //Ensure manager is stopped properly + defer func() { + close(stopMgr) + mgrStopped.Wait() + }() + + instance := testInstance(tstName) + err = c.Create(context.TODO(), instance) + // The instance object may not be a valid object because it might be missing some required fields. + // Please modify the instance object by adding required fields and then remove the following if statement. + if apierrors.IsInvalid(err) { + Fail(fmt.Sprintf("failed to create object, got an invalid object error: %v", err)) + return + } + Expect(err).NotTo(HaveOccurred()) + defer c.Delete(context.TODO(), instance) + Eventually(requests, timeout).Should(Receive(Equal(*expectedRequest))) + + //Verify the created CR instance status + var retrieved hydrav1alpha1.OAuth2Client + ok := client.ObjectKey{Name: tstName, Namespace: tstNamespace} + err = c.Get(context.TODO(), ok, &retrieved) + Expect(err).NotTo(HaveOccurred()) + + Expect(retrieved.Status.ClientID).To(BeNil()) + Expect(retrieved.Status.Secret).To(BeNil()) + Expect(retrieved.Status.ReconciliationError).NotTo(BeNil()) + Expect(retrieved.Status.ReconciliationError.Code).To(Equal(hydrav1alpha1.StatusRegistrationFailed)) + Expect(retrieved.Status.ReconciliationError.Description).To(Equal("error")) + + }) + + tstClientID = "testClientID2" + + It("should call create OAuth2 client in Hydra, create Secret and update object status if Secret creation failed", func() { + + tstName := "test3" + expectedRequest := &reconcile.Request{NamespacedName: types.NamespacedName{Name: tstName, Namespace: tstNamespace}} + + s := scheme.Scheme + err := hydrav1alpha1.AddToScheme(s) + Expect(err).NotTo(HaveOccurred()) + + // Setup the Manager and Controller. Wrap the Controller Reconcile function so it writes each request to a + // channel when it is finished. + mgr, err := manager.New(cfg, manager.Options{Scheme: s}) + Expect(err).NotTo(HaveOccurred()) + c := mgr.GetClient() + + mch := mocks.HydraClientInterface{} + mch.On("PostOAuth2Client", AnythingOfType("*hydra.OAuth2ClientJSON")).Return(func(o *hydra.OAuth2ClientJSON) *hydra.OAuth2ClientJSON { + return &hydra.OAuth2ClientJSON{ + ClientID: &tstClientID, + Secret: &tstSecret, + Name: o.Name, + GrantTypes: o.GrantTypes, + ResponseTypes: o.ResponseTypes, + Scope: o.Scope, + } + }, func(o *hydra.OAuth2ClientJSON) error { + return nil + }) + + recFn, requests := SetupTestReconcile(getAPIReconciler(mgr, &mch)) + + Expect(add(mgr, recFn)).To(Succeed()) + + //Start the manager and the controller + stopMgr, mgrStopped := StartTestManager(mgr) + + //Ensure manager is stopped properly + defer func() { + close(stopMgr) + mgrStopped.Wait() + }() + + + //ensure conflicting entry exists + secret := apiv1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: tstName, + Namespace: tstNamespace, + }, + } + err = c.Create(context.TODO(), &secret) + Expect(err).NotTo(HaveOccurred()) + + instance := testInstance(tstName) + err = c.Create(context.TODO(), instance) + // The instance object may not be a valid object because it might be missing some required fields. + // Please modify the instance object by adding required fields and then remove the following if statement. + if apierrors.IsInvalid(err) { + Fail(fmt.Sprintf("failed to create object, got an invalid object error: %v", err)) + return + } + Expect(err).NotTo(HaveOccurred()) + defer c.Delete(context.TODO(), instance) + Eventually(requests, timeout).Should(Receive(Equal(*expectedRequest))) + + //Verify the created CR instance status + var retrieved hydrav1alpha1.OAuth2Client + ok := client.ObjectKey{Name: tstName, Namespace: tstNamespace} + err = c.Get(context.TODO(), ok, &retrieved) + Expect(err).NotTo(HaveOccurred()) + + Expect(retrieved.Status.ClientID).NotTo(BeNil()) + Expect(retrieved.Status.Secret).To(BeNil()) + Expect(retrieved.Status.ReconciliationError).NotTo(BeNil()) + Expect(retrieved.Status.ReconciliationError.Code).To(Equal(hydrav1alpha1.StatusCreateSecretFailed)) + Expect(retrieved.Status.ReconciliationError.Description).To(Equal(`secrets "test3" already exists`)) + }) }) }) @@ -123,7 +275,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { return nil } -func getAPIReconciler(mgr ctrl.Manager, mock *mockHydraClient) reconcile.Reconciler { +func getAPIReconciler(mgr ctrl.Manager, mock controllers.HydraClientInterface) reconcile.Reconciler { return &controllers.OAuth2ClientReconciler{ Client: mgr.GetClient(), Log: ctrl.Log.WithName("controllers").WithName("OAuth2Client"), @@ -131,67 +283,16 @@ func getAPIReconciler(mgr ctrl.Manager, mock *mockHydraClient) reconcile.Reconci } } -func testInstance(name, namespace, scopes string) *hydrav1alpha1.OAuth2Client { +func testInstance(name string) *hydrav1alpha1.OAuth2Client { return &hydrav1alpha1.OAuth2Client{ ObjectMeta: metav1.ObjectMeta{ Name: name, - Namespace: namespace, + Namespace: tstNamespace, }, Spec: hydrav1alpha1.OAuth2ClientSpec{ GrantTypes: []hydrav1alpha1.GrantType{"client_credentials"}, ResponseTypes: []hydrav1alpha1.ResponseType{"token"}, - Scope: scopes, + Scope: tstScopes, }} } - -//TODO: Replace with full-fledged mocking framework (mockery/go-mock) -type mockHydraClient struct { - resSecret string - resClientID string - postedData *hydra.OAuth2ClientJSON -} - -func (m *mockHydraClient) withSecret(secret string) *mockHydraClient { - m.resSecret = secret - return m -} - -func (m *mockHydraClient) withClientID(clientID string) *mockHydraClient { - m.resClientID = clientID - return m -} - -//Returns the data previously "stored" by PostOAuth2Client -func (m *mockHydraClient) GetOAuth2Client(id string) (*hydra.OAuth2ClientJSON, bool, error) { - res := &hydra.OAuth2ClientJSON{ - ClientID: &m.resClientID, - Secret: &m.resSecret, - Name: m.postedData.Name, - GrantTypes: m.postedData.GrantTypes, - ResponseTypes: m.postedData.ResponseTypes, - Scope: m.postedData.Scope, - } - return res, true, nil -} - -func (m *mockHydraClient) PostOAuth2Client(o *hydra.OAuth2ClientJSON) (*hydra.OAuth2ClientJSON, error) { - m.postedData = o - res := &hydra.OAuth2ClientJSON{ - ClientID: &m.resClientID, - Secret: &m.resSecret, - Name: o.Name, - GrantTypes: o.GrantTypes, - ResponseTypes: o.ResponseTypes, - Scope: o.Scope, - } - return res, nil -} - -func (m *mockHydraClient) DeleteOAuth2Client(id string) error { - panic("Should not be invoked!") -} - -func (m *mockHydraClient) PutOAuth2Client(o *hydra.OAuth2ClientJSON) (*hydra.OAuth2ClientJSON, error) { - panic("Should not be invoked!") -} diff --git a/go.mod b/go.mod index 7478161..f9de68b 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ require ( github.com/go-logr/logr v0.1.0 github.com/onsi/ginkgo v1.6.0 github.com/onsi/gomega v1.4.2 + github.com/pkg/errors v0.8.1 github.com/stretchr/testify v1.3.0 golang.org/x/net v0.0.0-20180906233101-161cd47e91fd k8s.io/api v0.0.0-20190409021203-6e4e0e4f393b diff --git a/go.sum b/go.sum index 39d372a..13386a1 100644 --- a/go.sum +++ b/go.sum @@ -66,6 +66,7 @@ github.com/prometheus/procfs v0.0.0-20180725123919-05ee40e3a273/go.mod h1:c3At6R github.com/spf13/afero v1.2.2/go.mod h1:9ZxEEn6pIJ8Rxe320qSDBk6AsU0r9pR7Q4OcevTdifk= github.com/spf13/pflag v1.0.2 h1:Fy0orTDgHdbnzHcsOgfCN4LtHf0ec3wwtiwJqwvf3Gc= github.com/spf13/pflag v1.0.2/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= +github.com/stretchr/objx v0.1.0 h1:4G4v2dO3VZwixGIRoQ5Lfboy6nUhCyYzaqnIAPPhYs4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q=