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

Merge pull request #4628 from irbekrm/sync_cleanup

Browse Source 
Order sync cleanup
This commit is contained in:
jetstack-bot 2022-01-10 20:27:04 +00:00 committed by GitHub
commit 2e465fbf34
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 94 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
pkg/controller/acmeorders/sync.go
View File

@ -152,6 +152,15 @@ func (c *controller) Sync(ctx context.Context, o *cmacme.Order) (err error) {
return err
}
if o.Status.State == cmacme.Ready {
log.V(logf.DebugLevel).Info("Finalizing Order as order state is 'Ready'")
return c.finalizeOrder(ctx, cl, o, genericIssuer)
}
// Note: each of the following code paths uses the ACME Order retrieved
// here. Be mindful when adding new code below this call to ACME server-
// if the new code does not need this ACME order, try to place it above
// this call to avoid extra calls to ACME.
acmeOrder, err := getACMEOrder(ctx, cl, o)
// Order probably has been deleted, we cannot recover here.
if acmeErr, ok := err.(*acmeapi.Error); ok {
@ -167,16 +176,12 @@ func (c *controller) Sync(ctx context.Context, o *cmacme.Order) (err error) {
}
switch {
case o.Status.State == cmacme.Ready:
log.V(logf.DebugLevel).Info("Finalizing Order as order state is 'Ready'")
return c.finalizeOrder(ctx, cl, o, genericIssuer)
case anyChallengesFailed(challenges):
// TODO (@munnerz): instead of waiting for the ACME server to mark this
// Order as failed, we could just mark the Order as failed as there is
// no way that we will attempt and continue the order anyway.
log.V(logf.DebugLevel).Info("Update Order status as at least one Challenge has failed")
_, err := c.updateOrderStatus(ctx, cl, o)
_, err := c.updateOrderStatusFromACMEOrder(ctx, cl, o, acmeOrder)
if acmeErr, ok := err.(*acmeapi.Error); ok {
if acmeErr.StatusCode >= 400 && acmeErr.StatusCode < 500 {
log.Error(err, "failed to update Order status due to a 4xx error, marking Order as failed")
@ -216,7 +221,7 @@ func (c *controller) Sync(ctx context.Context, o *cmacme.Order) (err error) {
case !anyChallengesFailed(challenges) && allChallengesFinal(challenges):
log.V(logf.DebugLevel).Info("All challenges are in a final state, updating order state")
_, err := c.updateOrderStatus(ctx, cl, o)
_, err := c.updateOrderStatusFromACMEOrder(ctx, cl, o, acmeOrder)
if acmeErr, ok := err.(*acmeapi.Error); ok {
if acmeErr.StatusCode >= 400 && acmeErr.StatusCode < 500 {
log.Error(err, "failed to update Order status due to a 4xx error, marking Order as failed")
@ -285,6 +290,11 @@ func (c *controller) updateOrderStatus(ctx context.Context, cl acmecl.Interface,
if err != nil {
return nil, err
}
return c.updateOrderStatusFromACMEOrder(ctx, cl, o, acmeOrder)
}
func (c *controller) updateOrderStatusFromACMEOrder(ctx context.Context, cl acmecl.Interface, o *cmacme.Order, acmeOrder *acmeapi.Order) (*acmeapi.Order, error) {
// Workaround bug in golang.org/x/crypto/acme implementation whereby the
// order's URI field will be empty when calling GetOrder due to the
// 'Location' header not being set on the response from the ACME server.
@ -514,11 +524,11 @@ func (c *controller) finalizeOrder(ctx context.Context, cl acmecl.Interface, o *
// if it is already in the 'valid' state, as upon retry we will
// then retrieve the Certificate resource.
_, errUpdate := c.updateOrderStatus(ctx, cl, o)
if acmeErr, ok := err.(*acmeapi.Error); ok {
if acmeErr, ok := errUpdate.(*acmeapi.Error); ok {
if acmeErr.StatusCode >= 400 && acmeErr.StatusCode < 500 {
log.Error(err, "failed to update Order status due to a 4xx error, marking Order as failed")
c.setOrderState(&o.Status, string(cmacme.Errored))
o.Status.Reason = fmt.Sprintf("Failed to retrieve Order resource: %v", err)
o.Status.Reason = fmt.Sprintf("Failed to retrieve Order resource: %v", errUpdate)
return nil
}
}

77
pkg/controller/acmeorders/sync_test.go
View File

@ -39,7 +39,7 @@ import (
"github.com/jetstack/cert-manager/test/unit/gen"
)
func TestSyncHappyPath(t *testing.T) {
func TestSync(t *testing.T) {
nowTime := time.Now()
nowMetaTime := metav1.NewTime(nowTime)
fixedClock := fakeclock.NewFakeClock(nowTime)
@ -113,12 +113,40 @@ func TestSyncHappyPath(t *testing.T) {
State: cmacme.Errored,
}
erroredStatusWithDetail := cmacme.OrderStatus{
State: cmacme.Errored,
FailureTime: &nowMetaTime,
URL: "http://testurl.com/abcde",
FinalizeURL: "http://testurl.com/abcde/finalize",
Reason: "Failed to finalize Order: 429 : some error",
Authorizations: []cmacme.ACMEAuthorization{
{
URL: "http://authzurl",
Identifier: "test.com",
InitialState: cmacme.Valid,
Challenges: []cmacme.ACMEChallenge{
{
URL: "http://chalurl",
Token: "token",
Type: "http-01",
},
},
},
},
}
acmeError429 := acmeapi.Error{
StatusCode: 429,
Detail: "some error",
}
testOrderPending := gen.OrderFrom(testOrder, gen.SetOrderStatus(pendingStatus))
testOrderInvalid := testOrderPending.DeepCopy()
testOrderInvalid.Status.State = cmacme.Invalid
testOrderInvalid.Status.FailureTime = &nowMetaTime
testOrderErrored := gen.OrderFrom(testOrder, gen.SetOrderStatus(erroredStatus))
testOrderErrored.Status.FailureTime = &nowMetaTime
testOrderErroredWithDetail := gen.OrderFrom(testOrderPending, gen.SetOrderStatus(erroredStatusWithDetail))
testOrderValid := testOrderPending.DeepCopy()
testOrderValid.Status.State = cmacme.Valid
// pem encoded word 'test'
@ -514,6 +542,53 @@ rUCGwbCUDI0mxadJ3Bz4WxR6fyNpBK2yAinWEsikxqEt
},
},
},
"call FinalizeOrder and update the order state to 'errored' if finalize fails with a 4xx ACME error": {
order: gen.OrderFrom(testOrderErroredWithDetail, gen.SetOrderState(cmacme.Ready)),
builder: &testpkg.Builder{
CertManagerObjects: []runtime.Object{testIssuerHTTP01TestCom, gen.OrderFrom(testOrderErroredWithDetail, gen.SetOrderState(cmacme.Ready))},
ExpectedActions: []testpkg.Action{
testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("orders"),
"status",
testOrderErroredWithDetail.Namespace, testOrderErroredWithDetail)),
},
},
acmeClient: &acmecl.FakeACME{
FakeGetOrder: func(_ context.Context, url string) (*acmeapi.Order, error) {
return testACMEOrderReady, nil
},
FakeCreateOrderCert: func(_ context.Context, url string, csr []byte, bundle bool) ([][]byte, string, error) {
return nil, "", &acmeError429
},
FakeHTTP01ChallengeResponse: func(s string) (string, error) {
// TODO: assert s = "token"
return "key", nil
},
},
},
"call FinalizeOrder, return error if finalize fails with an unspecified error": {
order: gen.OrderFrom(testOrderErroredWithDetail, gen.SetOrderState(cmacme.Ready)),
builder: &testpkg.Builder{
CertManagerObjects: []runtime.Object{testIssuerHTTP01TestCom, gen.OrderFrom(testOrderErroredWithDetail, gen.SetOrderState(cmacme.Ready))},
ExpectedActions: []testpkg.Action{
// testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("orders"),
// "status",
// testOrderErroredWithDetail.Namespace, testOrderErroredWithDetail)),
},
},
acmeClient: &acmecl.FakeACME{
FakeGetOrder: func(_ context.Context, url string) (*acmeapi.Order, error) {
return testACMEOrderReady, nil
},
FakeCreateOrderCert: func(_ context.Context, url string, csr []byte, bundle bool) ([][]byte, string, error) {
return nil, "", errors.New("some error")
},
FakeHTTP01ChallengeResponse: func(s string) (string, error) {
// TODO: assert s = "token"
return "key", nil
},
},
expectErr: true,
},
"call FinalizeOrder fetch alternate cert chain": {
order: testOrderReady.DeepCopy(),
builder: &testpkg.Builder{