diff --git a/pkg/controller/certificaterequests/selfsigned/BUILD.bazel b/pkg/controller/certificaterequests/selfsigned/BUILD.bazel index 8a0e7713e..7b099a72c 100644 --- a/pkg/controller/certificaterequests/selfsigned/BUILD.bazel +++ b/pkg/controller/certificaterequests/selfsigned/BUILD.bazel @@ -26,9 +26,11 @@ go_test( srcs = ["selfsigned_test.go"], embed = [":go_default_library"], deps = [ + "//pkg/api/util:go_default_library", + "//pkg/apis/certmanager:go_default_library", "//pkg/apis/certmanager/v1alpha1:go_default_library", + "//pkg/controller/certificaterequests:go_default_library", "//pkg/controller/test:go_default_library", - "//pkg/issuer:go_default_library", "//pkg/util/pki:go_default_library", "//test/unit/gen:go_default_library", "//test/unit/listers:go_default_library", @@ -36,6 +38,8 @@ go_test( "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/client-go/listers/core/v1:go_default_library", + "//vendor/k8s.io/client-go/testing:go_default_library", + "//vendor/k8s.io/utils/clock/testing:go_default_library", ], ) diff --git a/pkg/controller/certificaterequests/selfsigned/selfsigned.go b/pkg/controller/certificaterequests/selfsigned/selfsigned.go index 8ed986614..1798abd69 100644 --- a/pkg/controller/certificaterequests/selfsigned/selfsigned.go +++ b/pkg/controller/certificaterequests/selfsigned/selfsigned.go @@ -18,6 +18,8 @@ package selfsigned import ( "context" + "crypto" + "crypto/x509" "errors" "fmt" @@ -25,7 +27,7 @@ import ( corelisters "k8s.io/client-go/listers/core/v1" apiutil "github.com/jetstack/cert-manager/pkg/api/util" - "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1" + cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1" controllerpkg "github.com/jetstack/cert-manager/pkg/controller" "github.com/jetstack/cert-manager/pkg/controller/certificaterequests" crutil "github.com/jetstack/cert-manager/pkg/controller/certificaterequests/util" @@ -40,11 +42,16 @@ const ( CRControllerName = "certificaterequests-issuer-selfsigned" ) +type signingFn func(*x509.Certificate, *x509.Certificate, crypto.PublicKey, interface{}) ([]byte, *x509.Certificate, error) + type SelfSigned struct { issuerOptions controllerpkg.IssuerOptions secretsLister corelisters.SecretLister reporter *crutil.Reporter + + // Used for testing to get reproducible resulting certificates + signingFn signingFn } func init() { @@ -67,18 +74,19 @@ func NewSelfSigned(ctx *controllerpkg.Context) *SelfSigned { issuerOptions: ctx.IssuerOptions, secretsLister: ctx.KubeSharedInformerFactory.Core().V1().Secrets().Lister(), reporter: crutil.NewReporter(ctx.Clock, ctx.Recorder), + signingFn: pki.SignCertificate, } } -func (s *SelfSigned) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest, issuerObj v1alpha1.GenericIssuer) (*issuer.IssueResponse, error) { +func (s *SelfSigned) Sign(ctx context.Context, cr *cmapi.CertificateRequest, issuerObj cmapi.GenericIssuer) (*issuer.IssueResponse, error) { log := logf.FromContext(ctx, "sign") resourceNamespace := s.issuerOptions.ResourceNamespace(issuerObj) - secretName, ok := cr.ObjectMeta.Annotations[v1alpha1.CRPrivateKeyAnnotationKey] + secretName, ok := cr.ObjectMeta.Annotations[cmapi.CRPrivateKeyAnnotationKey] if !ok || secretName == "" { message := fmt.Sprintf("Annotation %q missing or reference empty", - v1alpha1.CRPrivateKeyAnnotationKey) + cmapi.CRPrivateKeyAnnotationKey) err := errors.New("secret name missing") s.reporter.Failed(cr, err, "MissingAnnotation", message) @@ -88,26 +96,26 @@ func (s *SelfSigned) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest, } privatekey, err := kube.SecretTLSKey(ctx, s.secretsLister, cr.Namespace, secretName) + if k8sErrors.IsNotFound(err) { + message := fmt.Sprintf("Referenced secret %s/%s not found", cr.Namespace, secretName) + + s.reporter.Pending(cr, err, "MissingSecret", message) + log.Error(err, message) + + return nil, nil + } + + if cmerrors.IsInvalidData(err) { + message := fmt.Sprintf("Failed to get key %q referenced in annotation %q", + secretName, cmapi.CRPrivateKeyAnnotationKey) + + s.reporter.Pending(cr, err, "ErrorParsingKey", message) + log.Error(err, message) + + return nil, nil + } + if err != nil { - if k8sErrors.IsNotFound(err) { - message := fmt.Sprintf("Referenced secret %s/%s not found", cr.Namespace, secretName) - - s.reporter.Pending(cr, err, "MissingSecret", message) - log.Error(err, message) - - return nil, nil - } - - if cmerrors.IsInvalidData(err) { - message := fmt.Sprintf("Failed to get key %q referenced in annotation %q", - secretName, v1alpha1.CRPrivateKeyAnnotationKey) - - s.reporter.Pending(cr, err, "ErrorParsingKey", message) - log.Error(err, message) - - return nil, nil - } - // We are probably in a network error here so we should backoff and retry message := fmt.Sprintf("Failed to get certificate key pair from secret %s/%s", resourceNamespace, secretName) s.reporter.Pending(cr, err, "ErrorGettingSecret", message) @@ -147,7 +155,7 @@ func (s *SelfSigned) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest, } // sign and encode the certificate - certPem, _, err := pki.SignCertificate(template, template, publickey, privatekey) + certPem, _, err := s.signingFn(template, template, publickey, privatekey) if err != nil { message := "Error signing certificate" s.reporter.Failed(cr, err, "ErrorSigning", message) diff --git a/pkg/controller/certificaterequests/selfsigned/selfsigned_test.go b/pkg/controller/certificaterequests/selfsigned/selfsigned_test.go index 115280fda..a4d730dde 100644 --- a/pkg/controller/certificaterequests/selfsigned/selfsigned_test.go +++ b/pkg/controller/certificaterequests/selfsigned/selfsigned_test.go @@ -17,7 +17,6 @@ limitations under the License. package selfsigned import ( - "bytes" "context" "crypto" "crypto/rand" @@ -27,20 +26,30 @@ import ( "encoding/pem" "errors" "testing" + "time" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" clientcorev1 "k8s.io/client-go/listers/core/v1" + coretesting "k8s.io/client-go/testing" + fakeclock "k8s.io/utils/clock/testing" - "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1" + apiutil "github.com/jetstack/cert-manager/pkg/api/util" + "github.com/jetstack/cert-manager/pkg/apis/certmanager" + cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1" + "github.com/jetstack/cert-manager/pkg/controller/certificaterequests" testpkg "github.com/jetstack/cert-manager/pkg/controller/test" - "github.com/jetstack/cert-manager/pkg/issuer" "github.com/jetstack/cert-manager/pkg/util/pki" "github.com/jetstack/cert-manager/test/unit/gen" listersfake "github.com/jetstack/cert-manager/test/unit/listers" ) +var ( + fixedClockStart = time.Now() + fixedClock = fakeclock.NewFakeClock(fixedClockStart) +) + func generateCSR(t *testing.T, secretKey crypto.Signer, alg x509.SignatureAlgorithm) []byte { asn1Subj, _ := asn1.Marshal(pkix.Name{ CommonName: "test", @@ -61,166 +70,239 @@ func generateCSR(t *testing.T, secretKey crypto.Signer, alg x509.SignatureAlgori return csr } -func mustNoResponse(builder *testpkg.Builder, args ...interface{}) { - resp := args[0].(*issuer.IssueResponse) - if resp != nil { - builder.T.Errorf("unexpected response, exp='nil' got='%+v'", resp) - } -} - func TestSign(t *testing.T) { + metaFixedClockStart := metav1.NewTime(fixedClockStart) + + baseIssuer := gen.Issuer("test-issuer", + gen.SetIssuerSelfSigned(cmapi.SelfSignedIssuer{}), + ) + skRSA, err := pki.GenerateRSAPrivateKey(2048) if err != nil { t.Errorf("failed to generate RSA private key: %s", err) t.FailNow() } - - skEC, err := pki.GenerateECPrivateKey(256) - if err != nil { - t.Errorf("failed to generate ECDA private key: %s", err) - t.FailNow() - } - - skAnotherRSA, err := pki.GenerateRSAPrivateKey(2048) - if err != nil { - t.Errorf("failed to generate RSA private key: %s", err) - t.FailNow() - } - - csrBytes := generateCSR(t, skRSA, x509.SHA256WithRSA) - csrECBytes := generateCSR(t, skEC, x509.ECDSAWithSHA256) - - skRSAPEMBytes := pki.EncodePKCS1PrivateKey(skRSA) - skAnotherRSAPEMBytes := pki.EncodePKCS1PrivateKey(skAnotherRSA) - skECPEMBytes, err := pki.EncodeECPrivateKey(skEC) - if err != nil { - t.Error(err) - t.FailNow() - } - + skRSAPEM := pki.EncodePKCS1PrivateKey(skRSA) rsaKeySecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "test-rsa-key", Namespace: gen.DefaultTestNamespace, }, Data: map[string][]byte{ - corev1.TLSPrivateKeyKey: skRSAPEMBytes, + corev1.TLSPrivateKeyKey: skRSAPEM, }, } - anotherRSAKeySecret := &corev1.Secret{ + invalidKeySecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-another-rsa-key", - Namespace: gen.DefaultTestNamespace, - }, - Data: map[string][]byte{ - corev1.TLSPrivateKeyKey: skAnotherRSAPEMBytes, - }, - } - ecKeySecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-ecdsa-key", - Namespace: gen.DefaultTestNamespace, - }, - Data: map[string][]byte{ - corev1.TLSPrivateKeyKey: skECPEMBytes, - }, - } - badKeySecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-bad-key", + Name: rsaKeySecret.Name, Namespace: gen.DefaultTestNamespace, }, Data: map[string][]byte{ corev1.TLSPrivateKeyKey: []byte("this is a bad key"), }, } + csrRSAPEM := generateCSR(t, skRSA, x509.SHA256WithRSA) + + skEC, err := pki.GenerateECPrivateKey(256) + if err != nil { + t.Errorf("failed to generate ECDA private key: %s", err) + t.FailNow() + } + skECPEM, err := pki.EncodeECPrivateKey(skEC) + if err != nil { + t.Error(err) + t.FailNow() + } + ecKeySecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: rsaKeySecret.Name, + Namespace: gen.DefaultTestNamespace, + }, + Data: map[string][]byte{ + corev1.TLSPrivateKeyKey: skECPEM, + }, + } + csrECPEM := generateCSR(t, skEC, x509.ECDSAWithSHA256) + + baseCR := gen.CertificateRequest("test-cr", + gen.SetCertificateRequestAnnotations( + map[string]string{ + cmapi.CRPrivateKeyAnnotationKey: rsaKeySecret.Name, + }, + ), + gen.SetCertificateRequestCSR(csrRSAPEM), + gen.SetCertificateRequestIssuer(cmapi.ObjectReference{ + Name: baseIssuer.Name, + Group: certmanager.GroupName, + Kind: "Issuer", + }), + ) + ecCR := gen.CertificateRequestFrom(baseCR, + gen.SetCertificateRequestCSR(csrECPEM), + ) + + templateRSA, err := pki.GenerateTemplateFromCertificateRequest(baseCR) + if err != nil { + t.Error(err) + t.FailNow() + } + certRSAPEM, _, err := pki.SignCertificate(templateRSA, templateRSA, skRSA.Public(), skRSA) + if err != nil { + t.Error(err) + t.FailNow() + } + + templateEC, err := pki.GenerateTemplateFromCertificateRequest(ecCR) + if err != nil { + t.Error(err) + t.FailNow() + } + certECPEM, _, err := pki.SignCertificate(templateEC, templateEC, skEC.Public(), skEC) + if err != nil { + t.Error(err) + t.FailNow() + } tests := map[string]testT{ - "a CertificateRequest with no certmanager.k8s.io/selfsigned-private-key annotation should record pending": { - // no annotation - certificaterequest: gen.CertificateRequest("test-cr"), + "a CertificateRequest with no certmanager.k8s.io/selfsigned-private-key annotation should fail": { + certificateRequest: gen.CertificateRequestFrom(baseCR, + // no annotation + gen.SetCertificateRequestAnnotations(map[string]string{}), + ), builder: &testpkg.Builder{ + KubeObjects: []runtime.Object{}, + CertManagerObjects: []runtime.Object{gen.CertificateRequestFrom(baseCR, + // no annotation + gen.SetCertificateRequestAnnotations(map[string]string{}), + ), baseIssuer}, ExpectedEvents: []string{ `Warning MissingAnnotation Annotation "certmanager.k8s.io/private-key-secret-name" missing or reference empty: secret name missing`, }, - CheckFn: mustNoResponse, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewUpdateAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(baseCR, + gen.SetCertificateRequestAnnotations(map[string]string{}), + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmapi.ConditionFalse, + Reason: cmapi.CertificateRequestReasonFailed, + Message: `Annotation "certmanager.k8s.io/private-key-secret-name" missing or reference empty: secret name missing`, + LastTransitionTime: &metaFixedClockStart, + }), + gen.SetCertificateRequestFailureTime(metaFixedClockStart), + ), + )), + }, }, - expectedErr: false, }, - "a CertificateRequest with a certmanager.k8s.io/private-key-secret-name annotation but empty string should record pending": { - // no data in annotation - certificaterequest: gen.CertificateRequest("test-cr", - gen.AddCertificateRequestAnnotations(map[string]string{ - v1alpha1.CRPrivateKeyAnnotationKey: "", - }), + "a CertificateRequest with a certmanager.k8s.io/private-key-secret-name annotation but empty string should fail": { + certificateRequest: gen.CertificateRequestFrom(baseCR, + // no data in annotation + gen.SetCertificateRequestAnnotations(map[string]string{cmapi.CRPrivateKeyAnnotationKey: ""}), ), builder: &testpkg.Builder{ + KubeObjects: []runtime.Object{}, + CertManagerObjects: []runtime.Object{gen.CertificateRequestFrom(baseCR, + // no data in annotation + gen.SetCertificateRequestAnnotations(map[string]string{cmapi.CRPrivateKeyAnnotationKey: ""}), + ), baseIssuer}, ExpectedEvents: []string{ `Warning MissingAnnotation Annotation "certmanager.k8s.io/private-key-secret-name" missing or reference empty: secret name missing`, }, - CheckFn: mustNoResponse, - }, - expectedErr: false, - }, - "a CertificateRequest with a certmanager.k8s.io/private-key-secret-name annotation but the referenced secret doesn't exist should record pending": { - certificaterequest: gen.CertificateRequest("test-cr", - gen.AddCertificateRequestAnnotations(map[string]string{ - v1alpha1.CRPrivateKeyAnnotationKey: "a-non-existent-secret", - }), - ), - builder: &testpkg.Builder{ - ExpectedEvents: []string{ - `Normal MissingSecret Referenced secret default-unit-test-ns/a-non-existent-secret not found: secret "a-non-existent-secret" not found`, - }, - CheckFn: mustNoResponse, - }, - expectedErr: false, - }, - "a CertificateRequest with a bad CSR should fail": { - certificaterequest: gen.CertificateRequest("test-cr", - gen.AddCertificateRequestAnnotations(map[string]string{ - v1alpha1.CRPrivateKeyAnnotationKey: "test-rsa-key", - }), - gen.SetCertificateRequestCSR([]byte("this is a bad CSR")), - ), - builder: &testpkg.Builder{ - KubeObjects: []runtime.Object{rsaKeySecret}, - ExpectedEvents: []string{ - `Warning ErrorGenerating Error generating certificate template: failed to decode csr from certificate request resource default-unit-test-ns/test-cr`, - }, - CheckFn: mustNoResponse, - }, - expectedErr: false, - }, - "a CertificateRequest referencing a bad key should record pending": { - certificaterequest: gen.CertificateRequest("test-cr", - gen.AddCertificateRequestAnnotations(map[string]string{ - v1alpha1.CRPrivateKeyAnnotationKey: "test-bad-key", - }), - gen.SetCertificateRequestCSR(csrBytes), - ), - builder: &testpkg.Builder{ - KubeObjects: []runtime.Object{badKeySecret}, - CheckFn: mustNoResponse, - ExpectedEvents: []string{ - `Normal ErrorParsingKey Failed to get key "test-bad-key" referenced in annotation "certmanager.k8s.io/private-key-secret-name": error decoding private key PEM block`, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewUpdateAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(baseCR, + gen.SetCertificateRequestAnnotations(map[string]string{cmapi.CRPrivateKeyAnnotationKey: ""}), + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmapi.ConditionFalse, + Reason: cmapi.CertificateRequestReasonFailed, + Message: `Annotation "certmanager.k8s.io/private-key-secret-name" missing or reference empty: secret name missing`, + LastTransitionTime: &metaFixedClockStart, + }), + gen.SetCertificateRequestFailureTime(metaFixedClockStart), + ), + )), + }, + }, + }, + "if the referenced secret doesn't exist then should record pending": { + certificateRequest: baseCR.DeepCopy(), + builder: &testpkg.Builder{ + KubeObjects: []runtime.Object{}, + CertManagerObjects: []runtime.Object{baseCR.DeepCopy(), baseIssuer}, + ExpectedEvents: []string{ + `Normal MissingSecret Referenced secret default-unit-test-ns/test-rsa-key not found: secret "test-rsa-key" not found`, + }, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewUpdateAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(baseCR, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmapi.ConditionFalse, + Reason: cmapi.CertificateRequestReasonPending, + Message: `Referenced secret default-unit-test-ns/test-rsa-key not found: secret "test-rsa-key" not found`, + LastTransitionTime: &metaFixedClockStart, + }), + ), + )), + }, + }, + }, + "if the referenced secret contains invalid data then should record pending": { + certificateRequest: baseCR.DeepCopy(), + builder: &testpkg.Builder{ + KubeObjects: []runtime.Object{invalidKeySecret}, + CertManagerObjects: []runtime.Object{baseCR.DeepCopy(), baseIssuer}, + ExpectedEvents: []string{ + `Normal ErrorParsingKey Failed to get key "test-rsa-key" referenced in annotation "certmanager.k8s.io/private-key-secret-name": error decoding private key PEM block`, + }, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewUpdateAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(baseCR, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmapi.ConditionFalse, + Reason: cmapi.CertificateRequestReasonPending, + Message: `Failed to get key "test-rsa-key" referenced in annotation "certmanager.k8s.io/private-key-secret-name": error decoding private key PEM block`, + LastTransitionTime: &metaFixedClockStart, + }), + ), + )), }, }, - expectedErr: false, }, "a CertificateRequest that transiently fails a secret lookup should backoff error to retry": { - certificaterequest: gen.CertificateRequest("test-cr", - gen.AddCertificateRequestAnnotations(map[string]string{ - v1alpha1.CRPrivateKeyAnnotationKey: "test-rsa-key", - }), - gen.SetCertificateRequestCSR(csrBytes), - ), + certificateRequest: baseCR.DeepCopy(), builder: &testpkg.Builder{ - KubeObjects: []runtime.Object{rsaKeySecret}, - CheckFn: mustNoResponse, + KubeObjects: []runtime.Object{rsaKeySecret}, + CertManagerObjects: []runtime.Object{baseCR.DeepCopy(), baseIssuer}, ExpectedEvents: []string{ `Normal ErrorGettingSecret Failed to get certificate key pair from secret default-unit-test-ns/test-rsa-key: this is a network error`, }, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewUpdateAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(baseCR, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmapi.ConditionFalse, + Reason: cmapi.CertificateRequestReasonPending, + Message: "Failed to get certificate key pair from secret default-unit-test-ns/test-rsa-key: this is a network error", + LastTransitionTime: &metaFixedClockStart, + }), + ), + )), + }, }, fakeLister: &listersfake.FakeSecretLister{ SecretsFn: func(namespace string) clientcorev1.SecretNamespaceLister { @@ -233,85 +315,172 @@ func TestSign(t *testing.T) { }, expectedErr: true, }, - "a CertificateRequest referencing a key which did not sign the CSR should fail": { - certificaterequest: gen.CertificateRequest("test-cr", - gen.AddCertificateRequestAnnotations(map[string]string{ - v1alpha1.CRPrivateKeyAnnotationKey: "test-another-rsa-key", - }), - gen.SetCertificateRequestCSR(csrBytes), - ), - builder: &testpkg.Builder{ - KubeObjects: []runtime.Object{anotherRSAKeySecret}, - ExpectedEvents: []string{ - "Warning ErrorKeyMatch Error generating certificate template: CSR not signed by referenced private key", - }, - CheckFn: mustNoResponse, - }, - expectedErr: false, - }, - "a valid RSA key should sign a self signed certificate": { - certificaterequest: gen.CertificateRequest("test-cr", - gen.AddCertificateRequestAnnotations(map[string]string{ - v1alpha1.CRPrivateKeyAnnotationKey: "test-rsa-key", - }), - gen.SetCertificateRequestCSR(csrBytes), + "a CertificateRequest with a bad CSR should fail": { + certificateRequest: gen.CertificateRequestFrom(baseCR, + gen.SetCertificateRequestCSR([]byte("this is a bad CSR")), ), builder: &testpkg.Builder{ KubeObjects: []runtime.Object{rsaKeySecret}, - CheckFn: func(builder *testpkg.Builder, args ...interface{}) { - resp := args[0].(*issuer.IssueResponse) - - // CA and cert should be the same. - if !bytes.Equal(resp.CA, resp.Certificate) { - t.Errorf("expected CA and cert to be the same but got:\nCA: %s\nCert: %s", - resp.CA, resp.Certificate) - } - - // No private key should be returned. - if len(resp.PrivateKey) > 0 { - t.Errorf("expected to private key returned but got: %s", - resp.PrivateKey) - } + CertManagerObjects: []runtime.Object{gen.CertificateRequestFrom(baseCR, + gen.SetCertificateRequestCSR([]byte("this is a bad CSR")), + ), baseIssuer}, + ExpectedEvents: []string{ + "Warning BadConfig Resource validation failed: spec.csr: Invalid value: []byte{0x74, 0x68, 0x69, 0x73, 0x20, 0x69, 0x73, 0x20, 0x61, 0x20, 0x62, 0x61, 0x64, 0x20, 0x43, 0x53, 0x52}: failed to decode csr: error decoding certificate request PEM block", + }, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewUpdateAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(baseCR, + gen.SetCertificateRequestCSR([]byte("this is a bad CSR")), + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmapi.ConditionFalse, + Reason: cmapi.CertificateRequestReasonFailed, + Message: "Resource validation failed: spec.csr: Invalid value: []byte{0x74, 0x68, 0x69, 0x73, 0x20, 0x69, 0x73, 0x20, 0x61, 0x20, 0x62, 0x61, 0x64, 0x20, 0x43, 0x53, 0x52}: failed to decode csr: error decoding certificate request PEM block", + LastTransitionTime: &metaFixedClockStart, + }), + gen.SetCertificateRequestFailureTime(metaFixedClockStart), + ), + )), }, }, - expectedErr: false, }, - "a valid ECDSA key should sign a self signed certificate": { - certificaterequest: gen.CertificateRequest("test-cr", - gen.AddCertificateRequestAnnotations(map[string]string{ - v1alpha1.CRPrivateKeyAnnotationKey: "test-ecdsa-key", - }), - gen.SetCertificateRequestCSR(csrECBytes), - ), + "a CSR that has not been signed with the same public key as the referenced private key should fail": { + certificateRequest: ecCR.DeepCopy(), builder: &testpkg.Builder{ - KubeObjects: []runtime.Object{ecKeySecret}, - CheckFn: func(builder *testpkg.Builder, args ...interface{}) { - resp := args[0].(*issuer.IssueResponse) - if resp == nil { - t.Errorf("expected a response but got: %+v", - args[1]) - return - } - - // CA and cert should be the same. - if !bytes.Equal(resp.CA, resp.Certificate) { - t.Errorf("expected CA and cert to be the same but got:\nCA: %s\nCert: %s", - resp.CA, resp.Certificate) - } - - // No private key should be returned. - if len(resp.PrivateKey) > 0 { - t.Errorf("expected to private key returned but got: %s", - resp.PrivateKey) - } + KubeObjects: []runtime.Object{rsaKeySecret}, + CertManagerObjects: []runtime.Object{ecCR.DeepCopy(), baseIssuer}, + ExpectedEvents: []string{ + "Warning ErrorKeyMatch Error generating certificate template: CSR not signed by referenced private key", + }, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewUpdateAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(ecCR, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmapi.ConditionFalse, + Reason: cmapi.CertificateRequestReasonFailed, + Message: "Error generating certificate template: CSR not signed by referenced private key", + LastTransitionTime: &metaFixedClockStart, + }), + gen.SetCertificateRequestFailureTime(metaFixedClockStart), + ), + )), + }, + }, + }, + "if signing fails then should report failure": { + certificateRequest: baseCR.DeepCopy(), + signingFn: func(*x509.Certificate, *x509.Certificate, crypto.PublicKey, interface{}) ([]byte, *x509.Certificate, error) { + return nil, nil, errors.New("this is a signing error") + }, + builder: &testpkg.Builder{ + KubeObjects: []runtime.Object{rsaKeySecret}, + CertManagerObjects: []runtime.Object{baseCR.DeepCopy(), baseIssuer}, + ExpectedEvents: []string{ + "Warning ErrorSigning Error signing certificate: this is a signing error", + }, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewUpdateAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(baseCR, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmapi.ConditionFalse, + Reason: cmapi.CertificateRequestReasonFailed, + Message: "Error signing certificate: this is a signing error", + LastTransitionTime: &metaFixedClockStart, + }), + gen.SetCertificateRequestFailureTime(metaFixedClockStart), + ), + )), + }, + }, + }, + "should sign an RSA key set condition to Ready": { + certificateRequest: baseCR.DeepCopy(), + signingFn: func(c1 *x509.Certificate, c2 *x509.Certificate, pk crypto.PublicKey, sk interface{}) ([]byte, *x509.Certificate, error) { + // We still check that it will sign and not error + // Return error if we do + _, _, err := pki.SignCertificate(c1, c2, pk, sk) + if err != nil { + return nil, nil, err + } + + return certRSAPEM, nil, nil + }, + builder: &testpkg.Builder{ + KubeObjects: []runtime.Object{rsaKeySecret}, + CertManagerObjects: []runtime.Object{baseCR.DeepCopy(), baseIssuer}, + ExpectedEvents: []string{ + "Normal CertificateIssued Certificate fetched from issuer successfully", + }, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewUpdateAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(baseCR, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmapi.ConditionTrue, + Reason: cmapi.CertificateRequestReasonIssued, + Message: "Certificate fetched from issuer successfully", + LastTransitionTime: &metaFixedClockStart, + }), + gen.SetCertificateRequestCertificate(certRSAPEM), + gen.SetCertificateRequestCA(certRSAPEM), + ), + )), + }, + }, + }, + "should sign an EC key set condition to Ready": { + certificateRequest: ecCR.DeepCopy(), + signingFn: func(c1 *x509.Certificate, c2 *x509.Certificate, pk crypto.PublicKey, sk interface{}) ([]byte, *x509.Certificate, error) { + // We still check that it will sign and not error + // Return error if we do + _, _, err := pki.SignCertificate(c1, c2, pk, sk) + if err != nil { + return nil, nil, err + } + + return certECPEM, nil, nil + }, + builder: &testpkg.Builder{ + KubeObjects: []runtime.Object{ecKeySecret}, + CertManagerObjects: []runtime.Object{ecCR.DeepCopy(), baseIssuer}, + ExpectedEvents: []string{ + "Normal CertificateIssued Certificate fetched from issuer successfully", + }, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewUpdateAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(ecCR, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmapi.ConditionTrue, + Reason: cmapi.CertificateRequestReasonIssued, + Message: "Certificate fetched from issuer successfully", + LastTransitionTime: &metaFixedClockStart, + }), + gen.SetCertificateRequestCertificate(certECPEM), + gen.SetCertificateRequestCA(certECPEM), + ), + )), }, }, - expectedErr: false, }, } for name, test := range tests { t.Run(name, func(t *testing.T) { + fixedClock.SetTime(fixedClockStart) + test.builder.Clock = fixedClock runTest(t, test) }) } @@ -319,9 +488,9 @@ func TestSign(t *testing.T) { type testT struct { builder *testpkg.Builder - certificaterequest *v1alpha1.CertificateRequest + certificateRequest *cmapi.CertificateRequest + signingFn signingFn - checkFn func(*testpkg.Builder, ...interface{}) expectedErr bool fakeLister *listersfake.FakeSecretLister @@ -332,25 +501,27 @@ func runTest(t *testing.T, test testT) { test.builder.Start() defer test.builder.Stop() - c := NewSelfSigned(test.builder.Context) + self := NewSelfSigned(test.builder.Context) if test.fakeLister != nil { - c.secretsLister = test.fakeLister + self.secretsLister = test.fakeLister } + if test.signingFn != nil { + self.signingFn = test.signingFn + } + + controller := certificaterequests.New(apiutil.IssuerSelfSigned, self) + controller.Register(test.builder.Context) test.builder.Sync() - selfSignedIssuer := gen.Issuer("selfsigned-issuer", - gen.SetIssuerSelfSigned(v1alpha1.SelfSignedIssuer{}), - ) - - resp, err := c.Sign(context.Background(), test.certificaterequest, - selfSignedIssuer) + err := controller.Sync(context.Background(), test.certificateRequest) if err != nil && !test.expectedErr { t.Errorf("expected to not get an error, but got: %v", err) } if err == nil && test.expectedErr { t.Errorf("expected to get an error but did not get one") } - test.builder.CheckAndFinish(resp, err) + + test.builder.CheckAndFinish(err) } diff --git a/test/unit/gen/certificaterequest.go b/test/unit/gen/certificaterequest.go index d2855d8de..4c8a8d454 100644 --- a/test/unit/gen/certificaterequest.go +++ b/test/unit/gen/certificaterequest.go @@ -121,6 +121,12 @@ func AddCertificateRequestAnnotations(annotations map[string]string) Certificate } } +func SetCertificateRequestAnnotations(annotations map[string]string) CertificateRequestModifier { + return func(cr *v1alpha1.CertificateRequest) { + cr.SetAnnotations(annotations) + } +} + func SetCertificateRequestFailureTime(p metav1.Time) CertificateRequestModifier { return func(cr *v1alpha1.CertificateRequest) { cr.Status.FailureTime = &p