Skip to content

Commit 5358689

Browse files
mfojtikmfojtik
committed
recreate generated service cert secret when deleted
1 parent 4ec486d commit 5358689

File tree

2 files changed

+206
-10
lines changed

2 files changed

+206
-10
lines changed

‎pkg/service/controller/servingcert/secret_creating_controller.go

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,11 @@ type ServiceServingCertController struct {
5858

5959
serviceCache cache.Store
6060
serviceController *cache.Controller
61+
serviceHasSynced informerSynced
62+
63+
secretCache cache.Store
64+
secretController *cache.Controller
65+
secretHasSynced informerSynced
6166

6267
ca *crypto.CA
6368
publicCert string
@@ -106,6 +111,24 @@ func NewServiceServingCertController(serviceClient kcoreclient.ServicesGetter, s
106111
},
107112
},
108113
)
114+
sc.serviceHasSynced = sc.serviceController.HasSynced
115+
116+
sc.secretCache, sc.secretController = cache.NewInformer(
117+
&cache.ListWatch{
118+
ListFunc: func(options kapi.ListOptions) (runtime.Object, error) {
119+
return sc.secretClient.Secrets(kapi.NamespaceAll).List(options)
120+
},
121+
WatchFunc: func(options kapi.ListOptions) (watch.Interface, error) {
122+
return sc.secretClient.Secrets(kapi.NamespaceAll).Watch(options)
123+
},
124+
},
125+
&kapi.Secret{},
126+
resyncInterval,
127+
cache.ResourceEventHandlerFuncs{
128+
DeleteFunc: sc.deleteSecret,
129+
},
130+
)
131+
sc.secretHasSynced = sc.secretController.HasSynced
109132

110133
sc.syncHandler = sc.syncService
111134

