Skip re-checking ACME account status if Issuer is ready and URL hosts match

Signed-off-by: James Munnelly <james@munnelly.eu>
This commit is contained in:
James Munnelly 2018-10-31 09:50:52 +00:00
parent eca128747a
commit b9947e3247
2 changed files with 88 additions and 16 deletions

View File

@ -87,9 +87,10 @@ func (h *helperImpl) ReadPrivateKey(sel cmapi.SecretKeySelector, ns string) (*rs
data, ok := s.Data[sel.Key]
if !ok {
return nil, cmerrors.NewInvalidData(fmt.Sprintf("no secret data found for key %q in secret %q", sel.Key, sel.Name))
return nil, cmerrors.NewInvalidData("No secret data found for key %q in secret %q", sel.Key, sel.Name)
}
// DecodePrivateKeyBytes already wraps errors with NewInvalidData.
pk, err := pki.DecodePrivateKeyBytes(data)
if err != nil {
return nil, err
@ -97,7 +98,7 @@ func (h *helperImpl) ReadPrivateKey(sel cmapi.SecretKeySelector, ns string) (*rs
rsaKey, ok := pk.(*rsa.PrivateKey)
if !ok {
return nil, fmt.Errorf("ACME private key in %q is not of type RSA", sel.Name)
return nil, cmerrors.NewInvalidData("ACME private key in %q is not of type RSA", sel.Name)
}
return rsaKey, nil

View File

@ -20,11 +20,12 @@ import (
"context"
"crypto/rsa"
"fmt"
"net/url"
"strings"
"github.com/golang/glog"
"k8s.io/api/core/v1"
k8sErrors "k8s.io/apimachinery/pkg/api/errors"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"github.com/jetstack/cert-manager/pkg/acme"
@ -68,16 +69,13 @@ func (a *Acme) Setup(ctx context.Context) (issuer.SetupResponse, error) {
ns = a.IssuerOptions.ClusterResourceNamespace
}
// attempt to obtain the existing private key, if it does not exist then
// we generate one
// attempt to obtain the existing private key from the apiserver.
// if it does not exist then we generate one
// if it contains invalid data, warn the user and return without error.
// if any other error occurs, return it and retry.
pk, err := a.helper.ReadPrivateKey(a.issuer.GetSpec().ACME.PrivateKey, ns)
if err != nil {
if !k8sErrors.IsNotFound(err) && !errors.IsInvalidData(err) {
s := messageAccountVerificationFailed + err.Error()
a.issuer.UpdateStatusCondition(v1alpha1.IssuerConditionReady, v1alpha1.ConditionFalse, errorAccountVerificationFailed, s)
return issuer.SetupResponse{Requeue: true}, fmt.Errorf(s)
}
switch {
case apierrors.IsNotFound(err):
glog.Infof("%s: generating acme account private key %q", a.issuer.GetObjectMeta().Name, a.issuer.GetSpec().ACME.PrivateKey.Name)
pk, err = a.createAccountPrivateKey(a.issuer.GetSpec().ACME.PrivateKey, ns)
if err != nil {
@ -85,26 +83,99 @@ func (a *Acme) Setup(ctx context.Context) (issuer.SetupResponse, error) {
a.issuer.UpdateStatusCondition(v1alpha1.IssuerConditionReady, v1alpha1.ConditionFalse, errorAccountRegistrationFailed, s)
return issuer.SetupResponse{Requeue: true}, fmt.Errorf(s)
}
// We clear the ACME account URI as we have generated a new private key
a.issuer.GetStatus().ACMEStatus().URI = ""
case errors.IsInvalidData(err):
a.issuer.UpdateStatusCondition(v1alpha1.IssuerConditionReady, v1alpha1.ConditionFalse, errorAccountVerificationFailed, fmt.Sprintf("Account private key is invalid: %v", err))
return issuer.SetupResponse{}, nil
case err != nil:
s := messageAccountVerificationFailed + err.Error()
a.issuer.UpdateStatusCondition(v1alpha1.IssuerConditionReady, v1alpha1.ConditionFalse, errorAccountVerificationFailed, s)
return issuer.SetupResponse{Requeue: true}, fmt.Errorf(s)
}
cl, err := acme.ClientWithKey(a.issuer, pk)
if err != nil {
s := messageAccountVerificationFailed + err.Error()
glog.V(4).Infof("%s: %s", a.issuer.GetObjectMeta().Name, s)
glog.Infof("%s: %s", a.issuer.GetObjectMeta().Name, s)
a.Recorder.Event(a.issuer, v1.EventTypeWarning, errorAccountVerificationFailed, s)
return issuer.SetupResponse{Requeue: true}, err
}
// TODO: perform a complex check to determine whether we need to verify
// the existing registration with the ACME server.
// This should take into account the ACME server URL, as well as a checksum
// of the private key's contents.
// Alternatively, we could add 'observed generation' fields here, tracking
// the most recent copy of the Issuer and Secret resource we have checked
// already.
rawServerURL := a.issuer.GetSpec().ACME.Server
parsedServerURL, err := url.Parse(a.issuer.GetStatus().ACMEStatus().URI)
if err != nil {
a.Recorder.Eventf(a.issuer, v1.EventTypeWarning, "InvalidURL",
"Failed to parse existing ACME server URI %q: %v", rawServerURL, err)
// absorb errors as retrying will not help resolve this error
return issuer.SetupResponse{}, nil
}
rawAccountURL := a.issuer.GetStatus().ACMEStatus().URI
parsedAccountURL, err := url.Parse(a.issuer.GetStatus().ACMEStatus().URI)
if err != nil {
a.Recorder.Eventf(a.issuer, v1.EventTypeWarning, "InvalidURL",
"Failed to parse existing ACME account URI %q: %v", rawAccountURL, err)
// absorb errors as retrying will not help resolve this error
return issuer.SetupResponse{}, nil
}
hasReadyCondition := a.issuer.HasCondition(v1alpha1.IssuerCondition{
Type: v1alpha1.IssuerConditionReady,
Status: v1alpha1.ConditionTrue,
})
// If the Host components of the server URL and the account URL match, 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 {
glog.Infof("Skipping re-verifying ACME account as cached registration " +
"details look sufficient.")
return issuer.SetupResponse{}, nil
}
if parsedAccountURL.Host != parsedServerURL.Host {
glog.Infof("ACME server URL host and ACME private key registration " +
"host differ. Re-checking ACME account registration.")
}
// registerAccount will also verify the account exists if it already
// exists.
account, err := a.registerAccount(ctx, cl)
if err != nil {
s := messageAccountVerificationFailed + err.Error()
glog.V(4).Infof("%s: %s", a.issuer.GetObjectMeta().Name, s)
glog.Infof("%s: %s", a.issuer.GetObjectMeta().Name, s)
a.Recorder.Event(a.issuer, v1.EventTypeWarning, errorAccountVerificationFailed, s)
a.issuer.UpdateStatusCondition(v1alpha1.IssuerConditionReady, v1alpha1.ConditionFalse, errorAccountRegistrationFailed, s)
acmeErr, ok := err.(*acmeapi.Error)
// If this is not an ACME error, we will simply return it and retry later
if !ok {
return issuer.SetupResponse{Requeue: true}, 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 {
glog.Infof("Skipping retrying account registration as a BadRequest response was returned from the ACME server: %v", acmeErr)
return issuer.SetupResponse{}, nil
}
// Otherwise if we receive anything other than a 400, we will retry.
return issuer.SetupResponse{Requeue: true}, err
}
@ -112,7 +183,7 @@ func (a *Acme) Setup(ctx context.Context) (issuer.SetupResponse, error) {
a.issuer.UpdateStatusCondition(v1alpha1.IssuerConditionReady, v1alpha1.ConditionTrue, successAccountRegistered, messageAccountRegistered)
a.issuer.GetStatus().ACMEStatus().URI = account.URL
return issuer.SetupResponse{}, err
return issuer.SetupResponse{}, nil
}
// registerAccount will register a new ACME account with the server. If an