Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix(controller): Ensure that OAuth2Client reconciliation creates hydra client for specs #83

Merged
merged 6 commits into from
Sep 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 0 additions & 51 deletions api/v1alpha1/oauth2client_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,6 @@ limitations under the License.
package v1alpha1

import (
"encoding/json"
"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"
)
Expand Down Expand Up @@ -185,49 +180,3 @@ type OAuth2ClientList struct {
func init() {
SchemeBuilder.Register(&OAuth2Client{}, &OAuth2ClientList{})
}

// ToOAuth2ClientJSON converts an OAuth2Client into a OAuth2ClientJSON object that represents an OAuth2 client digestible by ORY Hydra
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,
GrantTypes: grantToStringSlice(c.Spec.GrantTypes),
ResponseTypes: responseToStringSlice(c.Spec.ResponseTypes),
RedirectURIs: redirectToStringSlice(c.Spec.RedirectURIs),
PostLogoutRedirectURIs: redirectToStringSlice(c.Spec.PostLogoutRedirectURIs),
AllowedCorsOrigins: redirectToStringSlice(c.Spec.AllowedCorsOrigins),
Audience: c.Spec.Audience,
Scope: c.Spec.Scope,
Owner: fmt.Sprintf("%s/%s", c.Name, c.Namespace),
TokenEndpointAuthMethod: string(c.Spec.TokenEndpointAuthMethod),
Metadata: meta,
}, nil
}

func responseToStringSlice(rt []ResponseType) []string {
var output = make([]string, len(rt))
for i, elem := range rt {
output[i] = string(elem)
}
return output
}

func grantToStringSlice(gt []GrantType) []string {
var output = make([]string, len(gt))
for i, elem := range gt {
output[i] = string(elem)
}
return output
}

