diff --git a/pkg/controller/acmechallenges/sync.go b/pkg/controller/acmechallenges/sync.go index 6cae4a03e..bec5dc589 100644 --- a/pkg/controller/acmechallenges/sync.go +++ b/pkg/controller/acmechallenges/sync.go @@ -222,8 +222,25 @@ func (c *Controller) syncChallengeStatus(ctx context.Context, cl acmecl.Interfac return err } - // TODO: should we validate the State returned by the ACME server here? + switch acmeChallenge.Status { + case acmeapi.StatusPending, acmeapi.StatusValid, acmeapi.StatusInvalid: + // do nothing + default: + // bogus status from acme API that we can't handle. + return fmt.Errorf("acme api returned bogus status for challenge: %v", acmeChallenge.Status) + } cmState := cmapi.State(acmeChallenge.Status) + if cmState == cmapi.Invalid { + // be nice to our users and check if there is an error that we + // can tell them about in the reason field + // TODO(dmo): problems may be compound and they may be tagged with + // a type field that suggests changes we should make (like provisioning + // an account). We might be able to handle errors more gracefully using + // this info + if acmeChallenge.Error != nil { + ch.Status.Reason = acmeChallenge.Error.Detail + } + } ch.Status.State = cmState return nil diff --git a/pkg/controller/acmeorders/sync.go b/pkg/controller/acmeorders/sync.go index 8a1260a54..e17f5745f 100644 --- a/pkg/controller/acmeorders/sync.go +++ b/pkg/controller/acmeorders/sync.go @@ -318,7 +318,10 @@ func (c *Controller) createOrder(ctx context.Context, cl acmecl.Interface, issue return fmt.Errorf("error creating new order: %v", err) } - c.setOrderStatus(&o.Status, acmeOrder) + err = c.setOrderStatus(&o.Status, acmeOrder) + if err != nil { + return err + } chals := make([]cmapi.ChallengeSpec, len(acmeOrder.Authorizations)) // we only set the status.challenges field when we first create the order, @@ -427,9 +430,7 @@ func (c *Controller) syncOrderStatus(ctx context.Context, cl acmecl.Interface, o return err } - c.setOrderStatus(&o.Status, acmeOrder) - - return nil + return c.setOrderStatus(&o.Status, acmeOrder) } func buildChallenge(i int, o *cmapi.Order, chalSpec cmapi.ChallengeSpec) *cmapi.Challenge { @@ -449,13 +450,32 @@ func buildChallenge(i int, o *cmapi.Order, chalSpec cmapi.ChallengeSpec) *cmapi. // setOrderStatus will populate the given OrderStatus struct with the details from // the provided ACME Order. -func (c *Controller) setOrderStatus(o *cmapi.OrderStatus, acmeOrder *acmeapi.Order) { - // TODO: should we validate the State returned by the ACME server here? +func (c *Controller) setOrderStatus(o *cmapi.OrderStatus, acmeOrder *acmeapi.Order) error { + switch acmeOrder.Status { + case acmeapi.StatusPending, acmeapi.StatusReady, acmeapi.StatusProcessing, acmeapi.StatusValid, acmeapi.StatusInvalid: + // do nothing + default: + // bogus status from acme API that we can't handle. + return fmt.Errorf("acme api returned bogus status for order: %v", acmeOrder.Status) + } cmState := cmapi.State(acmeOrder.Status) + if cmState == cmapi.Invalid { + // be nice to our users and check if there is an error that we + // can tell them about in the reason field + // TODO(dmo): problems may be compound and they may be tagged with + // a type field that suggests changes we should make (like provisioning + // an account). We might be able to handle errors more gracefully using + // this info + if acmeOrder.Error != nil { + o.Reason = acmeOrder.Error.Detail + } + } c.setOrderState(o, cmState) o.URL = acmeOrder.URL o.FinalizeURL = acmeOrder.FinalizeURL + + return nil } func challengeLabelsForOrder(o *cmapi.Order) map[string]string {