From db1f2bd71fed9fba283cd0a39f52491c161ef4f1 Mon Sep 17 00:00:00 2001
From: Sam Shen <slshen@users.noreply.github.com>
Date: Fri, 26 Jun 2020 21:27:00 -0700
Subject: [PATCH] Fix renewal logic (#6)

Signed-off-by: Sam Shen <slshen@users.noreply.github.com>
---
 .github/workflows/main.yml |  7 +++++++
 .gitignore                 |  1 +
 cert.go                    |  1 -
 cert_test.go               | 10 ++++++++++
 ktls.go                    | 33 ++++++++++++++++-----------------
 5 files changed, 34 insertions(+), 18 deletions(-)
 create mode 100644 .gitignore

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index f492723..e3dce88 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -27,6 +27,13 @@ jobs:
       with:
         ref: ${{ github.event.pull_request.head.sha }}
         fetch-depth: 0
+    - name: Caching go modules
+      uses: actions/cache@v1
+      with:
+        path: ~/go/pkg/mod
+        key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
+        restore-keys: |
+          ${{ runner.os }}-go-        
     - name: Installing golangci-lint
       run: wget -O - -q https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s v1.23.8
     - run: ./hack/build.sh
diff --git a/.gitignore b/.gitignore
new file mode 100644
index 0000000..1d74e21
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1 @@
+.vscode/
diff --git a/cert.go b/cert.go
index 7a0bda6..ef0ea35 100644
--- a/cert.go
+++ b/cert.go
@@ -30,7 +30,6 @@ import (
 type CertificateKeyPair struct {
 	CertPem          []byte
 	KeyPem           []byte
-	Source           interface{}
 	parsedCertifcate *x509.Certificate
 }
 
diff --git a/cert_test.go b/cert_test.go
index 9ebc41f..add7b3d 100644
--- a/cert_test.go
+++ b/cert_test.go
@@ -46,3 +46,13 @@ func TestGenerateCerts(t *testing.T) {
 		t.Error("could not get CA cert pem")
 	}
 }
+
+func TestRenewal(t *testing.T) {
+	cackp, err := GenerateCert("Test Inc", nil, nil, time.Minute)
+	if err != nil {
+		t.Error(err)
+	}
+	if cackp.IsValid(2 * time.Minute) {
+		t.Error("certificate should require renewal")
+	}
+}
diff --git a/ktls.go b/ktls.go
index 307eb57..70efbb7 100644
--- a/ktls.go
+++ b/ktls.go
@@ -60,8 +60,6 @@ type TLSSecret struct {
 	kubeClientError error
 }
 
-const renewalDenomintor = 5
-
 func (t *TLSSecret) logf(format string, values ...interface{}) {
 	if t.Log != nil {
 		t.Log(format, values...)
@@ -145,7 +143,7 @@ func (t *TLSSecret) getSecret(name string) (*corev1.Secret, error) {
 }
 
 func (t *TLSSecret) GetCertificateKeyPair() (*CertificateKeyPair, error) {
-	ckp, err := t.getSecretCertificate(t.Name, t.getCertDuration()/renewalDenomintor)
+	ckp, _, err := t.getSecretCertificate(t.Name, 10*time.Minute)
 	if err != nil {
 		return nil, err
 	}
@@ -165,23 +163,22 @@ func (t *TLSSecret) GetCertificateKeyPair() (*CertificateKeyPair, error) {
 	return ckp, nil
 }
 
-func (t *TLSSecret) getSecretCertificate(name string, d time.Duration) (*CertificateKeyPair, error) {
+func (t *TLSSecret) getSecretCertificate(name string, d time.Duration) (*CertificateKeyPair, *corev1.Secret, error) {
 	secret, err := t.getSecret(name)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	if secret != nil && secret.Data != nil {
 		ckp := &CertificateKeyPair{
 			KeyPem:  getSecretData(secret, corev1.TLSPrivateKeyKey),
 			CertPem: getSecretData(secret, corev1.TLSCertKey),
-			Source:  secret,
 		}
 		if ckp.IsValid(d) {
-			t.logf("Using TLS secret from %s/%s", t.GetNamespace(), t.Name)
-			return ckp, nil
+			t.logf("Using TLS secret from %s/%s valid for at least %s", t.GetNamespace(), name, d)
+			return ckp, secret, nil
 		}
 	}
-	return nil, nil
+	return nil, secret, nil
 }
 
 func (t *TLSSecret) getCertDuration() time.Duration {
@@ -208,11 +205,17 @@ func (t *TLSSecret) generateCert() (*CertificateKeyPair, error) {
 	}
 	var caCert *CertificateKeyPair
 	var cert *CertificateKeyPair
-	caCert, err = t.getSecretCertificate(caName, t.getCACertDuration()/renewalDenomintor)
+	caCert, _, err = t.getSecretCertificate(caName, time.Hour)
 	caDuration := t.getCACertDuration()
+	if caDuration < 8*time.Hour {
+		return nil, fmt.Errorf("CA duration must be at least 8 hours")
+	}
 	certDuration := t.getCertDuration()
+	if certDuration < 30*time.Minute {
+		return nil, fmt.Errorf("cert duration must be at least 30 minutes")
+	}
 	if err == nil {
-		if caCert == nil || caCert.IsValid(caDuration/renewalDenomintor) {
+		if caCert == nil || !caCert.IsValid(time.Hour) {
 			t.logf("Generating new CA certificate %s/%s", t.GetNamespace(), caName)
 			caCert, err = GenerateCert(caName, nil, nil, caDuration)
 			if err == nil {
@@ -235,19 +238,15 @@ func (t *TLSSecret) generateCert() (*CertificateKeyPair, error) {
 
 func (t *TLSSecret) persistCert(ckp *CertificateKeyPair, name string) error {
 	err := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
-		c, err := t.getSecretCertificate(name, time.Minute)
+		c, secret, err := t.getSecretCertificate(name, time.Minute)
 		if err != nil {
 			return err
 		}
-		if c.IsValid(time.Minute) {
+		if c != nil {
 			t.logf("Updated certificate is now valid")
 			ckp.CopyFrom(c)
 			return nil
 		}
-		var secret *corev1.Secret
-		if c != nil {
-			secret = c.Source.(*corev1.Secret)
-		}
 		secretData := map[string][]byte{
 			corev1.TLSCertKey:       ckp.CertPem,
 			corev1.TLSPrivateKeyKey: ckp.KeyPem,