add support for specifying revision history limit via annotation and …

…update documentation
cybozu-go · Aug 5, 2024 · e11bbd9 · e11bbd9
1 parent 662b415
commit e11bbd9
Show file tree

Hide file tree

Showing 3 changed files with 109 additions and 0 deletions.
diff --git a/controllers/httpproxy_controller.go b/controllers/httpproxy_controller.go
@@ -3,6 +3,7 @@ package controllers
 import (
 	"context"
 	"net"
+	"strconv"
 	"strings"
 
 	"github.com/go-logr/logr"
@@ -25,6 +26,7 @@ const (
 	testACMETLSAnnotation             = "kubernetes.io/tls-acme"
 	issuerNameAnnotation              = "cert-manager.io/issuer"
 	clusterIssuerNameAnnotation       = "cert-manager.io/cluster-issuer"
+	revisionHistoryLimitAnnotation    = "cert-manager.io/revision-history-limit"
 	ingressClassNameAnnotation        = "kubernetes.io/ingress.class"
 	contourIngressClassNameAnnotation = "projectcontour.io/ingress.class"
 	delegatedDomainAnnotation         = "contour-plus.cybozu.com/delegated-domain"
@@ -291,6 +293,14 @@ func (r *HTTPProxyReconciler) reconcileCertificate(ctx context.Context, hp *proj
 	if r.CSRRevisionLimit > 0 {
 		certificateSpec["revisionHistoryLimit"] = r.CSRRevisionLimit
 	}
+	if value, ok := hp.Annotations[revisionHistoryLimitAnnotation]; ok {
+		limit, err := strconv.ParseUint(value, 10, 32)
+		if err != nil {
+			log.Error(err, "invalid revisionHistoryLimit", "value", value)
+			return nil
+		}
+		certificateSpec["revisionHistoryLimit"] = limit
+	}
 
 	obj := &unstructured.Unstructured{}
 	obj.SetGroupVersionKind(certManagerGroupVersion.WithKind(CertificateKind))

diff --git a/controllers/httpproxy_controller_test.go b/controllers/httpproxy_controller_test.go
@@ -775,6 +775,101 @@ func testHTTPProxyReconcile() {
 		}))
 		Expect(crtSpec["revisionHistoryLimit"]).Should(Equal(int64(1)))
 	})
