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

Merge pull request #5901 from irbekrm/remove_extra_acme_get

Browse Source 
Reduce the amount of ACME calls during issuance
This commit is contained in:
jetstack-bot 2023-04-06 09:02:01 +01:00 committed by GitHub
commit 18dae5f117
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 241 additions and 141 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -36,13 +36,24 @@ type Interface interface {
ListCertAlternates(ctx context.Context, url string) ([]string, error)
WaitOrder(ctx context.Context, url string) (*acme.Order, error)
CreateOrderCert(ctx context.Context, finalizeURL string, csr []byte, bundle bool) (der [][]byte, certURL string, err error)
// Accept will (in success cases) be called once per a Challenge once it
// has passed self-check and is ready to be verified by the ACME server.
Accept(ctx context.Context, chal *acme.Challenge) (*acme.Challenge, error)
GetChallenge(ctx context.Context, url string) (*acme.Challenge, error)
// GetAuthorization will be called once for each required authorization
// for an Order. Additionally it will be called most likely once when a
// Challenge has been scheduled for processing to retrieve its status.
GetAuthorization(ctx context.Context, url string) (*acme.Authorization, error)
// WaitAuthorization will, in success cases, be called once per
// Challenge after it has been accepted.
WaitAuthorization(ctx context.Context, url string) (*acme.Authorization, error)
Register(ctx context.Context, acct *acme.Account, prompt func(tosURL string) bool) (*acme.Account, error)
GetReg(ctx context.Context, url string) (*acme.Account, error)
// HTTP01ChallengeResponse will be called once when an cert-manager.io
// Challenge for an http-01 challenge type is being created.
HTTP01ChallengeResponse(token string) (string, error)
// DNS01ChallengeResponse will be called once when an cert-manager.io
// Challenge for an http-01 challenge type is being created.
DNS01ChallengeRecord(token string) (string, error)
Discover(ctx context.Context) (acme.Directory, error)
UpdateReg(ctx context.Context, a *acme.Account) (*acme.Account, error)

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

@ -121,7 +121,7 @@ func (c *controller) Sync(ctx context.Context, o *cmacme.Order) (err error) {
}
dbg.Info("Computing list of Challenge resources that need to exist to complete this Order")
requiredChallenges, err := buildRequiredChallenges(ctx, cl, genericIssuer, o)
requiredChallenges, err := buildPartialRequiredChallenges(ctx, genericIssuer, o)
if err != nil {
log.Error(err, "Failed to determine the list of Challenge resources needed for the Order")
c.recorder.Eventf(o, corev1.EventTypeWarning, reasonSolver, "Failed to determine a valid solver configuration for the set of domains on the Order: %v", err)
@ -142,6 +142,10 @@ func (c *controller) Sync(ctx context.Context, o *cmacme.Order) (err error) {
switch {
case needToCreateChallenges:
log.V(logf.DebugLevel).Info("Creating additional Challenge resources to complete Order")
requiredChallenges, err = ensureKeysForChallenges(cl, requiredChallenges)
if err != nil {
return err
}
return c.createRequiredChallenges(ctx, o, requiredChallenges)
case needToDeleteChallenges:
log.V(logf.DebugLevel).Info("Deleting leftover Challenge resources no longer required by Order")
@ -161,6 +165,14 @@ func (c *controller) Sync(ctx context.Context, o *cmacme.Order) (err error) {
return c.finalizeOrder(ctx, cl, o, genericIssuer)
}
// At this point, if no Challenges have failed or reached a final state,
// we can return without taking any action. This controller will resync
// the Order on any owned Challenge events.
if !anyChallengesFailed(challenges) && !allChallengesFinal(challenges) {
log.V(logf.DebugLevel).Info("No action taken")
return nil
}
// 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
@ -181,9 +193,14 @@ func (c *controller) Sync(ctx context.Context, o *cmacme.Order) (err error) {
switch {
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.
// 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. This might, however, be a breaking change
// in edge cases where the status of the order resource in ACME
// server cannot be determined from challenge resource statuses
// correctly. Do not change this unless there is a real need for
// it.
log.V(logf.DebugLevel).Info("Update Order status as at least one Challenge has failed")
_, err := c.updateOrderStatusFromACMEOrder(ctx, cl, o, acmeOrder)
if acmeErr, ok := err.(*acmeapi.Error); ok {
@ -386,7 +403,7 @@ func (c *controller) fetchMetadataForAuthorizations(ctx context.Context, o *cmac
return nil
}
func (c *controller) anyRequiredChallengesDoNotExist(requiredChallenges []cmacme.Challenge) (bool, error) {
func (c *controller) anyRequiredChallengesDoNotExist(requiredChallenges []*cmacme.Challenge) (bool, error) {
for _, ch := range requiredChallenges {
_, err := c.challengeLister.Challenges(ch.Namespace).Get(ch.Name)
if apierrors.IsNotFound(err) {
@ -399,9 +416,9 @@ func (c *controller) anyRequiredChallengesDoNotExist(requiredChallenges []cmacme
return false, nil
}
func (c *controller) createRequiredChallenges(ctx context.Context, o *cmacme.Order, requiredChallenges []cmacme.Challenge) error {
func (c *controller) createRequiredChallenges(ctx context.Context, o *cmacme.Order, requiredChallenges []*cmacme.Challenge) error {
for _, ch := range requiredChallenges {
_, err := c.cmClient.AcmeV1().Challenges(ch.Namespace).Create(ctx, &ch, metav1.CreateOptions{})
_, err := c.cmClient.AcmeV1().Challenges(ch.Namespace).Create(ctx, ch, metav1.CreateOptions{})
if apierrors.IsAlreadyExists(err) {
continue
}
@ -413,7 +430,7 @@ func (c *controller) createRequiredChallenges(ctx context.Context, o *cmacme.Ord
return nil
}
func (c *controller) anyLeftoverChallengesExist(o *cmacme.Order, requiredChallenges []cmacme.Challenge) (bool, error) {
func (c *controller) anyLeftoverChallengesExist(o *cmacme.Order, requiredChallenges []*cmacme.Challenge) (bool, error) {
leftoverChallenges, err := c.determineLeftoverChallenges(o, requiredChallenges)
if err != nil {
return false, err
@ -422,7 +439,7 @@ func (c *controller) anyLeftoverChallengesExist(o *cmacme.Order, requiredChallen
return len(leftoverChallenges) > 0, nil
}
func (c *controller) deleteLeftoverChallenges(ctx context.Context, o *cmacme.Order, requiredChallenges []cmacme.Challenge) error {
func (c *controller) deleteLeftoverChallenges(ctx context.Context, o *cmacme.Order, requiredChallenges []*cmacme.Challenge) error {
leftover, err := c.determineLeftoverChallenges(o, requiredChallenges)
if err != nil {
return err
@ -452,7 +469,7 @@ func (c *controller) deleteAllChallenges(ctx context.Context, o *cmacme.Order) e
return nil
}
func (c *controller) determineLeftoverChallenges(o *cmacme.Order, requiredChallenges []cmacme.Challenge) ([]*cmacme.Challenge, error) {
func (c *controller) determineLeftoverChallenges(o *cmacme.Order, requiredChallenges []*cmacme.Challenge) ([]*cmacme.Challenge, error) {
requiredNames := map[string]struct{}{}
for _, ch := range requiredChallenges {
requiredNames[ch.Name] = struct{}{}

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

@ -206,10 +206,16 @@ rUCGwbCUDI0mxadJ3Bz4WxR6fyNpBK2yAinWEsikxqEt
return "key", nil
},
}
testAuthorizationChallenge, err := buildChallenge(context.TODO(), fakeHTTP01ACMECl, testIssuerHTTP01TestCom, testOrderPending, testOrderPending.Status.Authorizations[0])
testAuthorizationChallenge, err := buildPartialChallenge(context.TODO(), testIssuerHTTP01TestCom, testOrderPending, testOrderPending.Status.Authorizations[0])
if err != nil {
t.Fatalf("error building Challenge resource test fixture: %v", err)
}
key, err := fakeHTTP01ACMECl.FakeHTTP01ChallengeResponse(testAuthorizationChallenge.Spec.Token)
if err != nil {
t.Fatalf("error building Challenge resource test fixture: %v", err)
}
testAuthorizationChallenge.Spec.Key = key
testAuthorizationChallengeValid := testAuthorizationChallenge.DeepCopy()
testAuthorizationChallengeValid.Status.State = cmacme.Valid
testAuthorizationChallengeInvalid := testAuthorizationChallenge.DeepCopy()
@ -338,7 +344,7 @@ rUCGwbCUDI0mxadJ3Bz4WxR6fyNpBK2yAinWEsikxqEt
testpkg.NewAction(coretesting.NewCreateAction(cmacme.SchemeGroupVersion.WithResource("challenges"), testAuthorizationChallenge.Namespace, testAuthorizationChallenge)),
},
ExpectedEvents: []string{
`Normal Created Created Challenge resource "testorder-2179654896" for domain "test.com"`,
`Normal Created Created Challenge resource "testorder-756011405" for domain "test.com"`,
},
},
acmeClient: &acmecl.FakeACME{
@ -490,9 +496,6 @@ rUCGwbCUDI0mxadJ3Bz4WxR6fyNpBK2yAinWEsikxqEt
ExpectedActions: []testpkg.Action{},
},
acmeClient: &acmecl.FakeACME{
FakeGetOrder: func(_ context.Context, url string) (*acmeapi.Order, error) {
return testACMEOrderPending, nil
},
FakeHTTP01ChallengeResponse: func(s string) (string, error) {
// TODO: assert s = "token"
return "key", nil

56
pkg/controller/acmeorders/util.go
View File

@ -35,8 +35,11 @@ var (
orderGvk = cmacme.SchemeGroupVersion.WithKind("Order")
)
func buildRequiredChallenges(ctx context.Context, cl acmecl.Interface, issuer cmapi.GenericIssuer, o *cmacme.Order) ([]cmacme.Challenge, error) {
chs := make([]cmacme.Challenge, 0)
// buildPartialRequiredChallenges builds partial required ACME challenges by
// looking at authorization on order spec and related issuer. It does not call
// ACME. ensureKeysForChallenge must be called before creating the Challenge.
func buildPartialRequiredChallenges(ctx context.Context, issuer cmapi.GenericIssuer, o *cmacme.Order) ([]*cmacme.Challenge, error) {
chs := make([]*cmacme.Challenge, 0)
for _, a := range o.Status.Authorizations {
if a.InitialState == cmacme.Valid {
wc := false
@ -46,17 +49,20 @@ func buildRequiredChallenges(ctx context.Context, cl acmecl.Interface, issuer cm
logf.FromContext(ctx).V(logf.DebugLevel).Info("Authorization already valid, not creating Challenge resource", "identifier", a.Identifier, "is_wildcard", wc)
continue
}
ch, err := buildChallenge(ctx, cl, issuer, o, a)
ch, err := buildPartialChallenge(ctx, issuer, o, a)
if err != nil {
return nil, err
}
chs = append(chs, *ch)
chs = append(chs, ch)
}
return chs, nil
}
func buildChallenge(ctx context.Context, cl acmecl.Interface, issuer cmapi.GenericIssuer, o *cmacme.Order, authz cmacme.ACMEAuthorization) (*cmacme.Challenge, error) {
chSpec, err := challengeSpecForAuthorization(ctx, cl, issuer, o, authz)
// buildPartialChallenge builds a challenge for the required ACME Authorization.
// The spec will be populated with fields that can be determined by looking at
// the ACME Authorization object returned in Order.
func buildPartialChallenge(ctx context.Context, issuer cmapi.GenericIssuer, o *cmacme.Order, authz cmacme.ACMEAuthorization) (*cmacme.Challenge, error) {
chSpec, err := partialChallengeSpecForAuthorization(ctx, issuer, o, authz)
if err != nil {
// TODO: in this case, we should probably not return the error as it's
// unlikely we can make it succeed by retrying.
@ -78,7 +84,10 @@ func buildChallenge(ctx context.Context, cl acmecl.Interface, issuer cmapi.Gener
}, nil
}
func challengeSpecForAuthorization(ctx context.Context, cl acmecl.Interface, issuer cmapi.GenericIssuer, o *cmacme.Order, authz cmacme.ACMEAuthorization) (*cmacme.ChallengeSpec, error) {
// partialChallengeSpecForAuthorization builds a partial challenge spec by
// looking at the ACME authorization object and issuer. It does not make any
// ACME calls.
func partialChallengeSpecForAuthorization(ctx context.Context, issuer cmapi.GenericIssuer, o *cmacme.Order, authz cmacme.ACMEAuthorization) (*cmacme.ChallengeSpec, error) {
log := logf.FromContext(ctx, "challengeSpecForAuthorization")
dbg := log.V(logf.DebugLevel)
@ -258,11 +267,6 @@ func challengeSpecForAuthorization(ctx context.Context, cl acmecl.Interface, iss
return nil, err
}
key, err := keyForChallenge(cl, selectedChallenge.Token, chType)
if err != nil {
return nil, err
}
// 4. handle overriding the HTTP01 ingress class and name fields using the
// ACMECertificateHTTP01IngressNameOverride & Class annotations
if err := applyIngressParameterAnnotationOverrides(o, selectedSolver); err != nil {
@ -276,7 +280,6 @@ func challengeSpecForAuthorization(ctx context.Context, cl acmecl.Interface, iss
URL: selectedChallenge.URL,
DNSName: authz.Identifier,
Token: selectedChallenge.Token,
Key: key,
// selectedSolver cannot be nil due to the check above.
Solver: *selectedSolver,
Wildcard: wc,
@ -321,15 +324,26 @@ func applyIngressParameterAnnotationOverrides(o *cmacme.Order, s *cmacme.ACMECha
return nil
}
func keyForChallenge(cl acmecl.Interface, token string, chType cmacme.ACMEChallengeType) (string, error) {
switch chType {
case cmacme.ACMEChallengeTypeHTTP01:
return cl.HTTP01ChallengeResponse(token)
case cmacme.ACMEChallengeTypeDNS01:
return cl.DNS01ChallengeRecord(token)
default:
return "", fmt.Errorf("unsupported challenge type: %v", chType)
func ensureKeysForChallenges(cl acmecl.Interface, challenges []*cmacme.Challenge) ([]*cmacme.Challenge, error) {
for _, ch := range challenges {
var (
key string
err error
)
switch ch.Spec.Type {
case cmacme.ACMEChallengeTypeHTTP01:
key, err = cl.HTTP01ChallengeResponse(ch.Spec.Token)
case cmacme.ACMEChallengeTypeDNS01:
key, err = cl.DNS01ChallengeRecord(ch.Spec.Token)
default:
return nil, fmt.Errorf("challenge %s has unsupported challenge type: %s", ch.Name, ch.Spec.Type)
}
if err != nil {
return nil, err
}
ch.Spec.Key = key
}
return challenges, nil
}
func anyChallengesFailed(chs []*cmacme.Challenge) bool {

259
pkg/controller/acmeorders/util_test.go
View File

@ -18,6 +18,7 @@ package acmeorders
import (
"context"
"fmt"
"reflect"
"testing"
@ -27,7 +28,8 @@ import (
acmecl "github.com/cert-manager/cert-manager/pkg/acme/client"
cmacme "github.com/cert-manager/cert-manager/pkg/apis/acme/v1"
v1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
"github.com/cert-manager/cert-manager/test/unit/gen"
)
func TestChallengeSpecForAuthorization(t *testing.T) {
@ -91,7 +93,7 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
tests := map[string]struct {
acmeClient acmecl.Interface
issuer v1.GenericIssuer
issuer cmapi.GenericIssuer
order *cmacme.Order
authz *cmacme.ACMEAuthorization
@ -100,9 +102,9 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
}{
"should override the ingress name to edit if override annotation is specified": {
acmeClient: basicACMEClient,
issuer: &v1.Issuer{
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
issuer: &cmapi.Issuer{
Spec: cmapi.IssuerSpec{
IssuerConfig: cmapi.IssuerConfig{
ACME: &cmacme.ACMEIssuer{
Solvers: []cmacme.ACMEChallengeSolver{emptySelectorSolverHTTP01},
},
@ -127,7 +129,6 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
Type: cmacme.ACMEChallengeTypeHTTP01,
DNSName: "example.com",
Token: acmeChallengeHTTP01.Token,
Key: "http01",
Solver: cmacme.ACMEChallengeSolver{
HTTP01: &cmacme.ACMEChallengeSolverHTTP01{
Ingress: &cmacme.ACMEChallengeSolverHTTP01Ingress{
@ -139,9 +140,9 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
},
"should override the ingress class to edit if override annotation is specified": {
acmeClient: basicACMEClient,
issuer: &v1.Issuer{
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
issuer: &cmapi.Issuer{
Spec: cmapi.IssuerSpec{
IssuerConfig: cmapi.IssuerConfig{
ACME: &cmacme.ACMEIssuer{
Solvers: []cmacme.ACMEChallengeSolver{emptySelectorSolverHTTP01},
},
@ -166,7 +167,6 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
Type: cmacme.ACMEChallengeTypeHTTP01,
DNSName: "example.com",
Token: acmeChallengeHTTP01.Token,
Key: "http01",
Solver: cmacme.ACMEChallengeSolver{
HTTP01: &cmacme.ACMEChallengeSolverHTTP01{
Ingress: &cmacme.ACMEChallengeSolverHTTP01Ingress{
@ -178,9 +178,9 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
},
"should return an error if both ingress class and name override annotations are set": {
acmeClient: basicACMEClient,
issuer: &v1.Issuer{
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
issuer: &cmapi.Issuer{
Spec: cmapi.IssuerSpec{
IssuerConfig: cmapi.IssuerConfig{
ACME: &cmacme.ACMEIssuer{
Solvers: []cmacme.ACMEChallengeSolver{emptySelectorSolverHTTP01},
},
@ -206,9 +206,9 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
},
"should ignore HTTP01 override annotations if DNS01 solver is chosen": {
acmeClient: basicACMEClient,
issuer: &v1.Issuer{
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
issuer: &cmapi.Issuer{
Spec: cmapi.IssuerSpec{
IssuerConfig: cmapi.IssuerConfig{
ACME: &cmacme.ACMEIssuer{
Solvers: []cmacme.ACMEChallengeSolver{emptySelectorSolverDNS01},
},
@ -233,15 +233,14 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
Type: cmacme.ACMEChallengeTypeDNS01,
DNSName: "example.com",
Token: acmeChallengeDNS01.Token,
Key: "dns01",
Solver: emptySelectorSolverDNS01,
},
},
"should use configured default solver when no others are present": {
acmeClient: basicACMEClient,
issuer: &v1.Issuer{
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
issuer: &cmapi.Issuer{
Spec: cmapi.IssuerSpec{
IssuerConfig: cmapi.IssuerConfig{
ACME: &cmacme.ACMEIssuer{
Solvers: []cmacme.ACMEChallengeSolver{emptySelectorSolverHTTP01},
},
@ -261,15 +260,14 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
Type: cmacme.ACMEChallengeTypeHTTP01,
DNSName: "example.com",
Token: acmeChallengeHTTP01.Token,
Key: "http01",
Solver: emptySelectorSolverHTTP01,
},
},
"should use configured default solver when no others are present but selector is non-nil": {
acmeClient: basicACMEClient,
issuer: &v1.Issuer{
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
issuer: &cmapi.Issuer{
Spec: cmapi.IssuerSpec{
IssuerConfig: cmapi.IssuerConfig{
ACME: &cmacme.ACMEIssuer{
Solvers: []cmacme.ACMEChallengeSolver{
{
@ -298,7 +296,6 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
Type: cmacme.ACMEChallengeTypeHTTP01,
DNSName: "example.com",
Token: acmeChallengeHTTP01.Token,
Key: "http01",
Solver: cmacme.ACMEChallengeSolver{
Selector: &cmacme.CertificateDNSNameSelector{},
HTTP01: &cmacme.ACMEChallengeSolverHTTP01{
@ -311,9 +308,9 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
},
"should use configured default solver when others do not match": {
acmeClient: basicACMEClient,
issuer: &v1.Issuer{
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
issuer: &cmapi.Issuer{
Spec: cmapi.IssuerSpec{
IssuerConfig: cmapi.IssuerConfig{
ACME: &cmacme.ACMEIssuer{
Solvers: []cmacme.ACMEChallengeSolver{
emptySelectorSolverHTTP01,
@ -336,15 +333,14 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
Type: cmacme.ACMEChallengeTypeHTTP01,
DNSName: "example.com",
Token: acmeChallengeHTTP01.Token,
Key: "http01",
Solver: emptySelectorSolverHTTP01,
},
},
"should use DNS01 solver over HTTP01 if challenge is of type DNS01": {
acmeClient: basicACMEClient,
issuer: &v1.Issuer{
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
issuer: &cmapi.Issuer{
Spec: cmapi.IssuerSpec{
IssuerConfig: cmapi.IssuerConfig{
ACME: &cmacme.ACMEIssuer{
Solvers: []cmacme.ACMEChallengeSolver{
emptySelectorSolverHTTP01,
@ -367,15 +363,14 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
Type: cmacme.ACMEChallengeTypeDNS01,
DNSName: "example.com",
Token: acmeChallengeDNS01.Token,
Key: "dns01",
Solver: emptySelectorSolverDNS01,
},
},
"should return an error if none match": {
acmeClient: basicACMEClient,
issuer: &v1.Issuer{
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
issuer: &cmapi.Issuer{
Spec: cmapi.IssuerSpec{
IssuerConfig: cmapi.IssuerConfig{
ACME: &cmacme.ACMEIssuer{
Solvers: []cmacme.ACMEChallengeSolver{
nonMatchingSelectorSolver,
@ -397,9 +392,9 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
},
"uses correct solver when selector explicitly names dnsName": {
acmeClient: basicACMEClient,
issuer: &v1.Issuer{
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
issuer: &cmapi.Issuer{
Spec: cmapi.IssuerSpec{
IssuerConfig: cmapi.IssuerConfig{
ACME: &cmacme.ACMEIssuer{
Solvers: []cmacme.ACMEChallengeSolver{
emptySelectorSolverHTTP01,
@ -422,15 +417,14 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
Type: cmacme.ACMEChallengeTypeHTTP01,
DNSName: "example.com",
Token: acmeChallengeHTTP01.Token,
Key: "http01",
Solver: exampleComDNSNameSelectorSolver,
},
},
"uses default solver if dnsName does not match": {
acmeClient: basicACMEClient,
issuer: &v1.Issuer{
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
issuer: &cmapi.Issuer{
Spec: cmapi.IssuerSpec{
IssuerConfig: cmapi.IssuerConfig{
ACME: &cmacme.ACMEIssuer{
Solvers: []cmacme.ACMEChallengeSolver{
emptySelectorSolverHTTP01,
@ -453,15 +447,14 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
Type: cmacme.ACMEChallengeTypeHTTP01,
DNSName: "notexample.com",
Token: acmeChallengeHTTP01.Token,
Key: "http01",
Solver: emptySelectorSolverHTTP01,
},
},
"if two solvers specify the same dnsName, the one with the most labels should be chosen": {
acmeClient: basicACMEClient,
issuer: &v1.Issuer{
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
issuer: &cmapi.Issuer{
Spec: cmapi.IssuerSpec{
IssuerConfig: cmapi.IssuerConfig{
ACME: &cmacme.ACMEIssuer{
Solvers: []cmacme.ACMEChallengeSolver{
exampleComDNSNameSelectorSolver,
@ -501,7 +494,6 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
Type: cmacme.ACMEChallengeTypeHTTP01,
DNSName: "example.com",
Token: acmeChallengeHTTP01.Token,
Key: "http01",
Solver: cmacme.ACMEChallengeSolver{
Selector: &cmacme.CertificateDNSNameSelector{
MatchLabels: map[string]string{
@ -519,9 +511,9 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
},
"if one solver matches with dnsNames, and the other solver matches with labels, the dnsName solver should be chosen": {
acmeClient: basicACMEClient,
issuer: &v1.Issuer{
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
issuer: &cmapi.Issuer{
Spec: cmapi.IssuerSpec{
IssuerConfig: cmapi.IssuerConfig{
ACME: &cmacme.ACMEIssuer{
Solvers: []cmacme.ACMEChallengeSolver{
exampleComDNSNameSelectorSolver,
@ -560,7 +552,6 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
Type: cmacme.ACMEChallengeTypeHTTP01,
DNSName: "example.com",
Token: acmeChallengeHTTP01.Token,
Key: "http01",
Solver: exampleComDNSNameSelectorSolver,
},
},
@ -568,9 +559,9 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
// order to ensure that this behaviour isn't just incidental
"if one solver matches with dnsNames, and the other solver matches with labels, the dnsName solver should be chosen (solvers listed in reverse order)": {
acmeClient: basicACMEClient,
issuer: &v1.Issuer{
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
issuer: &cmapi.Issuer{
Spec: cmapi.IssuerSpec{
IssuerConfig: cmapi.IssuerConfig{
ACME: &cmacme.ACMEIssuer{
Solvers: []cmacme.ACMEChallengeSolver{
{
@ -609,15 +600,14 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
Type: cmacme.ACMEChallengeTypeHTTP01,
DNSName: "example.com",
Token: acmeChallengeHTTP01.Token,
Key: "http01",
Solver: exampleComDNSNameSelectorSolver,
},
},
"if one solver matches with dnsNames, and the other solver matches with 2 labels, the dnsName solver should be chosen": {
acmeClient: basicACMEClient,
issuer: &v1.Issuer{
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
issuer: &cmapi.Issuer{
Spec: cmapi.IssuerSpec{
IssuerConfig: cmapi.IssuerConfig{
ACME: &cmacme.ACMEIssuer{
Solvers: []cmacme.ACMEChallengeSolver{
exampleComDNSNameSelectorSolver,
@ -658,15 +648,14 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
Type: cmacme.ACMEChallengeTypeHTTP01,
DNSName: "example.com",
Token: acmeChallengeHTTP01.Token,
Key: "http01",
Solver: exampleComDNSNameSelectorSolver,
},
},
"should choose the solver with the most labels matching if multiple match": {
acmeClient: basicACMEClient,
issuer: &v1.Issuer{
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
issuer: &cmapi.Issuer{
Spec: cmapi.IssuerSpec{
IssuerConfig: cmapi.IssuerConfig{
ACME: &cmacme.ACMEIssuer{
Solvers: []cmacme.ACMEChallengeSolver{
{
@ -718,7 +707,6 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
Type: cmacme.ACMEChallengeTypeHTTP01,
DNSName: "example.com",
Token: acmeChallengeHTTP01.Token,
Key: "http01",
Solver: cmacme.ACMEChallengeSolver{
Selector: &cmacme.CertificateDNSNameSelector{
MatchLabels: map[string]string{
@ -736,9 +724,9 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
},
"should match wildcard dnsName solver if authorization has Wildcard=true": {
acmeClient: basicACMEClient,
issuer: &v1.Issuer{
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
issuer: &cmapi.Issuer{
Spec: cmapi.IssuerSpec{
IssuerConfig: cmapi.IssuerConfig{
ACME: &cmacme.ACMEIssuer{
Solvers: []cmacme.ACMEChallengeSolver{
emptySelectorSolverDNS01,
@ -772,7 +760,6 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
DNSName: "example.com",
Wildcard: true,
Token: acmeChallengeDNS01.Token,
Key: "dns01",
Solver: cmacme.ACMEChallengeSolver{
Selector: &cmacme.CertificateDNSNameSelector{
DNSNames: []string{"*.example.com"},
@ -787,9 +774,9 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
},
"dnsName selectors should take precedence over dnsZone selectors": {
acmeClient: basicACMEClient,
issuer: &v1.Issuer{
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
issuer: &cmapi.Issuer{
Spec: cmapi.IssuerSpec{
IssuerConfig: cmapi.IssuerConfig{
ACME: &cmacme.ACMEIssuer{
Solvers: []cmacme.ACMEChallengeSolver{
exampleComDNSNameSelectorSolver,
@ -821,15 +808,14 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
Type: cmacme.ACMEChallengeTypeHTTP01,
DNSName: "example.com",
Token: acmeChallengeHTTP01.Token,
Key: "http01",
Solver: exampleComDNSNameSelectorSolver,
},
},
"dnsName selectors should take precedence over dnsZone selectors (reversed order)": {
acmeClient: basicACMEClient,
issuer: &v1.Issuer{
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
issuer: &cmapi.Issuer{
Spec: cmapi.IssuerSpec{
IssuerConfig: cmapi.IssuerConfig{
ACME: &cmacme.ACMEIssuer{
Solvers: []cmacme.ACMEChallengeSolver{
{
@ -861,15 +847,14 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
Type: cmacme.ACMEChallengeTypeHTTP01,
DNSName: "example.com",
Token: acmeChallengeHTTP01.Token,
Key: "http01",
Solver: exampleComDNSNameSelectorSolver,
},
},
"should allow matching with dnsZones": {
acmeClient: basicACMEClient,
issuer: &v1.Issuer{
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
issuer: &cmapi.Issuer{
Spec: cmapi.IssuerSpec{
IssuerConfig: cmapi.IssuerConfig{
ACME: &cmacme.ACMEIssuer{
Solvers: []cmacme.ACMEChallengeSolver{
emptySelectorSolverDNS01,
@ -903,7 +888,6 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
DNSName: "www.example.com",
Wildcard: true,
Token: acmeChallengeDNS01.Token,
Key: "dns01",
Solver: cmacme.ACMEChallengeSolver{
Selector: &cmacme.CertificateDNSNameSelector{
DNSZones: []string{"example.com"},
@ -918,9 +902,9 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
},
"most specific dnsZone should be selected if multiple match": {
acmeClient: basicACMEClient,
issuer: &v1.Issuer{
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
issuer: &cmapi.Issuer{
Spec: cmapi.IssuerSpec{
IssuerConfig: cmapi.IssuerConfig{
ACME: &cmacme.ACMEIssuer{
Solvers: []cmacme.ACMEChallengeSolver{
{
@ -963,7 +947,6 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
DNSName: "www.prod.example.com",
Wildcard: true,
Token: acmeChallengeDNS01.Token,
Key: "dns01",
Solver: cmacme.ACMEChallengeSolver{
Selector: &cmacme.CertificateDNSNameSelector{
DNSZones: []string{"prod.example.com"},
@ -978,9 +961,9 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
},
"most specific dnsZone should be selected if multiple match (reversed)": {
acmeClient: basicACMEClient,
issuer: &v1.Issuer{
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
issuer: &cmapi.Issuer{
Spec: cmapi.IssuerSpec{
IssuerConfig: cmapi.IssuerConfig{
ACME: &cmacme.ACMEIssuer{
Solvers: []cmacme.ACMEChallengeSolver{
{
@ -1023,7 +1006,6 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
DNSName: "www.prod.example.com",
Wildcard: true,
Token: acmeChallengeDNS01.Token,
Key: "dns01",
Solver: cmacme.ACMEChallengeSolver{
Selector: &cmacme.CertificateDNSNameSelector{
DNSZones: []string{"prod.example.com"},
@ -1038,9 +1020,9 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
},
"if two solvers specify the same dnsZone, the one with the most labels should be chosen": {
acmeClient: basicACMEClient,
issuer: &v1.Issuer{
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
issuer: &cmapi.Issuer{
Spec: cmapi.IssuerSpec{
IssuerConfig: cmapi.IssuerConfig{
ACME: &cmacme.ACMEIssuer{
Solvers: []cmacme.ACMEChallengeSolver{
{
@ -1089,7 +1071,6 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
Type: cmacme.ACMEChallengeTypeHTTP01,
DNSName: "www.example.com",
Token: acmeChallengeHTTP01.Token,
Key: "http01",
Solver: cmacme.ACMEChallengeSolver{
Selector: &cmacme.CertificateDNSNameSelector{
MatchLabels: map[string]string{
@ -1107,9 +1088,9 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
},
"if both solvers match dnsNames, and one also matches dnsZones, choose the one that matches dnsZones": {
acmeClient: basicACMEClient,
issuer: &v1.Issuer{
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
issuer: &cmapi.Issuer{
Spec: cmapi.IssuerSpec{
IssuerConfig: cmapi.IssuerConfig{
ACME: &cmacme.ACMEIssuer{
Solvers: []cmacme.ACMEChallengeSolver{
{
@ -1151,7 +1132,6 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
Type: cmacme.ACMEChallengeTypeHTTP01,
DNSName: "www.example.com",
Token: acmeChallengeHTTP01.Token,
Key: "http01",
Solver: cmacme.ACMEChallengeSolver{
Selector: &cmacme.CertificateDNSNameSelector{
DNSZones: []string{"example.com"},
@ -1167,9 +1147,9 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
},
"if both solvers match dnsNames, and one also matches dnsZones, choose the one that matches dnsZones (reversed)": {
acmeClient: basicACMEClient,
issuer: &v1.Issuer{
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
issuer: &cmapi.Issuer{
Spec: cmapi.IssuerSpec{
IssuerConfig: cmapi.IssuerConfig{
ACME: &cmacme.ACMEIssuer{
Solvers: []cmacme.ACMEChallengeSolver{
{
@ -1211,7 +1191,6 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
Type: cmacme.ACMEChallengeTypeHTTP01,
DNSName: "www.example.com",
Token: acmeChallengeHTTP01.Token,
Key: "http01",
Solver: cmacme.ACMEChallengeSolver{
Selector: &cmacme.CertificateDNSNameSelector{
DNSZones: []string{"example.com"},
@ -1227,9 +1206,9 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
},
"uses correct solver when selector explicitly names dnsName (reversed)": {
acmeClient: basicACMEClient,
issuer: &v1.Issuer{
Spec: v1.IssuerSpec{
IssuerConfig: v1.IssuerConfig{
issuer: &cmapi.Issuer{
Spec: cmapi.IssuerSpec{
IssuerConfig: cmapi.IssuerConfig{
ACME: &cmacme.ACMEIssuer{
Solvers: []cmacme.ACMEChallengeSolver{
exampleComDNSNameSelectorSolver,
@ -1252,7 +1231,6 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
Type: cmacme.ACMEChallengeTypeHTTP01,
DNSName: "example.com",
Token: acmeChallengeHTTP01.Token,
Key: "http01",
Solver: exampleComDNSNameSelectorSolver,
},
},
@ -1260,7 +1238,7 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
for name, test := range tests {
t.Run(name, func(t *testing.T) {
ctx := context.Background()
cs, err := challengeSpecForAuthorization(ctx, test.acmeClient, test.issuer, test.order, *test.authz)
cs, err := partialChallengeSpecForAuthorization(ctx, test.issuer, test.order, *test.authz)
if err != nil && !test.expectedError {
t.Errorf("expected to not get an error, but got: %v", err)
t.Fail()
@ -1274,3 +1252,74 @@ func TestChallengeSpecForAuthorization(t *testing.T) {
})
}
}
func Test_ensureKeysForChallenges(t *testing.T) {
basicACMEClient := &acmecl.FakeACME{
FakeHTTP01ChallengeResponse: func(token string) (string, error) {
switch token {
case "fooToken":
return "fooKeyHTTP01", nil
case "barToken":
return "barKeyHTTP01", nil
}
return "", fmt.Errorf("internal error: unexpected token value %s", token)
},
FakeDNS01ChallengeRecord: func(token string) (string, error) {
switch token {
case "fooToken":
return "fooKeyDNS01", nil
case "barToken":
return "barKeyDNS01", nil
}
return "", fmt.Errorf("internal error: unexpected token value %s", token)
},
}
fooChallenge := gen.Challenge("foo", gen.SetChallengeToken("fooToken"))
barChallenge := gen.Challenge("bar", gen.SetChallengeToken("barToken"))
tests := map[string]struct {
acmeClient acmecl.Interface
partialChallenges []*cmacme.Challenge
want []*cmacme.Challenge
wantErr bool
}{
"happy path with some http-01 challenges": {
acmeClient: basicACMEClient,
partialChallenges: []*cmacme.Challenge{
gen.ChallengeFrom(fooChallenge, gen.SetChallengeType(cmacme.ACMEChallengeTypeHTTP01)),
gen.ChallengeFrom(barChallenge, gen.SetChallengeType(cmacme.ACMEChallengeTypeHTTP01))},
want: []*cmacme.Challenge{
gen.ChallengeFrom(fooChallenge, gen.SetChallengeType(cmacme.ACMEChallengeTypeHTTP01),
gen.SetChallengeKey("fooKeyHTTP01")),
gen.ChallengeFrom(barChallenge, gen.SetChallengeType(cmacme.ACMEChallengeTypeHTTP01),
gen.SetChallengeKey("barKeyHTTP01"))},
},
"happy path with some dns-01 challenges": {
acmeClient: basicACMEClient,
partialChallenges: []*cmacme.Challenge{
gen.ChallengeFrom(fooChallenge, gen.SetChallengeType(cmacme.ACMEChallengeTypeDNS01)),
gen.ChallengeFrom(barChallenge, gen.SetChallengeType(cmacme.ACMEChallengeTypeDNS01))},
want: []*cmacme.Challenge{
gen.ChallengeFrom(fooChallenge, gen.SetChallengeType(cmacme.ACMEChallengeTypeDNS01),
gen.SetChallengeKey("fooKeyDNS01")),
gen.ChallengeFrom(barChallenge, gen.SetChallengeType(cmacme.ACMEChallengeTypeDNS01),
gen.SetChallengeKey("barKeyDNS01"))},
},
"unhappy path with an unknown challenge type": {
acmeClient: basicACMEClient,
partialChallenges: []*cmacme.Challenge{gen.ChallengeFrom(fooChallenge, gen.SetChallengeType(cmacme.ACMEChallengeType("foo")))},
wantErr: true,
},
}
for name, scenario := range tests {
t.Run(name, func(t *testing.T) {
got, err := ensureKeysForChallenges(scenario.acmeClient, scenario.partialChallenges)
if (err != nil) != scenario.wantErr {
t.Errorf("ensureKeysForChallenges() error = %v, wantErr %v", err, scenario.wantErr)
return
}
if !reflect.DeepEqual(got, scenario.want) {
t.Errorf("ensureKeysForChallenges() = %v, want %v", got, scenario.want)
}
})
}
}

6
test/unit/gen/challenge.go
View File

@ -60,6 +60,12 @@ func SetChallengeToken(t string) ChallengeModifier {
}
}
func SetChallengeKey(k string) ChallengeModifier {
return func(ch *cmacme.Challenge) {
ch.Spec.Key = k
}
}
// SetIssuer sets the challenge.spec.issuerRef field
func SetChallengeIssuer(o cmmeta.ObjectReference) ChallengeModifier {
return func(c *cmacme.Challenge) {