From 70121a3ffc8d73ad3fe8e0d78d6c37288b4a4f2d Mon Sep 17 00:00:00 2001 From: chiragkyal Date: Wed, 24 Jul 2024 23:27:53 +0530 Subject: [PATCH] update unit tests Signed-off-by: chiragkyal --- .../controller/route_secret_manager_test.go | 610 +++++++++++++++--- 1 file changed, 515 insertions(+), 95 deletions(-) diff --git a/pkg/router/controller/route_secret_manager_test.go b/pkg/router/controller/route_secret_manager_test.go index bd0b99693..018a72715 100644 --- a/pkg/router/controller/route_secret_manager_test.go +++ b/pkg/router/controller/route_secret_manager_test.go @@ -103,6 +103,26 @@ func (p *fakePluginDone) Commit() error { var _ router.Plugin = &fakePluginDone{} +type statusRecorder struct { + rejections []string + unservableInFutureVersions map[string]string +} + +func (r *statusRecorder) rejectionKey(route *routev1.Route) string { + return route.Namespace + "-" + route.Name +} +func (r *statusRecorder) RecordRouteRejection(route *routev1.Route, reason, message string) { + r.rejections = append(r.rejections, fmt.Sprintf("%s:%s", r.rejectionKey(route), reason)) +} +func (r *statusRecorder) RecordRouteUnservableInFutureVersionsClear(route *routev1.Route) { + delete(r.unservableInFutureVersions, r.rejectionKey(route)) +} +func (r *statusRecorder) RecordRouteUnservableInFutureVersions(route *routev1.Route, reason, message string) { + r.unservableInFutureVersions[r.rejectionKey(route)] = reason +} + +var _ RouteStatusRecorder = &statusRecorder{} + func TestRouteSecretManager(t *testing.T) { scenarios := []struct { @@ -113,7 +133,7 @@ func TestRouteSecretManager(t *testing.T) { allow bool expectedRoute *routev1.Route expectedEventType watch.EventType - expectedRejections map[string]string + expectedRejections []string expectedError bool }{ // scenarios when route is added @@ -151,8 +171,8 @@ func TestRouteSecretManager(t *testing.T) { }, }, expectedEventType: watch.Deleted, - expectedRejections: map[string]string{ - "sandbox-route-test": "ExternalCertificateValidationFailed", + expectedRejections: []string{ + "sandbox-route-test:ExternalCertificateValidationFailed", }, expectedError: true, }, @@ -190,8 +210,8 @@ func TestRouteSecretManager(t *testing.T) { }, }, expectedEventType: watch.Deleted, - expectedRejections: map[string]string{ - "sandbox-route-test": "ExternalCertificateValidationFailed", + expectedRejections: []string{ + "sandbox-route-test:ExternalCertificateValidationFailed", }, expectedError: true, }, @@ -229,8 +249,8 @@ func TestRouteSecretManager(t *testing.T) { }, }, expectedEventType: watch.Deleted, - expectedRejections: map[string]string{ - "sandbox-route-test": "ExternalCertificateValidationFailed", + expectedRejections: []string{ + "sandbox-route-test:ExternalCertificateValidationFailed", }, expectedError: true, }, @@ -341,7 +361,7 @@ func TestRouteSecretManager(t *testing.T) { "tls.crt": []byte("my-crt"), "tls.key": []byte("my-key"), }), - IsRegistered: false, + IsPresent: false, }, allow: false, eventType: watch.Modified, @@ -359,8 +379,8 @@ func TestRouteSecretManager(t *testing.T) { }, }, expectedEventType: watch.Deleted, - expectedRejections: map[string]string{ - "sandbox-route-test": "ExternalCertificateValidationFailed", + expectedRejections: []string{ + "sandbox-route-test:ExternalCertificateValidationFailed", }, expectedError: true, }, @@ -384,7 +404,7 @@ func TestRouteSecretManager(t *testing.T) { "tls.crt": []byte("my-crt"), "tls.key": []byte("my-key"), }), - IsRegistered: false, + IsPresent: false, }, allow: true, eventType: watch.Modified, @@ -402,8 +422,8 @@ func TestRouteSecretManager(t *testing.T) { }, }, expectedEventType: watch.Deleted, - expectedRejections: map[string]string{ - "sandbox-route-test": "ExternalCertificateValidationFailed", + expectedRejections: []string{ + "sandbox-route-test:ExternalCertificateValidationFailed", }, expectedError: true, }, @@ -427,7 +447,7 @@ func TestRouteSecretManager(t *testing.T) { "tls.crt": []byte("my-crt"), "tls.key": []byte("my-key"), }), - IsRegistered: false, + IsPresent: false, }, allow: true, eventType: watch.Modified, @@ -445,8 +465,8 @@ func TestRouteSecretManager(t *testing.T) { }, }, expectedEventType: watch.Deleted, - expectedRejections: map[string]string{ - "sandbox-route-test": "ExternalCertificateValidationFailed", + expectedRejections: []string{ + "sandbox-route-test:ExternalCertificateValidationFailed", }, expectedError: true, }, @@ -470,8 +490,8 @@ func TestRouteSecretManager(t *testing.T) { "tls.crt": []byte("my-crt"), "tls.key": []byte("my-key"), }), - IsRegistered: false, - Err: fmt.Errorf("something"), + IsPresent: false, + Err: fmt.Errorf("something"), }, allow: true, eventType: watch.Modified, @@ -497,7 +517,7 @@ func TestRouteSecretManager(t *testing.T) { "tls.crt": []byte("my-crt"), "tls.key": []byte("my-key"), }), - IsRegistered: false, + IsPresent: false, }, allow: true, eventType: watch.Modified, @@ -519,9 +539,9 @@ func TestRouteSecretManager(t *testing.T) { expectedEventType: watch.Modified, }, - // scenarios when route is updated (old route with externalCertificate, new route with externalCertificate) + // scenarios when route is updated (old route with externalCertificate, new route with same externalCertificate) { - name: "route updated: old route with externalCertificate, new route with externalCertificate denied", + name: "route updated: old route with externalCertificate, new route with same externalCertificate denied", route: &routev1.Route{ ObjectMeta: metav1.ObjectMeta{ Name: "route-test", @@ -540,7 +560,8 @@ func TestRouteSecretManager(t *testing.T) { "tls.crt": []byte("my-crt"), "tls.key": []byte("my-key"), }), - IsRegistered: true, + IsPresent: true, + SecretName: "tls-secret", }, allow: false, eventType: watch.Modified, @@ -558,13 +579,13 @@ func TestRouteSecretManager(t *testing.T) { }, }, expectedEventType: watch.Deleted, - expectedRejections: map[string]string{ - "sandbox-route-test": "ExternalCertificateValidationFailed", + expectedRejections: []string{ + "sandbox-route-test:ExternalCertificateValidationFailed", }, expectedError: true, }, { - name: "route updated: old route with externalCertificate, new route with externalCertificate allowed but secret not found", + name: "route updated: old route with externalCertificate, new route with same externalCertificate allowed but secret not found", route: &routev1.Route{ ObjectMeta: metav1.ObjectMeta{ Name: "route-test", @@ -583,7 +604,8 @@ func TestRouteSecretManager(t *testing.T) { "tls.crt": []byte("my-crt"), "tls.key": []byte("my-key"), }), - IsRegistered: true, + IsPresent: true, + SecretName: "tls-secret", }, allow: true, eventType: watch.Modified, @@ -601,13 +623,13 @@ func TestRouteSecretManager(t *testing.T) { }, }, expectedEventType: watch.Deleted, - expectedRejections: map[string]string{ - "sandbox-route-test": "ExternalCertificateValidationFailed", + expectedRejections: []string{ + "sandbox-route-test:ExternalCertificateValidationFailed", }, expectedError: true, }, { - name: "route updated: old route with externalCertificate, new route with externalCertificate allowed but secret of incorrect type", + name: "route updated: old route with externalCertificate, new route with same externalCertificate allowed but secret of incorrect type", route: &routev1.Route{ ObjectMeta: metav1.ObjectMeta{ Name: "route-test", @@ -626,7 +648,8 @@ func TestRouteSecretManager(t *testing.T) { "tls.crt": []byte("my-crt"), "tls.key": []byte("my-key"), }), - IsRegistered: true, + IsPresent: true, + SecretName: "tls-secret", }, allow: true, eventType: watch.Modified, @@ -644,13 +667,13 @@ func TestRouteSecretManager(t *testing.T) { }, }, expectedEventType: watch.Deleted, - expectedRejections: map[string]string{ - "sandbox-route-test": "ExternalCertificateValidationFailed", + expectedRejections: []string{ + "sandbox-route-test:ExternalCertificateValidationFailed", }, expectedError: true, }, { - name: "route updated: old route with externalCertificate, new route with externalCertificate allowed and correct secret but got error from secretManager", + name: "route updated: old route with externalCertificate, new route with same externalCertificate allowed and correct secret but got error from secretManager", route: &routev1.Route{ ObjectMeta: metav1.ObjectMeta{ Name: "route-test", @@ -669,15 +692,33 @@ func TestRouteSecretManager(t *testing.T) { "tls.crt": []byte("my-crt"), "tls.key": []byte("my-key"), }), - IsRegistered: true, - Err: fmt.Errorf("something"), + IsPresent: true, + SecretName: "tls-secret", + Err: fmt.Errorf("something"), + }, + allow: true, + eventType: watch.Modified, + expectedRoute: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "route-test", + Namespace: "sandbox", + }, + Spec: routev1.RouteSpec{ + TLS: &routev1.TLSConfig{ + ExternalCertificate: &routev1.LocalObjectReference{ + Name: "tls-secret", + }, + }, + }, + }, + expectedEventType: watch.Deleted, + expectedRejections: []string{ + "sandbox-route-test:ExternalCertificateGetFailed", }, - allow: true, - eventType: watch.Modified, expectedError: true, }, { - name: "route updated: old route with externalCertificate, new route with externalCertificate allowed and correct secret", + name: "route updated: old route with externalCertificate, new route with same externalCertificate allowed and correct secret", route: &routev1.Route{ ObjectMeta: metav1.ObjectMeta{ Name: "route-test", @@ -696,7 +737,8 @@ func TestRouteSecretManager(t *testing.T) { "tls.crt": []byte("my-crt"), "tls.key": []byte("my-key"), }), - IsRegistered: true, + IsPresent: true, + SecretName: "tls-secret", }, allow: true, eventType: watch.Modified, @@ -718,6 +760,210 @@ func TestRouteSecretManager(t *testing.T) { expectedEventType: watch.Modified, }, + // scenarios when route is updated (old route with externalCertificate, new route with different externalCertificate) + { + name: "route updated: old route with externalCertificate, new route with different externalCertificate denied", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "route-test", + Namespace: "sandbox", + }, + Spec: routev1.RouteSpec{ + TLS: &routev1.TLSConfig{ + ExternalCertificate: &routev1.LocalObjectReference{ + Name: "different-tls-secret", + }, + }, + }, + }, + secretManager: fake.SecretManager{ + Secret: fakeSecret("sandbox", "different-tls-secret", corev1.SecretTypeTLS, map[string][]byte{ + "tls.crt": []byte("my-crt"), + "tls.key": []byte("my-key"), + }), + IsPresent: true, + SecretName: "tls-secret", // Used by LookupRouteSecret() to get the old secretName + }, + allow: false, + eventType: watch.Modified, + expectedRoute: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "route-test", + Namespace: "sandbox", + }, + Spec: routev1.RouteSpec{ + TLS: &routev1.TLSConfig{ + ExternalCertificate: &routev1.LocalObjectReference{ + Name: "different-tls-secret", + }, + }, + }, + }, + expectedEventType: watch.Deleted, + expectedRejections: []string{ + "sandbox-route-test:ExternalCertificateValidationFailed", + }, + expectedError: true, + }, + { + name: "route updated: old route with externalCertificate, new route with different externalCertificate allowed but secret not found", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "route-test", + Namespace: "sandbox", + }, + Spec: routev1.RouteSpec{ + TLS: &routev1.TLSConfig{ + ExternalCertificate: &routev1.LocalObjectReference{ + Name: "different-tls-secret", + }, + }, + }, + }, + secretManager: fake.SecretManager{ + Secret: fakeSecret("other-sandbox", "different-tls-secret", corev1.SecretTypeTLS, map[string][]byte{ + "tls.crt": []byte("my-crt"), + "tls.key": []byte("my-key"), + }), + IsPresent: true, + SecretName: "tls-secret", // Used by LookupRouteSecret() to get the old secretName + }, + allow: true, + eventType: watch.Modified, + expectedRoute: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "route-test", + Namespace: "sandbox", + }, + Spec: routev1.RouteSpec{ + TLS: &routev1.TLSConfig{ + ExternalCertificate: &routev1.LocalObjectReference{ + Name: "different-tls-secret", + }, + }, + }, + }, + expectedEventType: watch.Deleted, + expectedRejections: []string{ + "sandbox-route-test:ExternalCertificateValidationFailed", + }, + expectedError: true, + }, + { + name: "route updated: old route with externalCertificate, new route with different externalCertificate allowed but secret of incorrect type", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "route-test", + Namespace: "sandbox", + }, + Spec: routev1.RouteSpec{ + TLS: &routev1.TLSConfig{ + ExternalCertificate: &routev1.LocalObjectReference{ + Name: "different-tls-secret", + }, + }, + }, + }, + secretManager: fake.SecretManager{ + Secret: fakeSecret("sandbox", "different-tls-secret", corev1.SecretTypeBasicAuth, map[string][]byte{ + "tls.crt": []byte("my-crt"), + "tls.key": []byte("my-key"), + }), + IsPresent: true, + SecretName: "tls-secret", // Used by LookupRouteSecret() to get the old secretName + }, + allow: true, + eventType: watch.Modified, + expectedRoute: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "route-test", + Namespace: "sandbox", + }, + Spec: routev1.RouteSpec{ + TLS: &routev1.TLSConfig{ + ExternalCertificate: &routev1.LocalObjectReference{ + Name: "different-tls-secret", + }, + }, + }, + }, + expectedEventType: watch.Deleted, + expectedRejections: []string{ + "sandbox-route-test:ExternalCertificateValidationFailed", + }, + expectedError: true, + }, + { + name: "route updated: old route with externalCertificate, new route with different externalCertificate allowed and correct secret but got error from secretManager", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "route-test", + Namespace: "sandbox", + }, + Spec: routev1.RouteSpec{ + TLS: &routev1.TLSConfig{ + ExternalCertificate: &routev1.LocalObjectReference{ + Name: "different-tls-secret", + }, + }, + }, + }, + secretManager: fake.SecretManager{ + Secret: fakeSecret("sandbox", "different-tls-secret", corev1.SecretTypeTLS, map[string][]byte{ + "tls.crt": []byte("my-crt"), + "tls.key": []byte("my-key"), + }), + IsPresent: true, + SecretName: "tls-secret", // Used by LookupRouteSecret() to get the old secretName + Err: fmt.Errorf("something"), + }, + allow: true, + eventType: watch.Modified, + expectedError: true, + }, + { + name: "route updated: old route with externalCertificate, new route with different externalCertificate allowed and correct secret", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "route-test", + Namespace: "sandbox", + }, + Spec: routev1.RouteSpec{ + TLS: &routev1.TLSConfig{ + ExternalCertificate: &routev1.LocalObjectReference{ + Name: "different-tls-secret", + }, + }, + }, + }, + secretManager: fake.SecretManager{ + Secret: fakeSecret("sandbox", "different-tls-secret", corev1.SecretTypeTLS, map[string][]byte{ + "tls.crt": []byte("my-crt"), + "tls.key": []byte("my-key"), + }), + IsPresent: true, + SecretName: "tls-secret", // Used by LookupRouteSecret() to get the old secretName + }, + allow: true, + eventType: watch.Modified, + expectedRoute: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "route-test", + Namespace: "sandbox", + }, + Spec: routev1.RouteSpec{ + TLS: &routev1.TLSConfig{ + ExternalCertificate: &routev1.LocalObjectReference{ + Name: "different-tls-secret", + }, + Certificate: "my-crt", + Key: "my-key", + }, + }, + }, + expectedEventType: watch.Modified, + }, + // scenarios when route is updated (old route with externalCertificate, new route without externalCertificate) { name: "route updated: old route with externalCertificate, new route without externalCertificate but got error from secretManager", @@ -729,8 +975,9 @@ func TestRouteSecretManager(t *testing.T) { Spec: routev1.RouteSpec{}, }, secretManager: fake.SecretManager{ - IsRegistered: true, - Err: fmt.Errorf("something"), + IsPresent: true, + SecretName: "tls-secret", + Err: fmt.Errorf("something"), }, eventType: watch.Modified, expectedError: true, @@ -745,7 +992,8 @@ func TestRouteSecretManager(t *testing.T) { Spec: routev1.RouteSpec{}, }, secretManager: fake.SecretManager{ - IsRegistered: true, + IsPresent: true, + SecretName: "tls-secret", }, eventType: watch.Modified, expectedRoute: &routev1.Route{ @@ -769,7 +1017,7 @@ func TestRouteSecretManager(t *testing.T) { Spec: routev1.RouteSpec{}, }, secretManager: fake.SecretManager{ - IsRegistered: false, + IsPresent: false, }, eventType: watch.Modified, expectedRoute: &routev1.Route{ @@ -792,7 +1040,7 @@ func TestRouteSecretManager(t *testing.T) { }, Spec: routev1.RouteSpec{}, }, - secretManager: fake.SecretManager{IsRegistered: false}, + secretManager: fake.SecretManager{IsPresent: false}, eventType: watch.Deleted, expectedRoute: &routev1.Route{ ObjectMeta: metav1.ObjectMeta{ @@ -818,8 +1066,11 @@ func TestRouteSecretManager(t *testing.T) { }, }, }, - secretManager: fake.SecretManager{IsRegistered: true}, - eventType: watch.Deleted, + secretManager: fake.SecretManager{ + IsPresent: true, + SecretName: "tls-secret", + }, + eventType: watch.Deleted, expectedRoute: &routev1.Route{ ObjectMeta: metav1.ObjectMeta{ Name: "route-test", @@ -850,7 +1101,11 @@ func TestRouteSecretManager(t *testing.T) { }, }, }, - secretManager: fake.SecretManager{IsRegistered: true, Err: fmt.Errorf("something")}, + secretManager: fake.SecretManager{ + IsPresent: true, + SecretName: "tls-secret", + Err: fmt.Errorf("something"), + }, eventType: watch.Deleted, expectedError: true, }, @@ -859,12 +1114,7 @@ func TestRouteSecretManager(t *testing.T) { for _, s := range scenarios { t.Run(s.name, func(t *testing.T) { p := &fakePlugin{} - recorder := routeStatusRecorder{rejections: make(map[string]string)} - - // assign default value to expectedRejections - if s.expectedRejections == nil { - s.expectedRejections = map[string]string{} - } + recorder := &statusRecorder{} rsm := NewRouteSecretManager(p, recorder, &s.secretManager, &testSecretGetter{namespace: s.route.Namespace, secret: s.secretManager.Secret}, &testSARCreator{allow: s.allow}) gotErr := rsm.HandleRoute(s.eventType, s.route) @@ -880,6 +1130,9 @@ func TestRouteSecretManager(t *testing.T) { if !reflect.DeepEqual(s.expectedRejections, recorder.rejections) { t.Fatalf("expected rejections %v, but got %v", s.expectedRejections, recorder.rejections) } + if _, exists := rsm.deletedSecrets.Load(generateKey(s.route)); exists { + t.Fatalf("deletedSecrets must be cleaned") + } }) } } @@ -887,14 +1140,15 @@ func TestRouteSecretManager(t *testing.T) { func TestSecretUpdateAndDelete(t *testing.T) { scenarios := []struct { - name string - route *routev1.Route - secretManager fake.SecretManager - allow bool - deleteSecret bool - expectedRoute *routev1.Route - expectedEventType watch.EventType - expectedRejections map[string]string + name string + route *routev1.Route + secretManager fake.SecretManager + allow bool + deleteSecret bool + expectedRoute *routev1.Route + expectedEventType watch.EventType + expectedRejections []string + expectedDeletedSecrets any }{ { name: "secret updated but permission revoked", @@ -916,6 +1170,8 @@ func TestSecretUpdateAndDelete(t *testing.T) { "tls.crt": []byte("my-crt"), "tls.key": []byte("my-key"), }), + IsPresent: true, + SecretName: "tls-secret", }, allow: false, expectedRoute: &routev1.Route{ @@ -932,8 +1188,8 @@ func TestSecretUpdateAndDelete(t *testing.T) { }, }, expectedEventType: watch.Deleted, - expectedRejections: map[string]string{ - "sandbox-route-test": "ExternalCertificateValidationFailed", + expectedRejections: []string{ + "sandbox-route-test:ExternalCertificateValidationFailed", }, }, { @@ -956,7 +1212,9 @@ func TestSecretUpdateAndDelete(t *testing.T) { "tls.crt": []byte("my-crt"), "tls.key": []byte("my-key"), }), - Err: fmt.Errorf("something"), + IsPresent: true, + SecretName: "tls-secret", + Err: fmt.Errorf("something"), }, allow: true, expectedRoute: &routev1.Route{ @@ -973,8 +1231,8 @@ func TestSecretUpdateAndDelete(t *testing.T) { }, }, expectedEventType: watch.Deleted, - expectedRejections: map[string]string{ - "sandbox-route-test": "ExternalCertificateGetFailed", + expectedRejections: []string{ + "sandbox-route-test:ExternalCertificateGetFailed", }, }, { @@ -997,6 +1255,8 @@ func TestSecretUpdateAndDelete(t *testing.T) { "tls.crt": []byte("my-crt"), "tls.key": []byte("my-key"), }), + IsPresent: true, + SecretName: "tls-secret", }, allow: true, expectedRoute: &routev1.Route{ @@ -1014,11 +1274,10 @@ func TestSecretUpdateAndDelete(t *testing.T) { }, }, }, - expectedEventType: watch.Modified, - expectedRejections: map[string]string{}, + expectedEventType: watch.Modified, }, { - name: "secret deleted but got error from secretManager", + name: "secret deleted and route successfully stored into deletedSecrets", route: &routev1.Route{ ObjectMeta: metav1.ObjectMeta{ Name: "route-test", @@ -1037,7 +1296,8 @@ func TestSecretUpdateAndDelete(t *testing.T) { "tls.crt": []byte("my-crt"), "tls.key": []byte("my-key"), }), - Err: fmt.Errorf("something"), + IsPresent: true, + SecretName: "tls-secret", }, deleteSecret: true, expectedRoute: &routev1.Route{ @@ -1054,12 +1314,79 @@ func TestSecretUpdateAndDelete(t *testing.T) { }, }, expectedEventType: watch.Deleted, - expectedRejections: map[string]string{ - "sandbox-route-test": "ExternalCertificateSecretDeleted", + expectedRejections: []string{ + "sandbox-route-test:ExternalCertificateSecretDeleted", }, + expectedDeletedSecrets: true, }, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + oldSecret := fakeSecret("sandbox", "tls-secret", corev1.SecretTypeTLS, map[string][]byte{}) + kubeClient := testclient.NewSimpleClientset(oldSecret) + informer := fakeSecretInformer(kubeClient, "sandbox", "tls-secret") + go informer.Run(context.TODO().Done()) + + // wait for informer to start + if !cache.WaitForCacheSync(context.TODO().Done(), informer.HasSynced) { + t.Fatal("cache not synced yet") + } + + p := &fakePluginDone{ + doneCh: make(chan struct{}), + } + recorder := &statusRecorder{} + rsm := NewRouteSecretManager(p, recorder, &s.secretManager, &testSecretGetter{namespace: s.route.Namespace, secret: oldSecret}, &testSARCreator{allow: s.allow}) + + if _, err := informer.AddEventHandler(rsm.generateSecretHandler(s.route)); err != nil { + t.Fatalf("failed to add handler: %v", err) + } + + if s.deleteSecret { + // delete the secret + if err := kubeClient.CoreV1().Secrets(s.route.Namespace).Delete(context.TODO(), s.secretManager.Secret.Name, metav1.DeleteOptions{}); err != nil { + t.Fatalf("failed to delete secret: %v", err) + } + + } else { + // update the secret + if _, err := kubeClient.CoreV1().Secrets(s.route.Namespace).Update(context.TODO(), s.secretManager.Secret, metav1.UpdateOptions{}); err != nil { + t.Fatalf("failed to update secret: %v", err) + } + } + // wait until p.plugin.HandleRoute() completes (required to handle race condition) + <-p.doneCh + + if !reflect.DeepEqual(s.expectedRoute, p.route) { + t.Fatalf("expected route for next plugin %v, but got %v", s.expectedRoute, p.route) + } + if s.expectedEventType != p.eventType { + t.Fatalf("expected %s event for next plugin, but got %s", s.expectedEventType, p.eventType) + } + if !reflect.DeepEqual(s.expectedRejections, recorder.rejections) { + t.Fatalf("expected rejections %v, but got %v", s.expectedRejections, recorder.rejections) + } + + if val, _ := rsm.deletedSecrets.Load(generateKey(s.route)); !reflect.DeepEqual(val, s.expectedDeletedSecrets) { + t.Fatalf("expected deletedSecrets %v, but got %v", s.expectedDeletedSecrets, val) + } + }) + } +} + +func TestSecretRecreation(t *testing.T) { + scenarios := []struct { + name string + route *routev1.Route + secretManager fake.SecretManager + allow bool + expectedRoute *routev1.Route + expectedEventType watch.EventType + expectedRejections []string + }{ { - name: "secret deleted and route successfully unregistered", + name: "secret deleted and recreated with permission revoked", route: &routev1.Route{ ObjectMeta: metav1.ObjectMeta{ Name: "route-test", @@ -1078,8 +1405,10 @@ func TestSecretUpdateAndDelete(t *testing.T) { "tls.crt": []byte("my-crt"), "tls.key": []byte("my-key"), }), + IsPresent: true, + SecretName: "tls-secret", }, - deleteSecret: true, + allow: false, expectedRoute: &routev1.Route{ ObjectMeta: metav1.ObjectMeta{ Name: "route-test", @@ -1094,8 +1423,97 @@ func TestSecretUpdateAndDelete(t *testing.T) { }, }, expectedEventType: watch.Deleted, - expectedRejections: map[string]string{ - "sandbox-route-test": "ExternalCertificateSecretDeleted", + expectedRejections: []string{ + "sandbox-route-test:ExternalCertificateSecretDeleted", + "sandbox-route-test:ExternalCertificateValidationFailed", + }, + }, + { + name: "secret deleted and recreated with permission but got error from secretManager", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "route-test", + Namespace: "sandbox", + }, + Spec: routev1.RouteSpec{ + TLS: &routev1.TLSConfig{ + ExternalCertificate: &routev1.LocalObjectReference{ + Name: "tls-secret", + }, + }, + }, + }, + secretManager: fake.SecretManager{ + Secret: fakeSecret("sandbox", "tls-secret", corev1.SecretTypeTLS, map[string][]byte{ + "tls.crt": []byte("my-crt"), + "tls.key": []byte("my-key"), + }), + IsPresent: true, + SecretName: "tls-secret", + Err: fmt.Errorf("something"), + }, + allow: true, + expectedRoute: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "route-test", + Namespace: "sandbox", + }, + Spec: routev1.RouteSpec{ + TLS: &routev1.TLSConfig{ + ExternalCertificate: &routev1.LocalObjectReference{ + Name: "tls-secret", + }, + }, + }, + }, + expectedEventType: watch.Deleted, + expectedRejections: []string{ + "sandbox-route-test:ExternalCertificateSecretDeleted", + "sandbox-route-test:ExternalCertificateGetFailed", + }, + }, + { + name: "secret deleted and recreated with permission correctly", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "route-test", + Namespace: "sandbox", + }, + Spec: routev1.RouteSpec{ + TLS: &routev1.TLSConfig{ + ExternalCertificate: &routev1.LocalObjectReference{ + Name: "tls-secret", + }, + }, + }, + }, + secretManager: fake.SecretManager{ + Secret: fakeSecret("sandbox", "tls-secret", corev1.SecretTypeTLS, map[string][]byte{ + "tls.crt": []byte("my-crt"), + "tls.key": []byte("my-key"), + }), + IsPresent: true, + SecretName: "tls-secret", + }, + allow: true, + expectedRoute: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "route-test", + Namespace: "sandbox", + }, + Spec: routev1.RouteSpec{ + TLS: &routev1.TLSConfig{ + ExternalCertificate: &routev1.LocalObjectReference{ + Name: "tls-secret", + }, + Certificate: "my-crt", + Key: "my-key", + }, + }, + }, + expectedEventType: watch.Modified, + expectedRejections: []string{ + "sandbox-route-test:ExternalCertificateSecretDeleted", }, }, } @@ -1112,40 +1530,42 @@ func TestSecretUpdateAndDelete(t *testing.T) { t.Fatal("cache not synced yet") } - p := &fakePluginDone{ + nextPlugin := &fakePluginDone{ doneCh: make(chan struct{}), } - recorder := routeStatusRecorder{rejections: make(map[string]string)} - rsm := NewRouteSecretManager(p, recorder, &s.secretManager, &testSecretGetter{namespace: s.route.Namespace, secret: oldSecret}, &testSARCreator{allow: s.allow}) + recorder := &statusRecorder{} + rsm := NewRouteSecretManager(nextPlugin, recorder, &s.secretManager, &testSecretGetter{namespace: s.route.Namespace, secret: oldSecret}, &testSARCreator{allow: s.allow}) if _, err := informer.AddEventHandler(rsm.generateSecretHandler(s.route)); err != nil { t.Fatalf("failed to add handler: %v", err) } - if s.deleteSecret { - // delete the secret - if err := kubeClient.CoreV1().Secrets(s.route.Namespace).Delete(context.TODO(), s.secretManager.Secret.Name, metav1.DeleteOptions{}); err != nil { - t.Fatalf("failed to delete secret: %v", err) - } + // secret deletion + if err := kubeClient.CoreV1().Secrets(s.route.Namespace).Delete(context.TODO(), s.secretManager.Secret.Name, metav1.DeleteOptions{}); err != nil { + t.Fatalf("failed to delete secret: %v", err) + } + <-nextPlugin.doneCh // wait for HandleRoute (deletion) to complete - } else { - // update the secret - if _, err := kubeClient.CoreV1().Secrets(s.route.Namespace).Update(context.TODO(), s.secretManager.Secret, metav1.UpdateOptions{}); err != nil { - t.Fatalf("failed to update secret: %v", err) - } + // recreate the secret + nextPlugin.doneCh = make(chan struct{}) // need a new doneCh for recreation + if _, err := kubeClient.CoreV1().Secrets(s.route.Namespace).Create(context.TODO(), s.secretManager.Secret, metav1.CreateOptions{}); err != nil { + t.Fatalf("failed to create secret: %v", err) } - // wait until p.plugin.HandleRoute() completes (required to handle race condition) - <-p.doneCh - if !reflect.DeepEqual(s.expectedRoute, p.route) { - t.Fatalf("expected route for next plugin %v, but got %v", s.expectedRoute, p.route) + <-nextPlugin.doneCh // wait for HandleRoute (recreation) to complete + + if !reflect.DeepEqual(s.expectedRoute, nextPlugin.route) { + t.Fatalf("expected route for next plugin %v, but got %v", s.expectedRoute, nextPlugin.route) } - if s.expectedEventType != p.eventType { - t.Fatalf("expected %s event for next plugin, but got %s", s.expectedEventType, p.eventType) + if s.expectedEventType != nextPlugin.eventType { + t.Fatalf("expected %s event for next plugin, but got %s", s.expectedEventType, nextPlugin.eventType) } if !reflect.DeepEqual(s.expectedRejections, recorder.rejections) { t.Fatalf("expected rejections %v, but got %v", s.expectedRejections, recorder.rejections) } + if _, exists := rsm.deletedSecrets.Load(generateKey(s.route)); exists { + t.Fatalf("deletedSecrets must be cleaned") + } }) } }