Integration test for ACME Orders controller

Signed-off-by: irbekrm <irbekrm@gmail.com>

BUILD.bazel

@@ -93,7 +93,6 @@ filegroup(
         "//test/unit/coreclients:all-srcs",
         "//test/unit/gen:all-srcs",
         "//test/unit/listers:all-srcs",
-        "//test/utils:all-srcs",
         "//tools/cobra:all-srcs",
     ],
     tags = ["automanaged"],

pkg/controller/acmeorders/sync.go

@@ -32,6 +32,7 @@ import (
 	"k8s.io/apimachinery/pkg/labels"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/client-go/tools/cache"
 
 	"github.com/jetstack/cert-manager/pkg/acme"
 	acmecl "github.com/jetstack/cert-manager/pkg/acme/client"
@@ -186,7 +187,13 @@ func (c *controller) Sync(ctx context.Context, o *cmacme.Order) (err error) {
 		// should already be Pending, but set it anyway to be explicit.
 		c.setOrderState(&o.Status, string(cmacme.Pending))
 		// Re-queue the Order to be processed after RequeuePeriod
-		c.scheduledWorkQueue.Add(o, RequeuePeriod)
+		key, err := cache.MetaNamespaceKeyFunc(o)
+		if err != nil {
+			// if we return an error here, at least the Order will get re-queued.
+			log.V(logf.InfoLevel).Info("Internal error: failed to construct key for pending Order")
+			return err
+		}
+		c.scheduledWorkQueue.Add(key, time.Second*5)
 		return nil
 
 	case !anyChallengesFailed(challenges) && allChallengesFinal(challenges):

test/integration/BUILD.bazel

@@ -9,7 +9,7 @@ filegroup(
     name = "all-srcs",
     srcs = [
         ":package-srcs",
-        "//test/integration/acmeorders:all-srcs",
+        "//test/integration/acme:all-srcs",
         "//test/integration/certificates:all-srcs",
         "//test/integration/conversion:all-srcs",
         "//test/integration/ctl:all-srcs",

test/integration/acme/BUILD.bazel

@@ -0,0 +1,37 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_test")
+
+go_test(
+    name = "go_default_test",
+    srcs = ["orders_controller_test.go"],
+    deps = [
+        "//pkg/acme/accounts/test:go_default_library",
+        "//pkg/acme/client:go_default_library",
+        "//pkg/apis/acme/v1:go_default_library",
+        "//pkg/apis/meta/v1:go_default_library",
+        "//pkg/controller:go_default_library",
+        "//pkg/controller/acmeorders:go_default_library",
+        "//pkg/logs:go_default_library",
+        "//pkg/metrics:go_default_library",
+        "//test/integration/framework:go_default_library",
+        "//test/unit/gen:go_default_library",
+        "@io_k8s_api//core/v1:go_default_library",
+        "@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library",
+        "@io_k8s_apimachinery//pkg/util/wait:go_default_library",
+        "@io_k8s_utils//clock:go_default_library",
+        "@org_golang_x_crypto//acme:go_default_library",
+    ],
+)
+
+filegroup(
+    name = "package-srcs",
+    srcs = glob(["**"]),
+    tags = ["automanaged"],
+    visibility = ["//visibility:private"],
+)
+
+filegroup(
+    name = "all-srcs",
+    srcs = [":package-srcs"],
+    tags = ["automanaged"],
+    visibility = ["//visibility:public"],
+)

test/integration/acme/orders_controller_test.go

@@ -0,0 +1,280 @@
+/*
+Copyright 2020 The cert-manager Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package acme
+
+import (
+	"context"
+	"fmt"
+	"testing"
+	"time"
+
+	acmeapi "golang.org/x/crypto/acme"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/wait"
+	"k8s.io/utils/clock"
+
+	accountstest "github.com/jetstack/cert-manager/pkg/acme/accounts/test"
+	acmecl "github.com/jetstack/cert-manager/pkg/acme/client"
+	cmacme "github.com/jetstack/cert-manager/pkg/apis/acme/v1"
+	cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1"
+	controllerpkg "github.com/jetstack/cert-manager/pkg/controller"
+	"github.com/jetstack/cert-manager/pkg/controller/acmeorders"
+	logf "github.com/jetstack/cert-manager/pkg/logs"
+	"github.com/jetstack/cert-manager/pkg/metrics"
+	"github.com/jetstack/cert-manager/test/integration/framework"
+	"github.com/jetstack/cert-manager/test/unit/gen"
+)
+
+func TestAcmeOrdersController(t *testing.T) {
+
+	config, stopFn := framework.RunControlPlane(t)
+	defer stopFn()
+
+	// Create clients and informer factories for Kubernetes API and
+	// cert-manager.
+	kubeClient, factory, cmCl, cmFactory := framework.NewClients(t, config)
+
+	// some test values
+	var (
+		testName      string = "acmetest"
+		challengeType string = "dns-01"
+		authType      string = "dns"
+	)
+
+	// Initial ACME authorization to be returned by GetAuthorization.
+	auth := &acmeapi.Authorization{
+		URI:    testName,
+		Status: acmeapi.StatusPending,
+		Challenges: []*acmeapi.Challenge{
+			{
+				Type:  challengeType,
+				URI:   testName,
+				Token: testName,
+			},
+		},
+		Identifier: acmeapi.AuthzID{
+			Type:  authType,
+			Value: testName,
+		},
+	}
+	// ACME order to be returned by calls to the fake registry.
+	acmeOrder := &acmeapi.Order{
+		URI: testName,
+		Identifiers: []acmeapi.AuthzID{
+			{
+				Type:  authType,
+				Value: testName,
+			},
+		},
+		FinalizeURL: testName,
+		AuthzURLs:   []string{testName},
+		Status:      acmeapi.StatusPending,
+	}
+	// ACME client with stubbed methods to simulate a specific response from the
+	// ACME server.
+	acmeClient := &acmecl.FakeACME{
+		FakeAuthorizeOrder: func(_ context.Context, _ []acmeapi.AuthzID, _ ...acmeapi.OrderOption) (*acmeapi.Order, error) {
+			return acmeOrder, nil
+		},
+		FakeGetAuthorization: func(_ context.Context, _ string) (*acmeapi.Authorization, error) {
+			return auth, nil
+		},
+		FakeGetOrder: func(_ context.Context, _ string) (*acmeapi.Order, error) {
+			return acmeOrder, nil
+		},
+		FakeDNS01ChallengeRecord: func(_ string) (string, error) {
+			return testName, nil
+		},
+		FakeCreateOrderCert: func(_ context.Context, _ string, _ []byte, _ bool) ([][]byte, string, error) {
+			// A hack to ensure the status of the _ACME_ order gets set to valid
+			// when we're finalizing the order.
+			acmeOrder.Status = acmeapi.StatusValid
+			return [][]byte{}, "", nil
+		},
+	}
+
+	// Create a fake ACME registry with a GetClientFunc that returns the
+	// acmeClient with the stubbed methods.
+	accountRegistry := &accountstest.FakeRegistry{
+		GetClientFunc: func(_ string) (acmecl.Interface, error) {
+			return acmeClient, nil
+		},
+	}
+
+	// Create a new orders controller.
+	ctrl, queue, mustSync := acmeorders.NewController(
+		logf.Log,
+		cmCl,
+		factory,
+		cmFactory,
+		accountRegistry,
+		framework.NewEventRecorder(t),
+		clock.RealClock{},
+		false,
+	)
+	c := controllerpkg.NewController(
+		context.Background(),
+		"orders_test",
+		metrics.New(logf.Log),
+		ctrl.ProcessItem,
+		mustSync,
+		nil,
+		queue,
+	)
+
+	// Ensure the controller is started now and stopped after the tests.
+	stopController := framework.StartInformersAndController(
+		t,
+		factory,
+		cmFactory,
+		c,
+	)
+	defer stopController()
+
+	ctx, cancel := context.WithTimeout(context.TODO(), time.Second*20)
+	defer cancel()
+
+	// Create a Namespace.
+	ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: testName}}
+	_, err := kubeClient.CoreV1().Namespaces().Create(ctx, ns, metav1.CreateOptions{})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Create an Issuer for Order.
+	acmeIssuer := cmacme.ACMEIssuer{
+		PrivateKey: cmmeta.SecretKeySelector{Key: testName},
+		Server:     testName,
+		Solvers: []cmacme.ACMEChallengeSolver{
+			{
+				DNS01: &cmacme.ACMEChallengeSolverDNS01{
+					Cloudflare: &cmacme.ACMEIssuerDNS01ProviderCloudflare{
+						Email:    testName,
+						APIToken: &cmmeta.SecretKeySelector{Key: testName},
+					},
+				},
+			},
+		},
+	}
+	acmeIssuer.PrivateKey.Name = testName
+	acmeIssuer.Solvers[0].DNS01.Cloudflare.APIToken.Name = testName
+	iss := gen.Issuer(testName,
+		gen.SetIssuerNamespace(testName),
+		gen.SetIssuerACME(acmeIssuer))
+
+	iss, err = cmCl.CertmanagerV1().Issuers(testName).Create(ctx, iss, metav1.CreateOptions{})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// 1. Test that a Challenge is created for a new Order.
+
+	// Create an Order CR.
+	order := gen.Order(testName,
+		gen.SetOrderIssuer(cmmeta.ObjectReference{
+			Name: testName,
+		}),
+		gen.SetOrderNamespace(testName),
+		gen.SetOrderCsr([]byte(testName)),
+		gen.SetOrderDNSNames(testName))
+
+	order, err = cmCl.AcmeV1().Orders(testName).Create(ctx, order, metav1.CreateOptions{})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Wait for the Challenge to be created.
+	var chal *cmacme.Challenge
+	err = wait.Poll(time.Millisecond*100, time.Minute, func() (done bool, err error) {
+		chals, err := cmCl.AcmeV1().Challenges(testName).List(ctx, metav1.ListOptions{})
+		l := len(chals.Items)
+		// Challenge has not been created yet
+		if l == 0 {
+			return false, nil
+		}
+		// this should never happen
+		if l > 1 {
+			return false, fmt.Errorf("expected maximum 1 challenge, got %d", l)
+		}
+		// Check that the Challenge is owned by our Order.
+		chal = &chals.Items[0]
+		if !metav1.IsControlledBy(chal, order) {
+			return false, fmt.Errorf("found an unexpected Challenge resource: %v", chal.Name)
+		}
+		return true, nil
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// 2. Test that in an edge case, where an ACME server is misbehaving and
+	// despite Challenges being valid, the ACME order status is 'pending', we
+	// re-queue the Order so that when the ACME order does become 'ready', we
+	// finalize our Order and, in the success scenario, it eventually becomes
+	// valid.
+	// https://github.com/jetstack/cert-manager/issues/2868
+
+	// Set the Challenge state to valid- the status of the ACME order remains 'pending'.
+	chal = chal.DeepCopy()
+	chal.Status.State = cmacme.Valid
+	_, err = cmCl.AcmeV1().Challenges(testName).UpdateStatus(ctx, chal, metav1.UpdateOptions{})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Sit here for a little bit checking that the Order status remains pending
+	// and also to verify that this test works.
+	var pendingOrder *cmacme.Order
+	err = wait.Poll(time.Millisecond*200, time.Second*5, func() (bool, error) {
+		pendingOrder, err = cmCl.AcmeV1().Orders(testName).Get(ctx, testName, metav1.GetOptions{})
+		if err != nil {
+			return false, err
+		}
+		if pendingOrder.Status.State != cmacme.Pending {
+			return true, nil
+		}
+		return false, nil
+	})
+	switch {
+	case err == nil:
+		t.Fatalf("Expected Order to have pending status instead got: %v", pendingOrder.Status.State)
+	case err == wait.ErrWaitTimeout:
+		// 'happy case' - Order remained pending
+	default:
+		t.Fatal(err)
+	}
+
+	// Set status of the ACME order to 'ready.
+	acmeOrder.Status = acmeapi.StatusReady
+
+	// Wait for the status of the Order to become Valid.
+	err = wait.Poll(time.Millisecond*100, time.Second*20, func() (bool, error) {
+		o, err := cmCl.AcmeV1().Orders(testName).Get(ctx, testName, metav1.GetOptions{})
+		if err != nil {
+			return false, err
+		}
+		// not valid yet
+		if o.Status.State != cmacme.Valid {
+			return false, nil
+		}
+		return true, nil
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+}