From b83ba20479e48faf65400193642f6f714386074a Mon Sep 17 00:00:00 2001 From: Jack Henschel Date: Tue, 13 Feb 2024 10:33:05 +0100 Subject: [PATCH 1/2] Use Certificates instead of CertificateRequests As discussed in https://github.com/cert-manager/openshift-routes/issues/49 , using `CertificateRequest` resources for interacting with cert-manager has several drawbacks: - this project (openshift-routes) has to duplicate logic for handling the creation of certificates that already exists in cert-manager. - observability is impaired because cert-manager does not offer metrics for `CertificateRequest` resources, only for `Certificates`. Therefore, this patch replaces the management of `CertificateRequests` with `Certificates` (and `Secrets`). In return, it requires slightly higher privileges because openshift-routes needs to be able to create and read `Certificates`. At the same time, the code complexity and LOC has been reduced. Signed-off-by: Jack Henschel --- README.md | 8 + internal/controller/controller.go | 20 +- internal/controller/sync.go | 446 +++++++++++------------------- 3 files changed, 185 insertions(+), 289 deletions(-) diff --git a/README.md b/README.md index c49a093..ab9d4b9 100644 --- a/README.md +++ b/README.md @@ -143,6 +143,14 @@ After modifying the source code, you can execute the tests with: go test ./... ``` +To run the controller locally, export the location of your kubeconfig file: + +```sh +export KUBECONFIG=$HOME/path/to/kubeconfig +# adjust namespace as necessary +go run internal/cmd/main.go --namespace cert-manager --enable-leader-election=false +``` + # Why is This a Separate Project? We do not wish to support non Kubernetes (or kubernetes-sigs) APIs in cert-manager core. This adds diff --git a/internal/controller/controller.go b/internal/controller/controller.go index 7215706..299fb04 100644 --- a/internal/controller/controller.go +++ b/internal/controller/controller.go @@ -24,8 +24,11 @@ import ( "github.com/go-logr/logr" routev1 "github.com/openshift/api/route/v1" routev1client "github.com/openshift/client-go/route/clientset/versioned" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + corev1client "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/rest" "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/builder" @@ -35,9 +38,10 @@ import ( "github.com/cert-manager/openshift-routes/internal/cmd/app/options" ) -type Route struct { +type RouteController struct { routeClient routev1client.Interface certClient cmclient.Interface + coreClient corev1client.CoreV1Interface eventRecorder record.EventRecorder log logr.Logger @@ -67,7 +71,7 @@ func shouldSync(log logr.Logger, route *routev1.Route) bool { return false } -func (r *Route) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) { +func (r *RouteController) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) { log := r.log.WithValues("object", req.NamespacedName) log.V(5).Info("started reconciling") route, err := r.routeClient.RouteV1().Routes(req.Namespace).Get(ctx, req.Name, metav1.GetOptions{}) @@ -86,7 +90,7 @@ func (r *Route) Reconcile(ctx context.Context, req reconcile.Request) (reconcile return r.sync(ctx, req, route.DeepCopy()) } -func New(base logr.Logger, config *rest.Config, recorder record.EventRecorder) (*Route, error) { +func New(base logr.Logger, config *rest.Config, recorder record.EventRecorder) (*RouteController, error) { routeClient, err := routev1client.NewForConfig(config) if err != nil { return nil, err @@ -95,10 +99,15 @@ func New(base logr.Logger, config *rest.Config, recorder record.EventRecorder) ( if err != nil { return nil, err } + clientset, err := kubernetes.NewForConfig(config) + if err != nil { + return nil, err + } - return &Route{ + return &RouteController{ routeClient: routeClient, certClient: certClient, + coreClient: clientset.CoreV1(), log: base.WithName("route"), eventRecorder: recorder, }, nil @@ -112,6 +121,7 @@ func AddToManager(mgr manager.Manager, opts *options.Options) error { return builder. ControllerManagedBy(mgr). For(&routev1.Route{}). - Owns(&cmapi.CertificateRequest{}). + Owns(&cmapi.Certificate{}). + Owns(&corev1.Secret{}). Complete(controller) } diff --git a/internal/controller/sync.go b/internal/controller/sync.go index 24f76f4..3e12269 100644 --- a/internal/controller/sync.go +++ b/internal/controller/sync.go @@ -1,5 +1,5 @@ /* -Copyright 2022 The cert-manager Authors. +Copyright 2024 The cert-manager Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -19,13 +19,7 @@ package controller import ( "context" "crypto" - "crypto/rand" - "crypto/x509" - "crypto/x509/pkix" - "encoding/pem" "fmt" - "net" - "net/url" "strconv" "strings" "time" @@ -55,7 +49,7 @@ const ( const DefaultCertificateDuration = time.Hour * 24 * 90 // 90 days // sync reconciles an Openshift route. -func (r *Route) sync(ctx context.Context, req reconcile.Request, route *routev1.Route) (reconcile.Result, error) { +func (r *RouteController) sync(ctx context.Context, req reconcile.Request, route *routev1.Route) (reconcile.Result, error) { var result reconcile.Result var err error @@ -74,63 +68,83 @@ func (r *Route) sync(ctx context.Context, req reconcile.Request, route *routev1. log.V(5).Info("route has valid cert") return result, err } - // Do we have a revision? If not set revision to 0 - revision, err := getCurrentRevision(route) - if err != nil { - err = r.setRevision(ctx, route, 0) - log.V(5).Info("generated revision 0") - return result, err - } - // Do we have a next key? - if !r.hasNextPrivateKey(route) { - err = r.generateNextPrivateKey(ctx, route) - log.V(5).Info("generated next private key for route") - return result, err - } - // Is there a CertificateRequest for the Next revision? If not, make it. - hasNext, err := r.hasNextCR(ctx, route, revision) + + // Do we already have a Certificate? If not, make it. + cert, err := r.getCertificateForRoute(ctx, route) if err != nil { return result, err } - if !hasNext { - // generate manifest for new CR - log.V(5).Info("route has no matching certificate request", "revision", revision) - var cr *cmapi.CertificateRequest - cr, err = r.buildNextCR(ctx, route, revision) + if cert == nil { + // generate manifest for new certificate + log.V(5).Info("Route has no matching certificate", req.NamespacedName) + var cert *cmapi.Certificate + cert, err = r.buildNextCert(ctx, route) if err != nil { - log.V(1).Error(err, "error generating certificate request", "object", req.NamespacedName) + log.V(1).Error(err, "error generating certificate", "object", req.NamespacedName) // Not a reconcile error, so don't retry this revision return result, nil } - // create CR and return. We own the CR so it will cause a re-reconcile - _, err = r.certClient.CertmanagerV1().CertificateRequests(route.Namespace).Create(ctx, cr, metav1.CreateOptions{}) + // create the secret that will hold the contents of the certificate + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: cert.Spec.SecretName, + Namespace: route.Namespace, + OwnerReferences: []metav1.OwnerReference{ + *metav1.NewControllerRef( + route, + routev1.GroupVersion.WithKind("Route"), + ), + }, + }, + Type: corev1.SecretTypeTLS, + Data: map[string][]byte{ + // will be filled by cert-manager with the certificate and private key + "tls.crt": []byte{}, + "tls.key": []byte{}, + }, + } + // TODO: what should we do when the secret already exists? by default, cert-manager does not clean up secrets when a certificate is deleted + _, err = r.coreClient.Secrets(route.Namespace).Create(ctx, secret, metav1.CreateOptions{}) if err != nil { return result, err } - r.eventRecorder.Event(route, corev1.EventTypeNormal, ReasonIssuing, "Created new CertificateRequest") + + // create certificate and return. We own the certificate so it will cause a re-reconcile + _, err = r.certClient.CertmanagerV1().Certificates(route.Namespace).Create(ctx, cert, metav1.CreateOptions{}) + if err != nil { + return result, err + } + + r.eventRecorder.Event(route, corev1.EventTypeNormal, ReasonIssuing, "Created new Certificate") return result, nil } - // is the CR Ready and Approved? - ready, cr, err := r.certificateRequestReadyAndApproved(ctx, route, revision) + // is the certificate ready? + ready, cert, err := r.isCertificateReady(ctx, route) if err != nil { return result, err } if !ready { - log.V(5).Info("cr is not ready yet") + log.V(5).Info("Certificate is not ready yet") return result, nil } - // Cert is ready. Populate the route. - err = r.populateRoute(ctx, route, cr, revision) + // Cert is ready. Retrieve the associated secret + secret, err := r.coreClient.Secrets(route.Namespace).Get(ctx, cert.Spec.SecretName, metav1.GetOptions{}) if err != nil { - log.V(1).Error(err, "failed to populate route certificate") return result, err } - log.V(5).Info("populated route cert") + + // Populate the route. + err = r.populateRoute(ctx, route, cert, secret) + if err != nil { + log.V(1).Error(err, "Failed to populate Route from Certificate") + return result, err + } + log.V(5).Info("Populated Route from Cert", route.Name) r.eventRecorder.Event(route, corev1.EventTypeNormal, ReasonIssued, "Route updated with issued certificate") return result, err } -func (r *Route) hasValidCertificate(route *routev1.Route) bool { +func (r *RouteController) hasValidCertificate(route *routev1.Route) bool { // Valid certificate predicates: // TLS config set? @@ -207,147 +221,41 @@ func (r *Route) hasValidCertificate(route *routev1.Route) bool { return true } -func (r *Route) hasNextPrivateKey(route *routev1.Route) bool { - if metav1.HasAnnotation(route.ObjectMeta, cmapi.IsNextPrivateKeySecretLabelKey) { - // Check if the key is valid - _, err := utilpki.DecodePrivateKeyBytes([]byte(route.Annotations[cmapi.IsNextPrivateKeySecretLabelKey])) - if err != nil { - r.eventRecorder.Event(route, corev1.EventTypeWarning, ReasonInvalidKey, "Regenerating Next Private Key as the existing key is invalid: "+err.Error()) - return false - } - return true - } - return false -} - -func (r *Route) generateNextPrivateKey(ctx context.Context, route *routev1.Route) error { - privateKeyAlgorithm, found := route.Annotations[cmapi.PrivateKeyAlgorithmAnnotationKey] - if !found { - privateKeyAlgorithm = string(cmapi.RSAKeyAlgorithm) - } - - var privateKeySize int - privateKeySizeStr, found := route.Annotations[cmapi.PrivateKeySizeAnnotationKey] - if found { - var err error - privateKeySize, err = strconv.Atoi(privateKeySizeStr) - if err != nil { - r.eventRecorder.Event(route, corev1.EventTypeWarning, ReasonInvalidPrivateKeySize, "invalid private key size:"+privateKeySizeStr) - return fmt.Errorf("invalid private key size, %s: %v", privateKeySizeStr, err) - } - } else { - switch privateKeyAlgorithm { - case string(cmapi.ECDSAKeyAlgorithm): - privateKeySize = utilpki.ECCurve256 - case string(cmapi.RSAKeyAlgorithm): - privateKeySize = utilpki.MinRSAKeySize - } - } - - var privateKey crypto.PrivateKey - var err error - switch privateKeyAlgorithm { - case string(cmapi.ECDSAKeyAlgorithm): - privateKey, err = utilpki.GenerateECPrivateKey(privateKeySize) - if err != nil { - return fmt.Errorf("could not generate ECDSA key: %w", err) - } - case string(cmapi.RSAKeyAlgorithm): - privateKey, err = utilpki.GenerateRSAPrivateKey(privateKeySize) - if err != nil { - return fmt.Errorf("could not generate RSA Key: %w", err) - } - default: - r.eventRecorder.Event(route, corev1.EventTypeWarning, ReasonInvalidPrivateKeyAlgorithm, "invalid private key algorithm: "+privateKeyAlgorithm) - return fmt.Errorf("invalid private key algorithm: %s", privateKeyAlgorithm) - } - encodedKey, err := utilpki.EncodePrivateKey(privateKey, cmapi.PrivateKeyEncoding(cmapi.PKCS1)) - if err != nil { - return fmt.Errorf("could not encode %s key: %w", privateKeyAlgorithm, err) - } - route.Annotations[cmapi.IsNextPrivateKeySecretLabelKey] = string(encodedKey) - _, err = r.routeClient.RouteV1().Routes(route.Namespace).Update(ctx, route, metav1.UpdateOptions{}) - if err != nil { - return err - } - r.eventRecorder.Event(route, corev1.EventTypeNormal, ReasonIssuing, "Generated Private Key for route") - return nil -} - -func getCurrentRevision(route *routev1.Route) (int, error) { - revision, found := route.Annotations[cmapi.CertificateRequestRevisionAnnotationKey] - if !found { - return 0, fmt.Errorf("no revision found") - } - return strconv.Atoi(revision) -} - -func (r *Route) setRevision(ctx context.Context, route *routev1.Route, revision int) error { - revisionString := strconv.Itoa(revision) - route.Annotations[cmapi.CertificateRequestRevisionAnnotationKey] = revisionString - _, err := r.routeClient.RouteV1().Routes(route.Namespace).Update(ctx, route, metav1.UpdateOptions{}) - if err != nil { - return err - } - return nil -} +func (r *RouteController) getCertificateForRoute(ctx context.Context, route *routev1.Route) (*cmapi.Certificate, error) { + // Note: this could also implement logic to re-use an existing certificate: route.Annotations[cmapi.CertificateNameKey] -func (r *Route) hasNextCR(ctx context.Context, route *routev1.Route, revision int) (bool, error) { - cr, err := r.findNextCR(ctx, route, revision) - if err != nil { - return false, err - } - if cr != nil { - return true, nil - } - return false, nil -} - -func (r *Route) findNextCR(ctx context.Context, route *routev1.Route, revision int) (*cmapi.CertificateRequest, error) { - // Grab all certificateRequests in this namespace - allCRs, err := r.certClient.CertmanagerV1().CertificateRequests(route.Namespace).List(ctx, metav1.ListOptions{}) + // Grab all Certificates in this namespace + allCerts, err := r.certClient.CertmanagerV1().Certificates(route.Namespace).List(ctx, metav1.ListOptions{}) if err != nil { return nil, err } - var candidates []*cmapi.CertificateRequest - for _, cr := range allCRs.Items { + + var candidates []*cmapi.Certificate + for _, cert := range allCerts.Items { // Beware: The cert-manager generated client re-uses the memory behind the slice next time List is called. // You must copy here to avoid a race condition where the CR contents changes underneath you! - crCandidate := cr.DeepCopy() - for _, owner := range crCandidate.OwnerReferences { + certCandidate := cert.DeepCopy() + for _, owner := range certCandidate.OwnerReferences { if owner.UID == route.UID { - crRevision := crCandidate.Annotations[cmapi.CertificateRequestRevisionAnnotationKey] - crRevisionInt, err := strconv.Atoi(crRevision) - if err != nil { - continue - } - if crRevisionInt == revision+1 { - candidates = append(candidates, crCandidate) - } + candidates = append(candidates, certCandidate) } } } + if len(candidates) == 1 { return candidates[0], nil } + if len(candidates) == 0 { return nil, nil } - return nil, fmt.Errorf("multiple certificateRequests found for this route at revision %d", revision) -} -// buildNextCR generates the manifest of a Certificate Request that is needed for a given Route and revision -// This method expects that the private key has already been generated and added as an annotation on the route -func (r *Route) buildNextCR(ctx context.Context, route *routev1.Route, revision int) (*cmapi.CertificateRequest, error) { - var key crypto.Signer - // get private key from route - k2, err := utilpki.DecodePrivateKeyBytes([]byte(route.Annotations[cmapi.IsNextPrivateKeySecretLabelKey])) - if err != nil { - return nil, err - } - key = k2 + return nil, fmt.Errorf("multiple matching Certificates found for Route %s/%s", route.Namespace, route.Name) +} - // get duration from route +// buildNextCert generates the manifest of a Certificate that is needed for a given Route (based on the annotations) +func (r *RouteController) buildNextCert(ctx context.Context, route *routev1.Route) (*cmapi.Certificate, error) { + // Extract various pieces of information from the Route annotations duration, err := certDurationFromRoute(route) if err != nil { r.log.V(1).Error(err, "the duration annotation is invalid", @@ -357,9 +265,17 @@ func (r *Route) buildNextCR(ctx context.Context, route *routev1.Route, revision return nil, fmt.Errorf("Invalid duration annotation on Route %s/%s", route.Namespace, route.Name) } - privateKeyAlgorithm, found := route.Annotations[cmapi.PrivateKeyAlgorithmAnnotationKey] - if !found { - privateKeyAlgorithm = string(cmapi.RSAKeyAlgorithm) + var privateKeyAlgorithm cmapi.PrivateKeyAlgorithm + privateKeyAlgorithmStr, found := route.Annotations[cmapi.PrivateKeyAlgorithmAnnotationKey] + switch privateKeyAlgorithmStr { + case "RSA": + privateKeyAlgorithm = cmapi.RSAKeyAlgorithm + case "ECDSA": + privateKeyAlgorithm = cmapi.ECDSAKeyAlgorithm + case "Ed25519": + privateKeyAlgorithm = cmapi.Ed25519KeyAlgorithm + default: + privateKeyAlgorithm = cmapi.RSAKeyAlgorithm } var privateKeySize int @@ -372,39 +288,6 @@ func (r *Route) buildNextCR(ctx context.Context, route *routev1.Route, revision } } - var signatureAlgorithm x509.SignatureAlgorithm - var publicKeyAlgorithm x509.PublicKeyAlgorithm - switch privateKeyAlgorithm { - case string(cmapi.ECDSAKeyAlgorithm): - switch privateKeySize { - case 521: - signatureAlgorithm = x509.ECDSAWithSHA512 - case 384: - signatureAlgorithm = x509.ECDSAWithSHA384 - case 256: - signatureAlgorithm = x509.ECDSAWithSHA256 - default: - signatureAlgorithm = x509.ECDSAWithSHA256 - } - publicKeyAlgorithm = x509.ECDSA - case string(cmapi.RSAKeyAlgorithm): - switch { - case privateKeySize >= 4096: - signatureAlgorithm = x509.SHA512WithRSA - case privateKeySize >= 3072: - signatureAlgorithm = x509.SHA384WithRSA - case privateKeySize >= 2048: - signatureAlgorithm = x509.SHA256WithRSA - default: - signatureAlgorithm = x509.SHA256WithRSA - } - publicKeyAlgorithm = x509.RSA - - default: - r.eventRecorder.Event(route, corev1.EventTypeWarning, ReasonInvalidPrivateKeyAlgorithm, "invalid private key algorithm: "+privateKeyAlgorithm) - return nil, fmt.Errorf("invalid private key algorithm, %s", privateKeyAlgorithm) - } - var dnsNames []string // Get the canonical hostname(s) of the Route (from .spec.host or .spec.subdomain) dnsNames = getRouteHostnames(route) @@ -419,28 +302,28 @@ func (r *Route) buildNextCR(ctx context.Context, route *routev1.Route, revision altNames := strings.Split(route.Annotations[cmapi.AltNamesAnnotationKey], ",") dnsNames = append(dnsNames, altNames...) } - var ipSans []net.IP - if metav1.HasAnnotation(route.ObjectMeta, cmapi.IPSANAnnotationKey) { - ipAddresses := strings.Split(route.Annotations[cmapi.IPSANAnnotationKey], ",") - for _, i := range ipAddresses { - ip := net.ParseIP(i) - if ip != nil { - ipSans = append(ipSans, ip) - } - } - } - var uriSans []*url.URL - if metav1.HasAnnotation(route.ObjectMeta, cmapi.URISANAnnotationKey) { - urls := strings.Split(route.Annotations[cmapi.URISANAnnotationKey], ",") - for _, u := range urls { - ur, err := url.Parse(u) - if err != nil { - r.eventRecorder.Event(route, corev1.EventTypeWarning, ReasonInvalidValue, "Ignoring malformed URI SAN "+u) - continue - } - uriSans = append(uriSans, ur) - } - } + // var ipSans []net.IP + // if metav1.HasAnnotation(route.ObjectMeta, cmapi.IPSANAnnotationKey) { + // ipAddresses := strings.Split(route.Annotations[cmapi.IPSANAnnotationKey], ",") + // for _, i := range ipAddresses { + // ip := net.ParseIP(i) + // if ip != nil { + // ipSans = append(ipSans, ip) + // } + // } + // } + // var uriSans []*url.URL + // if metav1.HasAnnotation(route.ObjectMeta, cmapi.URISANAnnotationKey) { + // urls := strings.Split(route.Annotations[cmapi.URISANAnnotationKey], ",") + // for _, u := range urls { + // ur, err := url.Parse(u) + // if err != nil { + // r.eventRecorder.Event(route, corev1.EventTypeWarning, ReasonInvalidValue, "Ignoring malformed URI SAN "+u) + // continue + // } + // uriSans = append(uriSans, ur) + // } + // } var emailAddresses []string if metav1.HasAnnotation(route.ObjectMeta, cmapi.EmailsAnnotationKey) { emailAddresses = strings.Split(route.Annotations[cmapi.EmailsAnnotationKey], ",") @@ -541,38 +424,6 @@ func (r *Route) buildNextCR(ctx context.Context, route *routev1.Route, revision if metav1.HasAnnotation(route.ObjectMeta, cmapi.SubjectSerialNumberAnnotationKey) { serialNumber = route.Annotations[cmapi.SubjectSerialNumberAnnotationKey] } - csr, err := x509.CreateCertificateRequest( - rand.Reader, - &x509.CertificateRequest{ - Version: 0, - SignatureAlgorithm: signatureAlgorithm, - PublicKeyAlgorithm: publicKeyAlgorithm, - Subject: pkix.Name{ - CommonName: route.Annotations[cmapi.CommonNameAnnotationKey], - Country: countries, - Locality: localities, - Organization: organizations, - OrganizationalUnit: organizationalUnits, - PostalCode: postalCodes, - Province: provinces, - SerialNumber: serialNumber, - StreetAddress: streetAddresses, - }, - EmailAddresses: emailAddresses, - DNSNames: dnsNames, - IPAddresses: ipSans, - URIs: uriSans, - }, - key, - ) - if err != nil { - return nil, err - } - csrPEM := pem.EncodeToMemory(&pem.Block{ - Type: "CERTIFICATE REQUEST", - Bytes: csr, - }) - var issuerName string if metav1.HasAnnotation(route.ObjectMeta, cmapi.IngressIssuerNameAnnotationKey) { issuerName = route.Annotations[cmapi.IngressIssuerNameAnnotationKey] @@ -580,11 +431,14 @@ func (r *Route) buildNextCR(ctx context.Context, route *routev1.Route, revision issuerName = route.Annotations[cmapi.IssuerNameAnnotationKey] } - cr := &cmapi.CertificateRequest{ + secretName := route.Name + "-tls-cert" + + // Build the Certificate resource with the collected information + // https://cert-manager.io/docs/usage/certificate/ + cert := &cmapi.Certificate{ ObjectMeta: metav1.ObjectMeta{ GenerateName: route.Name + "-", Namespace: route.Namespace, - Annotations: map[string]string{cmapi.CertificateRequestRevisionAnnotationKey: strconv.Itoa(revision + 1)}, OwnerReferences: []metav1.OwnerReference{ *metav1.NewControllerRef( route, @@ -592,65 +446,87 @@ func (r *Route) buildNextCR(ctx context.Context, route *routev1.Route, revision ), }, }, - Spec: cmapi.CertificateRequestSpec{ - Duration: &metav1.Duration{Duration: duration}, + Spec: cmapi.CertificateSpec{ + SecretName: secretName, + Duration: &metav1.Duration{Duration: duration}, + EmailAddresses: emailAddresses, + // RenewBefore? + // RevisionHistoryLimit? + CommonName: route.Annotations[cmapi.CommonNameAnnotationKey], + Subject: &cmapi.X509Subject{ + Countries: countries, + Localities: localities, + Organizations: organizations, + OrganizationalUnits: organizationalUnits, + PostalCodes: postalCodes, + Provinces: provinces, + SerialNumber: serialNumber, + StreetAddresses: streetAddresses, + }, + PrivateKey: &cmapi.CertificatePrivateKey{ + Algorithm: privateKeyAlgorithm, + Size: privateKeySize, + // RotationPolicy? + }, + DNSNames: dnsNames, + // TODO: + // URIs: uriSans, + // IPAddresses: ipSans, IssuerRef: cmmeta.ObjectReference{ Name: issuerName, Kind: route.Annotations[cmapi.IssuerKindAnnotationKey], Group: route.Annotations[cmapi.IssuerGroupAnnotationKey], }, - Request: csrPEM, - IsCA: false, - Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + IsCA: false, + Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, }, } if route.Spec.TLS != nil && route.Spec.TLS.Termination == routev1.TLSTerminationReencrypt { - cr.Spec.Usages = append(cr.Spec.Usages, cmapi.UsageClientAuth) + cert.Spec.Usages = append(cert.Spec.Usages, cmapi.UsageClientAuth) } - return cr, nil + return cert, nil } -func (r *Route) certificateRequestReadyAndApproved(ctx context.Context, route *routev1.Route, revision int) (bool, *cmapi.CertificateRequest, error) { - cr, err := r.findNextCR(ctx, route, revision) +func (r *RouteController) isCertificateReady(ctx context.Context, route *routev1.Route) (bool, *cmapi.Certificate, error) { + cert, err := r.getCertificateForRoute(ctx, route) if err != nil { return false, nil, err } - if cr == nil { - r.log.Info("BUG: no certificateRequests found, this should never happen") + if cert == nil { + r.log.Info("BUG: no Certificate found, this should never happen") return false, nil, nil } - if cmapiutil.CertificateRequestIsApproved(cr) && - cmapiutil.CertificateRequestHasCondition( - cr, - cmapi.CertificateRequestCondition{ - Type: cmapi.CertificateRequestConditionReady, - Status: cmmeta.ConditionTrue, - }, - ) { - return true, cr, nil + if cmapiutil.CertificateHasCondition( + cert, + cmapi.CertificateCondition{ + Type: cmapi.CertificateConditionReady, + Status: cmmeta.ConditionTrue, + }, + ) { + return true, cert, nil } else { return false, nil, nil } } -func (r *Route) populateRoute(ctx context.Context, route *routev1.Route, cr *cmapi.CertificateRequest, revision int) error { +func (r *RouteController) populateRoute(ctx context.Context, route *routev1.Route, cert *cmapi.Certificate, secret *corev1.Secret) error { // final Sanity checks var key crypto.Signer - // get private key from route - k, err := utilpki.DecodePrivateKeyBytes([]byte(route.Annotations[cmapi.IsNextPrivateKeySecretLabelKey])) + // get private key and signed certificate from Secret + k, err := utilpki.DecodePrivateKeyBytes(secret.Data["tls.key"]) if err != nil { return err } key = k - cert, err := utilpki.DecodeX509CertificateBytes(cr.Status.Certificate) + certificate, err := utilpki.DecodeX509CertificateBytes(secret.Data["tls.crt"]) if err != nil { return err } - matches, err := utilpki.PublicKeyMatchesCertificate(key.Public(), cert) + matches, err := utilpki.PublicKeyMatchesCertificate(key.Public(), certificate) if err != nil { return err } @@ -658,7 +534,6 @@ func (r *Route) populateRoute(ctx context.Context, route *routev1.Route, cr *cma return fmt.Errorf("key does not match certificate (route: %s/%s)", route.Namespace, route.Name) } - route.Annotations[cmapi.CertificateRequestRevisionAnnotationKey] = strconv.Itoa(revision + 1) if route.Spec.TLS == nil { route.Spec.TLS = &routev1.TLSConfig{ Termination: routev1.TLSTerminationEdge, @@ -670,14 +545,17 @@ func (r *Route) populateRoute(ctx context.Context, route *routev1.Route, cr *cma return err } route.Spec.TLS.Key = string(encodedKey) - delete(route.Annotations, cmapi.IsNextPrivateKeySecretLabelKey) - route.Spec.TLS.Certificate = string(cr.Status.Certificate) + encodedCert, err := utilpki.EncodeX509(certificate) + if err != nil { + return err + } + route.Spec.TLS.Certificate = string(encodedCert) _, err = r.routeClient.RouteV1().Routes(route.Namespace).Update(ctx, route, metav1.UpdateOptions{}) return err } -func (r *Route) getRequeueAfterDuration(route *routev1.Route) time.Duration { +func (r *RouteController) getRequeueAfterDuration(route *routev1.Route) time.Duration { cert, err := utilpki.DecodeX509CertificateBytes([]byte(route.Spec.TLS.Certificate)) if err != nil { // Not expecting the cert to be invalid by the time we get here From b55e58748845642d54826b71c5311216322d0e93 Mon Sep 17 00:00:00 2001 From: Ashley Davis Date: Mon, 30 Sep 2024 15:17:45 +0100 Subject: [PATCH 2/2] fix up Certificate integration The main aim here is to improve testing and to add other annotations that weren't added in the open source PR. This is a squashed commit of several: - ensure logr calls have named args to prevent panics - update RBAC to use certs rather than certreqs - fix duration in smoke test, log when polling - don't try to create secret, clean up tests - fix lint error, allow more permissive algorithm specification - change check for CRs to certs - don't Own Secrets since we don't generate them - add support for IP addresses + URIs for certs - add most remaining annotations and improve integration/e2e tests - remove readme notice - remove create secrets permission - ensure deterministic cert name + check names aren't too long - Review comments from Mael: A grammar fix + context on IngressController Signed-off-by: Ashley Davis --- README.md | 8 - .../openshift-routes/templates/rbac.yaml | 12 +- internal/cmd/app/app.go | 12 +- internal/controller/controller.go | 3 +- internal/controller/sync.go | 198 ++- internal/controller/sync_test.go | 1560 +++++++++-------- internal/controller/util.go | 50 + internal/controller/util_test.go | 58 + make/test-smoke.mk | 4 +- test/test-smoke.sh | 84 +- 10 files changed, 1173 insertions(+), 816 deletions(-) create mode 100644 internal/controller/util.go create mode 100644 internal/controller/util_test.go diff --git a/README.md b/README.md index ab9d4b9..c49a093 100644 --- a/README.md +++ b/README.md @@ -143,14 +143,6 @@ After modifying the source code, you can execute the tests with: go test ./... ``` -To run the controller locally, export the location of your kubeconfig file: - -```sh -export KUBECONFIG=$HOME/path/to/kubeconfig -# adjust namespace as necessary -go run internal/cmd/main.go --namespace cert-manager --enable-leader-election=false -``` - # Why is This a Separate Project? We do not wish to support non Kubernetes (or kubernetes-sigs) APIs in cert-manager core. This adds diff --git a/deploy/charts/openshift-routes/templates/rbac.yaml b/deploy/charts/openshift-routes/templates/rbac.yaml index 09fe2d5..b557117 100644 --- a/deploy/charts/openshift-routes/templates/rbac.yaml +++ b/deploy/charts/openshift-routes/templates/rbac.yaml @@ -32,7 +32,7 @@ rules: - apiGroups: - cert-manager.io resources: - - certificaterequests + - certificates verbs: - create - get @@ -41,11 +41,19 @@ rules: - apiGroups: - cert-manager.io resources: - - certificaterequests/status + - certificates/status verbs: - get - list - watch +- apiGroups: + - "" + resources: + - secrets + verbs: + - get + - list + - watch - apiGroups: - "" resources: diff --git a/internal/cmd/app/app.go b/internal/cmd/app/app.go index 3238c54..6a59a73 100644 --- a/internal/cmd/app/app.go +++ b/internal/cmd/app/app.go @@ -69,19 +69,21 @@ func Command() *cobra.Command { return fmt.Errorf("connected to the Kubernetes API, but the Openshift Route v1 CRD does not appear to be installed") } - // Check if v1 cert-manager CertificateRequests exist in the API server - apiServerHasCertificateRequests := false + // Check if v1 cert-manager Certificates exist in the API server + apiServerHasCertificates := false cmResources, err := cl.Discovery().ServerResourcesForGroupVersion("cert-manager.io/v1") if err != nil { return fmt.Errorf("couldn't check if cert-manager.io/v1 exists in the kubernetes API: %w", err) } + for _, r := range cmResources.APIResources { - if r.Kind == "CertificateRequest" { - apiServerHasCertificateRequests = true + if r.Kind == "Certificate" { + apiServerHasCertificates = true break } } - if !apiServerHasCertificateRequests { + + if !apiServerHasCertificates { return fmt.Errorf("connected to the Kubernetes API, but the cert-manager v1 CRDs do not appear to be installed") } diff --git a/internal/controller/controller.go b/internal/controller/controller.go index 299fb04..9407a8c 100644 --- a/internal/controller/controller.go +++ b/internal/controller/controller.go @@ -24,7 +24,6 @@ import ( "github.com/go-logr/logr" routev1 "github.com/openshift/api/route/v1" routev1client "github.com/openshift/client-go/route/clientset/versioned" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" @@ -118,10 +117,10 @@ func AddToManager(mgr manager.Manager, opts *options.Options) error { if err != nil { return err } + return builder. ControllerManagedBy(mgr). For(&routev1.Route{}). Owns(&cmapi.Certificate{}). - Owns(&corev1.Secret{}). Complete(controller) } diff --git a/internal/controller/sync.go b/internal/controller/sync.go index 3e12269..7bcbe00 100644 --- a/internal/controller/sync.go +++ b/internal/controller/sync.go @@ -20,6 +20,8 @@ import ( "context" "crypto" "fmt" + "net" + "net/url" "strconv" "strings" "time" @@ -76,7 +78,8 @@ func (r *RouteController) sync(ctx context.Context, req reconcile.Request, route } if cert == nil { // generate manifest for new certificate - log.V(5).Info("Route has no matching certificate", req.NamespacedName) + log.V(5).Info("Route has no matching certificate", "namespace", req.NamespacedName.Namespace, "name", req.NamespacedName.Name) + var cert *cmapi.Certificate cert, err = r.buildNextCert(ctx, route) if err != nil { @@ -84,30 +87,6 @@ func (r *RouteController) sync(ctx context.Context, req reconcile.Request, route // Not a reconcile error, so don't retry this revision return result, nil } - // create the secret that will hold the contents of the certificate - secret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: cert.Spec.SecretName, - Namespace: route.Namespace, - OwnerReferences: []metav1.OwnerReference{ - *metav1.NewControllerRef( - route, - routev1.GroupVersion.WithKind("Route"), - ), - }, - }, - Type: corev1.SecretTypeTLS, - Data: map[string][]byte{ - // will be filled by cert-manager with the certificate and private key - "tls.crt": []byte{}, - "tls.key": []byte{}, - }, - } - // TODO: what should we do when the secret already exists? by default, cert-manager does not clean up secrets when a certificate is deleted - _, err = r.coreClient.Secrets(route.Namespace).Create(ctx, secret, metav1.CreateOptions{}) - if err != nil { - return result, err - } // create certificate and return. We own the certificate so it will cause a re-reconcile _, err = r.certClient.CertmanagerV1().Certificates(route.Namespace).Create(ctx, cert, metav1.CreateOptions{}) @@ -118,15 +97,18 @@ func (r *RouteController) sync(ctx context.Context, req reconcile.Request, route r.eventRecorder.Event(route, corev1.EventTypeNormal, ReasonIssuing, "Created new Certificate") return result, nil } + // is the certificate ready? ready, cert, err := r.isCertificateReady(ctx, route) if err != nil { return result, err } + if !ready { log.V(5).Info("Certificate is not ready yet") return result, nil } + // Cert is ready. Retrieve the associated secret secret, err := r.coreClient.Secrets(route.Namespace).Get(ctx, cert.Spec.SecretName, metav1.GetOptions{}) if err != nil { @@ -139,9 +121,11 @@ func (r *RouteController) sync(ctx context.Context, req reconcile.Request, route log.V(1).Error(err, "Failed to populate Route from Certificate") return result, err } - log.V(5).Info("Populated Route from Cert", route.Name) + + log.V(5).Info("Populated Route from Cert", "name", route.Name) r.eventRecorder.Event(route, corev1.EventTypeNormal, ReasonIssued, "Route updated with issued certificate") - return result, err + + return result, nil } func (r *RouteController) hasValidCertificate(route *routev1.Route) bool { @@ -255,6 +239,17 @@ func (r *RouteController) getCertificateForRoute(ctx context.Context, route *rou // buildNextCert generates the manifest of a Certificate that is needed for a given Route (based on the annotations) func (r *RouteController) buildNextCert(ctx context.Context, route *routev1.Route) (*cmapi.Certificate, error) { + var issuerName string + if metav1.HasAnnotation(route.ObjectMeta, cmapi.IngressIssuerNameAnnotationKey) { + issuerName = route.Annotations[cmapi.IngressIssuerNameAnnotationKey] + } else { + issuerName = route.Annotations[cmapi.IssuerNameAnnotationKey] + } + + if issuerName == "" { + return nil, fmt.Errorf("missing issuer-name annotation on %s/%s", route.Namespace, route.Name) + } + // Extract various pieces of information from the Route annotations duration, err := certDurationFromRoute(route) if err != nil { @@ -262,19 +257,35 @@ func (r *RouteController) buildNextCert(ctx context.Context, route *routev1.Rout "object", route.Namespace+"/"+route.Name, cmapi.DurationAnnotationKey, route.Annotations[cmapi.DurationAnnotationKey]) r.eventRecorder.Event(route, corev1.EventTypeWarning, ReasonInvalidKey, "annotation "+cmapi.DurationAnnotationKey+": "+route.Annotations[cmapi.DurationAnnotationKey]+" is not a valid duration") - return nil, fmt.Errorf("Invalid duration annotation on Route %s/%s", route.Namespace, route.Name) + return nil, fmt.Errorf("invalid duration annotation on Route %s/%s", route.Namespace, route.Name) + } + + var renewBefore time.Duration + if metav1.HasAnnotation(route.ObjectMeta, cmapi.RenewBeforeAnnotationKey) { + renewBeforeAnnotation := route.Annotations[cmapi.RenewBeforeAnnotationKey] + + var err error + renewBefore, err = time.ParseDuration(renewBeforeAnnotation) + if err != nil { + return nil, fmt.Errorf("invalid renew-before annotation %q on Route %s/%s", renewBeforeAnnotation, route.Namespace, route.Name) + } } var privateKeyAlgorithm cmapi.PrivateKeyAlgorithm - privateKeyAlgorithmStr, found := route.Annotations[cmapi.PrivateKeyAlgorithmAnnotationKey] - switch privateKeyAlgorithmStr { - case "RSA": + privateKeyAlgorithmStrRaw, found := route.Annotations[cmapi.PrivateKeyAlgorithmAnnotationKey] + if !found { + privateKeyAlgorithmStrRaw = "RSA" + } + + switch strings.ToLower(privateKeyAlgorithmStrRaw) { + case "rsa": privateKeyAlgorithm = cmapi.RSAKeyAlgorithm - case "ECDSA": + case "ecdsa": privateKeyAlgorithm = cmapi.ECDSAKeyAlgorithm - case "Ed25519": + case "ed25519": privateKeyAlgorithm = cmapi.Ed25519KeyAlgorithm default: + r.log.V(1).Info("unknown private key algorithm, defaulting to RSA", "algorithm", privateKeyAlgorithmStrRaw) privateKeyAlgorithm = cmapi.RSAKeyAlgorithm } @@ -284,10 +295,19 @@ func (r *RouteController) buildNextCert(ctx context.Context, route *routev1.Rout privateKeySize, err = strconv.Atoi(privateKeySizeStr) if err != nil { r.eventRecorder.Event(route, corev1.EventTypeWarning, ReasonInvalidPrivateKeySize, "invalid private key size:"+privateKeySizeStr) - return nil, fmt.Errorf("invalid private key size, %s: %v", privateKeySizeStr, err) + return nil, fmt.Errorf("invalid private key size annotation %q on %s/%s", privateKeySizeStr, route.Namespace, route.Name) } } + var privateKeyRotationPolicy cmapi.PrivateKeyRotationPolicy + + if metav1.HasAnnotation(route.ObjectMeta, cmapi.PrivateKeyRotationPolicyAnnotationKey) { + // Don't validate the policy here because that would mean we'd need to update this codebase + // if cert-manager adds new values. Just rely on cert-manager validation when the cert is + // created. This is brittle; ideally, cert-manager should expose a function for this. + privateKeyRotationPolicy = cmapi.PrivateKeyRotationPolicy(route.Annotations[cmapi.PrivateKeyRotationPolicyAnnotationKey]) + } + var dnsNames []string // Get the canonical hostname(s) of the Route (from .spec.host or .spec.subdomain) dnsNames = getRouteHostnames(route) @@ -302,32 +322,43 @@ func (r *RouteController) buildNextCert(ctx context.Context, route *routev1.Rout altNames := strings.Split(route.Annotations[cmapi.AltNamesAnnotationKey], ",") dnsNames = append(dnsNames, altNames...) } - // var ipSans []net.IP - // if metav1.HasAnnotation(route.ObjectMeta, cmapi.IPSANAnnotationKey) { - // ipAddresses := strings.Split(route.Annotations[cmapi.IPSANAnnotationKey], ",") - // for _, i := range ipAddresses { - // ip := net.ParseIP(i) - // if ip != nil { - // ipSans = append(ipSans, ip) - // } - // } - // } - // var uriSans []*url.URL - // if metav1.HasAnnotation(route.ObjectMeta, cmapi.URISANAnnotationKey) { - // urls := strings.Split(route.Annotations[cmapi.URISANAnnotationKey], ",") - // for _, u := range urls { - // ur, err := url.Parse(u) - // if err != nil { - // r.eventRecorder.Event(route, corev1.EventTypeWarning, ReasonInvalidValue, "Ignoring malformed URI SAN "+u) - // continue - // } - // uriSans = append(uriSans, ur) - // } - // } + + var ipSANs []string + if metav1.HasAnnotation(route.ObjectMeta, cmapi.IPSANAnnotationKey) { + ipAddresses := strings.Split(route.Annotations[cmapi.IPSANAnnotationKey], ",") + for _, i := range ipAddresses { + ip := net.ParseIP(i) + if ip == nil { + r.eventRecorder.Event(route, corev1.EventTypeWarning, ReasonInvalidValue, fmt.Sprintf("Ignoring unparseable IP SAN %q", i)) + r.log.V(1).Error(nil, "ignoring unparseble IP address on route", "rawIP", i) + continue + } + + ipSANs = append(ipSANs, ip.String()) + } + } + + var uriSANs []string + if metav1.HasAnnotation(route.ObjectMeta, cmapi.URISANAnnotationKey) { + urls := strings.Split(route.Annotations[cmapi.URISANAnnotationKey], ",") + + for _, u := range urls { + ur, err := url.Parse(u) + if err != nil { + r.eventRecorder.Event(route, corev1.EventTypeWarning, ReasonInvalidValue, fmt.Sprintf("Ignoring malformed URI SAN %q", u)) + r.log.V(1).Error(err, "ignoring unparseble URI SAN on route", "uri", u) + continue + } + + uriSANs = append(uriSANs, ur.String()) + } + } + var emailAddresses []string if metav1.HasAnnotation(route.ObjectMeta, cmapi.EmailsAnnotationKey) { emailAddresses = strings.Split(route.Annotations[cmapi.EmailsAnnotationKey], ",") } + var organizations []string if metav1.HasAnnotation(route.ObjectMeta, cmapi.SubjectOrganizationsAnnotationKey) { subjectOrganizations, err := cmutil.SplitWithEscapeCSV(route.Annotations[cmapi.SubjectOrganizationsAnnotationKey]) @@ -341,6 +372,7 @@ func (r *RouteController) buildNextCert(ctx context.Context, route *routev1.Rout return nil, err } } + var organizationalUnits []string if metav1.HasAnnotation(route.ObjectMeta, cmapi.SubjectOrganizationalUnitsAnnotationKey) { subjectOrganizationalUnits, err := cmutil.SplitWithEscapeCSV(route.Annotations[cmapi.SubjectOrganizationalUnitsAnnotationKey]) @@ -355,6 +387,7 @@ func (r *RouteController) buildNextCert(ctx context.Context, route *routev1.Rout } } + var countries []string if metav1.HasAnnotation(route.ObjectMeta, cmapi.SubjectCountriesAnnotationKey) { subjectCountries, err := cmutil.SplitWithEscapeCSV(route.Annotations[cmapi.SubjectCountriesAnnotationKey]) @@ -368,6 +401,7 @@ func (r *RouteController) buildNextCert(ctx context.Context, route *routev1.Rout return nil, err } } + var provinces []string if metav1.HasAnnotation(route.ObjectMeta, cmapi.SubjectProvincesAnnotationKey) { subjectProvinces, err := cmutil.SplitWithEscapeCSV(route.Annotations[cmapi.SubjectProvincesAnnotationKey]) @@ -381,6 +415,7 @@ func (r *RouteController) buildNextCert(ctx context.Context, route *routev1.Rout return nil, err } } + var localities []string if metav1.HasAnnotation(route.ObjectMeta, cmapi.SubjectLocalitiesAnnotationKey) { subjectLocalities, err := cmutil.SplitWithEscapeCSV(route.Annotations[cmapi.SubjectLocalitiesAnnotationKey]) @@ -394,6 +429,7 @@ func (r *RouteController) buildNextCert(ctx context.Context, route *routev1.Rout return nil, err } } + var postalCodes []string if metav1.HasAnnotation(route.ObjectMeta, cmapi.SubjectPostalCodesAnnotationKey) { subjectPostalCodes, err := cmutil.SplitWithEscapeCSV(route.Annotations[cmapi.SubjectPostalCodesAnnotationKey]) @@ -407,6 +443,7 @@ func (r *RouteController) buildNextCert(ctx context.Context, route *routev1.Rout return nil, err } } + var streetAddresses []string if metav1.HasAnnotation(route.ObjectMeta, cmapi.SubjectStreetAddressesAnnotationKey) { subjectStreetAddresses, err := cmutil.SplitWithEscapeCSV(route.Annotations[cmapi.SubjectStreetAddressesAnnotationKey]) @@ -420,25 +457,33 @@ func (r *RouteController) buildNextCert(ctx context.Context, route *routev1.Rout return nil, err } } + var serialNumber string if metav1.HasAnnotation(route.ObjectMeta, cmapi.SubjectSerialNumberAnnotationKey) { serialNumber = route.Annotations[cmapi.SubjectSerialNumberAnnotationKey] } - var issuerName string - if metav1.HasAnnotation(route.ObjectMeta, cmapi.IngressIssuerNameAnnotationKey) { - issuerName = route.Annotations[cmapi.IngressIssuerNameAnnotationKey] - } else { - issuerName = route.Annotations[cmapi.IssuerNameAnnotationKey] + + var revisionHistoryLimit *int32 + if metav1.HasAnnotation(route.ObjectMeta, cmapi.RevisionHistoryLimitAnnotationKey) { + historyLimitRaw := route.Annotations[cmapi.RevisionHistoryLimitAnnotationKey] + + parsedLimit, err := strconv.ParseInt(historyLimitRaw, 10, 32) + if err != nil { + return nil, fmt.Errorf("invalid revision-history-limit annotation %q on %s/%s", historyLimitRaw, route.Namespace, route.Name) + } + + typedLimit := int32(parsedLimit) + revisionHistoryLimit = &typedLimit } - secretName := route.Name + "-tls-cert" + secretName := safeKubernetesNameAppend(route.Name, "tls") // Build the Certificate resource with the collected information // https://cert-manager.io/docs/usage/certificate/ cert := &cmapi.Certificate{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: route.Name + "-", - Namespace: route.Namespace, + Name: safeKubernetesNameAppend(route.Name, "cert"), + Namespace: route.Namespace, OwnerReferences: []metav1.OwnerReference{ *metav1.NewControllerRef( route, @@ -447,12 +492,11 @@ func (r *RouteController) buildNextCert(ctx context.Context, route *routev1.Rout }, }, Spec: cmapi.CertificateSpec{ - SecretName: secretName, - Duration: &metav1.Duration{Duration: duration}, - EmailAddresses: emailAddresses, - // RenewBefore? - // RevisionHistoryLimit? - CommonName: route.Annotations[cmapi.CommonNameAnnotationKey], + SecretName: secretName, + Duration: &metav1.Duration{Duration: duration}, + RenewBefore: &metav1.Duration{Duration: renewBefore}, + RevisionHistoryLimit: revisionHistoryLimit, + CommonName: route.Annotations[cmapi.CommonNameAnnotationKey], Subject: &cmapi.X509Subject{ Countries: countries, Localities: localities, @@ -464,14 +508,14 @@ func (r *RouteController) buildNextCert(ctx context.Context, route *routev1.Rout StreetAddresses: streetAddresses, }, PrivateKey: &cmapi.CertificatePrivateKey{ - Algorithm: privateKeyAlgorithm, - Size: privateKeySize, - // RotationPolicy? + Algorithm: privateKeyAlgorithm, + Size: privateKeySize, + RotationPolicy: privateKeyRotationPolicy, }, - DNSNames: dnsNames, - // TODO: - // URIs: uriSans, - // IPAddresses: ipSans, + EmailAddresses: emailAddresses, + DNSNames: dnsNames, + URIs: uriSANs, + IPAddresses: ipSANs, IssuerRef: cmmeta.ObjectReference{ Name: issuerName, Kind: route.Annotations[cmapi.IssuerKindAnnotationKey], diff --git a/internal/controller/sync_test.go b/internal/controller/sync_test.go index 641bb6a..98a7605 100644 --- a/internal/controller/sync_test.go +++ b/internal/controller/sync_test.go @@ -21,14 +21,11 @@ import ( "crypto/ecdsa" "crypto/elliptic" "crypto/rand" - "crypto/rsa" "crypto/x509" "crypto/x509/pkix" "encoding/pem" "fmt" "math/big" - "net" - "net/url" "sort" "strconv" "testing" @@ -38,7 +35,6 @@ import ( cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" utilpki "github.com/cert-manager/cert-manager/pkg/util/pki" routev1 "github.com/openshift/api/route/v1" - fakeroutev1client "github.com/openshift/client-go/route/clientset/versioned/fake" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" @@ -355,7 +351,7 @@ SOME GARBAGE for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { recorder := record.NewFakeRecorder(100) - r := &Route{ + r := &RouteController{ eventRecorder: recorder, } assert.Equal(t, tt.want, r.hasValidCertificate(tt.route), "hasValidCertificate() return value") @@ -371,340 +367,636 @@ SOME GARBAGE } } -func TestRoute_hasNextPrivateKey(t *testing.T) { - // set up key for test cases - ecdsaKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - require.NoError(t, err) - ecdsaKeyPEM, err := utilpki.EncodePKCS8PrivateKey(ecdsaKey) - require.NoError(t, err) +// Trivial logic that re-implements OpenShift's IngressController behavior. For context, +// the OpenShift IngressController code that deals with this is visible at: +// https://github.com/openshift/router/blob/72114ea/pkg/router/controller/status.go +func generateRouteStatus(route *routev1.Route, admitted bool) *routev1.Route { + var host string + if route.Spec.Host != "" { + host = route.Spec.Host + } + if route.Spec.Subdomain != "" { + host = route.Spec.Subdomain + ".cert-manager.io" // suffix depends on IC config + } + + var admittedStatus = corev1.ConditionTrue + if admitted == false { + admittedStatus = corev1.ConditionFalse + } + + route.Status = routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: host, + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: admittedStatus, + }, + }, + }, + }, + } + return route +} + +func TestRoute_buildNextCertificate(t *testing.T) { + domain := "some-host.some-domain.tld" + domainSlice := []string{domain} + + routeName := "some-route" + certName := routeName + "-cert" + secretName := routeName + "-tls" + + // see util_test.go for details + reallyLongRouteName := "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + reallyLongCertName := "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-03aaf5-cert" + reallyLongSecretName := "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-03aaf5-tls" + tests := []struct { - name string - route *routev1.Route - want bool - wantedEvents []string + name string + route *routev1.Route + want *cmapi.Certificate + wantErr error + wantEvents []string }{ { - name: "route has a private key", - route: &routev1.Route{ + name: "Basic test with duration and hostname", + route: generateRouteStatus(&routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Name: routeName, + Namespace: "some-namespace", Annotations: map[string]string{ - cmapi.IssuerNameAnnotationKey: "some-issuer", - cmapi.IsNextPrivateKeySecretLabelKey: string(ecdsaKeyPEM), + cmapi.IssuerNameAnnotationKey: "self-signed-issuer", + cmapi.DurationAnnotationKey: "42m", + }, + }, + Spec: routev1.RouteSpec{ + Host: domain, + }, + Status: routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: domain, + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "True", + }, + }, + }, }, }, - Spec: routev1.RouteSpec{}, }, - want: true, - wantedEvents: nil, + true), + want: &cmapi.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: certName, + Namespace: "some-namespace", + }, + Spec: cmapi.CertificateSpec{ + Duration: &metav1.Duration{Duration: 42 * time.Minute}, + DNSNames: domainSlice, + IsCA: false, + Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + SecretName: secretName, + }, + }, + wantErr: nil, }, + { - name: "route has no private key", - route: &routev1.Route{ + name: "Basic test with long route name", + route: generateRouteStatus(&routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Name: reallyLongRouteName, + Namespace: "some-namespace", Annotations: map[string]string{ - cmapi.IssuerNameAnnotationKey: "some-issuer", + cmapi.IssuerNameAnnotationKey: "self-signed-issuer", + }, + }, + Spec: routev1.RouteSpec{ + Host: domain, + }, + Status: routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: domain, + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "True", + }, + }, + }, }, }, - Spec: routev1.RouteSpec{}, }, - want: false, - wantedEvents: nil, + true), + want: &cmapi.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: reallyLongCertName, + Namespace: "some-namespace", + }, + Spec: cmapi.CertificateSpec{ + Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + DNSNames: domainSlice, + IsCA: false, + Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + SecretName: reallyLongSecretName, + }, + }, + wantErr: nil, }, + { - name: "route has garbage data in private key", - route: &routev1.Route{ + name: "Basic test with issuer name + kind", + route: generateRouteStatus(&routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Name: routeName, + Namespace: "some-namespace", Annotations: map[string]string{ - cmapi.IssuerNameAnnotationKey: "some-issuer", - cmapi.IsNextPrivateKeySecretLabelKey: `-----BEGIN PRIVATE KEY----- -SOME GARBAGE ------END PRIVATE KEY-----`, + cmapi.IssuerNameAnnotationKey: "self-signed-issuer", + cmapi.IssuerKindAnnotationKey: "SomeIssuer", + }, + }, + Spec: routev1.RouteSpec{ + Host: domain, + }, + Status: routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: domain, + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "True", + }, + }, + }, }, }, - Spec: routev1.RouteSpec{}, }, - want: false, - wantedEvents: []string{"Warning InvalidKey Regenerating Next Private Key as the existing key is invalid: error decoding private key PEM block"}, + true), + want: &cmapi.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: certName, + Namespace: "some-namespace", + }, + Spec: cmapi.CertificateSpec{ + Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + DNSNames: domainSlice, + IsCA: false, + Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + SecretName: secretName, + + IssuerRef: cmmeta.ObjectReference{ + Name: "self-signed-issuer", + Kind: "SomeIssuer", + }, + }, + }, + wantErr: nil, }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - recorder := record.NewFakeRecorder(100) - r := &Route{ - eventRecorder: recorder, - } - assert.Equal(t, tt.want, r.hasNextPrivateKey(tt.route), "hasNextPrivateKey()") - close(recorder.Events) - var gotEvents []string - for e := range recorder.Events { - gotEvents = append(gotEvents, e) - } - sort.Strings(tt.wantedEvents) - sort.Strings(gotEvents) - assert.Equal(t, tt.wantedEvents, gotEvents, "hasNextPrivateKey() events") - }) - } -} -func TestRoute_generateNextPrivateKey(t *testing.T) { - tests := []struct { - name string - route *routev1.Route - want error - wantedEvents []string - wantedPrivateKeyHeader string - }{ { - name: "route without algorithm annotation has no private key", - route: &routev1.Route{ + name: "Basic test with issuer name, kind + group", + route: generateRouteStatus(&routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Name: routeName, + Namespace: "some-namespace", Annotations: map[string]string{ - cmapi.IssuerNameAnnotationKey: "some-issuer", + cmapi.IssuerNameAnnotationKey: "self-signed-issuer", + cmapi.IssuerKindAnnotationKey: "SomeIssuer", + cmapi.IssuerGroupAnnotationKey: "group.example.com", + }, + }, + Spec: routev1.RouteSpec{ + Host: domain, + }, + Status: routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: domain, + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "True", + }, + }, + }, }, }, - Spec: routev1.RouteSpec{}, }, - want: nil, - wantedEvents: []string{"Normal Issuing Generated Private Key for route"}, - wantedPrivateKeyHeader: "BEGIN RSA PRIVATE KEY", + true), + want: &cmapi.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: certName, + Namespace: "some-namespace", + }, + Spec: cmapi.CertificateSpec{ + Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + DNSNames: domainSlice, + IsCA: false, + Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + SecretName: secretName, + + IssuerRef: cmmeta.ObjectReference{ + Name: "self-signed-issuer", + Kind: "SomeIssuer", + Group: "group.example.com", + }, + }, + }, + wantErr: nil, }, + { - name: "route with rsa algorithm annotation has no private key", - route: &routev1.Route{ + name: "Basic test with alternate ingress issuer name annotation", + route: generateRouteStatus(&routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Name: routeName, + Namespace: "some-namespace", Annotations: map[string]string{ - cmapi.IssuerNameAnnotationKey: "some-issuer", - cmapi.PrivateKeyAlgorithmAnnotationKey: "RSA", + cmapi.IngressIssuerNameAnnotationKey: "self-signed-issuer", + cmapi.IssuerKindAnnotationKey: "Issuer", + cmapi.IssuerGroupAnnotationKey: "external-issuer.io", }, }, Spec: routev1.RouteSpec{ - Host: "some-host.some-domain.tld", + Host: domain, + }, + Status: routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: domain, + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "True", + }, + }, + }, + }, }, }, - want: nil, - wantedEvents: []string{"Normal Issuing Generated Private Key for route"}, - wantedPrivateKeyHeader: "BEGIN RSA PRIVATE KEY", + true), + want: &cmapi.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: certName, + Namespace: "some-namespace", + }, + Spec: cmapi.CertificateSpec{ + Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + DNSNames: domainSlice, + IsCA: false, + Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + SecretName: secretName, + + IssuerRef: cmmeta.ObjectReference{ + Name: "self-signed-issuer", + Kind: "Issuer", + Group: "external-issuer.io", + }, + }, + }, + wantErr: nil, }, + { - name: "route with ecdsa algorithm annotation has no private key", + name: "With subdomain and multiple ICs", route: &routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Name: routeName, + Namespace: "some-namespace", Annotations: map[string]string{ - cmapi.IssuerNameAnnotationKey: "some-issuer", - cmapi.PrivateKeyAlgorithmAnnotationKey: "ECDSA", + cmapi.IssuerNameAnnotationKey: "self-signed-issuer", }, }, Spec: routev1.RouteSpec{ - Host: "some-host.some-domain.tld", + Subdomain: "some-sub-domain", + }, + Status: routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: "some-sub-domain.some-domain.tld", // suffix depends on IC config + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "True", + }, + }, + }, + { + Host: "some-sub-domain.some-other-ic.example.com", + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "True", + }, + }, + }, + { + Host: "some-sub-domain.not-admitted.example.com", + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "False", + }, + }, + }, + }, }, }, - want: nil, - wantedEvents: []string{"Normal Issuing Generated Private Key for route"}, - wantedPrivateKeyHeader: "BEGIN EC PRIVATE KEY", + want: &cmapi.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: certName, + Namespace: "some-namespace", + }, + Spec: cmapi.CertificateSpec{ + Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + IsCA: false, + SecretName: secretName, + + DNSNames: []string{ + "some-sub-domain.some-domain.tld", + "some-sub-domain.some-other-ic.example.com", + }, + }, + }, + wantErr: nil, }, + { - name: "route with invalid algorithm annotation has no private key", - route: &routev1.Route{ + name: "With ECDSA private key algorithm annotation", + route: generateRouteStatus(&routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Name: routeName, + Namespace: "some-namespace", Annotations: map[string]string{ - cmapi.IssuerNameAnnotationKey: "some-issuer", - cmapi.PrivateKeyAlgorithmAnnotationKey: "notreal", + cmapi.IssuerNameAnnotationKey: "self-signed-issuer", + cmapi.PrivateKeyAlgorithmAnnotationKey: string(cmapi.ECDSAKeyAlgorithm), }, }, Spec: routev1.RouteSpec{ - Host: "some-host.some-domain.tld", + Host: domain, + }, + Status: routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: domain, + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "True", + }, + }, + }, + }, }, }, - want: fmt.Errorf("invalid private key algorithm: notreal"), - wantedEvents: []string{"Warning InvalidPrivateKeyAlgorithm invalid private key algorithm: notreal"}, - wantedPrivateKeyHeader: "", + true), + want: &cmapi.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: certName, + Namespace: "some-namespace", + }, + Spec: cmapi.CertificateSpec{ + Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + IsCA: false, + DNSNames: domainSlice, + SecretName: secretName, + + PrivateKey: &cmapi.CertificatePrivateKey{ + Algorithm: cmapi.ECDSAKeyAlgorithm, + }, + }, + }, + wantErr: nil, }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - recorder := record.NewFakeRecorder(100) - fakeClient := fakeroutev1client.NewSimpleClientset() - _, err := fakeClient.RouteV1().Routes(tt.route.Namespace).Create(context.TODO(), tt.route, metav1.CreateOptions{}) - assert.NoError(t, err, "fake client returned an error while creating route") - r := &Route{ - eventRecorder: recorder, - routeClient: fakeClient, - } - err = r.generateNextPrivateKey(context.TODO(), tt.route) - assert.Equal(t, tt.want, err, "generateNextPrivateKey()") - close(recorder.Events) - var gotEvents []string - for e := range recorder.Events { - gotEvents = append(gotEvents, e) - } - sort.Strings(tt.wantedEvents) - sort.Strings(gotEvents) - assert.Equal(t, tt.wantedEvents, gotEvents, "hasNextPrivateKey() events") - // If generating the private key failed, there would not be a key to decode/validate - if tt.want == nil { - actualRoute, err := fakeClient.RouteV1().Routes(tt.route.Namespace).Get(context.TODO(), tt.route.Name, metav1.GetOptions{}) - assert.NoError(t, err) - _, err = utilpki.DecodePrivateKeyBytes([]byte(actualRoute.Annotations[cmapi.IsNextPrivateKeySecretLabelKey])) - assert.NoError(t, err) - assert.Contains(t, actualRoute.Annotations[cmapi.IsNextPrivateKeySecretLabelKey], tt.wantedPrivateKeyHeader) - } - }) - } -} -func Test_getCurrentRevision(t *testing.T) { - tests := []struct { - name string - route *routev1.Route - want int - wantErr error - }{ { - name: "route with revision", - route: &routev1.Route{ + name: "With ECDSA P-384 private key algorithm and size annotation", + route: generateRouteStatus(&routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Name: routeName, + Namespace: "some-namespace", Annotations: map[string]string{ - cmapi.IssuerNameAnnotationKey: "some-issuer", - cmapi.CertificateRequestRevisionAnnotationKey: "1337", + cmapi.IssuerNameAnnotationKey: "self-signed-issuer", + cmapi.PrivateKeyAlgorithmAnnotationKey: string(cmapi.ECDSAKeyAlgorithm), + cmapi.PrivateKeySizeAnnotationKey: strconv.Itoa(384), + }, + }, + Spec: routev1.RouteSpec{ + Host: domain, + }, + Status: routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: domain, + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "True", + }, + }, + }, + }, + }, + }, + true), + want: &cmapi.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: certName, + Namespace: "some-namespace", + }, + Spec: cmapi.CertificateSpec{ + Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + IsCA: false, + DNSNames: domainSlice, + SecretName: secretName, + + PrivateKey: &cmapi.CertificatePrivateKey{ + Algorithm: cmapi.ECDSAKeyAlgorithm, + Size: 384, }, }, - Spec: routev1.RouteSpec{}, }, - want: 1337, wantErr: nil, }, + { - name: "route without revision", - route: &routev1.Route{ + name: "With ECDSA P-521 private key algorithm and size annotation", + route: generateRouteStatus(&routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Name: routeName, + Namespace: "some-namespace", Annotations: map[string]string{ - cmapi.IssuerNameAnnotationKey: "some-issuer", + cmapi.IssuerNameAnnotationKey: "self-signed-issuer", + cmapi.PrivateKeyAlgorithmAnnotationKey: string(cmapi.ECDSAKeyAlgorithm), + cmapi.PrivateKeySizeAnnotationKey: strconv.Itoa(521), + }, + }, + Spec: routev1.RouteSpec{ + Host: domain, + }, + Status: routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: domain, + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "True", + }, + }, + }, }, }, - Spec: routev1.RouteSpec{}, }, - want: 0, - wantErr: fmt.Errorf("no revision found"), + true), + want: &cmapi.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: certName, + Namespace: "some-namespace", + }, + Spec: cmapi.CertificateSpec{ + Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + IsCA: false, + DNSNames: domainSlice, + SecretName: secretName, + + PrivateKey: &cmapi.CertificatePrivateKey{ + Algorithm: cmapi.ECDSAKeyAlgorithm, + Size: 521, + }, + }, + }, + wantErr: nil, }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := getCurrentRevision(tt.route) - assert.Equal(t, tt.want, got, "getCurrentRevision()") - assert.Equal(t, tt.wantErr, err, "getCurrentRevision()") - }) - } -} -func TestRoute_setRevision(t *testing.T) { - tests := []struct { - name string - route *routev1.Route - revision int - want string - wantErr error - }{ { - name: "setting revision works", - route: &routev1.Route{ + name: "With RSA private key algorithm annotation", + route: generateRouteStatus(&routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Name: routeName, + Namespace: "some-namespace", Annotations: map[string]string{ - cmapi.IssuerNameAnnotationKey: "some-issuer", + cmapi.IssuerNameAnnotationKey: "self-signed-issuer", + cmapi.PrivateKeyAlgorithmAnnotationKey: string(cmapi.RSAKeyAlgorithm), + }, + }, + Spec: routev1.RouteSpec{ + Host: domain, + }, + Status: routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: domain, + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "True", + }, + }, + }, }, }, - Spec: routev1.RouteSpec{}, }, - revision: 1337, - want: "1337", - wantErr: nil, + true), + want: &cmapi.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: certName, + Namespace: "some-namespace", + }, + Spec: cmapi.CertificateSpec{ + Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + IsCA: false, + DNSNames: domainSlice, + SecretName: secretName, + + PrivateKey: &cmapi.CertificatePrivateKey{ + Algorithm: cmapi.RSAKeyAlgorithm, + }, + }, + }, + wantErr: nil, }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - fakeClient := fakeroutev1client.NewSimpleClientset() - _, err := fakeClient.RouteV1().Routes(tt.route.Namespace).Create(context.TODO(), tt.route, metav1.CreateOptions{}) - assert.NoError(t, err, "fake client returned an error while creating route") - r := &Route{ - routeClient: fakeClient, - } - err = r.setRevision(context.TODO(), tt.route, tt.revision) - assert.Equal(t, tt.wantErr, err, "setRevision()") - actualRoute, err := fakeClient.RouteV1().Routes(tt.route.Namespace).Get(context.TODO(), tt.route.Name, metav1.GetOptions{}) - assert.NoError(t, err) - assert.Equal(t, tt.want, actualRoute.Annotations[cmapi.CertificateRequestRevisionAnnotationKey], "setRevision()") - }) - } -} -func TestRoute_buildNextCR(t *testing.T) { - // set up key for test cases - rsaKey, err := rsa.GenerateKey(rand.Reader, 4096) - require.NoError(t, err) - rsaPEM, err := utilpki.EncodePKCS8PrivateKey(rsaKey) - require.NoError(t, err) - ecdsaKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - require.NoError(t, err) - ecdsaPEM, err := utilpki.EncodePKCS8PrivateKey(ecdsaKey) - require.NoError(t, err) + { + name: "With RSA 3072 private key algorithm and size annotation", + route: generateRouteStatus(&routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: routeName, + Namespace: "some-namespace", + Annotations: map[string]string{ + cmapi.IssuerNameAnnotationKey: "self-signed-issuer", + cmapi.PrivateKeyAlgorithmAnnotationKey: string(cmapi.RSAKeyAlgorithm), + cmapi.PrivateKeySizeAnnotationKey: strconv.Itoa(3072), + }, + }, + Spec: routev1.RouteSpec{ + Host: domain, + }, + Status: routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: domain, + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "True", + }, + }, + }, + }, + }, + }, + true), + want: &cmapi.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: certName, + Namespace: "some-namespace", + }, + Spec: cmapi.CertificateSpec{ + Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + IsCA: false, + DNSNames: domainSlice, + SecretName: secretName, + + PrivateKey: &cmapi.CertificatePrivateKey{ + Algorithm: cmapi.RSAKeyAlgorithm, + Size: 3072, + }, + }, + }, + wantErr: nil, + }, - tests := []struct { - name string - route *routev1.Route - revision int - want *cmapi.CertificateRequest - wantErr error - wantCSR *x509.CertificateRequest - wantEvents []string - }{ { - name: "Basic test with duration and hostname", - revision: 1337, + name: "With Ed25519 private key algorithm and size annotation", route: generateRouteStatus(&routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", + Name: routeName, Namespace: "some-namespace", Annotations: map[string]string{ - cmapi.DurationAnnotationKey: "42m", - cmapi.IsNextPrivateKeySecretLabelKey: string(rsaPEM), + cmapi.IssuerNameAnnotationKey: "self-signed-issuer", + cmapi.PrivateKeyAlgorithmAnnotationKey: string(cmapi.Ed25519KeyAlgorithm), }, }, Spec: routev1.RouteSpec{ - Host: "some-host.some-domain.tld", + Host: domain, }, Status: routev1.RouteStatus{ Ingress: []routev1.RouteIngress{ { - Host: "some-host.some-domain.tld", + Host: domain, Conditions: []routev1.RouteIngressCondition{ { Type: "Admitted", @@ -716,43 +1008,52 @@ func TestRoute_buildNextCR(t *testing.T) { }, }, true), - want: &cmapi.CertificateRequest{ + want: &cmapi.Certificate{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "some-route-", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.CertificateRequestRevisionAnnotationKey: "1338", - }, + Name: certName, + Namespace: "some-namespace", }, - Spec: cmapi.CertificateRequestSpec{ - Duration: &metav1.Duration{Duration: 42 * time.Minute}, - IsCA: false, - Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + Spec: cmapi.CertificateSpec{ + Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + IsCA: false, + DNSNames: domainSlice, + SecretName: secretName, + + PrivateKey: &cmapi.CertificatePrivateKey{ + Algorithm: cmapi.Ed25519KeyAlgorithm, + }, }, }, wantErr: nil, }, + { - name: "Basic test with issuer", - revision: 1337, - route: generateRouteStatus(&routev1.Route{ + name: "With subject annotations", + route: &routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", + Name: routeName, Namespace: "some-namespace", Annotations: map[string]string{ - cmapi.DurationAnnotationKey: "42m", - cmapi.IsNextPrivateKeySecretLabelKey: string(rsaPEM), - cmapi.IssuerNameAnnotationKey: "self-signed-issuer", - cmapi.IssuerKindAnnotationKey: "Issuer", + cmapi.IssuerNameAnnotationKey: "self-signed-issuer", + + cmapi.SubjectOrganizationsAnnotationKey: "Company 1,Company 2", + cmapi.SubjectOrganizationalUnitsAnnotationKey: "Tech Division,Other Division", + cmapi.SubjectCountriesAnnotationKey: "Country 1,Country 2", + cmapi.SubjectProvincesAnnotationKey: "Province 1,Province 2", + cmapi.SubjectStreetAddressesAnnotationKey: "123 Example St,456 Example Ave", + cmapi.SubjectLocalitiesAnnotationKey: "City 1,City 2", + cmapi.SubjectPostalCodesAnnotationKey: "123ABC,456DEF", + cmapi.SubjectSerialNumberAnnotationKey: "10978342379280287615", }, }, Spec: routev1.RouteSpec{ - Host: "some-host.some-domain.tld", + Host: domain, }, Status: routev1.RouteStatus{ Ingress: []routev1.RouteIngress{ { - Host: "some-host.some-domain.tld", + Host: domain, Conditions: []routev1.RouteIngressCondition{ { Type: "Admitted", @@ -763,49 +1064,51 @@ func TestRoute_buildNextCR(t *testing.T) { }, }, }, - true), - want: &cmapi.CertificateRequest{ + want: &cmapi.Certificate{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "some-route-", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.CertificateRequestRevisionAnnotationKey: "1338", - }, + Name: certName, + Namespace: "some-namespace", }, - Spec: cmapi.CertificateRequestSpec{ - Duration: &metav1.Duration{Duration: 42 * time.Minute}, - IsCA: false, - Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, - IssuerRef: cmmeta.ObjectReference{ - Name: "self-signed-issuer", - Kind: "Issuer", + Spec: cmapi.CertificateSpec{ + Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + IsCA: false, + DNSNames: domainSlice, + SecretName: secretName, + + Subject: &cmapi.X509Subject{ + Organizations: []string{"Company 1", "Company 2"}, + OrganizationalUnits: []string{"Tech Division", "Other Division"}, + Countries: []string{"Country 1", "Country 2"}, + Provinces: []string{"Province 1", "Province 2"}, + StreetAddresses: []string{"123 Example St", "456 Example Ave"}, + Localities: []string{"City 1", "City 2"}, + PostalCodes: []string{"123ABC", "456DEF"}, + SerialNumber: "10978342379280287615", }, }, }, wantErr: nil, }, + { - name: "Basic test with external issuer", - revision: 1337, + name: "With custom URI SAN", route: generateRouteStatus(&routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", + Name: routeName, Namespace: "some-namespace", Annotations: map[string]string{ - cmapi.DurationAnnotationKey: "42m", - cmapi.IsNextPrivateKeySecretLabelKey: string(rsaPEM), - cmapi.IssuerKindAnnotationKey: "Issuer", - cmapi.IssuerNameAnnotationKey: "self-signed-issuer", - cmapi.IssuerGroupAnnotationKey: "external-issuer.io", + cmapi.IssuerNameAnnotationKey: "self-signed-issuer", + cmapi.URISANAnnotationKey: "spiffe://example.com/myuri", }, }, Spec: routev1.RouteSpec{ - Host: "some-host.some-domain.tld", + Host: domain, }, Status: routev1.RouteStatus{ Ingress: []routev1.RouteIngress{ { - Host: "some-host.some-domain.tld", + Host: domain, Conditions: []routev1.RouteIngressCondition{ { Type: "Admitted", @@ -817,49 +1120,42 @@ func TestRoute_buildNextCR(t *testing.T) { }, }, true), - want: &cmapi.CertificateRequest{ + want: &cmapi.Certificate{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "some-route-", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.CertificateRequestRevisionAnnotationKey: "1338", - }, + Name: certName, + Namespace: "some-namespace", }, - Spec: cmapi.CertificateRequestSpec{ - Duration: &metav1.Duration{Duration: 42 * time.Minute}, - IsCA: false, - Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, - IssuerRef: cmmeta.ObjectReference{ - Name: "self-signed-issuer", - Kind: "Issuer", - Group: "external-issuer.io", - }, + Spec: cmapi.CertificateSpec{ + Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + DNSNames: domainSlice, + IsCA: false, + Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + SecretName: secretName, + + URIs: []string{"spiffe://example.com/myuri"}, }, }, wantErr: nil, }, + { - name: "Basic test with alternate ingress issuer name annotation", - revision: 1337, + name: "With extra DNS names", route: generateRouteStatus(&routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", + Name: routeName, Namespace: "some-namespace", Annotations: map[string]string{ - cmapi.DurationAnnotationKey: "42m", - cmapi.IsNextPrivateKeySecretLabelKey: string(rsaPEM), - cmapi.IssuerKindAnnotationKey: "Issuer", - cmapi.IngressIssuerNameAnnotationKey: "self-signed-issuer", - cmapi.IssuerGroupAnnotationKey: "external-issuer.io", + cmapi.IssuerNameAnnotationKey: "self-signed-issuer", + cmapi.AltNamesAnnotationKey: "example.com,another.example.com", }, }, Spec: routev1.RouteSpec{ - Host: "some-host.some-domain.tld", + Host: domain, }, Status: routev1.RouteStatus{ Ingress: []routev1.RouteIngress{ { - Host: "some-host.some-domain.tld", + Host: domain, Conditions: []routev1.RouteIngressCondition{ { Type: "Admitted", @@ -871,54 +1167,41 @@ func TestRoute_buildNextCR(t *testing.T) { }, }, true), - want: &cmapi.CertificateRequest{ + want: &cmapi.Certificate{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "some-route-", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.CertificateRequestRevisionAnnotationKey: "1338", - }, + Name: certName, + Namespace: "some-namespace", }, - Spec: cmapi.CertificateRequestSpec{ - Duration: &metav1.Duration{Duration: 42 * time.Minute}, - IsCA: false, - Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, - IssuerRef: cmmeta.ObjectReference{ - Name: "self-signed-issuer", - Kind: "Issuer", - Group: "external-issuer.io", - }, + Spec: cmapi.CertificateSpec{ + Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + IsCA: false, + Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + SecretName: secretName, + + DNSNames: []string{domain, "example.com", "another.example.com"}, }, }, wantErr: nil, }, + { - name: "With subdomain and multiple ICs", - revision: 1337, - route: &routev1.Route{ + name: "With custom IPv4 address", + route: generateRouteStatus(&routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "some-route-with-subdomain", + Name: routeName, Namespace: "some-namespace", Annotations: map[string]string{ - cmapi.IsNextPrivateKeySecretLabelKey: string(rsaPEM), + cmapi.IssuerNameAnnotationKey: "self-signed-issuer", + cmapi.IPSANAnnotationKey: "169.50.50.50", }, }, Spec: routev1.RouteSpec{ - Subdomain: "some-sub-domain", + Host: domain, }, Status: routev1.RouteStatus{ Ingress: []routev1.RouteIngress{ { - Host: "some-sub-domain.some-domain.tld", // suffix depends on IC config - Conditions: []routev1.RouteIngressCondition{ - { - Type: "Admitted", - Status: "True", - }, - }, - }, - { - Host: "some-sub-domain.some-other-ic.example.com", + Host: domain, Conditions: []routev1.RouteIngressCondition{ { Type: "Admitted", @@ -926,62 +1209,46 @@ func TestRoute_buildNextCR(t *testing.T) { }, }, }, - { - Host: "some-sub-domain.not-admitted.example.com", - Conditions: []routev1.RouteIngressCondition{ - { - Type: "Admitted", - Status: "False", - }, - }, - }, }, }, }, - want: &cmapi.CertificateRequest{ + true), + want: &cmapi.Certificate{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "some-route-with-subdomain-", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.CertificateRequestRevisionAnnotationKey: "1338", - }, + Name: certName, + Namespace: "some-namespace", }, - Spec: cmapi.CertificateRequestSpec{ - Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, - Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + Spec: cmapi.CertificateSpec{ + Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + DNSNames: domainSlice, + IsCA: false, + Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + SecretName: secretName, + + IPAddresses: []string{"169.50.50.50"}, }, }, - wantCSR: &x509.CertificateRequest{ - SignatureAlgorithm: x509.SHA256WithRSA, - PublicKeyAlgorithm: x509.RSA, - Subject: pkix.Name{ - CommonName: "", - }, - DNSNames: []string{"some-sub-domain.some-domain.tld", "some-sub-domain.some-other-ic.example.com"}, - IPAddresses: []net.IP(nil), - URIs: []*url.URL(nil), - }, wantErr: nil, }, + { - name: "With ECDSA private key algorithm annotation", - revision: 1337, + name: "With custom IPv6 address", route: generateRouteStatus(&routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", + Name: routeName, Namespace: "some-namespace", Annotations: map[string]string{ - cmapi.IsNextPrivateKeySecretLabelKey: string(ecdsaPEM), - cmapi.PrivateKeyAlgorithmAnnotationKey: string(cmapi.ECDSAKeyAlgorithm), + cmapi.IssuerNameAnnotationKey: "self-signed-issuer", + cmapi.IPSANAnnotationKey: "2a02:ec80:300:ed1a::1", }, }, Spec: routev1.RouteSpec{ - Host: "some-host.some-domain.tld", + Host: domain, }, Status: routev1.RouteStatus{ Ingress: []routev1.RouteIngress{ { - Host: "some-host.some-domain.tld", + Host: domain, Conditions: []routev1.RouteIngressCondition{ { Type: "Admitted", @@ -993,51 +1260,42 @@ func TestRoute_buildNextCR(t *testing.T) { }, }, true), - want: &cmapi.CertificateRequest{ + want: &cmapi.Certificate{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "some-route-", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.CertificateRequestRevisionAnnotationKey: "1338", - }, + Name: certName, + Namespace: "some-namespace", }, - Spec: cmapi.CertificateRequestSpec{ - Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, - Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + Spec: cmapi.CertificateSpec{ + Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + DNSNames: domainSlice, + IsCA: false, + Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + SecretName: secretName, + + IPAddresses: []string{"2a02:ec80:300:ed1a::1"}, }, }, - wantCSR: &x509.CertificateRequest{ - SignatureAlgorithm: x509.ECDSAWithSHA256, - PublicKeyAlgorithm: x509.ECDSA, - Subject: pkix.Name{ - CommonName: "", - }, - DNSNames: []string{"some-host.some-domain.tld"}, - IPAddresses: []net.IP(nil), - URIs: []*url.URL(nil), - }, wantErr: nil, }, + { - name: "With ECDSA 384 private key algorithm and size annotation", - revision: 1337, + name: "With custom mixed IP addresses", route: generateRouteStatus(&routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", + Name: routeName, Namespace: "some-namespace", Annotations: map[string]string{ - cmapi.IsNextPrivateKeySecretLabelKey: string(ecdsaPEM), - cmapi.PrivateKeyAlgorithmAnnotationKey: string(cmapi.ECDSAKeyAlgorithm), - cmapi.PrivateKeySizeAnnotationKey: strconv.Itoa(384), + cmapi.IssuerNameAnnotationKey: "self-signed-issuer", + cmapi.IPSANAnnotationKey: "169.50.50.50,2a02:ec80:300:ed1a::1,::ffff:192.168.0.1", }, }, Spec: routev1.RouteSpec{ - Host: "some-host.some-domain.tld", + Host: domain, }, Status: routev1.RouteStatus{ Ingress: []routev1.RouteIngress{ { - Host: "some-host.some-domain.tld", + Host: domain, Conditions: []routev1.RouteIngressCondition{ { Type: "Admitted", @@ -1049,51 +1307,42 @@ func TestRoute_buildNextCR(t *testing.T) { }, }, true), - want: &cmapi.CertificateRequest{ + want: &cmapi.Certificate{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "some-route-", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.CertificateRequestRevisionAnnotationKey: "1338", - }, + Name: certName, + Namespace: "some-namespace", }, - Spec: cmapi.CertificateRequestSpec{ - Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, - Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + Spec: cmapi.CertificateSpec{ + Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + DNSNames: domainSlice, + IsCA: false, + Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + SecretName: secretName, + + IPAddresses: []string{"169.50.50.50", "2a02:ec80:300:ed1a::1", "192.168.0.1"}, }, }, - wantCSR: &x509.CertificateRequest{ - SignatureAlgorithm: x509.ECDSAWithSHA256, - PublicKeyAlgorithm: x509.ECDSA, - Subject: pkix.Name{ - CommonName: "", - }, - DNSNames: []string{"some-host.some-domain.tld"}, - IPAddresses: []net.IP(nil), - URIs: []*url.URL(nil), - }, wantErr: nil, }, + { - name: "With ECDSA 521 private key algorithm and size annotation", - revision: 1337, + name: "With custom emails", route: generateRouteStatus(&routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", + Name: routeName, Namespace: "some-namespace", Annotations: map[string]string{ - cmapi.IsNextPrivateKeySecretLabelKey: string(ecdsaPEM), - cmapi.PrivateKeyAlgorithmAnnotationKey: string(cmapi.ECDSAKeyAlgorithm), - cmapi.PrivateKeySizeAnnotationKey: strconv.Itoa(521), + cmapi.IssuerNameAnnotationKey: "self-signed-issuer", + cmapi.EmailsAnnotationKey: "test@example.com,hello@example.com", }, }, Spec: routev1.RouteSpec{ - Host: "some-host.some-domain.tld", + Host: domain, }, Status: routev1.RouteStatus{ Ingress: []routev1.RouteIngress{ { - Host: "some-host.some-domain.tld", + Host: domain, Conditions: []routev1.RouteIngressCondition{ { Type: "Admitted", @@ -1105,50 +1354,46 @@ func TestRoute_buildNextCR(t *testing.T) { }, }, true), - want: &cmapi.CertificateRequest{ + want: &cmapi.Certificate{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "some-route-", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.CertificateRequestRevisionAnnotationKey: "1338", - }, + Name: certName, + Namespace: "some-namespace", }, - Spec: cmapi.CertificateRequestSpec{ - Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, - Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + Spec: cmapi.CertificateSpec{ + Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + DNSNames: domainSlice, + IsCA: false, + Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + SecretName: secretName, + + EmailAddresses: []string{"test@example.com", "hello@example.com"}, }, }, - wantCSR: &x509.CertificateRequest{ - SignatureAlgorithm: x509.ECDSAWithSHA256, - PublicKeyAlgorithm: x509.ECDSA, - Subject: pkix.Name{ - CommonName: "", - }, - DNSNames: []string{"some-host.some-domain.tld"}, - IPAddresses: []net.IP(nil), - URIs: []*url.URL(nil), - }, wantErr: nil, }, + { - name: "With RSA private key algorithm annotation", - revision: 1337, + name: "With all SAN fields", route: generateRouteStatus(&routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", + Name: routeName, Namespace: "some-namespace", Annotations: map[string]string{ - cmapi.IsNextPrivateKeySecretLabelKey: string(rsaPEM), - cmapi.PrivateKeyAlgorithmAnnotationKey: string(cmapi.RSAKeyAlgorithm), + cmapi.IssuerNameAnnotationKey: "self-signed-issuer", + + cmapi.AltNamesAnnotationKey: "example.com,another.example.com", + cmapi.URISANAnnotationKey: "spiffe://example.com/myuri", + cmapi.IPSANAnnotationKey: "169.50.50.50,2a02:ec80:300:ed1a::1,::ffff:192.168.0.1", + cmapi.EmailsAnnotationKey: "test@example.com,hello@example.com", }, }, Spec: routev1.RouteSpec{ - Host: "some-host.some-domain.tld", + Host: domain, }, Status: routev1.RouteStatus{ Ingress: []routev1.RouteIngress{ { - Host: "some-host.some-domain.tld", + Host: domain, Conditions: []routev1.RouteIngressCondition{ { Type: "Admitted", @@ -1160,51 +1405,44 @@ func TestRoute_buildNextCR(t *testing.T) { }, }, true), - want: &cmapi.CertificateRequest{ + want: &cmapi.Certificate{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "some-route-", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.CertificateRequestRevisionAnnotationKey: "1338", - }, + Name: certName, + Namespace: "some-namespace", }, - Spec: cmapi.CertificateRequestSpec{ - Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, - Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + Spec: cmapi.CertificateSpec{ + Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + IsCA: false, + Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + SecretName: secretName, + + DNSNames: []string{domain, "example.com", "another.example.com"}, + URIs: []string{"spiffe://example.com/myuri"}, + IPAddresses: []string{"169.50.50.50", "2a02:ec80:300:ed1a::1", "192.168.0.1"}, + EmailAddresses: []string{"test@example.com", "hello@example.com"}, }, }, - wantCSR: &x509.CertificateRequest{ - SignatureAlgorithm: x509.SHA256WithRSA, - PublicKeyAlgorithm: x509.RSA, - Subject: pkix.Name{ - CommonName: "", - }, - DNSNames: []string{"some-host.some-domain.tld"}, - IPAddresses: []net.IP(nil), - URIs: []*url.URL(nil), - }, wantErr: nil, }, + { - name: "With RSA 3072 private key algorithm and size annotation", - revision: 1337, + name: "With custom renewBefore", route: generateRouteStatus(&routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", + Name: routeName, Namespace: "some-namespace", Annotations: map[string]string{ - cmapi.IsNextPrivateKeySecretLabelKey: string(rsaPEM), - cmapi.PrivateKeyAlgorithmAnnotationKey: string(cmapi.RSAKeyAlgorithm), - cmapi.PrivateKeySizeAnnotationKey: strconv.Itoa(3072), + cmapi.IssuerNameAnnotationKey: "self-signed-issuer", + cmapi.RenewBeforeAnnotationKey: "30m", }, }, Spec: routev1.RouteSpec{ - Host: "some-host.some-domain.tld", + Host: domain, }, Status: routev1.RouteStatus{ Ingress: []routev1.RouteIngress{ { - Host: "some-host.some-domain.tld", + Host: domain, Conditions: []routev1.RouteIngressCondition{ { Type: "Admitted", @@ -1216,51 +1454,41 @@ func TestRoute_buildNextCR(t *testing.T) { }, }, true), - want: &cmapi.CertificateRequest{ + want: &cmapi.Certificate{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "some-route-", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.CertificateRequestRevisionAnnotationKey: "1338", - }, + Name: certName, + Namespace: "some-namespace", }, - Spec: cmapi.CertificateRequestSpec{ - Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, - Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + Spec: cmapi.CertificateSpec{ + Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + DNSNames: domainSlice, + IsCA: false, + Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + SecretName: secretName, + + RenewBefore: &metav1.Duration{Duration: 30 * time.Minute}, }, }, - wantCSR: &x509.CertificateRequest{ - SignatureAlgorithm: x509.SHA384WithRSA, - PublicKeyAlgorithm: x509.RSA, - Subject: pkix.Name{ - CommonName: "", - }, - DNSNames: []string{"some-host.some-domain.tld"}, - IPAddresses: []net.IP(nil), - URIs: []*url.URL(nil), - }, wantErr: nil, }, + { - name: "With RSA 3072 private key algorithm and size annotation", - revision: 1337, + name: "missing issuer-name is an error", route: generateRouteStatus(&routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", + Name: routeName, Namespace: "some-namespace", Annotations: map[string]string{ - cmapi.IsNextPrivateKeySecretLabelKey: string(rsaPEM), - cmapi.PrivateKeyAlgorithmAnnotationKey: string(cmapi.RSAKeyAlgorithm), - cmapi.PrivateKeySizeAnnotationKey: strconv.Itoa(4096), + cmapi.RenewBeforeAnnotationKey: "30m", }, }, Spec: routev1.RouteSpec{ - Host: "some-host.some-domain.tld", + Host: domain, }, Status: routev1.RouteStatus{ Ingress: []routev1.RouteIngress{ { - Host: "some-host.some-domain.tld", + Host: domain, Conditions: []routev1.RouteIngressCondition{ { Type: "Admitted", @@ -1272,57 +1500,28 @@ func TestRoute_buildNextCR(t *testing.T) { }, }, true), - want: &cmapi.CertificateRequest{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "some-route-", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.CertificateRequestRevisionAnnotationKey: "1338", - }, - }, - Spec: cmapi.CertificateRequestSpec{ - Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, - Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, - }, - }, - wantCSR: &x509.CertificateRequest{ - SignatureAlgorithm: x509.SHA512WithRSA, - PublicKeyAlgorithm: x509.RSA, - Subject: pkix.Name{ - CommonName: "", - }, - DNSNames: []string{"some-host.some-domain.tld"}, - IPAddresses: []net.IP(nil), - URIs: []*url.URL(nil), - }, - wantErr: nil, + want: nil, + wantErr: fmt.Errorf("missing issuer-name annotation on some-namespace/some-route"), }, + { - name: "With subject annotations", - revision: 1337, - route: &routev1.Route{ + name: "invalid duration is an error", + route: generateRouteStatus(&routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "some-route-with-subject-annotations", + Name: routeName, Namespace: "some-namespace", Annotations: map[string]string{ - cmapi.IsNextPrivateKeySecretLabelKey: string(rsaPEM), - cmapi.SubjectOrganizationsAnnotationKey: "Company 1,Company 2", - cmapi.SubjectOrganizationalUnitsAnnotationKey: "Tech Division,Other Division", - cmapi.SubjectCountriesAnnotationKey: "Country 1,Country 2", - cmapi.SubjectProvincesAnnotationKey: "Province 1,Province 2", - cmapi.SubjectStreetAddressesAnnotationKey: "123 Example St,456 Example Ave", - cmapi.SubjectLocalitiesAnnotationKey: "City 1,City 2", - cmapi.SubjectPostalCodesAnnotationKey: "123ABC,456DEF", - cmapi.SubjectSerialNumberAnnotationKey: "10978342379280287615", + cmapi.IssuerNameAnnotationKey: "self-signed-issuer", + cmapi.DurationAnnotationKey: "not-a-time", }, }, Spec: routev1.RouteSpec{ - Host: "example-route.example.com", + Host: domain, }, Status: routev1.RouteStatus{ Ingress: []routev1.RouteIngress{ { - Host: "example-route.example.com", + Host: domain, Conditions: []routev1.RouteIngressCondition{ { Type: "Admitted", @@ -1333,71 +1532,62 @@ func TestRoute_buildNextCR(t *testing.T) { }, }, }, - want: &cmapi.CertificateRequest{ + true), + want: nil, + wantErr: fmt.Errorf("invalid duration annotation on Route %s/%s", "some-namespace", "some-route"), + }, + + { + name: "invalid renew-before is an error", + route: generateRouteStatus(&routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "some-route-with-subject-annotations-", - Namespace: "some-namespace", + Name: routeName, + Namespace: "some-namespace", Annotations: map[string]string{ - cmapi.CertificateRequestRevisionAnnotationKey: "1338", + cmapi.IssuerNameAnnotationKey: "self-signed-issuer", + cmapi.RenewBeforeAnnotationKey: "not-a-time", }, }, - Spec: cmapi.CertificateRequestSpec{ - Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, - Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + Spec: routev1.RouteSpec{ + Host: domain, + }, + Status: routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: domain, + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "True", + }, + }, + }, + }, }, }, - wantCSR: &x509.CertificateRequest{ - SignatureAlgorithm: x509.SHA256WithRSA, - PublicKeyAlgorithm: x509.RSA, - Subject: pkix.Name{ - CommonName: "", - Organization: []string{"Company 1", "Company 2"}, - OrganizationalUnit: []string{"Tech Division", "Other Division"}, - Country: []string{"Country 1", "Country 2"}, - Province: []string{"Province 1", "Province 2"}, - Locality: []string{"City 1", "City 2"}, - PostalCode: []string{"123ABC", "456DEF"}, - StreetAddress: []string{"123 Example St", "456 Example Ave"}, - SerialNumber: "10978342379280287615", - }, - DNSNames: []string{"example-route.example.com"}, - IPAddresses: []net.IP{}, - URIs: []*url.URL{}, - }, - wantErr: nil, + true), + want: nil, + wantErr: fmt.Errorf("invalid renew-before annotation %q on Route %s/%s", "not-a-time", "some-namespace", "some-route"), }, + { - name: "With all annotations", - revision: 1337, - route: &routev1.Route{ + name: "invalid private key size is an error", + route: generateRouteStatus(&routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "some-route-with-all-annotations", + Name: routeName, Namespace: "some-namespace", Annotations: map[string]string{ - cmapi.IsNextPrivateKeySecretLabelKey: string(rsaPEM), - cmapi.DurationAnnotationKey: "720h", - cmapi.IPSANAnnotationKey: "10.20.30.40,192.168.192.168", - cmapi.AltNamesAnnotationKey: "mycooldomain.com,mysecondarydomain.com", - cmapi.URISANAnnotationKey: "spiffe://trustdomain/workload", - cmapi.CommonNameAnnotationKey: "mycommonname.com", - cmapi.EmailsAnnotationKey: "email@example.com", - cmapi.SubjectOrganizationsAnnotationKey: "Company 1,Company 2", - cmapi.SubjectOrganizationalUnitsAnnotationKey: "Tech Division,Other Division", - cmapi.SubjectCountriesAnnotationKey: "Country 1,Country 2", - cmapi.SubjectProvincesAnnotationKey: "Province 1,Province 2", - cmapi.SubjectStreetAddressesAnnotationKey: "123 Example St,456 Example Ave", - cmapi.SubjectLocalitiesAnnotationKey: "City 1,City 2", - cmapi.SubjectPostalCodesAnnotationKey: "123ABC,456DEF", - cmapi.SubjectSerialNumberAnnotationKey: "10978342379280287615", + cmapi.IssuerNameAnnotationKey: "self-signed-issuer", + cmapi.PrivateKeySizeAnnotationKey: "not-a-number", }, }, Spec: routev1.RouteSpec{ - Host: "example-route.example.com", + Host: domain, }, Status: routev1.RouteStatus{ Ingress: []routev1.RouteIngress{ { - Host: "example-route.example.com", + Host: domain, Conditions: []routev1.RouteIngressCondition{ { Type: "Admitted", @@ -1408,138 +1598,96 @@ func TestRoute_buildNextCR(t *testing.T) { }, }, }, - want: &cmapi.CertificateRequest{ + true), + want: nil, + wantErr: fmt.Errorf("invalid private key size annotation %q on %s/%s", "not-a-number", "some-namespace", "some-route"), + }, + + { + name: "invalid revision history limit is an error", + route: generateRouteStatus(&routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "some-route-with-all-annotations-", - Namespace: "some-namespace", + Name: routeName, + Namespace: "some-namespace", Annotations: map[string]string{ - cmapi.CertificateRequestRevisionAnnotationKey: "1338", + cmapi.IssuerNameAnnotationKey: "self-signed-issuer", + cmapi.RevisionHistoryLimitAnnotationKey: "not-a-number", }, }, - Spec: cmapi.CertificateRequestSpec{ - Duration: &metav1.Duration{Duration: time.Hour * 24 * 30}, - Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + Spec: routev1.RouteSpec{ + Host: domain, + }, + Status: routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: domain, + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "True", + }, + }, + }, + }, }, }, - wantCSR: &x509.CertificateRequest{ - SignatureAlgorithm: x509.SHA256WithRSA, - PublicKeyAlgorithm: x509.RSA, - Subject: pkix.Name{ - CommonName: "mycommonname.com", - Organization: []string{"Company 1", "Company 2"}, - OrganizationalUnit: []string{"Tech Division", "Other Division"}, - Country: []string{"Country 1", "Country 2"}, - Province: []string{"Province 1", "Province 2"}, - Locality: []string{"City 1", "City 2"}, - PostalCode: []string{"123ABC", "456DEF"}, - StreetAddress: []string{"123 Example St", "456 Example Ave"}, - SerialNumber: "10978342379280287615", - }, - DNSNames: []string{"example-route.example.com", "mycooldomain.com", "mysecondarydomain.com"}, - IPAddresses: []net.IP{net.IPv4(10, 20, 30, 40), net.IPv4(192, 168, 192, 168)}, - URIs: []*url.URL{{Scheme: "spiffe", Host: "trustdomain", Path: "workload"}}, - EmailAddresses: []string{"email@example.com"}, - }, - wantErr: nil, + true), + want: nil, + wantErr: fmt.Errorf("invalid revision-history-limit annotation %q on %s/%s", "not-a-number", "some-namespace", "some-route"), }, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { recorder := record.NewFakeRecorder(100) - r := &Route{ + r := &RouteController{ eventRecorder: recorder, } + // test "buildNextCR" function - cr, err := r.buildNextCR(context.TODO(), tt.route, tt.revision) + cert, err := r.buildNextCert(context.TODO(), tt.route) // check that we got the expected error (including nil) - assert.Equal(t, tt.wantErr, err, "buildNextCR()") + assert.Equal(t, tt.wantErr, err, "buildNextCert()") - // check that the returned object is as expected - assert.Equal(t, tt.want.ObjectMeta.GenerateName, cr.ObjectMeta.GenerateName) - assert.Equal(t, tt.want.ObjectMeta.Namespace, cr.ObjectMeta.Namespace) - assert.Equal(t, tt.want.ObjectMeta.Annotations, cr.ObjectMeta.Annotations) - assert.Equal(t, tt.want.ObjectMeta.Labels, cr.ObjectMeta.Labels) - assert.Equal(t, tt.want.Spec.Duration, cr.Spec.Duration) - assert.Equal(t, tt.want.Spec.IsCA, cr.Spec.IsCA) - assert.Equal(t, tt.want.Spec.Usages, cr.Spec.Usages) - assert.Equal(t, tt.want.Spec.IssuerRef, cr.Spec.IssuerRef) - - // check the CSR - if tt.wantCSR != nil { - var privateKey any - if tt.wantCSR.PublicKeyAlgorithm == x509.ECDSA { - privateKey = ecdsaKey - } else if tt.wantCSR.PublicKeyAlgorithm == x509.RSA { - privateKey = rsaKey - } - csr, err := x509.CreateCertificateRequest(rand.Reader, tt.wantCSR, privateKey) - assert.NoError(t, err) - - if tt.wantCSR.PublicKeyAlgorithm == x509.ECDSA { - // The signature for a ECDSA CSR varies based on a random number, therefore we can not expect - // the CSR to be identical like we can for RSA. Instead, compare the CSR excluding the signature. - parsedCSR, err := x509.ParseCertificateRequest(csr) - assert.NoError(t, err) - assert.Equal(t, tt.wantCSR.DNSNames, parsedCSR.DNSNames) - assert.Equal(t, tt.wantCSR.IPAddresses, parsedCSR.IPAddresses) - assert.Equal(t, tt.wantCSR.PublicKeyAlgorithm, parsedCSR.PublicKeyAlgorithm) - assert.Equal(t, tt.wantCSR.SignatureAlgorithm, parsedCSR.SignatureAlgorithm) - assert.Equal(t, tt.wantCSR.Subject.CommonName, parsedCSR.Subject.CommonName) - assert.Equal(t, tt.wantCSR.URIs, parsedCSR.URIs) - - } else if tt.wantCSR.PublicKeyAlgorithm == x509.RSA { - csrPEM := pem.EncodeToMemory(&pem.Block{ - Type: "CERTIFICATE REQUEST", - Bytes: csr, - }) - assert.Equal(t, cr.Spec.Request, csrPEM) - } + if tt.wantErr != nil || err != nil { + return } - // check the events that were generated - close(recorder.Events) - if len(tt.wantEvents) > 0 { - var gotEvents []string - for e := range recorder.Events { - gotEvents = append(gotEvents, e) - } - sort.Strings(tt.wantEvents) - sort.Strings(gotEvents) - assert.Equal(t, tt.wantEvents, gotEvents, "buildNextCR() events") + // check that the returned object is as expected + + if tt.want.Spec.IssuerRef.Name != "" { + // only check issuerRef if it was specified on want; this saves copying lots + // of issuerRefs around + assert.Equal(t, tt.want.Spec.IssuerRef, cert.Spec.IssuerRef) } - }) - } -} + assert.Equal(t, tt.want.ObjectMeta.GenerateName, cert.ObjectMeta.GenerateName) + assert.Equal(t, tt.want.ObjectMeta.Namespace, cert.ObjectMeta.Namespace) + assert.Equal(t, tt.want.ObjectMeta.Annotations, cert.ObjectMeta.Annotations) + assert.Equal(t, tt.want.ObjectMeta.Labels, cert.ObjectMeta.Labels) + assert.Equal(t, tt.want.Spec.Duration, cert.Spec.Duration) + assert.Equal(t, tt.want.Spec.IsCA, cert.Spec.IsCA) + assert.Equal(t, tt.want.Spec.Usages, cert.Spec.Usages) + assert.Equal(t, tt.want.Spec.DNSNames, cert.Spec.DNSNames) + assert.Equal(t, tt.want.Spec.EmailAddresses, cert.Spec.EmailAddresses) + assert.Equal(t, tt.want.Spec.IPAddresses, cert.Spec.IPAddresses) + assert.Equal(t, tt.want.Spec.URIs, cert.Spec.URIs) + assert.Equal(t, tt.want.Spec.SecretName, cert.Spec.SecretName) -// trivial logic that re-implements OpenShift's IngressController behavior -func generateRouteStatus(route *routev1.Route, admitted bool) *routev1.Route { - var host string - if route.Spec.Host != "" { - host = route.Spec.Host - } - if route.Spec.Subdomain != "" { - host = route.Spec.Subdomain + ".cert-manager.io" // suffix depends on IC config - } + if tt.want.Spec.PrivateKey != nil { + assert.Equal(t, tt.want.Spec.PrivateKey, cert.Spec.PrivateKey) + } - var admittedStatus = corev1.ConditionTrue - if admitted == false { - admittedStatus = corev1.ConditionFalse - } + if tt.want.Spec.Subject != nil { + assert.Equal(t, tt.want.Spec.Subject, cert.Spec.Subject) + } - route.Status = routev1.RouteStatus{ - Ingress: []routev1.RouteIngress{ - { - Host: host, - Conditions: []routev1.RouteIngressCondition{ - { - Type: "Admitted", - Status: admittedStatus, - }, - }, - }, - }, + if tt.want.Spec.RenewBefore != nil { + assert.Equal(t, tt.want.Spec.RenewBefore, cert.Spec.RenewBefore) + } + + close(recorder.Events) + }) } - return route } diff --git a/internal/controller/util.go b/internal/controller/util.go new file mode 100644 index 0000000..3b2957e --- /dev/null +++ b/internal/controller/util.go @@ -0,0 +1,50 @@ +/* +Copyright 2024 The cert-manager Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "crypto/sha256" + "encoding/hex" + "strings" +) + +const ( + maxKubernetesResourceNameLength = 253 +) + +func safeKubernetesNameAppend(name string, suffix string) string { + dumbAppend := strings.Join([]string{name, suffix}, "-") + if len(dumbAppend) < maxKubernetesResourceNameLength { + // if simply appending the suffix isn't too long, just do that + return dumbAppend + } + + // We're going to need to remove some of the end of `name` to be able to append `suffix` + // Take a hash of the full name and add it between `name` and `suffix` so that we don't + // risk collisions for long names that only differ in the final few characters + h := sha256.Sum256([]byte(name)) + + hashStr := hex.EncodeToString(h[:])[:6] + + // We'll have the form -- + // Hash is 6 chars long (because we take the last 6 for hashStr below) + // Suffix is len(suffix) charts long + // There are two chars for "-" joining characters + name = name[:len(name)-2-6-len(suffix)] + + return strings.Join([]string{name, hashStr, suffix}, "-") +} diff --git a/internal/controller/util_test.go b/internal/controller/util_test.go new file mode 100644 index 0000000..84050b6 --- /dev/null +++ b/internal/controller/util_test.go @@ -0,0 +1,58 @@ +/* +Copyright 2024 The cert-manager Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import "testing" + +// For reference below: +// $ echo -n "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" | sha256sum +// 03aaf5773717feae6f704bf2637ae0a9af8b1b26c3493ef29553818378773a04 - + +// $ echo -n "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaab" | sha256sum +// 9124f8b01de4e3e64e86f1f98309adf6a4cb474aacd78e5f9b7247bbb08a5c20 - + +func Test_safeKubernetesNameAppend(t *testing.T) { + tests := map[string]struct { + name string + expected string + }{ + "short name uses dumb approach": { + name: "short", + expected: "short-test", + }, + "long name has a unique hash and suffix appended": { + // 252 "a" characters + name: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + expected: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-03aaf5-test", + }, + "long name with different end has a unique hash and suffix appended": { + // 251 "a" characters followed by a "b" + name: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaab", + expected: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-9124f8-test", + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + out := safeKubernetesNameAppend(test.name, "test") + + if test.expected != out { + t.Errorf("wanted %s\n got %s", test.expected, out) + } + }) + } +} diff --git a/make/test-smoke.mk b/make/test-smoke.mk index ba29dbc..e6da0d6 100644 --- a/make/test-smoke.mk +++ b/make/test-smoke.mk @@ -58,5 +58,5 @@ test-smoke-deps: install .PHONY: test-smoke ## Smoke end-to-end tests ## @category Testing -test-smoke: test-smoke-deps | kind-cluster - ./test/test-smoke.sh +test-smoke: test-smoke-deps | kind-cluster $(NEEDS_YQ) + ./test/test-smoke.sh $(YQ) diff --git a/test/test-smoke.sh b/test/test-smoke.sh index 7b030dd..bd3ed6c 100755 --- a/test/test-smoke.sh +++ b/test/test-smoke.sh @@ -18,6 +18,8 @@ set -o errexit set -o nounset set -o pipefail +YQ=${1:-yq} + # Create a self-signed CA certificate and Issuer cat < /dev/null && echo "Found 'provinces = [Ontario]' in Certificate YAML" +echo "$cert_yaml" | $YQ eval --exit-status 'select(.spec.subject.streetAddresses[0] == "1725 Slough Avenue")' > /dev/null && echo "Found 'streetAddresses = [1725 Slough Avenue]' in Certificate YAML" +echo "$cert_yaml" | $YQ eval --exit-status 'select(.spec.subject.countries[0] == "UK")' > /dev/null && echo "Found 'countries = [UK]' in Certificate YAML" +echo "$cert_yaml" | $YQ eval --exit-status 'select(.spec.subject.postalCodes[0] == "SW1A 2AA")' > /dev/null && echo "Found 'postal codes = [SW1A 2AA]' in Certificate YAML" +echo "$cert_yaml" | $YQ eval --exit-status 'select(.spec.subject.organizations[0] == "cert-manager")' > /dev/null && echo "Found 'organizations = [cert-manager]' in Certificate YAML" +echo "$cert_yaml" | $YQ eval --exit-status 'select(.spec.subject.organizationalUnits[0] == "my-ou")' > /dev/null && echo "Found 'organizationalUnits = [my-ou]' in Certificate YAML" + +echo "$cert_yaml" | $YQ eval --exit-status 'select(.spec.privateKey.rotationPolicy == "Always")' > /dev/null && echo "Found 'rotationPolicy == Always' in Certificate YAML" + +echo "$cert_yaml" | $YQ eval --exit-status 'select(.spec.renewBefore == "30m0s")' > /dev/null && echo "Found 'renewBefore == 30m0s' in Certificate YAML" + +echo "$cert_yaml" | $YQ eval --exit-status 'select(.spec.revisionHistoryLimit == 2)' > /dev/null && echo "Found 'revisionHistoryLimit == 2' in Certificate YAML" kubectl delete route "$route_name"