diff --git a/controllers/secretgenerator/secretgenerator_controller.go b/controllers/secretgenerator/secretgenerator_controller.go index 49790806c3f..0a41883d46a 100644 --- a/controllers/secretgenerator/secretgenerator_controller.go +++ b/controllers/secretgenerator/secretgenerator_controller.go @@ -57,16 +57,15 @@ type SecretGeneratorReconciler struct { // SetupWithManager sets up the controller with the Manager. func (r *SecretGeneratorReconciler) SetupWithManager(mgr ctrl.Manager) error { - r.Log.Info("Adding controller for Secret Generation.") + log := r.Log + log.Info("Adding controller for Secret Generation.") // Watch only new secrets with the corresponding annotation + // seems we do get multiple events triggered for the same secret creation and even deletion predicates := predicate.Funcs{ CreateFunc: func(e event.CreateEvent) bool { - if _, found := e.Object.GetAnnotations()[annotation.SecretNameAnnotation]; found { - return true - } - - return false + _, found := e.Object.GetAnnotations()[annotation.SecretNameAnnotation] + return found }, GenericFunc: func(e event.GenericEvent) bool { return false @@ -74,11 +73,8 @@ func (r *SecretGeneratorReconciler) SetupWithManager(mgr ctrl.Manager) error { // this only watch for secret deletion if has with annotation // e.g. dashboard-oauth-client but not dashboard-oauth-client-generated DeleteFunc: func(e event.DeleteEvent) bool { - if _, found := e.Object.GetAnnotations()[annotation.SecretNameAnnotation]; found { - return true - } - - return false + _, found := e.Object.GetAnnotations()[annotation.SecretNameAnnotation] + return found }, UpdateFunc: func(e event.UpdateEvent) bool { return false @@ -105,17 +101,25 @@ func (r *SecretGeneratorReconciler) SetupWithManager(mgr ctrl.Manager) error { // based on the specified type and complexity. This will avoid possible race // conditions when a deployment mounts the secret before it is reconciled. func (r *SecretGeneratorReconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) { + log := r.Log foundSecret := &corev1.Secret{} err := r.Client.Get(ctx, request.NamespacedName, foundSecret) + + // deletion case if err != nil { - if k8serr.IsNotFound(err) { - // If Secret is deleted, delete OAuthClient if exists - err = r.deleteOAuthClient(ctx, request.Name) + if k8serr.IsNotFound(err) || foundSecret.GetDeletionTimestamp() != nil { + log.Info("Reconciling Secret on deletion.", "Request.Name", request.Name) + // delete OAuthClient if exists + if err = r.deleteOAuthClient(ctx, request.Name); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil } - return ctrl.Result{}, err } + log.Info("Reconciling Secret on creation.", "Request.Name", request.Name) + // creation case owner := []metav1.OwnerReference{ *metav1.NewControllerRef(foundSecret, foundSecret.GroupVersionKind()), } @@ -129,47 +133,40 @@ func (r *SecretGeneratorReconciler) Reconcile(ctx context.Context, request ctrl. }, } - generatedSecretKey := types.NamespacedName{ - Name: generatedSecret.Name, Namespace: generatedSecret.Namespace, - } - err = r.Client.Get(ctx, generatedSecretKey, generatedSecret) + err = r.Client.Get(ctx, client.ObjectKey{Name: generatedSecret.Name, Namespace: generatedSecret.Namespace}, generatedSecret) if err != nil { if k8serr.IsNotFound(err) { - // Generate secret random value - r.Log.Info("Generating a random value for a secret in a namespace", - "secret", generatedSecret.Name, "namespace", generatedSecret.Namespace) - + // create a secret instance with values secret, err := NewSecretFrom(foundSecret.GetAnnotations()) if err != nil { - r.Log.Error(err, "error creating secret %s in %s", generatedSecret.Name, generatedSecret.Namespace) + log.Error(err, "error setting secret values for %s "+generatedSecret.Name) return ctrl.Result{}, err } - generatedSecret.StringData = map[string]string{ secret.Name: secret.Value, } err = r.Client.Create(ctx, generatedSecret) if err != nil { + log.Error(err, "error generating secret %s in %s", generatedSecret.Name, generatedSecret.Namespace) return ctrl.Result{}, err } - r.Log.Info("Done generating secret in namespace", - "secret", generatedSecret.Name, "namespace", generatedSecret.Namespace) + log.Info("Done generating secret", "secret", generatedSecret.Name, "namespace", generatedSecret.Namespace) + // check if annotation oauth-client-route exists + // this is for dashboard-oauth-client secret, not dashboard-oauth-config if secret.OAuthClientRoute != "" { // Get OauthClient Route oauthClientRoute, err := r.getRoute(ctx, secret.OAuthClientRoute, request.Namespace) if err != nil { - r.Log.Error(err, "Unable to retrieve route from OAuthClient", "route-name", secret.OAuthClientRoute) + log.Error(err, "Unable to retrieve route for OAuthClient", "route-name", secret.OAuthClientRoute) return ctrl.Result{}, err } // Generate OAuthClient for the generated secret - r.Log.Info("Generating an OAuthClient CR for route", "route-name", oauthClientRoute.Name) + log.Info("Generating an OAuthClient CR for route", "route-name", oauthClientRoute.Name) err = r.createOAuthClient(ctx, foundSecret.Name, secret.Value, oauthClientRoute.Spec.Host) if err != nil { - r.Log.Error(err, "error creating oauth client resource. Recreate the Secret", "secret-name", - foundSecret.Name) - + log.Error(err, "error creating AuthClient CR. Recreate the Secret", "secret-name", foundSecret.Name) return ctrl.Result{}, err } } @@ -178,8 +175,7 @@ func (r *SecretGeneratorReconciler) Reconcile(ctx context.Context, request ctrl. } } - // Don't requeue if secret is created successfully - return ctrl.Result{}, err + return ctrl.Result{}, nil } // getRoute returns an OpenShift route object. It waits until the .spec.host value exists to avoid possible race conditions, fails otherwise. @@ -207,6 +203,7 @@ func (r *SecretGeneratorReconciler) getRoute(ctx context.Context, name string, n } func (r *SecretGeneratorReconciler) createOAuthClient(ctx context.Context, name string, secretName string, uri string) error { + log := r.Log // Create OAuthClient resource oauthClient := &oauthv1.OAuthClient{ TypeMeta: metav1.TypeMeta{ @@ -224,7 +221,7 @@ func (r *SecretGeneratorReconciler) createOAuthClient(ctx context.Context, name err := r.Client.Create(ctx, oauthClient) if err != nil { if k8serr.IsAlreadyExists(err) { - r.Log.Info("OAuth client resource already exists, patch it", "name", oauthClient.Name) + log.Info("OAuth client resource already exists, patch it", "name", oauthClient.Name) data, err := json.Marshal(oauthClient) if err != nil { return fmt.Errorf("failed to get DataScienceCluster custom resource data: %w", err)