diff --git a/BUILD.bazel b/BUILD.bazel index 67359892f..d98ea39f0 100644 --- a/BUILD.bazel +++ b/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"], diff --git a/pkg/controller/acmeorders/sync.go b/pkg/controller/acmeorders/sync.go index 558546b92..16c06b9d8 100644 --- a/pkg/controller/acmeorders/sync.go +++ b/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): diff --git a/test/integration/BUILD.bazel b/test/integration/BUILD.bazel index 3bfae13f4..50ce22018 100644 --- a/test/integration/BUILD.bazel +++ b/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", diff --git a/test/integration/acme/BUILD.bazel b/test/integration/acme/BUILD.bazel new file mode 100644 index 000000000..21c3b0c2a --- /dev/null +++ b/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"], +) diff --git a/test/integration/acme/orders_controller_test.go b/test/integration/acme/orders_controller_test.go new file mode 100644 index 000000000..6e706c2b5 --- /dev/null +++ b/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) + } +}