+
+	It(`should create Certificate with revisionHistoryLimit set if cert-manager.io/revision-history-limit is specified`, func() {
+		ns := testNamespacePrefix + randomString(10)
+		Expect(k8sClient.Create(context.Background(), &corev1.Namespace{
+			ObjectMeta: ctrl.ObjectMeta{Name: ns},
+		})).ShouldNot(HaveOccurred())
+
+		scm, mgr := setupManager()
+
+		Expect(SetupReconciler(mgr, scm, ReconcilerOptions{
+			ServiceKey:        testServiceKey,
+			DefaultIssuerName: "test-issuer",
+			DefaultIssuerKind: IssuerKind,
+			CreateCertificate: true,
+		})).ShouldNot(HaveOccurred())
+
+		stopMgr := startTestManager(mgr)
+		defer stopMgr()
+
+		By("creating HTTPProxy")
+		hpKey := client.ObjectKey{Name: "foo", Namespace: ns}
+		hp := newDummyHTTPProxy(hpKey)
+		hp.Annotations[revisionHistoryLimitAnnotation] = "2"
+		Expect(k8sClient.Create(context.Background(), hp)).ShouldNot(HaveOccurred())
+
+		By("getting Certificate")
+		crt := certificate()
+		objKey := client.ObjectKey{
+			Name:      hpKey.Name,
+			Namespace: hpKey.Namespace,
+		}
+		Eventually(func() error {
+			return k8sClient.Get(context.Background(), objKey, crt)
+		}).Should(Succeed())
+
+		crtSpec := crt.UnstructuredContent()["spec"].(map[string]interface{})
+		Expect(crtSpec["dnsNames"]).Should(Equal([]interface{}{dnsName}))
+		Expect(crtSpec["secretName"]).Should(Equal(testSecretName))
+		Expect(crtSpec["commonName"]).Should(Equal(dnsName))
+		Expect(crtSpec["usages"]).Should(Equal([]interface{}{
+			usageDigitalSignature,
+			usageKeyEncipherment,
+			usageServerAuth,
+			usageClientAuth,
+		}))
+		Expect(crtSpec["revisionHistoryLimit"]).Should(Equal(int64(2)))
+	})
+
+	It(`should prioritize revisionHistoryLimit set by cert-manager.io/revision-history-limit is specified`, func() {
+		ns := testNamespacePrefix + randomString(10)
+		Expect(k8sClient.Create(context.Background(), &corev1.Namespace{
+			ObjectMeta: ctrl.ObjectMeta{Name: ns},
+		})).ShouldNot(HaveOccurred())
+
+		scm, mgr := setupManager()
+
+		Expect(SetupReconciler(mgr, scm, ReconcilerOptions{
+			ServiceKey:        testServiceKey,
+			DefaultIssuerName: "test-issuer",
+			DefaultIssuerKind: IssuerKind,
+			CreateCertificate: true,
+			CSRRevisionLimit:  1,
+		})).ShouldNot(HaveOccurred())
+
+		stopMgr := startTestManager(mgr)
+		defer stopMgr()
+
+		By("creating HTTPProxy")
+		hpKey := client.ObjectKey{Name: "foo", Namespace: ns}
+		hp := newDummyHTTPProxy(hpKey)
+		hp.Annotations[revisionHistoryLimitAnnotation] = "2"
+		Expect(k8sClient.Create(context.Background(), hp)).ShouldNot(HaveOccurred())
+
+		By("getting Certificate")
+		crt := certificate()
+		objKey := client.ObjectKey{
+			Name:      hpKey.Name,
+			Namespace: hpKey.Namespace,
+		}
+		Eventually(func() error {
+			return k8sClient.Get(context.Background(), objKey, crt)
+		}).Should(Succeed())
+
+		crtSpec := crt.UnstructuredContent()["spec"].(map[string]interface{})
+		Expect(crtSpec["dnsNames"]).Should(Equal([]interface{}{dnsName}))
+		Expect(crtSpec["secretName"]).Should(Equal(testSecretName))
+		Expect(crtSpec["commonName"]).Should(Equal(dnsName))
+		Expect(crtSpec["usages"]).Should(Equal([]interface{}{
+			usageDigitalSignature,
+			usageKeyEncipherment,
+			usageServerAuth,
+			usageClientAuth,
+		}))
+		Expect(crtSpec["revisionHistoryLimit"]).Should(Equal(int64(2)))
+	})
 }
 
 func newDummyHTTPProxy(hpKey client.ObjectKey) *projectcontourv1.HTTPProxy {

diff --git a/docs/usage.md b/docs/usage.md
@@ -21,6 +21,7 @@ If both is specified, command-line flags take precedence.
 | `default-issuer-kind` | `CP_DEFAULT_ISSUER_KIND` | `ClusterIssuer`           | Issuer kind used by default                        |
 | `default-delegated-domain` | `CP_DEFAULT_DELEGATED_DOMAIN` | ""            | Domain to which DNS-01 validation is delegated to   |
 | `allow-custom-delegations` | `CP_ALLOW_CUSTOM_DELEGATIONS` | `false`       | Allow users to specify a custom delegated domain |
+| `csr-revision-limit`  | `CP_CSR_REVISION_LIMIT`  | 0                         | Maximum number of CertificateRequests to be kept for a Certificate. By default, all CertificateRequests are kept             |
 | `leader-election`     | `CP_LEADER_ELECTION`     | `true`                    | Enable / disable leader election                   |
 | `ingress-class-name`  | `CP_INGRESS_CLASS_NAME`  | ""                        | Ingress class name that watched by Contour Plus. If not specified, then all classes are watched    |
 
@@ -125,11 +126,14 @@ contour-plus interprets following annotations for HTTPProxy.
 - `contour-plus.cybozu.com/exclude: "true"` - With this, contour-plus ignores this HTTPProxy.
 - `cert-manager.io/issuer` - The name of an  [Issuer][] to acquire the certificate required for this HTTPProxy from. The Issuer must be in the same namespace as the HTTPProxy.
 - `cert-manager.io/cluster-issuer` - The name of a [ClusterIssuer][Issuer] to acquire the certificate required for this ingress from. It does not matter which namespace your Ingress resides, as ClusterIssuers are non-namespaced resources.
+- `cert-manager.io/revision-history-limit` - The maximum number of CertificateRequests to keep for a given Certificate.
 - `kubernetes.io/tls-acme: "true"` - With this, contour-plus generates Certificate automatically from HTTPProxy.
 - `contour-plus.cybozu.com/delegated-domain: "acme.example.com"` - With this, contour-plus generates a [DNSEndpoint][] to create a CNAME record pointing to the delegation domain for use when performing DNS-01 DCV during the Certificate creation.
 
 If both of `cert-manager.io/issuer` and `cert-manager.io/cluster-issuer` exist, `cluster-issuer` takes precedence.
 
+If `cert-manager.io/revision-history-limit` is present, it takes precedence over the value globally specified via the `--csr-revision-limit` command-line flag.
+
 [Contour]: https://github.com/heptio/contour
 [HTTPProxy]: https://github.com/projectcontour/contour/blob/master/site/docs/master/httpproxy.md
 [DNSEndpoint]: https://github.com/kubernetes-incubator/external-dns/blob/master/docs/contributing/crd-source.md