From 6642b082d3020edd20ee000dc99d7e96f2114579 Mon Sep 17 00:00:00 2001
From: Ram Lavi <ralavi@redhat.com>
Date: Thu, 7 Oct 2021 15:55:05 +0300
Subject: [PATCH] certs, Add Certs list size limit

certs, Add Certs list size limit
Currently certs are appended without limit. This can
cause a problem when applying the caBundle/certs on the
mutatingwebhookconfiguration/secret, appropriately - due
to etcd's POST size limit.
Limited the amount of certs to 100.

Signed-off-by: Ram Lavi <ralavi@redhat.com>
---
 pkg/certificate/configuration.go            |  1 +
 pkg/certificate/triple/pem.go               | 11 +++++
 pkg/certificate/triple/triple_suite_test.go | 15 ++++++
 pkg/certificate/triple/triple_test.go       | 55 +++++++++++++++++++++
 4 files changed, 82 insertions(+)
 create mode 100644 pkg/certificate/triple/triple_suite_test.go
 create mode 100644 pkg/certificate/triple/triple_test.go

diff --git a/pkg/certificate/configuration.go b/pkg/certificate/configuration.go
index 70af1ca5..09e6d90d 100644
--- a/pkg/certificate/configuration.go
+++ b/pkg/certificate/configuration.go
@@ -92,6 +92,7 @@ func (m *Manager) addCertificateToCABundle(caCert *x509.Certificate) error {
 			}
 		}
 		cas = append(cas, caCert)
+		cas = triple.RemoveOldestCerts(cas, triple.CertsListSizeLimit)
 		return triple.EncodeCertsPEM(cas), nil
 	})
 	if err != nil {
diff --git a/pkg/certificate/triple/pem.go b/pkg/certificate/triple/pem.go
index 9916819c..cd8f7343 100644
--- a/pkg/certificate/triple/pem.go
+++ b/pkg/certificate/triple/pem.go
@@ -38,6 +38,8 @@ const (
 	CertificateBlockType = "CERTIFICATE"
 	// CertificateRequestBlockType is a possible value for pem.Block.Type.
 	CertificateRequestBlockType = "CERTIFICATE REQUEST"
+	// CertsListSizeLimit sets the max size of a certs list
+	CertsListSizeLimit = 100
 )
 
 // EncodePublicKeyPEM returns PEM-encoded public data
@@ -81,6 +83,15 @@ func EncodeCertsPEM(certs []*x509.Certificate) []byte {
 	return certsPEM
 }
 
+// RemoveOldestCerts removes old certs to avoid bloating
+func RemoveOldestCerts(certs []*x509.Certificate, maxListSize int) []*x509.Certificate {
+	if len(certs) <= maxListSize {
+		return certs
+	}
+	// oldest certs are in the start
+	return certs[len(certs)-maxListSize:]
+}
+
 // ParsePrivateKeyPEM returns a private key parsed from a PEM block in the supplied data.
 // Recognizes PEM blocks for "EC PRIVATE KEY", "RSA PRIVATE KEY", or "PRIVATE KEY"
 func ParsePrivateKeyPEM(keyData []byte) (interface{}, error) {
diff --git a/pkg/certificate/triple/triple_suite_test.go b/pkg/certificate/triple/triple_suite_test.go
new file mode 100644
index 00000000..10c47637
--- /dev/null
+++ b/pkg/certificate/triple/triple_suite_test.go
@@ -0,0 +1,15 @@
+package triple
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo"
+	"github.com/onsi/ginkgo/reporters"
+	. "github.com/onsi/gomega"
+)
+
+func TestTriple(t *testing.T) {
+	RegisterFailHandler(Fail)
+	junitReporter := reporters.NewJUnitReporter("junit.triple_suite_test.xml")
+	RunSpecsWithDefaultAndCustomReporters(t, "Certificate Test Suite", []Reporter{junitReporter})
+}
diff --git a/pkg/certificate/triple/triple_test.go b/pkg/certificate/triple/triple_test.go
new file mode 100644
index 00000000..5fce4e9c
--- /dev/null
+++ b/pkg/certificate/triple/triple_test.go
@@ -0,0 +1,55 @@
+package triple
+
+import (
+	"crypto/x509"
+	"time"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/ginkgo/extensions/table"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("Cert library", func() {
+
+	type removeOldestCertsParams struct {
+		certsList         []*x509.Certificate
+		maxListSize       int
+		expectedCertsList []*x509.Certificate
+	}
+	certOldest := &x509.Certificate{
+		NotBefore: time.Now().Add(10*time.Hour),
+		NotAfter:  time.Now(),
+	}
+	certOld := &x509.Certificate{
+		NotBefore: time.Now().Add(5*time.Hour),
+		NotAfter:  time.Now(),
+	}
+	certCurrent := &x509.Certificate{
+		NotBefore: time.Now(),
+		NotAfter:  time.Now(),
+	}
+
+	DescribeTable("removeOldestCerts",
+		func(c removeOldestCertsParams) {
+			Expect(RemoveOldestCerts(c.certsList, c.maxListSize)).To(ConsistOf(c.expectedCertsList), "should remove the oldest certs")
+		},
+		Entry("when list is empty",
+			removeOldestCertsParams{
+				certsList:         []*x509.Certificate{},
+				maxListSize:       2,
+				expectedCertsList: []*x509.Certificate{},
+			}),
+		Entry("when list size is less or equal to max certs, should keep the certs list intact",
+			removeOldestCertsParams{
+				certsList:         []*x509.Certificate{certOldest, certOld, certCurrent},
+				maxListSize:        3,
+				expectedCertsList: []*x509.Certificate{certOldest, certOld, certCurrent},
+			}),
+		Entry("when list size is bigger than max certs, should remove the oldest certs",
+			removeOldestCertsParams{
+				certsList:         []*x509.Certificate{certOldest, certOld, certCurrent},
+				maxListSize:       2,
+				expectedCertsList: []*x509.Certificate{certOld, certCurrent},
+			}),
+	)
+})