diff --git a/controllers/secretgenerator/secretgenerator_controller.go b/controllers/secretgenerator/secretgenerator_controller.go index 931e45d42f3..409e02e7cc7 100644 --- a/controllers/secretgenerator/secretgenerator_controller.go +++ b/controllers/secretgenerator/secretgenerator_controller.go @@ -107,12 +107,17 @@ func (r *SecretGeneratorReconciler) Reconcile(ctx context.Context, request ctrl. foundSecret := &corev1.Secret{} err := r.Client.Get(ctx, request.NamespacedName, foundSecret) 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) { + return ctrl.Result{}, err } - return ctrl.Result{}, err + // If Secret is deleted, delete OAuthClient if exists + err = r.deleteOAuthClient(ctx, request.NamespacedName) + if err != nil { + return ctrl.Result{}, err + } + + return ctrl.Result{}, nil } // Generate the secret if it does not previously exist @@ -252,17 +257,16 @@ func (r *SecretGeneratorReconciler) createOAuthClient(ctx context.Context, name return err } -func (r *SecretGeneratorReconciler) deleteOAuthClient(ctx context.Context, secretName string) error { - oauthClient := &oauthv1.OAuthClient{} - - err := r.Client.Get(ctx, client.ObjectKey{ - Name: secretName, - }, oauthClient) - if err != nil { - return client.IgnoreNotFound(err) +func (r *SecretGeneratorReconciler) deleteOAuthClient(ctx context.Context, secretNamespacedName types.NamespacedName) error { + oauthClient := &oauthv1.OAuthClient{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretNamespacedName.Name, + Namespace: secretNamespacedName.Namespace, + }, } - if err = r.Client.Delete(ctx, oauthClient); err != nil { + err := r.Client.Delete(ctx, oauthClient) + if err != nil && !k8serr.IsNotFound(err) { return fmt.Errorf("error deleting OAuthClient %s: %w", oauthClient.Name, err) } diff --git a/controllers/secretgenerator/secretgenerator_controller_test.go b/controllers/secretgenerator/secretgenerator_controller_test.go index a13bd8d1828..5ba5bf0a2ea 100644 --- a/controllers/secretgenerator/secretgenerator_controller_test.go +++ b/controllers/secretgenerator/secretgenerator_controller_test.go @@ -5,8 +5,10 @@ import ( "testing" "github.com/onsi/gomega/gstruct" + oauthv1 "github.com/openshift/api/oauth/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + k8serr "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -26,6 +28,7 @@ func newFakeClient(objs ...client.Object) client.Client { scheme := runtime.NewScheme() utilruntime.Must(corev1.AddToScheme(scheme)) utilruntime.Must(appsv1.AddToScheme(scheme)) + utilruntime.Must(oauthv1.AddToScheme(scheme)) return fake.NewClientBuilder(). WithScheme(scheme). @@ -152,3 +155,94 @@ func TestExistingSecret(t *testing.T) { g.Expect(generatedSecret.Labels).To(BeEmpty()) g.Expect(generatedSecret.StringData).To(BeEmpty()) } + +func TestSecretNotFound(t *testing.T) { + g := NewWithT(t) + ctx := context.Background() + + secretName := "foo" + secretNs := "ns" + + cli := newFakeClient() + + r := secretgenerator.SecretGeneratorReconciler{ + Client: cli, + } + + _, err := r.Reconcile(ctx, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: secretName, + Namespace: secretNs, + }, + }) + // secret not found, reconcile should end without error + g.Expect(err).ShouldNot(HaveOccurred()) +} + +func TestDeleteOAuthClientIfSecretNotFound(t *testing.T) { + g := NewWithT(t) + ctx := context.Background() + + secretName := "foo" + secretNs := "ns" + + // secret expected to be deleted + existingSecret := corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: secretNs, + Labels: map[string]string{ + "foo": "bar", + }, + Annotations: map[string]string{ + annotations.SecretNameAnnotation: "bar", + annotations.SecretTypeAnnotation: "random", + }, + }, + } + + // future left-over oauth client to be cleaned up during reconcile + existingOauthClient := oauthv1.OAuthClient{ + TypeMeta: metav1.TypeMeta{ + Kind: "OAuthClient", + APIVersion: "oauth.openshift.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: secretNs, + }, + Secret: secretName, + RedirectURIs: []string{"https://foo.bar"}, + GrantMethod: oauthv1.GrantHandlerAuto, + } + + cli := newFakeClient(&existingSecret, &existingOauthClient) + + r := secretgenerator.SecretGeneratorReconciler{ + Client: cli, + } + + // delete secret + err := cli.Delete(ctx, &existingSecret) + g.Expect(err).ShouldNot(HaveOccurred()) + + // ensure the secret is deleted + err = cli.Get(ctx, client.ObjectKeyFromObject(&existingSecret), &existingSecret) + g.Expect(err).Should(HaveOccurred()) + g.Expect(k8serr.IsNotFound(err)).Should(BeTrue()) + + _, err = r.Reconcile(ctx, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: secretName, + Namespace: secretNs, + }, + }) + // secret not found, reconcile should clean-up left-over oauth client and end without error + g.Expect(err).ShouldNot(HaveOccurred()) + + // ensure the leftover OauthClient was deleted during reconcile + foundOauthClient := oauthv1.OAuthClient{} + err = cli.Get(ctx, client.ObjectKeyFromObject(&existingOauthClient), &foundOauthClient) + g.Expect(err).Should(HaveOccurred()) + g.Expect(k8serr.IsNotFound(err)).Should(BeTrue()) +}