@@ -116,6 +139,11 @@ func NewServiceServingCertController(serviceClient kcoreclient.ServicesGetter, s
116139
func (sc *ServiceServingCertController) Run(workers int, stopCh <-chan struct{}) {
117140
defer utilruntime.HandleCrash()
118141
go sc.serviceController.Run(stopCh)
142+
go sc.secretController.Run(stopCh)
143+
if !waitForCacheSync(stopCh, sc.serviceHasSynced, sc.secretHasSynced) {
144+
return
145+
}
146+
119147
for i := 0; i < workers; i++ {
120148
go wait.Until(sc.worker, time.Second, stopCh)
121149
}
@@ -125,7 +153,34 @@ func (sc *ServiceServingCertController) Run(workers int, stopCh <-chan struct{})
125153
sc.queue.ShutDown()
126154
}
127155

156+
// deleteSecret handles the case when the service certificate secret is manually removed.
157+
// In that case the secret will be automatically recreated.
158+
func (sc *ServiceServingCertController) deleteSecret(obj interface{}) {
159+
secret, ok := obj.(*kapi.Secret)
160+
if !ok {
161+
return
162+
}
163+
if _, exists := secret.Annotations[ServiceNameAnnotation]; !exists {
164+
return
165+
}
166+
serviceObj, exists, err := sc.serviceCache.GetByKey(secret.Namespace + "/" + secret.Annotations[ServiceNameAnnotation])
167+
if !exists {
168+
return
169+
}
170+
if err != nil {
171+
return
172+
}
173+
service := serviceObj.(*kapi.Service)
174+
glog.V(4).Infof("Recreating secret for service %q", service.Namespace+"/"+service.Name)
175+
176+
sc.enqueueService(serviceObj)
177+
}
178+
128179
func (sc *ServiceServingCertController) enqueueService(obj interface{}) {
180+
_, ok := obj.(*kapi.Service)
181+
if !ok {
182+
return
183+
}
129184
key, err := controller.KeyFunc(obj)
130185
if err != nil {
131186
glog.Errorf("Couldn't get key for object %+v: %v", obj, err)
@@ -292,7 +347,8 @@ func getNumFailures(service *kapi.Service) int {
292347
}
293348

294349
func (sc *ServiceServingCertController) requiresCertGeneration(service *kapi.Service) bool {
295-
if secretName := service.Annotations[ServingCertSecretAnnotation]; len(secretName) == 0 {
350+
secretName := service.Annotations[ServingCertSecretAnnotation]
351+
if len(secretName) == 0 {
296352
return false
297353
}
298354
if getNumFailures(service) >= sc.maxRetries {
@@ -301,6 +357,9 @@ func (sc *ServiceServingCertController) requiresCertGeneration(service *kapi.Ser
301357
if service.Annotations[ServingCertCreatedByAnnotation] == sc.ca.Config.Certs[0].Subject.CommonName {
302358
return false
303359
}
304-
360+
// TODO: use the lister here
361+
if _, exists, _ := sc.secretCache.GetByKey(service.Namespace + "/" + secretName); !exists {
362+
return true
363+
}
305364
return true
306365
}

‎pkg/service/controller/servingcert/secret_creating_controller_test.go

Lines changed: 145 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import (
2222
"github.com/openshift/origin/pkg/cmd/server/crypto/extensions"
2323
)
2424

25-
func controllerSetup(startingObjects []runtime.Object, stopChannel chan struct{}, t *testing.T) ( /*caName*/ string, *fake.Clientset, *watch.FakeWatcher, *ServiceServingCertController) {
25+
func controllerSetup(startingObjects []runtime.Object, stopChannel chan struct{}, t *testing.T) ( /*caName*/ string, *fake.Clientset, *watch.FakeWatcher, *watch.FakeWatcher, *ServiceServingCertController) {
2626
certDir, err := ioutil.TempDir("", "serving-cert-unit-")
2727
if err != nil {
2828
t.Fatalf("unexpected error: %v", err)
@@ -42,17 +42,21 @@ func controllerSetup(startingObjects []runtime.Object, stopChannel chan struct{}
4242

4343
kubeclient := fake.NewSimpleClientset(startingObjects...)
4444
fakeWatch := watch.NewFake()
45+
fakeSecretWatch := watch.NewFake()
4546
kubeclient.PrependReactor("create", "*", func(action core.Action) (handled bool, ret runtime.Object, err error) {
4647
return true, action.(core.CreateAction).GetObject(), nil
4748
})
4849
kubeclient.PrependReactor("update", "*", func(action core.Action) (handled bool, ret runtime.Object, err error) {
4950
return true, action.(core.UpdateAction).GetObject(), nil
5051
})
51-
kubeclient.PrependWatchReactor("*", core.DefaultWatchReactor(fakeWatch, nil))
52+
kubeclient.PrependWatchReactor("services", core.DefaultWatchReactor(fakeWatch, nil))
53+
kubeclient.PrependWatchReactor("secrets", core.DefaultWatchReactor(fakeSecretWatch, nil))
5254

5355
controller := NewServiceServingCertController(kubeclient.Core(), kubeclient.Core(), ca, "cluster.local", 10*time.Minute)
56+
controller.serviceHasSynced = func() bool { return true }
57+
controller.secretHasSynced = func() bool { return true }
5458

55-
return caOptions.Name, kubeclient, fakeWatch, controller
59+
return caOptions.Name, kubeclient, fakeWatch, fakeSecretWatch, controller
5660
}
5761

5862
func checkGeneratedCertificate(t *testing.T, certData []byte, service *kapi.Service) {
@@ -105,7 +109,7 @@ func TestBasicControllerFlow(t *testing.T) {
105109
defer close(stopChannel)
106110
received := make(chan bool)
107111

108-
caName, kubeclient, fakeWatch, controller := controllerSetup([]runtime.Object{}, stopChannel, t)
112+
caName, kubeclient, fakeWatch, _, controller := controllerSetup([]runtime.Object{}, stopChannel, t)
109113
controller.syncHandler = func(serviceKey string) error {
110114
defer func() { received <- true }()
111115

@@ -200,7 +204,7 @@ func TestAlreadyExistingSecretControllerFlow(t *testing.T) {
200204
existingSecret.Type = kapi.SecretTypeTLS
201205
existingSecret.Annotations = expectedSecretAnnotations
202206

203-
caName, kubeclient, fakeWatch, controller := controllerSetup([]runtime.Object{existingSecret}, stopChannel, t)
207+
caName, kubeclient, fakeWatch, _, controller := controllerSetup([]runtime.Object{existingSecret}, stopChannel, t)
204208
kubeclient.PrependReactor("create", "secrets", func(action core.Action) (handled bool, ret runtime.Object, err error) {
205209
return true, &kapi.Secret{}, kapierrors.NewAlreadyExists(kapi.Resource("secrets"), "new-secret")
206210
})
@@ -277,7 +281,7 @@ func TestAlreadyExistingSecretForDifferentUIDControllerFlow(t *testing.T) {
277281
existingSecret.Type = kapi.SecretTypeTLS
278282
existingSecret.Annotations = map[string]string{ServiceUIDAnnotation: "wrong-uid", ServiceNameAnnotation: serviceName}
279283

280-
_, kubeclient, fakeWatch, controller := controllerSetup([]runtime.Object{existingSecret}, stopChannel, t)
284+
_, kubeclient, fakeWatch, _, controller := controllerSetup([]runtime.Object{existingSecret}, stopChannel, t)
281285
kubeclient.PrependReactor("create", "secrets", func(action core.Action) (handled bool, ret runtime.Object, err error) {
282286
return true, &kapi.Secret{}, kapierrors.NewAlreadyExists(kapi.Resource("secrets"), "new-secret")
283287
})
@@ -347,7 +351,7 @@ func TestSecretCreationErrorControllerFlow(t *testing.T) {
347351
serviceUID := "some-uid"
348352
namespace := "ns"
349353

350-
_, kubeclient, fakeWatch, controller := controllerSetup([]runtime.Object{}, stopChannel, t)
354+
_, kubeclient, fakeWatch, _, controller := controllerSetup([]runtime.Object{}, stopChannel, t)
351355
kubeclient.PrependReactor("create", "secrets", func(action core.Action) (handled bool, ret runtime.Object, err error) {
352356
return true, &kapi.Secret{}, kapierrors.NewForbidden(kapi.Resource("secrets"), "new-secret", fmt.Errorf("any reason"))
353357
})
@@ -409,7 +413,7 @@ func TestSkipGenerationControllerFlow(t *testing.T) {
409413
serviceUID := "some-uid"
410414
namespace := "ns"
411415

412-
caName, kubeclient, fakeWatch, controller := controllerSetup([]runtime.Object{}, stopChannel, t)
416+
caName, kubeclient, fakeWatch, _, controller := controllerSetup([]runtime.Object{}, stopChannel, t)
413417
kubeclient.PrependReactor("update", "service", func(action core.Action) (handled bool, ret runtime.Object, err error) {
414418
return true, &kapi.Service{}, kapierrors.NewForbidden(kapi.Resource("fdsa"), "new-service", fmt.Errorf("any service reason"))
415419
})
@@ -470,3 +474,136 @@ func TestSkipGenerationControllerFlow(t *testing.T) {
470474
}
471475
}
472476
}
477+
478+
func TestRecreateSecretControllerFlow(t *testing.T) {
479+
stopChannel := make(chan struct{})
480+
defer close(stopChannel)
481+
received := make(chan bool)
482+
483+
caName, kubeclient, fakeWatch, fakeSecretWatch, controller := controllerSetup([]runtime.Object{}, stopChannel, t)
484+
controller.syncHandler = func(serviceKey string) error {
485+
defer func() { received <- true }()
486+
487+
err := controller.syncService(serviceKey)
488+
if err != nil {
489+
t.Errorf("unexpected error: %v", err)
490+
}
491+
492+
return err
493+
}
494+
go controller.Run(1, stopChannel)
495+
496+
expectedSecretName := "new-secret"
497+
serviceName := "svc-name"
498+
serviceUID := "some-uid"
499+
expectedServiceAnnotations := map[string]string{ServingCertSecretAnnotation: expectedSecretName, ServingCertCreatedByAnnotation: caName}
500+
expectedSecretAnnotations := map[string]string{ServiceUIDAnnotation: serviceUID, ServiceNameAnnotation: serviceName}
501+
namespace := "ns"
502+
503+
serviceToAdd := &kapi.Service{}
504+
serviceToAdd.Name = serviceName
505+
serviceToAdd.Namespace = namespace
506+
serviceToAdd.UID = types.UID(serviceUID)
507+
serviceToAdd.Annotations = map[string]string{ServingCertSecretAnnotation: expectedSecretName}
508+
fakeWatch.Add(serviceToAdd)
509+
510+
secretToDelete := &kapi.Secret{}
511+
secretToDelete.Name = expectedSecretName
512+
secretToDelete.Namespace = namespace
513+
secretToDelete.Annotations = map[string]string{ServiceNameAnnotation: serviceName}
514+
515+
t.Log("waiting to reach syncHandler")
516+
select {
517+
case <-received:
518+
case <-time.After(time.Duration(30 * time.Second)):
519+
t.Fatalf("failed to call into syncService")
520+
}
521+
522+
foundSecret := false
523+
foundServiceUpdate := false
524+
for _, action := range kubeclient.Actions() {
525+
switch {
526+
case action.Matches("create", "secrets"):
527+
createSecret := action.(core.CreateAction)
528+
newSecret := createSecret.GetObject().(*kapi.Secret)
529+
if newSecret.Name != expectedSecretName {
530+
t.Errorf("expected %v, got %v", expectedSecretName, newSecret.Name)
531+
continue
532+
}
533+
if newSecret.Namespace != namespace {
534+
t.Errorf("expected %v, got %v", namespace, newSecret.Namespace)
535+
continue
536+
}
537+
delete(newSecret.Annotations, ServingCertExpiryAnnotation)
538+
if !reflect.DeepEqual(newSecret.Annotations, expectedSecretAnnotations) {
539+
t.Errorf("expected %v, got %v", expectedSecretAnnotations, newSecret.Annotations)
540+
continue
541+
}
542+
543+
checkGeneratedCertificate(t, newSecret.Data["tls.crt"], serviceToAdd)
544+
foundSecret = true
545+
546+
case action.Matches("update", "services"):
547+
updateService := action.(core.UpdateAction)
548+
service := updateService.GetObject().(*kapi.Service)
549+
if !reflect.DeepEqual(service.Annotations, expectedServiceAnnotations) {
550+
t.Errorf("expected %v, got %v", expectedServiceAnnotations, service.Annotations)
551+
continue
552+
}
553+
foundServiceUpdate = true
554+
555+
}
556+
}
557+
558+
if !foundSecret {
559+
t.Errorf("secret wasn't created. Got %v\n", kubeclient.Actions())
560+
}
561+
if !foundServiceUpdate {
562+
t.Errorf("service wasn't updated. Got %v\n", kubeclient.Actions())
563+
}
564+
565+
kubeclient.ClearActions()
566+
fakeSecretWatch.Add(secretToDelete)
567+
fakeSecretWatch.Delete(secretToDelete)
568+
569+
t.Log("waiting to reach syncHandler")
570+
select {
571+
case <-received:
572+
case <-time.After(time.Duration(30 * time.Second)):
573+
t.Fatalf("failed to call into syncService")
574+
}
575+
576+
for _, action := range kubeclient.Actions() {
577+
switch {
578+
case action.Matches("create", "secrets"):
579+
createSecret := action.(core.CreateAction)
580+
newSecret := createSecret.GetObject().(*kapi.Secret)
581+
if newSecret.Name != expectedSecretName {
582+
t.Errorf("expected %v, got %v", expectedSecretName, newSecret.Name)
583+
continue
584+
}
585+
if newSecret.Namespace != namespace {
586+
t.Errorf("expected %v, got %v", namespace, newSecret.Namespace)
587+
continue
588+
}
589+
delete(newSecret.Annotations, ServingCertExpiryAnnotation)
590+
if !reflect.DeepEqual(newSecret.Annotations, expectedSecretAnnotations) {
591+
t.Errorf("expected %v, got %v", expectedSecretAnnotations, newSecret.Annotations)
592+
continue
593+
}
594+
595+
checkGeneratedCertificate(t, newSecret.Data["tls.crt"], serviceToAdd)
596+
foundSecret = true
597+
598+
case action.Matches("update", "services"):
599+
updateService := action.(core.UpdateAction)
600+
service := updateService.GetObject().(*kapi.Service)
601+
if !reflect.DeepEqual(service.Annotations, expectedServiceAnnotations) {
602+
t.Errorf("expected %v, got %v", expectedServiceAnnotations, service.Annotations)
603+
continue
604+
}
605+
foundServiceUpdate = true
606+
607+
}
608+
}
609+
}

0 commit comments

Comments
 (0)