diff --git a/Makefile b/Makefile index 59a8110..484f302 100644 --- a/Makefile +++ b/Makefile @@ -8,7 +8,7 @@ all: manager # Run tests test: generate fmt vet manifests - go test ./api/... ./controllers/... ./hydra... -coverprofile cover.out + go test ./api/... ./controllers/... ./hydra/... -coverprofile cover.out # Run integration tests on local KIND cluster # TODO: modify once integration tests have been implemented diff --git a/controllers/oauth2client_controller.go b/controllers/oauth2client_controller.go index 1e80dfa..c6cea9d 100644 --- a/controllers/oauth2client_controller.go +++ b/controllers/oauth2client_controller.go @@ -117,66 +117,68 @@ func (r *OAuth2ClientReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error } - if oauth2client.Generation != oauth2client.Status.ObservedGeneration { - - 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) { - if registerErr := r.registerOAuth2Client(ctx, &oauth2client, nil); registerErr != nil { - return ctrl.Result{}, registerErr - } - return ctrl.Result{}, nil + 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) { + if registerErr := r.registerOAuth2Client(ctx, &oauth2client, nil); registerErr != nil { + return ctrl.Result{}, registerErr } - return ctrl.Result{}, err + return ctrl.Result{}, nil } + return ctrl.Result{}, err + } - credentials, err := parseSecret(secret) - if err != nil { - r.Log.Error(err, fmt.Sprintf("secret %s/%s is invalid", secret.Name, secret.Namespace)) - if updateErr := r.updateReconciliationStatusError(ctx, &oauth2client, hydrav1alpha1.StatusInvalidSecret, err); updateErr != nil { - return ctrl.Result{}, updateErr - } - return ctrl.Result{}, nil + credentials, err := parseSecret(secret) + if err != nil { + r.Log.Error(err, fmt.Sprintf("secret %s/%s is invalid", secret.Name, secret.Namespace)) + if updateErr := r.updateReconciliationStatusError(ctx, &oauth2client, hydrav1alpha1.StatusInvalidSecret, err); updateErr != nil { + return ctrl.Result{}, updateErr } + return ctrl.Result{}, nil + } - hydraClient, err := r.getHydraClientForClient(oauth2client) - if err != nil { - r.Log.Error(err, fmt.Sprintf( - "hydra address %s:%d%s is invalid", - oauth2client.Spec.HydraAdmin.URL, - oauth2client.Spec.HydraAdmin.Port, - oauth2client.Spec.HydraAdmin.Endpoint, - )) - if updateErr := r.updateReconciliationStatusError(ctx, &oauth2client, hydrav1alpha1.StatusInvalidHydraAddress, err); updateErr != nil { - return ctrl.Result{}, updateErr - } - return ctrl.Result{}, nil + hydraClient, err := r.getHydraClientForClient(oauth2client) + if err != nil { + r.Log.Error(err, fmt.Sprintf( + "hydra address %s:%d%s is invalid", + oauth2client.Spec.HydraAdmin.URL, + oauth2client.Spec.HydraAdmin.Port, + oauth2client.Spec.HydraAdmin.Endpoint, + )) + if updateErr := r.updateReconciliationStatusError(ctx, &oauth2client, hydrav1alpha1.StatusInvalidHydraAddress, err); updateErr != nil { + return ctrl.Result{}, updateErr } + return ctrl.Result{}, nil + } - fetched, found, err := hydraClient.GetOAuth2Client(string(credentials.ID)) - if err != nil { - return ctrl.Result{}, err + fetched, found, err := hydraClient.GetOAuth2Client(string(credentials.ID)) + if err != nil { + return ctrl.Result{}, err - } + } - if found { - if fetched.Owner != fmt.Sprintf("%s/%s", oauth2client.Name, oauth2client.Namespace) { - conflictErr := errors.Errorf("ID provided in secret %s/%s is assigned to another resource", secret.Name, secret.Namespace) - if updateErr := r.updateReconciliationStatusError(ctx, &oauth2client, hydrav1alpha1.StatusInvalidSecret, conflictErr); updateErr != nil { - return ctrl.Result{}, updateErr - } - return ctrl.Result{}, nil - } + if found { + //conclude reconciliation if the client exists and has not been updated + if oauth2client.Generation == oauth2client.Status.ObservedGeneration { + return ctrl.Result{}, nil + } - if updateErr := r.updateRegisteredOAuth2Client(ctx, &oauth2client, credentials); updateErr != nil { + if fetched.Owner != fmt.Sprintf("%s/%s", oauth2client.Name, oauth2client.Namespace) { + conflictErr := errors.Errorf("ID provided in secret %s/%s is assigned to another resource", secret.Name, secret.Namespace) + if updateErr := r.updateReconciliationStatusError(ctx, &oauth2client, hydrav1alpha1.StatusInvalidSecret, conflictErr); updateErr != nil { return ctrl.Result{}, updateErr } return ctrl.Result{}, nil } - if registerErr := r.registerOAuth2Client(ctx, &oauth2client, credentials); registerErr != nil { - return ctrl.Result{}, registerErr + if updateErr := r.updateRegisteredOAuth2Client(ctx, &oauth2client, credentials); updateErr != nil { + return ctrl.Result{}, updateErr } + return ctrl.Result{}, nil + } + + if registerErr := r.registerOAuth2Client(ctx, &oauth2client, credentials); registerErr != nil { + return ctrl.Result{}, registerErr } return ctrl.Result{}, nil diff --git a/controllers/oauth2client_controller_integration_test.go b/controllers/oauth2client_controller_integration_test.go index 25e57b6..2d35693 100644 --- a/controllers/oauth2client_controller_integration_test.go +++ b/controllers/oauth2client_controller_integration_test.go @@ -60,6 +60,7 @@ var _ = Describe("OAuth2Client Controller", func() { c := mgr.GetClient() mch := &mocks.HydraClientInterface{} + mch.On("GetOAuth2Client", Anything).Return(nil, false, nil) mch.On("DeleteOAuth2Client", Anything).Return(nil) mch.On("ListOAuth2Client", Anything).Return(nil, nil) mch.On("PostOAuth2Client", AnythingOfType("*hydra.OAuth2ClientJSON")).Return(func(o *hydra.OAuth2ClientJSON) *hydra.OAuth2ClientJSON { @@ -139,6 +140,7 @@ var _ = Describe("OAuth2Client Controller", func() { c := mgr.GetClient() mch := &mocks.HydraClientInterface{} + mch.On("GetOAuth2Client", Anything).Return(nil, false, nil) mch.On("PostOAuth2Client", Anything).Return(nil, errors.New("error")) mch.On("DeleteOAuth2Client", Anything).Return(nil) mch.On("ListOAuth2Client", Anything).Return(nil, nil) @@ -205,6 +207,7 @@ var _ = Describe("OAuth2Client Controller", func() { c := mgr.GetClient() mch := mocks.HydraClientInterface{} + mch.On("GetOAuth2Client", Anything).Return(nil, false, nil) mch.On("DeleteOAuth2Client", Anything).Return(nil) mch.On("ListOAuth2Client", Anything).Return(nil, nil) mch.On("GetOAuth2Client", Anything).Return(nil, false, nil) @@ -300,6 +303,7 @@ var _ = Describe("OAuth2Client Controller", func() { c := mgr.GetClient() mch := mocks.HydraClientInterface{} + mch.On("GetOAuth2Client", Anything).Return(nil, false, nil) mch.On("DeleteOAuth2Client", Anything).Return(nil) mch.On("ListOAuth2Client", Anything).Return(nil, nil) diff --git a/main.go b/main.go index 3391755..3d5f822 100644 --- a/main.go +++ b/main.go @@ -21,6 +21,7 @@ import ( "net/http" "net/url" "os" + "time" "github.com/ory/hydra-maester/hydra" @@ -48,9 +49,9 @@ func init() { func main() { var ( - metricsAddr, hydraURL, endpoint, forwardedProto string - hydraPort int - enableLeaderElection bool + metricsAddr, hydraURL, endpoint, forwardedProto, syncPeriod string + hydraPort int + enableLeaderElection bool ) flag.StringVar(&metricsAddr, "metrics-addr", ":8080", "The address the metric endpoint binds to.") @@ -58,16 +59,24 @@ func main() { flag.IntVar(&hydraPort, "hydra-port", 4445, "Port ORY Hydra is listening on") flag.StringVar(&endpoint, "endpoint", "/clients", "ORY Hydra's client endpoint") flag.StringVar(&forwardedProto, "forwarded-proto", "", "If set, this adds the value as the X-Forwarded-Proto header in requests to the ORY Hydra admin server") + flag.StringVar(&syncPeriod, "sync-period", "10h", "Determines the minimum frequency at which watched resources are reconciled") flag.BoolVar(&enableLeaderElection, "enable-leader-election", false, "Enable leader election for controller manager. Enabling this will ensure there is only one active controller manager.") flag.Parse() ctrl.SetLogger(zap.Logger(true)) + syncPeriodParsed, err := time.ParseDuration(syncPeriod) + if err != nil { + setupLog.Error(err, "unable to start manager") + os.Exit(1) + } + mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ Scheme: scheme, MetricsBindAddress: metricsAddr, LeaderElection: enableLeaderElection, + SyncPeriod: &syncPeriodParsed, }) if err != nil { setupLog.Error(err, "unable to start manager") @@ -118,7 +127,6 @@ func getHydraClientMaker(defaultSpec hydrav1alpha1.OAuth2ClientSpec) controllers return controllers.HydraClientMakerFunc(func(spec hydrav1alpha1.OAuth2ClientSpec) (controllers.HydraClientInterface, error) { - if spec.HydraAdmin.URL == "" { spec.HydraAdmin.URL = defaultSpec.HydraAdmin.URL }