diff --git a/pkg/controller/acmechallenges/sync.go b/pkg/controller/acmechallenges/sync.go index 5114f4cda..c5e492e00 100644 --- a/pkg/controller/acmechallenges/sync.go +++ b/pkg/controller/acmechallenges/sync.go @@ -18,6 +18,7 @@ package acmechallenges import ( "context" + "errors" "fmt" acmeapi "golang.org/x/crypto/acme" @@ -300,11 +301,23 @@ func (c *controller) syncChallengeStatus(ctx context.Context, cl acmecl.Interfac return fmt.Errorf("challenge URL is blank - challenge has not been created yet") } - acmeChallenge, err := cl.GetChallenge(ctx, ch.Spec.URL) + acmeAuthorization, err := cl.GetAuthorization(ctx, ch.Spec.AuthorizationURL) if err != nil { return err } + var acmeChallenge *acmeapi.Challenge + for _, challenge := range acmeAuthorization.Challenges { + if challenge.URI == ch.Spec.URL { + acmeChallenge = challenge + break + } + } + + if acmeChallenge == nil { + return errors.New("challenge was not present in authorization") + } + // TODO: should we validate the State returned by the ACME server here? cmState := cmacme.State(acmeChallenge.Status) // be nice to our users and check if there is an error that we diff --git a/pkg/controller/acmechallenges/sync_test.go b/pkg/controller/acmechallenges/sync_test.go index 45103914e..a31fbb0bf 100644 --- a/pkg/controller/acmechallenges/sync_test.go +++ b/pkg/controller/acmechallenges/sync_test.go @@ -86,6 +86,61 @@ func TestSyncHappyPath(t *testing.T) { ) tests := map[string]testT{ + "if GetAuthorization doesn't return challenge, error": { + challenge: gen.ChallengeFrom(baseChallenge, + gen.SetChallengeProcessing(true), + gen.SetChallengeURL("testurl"), + ), + builder: &testpkg.Builder{ + CertManagerObjects: []runtime.Object{gen.ChallengeFrom(baseChallenge, + gen.SetChallengeProcessing(true), + gen.SetChallengeURL("testurl"), + ), testIssuerHTTP01Enabled}, + ExpectedActions: []testpkg.Action{}, + }, + expectErr: true, + acmeClient: &acmecl.FakeACME{ + FakeGetAuthorization: func(ctx context.Context, url string) (*acmeapi.Authorization, error) { + return &acmeapi.Authorization{ + Challenges: []*acmeapi.Challenge{ + {URI: "foo", Status: acmeapi.StatusPending}, + }, + }, nil + }, + }, + }, + "if GetAuthorization returns challenge ready, update ready": { + challenge: gen.ChallengeFrom(baseChallenge, + gen.SetChallengeProcessing(true), + gen.SetChallengeURL("testurl"), + ), + builder: &testpkg.Builder{ + CertManagerObjects: []runtime.Object{gen.ChallengeFrom(baseChallenge, + gen.SetChallengeProcessing(true), + gen.SetChallengeURL("testurl"), + ), testIssuerHTTP01Enabled}, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction( + coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("challenges"), + "status", + gen.DefaultTestNamespace, + gen.ChallengeFrom(baseChallenge, + gen.SetChallengeProcessing(true), + gen.SetChallengeURL("testurl"), + gen.SetChallengeState(cmacme.Ready), + ))), + }, + }, + acmeClient: &acmecl.FakeACME{ + FakeGetAuthorization: func(ctx context.Context, url string) (*acmeapi.Authorization, error) { + return &acmeapi.Authorization{ + Challenges: []*acmeapi.Challenge{ + {URI: "testurl", Status: acmeapi.StatusReady}, + }, + }, nil + }, + }, + }, "update status if state is unknown": { challenge: gen.ChallengeFrom(baseChallenge, gen.SetChallengeProcessing(true), @@ -109,8 +164,12 @@ func TestSyncHappyPath(t *testing.T) { }, }, acmeClient: &acmecl.FakeACME{ - FakeGetChallenge: func(ctx context.Context, url string) (*acmeapi.Challenge, error) { - return &acmeapi.Challenge{Status: acmeapi.StatusPending}, nil + FakeGetAuthorization: func(ctx context.Context, url string) (*acmeapi.Authorization, error) { + return &acmeapi.Authorization{ + Challenges: []*acmeapi.Challenge{ + {URI: "testurl", Status: acmeapi.StatusPending}, + }, + }, nil }, }, },