Only update certificate status in the controller package to stop conflicts

This commit is contained in:
James Munnelly 2017-10-13 20:15:29 +01:00
parent 29f44c2ed6
commit 9d933d9e11
9 changed files with 122 additions and 127 deletions

View File

@ -4,7 +4,6 @@ import (
"context"
"crypto/x509"
"fmt"
"reflect"
"time"
api "k8s.io/api/core/v1"
@ -19,6 +18,7 @@ import (
"github.com/jetstack-experimental/cert-manager/pkg/util"
"github.com/jetstack-experimental/cert-manager/pkg/util/errors"
"github.com/jetstack-experimental/cert-manager/pkg/util/kube"
"github.com/jetstack-experimental/cert-manager/pkg/util/pki"
)
const renewBefore = time.Hour * 24 * 30
@ -76,7 +76,6 @@ func (c *Controller) Sync(ctx context.Context, crt *v1alpha1.Certificate) (err e
Type: v1alpha1.IssuerConditionReady,
Status: v1alpha1.ConditionTrue,
})
if !issuerReady {
s := fmt.Sprintf(messageIssuerNotReady, issuerObj.GetObjectMeta().Name)
glog.Info(s)
@ -85,7 +84,6 @@ func (c *Controller) Sync(ctx context.Context, crt *v1alpha1.Certificate) (err e
}
i, err := c.issuerFactory.IssuerFor(issuerObj)
if err != nil {
s := messageIssuerErrorInit + err.Error()
glog.Info(s)
@ -95,7 +93,6 @@ func (c *Controller) Sync(ctx context.Context, crt *v1alpha1.Certificate) (err e
// grab existing certificate and validate private key
cert, err := kube.SecretTLSCert(c.secretLister, crt.Namespace, crt.Spec.SecretName)
if err != nil {
s := messageErrorCheckCertificate + err.Error()
glog.Info(s)
@ -113,22 +110,22 @@ func (c *Controller) Sync(ctx context.Context, crt *v1alpha1.Certificate) (err e
// execution.
defer c.scheduleRenewal(crt)
// if the certificate was not found, or the certificate data is invalid, we
// should issue a new certificate
if k8sErrors.IsNotFound(err) || errors.IsInvalidData(err) {
return c.issue(ctx, i, crt)
}
crtCopy := crt.DeepCopy()
expectedCN := pki.CommonNameForCertificate(crtCopy)
expectedDNSNames := pki.DNSNamesForCertificate(crtCopy)
expectedCN := crt.Spec.CommonName
if len(expectedCN) == 0 {
if len(crt.Spec.DNSNames) > 0 {
expectedCN = crt.Spec.DNSNames[0]
}
}
// if the certificate was not found, or the certificate data is invalid, we
// should issue a new certificate.
// if the certificate is valid for a list of domains other than those
// listed in the certificate spec, we should re-issue the certificate
if expectedCN != cert.Subject.CommonName || !util.EqualUnsorted(crt.Spec.DNSNames, cert.DNSNames) {
return c.issue(ctx, i, crt)
// listed in the certificate spec, we should re-issue the certificate.
if k8sErrors.IsNotFound(err) || errors.IsInvalidData(err) ||
expectedCN != cert.Subject.CommonName || !util.EqualUnsorted(cert.DNSNames, expectedDNSNames) {
err := c.issue(ctx, i, crtCopy)
updateErr := c.updateCertificateStatus(crtCopy)
if err != nil || updateErr != nil {
return utilerrors.NewAggregate([]error{err, updateErr})
}
return nil
}
// calculate the amount of time until expiry
@ -136,10 +133,13 @@ func (c *Controller) Sync(ctx context.Context, crt *v1alpha1.Certificate) (err e
// calculate how long until we should start attempting to renew the
// certificate
renewIn := durationUntilExpiry - renewBefore
// if we should being attempting to renew now, then trigger a renewal
if renewIn <= 0 {
return c.renew(ctx, i, crt)
err := c.renew(ctx, i, crtCopy)
updateErr := c.updateCertificateStatus(crtCopy)
if err != nil || updateErr != nil {
return utilerrors.NewAggregate([]error{err, updateErr})
}
}
return nil
@ -191,31 +191,14 @@ func (c *Controller) scheduleRenewal(crt *v1alpha1.Certificate) {
c.recorder.Event(crt, api.EventTypeNormal, successRenewalScheduled, s)
}
func (c *Controller) prepare(ctx context.Context, issuer issuer.Interface, crt *v1alpha1.Certificate) (err error) {
var status v1alpha1.CertificateStatus
status, err = issuer.Prepare(ctx, crt)
defer func() {
if saveErr := c.updateCertificateStatus(crt, status); saveErr != nil {
errs := []error{saveErr}
if err != nil {
errs = append(errs, err)
}
err = utilerrors.NewAggregate(errs)
}
}()
return
}
// return an error on failure. If retrieval is succesful, the certificate data
// and private key will be stored in the named secret
func (c *Controller) issue(ctx context.Context, issuer issuer.Interface, crt *v1alpha1.Certificate) (err error) {
func (c *Controller) issue(ctx context.Context, issuer issuer.Interface, crt *v1alpha1.Certificate) error {
var err error
s := messagePreparingCertificate
glog.Info(s)
c.recorder.Event(crt, api.EventTypeNormal, reasonPreparingCertificate, s)
if err := c.prepare(ctx, issuer, crt); err != nil {
if err = issuer.Prepare(ctx, crt); err != nil {
s := messageErrorPreparingCertificate + err.Error()
glog.Info(s)
c.recorder.Event(crt, api.EventTypeWarning, errorPreparingCertificate, s)
@ -226,17 +209,8 @@ func (c *Controller) issue(ctx context.Context, issuer issuer.Interface, crt *v1
glog.Info(s)
c.recorder.Event(crt, api.EventTypeNormal, reasonIssuingCertificate, s)
status, key, cert, err := issuer.Issue(ctx, crt)
defer func() {
if saveErr := c.updateCertificateStatus(crt, status); saveErr != nil {
errs := []error{saveErr}
if err != nil {
errs = append(errs, err)
}
err = utilerrors.NewAggregate(errs)
}
}()
var key, cert []byte
key, cert, err = issuer.Issue(ctx, crt)
if err != nil {
s := messageErrorIssuingCertificate + err.Error()
@ -274,11 +248,12 @@ func (c *Controller) issue(ctx context.Context, issuer issuer.Interface, crt *v1
// return an error on failure. If renewal is succesful, the certificate data
// and private key will be stored in the named secret
func (c *Controller) renew(ctx context.Context, issuer issuer.Interface, crt *v1alpha1.Certificate) error {
var err error
s := messagePreparingCertificate
glog.Info(s)
c.recorder.Event(crt, api.EventTypeNormal, reasonPreparingCertificate, s)
if err := c.prepare(ctx, issuer, crt); err != nil {
if err = issuer.Prepare(ctx, crt); err != nil {
s := messageErrorPreparingCertificate + err.Error()
glog.Info(s)
c.recorder.Event(crt, api.EventTypeWarning, errorPreparingCertificate, s)
@ -289,17 +264,8 @@ func (c *Controller) renew(ctx context.Context, issuer issuer.Interface, crt *v1
glog.Info(s)
c.recorder.Event(crt, api.EventTypeNormal, reasonRenewingCertificate, s)
status, key, cert, err := issuer.Renew(ctx, crt)
defer func() {
if saveErr := c.updateCertificateStatus(crt, status); saveErr != nil {
errs := []error{saveErr}
if err != nil {
errs = append(errs, err)
}
err = utilerrors.NewAggregate(errs)
}
}()
var key, cert []byte
key, cert, err = issuer.Renew(ctx, crt)
if err != nil {
s := messageErrorRenewingCertificate + err.Error()
@ -333,15 +299,10 @@ func (c *Controller) renew(ctx context.Context, issuer issuer.Interface, crt *v1
return nil
}
func (c *Controller) updateCertificateStatus(crt *v1alpha1.Certificate, status v1alpha1.CertificateStatus) error {
updateCertificate := crt.DeepCopy()
updateCertificate.Status = status
if reflect.DeepEqual(crt.Status, updateCertificate.Status) {
return nil
}
func (c *Controller) updateCertificateStatus(crt *v1alpha1.Certificate) error {
// TODO: replace Update call with UpdateStatus. This requires a custom API
// server with the /status subresource enabled and/or subresource support
// for CRDs (https://github.com/kubernetes/kubernetes/issues/38113)
_, err := c.cmClient.CertmanagerV1alpha1().Certificates(crt.Namespace).Update(updateCertificate)
_, err := c.cmClient.CertmanagerV1alpha1().Certificates(crt.Namespace).Update(crt)
return err
}

View File

@ -79,16 +79,15 @@ func (a *Acme) obtainCertificate(ctx context.Context, crt *v1alpha1.Certificate)
return pki.EncodePKCS1PrivateKey(key), certBuffer.Bytes(), nil
}
func (a *Acme) Issue(ctx context.Context, crt *v1alpha1.Certificate) (v1alpha1.CertificateStatus, []byte, []byte, error) {
update := crt.DeepCopy()
func (a *Acme) Issue(ctx context.Context, crt *v1alpha1.Certificate) ([]byte, []byte, error) {
key, cert, err := a.obtainCertificate(ctx, crt)
if err != nil {
s := messageErrorIssueCert + err.Error()
update.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorIssueCert, s)
return update.Status, nil, nil, err
crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorIssueCert, s)
return nil, nil, err
}
update.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionTrue, successCertIssued, messageCertIssued)
crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionTrue, successCertIssued, messageCertIssued)
return update.Status, key, cert, err
return key, cert, err
}

View File

@ -13,6 +13,7 @@ import (
"github.com/jetstack-experimental/cert-manager/pkg/apis/certmanager/v1alpha1"
"github.com/jetstack-experimental/cert-manager/pkg/util"
"github.com/jetstack-experimental/cert-manager/pkg/util/pki"
)
const (
@ -36,38 +37,36 @@ const (
//
// It will send the appropriate Letsencrypt authorizations, and complete
// challenge requests if neccessary.
func (a *Acme) Prepare(ctx context.Context, crt *v1alpha1.Certificate) (v1alpha1.CertificateStatus, error) {
update := crt.DeepCopy()
func (a *Acme) Prepare(ctx context.Context, crt *v1alpha1.Certificate) error {
// obtain an ACME client
cl, err := a.acmeClient()
if err != nil {
s := messageErrorGetACMEAccount + err.Error()
update.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorGetACMEAccount, s)
return update.Status, errors.New(s)
crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorGetACMEAccount, s)
return errors.New(s)
}
// step one: check issuer to see if we already have authorizations
toAuthorize, err := a.authorizationsToObtain(ctx, cl, *crt)
toAuthorize, err := a.authorizationsToObtain(ctx, cl, crt)
if err != nil {
s := messageErrorCheckAuthorization + err.Error()
update.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorCheckAuthorization, s)
return update.Status, errors.New(s)
crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorCheckAuthorization, s)
return errors.New(s)
}
// if there are no more authorizations to obtain, we are done
if len(toAuthorize) == 0 {
// TODO: set a field in the status block to show authorizations have
// been obtained so we can periodically update the auth status
return update.Status, nil
return nil
}
// request authorizations from the ACME server
auths, err := getAuthorizations(ctx, cl, toAuthorize...)
if err != nil {
s := messageErrorCheckAuthorization + err.Error()
update.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorCheckAuthorization, s)
return update.Status, errors.New(s)
crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorCheckAuthorization, s)
return errors.New(s)
}
// TODO: move some of this logic into it's own function
@ -113,11 +112,11 @@ func (a *Acme) Prepare(ctx context.Context, crt *v1alpha1.Certificate) (v1alpha1
if len(errs) > 0 {
err = utilerrors.NewAggregate(errs)
s := messageErrorCheckAuthorization + err.Error()
update.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorCheckAuthorization, s)
return update.Status, err
crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorCheckAuthorization, s)
return err
}
return update.Status, nil
return nil
}
func keyForChallenge(cl *acme.Client, challenge *acme.Challenge) (string, error) {
@ -153,8 +152,12 @@ func (a *Acme) authorize(ctx context.Context, cl *acme.Client, crt *v1alpha1.Cer
if err != nil {
return nil, err
}
defer solver.CleanUp(ctx, crt, auth.domain, token, key)
defer func() {
err := solver.CleanUp(ctx, crt, auth.domain, token, key)
if err != nil {
glog.Errorf("Error cleaning up solver: %s", err.Error())
}
}()
a.recorder.Eventf(crt, v1.EventTypeNormal, reasonPresentChallenge, messagePresentChallenge, challengeType, auth.domain)
err = solver.Present(ctx, crt, auth.domain, token, key)
@ -210,15 +213,33 @@ func authorizationsMap(list []v1alpha1.ACMEDomainAuthorization) map[string]v1alp
return out
}
func (a *Acme) authorizationsToObtain(ctx context.Context, cl *acme.Client, crt v1alpha1.Certificate) ([]string, error) {
func removeDuplicates(in []string) []string {
var found []string
Outer:
for _, i := range in {
for _, i2 := range found {
if i2 == i {
continue Outer
}
}
found = append(found, i)
}
return found
}
func (a *Acme) authorizationsToObtain(ctx context.Context, cl *acme.Client, crt *v1alpha1.Certificate) ([]string, error) {
authMap := authorizationsMap(crt.Status.ACMEStatus().Authorizations)
expectedCN := pki.CommonNameForCertificate(crt)
expectedDNSNames := pki.DNSNamesForCertificate(crt)
check := removeDuplicates(append(expectedDNSNames, expectedCN))
toAuthorize := util.StringFilter(func(domain string) (bool, error) {
auth, ok := authMap[domain]
glog.Infof("Compare %q with %q", auth.Account, a.issuer.GetStatus().ACMEStatus().URI)
if !ok || auth.Account != a.issuer.GetStatus().ACMEStatus().URI {
return false, nil
}
return checkAuthorization(ctx, cl, auth.URI)
}, append(crt.Spec.DNSNames, crt.Spec.CommonName)...)
}, check...)
domains := make([]string, len(toAuthorize))
for i, v := range toAuthorize {

View File

@ -14,16 +14,15 @@ const (
messageCertRenewed = "Certificate renewed successfully"
)
func (a *Acme) Renew(ctx context.Context, crt *v1alpha1.Certificate) (v1alpha1.CertificateStatus, []byte, []byte, error) {
update := crt.DeepCopy()
func (a *Acme) Renew(ctx context.Context, crt *v1alpha1.Certificate) ([]byte, []byte, error) {
key, cert, err := a.obtainCertificate(ctx, crt)
if err != nil {
s := messageErrorIssueCert + err.Error()
update.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorRenewCert, s)
return update.Status, nil, nil, err
crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorRenewCert, s)
return nil, nil, err
}
update.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionTrue, successCertRenewed, messageCertRenewed)
crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionTrue, successCertRenewed, messageCertRenewed)
return update.Status, key, cert, err
return key, cert, err
}

View File

@ -35,9 +35,7 @@ const (
defaultOrganization = "cert-manager"
)
func (c *CA) Issue(ctx context.Context, crt *v1alpha1.Certificate) (v1alpha1.CertificateStatus, []byte, []byte, error) {
update := crt.DeepCopy()
func (c *CA) Issue(ctx context.Context, crt *v1alpha1.Certificate) ([]byte, []byte, error) {
signeeKey, err := kube.SecretTLSKey(c.secretsLister, crt.Namespace, crt.Spec.SecretName)
if k8sErrors.IsNotFound(err) {
@ -46,21 +44,21 @@ func (c *CA) Issue(ctx context.Context, crt *v1alpha1.Certificate) (v1alpha1.Cer
if err != nil {
s := messageErrorGetCertKeyPair + err.Error()
update.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorGetCertKeyPair, s)
return update.Status, nil, nil, err
crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorGetCertKeyPair, s)
return nil, nil, err
}
certPem, err := c.obtainCertificate(crt, &signeeKey.PublicKey)
if err != nil {
s := messageErrorIssueCert + err.Error()
update.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorIssueCert, s)
return update.Status, nil, nil, err
crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorIssueCert, s)
return nil, nil, err
}
update.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionTrue, successCertIssued, messageCertIssued)
crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionTrue, successCertIssued, messageCertIssued)
return update.Status, pki.EncodePKCS1PrivateKey(signeeKey), certPem, nil
return pki.EncodePKCS1PrivateKey(signeeKey), certPem, nil
}
func (c *CA) obtainCertificate(crt *v1alpha1.Certificate, signeeKey interface{}) ([]byte, error) {
@ -121,7 +119,7 @@ func createCertificateTemplate(publicKey interface{}, commonName string, altName
// publicKey is the public key of the signee, and signerKey is the private
// key of the signer.
func signCertificate(crt *v1alpha1.Certificate, issuerCert *x509.Certificate, publicKey interface{}, signerKey interface{}) ([]byte, *x509.Certificate, error) {
template, err := createCertificateTemplate(publicKey, crt.Spec.CommonName, crt.Spec.DNSNames...)
template, err := createCertificateTemplate(publicKey, pki.CommonNameForCertificate(crt), pki.DNSNamesForCertificate(crt)...)
if err != nil {
return nil, nil, fmt.Errorf("error creating x509 certificate template: %s", err.Error())
}

View File

@ -9,6 +9,6 @@ import (
// Prepare does nothing for the CA issuer. In future, this may validate
// the certificate request against the issuer, or set fields in the Status
// block to be consumed in Issue and Renew
func (c *CA) Prepare(ctx context.Context, crt *v1alpha1.Certificate) (v1alpha1.CertificateStatus, error) {
return crt.Status, nil
func (c *CA) Prepare(ctx context.Context, crt *v1alpha1.Certificate) error {
return nil
}

View File

@ -18,26 +18,24 @@ const (
messageCertRenewed = "Certificate issued successfully"
)
func (c *CA) Renew(ctx context.Context, crt *v1alpha1.Certificate) (v1alpha1.CertificateStatus, []byte, []byte, error) {
update := crt.DeepCopy()
func (c *CA) Renew(ctx context.Context, crt *v1alpha1.Certificate) ([]byte, []byte, error) {
signeeKey, err := kube.SecretTLSKey(c.secretsLister, crt.Namespace, crt.Spec.SecretName)
if err != nil {
s := messageErrorGetCertKeyPair + err.Error()
update.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorGetCertKeyPair, s)
return update.Status, nil, nil, err
crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorGetCertKeyPair, s)
return nil, nil, err
}
certPem, err := c.obtainCertificate(crt, signeeKey)
if err != nil {
s := messageErrorRenewCert + err.Error()
update.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorRenewCert, s)
return update.Status, nil, nil, err
crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorRenewCert, s)
return nil, nil, err
}
update.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionTrue, successCertRenewed, messageCertRenewed)
crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionTrue, successCertRenewed, messageCertRenewed)
return update.Status, pki.EncodePKCS1PrivateKey(signeeKey), certPem, nil
return pki.EncodePKCS1PrivateKey(signeeKey), certPem, nil
}

View File

@ -12,11 +12,11 @@ type Interface interface {
// credentials and authorization with a remote server.
Setup(ctx context.Context) (v1alpha1.IssuerStatus, error)
// Prepare
Prepare(context.Context, *v1alpha1.Certificate) (v1alpha1.CertificateStatus, error)
Prepare(context.Context, *v1alpha1.Certificate) error
// Issue attempts to issue a certificate as described by the certificate
// resource given
Issue(context.Context, *v1alpha1.Certificate) (v1alpha1.CertificateStatus, []byte, []byte, error)
Issue(context.Context, *v1alpha1.Certificate) ([]byte, []byte, error)
// Renew attempts to renew the certificate describe by the certificate
// resource given. If no certificate exists, an error is returned.
Renew(context.Context, *v1alpha1.Certificate) (v1alpha1.CertificateStatus, []byte, []byte, error)
Renew(context.Context, *v1alpha1.Certificate) ([]byte, []byte, error)
}

View File

@ -3,12 +3,31 @@ package pki
import (
"crypto/x509"
"crypto/x509/pkix"
"github.com/jetstack-experimental/cert-manager/pkg/apis/certmanager/v1alpha1"
)
func GenerateCSR(commonName string, altNames ...string) *x509.CertificateRequest {
if commonName == "" && len(altNames) > 0 {
commonName = altNames[0]
// The provided Certificate *must* specify either a DNS name or a
// CommonName else this function will panic.
func CommonNameForCertificate(crt *v1alpha1.Certificate) string {
if crt.Spec.CommonName == "" {
return crt.Spec.DNSNames[0]
}
return crt.Spec.CommonName
}
// The provided Certificate *must* specify either a DNS name or a
// CommonName else this function will panic.
func DNSNamesForCertificate(crt *v1alpha1.Certificate) []string {
if len(crt.Spec.DNSNames) == 0 {
return []string{crt.Spec.CommonName}
}
if crt.Spec.CommonName != "" {
return append([]string{crt.Spec.CommonName}, crt.Spec.DNSNames...)
}
return crt.Spec.DNSNames
}
func GenerateCSR(commonName string, altNames ...string) *x509.CertificateRequest {
template := x509.CertificateRequest{
Subject: pkix.Name{
CommonName: commonName,