From d9eeaace15eb37c484c5ee01b3ed3ed10fd2a45b Mon Sep 17 00:00:00 2001 From: Roman Lytvyn Date: Thu, 3 Jun 2021 15:00:02 +0200 Subject: [PATCH 1/7] fix: oauth client metadata as object in CRD (#71) --- api/v1alpha1/oauth2client_types.go | 9 +++++++-- api/v1alpha1/zz_generated.deepcopy.go | 7 +------ config/crd/bases/hydra.ory.sh_oauth2clients.yaml | 4 ++-- go.mod | 1 + 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/api/v1alpha1/oauth2client_types.go b/api/v1alpha1/oauth2client_types.go index 3f03950..9c83269 100644 --- a/api/v1alpha1/oauth2client_types.go +++ b/api/v1alpha1/oauth2client_types.go @@ -20,6 +20,7 @@ import ( "fmt" "github.com/ory/hydra-maester/hydra" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -120,8 +121,10 @@ type OAuth2ClientSpec struct { // Indication which authentication method shoud be used for the token endpoint TokenEndpointAuthMethod TokenEndpointAuthMethod `json:"tokenEndpointAuthMethod,omitempty"` + // +kubebuilder:validation:Type=object + // // Metadata is abritrary data - Metadata json.RawMessage `json:"metadata,omitempty"` + Metadata apiextensionsv1.JSON `json:"metadata,omitempty"` } // +kubebuilder:validation:Enum=client_credentials;authorization_code;implicit;refresh_token @@ -182,6 +185,8 @@ func init() { // ToOAuth2ClientJSON converts an OAuth2Client into a OAuth2ClientJSON object that represents an OAuth2 client digestible by ORY Hydra func (c *OAuth2Client) ToOAuth2ClientJSON() *hydra.OAuth2ClientJSON { + meta, _ := json.Marshal(c.Spec.Metadata) + return &hydra.OAuth2ClientJSON{ ClientName: c.Spec.ClientName, GrantTypes: grantToStringSlice(c.Spec.GrantTypes), @@ -193,7 +198,7 @@ func (c *OAuth2Client) ToOAuth2ClientJSON() *hydra.OAuth2ClientJSON { Scope: c.Spec.Scope, Owner: fmt.Sprintf("%s/%s", c.Name, c.Namespace), TokenEndpointAuthMethod: string(c.Spec.TokenEndpointAuthMethod), - Metadata: c.Spec.Metadata, + Metadata: meta, } } diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 1050062..fa8143c 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -20,7 +20,6 @@ limitations under the License. package v1alpha1 import ( - "encoding/json" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -132,11 +131,7 @@ func (in *OAuth2ClientSpec) DeepCopyInto(out *OAuth2ClientSpec) { copy(*out, *in) } out.HydraAdmin = in.HydraAdmin - if in.Metadata != nil { - in, out := &in.Metadata, &out.Metadata - *out = make(json.RawMessage, len(*in)) - copy(*out, *in) - } + in.Metadata.DeepCopyInto(&out.Metadata) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OAuth2ClientSpec. diff --git a/config/crd/bases/hydra.ory.sh_oauth2clients.yaml b/config/crd/bases/hydra.ory.sh_oauth2clients.yaml index 665e907..d1d8dfc 100644 --- a/config/crd/bases/hydra.ory.sh_oauth2clients.yaml +++ b/config/crd/bases/hydra.ory.sh_oauth2clients.yaml @@ -99,8 +99,8 @@ spec: type: object metadata: description: Metadata is abritrary data - format: byte - type: string + type: object + x-kubernetes-preserve-unknown-fields: true postLogoutRedirectUris: description: PostLogoutRedirectURIs is an array of the post logout redirect URIs allowed for the application diff --git a/go.mod b/go.mod index 4616dfa..55f5730 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/stretchr/testify v1.6.1 golang.org/x/net v0.0.0-20201110031124-69a78807bb2b k8s.io/api v0.20.2 + k8s.io/apiextensions-apiserver v0.20.1 k8s.io/apimachinery v0.20.2 k8s.io/client-go v0.20.2 k8s.io/utils v0.0.0-20210305010621-2afb4311ab10 From 1bd0d394b0687fe5725392494dfc02ad80aadf09 Mon Sep 17 00:00:00 2001 From: Roman Lytvyn Date: Thu, 3 Jun 2021 20:14:06 +0200 Subject: [PATCH 2/7] fix: mark oauth client metadata nullable and optional --- api/v1alpha1/oauth2client_types.go | 2 ++ config/crd/bases/hydra.ory.sh_oauth2clients.yaml | 1 + 2 files changed, 3 insertions(+) diff --git a/api/v1alpha1/oauth2client_types.go b/api/v1alpha1/oauth2client_types.go index 9c83269..5da7aaa 100644 --- a/api/v1alpha1/oauth2client_types.go +++ b/api/v1alpha1/oauth2client_types.go @@ -122,6 +122,8 @@ type OAuth2ClientSpec struct { TokenEndpointAuthMethod TokenEndpointAuthMethod `json:"tokenEndpointAuthMethod,omitempty"` // +kubebuilder:validation:Type=object + // +nullable + // +optional // // Metadata is abritrary data Metadata apiextensionsv1.JSON `json:"metadata,omitempty"` diff --git a/config/crd/bases/hydra.ory.sh_oauth2clients.yaml b/config/crd/bases/hydra.ory.sh_oauth2clients.yaml index d1d8dfc..316fd28 100644 --- a/config/crd/bases/hydra.ory.sh_oauth2clients.yaml +++ b/config/crd/bases/hydra.ory.sh_oauth2clients.yaml @@ -99,6 +99,7 @@ spec: type: object metadata: description: Metadata is abritrary data + nullable: true type: object x-kubernetes-preserve-unknown-fields: true postLogoutRedirectUris: From 80292241784587e9254ff30e14f45b010352353e Mon Sep 17 00:00:00 2001 From: Roman Lytvyn Date: Fri, 4 Jun 2021 13:10:08 +0200 Subject: [PATCH 3/7] add: metadata encoding error propagation --- api/v1alpha1/oauth2client_types.go | 10 +++++++--- controllers/oauth2client_controller.go | 16 +++++++++++++--- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/api/v1alpha1/oauth2client_types.go b/api/v1alpha1/oauth2client_types.go index 5da7aaa..b4324d3 100644 --- a/api/v1alpha1/oauth2client_types.go +++ b/api/v1alpha1/oauth2client_types.go @@ -20,6 +20,7 @@ import ( "fmt" "github.com/ory/hydra-maester/hydra" + "github.com/pkg/errors" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -186,8 +187,11 @@ func init() { } // ToOAuth2ClientJSON converts an OAuth2Client into a OAuth2ClientJSON object that represents an OAuth2 client digestible by ORY Hydra -func (c *OAuth2Client) ToOAuth2ClientJSON() *hydra.OAuth2ClientJSON { - meta, _ := json.Marshal(c.Spec.Metadata) +func (c *OAuth2Client) ToOAuth2ClientJSON() (*hydra.OAuth2ClientJSON, error) { + meta, err := json.Marshal(c.Spec.Metadata) + if err != nil { + return nil, errors.WithMessage(err, "unable to encode `metadata` property value to json") + } return &hydra.OAuth2ClientJSON{ ClientName: c.Spec.ClientName, @@ -201,7 +205,7 @@ func (c *OAuth2Client) ToOAuth2ClientJSON() *hydra.OAuth2ClientJSON { Owner: fmt.Sprintf("%s/%s", c.Name, c.Namespace), TokenEndpointAuthMethod: string(c.Spec.TokenEndpointAuthMethod), Metadata: meta, - } + }, nil } func responseToStringSlice(rt []ResponseType) []string { diff --git a/controllers/oauth2client_controller.go b/controllers/oauth2client_controller.go index 7863f8a..06e2e7d 100644 --- a/controllers/oauth2client_controller.go +++ b/controllers/oauth2client_controller.go @@ -205,8 +205,13 @@ func (r *OAuth2ClientReconciler) registerOAuth2Client(ctx context.Context, c *hy return err } + oauth2client, err := c.ToOAuth2ClientJSON() + if err != nil { + return err + } + if credentials != nil { - if _, err := hydra.PostOAuth2Client(c.ToOAuth2ClientJSON().WithCredentials(credentials)); err != nil { + if _, err := hydra.PostOAuth2Client(oauth2client.WithCredentials(credentials)); err != nil { if updateErr := r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusRegistrationFailed, err); updateErr != nil { return updateErr } @@ -214,7 +219,7 @@ func (r *OAuth2ClientReconciler) registerOAuth2Client(ctx context.Context, c *hy return r.ensureEmptyStatusError(ctx, c) } - created, err := hydra.PostOAuth2Client(c.ToOAuth2ClientJSON()) + created, err := hydra.PostOAuth2Client(oauth2client) if err != nil { if updateErr := r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusRegistrationFailed, err); updateErr != nil { return updateErr @@ -257,7 +262,12 @@ func (r *OAuth2ClientReconciler) updateRegisteredOAuth2Client(ctx context.Contex return err } - if _, err := hydra.PutOAuth2Client(c.ToOAuth2ClientJSON().WithCredentials(credentials)); err != nil { + oauth2client, err := c.ToOAuth2ClientJSON() + if err != nil { + return err + } + + if _, err := hydra.PutOAuth2Client(oauth2client.WithCredentials(credentials)); err != nil { if updateErr := r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusUpdateFailed, err); updateErr != nil { return updateErr } From 989dd1567a45b1960ec872218328d364a9e086a7 Mon Sep 17 00:00:00 2001 From: Roman Lytvyn Date: Fri, 4 Jun 2021 13:48:35 +0200 Subject: [PATCH 4/7] fix: add missing reconciliation status error update --- controllers/oauth2client_controller.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/controllers/oauth2client_controller.go b/controllers/oauth2client_controller.go index 06e2e7d..6c519f2 100644 --- a/controllers/oauth2client_controller.go +++ b/controllers/oauth2client_controller.go @@ -207,7 +207,10 @@ func (r *OAuth2ClientReconciler) registerOAuth2Client(ctx context.Context, c *hy oauth2client, err := c.ToOAuth2ClientJSON() if err != nil { - return err + if updateErr := r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusRegistrationFailed, err); updateErr != nil { + return updateErr + } + return nil } if credentials != nil { @@ -264,7 +267,10 @@ func (r *OAuth2ClientReconciler) updateRegisteredOAuth2Client(ctx context.Contex oauth2client, err := c.ToOAuth2ClientJSON() if err != nil { - return err + if updateErr := r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusRegistrationFailed, err); updateErr != nil { + return updateErr + } + return nil } if _, err := hydra.PutOAuth2Client(oauth2client.WithCredentials(credentials)); err != nil { From 9f58744fe5004bc569579349ccde4f199386a2d7 Mon Sep 17 00:00:00 2001 From: Roman Lytvyn Date: Fri, 4 Jun 2021 13:54:41 +0200 Subject: [PATCH 5/7] fix: use correct status code for update --- controllers/oauth2client_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/oauth2client_controller.go b/controllers/oauth2client_controller.go index 6c519f2..3ca6357 100644 --- a/controllers/oauth2client_controller.go +++ b/controllers/oauth2client_controller.go @@ -267,7 +267,7 @@ func (r *OAuth2ClientReconciler) updateRegisteredOAuth2Client(ctx context.Contex oauth2client, err := c.ToOAuth2ClientJSON() if err != nil { - if updateErr := r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusRegistrationFailed, err); updateErr != nil { + if updateErr := r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusUpdateFailed, err); updateErr != nil { return updateErr } return nil From cd33789adcfe396782f36e59144fdc4a27321d57 Mon Sep 17 00:00:00 2001 From: Roman Lytvyn Date: Fri, 4 Jun 2021 14:36:15 +0200 Subject: [PATCH 6/7] fix: return original error if no reconciliation update error --- controllers/oauth2client_controller.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/controllers/oauth2client_controller.go b/controllers/oauth2client_controller.go index 3ca6357..751cf0b 100644 --- a/controllers/oauth2client_controller.go +++ b/controllers/oauth2client_controller.go @@ -210,7 +210,7 @@ func (r *OAuth2ClientReconciler) registerOAuth2Client(ctx context.Context, c *hy if updateErr := r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusRegistrationFailed, err); updateErr != nil { return updateErr } - return nil + return err } if credentials != nil { @@ -270,7 +270,7 @@ func (r *OAuth2ClientReconciler) updateRegisteredOAuth2Client(ctx context.Contex if updateErr := r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusUpdateFailed, err); updateErr != nil { return updateErr } - return nil + return err } if _, err := hydra.PutOAuth2Client(oauth2client.WithCredentials(credentials)); err != nil { From f9bfa0a0ddb108cd312bcc127a84bda1bd82c635 Mon Sep 17 00:00:00 2001 From: Roman Lytvyn Date: Fri, 4 Jun 2021 15:05:23 +0200 Subject: [PATCH 7/7] fix: annotate errors with stack trace --- controllers/oauth2client_controller.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/controllers/oauth2client_controller.go b/controllers/oauth2client_controller.go index 751cf0b..623f420 100644 --- a/controllers/oauth2client_controller.go +++ b/controllers/oauth2client_controller.go @@ -210,7 +210,7 @@ func (r *OAuth2ClientReconciler) registerOAuth2Client(ctx context.Context, c *hy if updateErr := r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusRegistrationFailed, err); updateErr != nil { return updateErr } - return err + return errors.WithStack(err) } if credentials != nil { @@ -270,7 +270,7 @@ func (r *OAuth2ClientReconciler) updateRegisteredOAuth2Client(ctx context.Contex if updateErr := r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusUpdateFailed, err); updateErr != nil { return updateErr } - return err + return errors.WithStack(err) } if _, err := hydra.PutOAuth2Client(oauth2client.WithCredentials(credentials)); err != nil {