func redirectToStringSlice(ru []RedirectURI) []string {
var output = make([]string, len(ru))
for i, elem := range ru {
output[i] = string(elem)
}
return output
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

127 changes: 97 additions & 30 deletions controllers/oauth2client_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,46 +18,99 @@ package controllers
import (
"context"
"fmt"
"sync"

"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"

hydrav1alpha1 "github.com/ory/hydra-maester/api/v1alpha1"
"github.com/ory/hydra-maester/hydra"
)

const (
ClientIDKey = "client_id"
ClientSecretKey = "client_secret"
FinalizerName = "finalizer.ory.hydra.sh"

DefaultNamespace = "default"
)

type clientMapKey struct {
type clientKey struct {
url string
port int
endpoint string
forwardedProto string
}

type HydraClientInterface interface {
GetOAuth2Client(id string) (*hydra.OAuth2ClientJSON, bool, error)
ListOAuth2Client() ([]*hydra.OAuth2ClientJSON, error)
PostOAuth2Client(o *hydra.OAuth2ClientJSON) (*hydra.OAuth2ClientJSON, error)
PutOAuth2Client(o *hydra.OAuth2ClientJSON) (*hydra.OAuth2ClientJSON, error)
DeleteOAuth2Client(id string) error
}
// OAuth2ClientFactory is a function that creates oauth2 client.
// The OAuth2ClientReconciler defaults to use hydra.New and the factory allows
// to override this behavior for mocks during tests.
type OAuth2ClientFactory func(
spec hydrav1alpha1.OAuth2ClientSpec,
tlsTrustStore string,
insecureSkipVerify bool,
) (hydra.Client, error)

// OAuth2ClientReconciler reconciles a OAuth2Client object
// OAuth2ClientReconciler reconciles a OAuth2Client object.
type OAuth2ClientReconciler struct {
HydraClient HydraClientInterface
Log logr.Logger
otherClients map[clientMapKey]HydraClientInterface
client.Client
HydraClient hydra.Client
Log logr.Logger
ControllerNamespace string

oauth2Clients map[clientKey]hydra.Client
oauth2ClientFactory OAuth2ClientFactory
mu sync.Mutex
}

// Options represent options to pass to the oauth2 client reconciler.
type Options struct {
Namespace string
OAuth2ClientFactory OAuth2ClientFactory
}

// Option is a functional option.
type Option func(*Options)

// WithNamespace sets the kubernetes namespace for the controller.
// The default is "default".
func WithNamespace(ns string) Option {
return func(o *Options) {
o.Namespace = ns
}
}

// WithClientFactory sets a function to create new oauth2 clients during the reconciliation logic.
func WithClientFactory(factory OAuth2ClientFactory) Option {
return func(o *Options) {
o.OAuth2ClientFactory = factory
}
}

// New returns a new Oauth2ClientReconciler.
func New(c client.Client, hydraClient hydra.Client, log logr.Logger, opts ...Option) *OAuth2ClientReconciler {
options := &Options{
Namespace: DefaultNamespace,
OAuth2ClientFactory: hydra.New,
}
for _, opt := range opts {
opt(options)
}

return &OAuth2ClientReconciler{
Client: c,
HydraClient: hydraClient,
Log: log,
ControllerNamespace: options.Namespace,
oauth2Clients: make(map[clientKey]hydra.Client, 0),
oauth2ClientFactory: options.OAuth2ClientFactory,
}
}

// +kubebuilder:rbac:groups=hydra.ory.sh,resources=oauth2clients,verbs=get;list;watch;create;update;patch;delete
Expand Down Expand Up @@ -200,12 +253,12 @@ func (r *OAuth2ClientReconciler) registerOAuth2Client(ctx context.Context, c *hy
return err
}

hydra, err := r.getHydraClientForClient(*c)
hydraClient, err := r.getHydraClientForClient(*c)
if err != nil {
return err
}

oauth2client, err := c.ToOAuth2ClientJSON()
oauth2client, err := hydra.FromOAuth2Client(c)
if err != nil {
if updateErr := r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusRegistrationFailed, err); updateErr != nil {
return updateErr
Expand All @@ -214,15 +267,15 @@ func (r *OAuth2ClientReconciler) registerOAuth2Client(ctx context.Context, c *hy
}

if credentials != nil {
if _, err := hydra.PostOAuth2Client(oauth2client.WithCredentials(credentials)); err != nil {
if _, err := hydraClient.PostOAuth2Client(oauth2client.WithCredentials(credentials)); err != nil {
if updateErr := r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusRegistrationFailed, err); updateErr != nil {
return updateErr
}
}
return r.ensureEmptyStatusError(ctx, c)
}

created, err := hydra.PostOAuth2Client(oauth2client)
created, err := hydraClient.PostOAuth2Client(oauth2client)
if err != nil {
if updateErr := r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusRegistrationFailed, err); updateErr != nil {
return updateErr
Expand Down Expand Up @@ -260,20 +313,20 @@ func (r *OAuth2ClientReconciler) registerOAuth2Client(ctx context.Context, c *hy
}

func (r *OAuth2ClientReconciler) updateRegisteredOAuth2Client(ctx context.Context, c *hydrav1alpha1.OAuth2Client, credentials *hydra.Oauth2ClientCredentials) error {
hydra, err := r.getHydraClientForClient(*c)
hydraClient, err := r.getHydraClientForClient(*c)
if err != nil {
return err
}

oauth2client, err := c.ToOAuth2ClientJSON()
oauth2client, err := hydra.FromOAuth2Client(c)
if err != nil {
if updateErr := r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusUpdateFailed, err); updateErr != nil {
return updateErr
}
return errors.WithStack(err)
}

if _, err := hydra.PutOAuth2Client(oauth2client.WithCredentials(credentials)); err != nil {
if _, err := hydraClient.PutOAuth2Client(oauth2client.WithCredentials(credentials)); err != nil {
if updateErr := r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusUpdateFailed, err); updateErr != nil {
return updateErr
}
Expand Down Expand Up @@ -352,17 +405,31 @@ func parseSecret(secret apiv1.Secret, authMethod hydrav1alpha1.TokenEndpointAuth
}, nil
}

func (r *OAuth2ClientReconciler) getHydraClientForClient(oauth2client hydrav1alpha1.OAuth2Client) (HydraClientInterface, error) {
func (r *OAuth2ClientReconciler) getHydraClientForClient(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This IMO should have a test, unit at least

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll work on this today.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually I find it a bit anti-idiomatic to create a unit test for a private function.

I'm wondering if we should we create an integration test for this instead?

oauth2client hydrav1alpha1.OAuth2Client) (hydra.Client, error) {
spec := oauth2client.Spec
key := clientMapKey{
url: spec.HydraAdmin.URL,
port: spec.HydraAdmin.Port,
endpoint: spec.HydraAdmin.Endpoint,
forwardedProto: spec.HydraAdmin.ForwardedProto,
}
if c, ok := r.otherClients[key]; ok {
return c, nil
if spec.HydraAdmin.URL != "" {
key := clientKey{
url: spec.HydraAdmin.URL,
port: spec.HydraAdmin.Port,
endpoint: spec.HydraAdmin.Endpoint,
forwardedProto: spec.HydraAdmin.ForwardedProto,
}
r.mu.Lock()
defer r.mu.Unlock()
if c, ok := r.oauth2Clients[key]; ok {
return c, nil
}

client, err := r.oauth2ClientFactory(spec, "", false)
if err != nil {
return nil, errors.Wrap(err, "cannot create oauth2 client from CRD")
}

r.oauth2Clients[key] = client
return client, nil
}

if r.HydraClient == nil {
return nil, errors.New("Not default client or other clients configured")
}
Expand Down
Loading