weiguoqiang/cert-manager
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

Merge pull request #1763 from cheukwing/issue-1667

Browse Source 
Add handling of updates to ACME email field in Issuers
This commit is contained in:
jetstack-bot 2019-07-03 13:04:30 +01:00 committed by GitHub
commit 8e54b32d6e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 161 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
deploy/manifests/00-crds.yaml
View File

@ -750,6 +750,11 @@ spec:
properties:
acme:
properties:
lastRegisteredEmail:
description: LastRegisteredEmail is the email associated with the
latest registered ACME account, in order to track changes made
to registered account associated with the Issuer
type: string
uri:
description: URI is the unique account identifier, which can also
be used to retrieve account details from the CA
@ -1029,6 +1034,11 @@ spec:
properties:
acme:
properties:
lastRegisteredEmail:
description: LastRegisteredEmail is the email associated with the
latest registered ACME account, in order to track changes made
to registered account associated with the Issuer
type: string
uri:
description: URI is the unique account identifier, which can also
be used to retrieve account details from the CA

8
pkg/acme/client/fake.go
View File

@ -42,6 +42,7 @@ type FakeACME struct {
FakeHTTP01ChallengeResponse func(token string) (string, error)
FakeDNS01ChallengeRecord func(token string) (string, error)
FakeDiscover func(ctx context.Context) (acme.Directory, error)
FakeUpdateAccount func(ctx context.Context, a *acme.Account) (*acme.Account, error)
}
func (f *FakeACME) CreateOrder(ctx context.Context, order *acme.Order) (*acme.Order, error) {
@ -143,3 +144,10 @@ func (f *FakeACME) Discover(ctx context.Context) (acme.Directory, error) {
// empty directory here will be fine
return acme.Directory{}, nil
}
func (f *FakeACME) UpdateAccount(ctx context.Context, a *acme.Account) (*acme.Account, error) {
if f.FakeUpdateAccount != nil {
return f.FakeUpdateAccount(ctx, a)
}
return nil, fmt.Errorf("UpdateAccount not implemented")
}

1
pkg/acme/client/interfaces.go
View File

@ -37,6 +37,7 @@ type Interface interface {
HTTP01ChallengeResponse(token string) (string, error)
DNS01ChallengeRecord(token string) (string, error)
Discover(ctx context.Context) (acme.Directory, error)
UpdateAccount(ctx context.Context, a *acme.Account) (*acme.Account, error)
}
var _ Interface = &acme.Client{}

5
pkg/acme/client/middleware/logger.go
View File

@ -103,3 +103,8 @@ func (l *Logger) Discover(ctx context.Context) (acme.Directory, error) {
klog.Infof("Calling Discover")
return l.baseCl.Discover(ctx)
}
func (l *Logger) UpdateAccount(ctx context.Context, a *acme.Account) (*acme.Account, error) {
klog.Infof("Calling UpdateAccount")
return l.baseCl.UpdateAccount(ctx, a)
}

6
pkg/apis/certmanager/v1alpha1/types_issuer.go
View File

@ -538,6 +538,12 @@ type ACMEIssuerStatus struct {
// account details from the CA
// +optional
URI string `json:"uri,omitempty"`
// LastRegisteredEmail is the email associated with the latest registered
// ACME account, in order to track changes made to registered account
// associated with the Issuer
// +optional
LastRegisteredEmail string `json:"lastRegisteredEmail,omitempty"`
}
// IssuerCondition contains condition information for an Issuer.

58
pkg/issuer/acme/setup.go
View File

@ -23,7 +23,7 @@ import (
"net/url"
"strings"
"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@ -40,12 +40,14 @@ import (
const (
errorAccountRegistrationFailed = "ErrRegisterACMEAccount"
errorAccountVerificationFailed = "ErrVerifyACMEAccount"
errorAccountUpdateFailed = "ErrUpdateACMEAccount"
successAccountRegistered = "ACMEAccountRegistered"
successAccountVerified = "ACMEAccountVerified"
messageAccountRegistrationFailed = "Failed to register ACME account: "
messageAccountVerificationFailed = "Failed to verify ACME account: "
messageAccountUpdateFailed = "Failed to update ACME account:"
messageAccountRegistered = "The ACME account was registered with the ACME server"
messageAccountVerified = "The ACME account was verified with the ACME server"
)
@ -147,12 +149,14 @@ func (a *Acme) Setup(ctx context.Context) error {
Status: v1alpha1.ConditionTrue,
})
// If the Host components of the server URL and the account URL match, then
// If the Host components of the server URL and the account URL match,
// and the cached email matches the registered email, then
// we skip re-checking the account status to save excess calls to the
// ACME api.
if hasReadyCondition &&
a.issuer.GetStatus().ACMEStatus().URI != "" &&
parsedAccountURL.Host == parsedServerURL.Host {
parsedAccountURL.Host == parsedServerURL.Host &&
a.issuer.GetStatus().ACMEStatus().LastRegisteredEmail == a.issuer.GetSpec().ACME.Email {
log.Info("skipping re-verifying ACME account as cached registration " +
"details look sufficient")
return nil
@ -192,9 +196,57 @@ func (a *Acme) Setup(ctx context.Context) error {
return err
}
// if we got an account successfully, we must check if the registered
// email is the same as in the issuer spec
// if no email was specified, then registeredEmail will remain empty
registeredEmail := ""
if len(account.Contact) > 0 {
registeredEmail = strings.Replace(account.Contact[0], "mailto:", "", 1)
}
// if they are different, we update the account
specEmail := a.issuer.GetSpec().ACME.Email
if registeredEmail != specEmail {
log.Info("Updating ACME account with email %s", specEmail)
emailurl := []string(nil)
if a.issuer.GetSpec().ACME.Email != "" {
emailurl = []string{fmt.Sprintf("mailto:%s", strings.ToLower(specEmail))}
}
account.Contact = emailurl
account, err = cl.UpdateAccount(ctx, account)
if err != nil {
s := messageAccountUpdateFailed + err.Error()
log.Error(err, "failed to update ACME account")
a.Recorder.Event(a.issuer, v1.EventTypeWarning, errorAccountUpdateFailed, s)
apiutil.SetIssuerCondition(a.issuer, v1alpha1.IssuerConditionReady, v1alpha1.ConditionFalse, errorAccountUpdateFailed, s)
acmeErr, ok := err.(*acmeapi.Error)
// If this is not an ACME error, we will simply return it and retry later
if !ok {
return err
}
// If the status code is 400 (BadRequest), we will *not* retry this registration
// as it implies that something about the request (i.e. email address or private key)
// is invalid.
if acmeErr.StatusCode >= 400 && acmeErr.StatusCode < 500 {
log.Error(acmeErr, "skipping updating account email as a "+
"BadRequest response was returned from the ACME server")
return nil
}
// Otherwise if we receive anything other than a 400, we will retry.
return err
}
}
log.Info("verified existing registration with ACME server")
apiutil.SetIssuerCondition(a.issuer, v1alpha1.IssuerConditionReady, v1alpha1.ConditionTrue, successAccountRegistered, messageAccountRegistered)
a.issuer.GetStatus().ACMEStatus().URI = account.URL
a.issuer.GetStatus().ACMEStatus().LastRegisteredEmail = registeredEmail
return nil
}

76
test/e2e/suite/issuers/acme/issuer.go
View File

@ -33,6 +33,7 @@ import (
const invalidACMEURL = "http://not-a-real-acme-url.com"
const testingACMEEmail = "test@example.com"
const testingACMEEmailAlternative = "another-test@example.com"
const testingACMEPrivateKey = "test-acme-private-key"
var _ = framework.CertManagerDescribe("ACME Issuer", func() {
@ -190,4 +191,79 @@ var _ = framework.CertManagerDescribe("ACME Issuer", func() {
})
Expect(err).NotTo(HaveOccurred())
})
It("should handle updates to the email field", func() {
acmeURL := pebble.Details().Host
acmeIssuer := util.NewCertManagerACMEIssuer(issuerName, acmeURL, testingACMEEmail, testingACMEPrivateKey)
By("Creating an Issuer")
acmeIssuer, err := f.CertManagerClientSet.CertmanagerV1alpha1().Issuers(f.Namespace.Name).Create(acmeIssuer)
Expect(err).NotTo(HaveOccurred())
By("Waiting for Issuer to become Ready")
err = util.WaitForIssuerCondition(f.CertManagerClientSet.CertmanagerV1alpha1().Issuers(f.Namespace.Name),
acmeIssuer.Name,
v1alpha1.IssuerCondition{
Type: v1alpha1.IssuerConditionReady,
Status: v1alpha1.ConditionTrue,
})
Expect(err).NotTo(HaveOccurred())
By("Verifying the ACME account URI is set")
err = util.WaitForIssuerStatusFunc(f.CertManagerClientSet.CertmanagerV1alpha1().Issuers(f.Namespace.Name),
acmeIssuer.Name,
func(i *v1alpha1.Issuer) (bool, error) {
if i.GetStatus().ACMEStatus().URI == "" {
return false, nil
}
return true, nil
})
Expect(err).NotTo(HaveOccurred())
By("Verifying ACME account private key exists")
secret, err := f.KubeClientSet.CoreV1().Secrets(f.Namespace.Name).Get(testingACMEPrivateKey, metav1.GetOptions{})
Expect(err).NotTo(HaveOccurred())
if len(secret.Data) != 1 {
Fail("Expected 1 key in ACME account private key secret, but there was %d", len(secret.Data))
}
By("Verifying the ACME account email has been registered")
err = util.WaitForIssuerStatusFunc(f.CertManagerClientSet.CertmanagerV1alpha1().Issuers(f.Namespace.Name),
acmeIssuer.Name,
func(i *v1alpha1.Issuer) (bool, error) {
registeredEmail := i.GetStatus().ACMEStatus().LastRegisteredEmail
if registeredEmail == testingACMEEmail {
return true, nil
}
return false, nil
})
Expect(err).NotTo(HaveOccurred())
By("Changing the email field")
acmeIssuer, err = f.CertManagerClientSet.CertmanagerV1alpha1().Issuers(f.Namespace.Name).Get(acmeIssuer.Name, metav1.GetOptions{})
acmeIssuer.Spec.ACME.Email = testingACMEEmailAlternative
acmeIssuer, err = f.CertManagerClientSet.CertmanagerV1alpha1().Issuers(f.Namespace.Name).Update(acmeIssuer)
Expect(err).NotTo(HaveOccurred())
By("Waiting for Issuer to become Ready")
err = util.WaitForIssuerCondition(f.CertManagerClientSet.CertmanagerV1alpha1().Issuers(f.Namespace.Name),
acmeIssuer.Name,
v1alpha1.IssuerCondition{
Type: v1alpha1.IssuerConditionReady,
Status: v1alpha1.ConditionTrue,
})
Expect(err).NotTo(HaveOccurred())
By("Verifying the changed ACME account email has been registered")
err = util.WaitForIssuerStatusFunc(f.CertManagerClientSet.CertmanagerV1alpha1().Issuers(f.Namespace.Name),
acmeIssuer.Name,
func(i *v1alpha1.Issuer) (bool, error) {
registeredEmail := i.GetStatus().ACMEStatus().LastRegisteredEmail
if registeredEmail == testingACMEEmailAlternative {
return true, nil
}
return false, nil
})
Expect(err).NotTo(HaveOccurred())
})
})