From 294c171ac6738d53798fad7143c583cde6ac8593 Mon Sep 17 00:00:00 2001 From: Jakub Kabza Date: Fri, 13 Sep 2019 14:37:29 +0200 Subject: [PATCH] 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 | 2 +- README.md | 2 +- api/v1alpha1/oauth2client_types.go | 14 +- api/v1alpha1/oauth2client_types_test.go | 2 + api/v1alpha1/zz_generated.deepcopy.go | 12 +- .../crd/bases/hydra.ory.sh_oauth2clients.yaml | 15 +- .../samples/hydra_v1alpha1_oauth2client.yaml | 3 +- ...1alpha1_oauth2client_user_credentials.yaml | 27 + controllers/oauth2client_controller.go | 190 ++++--- ...auth2client_controller_integration_test.go | 471 ++++++++++-------- hydra/types.go | 16 +- 11 files changed, 474 insertions(+), 280 deletions(-) create mode 100644 config/samples/hydra_v1alpha1_oauth2client_user_credentials.yaml diff --git a/Makefile b/Makefile index 2a63c3e..be6661a 100644 --- a/Makefile +++ b/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 diff --git a/README.md b/README.md index 2086b50..1245484 100644 --- a/README.md +++ b/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). diff --git a/api/v1alpha1/oauth2client_types.go b/api/v1alpha1/oauth2client_types.go index d433686..3304fd2 100644 --- a/api/v1alpha1/oauth2client_types.go +++ b/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, diff --git a/api/v1alpha1/oauth2client_types_test.go b/api/v1alpha1/oauth2client_types_test.go index 84dc96d..61d9982 100644 --- a/api/v1alpha1/oauth2client_types_test.go +++ b/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", }, } } diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 056bf0e..d2b2e34 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/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 } diff --git a/config/crd/bases/hydra.ory.sh_oauth2clients.yaml b/config/crd/bases/hydra.ory.sh_oauth2clients.yaml index 2729aa9..249bfc2 100644 --- a/config/crd/bases/hydra.ory.sh_oauth2clients.yaml +++ b/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: diff --git a/config/samples/hydra_v1alpha1_oauth2client.yaml b/config/samples/hydra_v1alpha1_oauth2client.yaml index a644bf2..f77bee0 100644 --- a/config/samples/hydra_v1alpha1_oauth2client.yaml +++ b/config/samples/hydra_v1alpha1_oauth2client.yaml @@ -13,4 +13,5 @@ spec: - id_token - code - token - scope: "read write" \ No newline at end of file + scope: "read write" + secretName: my-secret-123 diff --git a/config/samples/hydra_v1alpha1_oauth2client_user_credentials.yaml b/config/samples/hydra_v1alpha1_oauth2client_user_credentials.yaml new file mode 100644 index 0000000..1394351 --- /dev/null +++ b/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 diff --git a/controllers/oauth2client_controller.go b/controllers/oauth2client_controller.go index e196ea0..225e21e 100644 --- a/controllers/oauth2client_controller.go +++ b/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" - clientSecretKey = "client_secret" + ClientIDKey = "client_id" + 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 - if err := r.Get(ctx, req.NamespacedName, &client); err != nil { + var oauth2client hydrav1alpha1.OAuth2Client + 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 err error - - if client.Status.ClientID != nil { - - _, registered, err = r.HydraClient.GetOAuth2Client(*client.Status.ClientID) - if err != nil { - return ctrl.Result{}, err + var secret apiv1.Secret + if err := r.Get(ctx, types.NamespacedName{Name: oauth2client.Spec.SecretName, Namespace: req.Namespace}, &secret); err != nil { + if apierrs.IsNotFound(err) { + return ctrl.Result{}, r.registerOAuth2Client(ctx, &oauth2client, nil) } + return ctrl.Result{}, err } - if !registered { - return ctrl.Result{}, r.registerOAuth2Client(ctx, &client) + credentials, err := parseSecret(secret) + 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 { - 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 { - r.Log.Error(err, fmt.Sprintf("error registring client %s/%s ", client.Name, client.Namespace), "oauth2client", "register") - return updateErr - } +func (r *OAuth2ClientReconciler) registerOAuth2Client(ctx context.Context, c *hydrav1alpha1.OAuth2Client, credentials *hydra.Oauth2ClientCredentials) error { + if err := r.unregisterOAuth2Clients(ctx, c.Name, c.Namespace); err != nil { + return err + } + 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, - Namespace: client.Namespace, + Name: c.Spec.SecretName, + 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 - client.Status.ObservedGeneration = client.Generation - - 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 + if err := r.Create(ctx, &clientSecret); err != nil { + return r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusCreateSecretFailed, err) } - return r.Status().Update(ctx, client) + return nil } -func (r *OAuth2ClientReconciler) unregisterOAuth2Client(ctx context.Context, namespacedName types.NamespacedName) error { - var sec apiv1.Secret - if err := r.Get(ctx, namespacedName, &sec); err != nil { - 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 - } +func (r *OAuth2ClientReconciler) updateRegisteredOAuth2Client(ctx context.Context, c *hydrav1alpha1.OAuth2Client, credentials *hydra.Oauth2ClientCredentials) error { + if _, err := r.HydraClient.PutOAuth2Client(c.ToOAuth2ClientJSON().WithCredentials(credentials)); err != nil { + return r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusUpdateFailed, err) + } + 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 { - _, err := r.HydraClient.PutOAuth2Client(client.ToOAuth2ClientJSON()) - return err +func (r *OAuth2ClientReconciler) updateReconciliationStatusError(ctx context.Context, c *hydrav1alpha1.OAuth2Client, code hydrav1alpha1.StatusCode, err error) error { + r.Log.Error(err, fmt.Sprintf("error processing client %s/%s ", c.Name, c.Namespace), "oauth2client", "register") + 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 } diff --git a/controllers/oauth2client_controller_integration_test.go b/controllers/oauth2client_controller_integration_test.go index 67004ff..4d6169c 100644 --- a/controllers/oauth2client_controller_integration_test.go +++ b/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" - "github.com/pkg/errors" + "k8s.io/utils/pointer" . "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" - expectedRequest := &reconcile.Request{NamespacedName: types.NamespacedName{Name: tstName, Namespace: tstNamespace}} + It("create a Secret if it does not exist", func() { - s := scheme.Scheme - err := hydrav1alpha1.AddToScheme(s) - Expect(err).NotTo(HaveOccurred()) + tstName, tstClientID, tstSecretName := "test", "testClientID", "my-secret-123" + expectedRequest := &reconcile.Request{NamespacedName: types.NamespacedName{Name: tstName, Namespace: tstNamespace}} - // 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() + s := scheme.Scheme + err := hydrav1alpha1.AddToScheme(s) + Expect(err).NotTo(HaveOccurred()) - 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, + err = apiv1.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("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 { - return nil + 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()) + + //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 - stopMgr, mgrStopped := StartTestManager(mgr) + s := scheme.Scheme + err := hydrav1alpha1.AddToScheme(s) + Expect(err).NotTo(HaveOccurred()) - //Ensure manager is stopped properly - defer func() { - close(stopMgr) - mgrStopped.Wait() - }() + err = apiv1.AddToScheme(s) + 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))) + // 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() - //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()) + mch := &mocks.HydraClientInterface{} + mch.On("PostOAuth2Client", Anything).Return(nil, errors.New("error")) + mch.On("DeleteOAuth2Client", Anything).Return(nil) - Expect(*retrieved.Status.ClientID).To(Equal(tstClientID)) - Expect(*retrieved.Status.Secret).To(Equal(tstName)) //Secret contents is not visible in the CR instance! + recFn, requests := SetupTestReconcile(getAPIReconciler(mgr, mch)) - //Verify the created Secret - var createdSecret = apiv1.Secret{} - k8sClient.Get(context.TODO(), ok, &createdSecret) - Expect(err).NotTo(HaveOccurred()) - Expect(createdSecret.Data["client_secret"]).To(Equal([]byte(tstSecret))) - }) + Expect(add(mgr, recFn)).To(Succeed()) - 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" - 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, + 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 { - return nil + 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.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 - stopMgr, mgrStopped := StartTestManager(mgr) + s := scheme.Scheme + err := hydrav1alpha1.AddToScheme(s) + Expect(err).NotTo(HaveOccurred()) - //Ensure manager is stopped properly - defer func() { + err = apiv1.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("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 - secret := apiv1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: tstName, - Namespace: tstNamespace, - }, - } - err = c.Create(context.TODO(), &secret) - Expect(err).NotTo(HaveOccurred()) + It("update object status if provided Secret is invalid", func() { - 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))) + tstName, tstClientID, tstSecretName := "test4", "testClientID-4", "my-secret-000" + expectedRequest := &reconcile.Request{NamespacedName: types.NamespacedName{Name: tstName, Namespace: tstNamespace}} - //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()) + s := scheme.Scheme + err := hydrav1alpha1.AddToScheme(s) + 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`)) + err = apiv1.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("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, }} } diff --git a/hydra/types.go b/hydra/types.go index e036fde..fe128d8 100644 --- a/hydra/types.go +++ b/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 }