diff --git a/pkg/controller/certificatesigningrequests/BUILD.bazel b/pkg/controller/certificatesigningrequests/BUILD.bazel index 2b1eec407..e809f39d4 100644 --- a/pkg/controller/certificatesigningrequests/BUILD.bazel +++ b/pkg/controller/certificatesigningrequests/BUILD.bazel @@ -25,6 +25,7 @@ go_library( "@io_k8s_apimachinery//pkg/api/errors:go_default_library", "@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library", "@io_k8s_apimachinery//pkg/labels:go_default_library", + "@io_k8s_apimachinery//pkg/runtime/schema:go_default_library", "@io_k8s_client_go//kubernetes/typed/authorization/v1:go_default_library", "@io_k8s_client_go//kubernetes/typed/certificates/v1:go_default_library", "@io_k8s_client_go//listers/certificates/v1:go_default_library", @@ -69,6 +70,7 @@ filegroup( name = "all-srcs", srcs = [ ":package-srcs", + "//pkg/controller/certificatesigningrequests/acme:all-srcs", "//pkg/controller/certificatesigningrequests/ca:all-srcs", "//pkg/controller/certificatesigningrequests/fake:all-srcs", "//pkg/controller/certificatesigningrequests/selfsigned:all-srcs", diff --git a/pkg/controller/certificatesigningrequests/acme/BUILD.bazel b/pkg/controller/certificatesigningrequests/acme/BUILD.bazel new file mode 100644 index 000000000..258a2ec33 --- /dev/null +++ b/pkg/controller/certificatesigningrequests/acme/BUILD.bazel @@ -0,0 +1,70 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = ["acme.go"], + importpath = "github.com/jetstack/cert-manager/pkg/controller/certificatesigningrequests/acme", + visibility = ["//visibility:public"], + deps = [ + "//pkg/acme:go_default_library", + "//pkg/api/util:go_default_library", + "//pkg/apis/acme/v1:go_default_library", + "//pkg/apis/certmanager/v1:go_default_library", + "//pkg/apis/experimental/v1alpha1:go_default_library", + "//pkg/apis/meta/v1:go_default_library", + "//pkg/client/clientset/versioned/typed/acme/v1:go_default_library", + "//pkg/client/listers/acme/v1:go_default_library", + "//pkg/controller:go_default_library", + "//pkg/controller/certificatesigningrequests:go_default_library", + "//pkg/controller/certificatesigningrequests/util:go_default_library", + "//pkg/logs:go_default_library", + "//pkg/util:go_default_library", + "//pkg/util/pki:go_default_library", + "@io_k8s_api//certificates/v1:go_default_library", + "@io_k8s_api//core/v1:go_default_library", + "@io_k8s_apimachinery//pkg/api/errors:go_default_library", + "@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library", + "@io_k8s_apimachinery//pkg/runtime/schema:go_default_library", + "@io_k8s_client_go//kubernetes/typed/certificates/v1:go_default_library", + "@io_k8s_client_go//tools/record:go_default_library", + ], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) + +go_test( + name = "go_default_test", + srcs = ["acme_test.go"], + embed = [":go_default_library"], + deps = [ + "//pkg/api/util:go_default_library", + "//pkg/apis/acme/v1:go_default_library", + "//pkg/apis/certmanager:go_default_library", + "//pkg/apis/certmanager/v1:go_default_library", + "//pkg/apis/meta/v1:go_default_library", + "//pkg/controller/certificatesigningrequests:go_default_library", + "//pkg/controller/certificatesigningrequests/util:go_default_library", + "//pkg/controller/test:go_default_library", + "//pkg/util/pki:go_default_library", + "//test/unit/gen:go_default_library", + "@io_k8s_api//authorization/v1:go_default_library", + "@io_k8s_api//certificates/v1:go_default_library", + "@io_k8s_api//core/v1:go_default_library", + "@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library", + "@io_k8s_apimachinery//pkg/runtime:go_default_library", + "@io_k8s_client_go//testing:go_default_library", + "@io_k8s_utils//clock/testing:go_default_library", + ], +) diff --git a/pkg/controller/certificatesigningrequests/acme/acme.go b/pkg/controller/certificatesigningrequests/acme/acme.go new file mode 100644 index 000000000..33d635b37 --- /dev/null +++ b/pkg/controller/certificatesigningrequests/acme/acme.go @@ -0,0 +1,320 @@ +/* +Copyright 2021 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 acme + +import ( + "context" + "crypto/x509" + "errors" + "fmt" + + certificatesv1 "k8s.io/api/certificates/v1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + certificatesclient "k8s.io/client-go/kubernetes/typed/certificates/v1" + "k8s.io/client-go/tools/record" + + "github.com/jetstack/cert-manager/pkg/acme" + apiutil "github.com/jetstack/cert-manager/pkg/api/util" + cmacme "github.com/jetstack/cert-manager/pkg/apis/acme/v1" + cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" + experimentalapi "github.com/jetstack/cert-manager/pkg/apis/experimental/v1alpha1" + cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1" + cmacmeclientset "github.com/jetstack/cert-manager/pkg/client/clientset/versioned/typed/acme/v1" + cmacmelisters "github.com/jetstack/cert-manager/pkg/client/listers/acme/v1" + controllerpkg "github.com/jetstack/cert-manager/pkg/controller" + "github.com/jetstack/cert-manager/pkg/controller/certificatesigningrequests" + ctrlutil "github.com/jetstack/cert-manager/pkg/controller/certificatesigningrequests/util" + logf "github.com/jetstack/cert-manager/pkg/logs" + "github.com/jetstack/cert-manager/pkg/util" + "github.com/jetstack/cert-manager/pkg/util/pki" +) + +const ( + CSRControllerName = "certificatesigningrequests-issuer-acme" +) + +// ACME is a Kubernetes CertificateSigningRequest controller, responsible for +// signing CertificateSigningRequests that reference a cert-manager ACME Issuer +// or ClusterIssuer +type ACME struct { + issuerOptions controllerpkg.IssuerOptions + + orderLister cmacmelisters.OrderLister + acmeClientV cmacmeclientset.AcmeV1Interface + certClient certificatesclient.CertificateSigningRequestInterface + + recorder record.EventRecorder +} + +func init() { + // create certificate request controller for acme issuer + controllerpkg.Register(CSRControllerName, func(ctx *controllerpkg.Context) (controllerpkg.Interface, error) { + return controllerpkg.NewBuilder(ctx, CSRControllerName). + For(certificatesigningrequests.New(apiutil.IssuerACME, NewACME(ctx), ctx.SharedInformerFactory.Acme().V1().Orders().Informer())). + Complete() + }) +} + +func NewACME(ctx *controllerpkg.Context) *ACME { + return &ACME{ + issuerOptions: ctx.IssuerOptions, + orderLister: ctx.SharedInformerFactory.Acme().V1().Orders().Lister(), + acmeClientV: ctx.CMClient.AcmeV1(), + certClient: ctx.Client.CertificatesV1().CertificateSigningRequests(), + recorder: ctx.Recorder, + } +} + +// Sign attempts to sign the given CertificateSigningRequest based on the +// provided ACME Issuer or ClusterIssuer. +// +// If no order exists for a CertificateSigningRequest, an order is constructed +// and sent back to the Kubernetes API server for processing. The order +// controller then processes the order. The CertificateSigningRequest is then +// updated with the result. +func (a *ACME) Sign(ctx context.Context, csr *certificatesv1.CertificateSigningRequest, issuerObj cmapi.GenericIssuer) error { + log := logf.FromContext(ctx, "sign") + + // If we can't decode the CSR PEM we have to hard fail + req, err := pki.DecodeX509CertificateRequestBytes(csr.Spec.Request) + if err != nil { + message := fmt.Sprintf("Failed to decode CSR in spec.request: %s", err) + log.Error(err, message) + a.recorder.Event(csr, corev1.EventTypeWarning, "RequestParsingError", message) + ctrlutil.CertificateSigningRequestSetFailed(csr, "RequestParsingError", message) + _, err = a.certClient.UpdateStatus(ctx, csr, metav1.UpdateOptions{}) + return err + } + + // If the CommonName is also not present in the DNS names or IP Addresses of the Request then hard fail. + if len(req.Subject.CommonName) > 0 && !util.Contains(req.DNSNames, req.Subject.CommonName) && !util.Contains(pki.IPAddressesToString(req.IPAddresses), req.Subject.CommonName) { + err = fmt.Errorf("%q does not exist in %s or %s", req.Subject.CommonName, req.DNSNames, pki.IPAddressesToString(req.IPAddresses)) + message := fmt.Sprintf("The CSR PEM requests a commonName that is not present in the list of dnsNames or ipAddresses. If a commonName is set, ACME requires that the value is also present in the list of dnsNames or ipAddresses: %s", err) + + log.Error(err, message) + a.recorder.Event(csr, corev1.EventTypeWarning, "InvalidOrder", message) + ctrlutil.CertificateSigningRequestSetFailed(csr, "InvalidOrder", message) + _, err = a.certClient.UpdateStatus(ctx, csr, metav1.UpdateOptions{}) + return err + } + + // If we fail to build the order we have to hard fail. + expectedOrder, err := a.buildOrder(csr, req, issuerObj) + if err != nil { + message := fmt.Sprintf("Failed to build order: %s", err) + + log.Error(err, message) + a.recorder.Event(csr, corev1.EventTypeWarning, "OrderBuildingError", message) + ctrlutil.CertificateSigningRequestSetFailed(csr, "OrderBuildingError", message) + _, err = a.certClient.UpdateStatus(ctx, csr, metav1.UpdateOptions{}) + return err + } + + order, err := a.orderLister.Orders(expectedOrder.Namespace).Get(expectedOrder.Name) + if apierrors.IsNotFound(err) { + _, err = a.acmeClientV.Orders(expectedOrder.Namespace).Create(ctx, expectedOrder, metav1.CreateOptions{}) + if err != nil { + // Failing to create the order here is most likely network related. + // We should backoff and keep trying. + message := fmt.Sprintf("Failed create new order resource %s/%s", expectedOrder.Namespace, expectedOrder.Name) + log.Error(err, message) + return err + } + + message := fmt.Sprintf("Created Order resource %s/%s", + expectedOrder.Namespace, expectedOrder.Name) + a.recorder.Event(csr, corev1.EventTypeNormal, "OrderCreated", message) + log.V(logf.DebugLevel).Info(message) + return nil + } + + if err != nil { + // We are probably in a network error here so we should backoff and retry + message := fmt.Sprintf("Failed to get order resource %s/%s", expectedOrder.Namespace, expectedOrder.Name) + log.Error(err, message) + return err + } + + if !metav1.IsControlledBy(order, csr) { + // This error should never really happen since CertificateSigningRequests + // are cluster scoped and so hashes won't conflict. This is likely from + // someone manually creating the order out of band. We can only error. + return errors.New("found Order resource not owned by this CertificateSigningRequest, retrying") + } + + log = logf.WithRelatedResource(log, order) + + // If the acme order has failed then so too does the + // CertificateSigningRequest meet the same fate. + if acme.IsFailureState(order.Status.State) { + err := fmt.Errorf("order is in %q state: %s", order.Status.State, order.Status.Reason) + message := fmt.Sprintf("Failed to wait for order resource %s/%s to become ready: %s", expectedOrder.Namespace, expectedOrder.Name, err) + + log.Error(err, message) + a.recorder.Event(csr, corev1.EventTypeWarning, "OrderFailed", message) + ctrlutil.CertificateSigningRequestSetFailed(csr, "OrderFailed", message) + _, err = a.certClient.UpdateStatus(ctx, csr, metav1.UpdateOptions{}) + return err + } + + if order.Status.State != cmacme.Valid { + a.recorder.Event(csr, corev1.EventTypeNormal, "OrderPending", + fmt.Sprintf("Waiting on certificate issuance from order %s/%s: %q", + expectedOrder.Namespace, order.Name, order.Status.State)) + + log.V(logf.DebugLevel).Info("acme Order resource is not in a ready state, waiting...") + return nil + } + + if len(order.Status.Certificate) == 0 { + a.recorder.Event(csr, corev1.EventTypeNormal, "OrderPending", + fmt.Sprintf("Waiting for order-controller to add certificate data to Order %s/%s", + expectedOrder.Namespace, order.Name)) + + log.V(logf.DebugLevel).Info("order controller has not added certificate data to the Order, waiting...") + return nil + } + + x509Cert, err := pki.DecodeX509CertificateBytes(order.Status.Certificate) + if err != nil { + message := fmt.Sprintf("Deleting Order with bad certificate: %s", err) + a.recorder.Event(csr, corev1.EventTypeWarning, "OrderBadCertificate", message) + log.Error(err, "failed to decode x509 certificate data on Order resource.") + // Deleting the order here will cause a re-sync since the Order is owned by + // this CertificateSigningRequest + return a.acmeClientV.Orders(order.Namespace).Delete(ctx, order.Name, metav1.DeleteOptions{}) + } + + if ok, err := pki.PublicKeyMatchesCertificate(req.PublicKey, x509Cert); err != nil || !ok { + a.recorder.Event(csr, corev1.EventTypeWarning, "OrderBadCertificate", "Deleting Order as the signed certificate's key does not match the request") + log.Error(err, "The public key in Order.Status.Certificate does not match the public key in CertificateSigningRequest.Spec.Request. Deleting the order.") + // Deleting the order here will cause a re-sync since the Order is owned by + // this CertificateSigningRequest + return a.acmeClientV.Orders(order.Namespace).Delete(ctx, order.Name, metav1.DeleteOptions{}) + } + + // Update the status.certificate first so that the sync from updating will + // not cause another issuance before setting the CA. + csr.Status.Certificate = order.Status.Certificate + csr, err = a.certClient.UpdateStatus(ctx, csr, metav1.UpdateOptions{}) + if err != nil { + message := "Error updating certificate" + a.recorder.Eventf(csr, corev1.EventTypeWarning, "SigningError", "%s: %s", message, err) + return err + } + + if csr.Annotations == nil { + csr.Annotations = make(map[string]string) + } + csr.Annotations[experimentalapi.CertificateSigningRequestCAAnnotationKey] = "" + _, err = a.certClient.Update(ctx, csr, metav1.UpdateOptions{}) + if err != nil { + message := fmt.Sprintf("Error setting %q", experimentalapi.CertificateSigningRequestCAAnnotationKey) + a.recorder.Eventf(csr, corev1.EventTypeWarning, "SigningError", "%s: %s", message, err) + return err + } + + log.V(logf.DebugLevel).Info("certificate issued") + a.recorder.Event(csr, corev1.EventTypeNormal, "CertificateIssued", "Certificate fetched from issuer successfully") + + return nil +} + +// Build order. If we error here it is a terminating failure. +func (a *ACME) buildOrder(csr *certificatesv1.CertificateSigningRequest, req *x509.CertificateRequest, iss cmapi.GenericIssuer) (*cmacme.Order, error) { + var ipAddresses []string + for _, ip := range req.IPAddresses { + ipAddresses = append(ipAddresses, ip.String()) + } + + var dnsNames []string + if req.DNSNames != nil { + dnsNames = req.DNSNames + } + + ref, ok := ctrlutil.SignerIssuerRefFromSignerName(csr.Spec.SignerName) + if !ok { + return nil, errors.New("failed to construct issuer reference from signer name") + } + + kind, ok := ctrlutil.IssuerKindFromType(ref.Type) + if !ok { + return nil, errors.New("failed to construct issuer kind from signer name") + } + + spec := cmacme.OrderSpec{ + Request: csr.Spec.Request, + IssuerRef: cmmeta.ObjectReference{ + Name: ref.Name, + Kind: kind, + Group: ref.Group, + }, + CommonName: req.Subject.CommonName, + DNSNames: dnsNames, + IPAddresses: ipAddresses, + } + + if iss.GetSpec().ACME.EnableDurationFeature { + duration, err := pki.DurationFromCertificateSigningRequest(csr) + if err != nil { + return nil, err + } + spec.Duration = &metav1.Duration{Duration: duration} + } + + computeNameSpec := spec.DeepCopy() + // create a deep copy of the OrderSpec so we can overwrite the Request and NotAfter field + computeNameSpec.Request = nil + + var hashObj interface{} + hashObj = computeNameSpec + if len(csr.Name) >= 52 { + // Pass a unique struct for hashing so that names at or longer than 52 characters + // receive a unique hash. Otherwise, orders will have truncated names with colliding + // hashes, possibly leading to non-renewal. + hashObj = struct { + CSRName string `json:"certificateSigningRequestName"` + Spec *cmacme.OrderSpec `json:"spec"` + }{ + CSRName: csr.Name, + Spec: computeNameSpec, + } + } + name, err := apiutil.ComputeName(csr.Name, hashObj) + if err != nil { + return nil, err + } + + // truncate certificate name so final name will be <= 63 characters. + // hash (uint32) will be at most 10 digits long, and we account for + // the hyphen. + return &cmacme.Order{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: a.issuerOptions.ResourceNamespace(iss), + Labels: csr.Labels, + Annotations: csr.Annotations, + OwnerReferences: []metav1.OwnerReference{ + *metav1.NewControllerRef(csr, schema.GroupVersionKind{Group: "certificates.k8s.io", Version: "v1", Kind: "CertificateSigningRequest"}), + }, + }, + Spec: spec, + }, nil +} diff --git a/pkg/controller/certificatesigningrequests/acme/acme_test.go b/pkg/controller/certificatesigningrequests/acme/acme_test.go new file mode 100644 index 000000000..0feadf234 --- /dev/null +++ b/pkg/controller/certificatesigningrequests/acme/acme_test.go @@ -0,0 +1,949 @@ +/* +Copyright 2021 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 acme + +import ( + "context" + "crypto/x509" + "reflect" + "testing" + "time" + + apiutil "github.com/jetstack/cert-manager/pkg/api/util" + authzv1 "k8s.io/api/authorization/v1" + certificatesv1 "k8s.io/api/certificates/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + coretesting "k8s.io/client-go/testing" + fakeclock "k8s.io/utils/clock/testing" + + cmacme "github.com/jetstack/cert-manager/pkg/apis/acme/v1" + "github.com/jetstack/cert-manager/pkg/apis/certmanager" + cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" + cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1" + "github.com/jetstack/cert-manager/pkg/controller/certificatesigningrequests" + "github.com/jetstack/cert-manager/pkg/controller/certificatesigningrequests/util" + testpkg "github.com/jetstack/cert-manager/pkg/controller/test" + "github.com/jetstack/cert-manager/pkg/util/pki" + "github.com/jetstack/cert-manager/test/unit/gen" +) + +var ( + fixedClockStart = time.Now() + fixedClock = fakeclock.NewFakeClock(fixedClockStart) +) + +func TestProcessItem(t *testing.T) { + metaFixedClockStart := metav1.NewTime(fixedClockStart) + util.Clock = fixedClock + + baseIssuer := gen.Issuer("test-issuer", + gen.SetIssuerACME(cmacme.ACMEIssuer{}), + gen.AddIssuerCondition(cmapi.IssuerCondition{ + Type: cmapi.IssuerConditionReady, + Status: cmmeta.ConditionTrue, + }), + ) + + csrPEM, sk, err := gen.CSR(x509.ECDSA, + gen.SetCSRCommonName("example.com"), + gen.SetCSRDNSNames("example.com"), + ) + if err != nil { + t.Fatal(err) + } + + req, err := pki.DecodeX509CertificateRequestBytes(csrPEM) + if err != nil { + t.Fatal(err) + } + + csrPEMExampleNotPresent, skExampleNotPresent, err := gen.CSR(x509.ECDSA, + gen.SetCSRCommonName("example.com"), + gen.SetCSRDNSNames("foo.com"), + ) + if err != nil { + t.Fatal(err) + } + + baseCSR := gen.CertificateSigningRequest("test-csr", + gen.SetCertificateSigningRequestRequest(csrPEM), + gen.SetCertificateSigningRequestSignerName("issuers.cert-manager.io/default-unit-test-ns.test-issuer"), + gen.SetCertificateSigningRequestDuration("1440h"), + gen.SetCertificateSigningRequestUsername("user-1"), + gen.SetCertificateSigningRequestGroups([]string{"group-1", "group-2"}), + gen.SetCertificateSigningRequestUID("uid-1"), + gen.SetCertificateSigningRequestExtra(map[string]certificatesv1.ExtraValue{ + "extra": []string{"1", "2"}, + }), + ) + + tmpl, err := pki.GenerateTemplateFromCertificateSigningRequest(baseCSR) + if err != nil { + t.Fatal(err) + } + certPEM, _, err := pki.SignCertificate(tmpl, tmpl, sk.Public(), sk) + if err != nil { + t.Fatal(err) + } + + tmpl, err = pki.GenerateTemplateFromCertificateSigningRequest(gen.CertificateSigningRequestFrom(baseCSR, + gen.SetCertificateSigningRequestRequest(csrPEMExampleNotPresent), + )) + if err != nil { + t.Fatal(err) + } + certPEMExampleNotPresent, _, err := pki.SignCertificate(tmpl, tmpl, skExampleNotPresent.Public(), skExampleNotPresent) + if err != nil { + t.Fatal(err) + } + + baseOrder, err := new(ACME).buildOrder(baseCSR, req, baseIssuer) + if err != nil { + t.Fatal(err) + } + + tests := map[string]struct { + builder *testpkg.Builder + csr *certificatesv1.CertificateSigningRequest + expectedErr bool + }{ + "a CertificateSigningRequest without an approved condition should do nothing": { + csr: gen.CertificateSigningRequestFrom(baseCSR), + builder: &testpkg.Builder{ + CertManagerObjects: []runtime.Object{baseIssuer.DeepCopy()}, + }, + }, + "a CertificateSigningRequest with a denied condition should do nothing": { + csr: gen.CertificateSigningRequestFrom(baseCSR, + gen.SetCertificateSigningRequestStatusCondition(certificatesv1.CertificateSigningRequestCondition{ + Type: certificatesv1.CertificateDenied, + Status: corev1.ConditionTrue, + }), + ), + builder: &testpkg.Builder{ + CertManagerObjects: []runtime.Object{baseIssuer.DeepCopy()}, + ExpectedEvents: []string{}, + ExpectedActions: nil, + }, + }, + "an approved CSR that contains a garbage request should be marked as Failed": { + csr: gen.CertificateSigningRequestFrom(baseCSR, + gen.SetCertificateSigningRequestRequest([]byte("garbage-data")), + gen.SetCertificateSigningRequestStatusCondition(certificatesv1.CertificateSigningRequestCondition{ + Type: certificatesv1.CertificateApproved, + Status: corev1.ConditionTrue, + }), + ), + builder: &testpkg.Builder{ + CertManagerObjects: []runtime.Object{baseIssuer.DeepCopy()}, + ExpectedEvents: []string{ + "Warning RequestParsingError Failed to decode CSR in spec.request: error decoding certificate request PEM block", + }, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewCreateAction( + authzv1.SchemeGroupVersion.WithResource("subjectaccessreviews"), + "", + &authzv1.SubjectAccessReview{ + Spec: authzv1.SubjectAccessReviewSpec{ + User: "user-1", + Groups: []string{"group-1", "group-2"}, + Extra: map[string]authzv1.ExtraValue{ + "extra": []string{"1", "2"}, + }, + UID: "uid-1", + + ResourceAttributes: &authzv1.ResourceAttributes{ + Group: certmanager.GroupName, + Resource: "signers", + Verb: "reference", + Namespace: baseIssuer.Namespace, + Name: baseIssuer.Name, + Version: "*", + }, + }, + }, + )), + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( + certificatesv1.SchemeGroupVersion.WithResource("certificatesigningrequests"), + "status", + "", + gen.CertificateSigningRequestFrom(baseCSR.DeepCopy(), + gen.SetCertificateSigningRequestRequest([]byte("garbage-data")), + gen.SetCertificateSigningRequestStatusCondition(certificatesv1.CertificateSigningRequestCondition{ + Type: certificatesv1.CertificateApproved, + Status: corev1.ConditionTrue, + }), + gen.SetCertificateSigningRequestStatusCondition(certificatesv1.CertificateSigningRequestCondition{ + Type: certificatesv1.CertificateFailed, + Status: corev1.ConditionTrue, + Reason: "RequestParsingError", + Message: "Failed to decode CSR in spec.request: error decoding certificate request PEM block", + LastTransitionTime: metaFixedClockStart, + LastUpdateTime: metaFixedClockStart, + }), + ), + )), + }, + }, + }, + "an approved CSR where the common name is not included in the DNS Names be marked as Failed": { + csr: gen.CertificateSigningRequestFrom(baseCSR, + gen.SetCertificateSigningRequestRequest(csrPEMExampleNotPresent), + gen.SetCertificateSigningRequestStatusCondition(certificatesv1.CertificateSigningRequestCondition{ + Type: certificatesv1.CertificateApproved, + Status: corev1.ConditionTrue, + }), + ), + builder: &testpkg.Builder{ + CertManagerObjects: []runtime.Object{baseIssuer.DeepCopy()}, + ExpectedEvents: []string{ + `Warning InvalidOrder The CSR PEM requests a commonName that is not present in the list of dnsNames or ipAddresses. If a commonName is set, ACME requires that the value is also present in the list of dnsNames or ipAddresses: "example.com" does not exist in [foo.com] or []`, + }, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewCreateAction( + authzv1.SchemeGroupVersion.WithResource("subjectaccessreviews"), + "", + &authzv1.SubjectAccessReview{ + Spec: authzv1.SubjectAccessReviewSpec{ + User: "user-1", + Groups: []string{"group-1", "group-2"}, + Extra: map[string]authzv1.ExtraValue{ + "extra": []string{"1", "2"}, + }, + UID: "uid-1", + + ResourceAttributes: &authzv1.ResourceAttributes{ + Group: certmanager.GroupName, + Resource: "signers", + Verb: "reference", + Namespace: baseIssuer.Namespace, + Name: baseIssuer.Name, + Version: "*", + }, + }, + }, + )), + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( + certificatesv1.SchemeGroupVersion.WithResource("certificatesigningrequests"), + "status", + "", + gen.CertificateSigningRequestFrom(baseCSR.DeepCopy(), + gen.SetCertificateSigningRequestRequest(csrPEMExampleNotPresent), + gen.SetCertificateSigningRequestStatusCondition(certificatesv1.CertificateSigningRequestCondition{ + Type: certificatesv1.CertificateApproved, + Status: corev1.ConditionTrue, + }), + gen.SetCertificateSigningRequestStatusCondition(certificatesv1.CertificateSigningRequestCondition{ + Type: certificatesv1.CertificateFailed, + Status: corev1.ConditionTrue, + Reason: "InvalidOrder", + Message: `The CSR PEM requests a commonName that is not present in the list of dnsNames or ipAddresses. If a commonName is set, ACME requires that the value is also present in the list of dnsNames or ipAddresses: "example.com" does not exist in [foo.com] or []`, + LastTransitionTime: metaFixedClockStart, + LastUpdateTime: metaFixedClockStart, + }), + ), + )), + }, + }, + }, + "an approved CSR which contains a garbage duration and has duration enabled, should fail when building the order and be marked as Failed": { + csr: gen.CertificateSigningRequestFrom(baseCSR, + gen.SetCertificateSigningRequestDuration("garbage-data"), + gen.SetCertificateSigningRequestStatusCondition(certificatesv1.CertificateSigningRequestCondition{ + Type: certificatesv1.CertificateApproved, + Status: corev1.ConditionTrue, + }), + ), + builder: &testpkg.Builder{ + CertManagerObjects: []runtime.Object{ + gen.IssuerFrom(baseIssuer.DeepCopy(), + gen.SetIssuerACME(cmacme.ACMEIssuer{EnableDurationFeature: true}), + )}, + ExpectedEvents: []string{ + `Warning OrderBuildingError Failed to build order: failed to parse requested duration on annotation "experimental.cert-manager.io/request-duration": time: invalid duration "garbage-data"`, + }, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewCreateAction( + authzv1.SchemeGroupVersion.WithResource("subjectaccessreviews"), + "", + &authzv1.SubjectAccessReview{ + Spec: authzv1.SubjectAccessReviewSpec{ + User: "user-1", + Groups: []string{"group-1", "group-2"}, + Extra: map[string]authzv1.ExtraValue{ + "extra": []string{"1", "2"}, + }, + UID: "uid-1", + + ResourceAttributes: &authzv1.ResourceAttributes{ + Group: certmanager.GroupName, + Resource: "signers", + Verb: "reference", + Namespace: baseIssuer.Namespace, + Name: baseIssuer.Name, + Version: "*", + }, + }, + }, + )), + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( + certificatesv1.SchemeGroupVersion.WithResource("certificatesigningrequests"), + "status", + "", + gen.CertificateSigningRequestFrom(baseCSR.DeepCopy(), + gen.SetCertificateSigningRequestDuration("garbage-data"), + gen.SetCertificateSigningRequestStatusCondition(certificatesv1.CertificateSigningRequestCondition{ + Type: certificatesv1.CertificateApproved, + Status: corev1.ConditionTrue, + }), + gen.SetCertificateSigningRequestStatusCondition(certificatesv1.CertificateSigningRequestCondition{ + Type: certificatesv1.CertificateFailed, + Status: corev1.ConditionTrue, + Reason: "OrderBuildingError", + Message: `Failed to build order: failed to parse requested duration on annotation "experimental.cert-manager.io/request-duration": time: invalid duration "garbage-data"`, + LastTransitionTime: metaFixedClockStart, + LastUpdateTime: metaFixedClockStart, + }), + ), + )), + }, + }, + }, + "an approved CSR where the order does not yet exist, should create the order and fire an event": { + csr: gen.CertificateSigningRequestFrom(baseCSR, + gen.SetCertificateSigningRequestStatusCondition(certificatesv1.CertificateSigningRequestCondition{ + Type: certificatesv1.CertificateApproved, + Status: corev1.ConditionTrue, + }), + ), + builder: &testpkg.Builder{ + CertManagerObjects: []runtime.Object{ + gen.IssuerFrom(baseIssuer.DeepCopy(), + gen.SetIssuerACME(cmacme.ACMEIssuer{}), + )}, + ExpectedEvents: []string{ + `Normal OrderCreated Created Order resource default-unit-test-ns/test-csr-3290353799`, + }, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewCreateAction( + authzv1.SchemeGroupVersion.WithResource("subjectaccessreviews"), + "", + &authzv1.SubjectAccessReview{ + Spec: authzv1.SubjectAccessReviewSpec{ + User: "user-1", + Groups: []string{"group-1", "group-2"}, + Extra: map[string]authzv1.ExtraValue{ + "extra": []string{"1", "2"}, + }, + UID: "uid-1", + + ResourceAttributes: &authzv1.ResourceAttributes{ + Group: certmanager.GroupName, + Resource: "signers", + Verb: "reference", + Namespace: baseIssuer.Namespace, + Name: baseIssuer.Name, + Version: "*", + }, + }, + }, + )), + testpkg.NewAction(coretesting.NewCreateAction( + cmacme.SchemeGroupVersion.WithResource("orders"), + gen.DefaultTestNamespace, + baseOrder, + )), + }, + }, + }, + "an approved CSR where the order already exists, but is owned by another CSR, return error": { + csr: gen.CertificateSigningRequestFrom(baseCSR, + gen.SetCertificateSigningRequestStatusCondition(certificatesv1.CertificateSigningRequestCondition{ + Type: certificatesv1.CertificateApproved, + Status: corev1.ConditionTrue, + }), + ), + expectedErr: true, + builder: &testpkg.Builder{ + CertManagerObjects: []runtime.Object{ + gen.IssuerFrom(baseIssuer.DeepCopy(), + gen.SetIssuerACME(cmacme.ACMEIssuer{}), + ), + gen.OrderFrom(baseOrder, + gen.SetOrderOwnerReference(metav1.OwnerReference{}), + ), + }, + ExpectedEvents: []string{}, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewCreateAction( + authzv1.SchemeGroupVersion.WithResource("subjectaccessreviews"), + "", + &authzv1.SubjectAccessReview{ + Spec: authzv1.SubjectAccessReviewSpec{ + User: "user-1", + Groups: []string{"group-1", "group-2"}, + Extra: map[string]authzv1.ExtraValue{ + "extra": []string{"1", "2"}, + }, + UID: "uid-1", + + ResourceAttributes: &authzv1.ResourceAttributes{ + Group: certmanager.GroupName, + Resource: "signers", + Verb: "reference", + Namespace: baseIssuer.Namespace, + Name: baseIssuer.Name, + Version: "*", + }, + }, + }, + )), + }, + }, + }, + "an approved CSR where the order already exists but is in a Failure state should mark the CSR and Failed": { + csr: gen.CertificateSigningRequestFrom(baseCSR, + gen.SetCertificateSigningRequestStatusCondition(certificatesv1.CertificateSigningRequestCondition{ + Type: certificatesv1.CertificateApproved, + Status: corev1.ConditionTrue, + }), + ), + builder: &testpkg.Builder{ + CertManagerObjects: []runtime.Object{ + gen.IssuerFrom(baseIssuer.DeepCopy(), + gen.SetIssuerACME(cmacme.ACMEIssuer{}), + ), + gen.OrderFrom(baseOrder, + gen.SetOrderStatus(cmacme.OrderStatus{ + Reason: "generic error", + State: cmacme.Invalid, + }), + ), + }, + ExpectedEvents: []string{ + `Warning OrderFailed Failed to wait for order resource default-unit-test-ns/test-csr-3290353799 to become ready: order is in "invalid" state: generic error`, + }, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewCreateAction( + authzv1.SchemeGroupVersion.WithResource("subjectaccessreviews"), + "", + &authzv1.SubjectAccessReview{ + Spec: authzv1.SubjectAccessReviewSpec{ + User: "user-1", + Groups: []string{"group-1", "group-2"}, + Extra: map[string]authzv1.ExtraValue{ + "extra": []string{"1", "2"}, + }, + UID: "uid-1", + + ResourceAttributes: &authzv1.ResourceAttributes{ + Group: certmanager.GroupName, + Resource: "signers", + Verb: "reference", + Namespace: baseIssuer.Namespace, + Name: baseIssuer.Name, + Version: "*", + }, + }, + }, + )), + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( + certificatesv1.SchemeGroupVersion.WithResource("certificatesigningrequests"), + "status", + "", + gen.CertificateSigningRequestFrom(baseCSR.DeepCopy(), + gen.SetCertificateSigningRequestStatusCondition(certificatesv1.CertificateSigningRequestCondition{ + Type: certificatesv1.CertificateApproved, + Status: corev1.ConditionTrue, + }), + gen.SetCertificateSigningRequestStatusCondition(certificatesv1.CertificateSigningRequestCondition{ + Type: certificatesv1.CertificateFailed, + Status: corev1.ConditionTrue, + Reason: "OrderFailed", + Message: `Failed to wait for order resource default-unit-test-ns/test-csr-3290353799 to become ready: order is in "invalid" state: generic error`, + LastTransitionTime: metaFixedClockStart, + LastUpdateTime: metaFixedClockStart, + }), + ), + )), + }, + }, + }, + "an approved CSR where the order is not in a Valid state should fire an event and return": { + csr: gen.CertificateSigningRequestFrom(baseCSR, + gen.SetCertificateSigningRequestStatusCondition(certificatesv1.CertificateSigningRequestCondition{ + Type: certificatesv1.CertificateApproved, + Status: corev1.ConditionTrue, + }), + ), + builder: &testpkg.Builder{ + CertManagerObjects: []runtime.Object{ + gen.IssuerFrom(baseIssuer.DeepCopy(), + gen.SetIssuerACME(cmacme.ACMEIssuer{}), + ), + gen.OrderFrom(baseOrder, + gen.SetOrderStatus(cmacme.OrderStatus{ + Reason: "pending", + State: cmacme.Pending, + }), + ), + }, + ExpectedEvents: []string{ + `Normal OrderPending Waiting on certificate issuance from order default-unit-test-ns/test-csr-3290353799: "pending"`, + }, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewCreateAction( + authzv1.SchemeGroupVersion.WithResource("subjectaccessreviews"), + "", + &authzv1.SubjectAccessReview{ + Spec: authzv1.SubjectAccessReviewSpec{ + User: "user-1", + Groups: []string{"group-1", "group-2"}, + Extra: map[string]authzv1.ExtraValue{ + "extra": []string{"1", "2"}, + }, + UID: "uid-1", + + ResourceAttributes: &authzv1.ResourceAttributes{ + Group: certmanager.GroupName, + Resource: "signers", + Verb: "reference", + Namespace: baseIssuer.Namespace, + Name: baseIssuer.Name, + Version: "*", + }, + }, + }, + )), + }, + }, + }, + "an approved CSR where the order is in a valid state, but the Certificate is empty should fire an event": { + csr: gen.CertificateSigningRequestFrom(baseCSR, + gen.SetCertificateSigningRequestStatusCondition(certificatesv1.CertificateSigningRequestCondition{ + Type: certificatesv1.CertificateApproved, + Status: corev1.ConditionTrue, + }), + ), + builder: &testpkg.Builder{ + CertManagerObjects: []runtime.Object{ + gen.IssuerFrom(baseIssuer.DeepCopy(), + gen.SetIssuerACME(cmacme.ACMEIssuer{}), + ), + gen.OrderFrom(baseOrder, + gen.SetOrderStatus(cmacme.OrderStatus{ + State: cmacme.Valid, + }), + gen.SetOrderCertificate(nil), + ), + }, + ExpectedEvents: []string{ + "Normal OrderPending Waiting for order-controller to add certificate data to Order default-unit-test-ns/test-csr-3290353799", + }, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewCreateAction( + authzv1.SchemeGroupVersion.WithResource("subjectaccessreviews"), + "", + &authzv1.SubjectAccessReview{ + Spec: authzv1.SubjectAccessReviewSpec{ + User: "user-1", + Groups: []string{"group-1", "group-2"}, + Extra: map[string]authzv1.ExtraValue{ + "extra": []string{"1", "2"}, + }, + UID: "uid-1", + + ResourceAttributes: &authzv1.ResourceAttributes{ + Group: certmanager.GroupName, + Resource: "signers", + Verb: "reference", + Namespace: baseIssuer.Namespace, + Name: baseIssuer.Name, + Version: "*", + }, + }, + }, + )), + }, + }, + }, + "an approved CSR where the order is in a valid state, but the certificate is garbage, should delete the Order and fire an event": { + csr: gen.CertificateSigningRequestFrom(baseCSR, + gen.SetCertificateSigningRequestStatusCondition(certificatesv1.CertificateSigningRequestCondition{ + Type: certificatesv1.CertificateApproved, + Status: corev1.ConditionTrue, + }), + ), + builder: &testpkg.Builder{ + CertManagerObjects: []runtime.Object{ + gen.IssuerFrom(baseIssuer.DeepCopy(), + gen.SetIssuerACME(cmacme.ACMEIssuer{}), + ), + gen.OrderFrom(baseOrder, + gen.SetOrderStatus(cmacme.OrderStatus{ + State: cmacme.Valid, + }), + gen.SetOrderCertificate([]byte("garbage-data")), + ), + }, + ExpectedEvents: []string{ + "Warning OrderBadCertificate Deleting Order with bad certificate: error decoding certificate PEM block", + }, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewCreateAction( + authzv1.SchemeGroupVersion.WithResource("subjectaccessreviews"), + "", + &authzv1.SubjectAccessReview{ + Spec: authzv1.SubjectAccessReviewSpec{ + User: "user-1", + Groups: []string{"group-1", "group-2"}, + Extra: map[string]authzv1.ExtraValue{ + "extra": []string{"1", "2"}, + }, + UID: "uid-1", + + ResourceAttributes: &authzv1.ResourceAttributes{ + Group: certmanager.GroupName, + Resource: "signers", + Verb: "reference", + Namespace: baseIssuer.Namespace, + Name: baseIssuer.Name, + Version: "*", + }, + }, + }, + )), + testpkg.NewAction(coretesting.NewDeleteAction( + cmacme.SchemeGroupVersion.WithResource("orders"), + gen.DefaultTestNamespace, + baseOrder.Name, + )), + }, + }, + }, + "an approved CSR where the order is in a valid state, but the certificate is singed for a different key than the request, delete the order": { + csr: gen.CertificateSigningRequestFrom(baseCSR, + gen.SetCertificateSigningRequestStatusCondition(certificatesv1.CertificateSigningRequestCondition{ + Type: certificatesv1.CertificateApproved, + Status: corev1.ConditionTrue, + }), + ), + builder: &testpkg.Builder{ + CertManagerObjects: []runtime.Object{ + gen.IssuerFrom(baseIssuer.DeepCopy(), + gen.SetIssuerACME(cmacme.ACMEIssuer{}), + ), + gen.OrderFrom(baseOrder, + gen.SetOrderStatus(cmacme.OrderStatus{ + State: cmacme.Valid, + }), + gen.SetOrderCertificate(certPEMExampleNotPresent), + ), + }, + ExpectedEvents: []string{ + "Warning OrderBadCertificate Deleting Order as the signed certificate's key does not match the request", + }, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewCreateAction( + authzv1.SchemeGroupVersion.WithResource("subjectaccessreviews"), + "", + &authzv1.SubjectAccessReview{ + Spec: authzv1.SubjectAccessReviewSpec{ + User: "user-1", + Groups: []string{"group-1", "group-2"}, + Extra: map[string]authzv1.ExtraValue{ + "extra": []string{"1", "2"}, + }, + UID: "uid-1", + + ResourceAttributes: &authzv1.ResourceAttributes{ + Group: certmanager.GroupName, + Resource: "signers", + Verb: "reference", + Namespace: baseIssuer.Namespace, + Name: baseIssuer.Name, + Version: "*", + }, + }, + }, + )), + testpkg.NewAction(coretesting.NewDeleteAction( + cmacme.SchemeGroupVersion.WithResource("orders"), + gen.DefaultTestNamespace, + baseOrder.Name, + )), + }, + }, + }, + "an approved CSR where the order is in a valid state, should update the CSR with the Certificate and an empty CA annotation": { + csr: gen.CertificateSigningRequestFrom(baseCSR, + gen.SetCertificateSigningRequestStatusCondition(certificatesv1.CertificateSigningRequestCondition{ + Type: certificatesv1.CertificateApproved, + Status: corev1.ConditionTrue, + }), + ), + builder: &testpkg.Builder{ + CertManagerObjects: []runtime.Object{ + gen.IssuerFrom(baseIssuer.DeepCopy(), + gen.SetIssuerACME(cmacme.ACMEIssuer{}), + ), + gen.OrderFrom(baseOrder, + gen.SetOrderStatus(cmacme.OrderStatus{ + State: cmacme.Valid, + Certificate: certPEM, + }), + gen.SetOrderCertificate(certPEM), + ), + }, + ExpectedEvents: []string{ + "Normal CertificateIssued Certificate fetched from issuer successfully", + }, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewCreateAction( + authzv1.SchemeGroupVersion.WithResource("subjectaccessreviews"), + "", + &authzv1.SubjectAccessReview{ + Spec: authzv1.SubjectAccessReviewSpec{ + User: "user-1", + Groups: []string{"group-1", "group-2"}, + Extra: map[string]authzv1.ExtraValue{ + "extra": []string{"1", "2"}, + }, + UID: "uid-1", + + ResourceAttributes: &authzv1.ResourceAttributes{ + Group: certmanager.GroupName, + Resource: "signers", + Verb: "reference", + Namespace: baseIssuer.Namespace, + Name: baseIssuer.Name, + Version: "*", + }, + }, + }, + )), + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( + certificatesv1.SchemeGroupVersion.WithResource("certificatesigningrequests"), + "status", + "", + gen.CertificateSigningRequestFrom(baseCSR, + gen.SetCertificateSigningRequestStatusCondition(certificatesv1.CertificateSigningRequestCondition{ + Type: certificatesv1.CertificateApproved, + Status: corev1.ConditionTrue, + }), + gen.SetCertificateSigningRequestCertificate(certPEM), + ), + )), + testpkg.NewAction(coretesting.NewUpdateAction( + certificatesv1.SchemeGroupVersion.WithResource("certificatesigningrequests"), + "", + gen.CertificateSigningRequestFrom(baseCSR, + gen.SetCertificateSigningRequestStatusCondition(certificatesv1.CertificateSigningRequestCondition{ + Type: certificatesv1.CertificateApproved, + Status: corev1.ConditionTrue, + }), + gen.SetCertificateSigningRequestCertificate(certPEM), + gen.SetCertificateSigningRequestCA([]byte{}), + ), + )), + }, + }, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + if test.csr != nil { + test.builder.KubeObjects = append(test.builder.KubeObjects, test.csr) + } + + fixedClock.SetTime(fixedClockStart) + test.builder.Clock = fixedClock + test.builder.T = t + test.builder.Init() + + // Always return true for SubjectAccessReviews in tests + test.builder.FakeKubeClient().PrependReactor("create", "*", func(action coretesting.Action) (bool, runtime.Object, error) { + if action.GetResource() != authzv1.SchemeGroupVersion.WithResource("subjectaccessreviews") { + return false, nil, nil + } + return true, &authzv1.SubjectAccessReview{ + Status: authzv1.SubjectAccessReviewStatus{ + Allowed: true, + }, + }, nil + }) + + defer test.builder.Stop() + + acme := NewACME(test.builder.Context) + + controller := certificatesigningrequests.New(apiutil.IssuerACME, acme) + controller.Register(test.builder.Context) + + test.builder.Start() + + err := controller.ProcessItem(context.Background(), test.csr.Name) + if (err != nil) != test.expectedErr { + t.Errorf("unexpected error, exp=%t got=%v", test.expectedErr, err) + } + + test.builder.CheckAndFinish(err) + }) + } +} + +func Test_buildOrder(t *testing.T) { + csrPEM, _, err := gen.CSR(x509.ECDSA, + gen.SetCSRCommonName("example.com"), + gen.SetCSRDNSNames("example.com"), + ) + if err != nil { + t.Fatal(err) + } + + req, err := pki.DecodeX509CertificateRequestBytes(csrPEM) + if err != nil { + t.Fatal(err) + } + + csr := gen.CertificateSigningRequest("test", + gen.SetCertificateSigningRequestDuration("1h"), + gen.SetCertificateSigningRequestRequest(csrPEM), + gen.SetCertificateSigningRequestSignerName("issuers.cert-manager.io/test-ns.test-name"), + ) + + tests := map[string]struct { + enableDurationFeature bool + + want *cmacme.Order + wantErr bool + }{ + "Normal building of order": { + enableDurationFeature: false, + want: &cmacme.Order{ + Spec: cmacme.OrderSpec{ + Request: csrPEM, + CommonName: "example.com", + DNSNames: []string{"example.com"}, + IssuerRef: cmmeta.ObjectReference{ + Name: "test-name", + Kind: "Issuer", + Group: "cert-manager.io", + }, + }, + }, + wantErr: false, + }, + "Building with enableDurationFeature": { + enableDurationFeature: true, + want: &cmacme.Order{ + Spec: cmacme.OrderSpec{ + Request: csrPEM, + CommonName: "example.com", + DNSNames: []string{"example.com"}, + Duration: &metav1.Duration{Duration: time.Hour}, + IssuerRef: cmmeta.ObjectReference{ + Name: "test-name", + Kind: "Issuer", + Group: "cert-manager.io", + }, + }, + }, + wantErr: false, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + got, err := new(ACME).buildOrder(csr, req, &cmapi.Issuer{ + Spec: cmapi.IssuerSpec{ + IssuerConfig: cmapi.IssuerConfig{ + ACME: &cmacme.ACMEIssuer{ + EnableDurationFeature: test.enableDurationFeature, + }, + }, + }, + }) + if (err != nil) != test.wantErr { + t.Errorf("buildOrder() error = %v, wantErr %v", err, test.wantErr) + return + } + + // for the current purpose we only test the spec + if !reflect.DeepEqual(got.Spec, test.want.Spec) { + t.Errorf("buildOrder() got = %v, want %v", got.Spec, test.want.Spec) + } + }) + } + + longCSROne := gen.CertificateSigningRequest( + "test-comparison-that-is-at-the-fifty-two-character-l", + gen.SetCertificateSigningRequestDuration("1h"), + gen.SetCertificateSigningRequestRequest(csrPEM), + gen.SetCertificateSigningRequestSignerName("issuers.cert-manager.io/test-ns.test-name"), + ) + orderOne, err := new(ACME).buildOrder(longCSROne, req, gen.Issuer("test-name", gen.SetIssuerACME(cmacme.ACMEIssuer{}))) + if err != nil { + t.Errorf("buildOrder() received error %v", err) + return + } + + t.Run("Builds two orders from different long CSRs to guarantee unique name", func(t *testing.T) { + longCSRTwo := gen.CertificateSigningRequest( + "test-comparison-that-is-at-the-fifty-two-character-l-two", + gen.SetCertificateSigningRequestDuration("1h"), + gen.SetCertificateSigningRequestRequest(csrPEM), + gen.SetCertificateSigningRequestSignerName("issuers.cert-manager.io/test-ns.test-name"), + ) + + orderTwo, err := new(ACME).buildOrder(longCSRTwo, req, gen.Issuer("test-name", gen.SetIssuerACME(cmacme.ACMEIssuer{}))) + if err != nil { + t.Errorf("buildOrder() received error %v", err) + return + } + + if orderOne.Name == orderTwo.Name { + t.Errorf( + "orders built from different CSRs have equal names: %s == %s", + orderOne.Name, + orderTwo.Name) + } + }) + + t.Run("Builds two orders from the same long CSRs to guarantee same name", func(t *testing.T) { + orderOne, err := new(ACME).buildOrder(longCSROne, req, gen.Issuer("test-name", gen.SetIssuerACME(cmacme.ACMEIssuer{}))) + if err != nil { + t.Errorf("buildOrder() received error %v", err) + return + } + + orderTwo, err := new(ACME).buildOrder(longCSROne, req, gen.Issuer("test-name", gen.SetIssuerACME(cmacme.ACMEIssuer{}))) + if err != nil { + t.Errorf("buildOrder() received error %v", err) + return + } + if orderOne.Name != orderTwo.Name { + t.Errorf( + "orders built from the same CSR have unequal names: %s != %s", + orderOne.Name, + orderTwo.Name) + } + }) +} diff --git a/pkg/controller/certificatesigningrequests/controller.go b/pkg/controller/certificatesigningrequests/controller.go index 28a513138..30d0bc706 100644 --- a/pkg/controller/certificatesigningrequests/controller.go +++ b/pkg/controller/certificatesigningrequests/controller.go @@ -22,6 +22,7 @@ import ( "github.com/go-logr/logr" certificatesv1 "k8s.io/api/certificates/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" authzclient "k8s.io/client-go/kubernetes/typed/authorization/v1" certificatesclient "k8s.io/client-go/kubernetes/typed/certificates/v1" certificateslisters "k8s.io/client-go/listers/certificates/v1" @@ -75,14 +76,31 @@ type Controller struct { // the signer kind to react to when a certificate signing request is synced signerType string + // Extra informers that should be watched by this CertificateSigningRequest + // controller instance. These resources can be owned by + // CertificateSigningRequests that we resolve. + extraInformers []cache.SharedIndexInformer + // used for testing clock clock.Clock } -func New(signerType string, signer Signer) *Controller { +// New will construct a new certificatesigningrequest controller using the +// given Signer implementation. +// Note: the extraInformers passed here will be 'waited' for when starting to +// ensure their corresponding listers have synced. +// An event handler will then be set on these informers that automatically +// resyncs CertificateSigningRequest resources that 'own' the objects in the +// informer. +// It's the callers responsibility to ensure the Run function on the informer +// is called in order to start the reflector. This is handled automatically +// when the informer factory's Start method is called, if the given informer +// was obtained using a SharedInformerFactory. +func New(signerType string, signer Signer, extraInformers ...cache.SharedIndexInformer) *Controller { return &Controller{ - signerType: signerType, - signer: signer, + signerType: signerType, + signer: signer, + extraInformers: extraInformers, } } @@ -100,12 +118,18 @@ func (c *Controller) Register(ctx *controllerpkg.Context) (workqueue.RateLimitin // obtain references to all the informers used by this controller csrInformer := ctx.KubeSharedInformerFactory.Certificates().V1().CertificateSigningRequests() + // Ensure we also catch all extra informers for this certificate controller instance + var extraInformersMustSync []cache.InformerSynced + for _, i := range c.extraInformers { + extraInformersMustSync = append(extraInformersMustSync, i.HasSynced) + } + // build a list of InformerSynced functions that will be returned by the Register method. // the controller will only begin processing items once all of these informers have synced. - mustSync := []cache.InformerSynced{ + mustSync := append([]cache.InformerSynced{ csrInformer.Informer().HasSynced, issuerInformer.Informer().HasSynced, - } + }, extraInformersMustSync...) // if scoped to a single namespace // if we are running in non-namespaced mode (i.e. --namespace=""), we also @@ -124,6 +148,17 @@ func (c *Controller) Register(ctx *controllerpkg.Context) (workqueue.RateLimitin csrInformer.Informer().AddEventHandler(&controllerpkg.QueuingEventHandler{Queue: c.queue}) issuerInformer.Informer().AddEventHandler(&controllerpkg.BlockingEventHandler{WorkFunc: c.handleGenericIssuer}) + // Ensure we catch extra informers that are owned by certificate signing requests + for _, i := range c.extraInformers { + i.AddEventHandler(&controllerpkg.BlockingEventHandler{ + WorkFunc: controllerpkg.HandleOwnedResourceNamespacedFunc(c.log, c.queue, + schema.GroupVersionKind{Version: "v1", Group: "certificates.k8s.io", Kind: "CertificateSigningRequest"}, + func(_, name string) (interface{}, error) { + return c.csrLister.Get(name) + }), + }) + } + // create an issuer helper for reading generic issuers c.helper = issuer.NewHelper(issuerInformer.Lister(), clusterIssuerInformer.Lister())