From f5b31e83aff401af5cbaf3a544402e4613425e6a Mon Sep 17 00:00:00 2001
From: Alan Moran <bonzofenix@gmail.com>
Date: Thu, 19 Dec 2024 12:48:39 -0300
Subject: [PATCH] Reduce load of parsing the certificate on every request to
 check cert expiration date

---
 src/autoscaler/helpers/httpclient.go      | 50 ++++++++++++++++-------
 src/autoscaler/helpers/httpclient_test.go | 19 ++++++---
 2 files changed, 50 insertions(+), 19 deletions(-)

diff --git a/src/autoscaler/helpers/httpclient.go b/src/autoscaler/helpers/httpclient.go
index f4f6c6daee..47a840baff 100644
--- a/src/autoscaler/helpers/httpclient.go
+++ b/src/autoscaler/helpers/httpclient.go
@@ -17,9 +17,27 @@ import (
 )
 
 type TLSReloadTransport struct {
-	Base     http.RoundTripper
-	logger   lager.Logger
-	tlsCerts *models.TLSCerts
+	Base           http.RoundTripper
+	logger         lager.Logger
+	tlsCerts       *models.TLSCerts
+	certExpiration *time.Time
+}
+
+func NewTLSReloadTransport(base http.RoundTripper, logger lager.Logger, tlsCerts *models.TLSCerts) *TLSReloadTransport {
+	return &TLSReloadTransport{
+		Base:     base,
+		logger:   logger,
+		tlsCerts: tlsCerts,
+	}
+}
+
+func (t *TLSReloadTransport) GetCertExpiration() *time.Time {
+	if t.certExpiration == nil {
+		x509Cert, _ := x509.ParseCertificate(t.tlsClientConfig().Certificates[0].Certificate[0])
+		t.certExpiration = &x509Cert.NotAfter
+	}
+
+	return t.certExpiration
 }
 
 func (t *TLSReloadTransport) tlsClientConfig() *tls.Config {
@@ -30,21 +48,24 @@ func (t *TLSReloadTransport) setTLSClientConfig(tlsConfig *tls.Config) {
 	t.Base.(*retryablehttp.RoundTripper).Client.HTTPClient.Transport.(*http.Transport).TLSClientConfig = tlsConfig
 }
 
-func (t *TLSReloadTransport) certificateExpiringWithin(dur time.Duration) bool {
-	x509Cert, err := x509.ParseCertificate(t.tlsClientConfig().Certificates[0].Certificate[0])
-	if err != nil {
-		return false
-	}
+func (t *TLSReloadTransport) reloadCert() {
+	tlsConfig, _ := t.tlsCerts.CreateClientConfig()
+	t.setTLSClientConfig(tlsConfig)
+	x509Cert, _ := x509.ParseCertificate(t.tlsClientConfig().Certificates[0].Certificate[0])
+	t.certExpiration = &x509Cert.NotAfter
+}
 
-	return x509Cert.NotAfter.Sub(time.Now()) < dur
+func (t *TLSReloadTransport) certificateExpiringWithin(dur time.Duration) bool {
+	return t.GetCertExpiration().Sub(time.Now()) < dur
 }
+
 func (t *TLSReloadTransport) RoundTrip(req *http.Request) (*http.Response, error) {
-	if t.certificateExpiringWithin(time.Hour) {
-		t.logger.Info("reloading-cert", lager.Data{"request": req})
-		tlsConfig, _ := t.tlsCerts.CreateClientConfig()
-		t.setTLSClientConfig(tlsConfig)
+	// Checks for cert validity within 5m timeframe. See https://docs.cloudfoundry.org/devguide/deploy-apps/instance-identity.html
+	if t.certificateExpiringWithin(5 * time.Minute) {
+		t.logger.Debug("reloading-cert", lager.Data{"request": req})
+		t.reloadCert()
 	} else {
-		t.logger.Info("cert-not-expiring", lager.Data{"request": req})
+		t.logger.Debug("cert-not-expiring", lager.Data{"request": req})
 	}
 
 	return t.Base.RoundTrip(req)
@@ -76,6 +97,7 @@ func CreateHTTPSClient(tlsCerts *models.TLSCerts, config cf.ClientConfig, logger
 		Base:     retryClient.Transport,
 		logger:   logger,
 		tlsCerts: tlsCerts,
+		// TODO: try sending reference to tls config of the client
 	}
 
 	return retryClient, nil
diff --git a/src/autoscaler/helpers/httpclient_test.go b/src/autoscaler/helpers/httpclient_test.go
index f5e9f1c278..9d182faaaf 100644
--- a/src/autoscaler/helpers/httpclient_test.go
+++ b/src/autoscaler/helpers/httpclient_test.go
@@ -6,6 +6,7 @@ import (
 	"crypto/tls"
 	"crypto/x509"
 	"encoding/pem"
+	"fmt"
 	"log"
 	"net/http"
 	"os"
@@ -78,9 +79,9 @@ var _ = Describe("HTTPClient", func() {
 			os.Remove(cfInstanceKeyFile)
 		})
 
-		When("Cert cert is not within 1 hour of expiration", func() {
+		When("Cert cert is not within 5m of expiration", func() {
 			BeforeEach(func() {
-				notAfter = time.Now().Add(63 * time.Minute)
+				notAfter = time.Now().Add(7 * time.Minute)
 			})
 
 			It("should reload the cert", func() {
@@ -90,11 +91,11 @@ var _ = Describe("HTTPClient", func() {
 			})
 		})
 
-		When("Cert cert is within 1 hour of expiration", func() {
+		When("Cert cert is within 5m of expiration", func() {
 			var cfInstanceCertFileToRotateContent []byte
 
 			BeforeEach(func() {
-				notAfter = time.Now().Add(59 * time.Minute)
+				notAfter = time.Now().Add(3 * time.Minute)
 			})
 
 			It("should reload the cert", func() {
@@ -105,16 +106,24 @@ var _ = Describe("HTTPClient", func() {
 				_, err = configutil.MaterializeContentInFile(certTmpDir, "eventgenerator.crt", string(cfInstanceCertFileToRotateContent))
 				Expect(err).NotTo(HaveOccurred())
 
+				oldCertExpiration := getCertExpirationFromClient(client)
+				fmt.Println(oldCertExpiration)
 				Expect(getCertFromClient(client)).To(Equal(string(cfInstanceCertContent)))
 				client.Get(fakeServer.URL())
 				Expect(logger).To(gbytes.Say("reloading-cert"))
-
+				newCertExpiration := getCertExpirationFromClient(client)
+				Expect(newCertExpiration).To(BeTemporally(">", oldCertExpiration))
 				Expect(getCertFromClient(client)).To(Equal(string(cfInstanceCertFileToRotateContent)))
 			})
 		})
 	})
 })
 
+func getCertExpirationFromClient(client *http.Client) time.Time {
+	GinkgoHelper()
+	return *client.Transport.(*helpers.TLSReloadTransport).GetCertExpiration()
+}
+
 func getCertFromClient(client *http.Client) string {
 	GinkgoHelper()
 	cert := client.Transport.(*helpers.TLSReloadTransport).Base.(*retryablehttp.RoundTripper).Client.HTTPClient.Transport.(*http.Transport).TLSClientConfig.Certificates[0]