From fea52453ec8be235b4c7d53d93a08ed0152a646d Mon Sep 17 00:00:00 2001 From: Jack Henschel Date: Tue, 13 Feb 2024 10:33:05 +0100 Subject: [PATCH 1/9] 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 913d27889a3bda5d3a66a8e09c6f196dc8966fa0 Mon Sep 17 00:00:00 2001 From: Ashley Davis Date: Mon, 30 Sep 2024 15:17:45 +0100 Subject: [PATCH 2/9] ensure logr calls have named args to prevent panics Signed-off-by: Ashley Davis --- internal/controller/sync.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/controller/sync.go b/internal/controller/sync.go index 3e12269..2bba12b 100644 --- a/internal/controller/sync.go +++ b/internal/controller/sync.go @@ -76,7 +76,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 { @@ -139,7 +140,8 @@ 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 } From 3699a5a766e2e3eb0d6e606eddb7fd108192005e Mon Sep 17 00:00:00 2001 From: Ashley Davis Date: Mon, 30 Sep 2024 15:18:03 +0100 Subject: [PATCH 3/9] update RBAC to use certs rather than certreqs Signed-off-by: Ashley Davis --- deploy/charts/openshift-routes/templates/rbac.yaml | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/deploy/charts/openshift-routes/templates/rbac.yaml b/deploy/charts/openshift-routes/templates/rbac.yaml index 09fe2d5..0ea134b 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,20 @@ rules: - apiGroups: - cert-manager.io resources: - - certificaterequests/status + - certificates/status verbs: - get - list - watch +- apiGroups: + - "" + resources: + - secrets + verbs: + - create + - get + - list + - watch - apiGroups: - "" resources: From 54b6107b4cfaed2f4580a2942bc876249b7471d3 Mon Sep 17 00:00:00 2001 From: Ashley Davis Date: Mon, 30 Sep 2024 15:18:52 +0100 Subject: [PATCH 4/9] fix duration in smoke test, log when polling Signed-off-by: Ashley Davis --- test/test-smoke.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/test-smoke.sh b/test/test-smoke.sh index 7b030dd..d2b6300 100755 --- a/test/test-smoke.sh +++ b/test/test-smoke.sh @@ -65,7 +65,7 @@ metadata: name: $route_name annotations: cert-manager.io/issuer-name: my-ca-issuer - cert-manager.io/duration: 1m + cert-manager.io/duration: 1h spec: host: hello-openshift-hello-openshift.test port: @@ -108,13 +108,15 @@ EOF kubectl patch route "$route_name" --type=merge --subresource=status -p="$patch" # Wait for the certificate to be issued +SLEEP_TIME=2 for _ in {1..10}; do certificate=$(kubectl get route "$route_name" -o jsonpath='{.spec.tls.certificate}') if [ "$certificate" != "" ]; then break fi - sleep 1 + echo "Didn't find certificate on route yet, retrying in $SLEEP_TIME seconds" + sleep $SLEEP_TIME done if [ "$certificate" == "" ]; then From 228aa7d18aaf8859fd4050cc4d9c72621f98ed26 Mon Sep 17 00:00:00 2001 From: Ashley Davis Date: Mon, 30 Sep 2024 15:19:08 +0100 Subject: [PATCH 5/9] don't try to create secret, clean up tests Signed-off-by: Ashley Davis --- internal/controller/sync.go | 30 +- internal/controller/sync_test.go | 1152 +----------------------------- 2 files changed, 6 insertions(+), 1176 deletions(-) diff --git a/internal/controller/sync.go b/internal/controller/sync.go index 2bba12b..1d35ca5 100644 --- a/internal/controller/sync.go +++ b/internal/controller/sync.go @@ -85,30 +85,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{}) @@ -119,15 +95,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 { @@ -143,7 +122,8 @@ func (r *RouteController) sync(ctx context.Context, req reconcile.Request, route 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 { diff --git a/internal/controller/sync_test.go b/internal/controller/sync_test.go index 641bb6a..835b886 100644 --- a/internal/controller/sync_test.go +++ b/internal/controller/sync_test.go @@ -17,28 +17,20 @@ limitations under the License. package controller import ( - "context" "crypto/ecdsa" "crypto/elliptic" "crypto/rand" - "crypto/rsa" "crypto/x509" "crypto/x509/pkix" "encoding/pem" - "fmt" "math/big" - "net" - "net/url" "sort" - "strconv" "testing" "time" cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" - 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 +347,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,1148 +363,6 @@ 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) - tests := []struct { - name string - route *routev1.Route - want bool - wantedEvents []string - }{ - { - name: "route has a private key", - route: &routev1.Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, - Annotations: map[string]string{ - cmapi.IssuerNameAnnotationKey: "some-issuer", - cmapi.IsNextPrivateKeySecretLabelKey: string(ecdsaKeyPEM), - }, - }, - Spec: routev1.RouteSpec{}, - }, - want: true, - wantedEvents: nil, - }, - { - name: "route has no private key", - route: &routev1.Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, - Annotations: map[string]string{ - cmapi.IssuerNameAnnotationKey: "some-issuer", - }, - }, - Spec: routev1.RouteSpec{}, - }, - want: false, - wantedEvents: nil, - }, - { - name: "route has garbage data in private key", - route: &routev1.Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, - Annotations: map[string]string{ - cmapi.IssuerNameAnnotationKey: "some-issuer", - cmapi.IsNextPrivateKeySecretLabelKey: `-----BEGIN PRIVATE KEY----- -SOME GARBAGE ------END PRIVATE KEY-----`, - }, - }, - 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"}, - }, - } - 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{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, - Annotations: map[string]string{ - cmapi.IssuerNameAnnotationKey: "some-issuer", - }, - }, - Spec: routev1.RouteSpec{}, - }, - want: nil, - wantedEvents: []string{"Normal Issuing Generated Private Key for route"}, - wantedPrivateKeyHeader: "BEGIN RSA PRIVATE KEY", - }, - { - name: "route with rsa algorithm annotation has no private key", - route: &routev1.Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, - Annotations: map[string]string{ - cmapi.IssuerNameAnnotationKey: "some-issuer", - cmapi.PrivateKeyAlgorithmAnnotationKey: "RSA", - }, - }, - Spec: routev1.RouteSpec{ - Host: "some-host.some-domain.tld", - }, - }, - want: nil, - wantedEvents: []string{"Normal Issuing Generated Private Key for route"}, - wantedPrivateKeyHeader: "BEGIN RSA PRIVATE KEY", - }, - { - name: "route with ecdsa algorithm annotation has no private key", - route: &routev1.Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, - Annotations: map[string]string{ - cmapi.IssuerNameAnnotationKey: "some-issuer", - cmapi.PrivateKeyAlgorithmAnnotationKey: "ECDSA", - }, - }, - Spec: routev1.RouteSpec{ - Host: "some-host.some-domain.tld", - }, - }, - want: nil, - wantedEvents: []string{"Normal Issuing Generated Private Key for route"}, - wantedPrivateKeyHeader: "BEGIN EC PRIVATE KEY", - }, - { - name: "route with invalid algorithm annotation has no private key", - route: &routev1.Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, - Annotations: map[string]string{ - cmapi.IssuerNameAnnotationKey: "some-issuer", - cmapi.PrivateKeyAlgorithmAnnotationKey: "notreal", - }, - }, - Spec: routev1.RouteSpec{ - Host: "some-host.some-domain.tld", - }, - }, - want: fmt.Errorf("invalid private key algorithm: notreal"), - wantedEvents: []string{"Warning InvalidPrivateKeyAlgorithm invalid private key algorithm: notreal"}, - wantedPrivateKeyHeader: "", - }, - } - 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{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, - Annotations: map[string]string{ - cmapi.IssuerNameAnnotationKey: "some-issuer", - cmapi.CertificateRequestRevisionAnnotationKey: "1337", - }, - }, - Spec: routev1.RouteSpec{}, - }, - want: 1337, - wantErr: nil, - }, - { - name: "route without revision", - route: &routev1.Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, - Annotations: map[string]string{ - cmapi.IssuerNameAnnotationKey: "some-issuer", - }, - }, - Spec: routev1.RouteSpec{}, - }, - want: 0, - wantErr: fmt.Errorf("no revision found"), - }, - } - 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{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, - Annotations: map[string]string{ - cmapi.IssuerNameAnnotationKey: "some-issuer", - }, - }, - Spec: routev1.RouteSpec{}, - }, - revision: 1337, - want: "1337", - 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) - - 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, - route: generateRouteStatus(&routev1.Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.DurationAnnotationKey: "42m", - cmapi.IsNextPrivateKeySecretLabelKey: string(rsaPEM), - }, - }, - Spec: routev1.RouteSpec{ - Host: "some-host.some-domain.tld", - }, - Status: routev1.RouteStatus{ - Ingress: []routev1.RouteIngress{ - { - Host: "some-host.some-domain.tld", - Conditions: []routev1.RouteIngressCondition{ - { - Type: "Admitted", - Status: "True", - }, - }, - }, - }, - }, - }, - true), - want: &cmapi.CertificateRequest{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "some-route-", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.CertificateRequestRevisionAnnotationKey: "1338", - }, - }, - Spec: cmapi.CertificateRequestSpec{ - Duration: &metav1.Duration{Duration: 42 * time.Minute}, - IsCA: false, - Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, - }, - }, - wantErr: nil, - }, - { - name: "Basic test with issuer", - revision: 1337, - route: generateRouteStatus(&routev1.Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.DurationAnnotationKey: "42m", - cmapi.IsNextPrivateKeySecretLabelKey: string(rsaPEM), - cmapi.IssuerNameAnnotationKey: "self-signed-issuer", - cmapi.IssuerKindAnnotationKey: "Issuer", - }, - }, - Spec: routev1.RouteSpec{ - Host: "some-host.some-domain.tld", - }, - Status: routev1.RouteStatus{ - Ingress: []routev1.RouteIngress{ - { - Host: "some-host.some-domain.tld", - Conditions: []routev1.RouteIngressCondition{ - { - Type: "Admitted", - Status: "True", - }, - }, - }, - }, - }, - }, - true), - want: &cmapi.CertificateRequest{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "some-route-", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.CertificateRequestRevisionAnnotationKey: "1338", - }, - }, - 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", - }, - }, - }, - wantErr: nil, - }, - { - name: "Basic test with external issuer", - revision: 1337, - route: generateRouteStatus(&routev1.Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - 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", - }, - }, - Spec: routev1.RouteSpec{ - Host: "some-host.some-domain.tld", - }, - Status: routev1.RouteStatus{ - Ingress: []routev1.RouteIngress{ - { - Host: "some-host.some-domain.tld", - Conditions: []routev1.RouteIngressCondition{ - { - Type: "Admitted", - Status: "True", - }, - }, - }, - }, - }, - }, - true), - want: &cmapi.CertificateRequest{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "some-route-", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.CertificateRequestRevisionAnnotationKey: "1338", - }, - }, - 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", - }, - }, - }, - wantErr: nil, - }, - { - name: "Basic test with alternate ingress issuer name annotation", - revision: 1337, - route: generateRouteStatus(&routev1.Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - 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", - }, - }, - Spec: routev1.RouteSpec{ - Host: "some-host.some-domain.tld", - }, - Status: routev1.RouteStatus{ - Ingress: []routev1.RouteIngress{ - { - Host: "some-host.some-domain.tld", - Conditions: []routev1.RouteIngressCondition{ - { - Type: "Admitted", - Status: "True", - }, - }, - }, - }, - }, - }, - true), - want: &cmapi.CertificateRequest{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "some-route-", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.CertificateRequestRevisionAnnotationKey: "1338", - }, - }, - 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", - }, - }, - }, - wantErr: nil, - }, - { - name: "With subdomain and multiple ICs", - revision: 1337, - route: &routev1.Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-route-with-subdomain", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.IsNextPrivateKeySecretLabelKey: string(rsaPEM), - }, - }, - Spec: routev1.RouteSpec{ - 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: &cmapi.CertificateRequest{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "some-route-with-subdomain-", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.CertificateRequestRevisionAnnotationKey: "1338", - }, - }, - Spec: cmapi.CertificateRequestSpec{ - Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, - Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, - }, - }, - 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, - route: generateRouteStatus(&routev1.Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.IsNextPrivateKeySecretLabelKey: string(ecdsaPEM), - cmapi.PrivateKeyAlgorithmAnnotationKey: string(cmapi.ECDSAKeyAlgorithm), - }, - }, - Spec: routev1.RouteSpec{ - Host: "some-host.some-domain.tld", - }, - Status: routev1.RouteStatus{ - Ingress: []routev1.RouteIngress{ - { - Host: "some-host.some-domain.tld", - Conditions: []routev1.RouteIngressCondition{ - { - Type: "Admitted", - Status: "True", - }, - }, - }, - }, - }, - }, - 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.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, - route: generateRouteStatus(&routev1.Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.IsNextPrivateKeySecretLabelKey: string(ecdsaPEM), - cmapi.PrivateKeyAlgorithmAnnotationKey: string(cmapi.ECDSAKeyAlgorithm), - cmapi.PrivateKeySizeAnnotationKey: strconv.Itoa(384), - }, - }, - Spec: routev1.RouteSpec{ - Host: "some-host.some-domain.tld", - }, - Status: routev1.RouteStatus{ - Ingress: []routev1.RouteIngress{ - { - Host: "some-host.some-domain.tld", - Conditions: []routev1.RouteIngressCondition{ - { - Type: "Admitted", - Status: "True", - }, - }, - }, - }, - }, - }, - 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.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, - route: generateRouteStatus(&routev1.Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.IsNextPrivateKeySecretLabelKey: string(ecdsaPEM), - cmapi.PrivateKeyAlgorithmAnnotationKey: string(cmapi.ECDSAKeyAlgorithm), - cmapi.PrivateKeySizeAnnotationKey: strconv.Itoa(521), - }, - }, - Spec: routev1.RouteSpec{ - Host: "some-host.some-domain.tld", - }, - Status: routev1.RouteStatus{ - Ingress: []routev1.RouteIngress{ - { - Host: "some-host.some-domain.tld", - Conditions: []routev1.RouteIngressCondition{ - { - Type: "Admitted", - Status: "True", - }, - }, - }, - }, - }, - }, - 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.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, - route: generateRouteStatus(&routev1.Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.IsNextPrivateKeySecretLabelKey: string(rsaPEM), - cmapi.PrivateKeyAlgorithmAnnotationKey: string(cmapi.RSAKeyAlgorithm), - }, - }, - Spec: routev1.RouteSpec{ - Host: "some-host.some-domain.tld", - }, - Status: routev1.RouteStatus{ - Ingress: []routev1.RouteIngress{ - { - Host: "some-host.some-domain.tld", - Conditions: []routev1.RouteIngressCondition{ - { - Type: "Admitted", - Status: "True", - }, - }, - }, - }, - }, - }, - 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.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, - route: generateRouteStatus(&routev1.Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.IsNextPrivateKeySecretLabelKey: string(rsaPEM), - cmapi.PrivateKeyAlgorithmAnnotationKey: string(cmapi.RSAKeyAlgorithm), - cmapi.PrivateKeySizeAnnotationKey: strconv.Itoa(3072), - }, - }, - Spec: routev1.RouteSpec{ - Host: "some-host.some-domain.tld", - }, - Status: routev1.RouteStatus{ - Ingress: []routev1.RouteIngress{ - { - Host: "some-host.some-domain.tld", - Conditions: []routev1.RouteIngressCondition{ - { - Type: "Admitted", - Status: "True", - }, - }, - }, - }, - }, - }, - 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.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, - route: generateRouteStatus(&routev1.Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-route", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.IsNextPrivateKeySecretLabelKey: string(rsaPEM), - cmapi.PrivateKeyAlgorithmAnnotationKey: string(cmapi.RSAKeyAlgorithm), - cmapi.PrivateKeySizeAnnotationKey: strconv.Itoa(4096), - }, - }, - Spec: routev1.RouteSpec{ - Host: "some-host.some-domain.tld", - }, - Status: routev1.RouteStatus{ - Ingress: []routev1.RouteIngress{ - { - Host: "some-host.some-domain.tld", - Conditions: []routev1.RouteIngressCondition{ - { - Type: "Admitted", - Status: "True", - }, - }, - }, - }, - }, - }, - 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, - }, - { - name: "With subject annotations", - revision: 1337, - route: &routev1.Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-route-with-subject-annotations", - 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", - }, - }, - Spec: routev1.RouteSpec{ - Host: "example-route.example.com", - }, - Status: routev1.RouteStatus{ - Ingress: []routev1.RouteIngress{ - { - Host: "example-route.example.com", - Conditions: []routev1.RouteIngressCondition{ - { - Type: "Admitted", - Status: "True", - }, - }, - }, - }, - }, - }, - want: &cmapi.CertificateRequest{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "some-route-with-subject-annotations-", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.CertificateRequestRevisionAnnotationKey: "1338", - }, - }, - Spec: cmapi.CertificateRequestSpec{ - Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, - Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, - }, - }, - 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, - }, - { - name: "With all annotations", - revision: 1337, - route: &routev1.Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-route-with-all-annotations", - 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", - }, - }, - Spec: routev1.RouteSpec{ - Host: "example-route.example.com", - }, - Status: routev1.RouteStatus{ - Ingress: []routev1.RouteIngress{ - { - Host: "example-route.example.com", - Conditions: []routev1.RouteIngressCondition{ - { - Type: "Admitted", - Status: "True", - }, - }, - }, - }, - }, - }, - want: &cmapi.CertificateRequest{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "some-route-with-all-annotations-", - Namespace: "some-namespace", - Annotations: map[string]string{ - cmapi.CertificateRequestRevisionAnnotationKey: "1338", - }, - }, - Spec: cmapi.CertificateRequestSpec{ - Duration: &metav1.Duration{Duration: time.Hour * 24 * 30}, - Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, - }, - }, - 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, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - recorder := record.NewFakeRecorder(100) - r := &Route{ - eventRecorder: recorder, - } - // test "buildNextCR" function - cr, err := r.buildNextCR(context.TODO(), tt.route, tt.revision) - - // check that we got the expected error (including nil) - assert.Equal(t, tt.wantErr, err, "buildNextCR()") - - // 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) - } - } - - // 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") - } - - }) - } -} - // trivial logic that re-implements OpenShift's IngressController behavior func generateRouteStatus(route *routev1.Route, admitted bool) *routev1.Route { var host string From 177d7cfbd5e3f5c41b6633076b13337cdbaf4210 Mon Sep 17 00:00:00 2001 From: Ashley Davis Date: Mon, 30 Sep 2024 15:30:40 +0100 Subject: [PATCH 6/9] fix lint error, allow more permissive algorithm specification Signed-off-by: Ashley Davis --- internal/controller/sync.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/internal/controller/sync.go b/internal/controller/sync.go index 1d35ca5..dbb3f35 100644 --- a/internal/controller/sync.go +++ b/internal/controller/sync.go @@ -248,15 +248,20 @@ func (r *RouteController) buildNextCert(ctx context.Context, route *routev1.Rout } 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.Info("unknown private key algorithm, defaulting to RSA", "algorithm", privateKeyAlgorithmStrRaw) privateKeyAlgorithm = cmapi.RSAKeyAlgorithm } From c0b0dd6636e7b1cb4a19fcc4983ab1f5382c6193 Mon Sep 17 00:00:00 2001 From: Ashley Davis Date: Mon, 30 Sep 2024 15:48:39 +0100 Subject: [PATCH 7/9] change check for CRs to certs Signed-off-by: Ashley Davis --- internal/cmd/app/app.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) 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") } From 212b89948bc49ea8ec030a07da4f003fbcd01d54 Mon Sep 17 00:00:00 2001 From: Ashley Davis Date: Tue, 1 Oct 2024 10:10:18 +0100 Subject: [PATCH 8/9] don't Own Secrets since we don't generate them Signed-off-by: Ashley Davis --- internal/controller/controller.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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) } From 6d684f7bcc1153aaca6baa71eb616d8023fca7f2 Mon Sep 17 00:00:00 2001 From: Ashley Davis Date: Mon, 30 Sep 2024 15:47:22 +0100 Subject: [PATCH 9/9] WIP: switching between cert / cr Signed-off-by: Ashley Davis --- deploy/charts/openshift-routes/README.md | 10 + .../templates/deployment.yaml | 1 + .../openshift-routes/templates/rbac.yaml | 20 + .../openshift-routes/values.schema.json | 8 + deploy/charts/openshift-routes/values.yaml | 7 + internal/cmd/app/app.go | 46 +- internal/cmd/app/options/options.go | 28 + internal/crcontroller/controller.go | 117 ++ internal/crcontroller/controller_test.go | 85 + internal/crcontroller/sync.go | 751 ++++++++ internal/crcontroller/sync_test.go | 1545 +++++++++++++++++ make/test-smoke.mk | 15 +- 12 files changed, 2625 insertions(+), 8 deletions(-) create mode 100644 internal/crcontroller/controller.go create mode 100644 internal/crcontroller/controller_test.go create mode 100644 internal/crcontroller/sync.go create mode 100644 internal/crcontroller/sync_test.go diff --git a/deploy/charts/openshift-routes/README.md b/deploy/charts/openshift-routes/README.md index 2533f72..06eb590 100644 --- a/deploy/charts/openshift-routes/README.md +++ b/deploy/charts/openshift-routes/README.md @@ -31,6 +31,16 @@ Override the "cert-manager.fullname" value. This value is used as part of most o Override the "cert-manager.name" value, which is used to annotate some of the resources that are created by this Chart (using "app.kubernetes.io/name"). NOTE: There are some inconsitencies in the Helm chart when it comes to these annotations (some resources use eg. "cainjector.name" which resolves to the value "cainjector"). +#### **issuanceMode** ~ `string` +> Default value: +> ```yaml +> certificate +> ``` + +Control how certificates are issued for routes. 'certificate' mode (the default) will create a cert-manager Certificate resource and store the issued certificate in a +Kubernetes Secret before adding it to the route. +'certificaterequest' mode will directly create a CertificateRequest resource, which makes +the cert harder to use outside of Routes but avoids creating a Secret #### **image.registry** ~ `string` Target image registry. This value is prepended to the target image repository, if set. diff --git a/deploy/charts/openshift-routes/templates/deployment.yaml b/deploy/charts/openshift-routes/templates/deployment.yaml index ae3f435..91b0a8d 100644 --- a/deploy/charts/openshift-routes/templates/deployment.yaml +++ b/deploy/charts/openshift-routes/templates/deployment.yaml @@ -36,6 +36,7 @@ spec: args: - "-v={{ .Values.logLevel }}" - "--leader-election-namespace={{ .Release.Namespace }}" + - "--issuance-mode={{ .Values.issuanceMode }}" ports: - containerPort: 6060 name: readiness diff --git a/deploy/charts/openshift-routes/templates/rbac.yaml b/deploy/charts/openshift-routes/templates/rbac.yaml index 0ea134b..b52c4f3 100644 --- a/deploy/charts/openshift-routes/templates/rbac.yaml +++ b/deploy/charts/openshift-routes/templates/rbac.yaml @@ -29,6 +29,25 @@ rules: verbs: - create - update +{{- if eq (lower .Values.issuanceMode) "certificaterequest" }} +- apiGroups: + - cert-manager.io + resources: + - certificaterequests + verbs: + - create + - get + - list + - watch +- apiGroups: + - cert-manager.io + resources: + - certificaterequests/status + verbs: + - get + - list + - watch +{{- else }} - apiGroups: - cert-manager.io resources: @@ -55,6 +74,7 @@ rules: - get - list - watch +{{- end }} - apiGroups: - "" resources: diff --git a/deploy/charts/openshift-routes/values.schema.json b/deploy/charts/openshift-routes/values.schema.json index 4727b29..28138cb 100644 --- a/deploy/charts/openshift-routes/values.schema.json +++ b/deploy/charts/openshift-routes/values.schema.json @@ -18,6 +18,9 @@ "imagePullSecrets": { "$ref": "#/$defs/helm-values.imagePullSecrets" }, + "issuanceMode": { + "$ref": "#/$defs/helm-values.issuanceMode" + }, "logLevel": { "$ref": "#/$defs/helm-values.logLevel" }, @@ -124,6 +127,11 @@ "items": {}, "type": "array" }, + "helm-values.issuanceMode": { + "default": "certificate", + "description": "Control how certificates are issued for routes. 'certificate' mode (the default) will create a cert-manager Certificate resource and store the issued certificate in a\nKubernetes Secret before adding it to the route.\n'certificaterequest' mode will directly create a CertificateRequest resource, which makes\nthe cert harder to use outside of Routes but avoids creating a Secret", + "type": "string" + }, "helm-values.logLevel": { "default": 5, "type": "number" diff --git a/deploy/charts/openshift-routes/values.yaml b/deploy/charts/openshift-routes/values.yaml index ee66c33..5eaca5e 100644 --- a/deploy/charts/openshift-routes/values.yaml +++ b/deploy/charts/openshift-routes/values.yaml @@ -21,6 +21,13 @@ namespace: "" # +docs:property # nameOverride: "my-cert-manager" +# Control how certificates are issued for routes. 'certificate' mode (the default) will +# create a cert-manager Certificate resource and store the issued certificate in a +# Kubernetes Secret before adding it to the route. +# 'certificaterequest' mode will directly create a CertificateRequest resource, which makes +# the cert harder to use outside of Routes but avoids creating a Secret +issuanceMode: "certificate" + image: # Target image registry. This value is prepended to the target image repository, if set. # For example: diff --git a/internal/cmd/app/app.go b/internal/cmd/app/app.go index 6a59a73..45c3bef 100644 --- a/internal/cmd/app/app.go +++ b/internal/cmd/app/app.go @@ -35,6 +35,7 @@ import ( "github.com/cert-manager/openshift-routes/internal/cmd/app/options" "github.com/cert-manager/openshift-routes/internal/controller" + "github.com/cert-manager/openshift-routes/internal/crcontroller" ) func Command() *cobra.Command { @@ -69,22 +70,33 @@ 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 Certificates exist in the API server + // Check if v1 cert-manager Certificates / CertificateRequests exist in the API server apiServerHasCertificates := false + apiServerHasCertificateRequests := 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 apiServerHasCertificates && apiServerHasCertificateRequests { + break + } + if r.Kind == "Certificate" { apiServerHasCertificates = true - break + continue + } + + if r.Kind == "CertificateRequest" { + apiServerHasCertificateRequests = true + continue } } - if !apiServerHasCertificates { - return fmt.Errorf("connected to the Kubernetes API, but the cert-manager v1 CRDs do not appear to be installed") + if !apiServerHasCertificates || !apiServerHasCertificateRequests { + return fmt.Errorf("connected to the Kubernetes API, but the cert-manager v1 CRDs do not appear to be installed: has Certificates=%v, has CertificateRequests=%v", apiServerHasCertificates, apiServerHasCertificateRequests) } logger := opts.Logr.WithName("controller-manager") @@ -121,6 +133,7 @@ func Command() *cobra.Command { if err != nil { return fmt.Errorf("could not create controller manager: %w", err) } + mgr.AddReadyzCheck("informers_synced", func(req *http.Request) error { // haven't got much time to wait in a readiness check ctx, cancel := context.WithTimeout(req.Context(), 2*time.Second) @@ -130,13 +143,32 @@ func Command() *cobra.Command { } return fmt.Errorf("informers not synced") }) - if err := controller.AddToManager(mgr, opts); err != nil { - return fmt.Errorf("could not add route controller to manager: %w", err) + + switch opts.IssuanceMode { + case options.CertificateIssuanceMode: + err := controller.AddToManager(mgr, opts) + if err != nil { + return fmt.Errorf("could not add certificate-based route controller to manager: %w", err) + } + + opts.Logr.V(5).Info("starting certificate-based controller") + + case options.CertificateRequestIssuanceMode: + err := crcontroller.AddToManager(mgr, opts) + if err != nil { + return fmt.Errorf("could not add certificate request-based route controller to manager: %w", err) + } + + opts.Logr.V(5).Info("starting certificate request-based controller") + + default: + return fmt.Errorf("invalid issuance mode %q", opts.IssuanceMode) } - opts.Logr.V(5).Info("starting controller") + return mgr.Start(ctrl.SetupSignalHandler()) }, } + opts.Prepare(cmd) return cmd } diff --git a/internal/cmd/app/options/options.go b/internal/cmd/app/options/options.go index 61488aa..ccea450 100644 --- a/internal/cmd/app/options/options.go +++ b/internal/cmd/app/options/options.go @@ -19,6 +19,7 @@ package options import ( "flag" "fmt" + "strings" "github.com/go-logr/logr" "github.com/spf13/cobra" @@ -31,6 +32,13 @@ import ( "k8s.io/klog/v2/klogr" ) +const ( + CertificateIssuanceMode = "certificate" + CertificateRequestIssuanceMode = "certificaterequest" + + defaultIssuanceMode = CertificateIssuanceMode +) + // Options is the main configuration struct for cert-manager-openshift-routes type Options struct { EventRecorder record.EventRecorder @@ -55,6 +63,10 @@ type Options struct { // RestConfig is the Kubernetes config RestConfig *rest.Config + // IssuanceMode switches between using Certificates and CertificateRequests + // to issue certs for routes + IssuanceMode string + logLevel string kubeConfigFlags *genericclioptions.ConfigFlags } @@ -82,6 +94,19 @@ func (o *Options) Complete() error { return fmt.Errorf("failed to build kubernetes rest config: %s", err) } + originalIssuanceMode := o.IssuanceMode + + if o.IssuanceMode == "" { + o.IssuanceMode = defaultIssuanceMode + } + + o.IssuanceMode = strings.ToLower(o.IssuanceMode) + o.IssuanceMode = strings.TrimSuffix(o.IssuanceMode, "s") + + if o.IssuanceMode != CertificateIssuanceMode && o.IssuanceMode != CertificateRequestIssuanceMode { + return fmt.Errorf("invalid issuance mode %q; must be either '%s' or '%s'", originalIssuanceMode, CertificateIssuanceMode, CertificateRequestIssuanceMode) + } + return nil } @@ -134,4 +159,7 @@ func (o *Options) addAppFlags(fs *pflag.FlagSet) { fs.StringVar(&o.LeaderElectionNamespace, "leader-election-namespace", "cert-manager", "Namespace to create leader election resources in.") + + fs.StringVar(&o.IssuanceMode, "issuance-mode", defaultIssuanceMode, + fmt.Sprintf("How certificates should be requested. Either '%s' or '%s'", CertificateIssuanceMode, CertificateRequestIssuanceMode)) } diff --git a/internal/crcontroller/controller.go b/internal/crcontroller/controller.go new file mode 100644 index 0000000..9921b09 --- /dev/null +++ b/internal/crcontroller/controller.go @@ -0,0 +1,117 @@ +/* +Copyright 2022 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 crcontroller + +import ( + "context" + + cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + cmclient "github.com/cert-manager/cert-manager/pkg/client/clientset/versioned" + "github.com/go-logr/logr" + routev1 "github.com/openshift/api/route/v1" + routev1client "github.com/openshift/client-go/route/clientset/versioned" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/cert-manager/openshift-routes/internal/cmd/app/options" +) + +type Route struct { + routeClient routev1client.Interface + certClient cmclient.Interface + eventRecorder record.EventRecorder + + log logr.Logger +} + +func shouldSync(log logr.Logger, route *routev1.Route) bool { + if len(route.ObjectMeta.OwnerReferences) > 0 { + for _, o := range route.ObjectMeta.OwnerReferences { + if o.Kind == "Ingress" { + log.V(5).Info("Route is owned by an Ingress") + return false + } + } + } + + if metav1.HasAnnotation(route.ObjectMeta, cmapi.IssuerNameAnnotationKey) { + log.V(5).Info("Route has the annotation", "annotation-key", cmapi.IssuerNameAnnotationKey, "annotation-value", route.Annotations[cmapi.IssuerNameAnnotationKey]) + return true + } + + if metav1.HasAnnotation(route.ObjectMeta, cmapi.IngressIssuerNameAnnotationKey) { + log.V(5).Info("Route has the annotation", "annotation-key", cmapi.IngressIssuerNameAnnotationKey, "annotation-value", route.Annotations[cmapi.IngressIssuerNameAnnotationKey]) + return true + } + + log.V(5).Info("Route does not have the cert-manager issuer annotation") + return false +} + +func (r *Route) 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{}) + if errors.IsNotFound(err) { + return reconcile.Result{}, nil + } + if err != nil { + return reconcile.Result{}, err + } + log.V(5).Info("retrieved route") + + if !shouldSync(log, route) { + return reconcile.Result{}, nil + } + + return r.sync(ctx, req, route.DeepCopy()) +} + +func New(base logr.Logger, config *rest.Config, recorder record.EventRecorder) (*Route, error) { + routeClient, err := routev1client.NewForConfig(config) + if err != nil { + return nil, err + } + certClient, err := cmclient.NewForConfig(config) + if err != nil { + return nil, err + } + + return &Route{ + routeClient: routeClient, + certClient: certClient, + log: base.WithName("route"), + eventRecorder: recorder, + }, nil +} + +func AddToManager(mgr manager.Manager, opts *options.Options) error { + controller, err := New(opts.Logr, opts.RestConfig, opts.EventRecorder) + if err != nil { + return err + } + return builder. + ControllerManagedBy(mgr). + For(&routev1.Route{}). + Owns(&cmapi.CertificateRequest{}). + Complete(controller) +} diff --git a/internal/crcontroller/controller_test.go b/internal/crcontroller/controller_test.go new file mode 100644 index 0000000..e1d92bb --- /dev/null +++ b/internal/crcontroller/controller_test.go @@ -0,0 +1,85 @@ +/* +Copyright 2022 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 crcontroller + +import ( + "testing" + + "github.com/go-logr/logr" + routev1 "github.com/openshift/api/route/v1" + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func Test_shouldReconcile(t *testing.T) { + tests := []struct { + name string + given *routev1.Route + want bool + }{ + { + name: "should reconcile with cert-manager.io/issuer-name annotation", + given: &routev1.Route{ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "cert-manager.io/issuer-name": "test", + }}, + }, + want: true, + }, + { + name: "should sync with cert-manager.io/issuer annotation", + given: &routev1.Route{ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "cert-manager.io/issuer": "test", + }}, + }, + want: true, + }, + { + name: "should not sync when Route owned by Ingress", + given: &routev1.Route{ObjectMeta: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "Ingress", + }, + }}, + }, + want: false, + }, + { + name: "should not sync when Route owned by Ingress", + given: &routev1.Route{ObjectMeta: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "Ingress", + }, + }}, + }, + want: false, + }, + { + name: "should not sync when no annotation is found", + given: &routev1.Route{ObjectMeta: metav1.ObjectMeta{}}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := shouldSync(logr.Discard(), tt.given) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/internal/crcontroller/sync.go b/internal/crcontroller/sync.go new file mode 100644 index 0000000..99b9ab1 --- /dev/null +++ b/internal/crcontroller/sync.go @@ -0,0 +1,751 @@ +/* +Copyright 2022 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 crcontroller + +import ( + "context" + "crypto" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "fmt" + "net" + "net/url" + "strconv" + "strings" + "time" + + cmapiutil "github.com/cert-manager/cert-manager/pkg/api/util" + cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" + cmutil "github.com/cert-manager/cert-manager/pkg/util" + utilpki "github.com/cert-manager/cert-manager/pkg/util/pki" + routev1 "github.com/openshift/api/route/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +const ( + ReasonIssuing = `Issuing` + ReasonIssued = `Issued` + ReasonInvalidKey = `InvalidKey` + ReasonInvalidPrivateKeyAlgorithm = `InvalidPrivateKeyAlgorithm` + ReasonInvalidPrivateKeySize = `InvalidPrivateKeySize` + ReasonInvalidValue = `InvalidValue` + ReasonInternalReconcileError = `InternalReconcileError` + ReasonMissingHostname = `MissingHostname` +) + +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) { + var result reconcile.Result + var err error + + log := r.log.WithName("sync").WithValues("route", req, "resourceVersion", route.ObjectMeta.ResourceVersion) + defer func() { + // Always send a warning event if err is not nil + if err != nil { + r.log.V(1).Error(err, "error while reconciling", "object", req.NamespacedName) + r.eventRecorder.Event(route, corev1.EventTypeWarning, ReasonInternalReconcileError, "error while reconciling: "+err.Error()) + } + }() + + // Does the route contain a valid certificate? + if r.hasValidCertificate(route) { + result, err = reconcile.Result{RequeueAfter: r.getRequeueAfterDuration(route)}, nil + 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) + 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 err != nil { + log.V(1).Error(err, "error generating certificate request", "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{}) + if err != nil { + return result, err + } + r.eventRecorder.Event(route, corev1.EventTypeNormal, ReasonIssuing, "Created new CertificateRequest") + return result, nil + } + // is the CR Ready and Approved? + ready, cr, err := r.certificateRequestReadyAndApproved(ctx, route, revision) + if err != nil { + return result, err + } + if !ready { + log.V(5).Info("cr is not ready yet") + return result, nil + } + // Cert is ready. Populate the route. + err = r.populateRoute(ctx, route, cr, revision) + if err != nil { + log.V(1).Error(err, "failed to populate route certificate") + return result, err + } + log.V(5).Info("populated route cert") + r.eventRecorder.Event(route, corev1.EventTypeNormal, ReasonIssued, "Route updated with issued certificate") + return result, err +} + +func (r *Route) hasValidCertificate(route *routev1.Route) bool { + // Valid certificate predicates: + + // TLS config set? + if route.Spec.TLS == nil { + r.eventRecorder.Event(route, corev1.EventTypeNormal, ReasonIssuing, "Issuing cert as no TLS is configured") + return false + } + // Cert exists? + if len(route.Spec.TLS.Certificate) == 0 { + r.eventRecorder.Event(route, corev1.EventTypeNormal, ReasonIssuing, "Issuing cert as no certificate exists") + return false + } + // Cert parses? + cert, err := utilpki.DecodeX509CertificateBytes([]byte(route.Spec.TLS.Certificate)) + if err != nil { + r.eventRecorder.Event(route, corev1.EventTypeNormal, ReasonIssuing, "Issuing cert as the existing cert is invalid: "+err.Error()) + return false + } + // Key exists? + if len(route.Spec.TLS.Key) == 0 { + r.eventRecorder.Event(route, corev1.EventTypeNormal, ReasonIssuing, "Issuing cert as no private key exists") + return false + } + // Key parses? + key, err := utilpki.DecodePrivateKeyBytes([]byte(route.Spec.TLS.Key)) + if err != nil { + r.eventRecorder.Event(route, corev1.EventTypeNormal, ReasonIssuing, "Issuing cert as the existing key is invalid: "+err.Error()) + return false + } + // Cert matches key? + matches, err := utilpki.PublicKeyMatchesCertificate(key.Public(), cert) + if err != nil { + r.eventRecorder.Event(route, corev1.EventTypeNormal, ReasonIssuing, "Issuing cert as the certificate's key type is invalid: "+err.Error()) + } + if !matches { + r.eventRecorder.Event(route, corev1.EventTypeNormal, ReasonIssuing, "Issuing cert as the public key does not match the certificate") + return false + } + // Cert matches Route hostname? + hostnames := getRouteHostnames(route) + for _, host := range hostnames { + if err := cert.VerifyHostname(host); err != nil { + r.eventRecorder.Event(route, corev1.EventTypeNormal, ReasonIssuing, "Issuing cert as the hostname does not match the certificate") + return false + } + } + // Still not after the renew-before window? + if metav1.HasAnnotation(route.ObjectMeta, cmapi.RenewBeforeAnnotationKey) { + renewBeforeDuration, err := time.ParseDuration(route.Annotations[cmapi.RenewBeforeAnnotationKey]) + if err == nil { + if time.Now().After(cert.NotAfter.Add(-renewBeforeDuration)) { + r.eventRecorder.Event(route, corev1.EventTypeNormal, ReasonIssuing, "Issuing cert as the renew-before period has been reached") + return false + } + } else { + r.eventRecorder.Eventf( + route, + corev1.EventTypeWarning, + ReasonInvalidKey, + "the duration %s: %s is invalid (%s)", + cmapi.RenewBeforeAnnotationKey, + route.Annotations[cmapi.RenewBeforeAnnotationKey], + err.Error(), + ) + } + } + // As there is no renew-before, is the cert more than 2/3 through its life? + totalDuration := cert.NotAfter.Sub(cert.NotBefore) + timeToExpiry := cert.NotAfter.Sub(time.Now()) + if timeToExpiry < (totalDuration * 1 / 3) { + r.eventRecorder.Event(route, corev1.EventTypeNormal, ReasonIssuing, "Issuing cert as the existing cert is more than 2/3 through its validity period") + return false + } + 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 *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{}) + if err != nil { + return nil, err + } + var candidates []*cmapi.CertificateRequest + for _, cr := range allCRs.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 { + 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) + } + } + } + } + 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 + + // get duration from route + duration, err := certDurationFromRoute(route) + if err != nil { + r.log.V(1).Error(err, "the duration annotation is invalid", + "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) + } + + privateKeyAlgorithm, found := route.Annotations[cmapi.PrivateKeyAlgorithmAnnotationKey] + if !found { + privateKeyAlgorithm = string(cmapi.RSAKeyAlgorithm) + } + + var privateKeySize int + privateKeySizeStr, found := route.Annotations[cmapi.PrivateKeySizeAnnotationKey] + if found { + 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) + } + } + + 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) + if len(dnsNames) == 0 { + err := fmt.Errorf("Route is not yet initialized with a hostname") + r.eventRecorder.Event(route, corev1.EventTypeWarning, ReasonMissingHostname, fmt.Sprint(err)) + return nil, err + } + + // Parse out SANs + if metav1.HasAnnotation(route.ObjectMeta, cmapi.AltNamesAnnotationKey) { + 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 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]) + organizations = subjectOrganizations + + if err != nil { + r.log.V(1).Error(err, "the organizations annotation is invalid", + "object", route.Namespace+"/"+route.Name, cmapi.SubjectOrganizationsAnnotationKey, + route.Annotations[cmapi.SubjectOrganizationsAnnotationKey]) + r.eventRecorder.Event(route, corev1.EventTypeWarning, ReasonInvalidValue, "annotation "+cmapi.SubjectOrganizationsAnnotationKey+": "+route.Annotations[cmapi.SubjectOrganizationsAnnotationKey]+" value is malformed") + return nil, err + } + } + var organizationalUnits []string + if metav1.HasAnnotation(route.ObjectMeta, cmapi.SubjectOrganizationalUnitsAnnotationKey) { + subjectOrganizationalUnits, err := cmutil.SplitWithEscapeCSV(route.Annotations[cmapi.SubjectOrganizationalUnitsAnnotationKey]) + organizationalUnits = subjectOrganizationalUnits + + if err != nil { + r.log.V(1).Error(err, "the organizational units annotation is invalid", + "object", route.Namespace+"/"+route.Name, cmapi.SubjectOrganizationalUnitsAnnotationKey, + route.Annotations[cmapi.SubjectOrganizationalUnitsAnnotationKey]) + r.eventRecorder.Event(route, corev1.EventTypeWarning, ReasonInvalidValue, "annotation "+cmapi.SubjectOrganizationalUnitsAnnotationKey+": "+route.Annotations[cmapi.SubjectOrganizationalUnitsAnnotationKey]+" value is malformed") + return nil, err + } + + } + var countries []string + if metav1.HasAnnotation(route.ObjectMeta, cmapi.SubjectCountriesAnnotationKey) { + subjectCountries, err := cmutil.SplitWithEscapeCSV(route.Annotations[cmapi.SubjectCountriesAnnotationKey]) + countries = subjectCountries + + if err != nil { + r.log.V(1).Error(err, "the countries annotation is invalid", + "object", route.Namespace+"/"+route.Name, cmapi.SubjectCountriesAnnotationKey, + route.Annotations[cmapi.SubjectCountriesAnnotationKey]) + r.eventRecorder.Event(route, corev1.EventTypeWarning, ReasonInvalidValue, "annotation "+cmapi.SubjectCountriesAnnotationKey+": "+route.Annotations[cmapi.SubjectCountriesAnnotationKey]+" value is malformed") + return nil, err + } + } + var provinces []string + if metav1.HasAnnotation(route.ObjectMeta, cmapi.SubjectProvincesAnnotationKey) { + subjectProvinces, err := cmutil.SplitWithEscapeCSV(route.Annotations[cmapi.SubjectProvincesAnnotationKey]) + provinces = subjectProvinces + + if err != nil { + r.log.V(1).Error(err, "the provinces annotation is invalid", + "object", route.Namespace+"/"+route.Name, cmapi.SubjectProvincesAnnotationKey, + route.Annotations[cmapi.SubjectProvincesAnnotationKey]) + r.eventRecorder.Event(route, corev1.EventTypeWarning, ReasonInvalidValue, "annotation "+cmapi.SubjectProvincesAnnotationKey+": "+route.Annotations[cmapi.SubjectProvincesAnnotationKey]+" value is malformed") + return nil, err + } + } + var localities []string + if metav1.HasAnnotation(route.ObjectMeta, cmapi.SubjectLocalitiesAnnotationKey) { + subjectLocalities, err := cmutil.SplitWithEscapeCSV(route.Annotations[cmapi.SubjectLocalitiesAnnotationKey]) + localities = subjectLocalities + + if err != nil { + r.log.V(1).Error(err, "the localities annotation is invalid", + "object", route.Namespace+"/"+route.Name, cmapi.SubjectLocalitiesAnnotationKey, + route.Annotations[cmapi.SubjectLocalitiesAnnotationKey]) + r.eventRecorder.Event(route, corev1.EventTypeWarning, ReasonInvalidValue, "annotation "+cmapi.SubjectLocalitiesAnnotationKey+": "+route.Annotations[cmapi.SubjectLocalitiesAnnotationKey]+" value is malformed") + return nil, err + } + } + var postalCodes []string + if metav1.HasAnnotation(route.ObjectMeta, cmapi.SubjectPostalCodesAnnotationKey) { + subjectPostalCodes, err := cmutil.SplitWithEscapeCSV(route.Annotations[cmapi.SubjectPostalCodesAnnotationKey]) + postalCodes = subjectPostalCodes + + if err != nil { + r.log.V(1).Error(err, "the postal codes annotation is invalid", + "object", route.Namespace+"/"+route.Name, cmapi.SubjectPostalCodesAnnotationKey, + route.Annotations[cmapi.SubjectPostalCodesAnnotationKey]) + r.eventRecorder.Event(route, corev1.EventTypeWarning, ReasonInvalidValue, "annotation "+cmapi.SubjectPostalCodesAnnotationKey+": "+route.Annotations[cmapi.SubjectPostalCodesAnnotationKey]+" value is malformed") + return nil, err + } + } + var streetAddresses []string + if metav1.HasAnnotation(route.ObjectMeta, cmapi.SubjectStreetAddressesAnnotationKey) { + subjectStreetAddresses, err := cmutil.SplitWithEscapeCSV(route.Annotations[cmapi.SubjectStreetAddressesAnnotationKey]) + streetAddresses = subjectStreetAddresses + + if err != nil { + r.log.V(1).Error(err, "the street addresses annotation is invalid", + "object", route.Namespace+"/"+route.Name, cmapi.SubjectStreetAddressesAnnotationKey, + route.Annotations[cmapi.SubjectStreetAddressesAnnotationKey]) + r.eventRecorder.Event(route, corev1.EventTypeWarning, ReasonInvalidValue, "annotation "+cmapi.SubjectStreetAddressesAnnotationKey+": "+route.Annotations[cmapi.SubjectStreetAddressesAnnotationKey]+" value is malformed") + return nil, err + } + } + var serialNumber string + 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] + } else { + issuerName = route.Annotations[cmapi.IssuerNameAnnotationKey] + } + + cr := &cmapi.CertificateRequest{ + 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, + routev1.GroupVersion.WithKind("Route"), + ), + }, + }, + Spec: cmapi.CertificateRequestSpec{ + Duration: &metav1.Duration{Duration: duration}, + 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}, + }, + } + + if route.Spec.TLS != nil && route.Spec.TLS.Termination == routev1.TLSTerminationReencrypt { + cr.Spec.Usages = append(cr.Spec.Usages, cmapi.UsageClientAuth) + } + + return cr, nil +} + +func (r *Route) certificateRequestReadyAndApproved(ctx context.Context, route *routev1.Route, revision int) (bool, *cmapi.CertificateRequest, error) { + cr, err := r.findNextCR(ctx, route, revision) + if err != nil { + return false, nil, err + } + if cr == nil { + r.log.Info("BUG: no certificateRequests 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 + } else { + return false, nil, nil + } +} + +func (r *Route) populateRoute(ctx context.Context, route *routev1.Route, cr *cmapi.CertificateRequest, revision int) error { + // final Sanity checks + var key crypto.Signer + + // get private key from route + k, err := utilpki.DecodePrivateKeyBytes([]byte(route.Annotations[cmapi.IsNextPrivateKeySecretLabelKey])) + if err != nil { + return err + } + key = k + + cert, err := utilpki.DecodeX509CertificateBytes(cr.Status.Certificate) + if err != nil { + return err + } + matches, err := utilpki.PublicKeyMatchesCertificate(key.Public(), cert) + if err != nil { + return err + } + if !matches { + 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, + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, + } + } + encodedKey, err := utilpki.EncodePrivateKey(key, cmapi.PKCS1) + if err != nil { + return err + } + route.Spec.TLS.Key = string(encodedKey) + delete(route.Annotations, cmapi.IsNextPrivateKeySecretLabelKey) + route.Spec.TLS.Certificate = string(cr.Status.Certificate) + + _, err = r.routeClient.RouteV1().Routes(route.Namespace).Update(ctx, route, metav1.UpdateOptions{}) + return err +} + +func (r *Route) 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 + return time.Second * 5 + } + // renew-before overrides default 2/3 behaviour + if metav1.HasAnnotation(route.ObjectMeta, cmapi.RenewBeforeAnnotationKey) { + renewBeforeDuration, err := time.ParseDuration(route.Annotations[cmapi.RenewBeforeAnnotationKey]) + if err != nil { + // duration is invalid + r.eventRecorder.Eventf( + route, + corev1.EventTypeWarning, + ReasonInvalidKey, + "the duration %s: %s is invalid (%s)", + cmapi.RenewBeforeAnnotationKey, + route.Annotations[cmapi.RenewBeforeAnnotationKey], + err.Error(), + ) + } else { + return time.Until(cert.NotAfter.Add(-renewBeforeDuration)) + } + } + certLifetime := cert.NotAfter.Sub(cert.NotBefore) * 2 / 3 + return time.Until(cert.NotBefore.Add(certLifetime)) +} + +func certDurationFromRoute(r *routev1.Route) (time.Duration, error) { + duration := DefaultCertificateDuration + durationAnnotation, exists := r.Annotations[cmapi.DurationAnnotationKey] + if exists { + durationOverride, err := time.ParseDuration(durationAnnotation) + if err != nil { // Not a reconcile error, so stop. + return 0, err + } + duration = durationOverride + } + return duration, nil +} + +// This function returns the hostnames that have been admitted by an Ingress Controller. +// Usually this is just `.spec.host`, but as of OpenShift 4.11 users may also specify `.spec.subdomain`, +// in which case the fully qualified hostname is derived from the hostname of the Ingress Controller. +// In both cases, the final hostname is reflected in `.status.ingress[].host`. +// Note that a Route can be admitted by multiple ingress controllers, so it may have multiple hostnames. +func getRouteHostnames(r *routev1.Route) []string { + hostnames := []string{} + for _, ing := range r.Status.Ingress { + // Iterate over all Ingress Controllers which have admitted the Route + for i := range ing.Conditions { + if ing.Conditions[i].Type == "Admitted" && ing.Conditions[i].Status == "True" { + // The same hostname can be exposed by multiple Ingress routers, + // but we only want a list of unique hostnames. + if !stringInSlice(hostnames, ing.Host) { + hostnames = append(hostnames, ing.Host) + } + } + } + } + + return hostnames +} + +func stringInSlice(slice []string, s string) bool { + for i := range slice { + if slice[i] == s { + return true + } + } + return false +} diff --git a/internal/crcontroller/sync_test.go b/internal/crcontroller/sync_test.go new file mode 100644 index 0000000..6e78264 --- /dev/null +++ b/internal/crcontroller/sync_test.go @@ -0,0 +1,1545 @@ +/* +Copyright 2022 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 crcontroller + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "fmt" + "math/big" + "net" + "net/url" + "sort" + "strconv" + "testing" + "time" + + cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + 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" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/record" +) + +func TestRoute_hasValidCertificate(t *testing.T) { + // set up some cert/key pairs for tests cases + ecdsaKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + ecdsaKeyPEM, err := utilpki.EncodePKCS8PrivateKey(ecdsaKey) + require.NoError(t, err) + anotherEcdsaKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + anotherEcdsaKeyPEM, err := utilpki.EncodePKCS8PrivateKey(anotherEcdsaKey) + require.NoError(t, err) + certTemplate := &x509.Certificate{ + SignatureAlgorithm: x509.ECDSAWithSHA256, + PublicKeyAlgorithm: x509.ECDSA, + Version: 0, + SerialNumber: big.NewInt(12345678), + Issuer: pkix.Name{CommonName: "test-cert"}, + Subject: pkix.Name{CommonName: "test-cert"}, + NotBefore: time.Now().Add(-time.Hour * 24 * 30), + NotAfter: time.Now().Add(time.Hour * 24 * 61), + KeyUsage: x509.KeyUsageCertSign, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}, + BasicConstraintsValid: true, + IsCA: true, + MaxPathLen: 0, + MaxPathLenZero: false, + DNSNames: []string{"some-host.some-domain.tld"}, + } + validEcdsaCert, err := x509.CreateCertificate(rand.Reader, certTemplate, certTemplate, ecdsaKey.Public(), ecdsaKey) + require.NoError(t, err) + validEcdsaCertPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: validEcdsaCert}) + certTemplate.NotAfter = time.Now().Add(time.Hour * 24) + expiringSoonEcdsaCert, err := x509.CreateCertificate(rand.Reader, certTemplate, certTemplate, ecdsaKey.Public(), ecdsaKey) + require.NoError(t, err) + expiringSoonEcdsaCertPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: expiringSoonEcdsaCert}) + + tests := []struct { + name string + route *routev1.Route + want bool + wantedEvents []string + }{ + { + name: "valid and up-to-date ecdsa cert is OK", + route: generateRouteStatus(&routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Annotations: map[string]string{cmapi.IssuerNameAnnotationKey: "some-issuer"}, + }, + Spec: routev1.RouteSpec{ + Host: "some-host.some-domain.tld", + TLS: &routev1.TLSConfig{ + Termination: routev1.TLSTerminationEdge, + Certificate: string(validEcdsaCertPEM), + Key: string(ecdsaKeyPEM), + CACertificate: string(validEcdsaCertPEM), + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, + }, + }, + }, + true), + want: true, + wantedEvents: nil, + }, + { + name: "route with renew-before annotation overrides the default 2/3 lifetime behaviour", + route: generateRouteStatus(&routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Annotations: map[string]string{ + cmapi.IssuerNameAnnotationKey: "some-issuer", + cmapi.RenewBeforeAnnotationKey: "1680h", + }, + }, + Spec: routev1.RouteSpec{ + Host: "some-host.some-domain.tld", + TLS: &routev1.TLSConfig{ + Termination: routev1.TLSTerminationEdge, + Certificate: string(validEcdsaCertPEM), + Key: string(ecdsaKeyPEM), + CACertificate: string(validEcdsaCertPEM), + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, + }, + }, + }, + true), + want: false, + wantedEvents: []string{"Normal Issuing Issuing cert as the renew-before period has been reached"}, + }, + { + name: "expiring soon ecdsa cert triggers a renewal", + route: generateRouteStatus(&routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Annotations: map[string]string{cmapi.IssuerNameAnnotationKey: "some-issuer"}, + }, + Spec: routev1.RouteSpec{ + Host: "some-host.some-domain.tld", + TLS: &routev1.TLSConfig{ + Termination: routev1.TLSTerminationEdge, + Certificate: string(expiringSoonEcdsaCertPEM), + Key: string(ecdsaKeyPEM), + CACertificate: string(expiringSoonEcdsaCertPEM), + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, + }, + }, + }, + true), + want: false, + wantedEvents: []string{"Normal Issuing Issuing cert as the existing cert is more than 2/3 through its validity period"}, + }, + { + name: "cert not matching key triggers a renewal", + route: generateRouteStatus(&routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Annotations: map[string]string{cmapi.IssuerNameAnnotationKey: "some-issuer"}, + }, + Spec: routev1.RouteSpec{ + Host: "some-host.some-domain.tld", + TLS: &routev1.TLSConfig{ + Termination: routev1.TLSTerminationEdge, + Certificate: string(validEcdsaCertPEM), + Key: string(anotherEcdsaKeyPEM), + CACertificate: string(validEcdsaCertPEM), + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, + }, + }, + }, + true), + want: false, + wantedEvents: []string{"Normal Issuing Issuing cert as the public key does not match the certificate"}, + }, + { + name: "junk data in key triggers a renewal", + route: generateRouteStatus(&routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Annotations: map[string]string{cmapi.IssuerNameAnnotationKey: "some-issuer"}, + }, + Spec: routev1.RouteSpec{ + Host: "some-host.some-domain.tld", + TLS: &routev1.TLSConfig{ + Termination: routev1.TLSTerminationEdge, + Certificate: string(validEcdsaCertPEM), + Key: `-----BEGIN PRIVATE KEY----- +SOME GARBAGE +-----END PRIVATE KEY-----`, + CACertificate: string(validEcdsaCertPEM), + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, + }, + }, + }, + true), + want: false, + wantedEvents: []string{"Normal Issuing Issuing cert as the existing key is invalid: error decoding private key PEM block"}, + }, + { + name: "missing private key triggers a renewal", + route: generateRouteStatus(&routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Annotations: map[string]string{cmapi.IssuerNameAnnotationKey: "some-issuer"}, + }, + Spec: routev1.RouteSpec{ + Host: "some-host.some-domain.tld", + TLS: &routev1.TLSConfig{ + Termination: routev1.TLSTerminationEdge, + Certificate: string(validEcdsaCertPEM), + CACertificate: string(validEcdsaCertPEM), + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, + }, + }, + }, + true), + want: false, + wantedEvents: []string{"Normal Issuing Issuing cert as no private key exists"}, + }, + { + name: "junk data in cert triggers a renewal", + route: generateRouteStatus(&routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Annotations: map[string]string{cmapi.IssuerNameAnnotationKey: "some-issuer"}, + }, + Spec: routev1.RouteSpec{ + Host: "some-host.some-domain.tld", + TLS: &routev1.TLSConfig{ + Termination: routev1.TLSTerminationEdge, + Certificate: `-----BEGIN CERTIFICATE----- +SOME GARBAGE +-----END CERTIFICATE-----`, + CACertificate: string(validEcdsaCertPEM), + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, + }, + }, + Status: routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: "some-host.some-domain.tld", + }, + }, + }, + }, + true), + want: false, + wantedEvents: []string{"Normal Issuing Issuing cert as the existing cert is invalid: error decoding certificate PEM block"}, + }, + { + name: "missing cert triggers a renewal", + route: generateRouteStatus(&routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Annotations: map[string]string{cmapi.IssuerNameAnnotationKey: "some-issuer"}, + }, + Spec: routev1.RouteSpec{ + Host: "some-host.some-domain.tld", + TLS: &routev1.TLSConfig{ + Termination: routev1.TLSTerminationEdge, + CACertificate: string(validEcdsaCertPEM), + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, + }, + }, + }, + true), + want: false, + wantedEvents: []string{"Normal Issuing Issuing cert as no certificate exists"}, + }, + { + name: "missing tls config triggers a renewal", + route: generateRouteStatus(&routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Annotations: map[string]string{cmapi.IssuerNameAnnotationKey: "some-issuer"}, + }, + Spec: routev1.RouteSpec{ + Host: "some-host.some-domain.tld", + }, + }, + true), + want: false, + wantedEvents: []string{"Normal Issuing Issuing cert as no TLS is configured"}, + }, + { + name: "route with changed 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)}, + Annotations: map[string]string{cmapi.IssuerNameAnnotationKey: "some-issuer"}, + }, + Spec: routev1.RouteSpec{ + Host: "some-other-host.some-domain.tld", + TLS: &routev1.TLSConfig{ + Termination: routev1.TLSTerminationEdge, + Certificate: string(validEcdsaCertPEM), + Key: string(ecdsaKeyPEM), + CACertificate: string(validEcdsaCertPEM), + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, + }, + }, + }, + true), + want: false, + wantedEvents: []string{ + "Normal Issuing Issuing cert as the hostname does not match the certificate", + }, + }, + { + name: "route with subdomain", + route: generateRouteStatus(&routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-uninitialized-route", + Namespace: "some-namespace", + CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Annotations: map[string]string{cmapi.IssuerNameAnnotationKey: "some-issuer"}, + }, + Spec: routev1.RouteSpec{ + Subdomain: "sub-domain", + }, + }, + true), + want: false, + wantedEvents: []string{ + "Normal Issuing Issuing cert as no TLS is configured", + }, + }, + } + 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.hasValidCertificate(tt.route), "hasValidCertificate() return value") + 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, "hasValidCertificate() events") + }) + } +} + +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) + tests := []struct { + name string + route *routev1.Route + want bool + wantedEvents []string + }{ + { + name: "route has a private key", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Annotations: map[string]string{ + cmapi.IssuerNameAnnotationKey: "some-issuer", + cmapi.IsNextPrivateKeySecretLabelKey: string(ecdsaKeyPEM), + }, + }, + Spec: routev1.RouteSpec{}, + }, + want: true, + wantedEvents: nil, + }, + { + name: "route has no private key", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Annotations: map[string]string{ + cmapi.IssuerNameAnnotationKey: "some-issuer", + }, + }, + Spec: routev1.RouteSpec{}, + }, + want: false, + wantedEvents: nil, + }, + { + name: "route has garbage data in private key", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Annotations: map[string]string{ + cmapi.IssuerNameAnnotationKey: "some-issuer", + cmapi.IsNextPrivateKeySecretLabelKey: `-----BEGIN PRIVATE KEY----- +SOME GARBAGE +-----END PRIVATE KEY-----`, + }, + }, + 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"}, + }, + } + 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{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Annotations: map[string]string{ + cmapi.IssuerNameAnnotationKey: "some-issuer", + }, + }, + Spec: routev1.RouteSpec{}, + }, + want: nil, + wantedEvents: []string{"Normal Issuing Generated Private Key for route"}, + wantedPrivateKeyHeader: "BEGIN RSA PRIVATE KEY", + }, + { + name: "route with rsa algorithm annotation has no private key", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Annotations: map[string]string{ + cmapi.IssuerNameAnnotationKey: "some-issuer", + cmapi.PrivateKeyAlgorithmAnnotationKey: "RSA", + }, + }, + Spec: routev1.RouteSpec{ + Host: "some-host.some-domain.tld", + }, + }, + want: nil, + wantedEvents: []string{"Normal Issuing Generated Private Key for route"}, + wantedPrivateKeyHeader: "BEGIN RSA PRIVATE KEY", + }, + { + name: "route with ecdsa algorithm annotation has no private key", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Annotations: map[string]string{ + cmapi.IssuerNameAnnotationKey: "some-issuer", + cmapi.PrivateKeyAlgorithmAnnotationKey: "ECDSA", + }, + }, + Spec: routev1.RouteSpec{ + Host: "some-host.some-domain.tld", + }, + }, + want: nil, + wantedEvents: []string{"Normal Issuing Generated Private Key for route"}, + wantedPrivateKeyHeader: "BEGIN EC PRIVATE KEY", + }, + { + name: "route with invalid algorithm annotation has no private key", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Annotations: map[string]string{ + cmapi.IssuerNameAnnotationKey: "some-issuer", + cmapi.PrivateKeyAlgorithmAnnotationKey: "notreal", + }, + }, + Spec: routev1.RouteSpec{ + Host: "some-host.some-domain.tld", + }, + }, + want: fmt.Errorf("invalid private key algorithm: notreal"), + wantedEvents: []string{"Warning InvalidPrivateKeyAlgorithm invalid private key algorithm: notreal"}, + wantedPrivateKeyHeader: "", + }, + } + 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{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Annotations: map[string]string{ + cmapi.IssuerNameAnnotationKey: "some-issuer", + cmapi.CertificateRequestRevisionAnnotationKey: "1337", + }, + }, + Spec: routev1.RouteSpec{}, + }, + want: 1337, + wantErr: nil, + }, + { + name: "route without revision", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Annotations: map[string]string{ + cmapi.IssuerNameAnnotationKey: "some-issuer", + }, + }, + Spec: routev1.RouteSpec{}, + }, + want: 0, + wantErr: fmt.Errorf("no revision found"), + }, + } + 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{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + CreationTimestamp: metav1.Time{Time: time.Now().Add(-time.Hour * 24 * 30)}, + Annotations: map[string]string{ + cmapi.IssuerNameAnnotationKey: "some-issuer", + }, + }, + Spec: routev1.RouteSpec{}, + }, + revision: 1337, + want: "1337", + 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) + + 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, + route: generateRouteStatus(&routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + Annotations: map[string]string{ + cmapi.DurationAnnotationKey: "42m", + cmapi.IsNextPrivateKeySecretLabelKey: string(rsaPEM), + }, + }, + Spec: routev1.RouteSpec{ + Host: "some-host.some-domain.tld", + }, + Status: routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: "some-host.some-domain.tld", + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "True", + }, + }, + }, + }, + }, + }, + true), + want: &cmapi.CertificateRequest{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "some-route-", + Namespace: "some-namespace", + Annotations: map[string]string{ + cmapi.CertificateRequestRevisionAnnotationKey: "1338", + }, + }, + Spec: cmapi.CertificateRequestSpec{ + Duration: &metav1.Duration{Duration: 42 * time.Minute}, + IsCA: false, + Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + }, + }, + wantErr: nil, + }, + { + name: "Basic test with issuer", + revision: 1337, + route: generateRouteStatus(&routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + Annotations: map[string]string{ + cmapi.DurationAnnotationKey: "42m", + cmapi.IsNextPrivateKeySecretLabelKey: string(rsaPEM), + cmapi.IssuerNameAnnotationKey: "self-signed-issuer", + cmapi.IssuerKindAnnotationKey: "Issuer", + }, + }, + Spec: routev1.RouteSpec{ + Host: "some-host.some-domain.tld", + }, + Status: routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: "some-host.some-domain.tld", + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "True", + }, + }, + }, + }, + }, + }, + true), + want: &cmapi.CertificateRequest{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "some-route-", + Namespace: "some-namespace", + Annotations: map[string]string{ + cmapi.CertificateRequestRevisionAnnotationKey: "1338", + }, + }, + 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", + }, + }, + }, + wantErr: nil, + }, + { + name: "Basic test with external issuer", + revision: 1337, + route: generateRouteStatus(&routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + 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", + }, + }, + Spec: routev1.RouteSpec{ + Host: "some-host.some-domain.tld", + }, + Status: routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: "some-host.some-domain.tld", + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "True", + }, + }, + }, + }, + }, + }, + true), + want: &cmapi.CertificateRequest{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "some-route-", + Namespace: "some-namespace", + Annotations: map[string]string{ + cmapi.CertificateRequestRevisionAnnotationKey: "1338", + }, + }, + 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", + }, + }, + }, + wantErr: nil, + }, + { + name: "Basic test with alternate ingress issuer name annotation", + revision: 1337, + route: generateRouteStatus(&routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + 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", + }, + }, + Spec: routev1.RouteSpec{ + Host: "some-host.some-domain.tld", + }, + Status: routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: "some-host.some-domain.tld", + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "True", + }, + }, + }, + }, + }, + }, + true), + want: &cmapi.CertificateRequest{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "some-route-", + Namespace: "some-namespace", + Annotations: map[string]string{ + cmapi.CertificateRequestRevisionAnnotationKey: "1338", + }, + }, + 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", + }, + }, + }, + wantErr: nil, + }, + { + name: "With subdomain and multiple ICs", + revision: 1337, + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route-with-subdomain", + Namespace: "some-namespace", + Annotations: map[string]string{ + cmapi.IsNextPrivateKeySecretLabelKey: string(rsaPEM), + }, + }, + Spec: routev1.RouteSpec{ + 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: &cmapi.CertificateRequest{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "some-route-with-subdomain-", + Namespace: "some-namespace", + Annotations: map[string]string{ + cmapi.CertificateRequestRevisionAnnotationKey: "1338", + }, + }, + Spec: cmapi.CertificateRequestSpec{ + Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + }, + }, + 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, + route: generateRouteStatus(&routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + Annotations: map[string]string{ + cmapi.IsNextPrivateKeySecretLabelKey: string(ecdsaPEM), + cmapi.PrivateKeyAlgorithmAnnotationKey: string(cmapi.ECDSAKeyAlgorithm), + }, + }, + Spec: routev1.RouteSpec{ + Host: "some-host.some-domain.tld", + }, + Status: routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: "some-host.some-domain.tld", + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "True", + }, + }, + }, + }, + }, + }, + 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.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, + route: generateRouteStatus(&routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + Annotations: map[string]string{ + cmapi.IsNextPrivateKeySecretLabelKey: string(ecdsaPEM), + cmapi.PrivateKeyAlgorithmAnnotationKey: string(cmapi.ECDSAKeyAlgorithm), + cmapi.PrivateKeySizeAnnotationKey: strconv.Itoa(384), + }, + }, + Spec: routev1.RouteSpec{ + Host: "some-host.some-domain.tld", + }, + Status: routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: "some-host.some-domain.tld", + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "True", + }, + }, + }, + }, + }, + }, + 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.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, + route: generateRouteStatus(&routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + Annotations: map[string]string{ + cmapi.IsNextPrivateKeySecretLabelKey: string(ecdsaPEM), + cmapi.PrivateKeyAlgorithmAnnotationKey: string(cmapi.ECDSAKeyAlgorithm), + cmapi.PrivateKeySizeAnnotationKey: strconv.Itoa(521), + }, + }, + Spec: routev1.RouteSpec{ + Host: "some-host.some-domain.tld", + }, + Status: routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: "some-host.some-domain.tld", + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "True", + }, + }, + }, + }, + }, + }, + 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.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, + route: generateRouteStatus(&routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + Annotations: map[string]string{ + cmapi.IsNextPrivateKeySecretLabelKey: string(rsaPEM), + cmapi.PrivateKeyAlgorithmAnnotationKey: string(cmapi.RSAKeyAlgorithm), + }, + }, + Spec: routev1.RouteSpec{ + Host: "some-host.some-domain.tld", + }, + Status: routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: "some-host.some-domain.tld", + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "True", + }, + }, + }, + }, + }, + }, + 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.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, + route: generateRouteStatus(&routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + Annotations: map[string]string{ + cmapi.IsNextPrivateKeySecretLabelKey: string(rsaPEM), + cmapi.PrivateKeyAlgorithmAnnotationKey: string(cmapi.RSAKeyAlgorithm), + cmapi.PrivateKeySizeAnnotationKey: strconv.Itoa(3072), + }, + }, + Spec: routev1.RouteSpec{ + Host: "some-host.some-domain.tld", + }, + Status: routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: "some-host.some-domain.tld", + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "True", + }, + }, + }, + }, + }, + }, + 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.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, + route: generateRouteStatus(&routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route", + Namespace: "some-namespace", + Annotations: map[string]string{ + cmapi.IsNextPrivateKeySecretLabelKey: string(rsaPEM), + cmapi.PrivateKeyAlgorithmAnnotationKey: string(cmapi.RSAKeyAlgorithm), + cmapi.PrivateKeySizeAnnotationKey: strconv.Itoa(4096), + }, + }, + Spec: routev1.RouteSpec{ + Host: "some-host.some-domain.tld", + }, + Status: routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: "some-host.some-domain.tld", + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "True", + }, + }, + }, + }, + }, + }, + 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, + }, + { + name: "With subject annotations", + revision: 1337, + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route-with-subject-annotations", + 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", + }, + }, + Spec: routev1.RouteSpec{ + Host: "example-route.example.com", + }, + Status: routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: "example-route.example.com", + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "True", + }, + }, + }, + }, + }, + }, + want: &cmapi.CertificateRequest{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "some-route-with-subject-annotations-", + Namespace: "some-namespace", + Annotations: map[string]string{ + cmapi.CertificateRequestRevisionAnnotationKey: "1338", + }, + }, + Spec: cmapi.CertificateRequestSpec{ + Duration: &metav1.Duration{Duration: DefaultCertificateDuration}, + Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + }, + }, + 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, + }, + { + name: "With all annotations", + revision: 1337, + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-route-with-all-annotations", + 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", + }, + }, + Spec: routev1.RouteSpec{ + Host: "example-route.example.com", + }, + Status: routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: "example-route.example.com", + Conditions: []routev1.RouteIngressCondition{ + { + Type: "Admitted", + Status: "True", + }, + }, + }, + }, + }, + }, + want: &cmapi.CertificateRequest{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "some-route-with-all-annotations-", + Namespace: "some-namespace", + Annotations: map[string]string{ + cmapi.CertificateRequestRevisionAnnotationKey: "1338", + }, + }, + Spec: cmapi.CertificateRequestSpec{ + Duration: &metav1.Duration{Duration: time.Hour * 24 * 30}, + Usages: []cmapi.KeyUsage{cmapi.UsageServerAuth, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}, + }, + }, + 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, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + recorder := record.NewFakeRecorder(100) + r := &Route{ + eventRecorder: recorder, + } + // test "buildNextCR" function + cr, err := r.buildNextCR(context.TODO(), tt.route, tt.revision) + + // check that we got the expected error (including nil) + assert.Equal(t, tt.wantErr, err, "buildNextCR()") + + // 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) + } + } + + // 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") + } + + }) + } +} + +// 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 + } + + 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 +} diff --git a/make/test-smoke.mk b/make/test-smoke.mk index ba29dbc..deda65b 100644 --- a/make/test-smoke.mk +++ b/make/test-smoke.mk @@ -56,7 +56,20 @@ test-smoke-deps: smoke-setup-routes-crd test-smoke-deps: install .PHONY: test-smoke -## Smoke end-to-end tests +## Smoke end-to-end tests using Certificates to issue certs ## @category Testing test-smoke: test-smoke-deps | kind-cluster ./test/test-smoke.sh + +test-smoke-cr-deps: INSTALL_OPTIONS := +test-smoke-cr-deps: INSTALL_OPTIONS += --set image.repository=$(oci_manager_image_name_development) +test-smoke-cr-deps: INSTALL_OPTIONS += --set issuanceMode=certificateRequest +test-smoke-cr-deps: smoke-setup-cert-manager +test-smoke-cr-deps: smoke-setup-routes-crd +test-smoke-cr-deps: install + +.PHONY: test-smoke-cr +## Smoke end-to-end tests using CertificateRequests to issue certs +## @category Testing +test-smoke-cr: test-smoke-cr-deps | kind-cluster + ./test/test-smoke.sh