do not rewrite certs unless route has changed

plugins/router/template/certmanager.go

@@ -54,6 +54,10 @@ func (cm *simpleCertificateManager) WriteCertificatesForConfig(config *ServiceAl
 	if config == nil {
 		return nil
 	}
+	if config.Status == ServiceAliasConfigStatusSaved {
+		glog.V(4).Infof("skipping certificate write for %s%s since it's status is already %s", config.Host, config.Path, ServiceAliasConfigStatusSaved)
+		return nil
+	}
 	if len(config.Certificates) > 0 {
 		if config.TLSTermination == routeapi.TLSTerminationEdge || config.TLSTermination == routeapi.TLSTerminationReencrypt {
 			certKey := cm.cfg.certKeyFunc(config)
@@ -75,8 +79,7 @@ func (cm *simpleCertificateManager) WriteCertificatesForConfig(config *ServiceAl
 					buffer.Write([]byte(caCertObj.Contents))
 				}
 
-				err := cm.w.WriteCertificate(cm.cfg.certDir, certObj.ID, buffer.Bytes())
-				if err != nil {
+				if err := cm.w.WriteCertificate(cm.cfg.certDir, certObj.ID, buffer.Bytes()); err != nil {
 					return err
 				}
 			}
@@ -87,8 +90,7 @@ func (cm *simpleCertificateManager) WriteCertificatesForConfig(config *ServiceAl
 			destCert, ok := config.Certificates[destCertKey]
 
 			if ok {
-				err := cm.w.WriteCertificate(cm.cfg.caCertDir, destCert.ID, []byte(destCert.Contents))
-				if err != nil {
+				if err := cm.w.WriteCertificate(cm.cfg.caCertDir, destCert.ID, []byte(destCert.Contents)); err != nil {
 					return err
 				}
 			}

plugins/router/template/certmanager_test.go

@@ -147,6 +147,38 @@ func TestCertManager(t *testing.T) {
 	}
 }
 
+func TestCertManagerSkipsWrittenConfigs(t *testing.T) {
+	fakeCertWriter := &fakeCertWriter{}
+	certManager, _ := newSimpleCertificateManager(newFakeCertificateManagerConfig(), fakeCertWriter)
+	cfg := &ServiceAliasConfig{
+		Host:           "www.example.com",
+		TLSTermination: routeapi.TLSTerminationEdge,
+		Certificates: map[string]Certificate{
+			"www.example.com": {
+				ID: "testCert",
+			},
+			"www.example.com" + caCertPostfix: {
+				ID: "testCert",
+			},
+		},
+	}
+	certManager.WriteCertificatesForConfig(cfg)
+	if len(fakeCertWriter.addedCerts) != 1 {
+		t.Errorf("expected 1 add for initial certificate write but got %d", len(fakeCertWriter.addedCerts))
+	}
+	cfg.Status = ServiceAliasConfigStatusSaved
+	certManager.WriteCertificatesForConfig(cfg)
+	if len(fakeCertWriter.addedCerts) != 1 {
+		t.Errorf("expected 1 add for initial certificate write but got %d", len(fakeCertWriter.addedCerts))
+	}
+	// clear status and ensure it is written
+	cfg.Status = ""
+	certManager.WriteCertificatesForConfig(cfg)
+	if len(fakeCertWriter.addedCerts) != 2 {
+		t.Errorf("expected 2 adds for initial certificate write but got %d", len(fakeCertWriter.addedCerts))
+	}
+}
+
 func TestCertManagerConfig(t *testing.T) {
 	validCfg := newFakeCertificateManagerConfig()
 
@@ -173,11 +205,11 @@ func TestCertManagerConfig(t *testing.T) {
 		shouldPass bool
 	}{
 		"valid": {shouldPass: true, config: validCfg},
-		"missing certificateKeyFunc": {shouldPass: false, config: missingCertKeyCfg},
-		"missing caCertificateKeyFunc": {shouldPass: false, config: missingCACertKeyCfg},
+		"missing certificateKeyFunc":     {shouldPass: false, config: missingCertKeyCfg},
+		"missing caCertificateKeyFunc":   {shouldPass: false, config: missingCACertKeyCfg},
 		"missing destCertificateKeyFunc": {shouldPass: false, config: missingDestCertKeyCfg},
 		"missing 	certificateDir": {shouldPass: false, config: missingCertDirCfg},
-		"missing caCertificateDir": {shouldPass: false, config: missingCACertDirCfg},
+		"missing caCertificateDir":                 {shouldPass: false, config: missingCACertDirCfg},
 		"matching certificateDir/caCertificateDir": {shouldPass: false, config: matchingCertDirCfg},
 	}
 

plugins/router/template/fake.go

@@ -2,10 +2,10 @@ package templaterouter
 
 // newFakeTemplateRouter provides an empty template router with a simple certificate manager
 // backed by a fake cert writer for testing
-func newFakeTemplateRouter() templateRouter{
+func newFakeTemplateRouter() templateRouter {
 	fakeCertManager, _ := newSimpleCertificateManager(newFakeCertificateManagerConfig(), &fakeCertWriter{})
 	return templateRouter{
-		state: map[string]ServiceUnit{},
+		state:       map[string]ServiceUnit{},
 		certManager: fakeCertManager,
 	}
 }

plugins/router/template/router.go

@@ -57,7 +57,7 @@ type templateData struct {
 
 func newTemplateRouter(templates map[string]*template.Template, reloadScriptPath, defaultCertificate string) (*templateRouter, error) {
 	glog.Infof("Creating a new template router")
-	certManagerConfig := &certificateManagerConfig {
+	certManagerConfig := &certificateManagerConfig{
 		certKeyFunc:     generateCertKey,
 		caCertKeyFunc:   generateCACertKey,
 		destCertKeyFunc: generateDestCertKey,
@@ -68,7 +68,7 @@ func newTemplateRouter(templates map[string]*template.Template, reloadScriptPath
 	if err != nil {
 		return nil, err
 	}
-	
+
 	router := &templateRouter{
 		templates:              templates,
 		reloadScriptPath:       reloadScriptPath,
@@ -154,12 +154,14 @@ func (r *templateRouter) writeState() error {
 func (r *templateRouter) writeConfig() error {
 	//write out any certificate files that don't exist
 	for _, serviceUnit := range r.state {
-		for _, cfg := range serviceUnit.ServiceAliasConfigs {
+		for k, cfg := range serviceUnit.ServiceAliasConfigs {
 			err := r.certManager.WriteCertificatesForConfig(&cfg)
 			if err != nil {
 				glog.Errorf("Error writing certificates for %s: %v", serviceUnit.Name, err)
 				return err
 			}
+			cfg.Status = ServiceAliasConfigStatusSaved
+			serviceUnit.ServiceAliasConfigs[k] = cfg
 		}
 	}
 

plugins/router/template/types.go

@@ -27,8 +27,20 @@ type ServiceAliasConfig struct {
 	TLSTermination routeapi.TLSTerminationType
 	// Certificates used for securing this backend.  Keyed by the cert id
 	Certificates map[string]Certificate
+	// Indicates the status of configuration that needs to be persisted.  Right now this only
+	// includes the certificates and is not an indicator of being written to the underlying
+	// router implementation
+	Status ServiceAliasConfigStatus
 }
 
+type ServiceAliasConfigStatus string
+
+const (
+	// ServiceAliasConfigStatusSaved indicates that the necessary files for this config have
+	// been persisted to disk.
+	ServiceAliasConfigStatusSaved ServiceAliasConfigStatus = "saved"
+)
+
 // Certificate represents a pub/private key pair.  It is identified by ID which will become the file name.
 // A CA certificate will not have a PrivateKey set.
 type Certificate struct {