Full upgrade (#19)

- SecretName is now mandatory
- One can update client_secret in Hydra by creating new Secret object and changing the SecretName in CR instance

Makefile

@@ -17,7 +17,7 @@ test-integration:
 
 # Build manager binary
 manager: generate fmt vet
-	go build -o bin/manager main.go
+	CGO_ENABLED=0 GO111MODULE=on GOOS=linux GOARCH=amd64 go build -a -o manager main.go
 
 # Run against the configured Kubernetes cluster in ~/.kube/config
 run: generate fmt vet

README.md

@@ -15,7 +15,7 @@
 
 This project contains a Kubernetes controller that uses Custom Resources (CR) to manage Hydra Oauth2 clients. ORY Hydra Maester watches for instances of `oauth2clients.oathkeeper.ory.sh/v1alpha1` CR and creates, updates, or deletes corresponding OAuth2 clients by communicating with ORY Hydra's API.
 
-Visit Hydra-maester's [chart documentation](https://github.com/ory/k8s/blob/master/docs/helm/hydra-maester.md) and view a [sample OAuth2 client resource](./config/samples/hydra_v1alpha1_oauth2client.yaml) to learn more about the `oauth2clients.oathkeeper.ory.sh/v1alpha1` CR. 
+Visit Hydra-maester's [chart documentation](https://github.com/ory/k8s/blob/master/docs/helm/hydra-maester.md) and view [sample OAuth2 client resources](config/samples) to learn more about the `oauth2clients.oathkeeper.ory.sh/v1alpha1` CR. 
 
 The project is based on [Kubebuilder](https://github.com/kubernetes-sigs/kubebuilder).
 

api/v1alpha1/oauth2client_types.go

@@ -25,6 +25,8 @@ type StatusCode string
 const (
 	StatusRegistrationFailed StatusCode = "CLIENT_REGISTRATION_FAILED"
 	StatusCreateSecretFailed StatusCode = "SECRET_CREATION_FAILED"
+	StatusUpdateFailed       StatusCode = "CLIENT_UPDATE_FAILED"
+	StatusInvalidSecret      StatusCode = "INVALID_SECRET"
 )
 
 // OAuth2ClientSpec defines the desired state of OAuth2Client
@@ -48,6 +50,13 @@ type OAuth2ClientSpec struct {
 	// described in Section 3.3 of OAuth 2.0 [RFC6749]) that the client
 	// can use when requesting access tokens.
 	Scope string `json:"scope"`
+
+	// +kubebuilder:validation:MinLength=1
+	// +kubebuilder:validation:MaxLength=253
+	// +kubebuilder:validation:Pattern=[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*
+	//
+	// SecretName points to the K8s secret that contains this client's ID and password
+	SecretName string `json:"secretName"`
 }
 
 // +kubebuilder:validation:Enum=client_credentials;authorization_code;implicit;refresh_token
@@ -60,10 +69,6 @@ type ResponseType string
 
 // OAuth2ClientStatus defines the observed state of OAuth2Client
 type OAuth2ClientStatus struct {
-	// Secret points to the K8s secret that contains this client's id and password
-	Secret *string `json:"secret,omitempty"`
-	// 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"`
 	ReconciliationError ReconciliationError `json:"reconciliationError,omitempty"`
@@ -106,7 +111,6 @@ func init() {
 func (c *OAuth2Client) ToOAuth2ClientJSON() *hydra.OAuth2ClientJSON {
 	return &hydra.OAuth2ClientJSON{
 		Name:          c.Name,
-		ClientID:      c.Status.ClientID,
 		GrantTypes:    grantToStringSlice(c.Spec.GrantTypes),
 		ResponseTypes: responseToStringSlice(c.Spec.ResponseTypes),
 		Scope:         c.Spec.Scope,

api/v1alpha1/oauth2client_types_test.go

@@ -77,6 +77,7 @@ func TestCreateAPI(t *testing.T) {
 				"invalid grant type":    func() { created.Spec.GrantTypes = []GrantType{"invalid"} },
 				"invalid response type": func() { created.Spec.ResponseTypes = []ResponseType{"invalid"} },
 				"invalid scope":         func() { created.Spec.Scope = "" },
+				"missing secret name":   func() { created.Spec.SecretName = "" },
 			} {
 				t.Run(fmt.Sprintf("case=%s", desc), func(t *testing.T) {
 
@@ -124,6 +125,7 @@ func resetTestClient() {
 			GrantTypes:    []GrantType{"implicit", "client_credentials", "authorization_code", "refresh_token"},
 			ResponseTypes: []ResponseType{"id_token", "code", "token"},
 			Scope:         "read,write",
+			SecretName:    "secret-name",
 		},
 	}
 }

api/v1alpha1/zz_generated.deepcopy.go

@@ -29,7 +29,7 @@ func (in *OAuth2Client) DeepCopyInto(out *OAuth2Client) {
 	out.TypeMeta = in.TypeMeta
 	in.ObjectMeta.DeepCopyInto(&out.ObjectMeta)
 	in.Spec.DeepCopyInto(&out.Spec)
-	in.Status.DeepCopyInto(&out.Status)
+	out.Status = in.Status
 }
 
 // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OAuth2Client.
@@ -110,16 +110,6 @@ func (in *OAuth2ClientSpec) DeepCopy() *OAuth2ClientSpec {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *OAuth2ClientStatus) DeepCopyInto(out *OAuth2ClientStatus) {
 	*out = *in
-	if in.Secret != nil {
-		in, out := &in.Secret, &out.Secret
-		*out = new(string)
-		**out = **in
-	}
-	if in.ClientID != nil {
-		in, out := &in.ClientID, &out.ClientID
-		*out = new(string)
-		**out = **in
-	}
 	out.ReconciliationError = in.ReconciliationError
 }
 

config/crd/bases/hydra.ory.sh_oauth2clients.yaml

@@ -418,15 +418,20 @@ spec:
                 that the client can use when requesting access tokens.
               pattern: ([a-zA-Z0-9\.\*]+\s?)+
               type: string
+            secretName:
+              description: SecretName points to the K8s secret that contains this
+                client's ID and password
+              maxLength: 253
+              minLength: 1
+              pattern: '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*'
+              type: string
           required:
           - grantTypes
           - scope
+          - secretName
           type: object
         status:
           properties:
-            clientID:
-              description: ClientID is the id for this client.
-              type: string
             observedGeneration:
               description: ObservedGeneration represents the most recent generation
                 observed by the daemon set controller.
@@ -442,10 +447,6 @@ spec:
                   description: Code is the status code of the reconciliation error
                   type: string
               type: object
-            secret:
-              description: Secret points to the K8s secret that contains this client's
-                id and password
-              type: string
           type: object
       type: object
   versions:

config/samples/hydra_v1alpha1_oauth2client.yaml

@@ -13,4 +13,5 @@ spec:
     - id_token
     - code
     - token
-  scope: "read write"
+  scope: "read write"
+  secretName: my-secret-123

config/samples/hydra_v1alpha1_oauth2client_user_credentials.yaml

@@ -0,0 +1,27 @@
+apiVersion: v1
+kind: Secret
+metadata:
+  name: my-secret-456
+  namespace: default
+type: Opaque
+data:
+  client_id: MDA5MDA5MDA=
+  client_secret: czNjUjM3cDRzc1ZWMHJEMTIzNA==
+---
+apiVersion: hydra.ory.sh/v1alpha1
+kind: OAuth2Client
+metadata:
+  name: my-oauth2-client-2
+  namespace: default
+spec:
+  grantTypes:
+    - client_credentials
+    - implicit
+    - authorization_code
+    - refresh_token
+  responseTypes:
+    - id_token
+    - code
+    - token
+  scope: "read write"
+  secretName: my-secret-456

controllers/oauth2client_controller.go

@@ -19,21 +19,22 @@ import (
 	"context"
 	"fmt"
 
-	"k8s.io/apimachinery/pkg/types"
-
 	"github.com/go-logr/logr"
 	hydrav1alpha1 "github.com/ory/hydra-maester/api/v1alpha1"
 	"github.com/ory/hydra-maester/hydra"
+	"github.com/pkg/errors"
 	apiv1 "k8s.io/api/core/v1"
 	apierrs "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 const (
-	clientIDKey     = "client_id"
+	ClientIDKey     = "client_id"
-	clientSecretKey = "client_secret"
+	ClientSecretKey = "client_secret"
+	ownerLabel      = "owner"
 )
 
 type HydraClientInterface interface {
@@ -58,10 +59,10 @@ func (r *OAuth2ClientReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error
 	ctx := context.Background()
 	_ = r.Log.WithValues("oauth2client", req.NamespacedName)
 
-	var client hydrav1alpha1.OAuth2Client
+	var oauth2client hydrav1alpha1.OAuth2Client
-	if err := r.Get(ctx, req.NamespacedName, &client); err != nil {
+	if err := r.Get(ctx, req.NamespacedName, &oauth2client); err != nil {
 		if apierrs.IsNotFound(err) {
-			if err := r.unregisterOAuth2Client(ctx, req.NamespacedName); err != nil {
+			if err := r.unregisterOAuth2Clients(ctx, req.Name, req.Namespace); err != nil {
 				return ctrl.Result{}, err
 			}
 			return ctrl.Result{}, nil
@@ -69,24 +70,37 @@ func (r *OAuth2ClientReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error
 		return ctrl.Result{}, err
 	}
 
-	if client.Generation != client.Status.ObservedGeneration {
+	if oauth2client.Generation != oauth2client.Status.ObservedGeneration {
 
-		var registered = false
+		var secret apiv1.Secret
-		var err error
+		if err := r.Get(ctx, types.NamespacedName{Name: oauth2client.Spec.SecretName, Namespace: req.Namespace}, &secret); err != nil {
-
+			if apierrs.IsNotFound(err) {
-		if client.Status.ClientID != nil {
+				return ctrl.Result{}, r.registerOAuth2Client(ctx, &oauth2client, nil)
-
-			_, registered, err = r.HydraClient.GetOAuth2Client(*client.Status.ClientID)
-			if err != nil {
-				return ctrl.Result{}, err
 			}
+			return ctrl.Result{}, err
 		}
 
-		if !registered {
+		credentials, err := parseSecret(secret)
-			return ctrl.Result{}, r.registerOAuth2Client(ctx, &client)
+		if err != nil {
+			r.Log.Error(err, fmt.Sprintf("secret %s/%s is invalid", secret.Name, secret.Namespace))
+			return ctrl.Result{}, r.updateReconciliationStatusError(ctx, &oauth2client, hydrav1alpha1.StatusInvalidSecret, err)
 		}
 
-		return ctrl.Result{}, r.updateRegisteredOAuth2Client(&client)
+		if err := r.labelSecretWithOwnerReference(ctx, &oauth2client, secret); err != nil {
+			return ctrl.Result{}, err
+		}
+
+		_, found, err := r.HydraClient.GetOAuth2Client(string(credentials.ID))
+		if err != nil {
+			return ctrl.Result{}, err
+
+		}
+
+		if found {
+			return ctrl.Result{}, r.updateRegisteredOAuth2Client(ctx, &oauth2client, credentials)
+		}
+
+		return ctrl.Result{}, r.registerOAuth2Client(ctx, &oauth2client, credentials)
 	}
 
 	return ctrl.Result{}, nil
@@ -98,64 +112,122 @@ func (r *OAuth2ClientReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Complete(r)
 }
 
-func (r *OAuth2ClientReconciler) registerOAuth2Client(ctx context.Context, client *hydrav1alpha1.OAuth2Client) error {
+func (r *OAuth2ClientReconciler) registerOAuth2Client(ctx context.Context, c *hydrav1alpha1.OAuth2Client, credentials *hydra.Oauth2ClientCredentials) error {
-	created, err := r.HydraClient.PostOAuth2Client(client.ToOAuth2ClientJSON())
+	if err := r.unregisterOAuth2Clients(ctx, c.Name, c.Namespace); err != nil {
-	if err != nil {
+		return err
-		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 {
-			r.Log.Error(err, fmt.Sprintf("error registring client %s/%s ", client.Name, client.Namespace), "oauth2client", "register")
-			return updateErr
-		}
 
+	if credentials != nil {
+		if _, err := r.HydraClient.PostOAuth2Client(c.ToOAuth2ClientJSON().WithCredentials(credentials)); err != nil {
+			return r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusRegistrationFailed, err)
+		}
 		return nil
 	}
 
+	created, err := r.HydraClient.PostOAuth2Client(c.ToOAuth2ClientJSON())
+	if err != nil {
+		return r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusRegistrationFailed, err)
+	}
+
 	clientSecret := apiv1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      client.Name,
+			Name:      c.Spec.SecretName,
-			Namespace: client.Namespace,
+			Namespace: c.Namespace,
+			Labels:    map[string]string{ownerLabel: c.Name},
 		},
 		Data: map[string][]byte{
-			clientSecretKey: []byte(*created.Secret),
+			ClientIDKey:     []byte(*created.ClientID),
-			clientIDKey:     []byte(*created.ClientID),
+			ClientSecretKey: []byte(*created.Secret),
 		},
 	}
 
-	client.Status.ClientID = created.ClientID
+	if err := r.Create(ctx, &clientSecret); err != nil {
-	client.Status.ObservedGeneration = client.Generation
+		return r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusCreateSecretFailed, err)
-
-	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
 	}
 
-	return r.Status().Update(ctx, client)
+	return nil
 }
 
-func (r *OAuth2ClientReconciler) unregisterOAuth2Client(ctx context.Context, namespacedName types.NamespacedName) error {
+func (r *OAuth2ClientReconciler) updateRegisteredOAuth2Client(ctx context.Context, c *hydrav1alpha1.OAuth2Client, credentials *hydra.Oauth2ClientCredentials) error {
-	var sec apiv1.Secret
+	if _, err := r.HydraClient.PutOAuth2Client(c.ToOAuth2ClientJSON().WithCredentials(credentials)); err != nil {
-	if err := r.Get(ctx, namespacedName, &sec); err != nil {
+		return r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusUpdateFailed, err)
-		if apierrs.IsNotFound(err) {
+	}
-			r.Log.Info(fmt.Sprintf("unable to find secret corresponding with client %s/%s. Manual deletion recommended", namespacedName.Name, namespacedName.Namespace))
+	return nil
-			return nil
+}
-		}
+
+func (r *OAuth2ClientReconciler) unregisterOAuth2Clients(ctx context.Context, name, namespace string) error {
+	var secretList apiv1.SecretList
+
+	err := r.List(
+		ctx,
+		&secretList,
+		client.InNamespace(namespace),
+		client.MatchingLabels(map[string]string{ownerLabel: name}))
+
+	if err != nil {
 		return err
 	}
 
-	return r.HydraClient.DeleteOAuth2Client(string(sec.Data[clientIDKey]))
+	if len(secretList.Items) == 0 {
+		return nil
+	}
+
+	ids := make(map[string]struct{})
+	for _, s := range secretList.Items {
+		ids[string(s.Data[ClientIDKey])] = struct{}{}
+	}
+
+	for id := range ids {
+		if err := r.HydraClient.DeleteOAuth2Client(id); err != nil {
+			return err
+		}
+	}
+
+	return nil
 }
 
-func (r *OAuth2ClientReconciler) updateRegisteredOAuth2Client(client *hydrav1alpha1.OAuth2Client) error {
+func (r *OAuth2ClientReconciler) updateReconciliationStatusError(ctx context.Context, c *hydrav1alpha1.OAuth2Client, code hydrav1alpha1.StatusCode, err error) error {
-	_, err := r.HydraClient.PutOAuth2Client(client.ToOAuth2ClientJSON())
+	r.Log.Error(err, fmt.Sprintf("error processing client %s/%s ", c.Name, c.Namespace), "oauth2client", "register")
-	return err
+	c.Status.ObservedGeneration = c.Generation
+	c.Status.ReconciliationError = hydrav1alpha1.ReconciliationError{
+		Code:        code,
+		Description: err.Error(),
+	}
+	if updateErr := r.Status().Update(ctx, c); updateErr != nil {
+		r.Log.Error(updateErr, fmt.Sprintf("status update failed for client %s/%s ", c.Name, c.Namespace), "oauth2client", "update status")
+		return updateErr
+	}
+
+	return nil
+}
+
+func parseSecret(secret apiv1.Secret) (*hydra.Oauth2ClientCredentials, error) {
+
+	id, found := secret.Data[ClientIDKey]
+	if !found {
+		return nil, errors.New(`"client_id property missing"`)
+	}
+
+	psw, found := secret.Data[ClientSecretKey]
+	if !found {
+		return nil, errors.New(`"client_secret property missing"`)
+	}
+
+	return &hydra.Oauth2ClientCredentials{
+		ID:       id,
+		Password: psw,
+	}, nil
+}
+
+func (r *OAuth2ClientReconciler) labelSecretWithOwnerReference(ctx context.Context, c *hydrav1alpha1.OAuth2Client, secret apiv1.Secret) error {
+
+	if secret.Labels[ownerLabel] != c.Name {
+		if secret.Labels == nil {
+			secret.Labels = make(map[string]string, 1)
+		}
+		secret.Labels[ownerLabel] = c.Name
+		return r.Update(ctx, &secret)
+	}
+
+	return nil
 }

controllers/oauth2client_controller_integration_test.go

@@ -2,16 +2,17 @@ package controllers_test
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"time"
 
-	"github.com/ory/hydra-maester/controllers/mocks"
+	"k8s.io/utils/pointer"
-	"github.com/pkg/errors"
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	hydrav1alpha1 "github.com/ory/hydra-maester/api/v1alpha1"
 	"github.com/ory/hydra-maester/controllers"
+	"github.com/ory/hydra-maester/controllers/mocks"
 	"github.com/ory/hydra-maester/hydra"
 	. "github.com/stretchr/testify/mock"
 	apiv1 "k8s.io/api/core/v1"
@@ -31,219 +32,310 @@ import (
 const (
 	timeout      = time.Second * 5
 	tstNamespace = "default"
-	tstScopes    = "a b c"
+	tstSecret    = "testSecret"
 )
 
-var tstClientID = "testClientID"
-var tstSecret = "testSecret"
-
 var _ = Describe("OAuth2Client Controller", func() {
+
 	Context("in a happy-path scenario", func() {
 
-		It("should call create OAuth2 client in Hydra and a Secret", func() {
+		Context("should call create OAuth2 client and", func() {
 
-			tstName := "test"
+			It("create a Secret if it does not exist", func() {
-			expectedRequest := &reconcile.Request{NamespacedName: types.NamespacedName{Name: tstName, Namespace: tstNamespace}}
 
-			s := scheme.Scheme
+				tstName, tstClientID, tstSecretName := "test", "testClientID", "my-secret-123"
-			err := hydrav1alpha1.AddToScheme(s)
+				expectedRequest := &reconcile.Request{NamespacedName: types.NamespacedName{Name: tstName, Namespace: tstNamespace}}
-			Expect(err).NotTo(HaveOccurred())
 
-			// Setup the Manager and Controller.  Wrap the Controller Reconcile function so it writes each request to a
+				s := scheme.Scheme
-			// channel when it is finished.
+				err := hydrav1alpha1.AddToScheme(s)
-			mgr, err := manager.New(cfg, manager.Options{Scheme: s})
+				Expect(err).NotTo(HaveOccurred())
-			Expect(err).NotTo(HaveOccurred())
-			c := mgr.GetClient()
 
-			mch := mocks.HydraClientInterface{}
+				err = apiv1.AddToScheme(s)
-			mch.On("PostOAuth2Client", AnythingOfType("*hydra.OAuth2ClientJSON")).Return(func(o *hydra.OAuth2ClientJSON) *hydra.OAuth2ClientJSON {
+				Expect(err).NotTo(HaveOccurred())
-				return &hydra.OAuth2ClientJSON{
+
-					ClientID:      &tstClientID,
+				// Setup the Manager and Controller.  Wrap the Controller Reconcile function so it writes each request to a
-					Secret:        &tstSecret,
+				// channel when it is finished.
-					Name:          o.Name,
+				mgr, err := manager.New(cfg, manager.Options{Scheme: s})
-					GrantTypes:    o.GrantTypes,
+				Expect(err).NotTo(HaveOccurred())
-					ResponseTypes: o.ResponseTypes,
+				c := mgr.GetClient()
-					Scope:         o.Scope,
+
+				mch := &mocks.HydraClientInterface{}
+				mch.On("DeleteOAuth2Client", Anything).Return(nil)
+				mch.On("PostOAuth2Client", AnythingOfType("*hydra.OAuth2ClientJSON")).Return(func(o *hydra.OAuth2ClientJSON) *hydra.OAuth2ClientJSON {
+					return &hydra.OAuth2ClientJSON{
+						ClientID:      &tstClientID,
+						Secret:        pointer.StringPtr(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)
+
+				instance := testInstance(tstName, tstSecretName)
+				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
 				}
-			}, func(o *hydra.OAuth2ClientJSON) error {
+				Expect(err).NotTo(HaveOccurred())
-				return nil
+				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.ReconciliationError.Code).To(BeEmpty())
+				Expect(retrieved.Status.ReconciliationError.Description).To(BeEmpty())
+
+				//Verify the created Secret
+				var createdSecret apiv1.Secret
+				ok = client.ObjectKey{Name: tstSecretName, Namespace: tstNamespace}
+				err = k8sClient.Get(context.TODO(), ok, &createdSecret)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(createdSecret.Data[controllers.ClientIDKey]).To(Equal([]byte(tstClientID)))
+				Expect(createdSecret.Data[controllers.ClientSecretKey]).To(Equal([]byte(tstSecret)))
+
+				//delete instance
+				c.Delete(context.TODO(), instance)
+
+				//Ensure manager is stopped properly
+				close(stopMgr)
+				mgrStopped.Wait()
 			})
 
-			recFn, requests := SetupTestReconcile(getAPIReconciler(mgr, &mch))
+			It("update object status if the call failed", func() {
 
-			Expect(add(mgr, recFn)).To(Succeed())
+				tstName, tstSecretName := "test2", "my-secret-456"
+				expectedRequest := &reconcile.Request{NamespacedName: types.NamespacedName{Name: tstName, Namespace: tstNamespace}}
 
-			//Start the manager and the controller
+				s := scheme.Scheme
-			stopMgr, mgrStopped := StartTestManager(mgr)
+				err := hydrav1alpha1.AddToScheme(s)
+				Expect(err).NotTo(HaveOccurred())
 
-			//Ensure manager is stopped properly
+				err = apiv1.AddToScheme(s)
-			defer func() {
+				Expect(err).NotTo(HaveOccurred())
-				close(stopMgr)
-				mgrStopped.Wait()
-			}()
 
-			instance := testInstance(tstName)
+				// Setup the Manager and Controller.  Wrap the Controller Reconcile function so it writes each request to a
-			err = c.Create(context.TODO(), instance)
+				// channel when it is finished.
-			// The instance object may not be a valid object because it might be missing some required fields.
+				mgr, err := manager.New(cfg, manager.Options{Scheme: s})
-			// Please modify the instance object by adding required fields and then remove the following if statement.
+				Expect(err).NotTo(HaveOccurred())
-			if apierrors.IsInvalid(err) {
+				c := mgr.GetClient()
-				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
+				mch := &mocks.HydraClientInterface{}
-			var retrieved hydrav1alpha1.OAuth2Client
+				mch.On("PostOAuth2Client", Anything).Return(nil, errors.New("error"))
-			ok := client.ObjectKey{Name: tstName, Namespace: tstNamespace}
+				mch.On("DeleteOAuth2Client", Anything).Return(nil)
-			err = c.Get(context.TODO(), ok, &retrieved)
-			Expect(err).NotTo(HaveOccurred())
 
-			Expect(*retrieved.Status.ClientID).To(Equal(tstClientID))
+				recFn, requests := SetupTestReconcile(getAPIReconciler(mgr, mch))
-			Expect(*retrieved.Status.Secret).To(Equal(tstName)) //Secret contents is not visible in the CR instance!
 
-			//Verify the created Secret
+				Expect(add(mgr, recFn)).To(Succeed())
-			var createdSecret = apiv1.Secret{}
-			k8sClient.Get(context.TODO(), ok, &createdSecret)
-			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() {
+				//Start the manager and the controller
+				stopMgr, mgrStopped := StartTestManager(mgr)
 
-			tstName := "test2"
+				instance := testInstance(tstName, tstSecretName)
-			expectedRequest := &reconcile.Request{NamespacedName: types.NamespacedName{Name: tstName, Namespace: tstNamespace}}
+				err = c.Create(context.TODO(), instance)
-
+				// The instance object may not be a valid object because it might be missing some required fields.
-			s := scheme.Scheme
+				// Please modify the instance object by adding required fields and then remove the following if statement.
-			err := hydrav1alpha1.AddToScheme(s)
+				if apierrors.IsInvalid(err) {
-			Expect(err).NotTo(HaveOccurred())
+					Fail(fmt.Sprintf("failed to create object, got an invalid object error: %v", err))
-
+					return
-			// 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 {
+				Expect(err).NotTo(HaveOccurred())
-				return nil
+				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.ReconciliationError).NotTo(BeNil())
+				Expect(retrieved.Status.ReconciliationError.Code).To(Equal(hydrav1alpha1.StatusRegistrationFailed))
+				Expect(retrieved.Status.ReconciliationError.Description).To(Equal("error"))
+
+				//Verify no secret has been created
+				var createdSecret apiv1.Secret
+				ok = client.ObjectKey{Name: tstSecretName, Namespace: tstNamespace}
+				err = k8sClient.Get(context.TODO(), ok, &createdSecret)
+				Expect(err).To(HaveOccurred())
+				Expect(apierrors.IsNotFound(err)).To(BeTrue())
+
+				//delete instance
+				c.Delete(context.TODO(), instance)
+
+				//Ensure manager is stopped properly
+				close(stopMgr)
+				mgrStopped.Wait()
 			})
 
-			recFn, requests := SetupTestReconcile(getAPIReconciler(mgr, &mch))
+			It("use provided Secret if it exists", func() {
 
-			Expect(add(mgr, recFn)).To(Succeed())
+				tstName, tstClientID, tstSecretName := "test3", "testClientID-3", "my-secret-789"
+				var postedClient *hydra.OAuth2ClientJSON
+				expectedRequest := &reconcile.Request{NamespacedName: types.NamespacedName{Name: tstName, Namespace: tstNamespace}}
 
-			//Start the manager and the controller
+				s := scheme.Scheme
-			stopMgr, mgrStopped := StartTestManager(mgr)
+				err := hydrav1alpha1.AddToScheme(s)
+				Expect(err).NotTo(HaveOccurred())
 
-			//Ensure manager is stopped properly
+				err = apiv1.AddToScheme(s)
-			defer func() {
+				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("DeleteOAuth2Client", Anything).Return(nil)
+				mch.On("GetOAuth2Client", Anything).Return(nil, false, nil)
+				mch.On("PostOAuth2Client", AnythingOfType("*hydra.OAuth2ClientJSON")).Return(func(o *hydra.OAuth2ClientJSON) *hydra.OAuth2ClientJSON {
+					postedClient = &hydra.OAuth2ClientJSON{
+						ClientID:      o.ClientID,
+						Secret:        o.Secret,
+						Name:          o.Name,
+						GrantTypes:    o.GrantTypes,
+						ResponseTypes: o.ResponseTypes,
+						Scope:         o.Scope,
+					}
+					return postedClient
+				}, 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 secret exists
+				secret := apiv1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      tstSecretName,
+						Namespace: tstNamespace,
+					},
+					Data: map[string][]byte{
+						controllers.ClientIDKey:     []byte(tstClientID),
+						controllers.ClientSecretKey: []byte(tstSecret),
+					},
+				}
+				err = c.Create(context.TODO(), &secret)
+				Expect(err).NotTo(HaveOccurred())
+
+				instance := testInstance(tstName, tstSecretName)
+				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())
+				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.ReconciliationError.Code).To(BeEmpty())
+				Expect(retrieved.Status.ReconciliationError.Description).To(BeEmpty())
+
+				Expect(*postedClient.ClientID).To(Equal(tstClientID))
+				Expect(*postedClient.Secret).To(Equal(tstSecret))
+
+				//delete instance
+				c.Delete(context.TODO(), instance)
+
+				//Ensure manager is stopped properly
 				close(stopMgr)
 				mgrStopped.Wait()
-			}()
+			})
 
-			//ensure conflicting entry exists
+			It("update object status if provided Secret is invalid", func() {
-			secret := apiv1.Secret{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      tstName,
-					Namespace: tstNamespace,
-				},
-			}
-			err = c.Create(context.TODO(), &secret)
-			Expect(err).NotTo(HaveOccurred())
 
-			instance := testInstance(tstName)
+				tstName, tstClientID, tstSecretName := "test4", "testClientID-4", "my-secret-000"
-			err = c.Create(context.TODO(), instance)
+				expectedRequest := &reconcile.Request{NamespacedName: types.NamespacedName{Name: tstName, Namespace: tstNamespace}}
-			// 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
+				s := scheme.Scheme
-			var retrieved hydrav1alpha1.OAuth2Client
+				err := hydrav1alpha1.AddToScheme(s)
-			ok := client.ObjectKey{Name: tstName, Namespace: tstNamespace}
+				Expect(err).NotTo(HaveOccurred())
-			err = c.Get(context.TODO(), ok, &retrieved)
-			Expect(err).NotTo(HaveOccurred())
 
-			Expect(retrieved.Status.ClientID).NotTo(BeNil())
+				err = apiv1.AddToScheme(s)
-			Expect(retrieved.Status.Secret).To(BeNil())
+				Expect(err).NotTo(HaveOccurred())
-			Expect(retrieved.Status.ReconciliationError).NotTo(BeNil())
+
-			Expect(retrieved.Status.ReconciliationError.Code).To(Equal(hydrav1alpha1.StatusCreateSecretFailed))
+				// Setup the Manager and Controller.  Wrap the Controller Reconcile function so it writes each request to a
-			Expect(retrieved.Status.ReconciliationError.Description).To(Equal(`secrets "test3" already exists`))
+				// 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("DeleteOAuth2Client", Anything).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 invalid secret exists
+				secret := apiv1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      tstSecretName,
+						Namespace: tstNamespace,
+					},
+					Data: map[string][]byte{
+						controllers.ClientIDKey: []byte(tstClientID),
+						//missing client secret key
+					},
+				}
+				err = c.Create(context.TODO(), &secret)
+				Expect(err).NotTo(HaveOccurred())
+
+				instance := testInstance(tstName, tstSecretName)
+				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())
+				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.ReconciliationError).NotTo(BeNil())
+				Expect(retrieved.Status.ReconciliationError.Code).To(Equal(hydrav1alpha1.StatusInvalidSecret))
+				Expect(retrieved.Status.ReconciliationError.Description).To(Equal(`"client_secret property missing"`))
+
+				//delete instance
+				c.Delete(context.TODO(), instance)
+
+				//Ensure manager is stopped properly
+				close(stopMgr)
+				mgrStopped.Wait()
+			})
 		})
 	})
 })
@@ -262,16 +354,6 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {
 		return err
 	}
 
-	// TODO(user): Modify this to be the types you create
-	// Uncomment watch a Deployment created by Guestbook - change this for objects you create
-	//err = c.Watch(&source.Kind{Type: &appsv1.Deployment{}}, &handler.EnqueueRequestForOwner{
-	//	IsController: true,
-	//	OwnerType:    &webappv1.Guestbook{},
-	//})
-	//if err != nil {
-	//	return err
-	//}
-
 	return nil
 }
 
@@ -283,7 +365,7 @@ func getAPIReconciler(mgr ctrl.Manager, mock controllers.HydraClientInterface) r
 	}
 }
 
-func testInstance(name string) *hydrav1alpha1.OAuth2Client {
+func testInstance(name, secretName string) *hydrav1alpha1.OAuth2Client {
 
 	return &hydrav1alpha1.OAuth2Client{
 		ObjectMeta: metav1.ObjectMeta{
@@ -293,6 +375,7 @@ func testInstance(name string) *hydrav1alpha1.OAuth2Client {
 		Spec: hydrav1alpha1.OAuth2ClientSpec{
 			GrantTypes:    []hydrav1alpha1.GrantType{"client_credentials"},
 			ResponseTypes: []hydrav1alpha1.ResponseType{"token"},
-			Scope:         tstScopes,
+			Scope:         "a b c",
+			SecretName:    secretName,
 		}}
 }

hydra/types.go

@@ -1,11 +1,25 @@
 package hydra
 
+import "k8s.io/utils/pointer"
+
 // OAuth2ClientJSON represents an OAuth2 client digestible by ORY Hydra
 type OAuth2ClientJSON struct {
 	ClientID      *string  `json:"client_id,omitempty"`
+	Secret        *string  `json:"client_secret,omitempty"`
 	Name          string   `json:"client_name"`
 	GrantTypes    []string `json:"grant_types"`
 	ResponseTypes []string `json:"response_types,omitempty"`
 	Scope         string   `json:"scope"`
-	Secret        *string  `json:"client_secret,omitempty"`
+}
+
+// Oauth2ClientCredentials represents client ID and password fetched from a Kubernetes secret
+type Oauth2ClientCredentials struct {
+	ID       []byte
+	Password []byte
+}
+
+func (oj *OAuth2ClientJSON) WithCredentials(credentials *Oauth2ClientCredentials) *OAuth2ClientJSON {
+	oj.ClientID = pointer.StringPtr(string(credentials.ID))
+	oj.Secret = pointer.StringPtr(string(credentials.Password))
+	return oj
 }