Merge pull request #3788 from maelvls/refactor-trigger-unit-tests
Refactor trigger-controller unit tests
This commit is contained in:
commit
7946df1da7
@ -55,14 +55,12 @@ go_test(
|
||||
embed = [":go_default_library"],
|
||||
deps = [
|
||||
"//pkg/apis/certmanager/v1:go_default_library",
|
||||
"//pkg/apis/meta/v1:go_default_library",
|
||||
"//pkg/controller:go_default_library",
|
||||
"//pkg/controller/certificates/trigger/policies:go_default_library",
|
||||
"//pkg/controller/test:go_default_library",
|
||||
"//test/unit/gen:go_default_library",
|
||||
"@com_github_go_logr_logr//testing:go_default_library",
|
||||
"@com_github_stretchr_testify//assert:go_default_library",
|
||||
"@io_k8s_api//core/v1:go_default_library",
|
||||
"@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library",
|
||||
"@io_k8s_client_go//testing:go_default_library",
|
||||
"@io_k8s_utils//clock/testing:go_default_library",
|
||||
|
||||
@ -62,16 +62,17 @@ const (
|
||||
// It triggers re-issuance by adding the `Issuing` status condition when a new
|
||||
// certificate is required.
|
||||
type controller struct {
|
||||
// the trigger policies to run - named here to make testing simpler
|
||||
policyChain policies.Chain
|
||||
certificateLister cmlisters.CertificateLister
|
||||
certificateRequestLister cmlisters.CertificateRequestLister
|
||||
secretLister corelisters.SecretLister
|
||||
client cmclient.Interface
|
||||
recorder record.EventRecorder
|
||||
clock clock.Clock
|
||||
scheduledWorkQueue scheduler.ScheduledWorkQueue
|
||||
gatherer *policies.Gatherer
|
||||
|
||||
// The following are used for testing purposes.
|
||||
clock clock.Clock
|
||||
shouldReissue policies.Func
|
||||
dataForCertificate func(context.Context, *cmapi.Certificate) (policies.Input, error)
|
||||
}
|
||||
|
||||
func NewController(
|
||||
@ -81,7 +82,7 @@ func NewController(
|
||||
cmFactory cminformers.SharedInformerFactory,
|
||||
recorder record.EventRecorder,
|
||||
clock clock.Clock,
|
||||
chain policies.Chain,
|
||||
shouldReissue policies.Func,
|
||||
) (*controller, workqueue.RateLimitingInterface, []cache.InformerSynced) {
|
||||
// create a queue used to queue up items to be processed
|
||||
queue := workqueue.NewNamedRateLimitingQueue(workqueue.NewItemExponentialFailureRateLimiter(time.Second*1, time.Second*30), ControllerName)
|
||||
@ -113,18 +114,20 @@ func NewController(
|
||||
}
|
||||
|
||||
return &controller{
|
||||
policyChain: chain,
|
||||
certificateLister: certificateInformer.Lister(),
|
||||
certificateRequestLister: certificateRequestInformer.Lister(),
|
||||
secretLister: secretsInformer.Lister(),
|
||||
client: client,
|
||||
recorder: recorder,
|
||||
clock: clock,
|
||||
scheduledWorkQueue: scheduler.NewScheduledWorkQueue(clock, queue.Add),
|
||||
gatherer: &policies.Gatherer{
|
||||
|
||||
// The following are used for testing purposes.
|
||||
clock: clock,
|
||||
shouldReissue: shouldReissue,
|
||||
dataForCertificate: (&policies.Gatherer{
|
||||
CertificateRequestLister: certificateRequestInformer.Lister(),
|
||||
SecretLister: secretsInformer.Lister(),
|
||||
},
|
||||
}).DataForCertificate,
|
||||
}, queue, mustSync
|
||||
}
|
||||
|
||||
@ -153,7 +156,7 @@ func (c *controller) ProcessItem(ctx context.Context, key string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
input, err := c.gatherer.DataForCertificate(ctx, crt)
|
||||
input, err := c.dataForCertificate(ctx, crt)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
@ -173,12 +176,18 @@ func (c *controller) ProcessItem(ctx context.Context, key string) error {
|
||||
c.scheduleRecheckOfCertificateIfRequired(log, key, crt.Status.RenewalTime.Time.Sub(c.clock.Now()))
|
||||
}
|
||||
|
||||
reason, message, reissue := c.policyChain.Evaluate(input)
|
||||
reason, message, reissue := c.shouldReissue(input)
|
||||
if !reissue {
|
||||
// no re-issuance required, return early
|
||||
return nil
|
||||
}
|
||||
|
||||
// Although the below recorder.Event already logs the event, the log
|
||||
// line is quite unreadable (very long). Since this information is very
|
||||
// important for the user and the operator, we log the following
|
||||
// message.
|
||||
log.V(logf.InfoLevel).Info("Certificate must be re-issued", "reason", reason, "message", message)
|
||||
|
||||
crt = crt.DeepCopy()
|
||||
apiutil.SetCertificateCondition(crt, crt.Generation, cmapi.CertificateConditionIssuing, cmmeta.ConditionTrue, reason, message)
|
||||
_, err = c.client.CertmanagerV1().Certificates(crt.Namespace).UpdateStatus(ctx, crt, metav1.UpdateOptions{})
|
||||
@ -244,7 +253,7 @@ func (c *controllerWrapper) Register(ctx *controllerpkg.Context) (workqueue.Rate
|
||||
ctx.SharedInformerFactory,
|
||||
ctx.Recorder,
|
||||
ctx.Clock,
|
||||
policies.NewTriggerPolicyChain(ctx.Clock, cmapi.DefaultRenewBefore),
|
||||
policies.NewTriggerPolicyChain(ctx.Clock, cmapi.DefaultRenewBefore).Evaluate,
|
||||
)
|
||||
c.controller = ctrl
|
||||
|
||||
|
||||
@ -18,77 +18,57 @@ package trigger
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
corev1 "k8s.io/api/core/v1"
|
||||
logtest "github.com/go-logr/logr/testing"
|
||||
"github.com/stretchr/testify/assert"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
coretesting "k8s.io/client-go/testing"
|
||||
fakeclock "k8s.io/utils/clock/testing"
|
||||
|
||||
logtest "github.com/go-logr/logr/testing"
|
||||
cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/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/certificates/trigger/policies"
|
||||
testpkg "github.com/jetstack/cert-manager/pkg/controller/test"
|
||||
"github.com/jetstack/cert-manager/test/unit/gen"
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
// policyFuncBuilder wraps a policies.Func to allow injecting a testing.T
|
||||
type policyFuncBuilder func(t *testing.T) policies.Func
|
||||
|
||||
func Test_controller_ProcessItem(t *testing.T) {
|
||||
// now time is the current time at the start of the test (the clock is fixed)
|
||||
now := time.Now()
|
||||
metaNow := metav1.NewTime(now)
|
||||
forceTriggeredReason := "ForceTriggered"
|
||||
forceTriggeredMessage := "Re-issuance forced by unit test case"
|
||||
fixedNow := metav1.NewTime(time.Now())
|
||||
|
||||
tests := map[string]struct {
|
||||
// key that should be passed to ProcessItem.
|
||||
// if not set, the 'namespace/name' of the 'Certificate' field will be used.
|
||||
// if neither is set, the key will be ""
|
||||
// key that should be passed to ProcessItem. If not set, the
|
||||
// 'namespace/name' of the 'Certificate' field will be used. If neither
|
||||
// is set, the key will be "".
|
||||
key string
|
||||
|
||||
// Certificate to be synced for the test.
|
||||
// if not set, the 'key' will be passed to ProcessItem instead.
|
||||
certificate *cmapi.Certificate
|
||||
// Certificate to be synced for the test. If not set, the 'key' will be
|
||||
// passed to ProcessItem instead.
|
||||
existingCertificate *cmapi.Certificate
|
||||
|
||||
// Secret, if set, will exist in the apiserver before the test is run.
|
||||
secret *corev1.Secret
|
||||
mockDataForCertificateReturn policies.Input
|
||||
mockDataForCertificateReturnErr error
|
||||
wantDataForCertificateCalled bool
|
||||
|
||||
// Request, if set, will exist in the apiserver before the test is run.
|
||||
requests []*cmapi.CertificateRequest
|
||||
mockShouldReissue func(t *testing.T) policies.Func
|
||||
wantShouldReissueCalled bool
|
||||
|
||||
// optional chain of policy functions that should be run, wrapped with
|
||||
// the policyFuncBuilder to allow injecting the sub-test's testing.T.
|
||||
policyFuncs []policyFuncBuilder
|
||||
|
||||
// chainShouldEvaluate will cause the test to error if the policy chain
|
||||
// was not attempted to be evaluated
|
||||
chainShouldEvaluate bool
|
||||
// chainShouldTriggerIssuance will cause the policy chain used in the
|
||||
// test to trigger issuance.
|
||||
// This policyFunc will be injected at the end of the policy chain.
|
||||
// If false, the policyFunc that forces an issuance will not be injected
|
||||
// but user-provided policyFuncs will still behave as usual.
|
||||
chainShouldTriggerIssuance bool
|
||||
|
||||
// expectedEvent, if set, is an 'event string' that is expected to be fired.
|
||||
// wantEvent, if set, is an 'event string' that is expected to be fired.
|
||||
// For example, "Normal Issuing Re-issuance forced by unit test case"
|
||||
// where 'Normal' is the event severity, 'Issuing' is the reason and the
|
||||
// remainder is the message.
|
||||
expectedEvent string
|
||||
wantEvent string
|
||||
|
||||
// expectedConditions is the expected set of conditions on the Certificate
|
||||
// wantConditions is the expected set of conditions on the Certificate
|
||||
// resource if an Update is made.
|
||||
// If nil, no update is expected.
|
||||
// If empty, an update to the empty set/nil is expected.
|
||||
expectedConditions []cmapi.CertificateCondition
|
||||
wantConditions []cmapi.CertificateCondition
|
||||
|
||||
// err is the expected error text returned by the controller, if any.
|
||||
err string
|
||||
// wantErr is the expected error text returned by the controller, if any.
|
||||
wantErr string
|
||||
}{
|
||||
"do nothing if an empty 'key' is used": {},
|
||||
"do nothing if an invalid 'key' is used": {
|
||||
@ -97,412 +77,266 @@ func Test_controller_ProcessItem(t *testing.T) {
|
||||
"do nothing if a key references a Certificate that does not exist": {
|
||||
key: "namespace/name",
|
||||
},
|
||||
"do nothing if Certificate already has 'Issuing' condition": {
|
||||
certificate: &cmapi.Certificate{
|
||||
ObjectMeta: metav1.ObjectMeta{Namespace: "testns", Name: "test", Generation: 3},
|
||||
Status: cmapi.CertificateStatus{
|
||||
Conditions: []cmapi.CertificateCondition{
|
||||
{
|
||||
Type: cmapi.CertificateConditionIssuing,
|
||||
Status: cmmeta.ConditionTrue,
|
||||
ObservedGeneration: 3,
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
"evaluate policy chain with only the Certificate if no Request or Secret exists": {
|
||||
certificate: &cmapi.Certificate{
|
||||
ObjectMeta: metav1.ObjectMeta{Namespace: "testns", Name: "test", Generation: 3},
|
||||
},
|
||||
chainShouldEvaluate: true,
|
||||
policyFuncs: []policyFuncBuilder{
|
||||
// Add a policy function that ensures only the input's 'certificate'
|
||||
// field is set.
|
||||
func(t *testing.T) policies.Func {
|
||||
return func(input policies.Input) (string, string, bool) {
|
||||
if input.Certificate == nil {
|
||||
t.Error("expected policy data 'Certificate' field to be set but it was not")
|
||||
}
|
||||
if input.Secret != nil {
|
||||
t.Errorf("expected policy data 'Secret' field to be unset but it was: %+v", input.Secret)
|
||||
}
|
||||
if input.CurrentRevisionRequest != nil {
|
||||
t.Errorf("expected policy data 'CurrentRevisionRequest' field to be unset but it was: %+v", input.CurrentRevisionRequest)
|
||||
}
|
||||
return "", "", false
|
||||
}
|
||||
},
|
||||
},
|
||||
},
|
||||
"evaluate policy chain with the Certificate and Secret if no Request exists": {
|
||||
certificate: &cmapi.Certificate{
|
||||
ObjectMeta: metav1.ObjectMeta{Namespace: "testns", Name: "test", Generation: 3},
|
||||
Spec: cmapi.CertificateSpec{
|
||||
SecretName: "test-secret",
|
||||
},
|
||||
},
|
||||
secret: &corev1.Secret{
|
||||
ObjectMeta: metav1.ObjectMeta{Namespace: "testns", Name: "test-secret"},
|
||||
},
|
||||
chainShouldEvaluate: true,
|
||||
policyFuncs: []policyFuncBuilder{
|
||||
// Add a policy function that ensures only the input's 'certificate'
|
||||
// field is set.
|
||||
func(t *testing.T) policies.Func {
|
||||
return func(input policies.Input) (string, string, bool) {
|
||||
if input.Certificate == nil {
|
||||
t.Error("expected policy data 'Certificate' field to be set but it was not")
|
||||
}
|
||||
if input.Secret == nil {
|
||||
t.Errorf("expected policy data 'Secret' field to be set but it was not")
|
||||
}
|
||||
if input.CurrentRevisionRequest != nil {
|
||||
t.Errorf("expected policy data 'CurrentRevisionRequest' field to be unset but it was: %+v", input.CurrentRevisionRequest)
|
||||
}
|
||||
return "", "", false
|
||||
}
|
||||
},
|
||||
},
|
||||
},
|
||||
"evaluate policy chain with the Certificate, Secret and Request if one exists": {
|
||||
certificate: &cmapi.Certificate{
|
||||
ObjectMeta: metav1.ObjectMeta{Namespace: "testns", Name: "test", Generation: 3},
|
||||
Spec: cmapi.CertificateSpec{
|
||||
SecretName: "test-secret",
|
||||
},
|
||||
Status: cmapi.CertificateStatus{
|
||||
Revision: func(i int) *int { return &i }(3),
|
||||
},
|
||||
},
|
||||
secret: &corev1.Secret{
|
||||
ObjectMeta: metav1.ObjectMeta{Namespace: "testns", Name: "test-secret"},
|
||||
},
|
||||
requests: []*cmapi.CertificateRequest{
|
||||
{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Namespace: "testns",
|
||||
Name: "test",
|
||||
Annotations: map[string]string{
|
||||
cmapi.CertificateRequestRevisionAnnotationKey: "3",
|
||||
},
|
||||
OwnerReferences: []metav1.OwnerReference{
|
||||
*metav1.NewControllerRef(&cmapi.Certificate{ObjectMeta: metav1.ObjectMeta{Namespace: "testns", Name: "test"}}, cmapi.SchemeGroupVersion.WithKind("Certificate")),
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
chainShouldEvaluate: true,
|
||||
policyFuncs: []policyFuncBuilder{
|
||||
// Add a policy function that ensures only the input's 'certificate'
|
||||
// field is set.
|
||||
func(t *testing.T) policies.Func {
|
||||
return func(input policies.Input) (string, string, bool) {
|
||||
if input.Certificate == nil {
|
||||
t.Error("expected policy data 'Certificate' field to be set but it was not")
|
||||
}
|
||||
if input.Secret == nil {
|
||||
t.Errorf("expected policy data 'Secret' field to be set but it was not")
|
||||
}
|
||||
if input.CurrentRevisionRequest == nil {
|
||||
t.Errorf("expected policy data 'CurrentRevisionRequest' field to be set but it was not")
|
||||
}
|
||||
return "", "", false
|
||||
}
|
||||
},
|
||||
},
|
||||
},
|
||||
"error if multiple owned CertificateRequest resources exist and have the same revision": {
|
||||
certificate: &cmapi.Certificate{
|
||||
ObjectMeta: metav1.ObjectMeta{Namespace: "testns", Name: "test", Generation: 3},
|
||||
Spec: cmapi.CertificateSpec{
|
||||
SecretName: "test-secret",
|
||||
},
|
||||
Status: cmapi.CertificateStatus{
|
||||
Revision: func(i int) *int { return &i }(3),
|
||||
},
|
||||
},
|
||||
secret: &corev1.Secret{
|
||||
ObjectMeta: metav1.ObjectMeta{Namespace: "testns", Name: "test-secret"},
|
||||
},
|
||||
requests: []*cmapi.CertificateRequest{
|
||||
{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Namespace: "testns",
|
||||
Name: "test",
|
||||
Annotations: map[string]string{
|
||||
cmapi.CertificateRequestRevisionAnnotationKey: "3",
|
||||
},
|
||||
OwnerReferences: []metav1.OwnerReference{
|
||||
*metav1.NewControllerRef(&cmapi.Certificate{ObjectMeta: metav1.ObjectMeta{Namespace: "testns", Name: "test"}}, cmapi.SchemeGroupVersion.WithKind("Certificate")),
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Namespace: "testns",
|
||||
Name: "test-number-two",
|
||||
Annotations: map[string]string{
|
||||
cmapi.CertificateRequestRevisionAnnotationKey: "3",
|
||||
},
|
||||
OwnerReferences: []metav1.OwnerReference{
|
||||
*metav1.NewControllerRef(&cmapi.Certificate{ObjectMeta: metav1.ObjectMeta{Namespace: "testns", Name: "test"}}, cmapi.SchemeGroupVersion.WithKind("Certificate")),
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
chainShouldEvaluate: false,
|
||||
err: "multiple CertificateRequest resources exist for the current revision, not triggering new issuance until requests have been cleaned up",
|
||||
},
|
||||
"should evaluate policy if no certificaterequest resource exists for the current revision": {
|
||||
certificate: &cmapi.Certificate{
|
||||
ObjectMeta: metav1.ObjectMeta{Namespace: "testns", Name: "test", Generation: 3},
|
||||
Spec: cmapi.CertificateSpec{
|
||||
SecretName: "test-secret",
|
||||
},
|
||||
Status: cmapi.CertificateStatus{
|
||||
Revision: func(i int) *int { return &i }(3),
|
||||
},
|
||||
},
|
||||
secret: &corev1.Secret{
|
||||
ObjectMeta: metav1.ObjectMeta{Namespace: "testns", Name: "test-secret"},
|
||||
},
|
||||
chainShouldEvaluate: true,
|
||||
},
|
||||
"should set the 'Issuing' status condition if the chain indicates an issuance is required": {
|
||||
certificate: &cmapi.Certificate{
|
||||
ObjectMeta: metav1.ObjectMeta{Namespace: "testns", Name: "test", Generation: 3},
|
||||
},
|
||||
chainShouldEvaluate: true,
|
||||
chainShouldTriggerIssuance: true,
|
||||
expectedEvent: "Normal Issuing Re-issuance forced by unit test case",
|
||||
expectedConditions: []cmapi.CertificateCondition{
|
||||
{
|
||||
Type: cmapi.CertificateConditionIssuing,
|
||||
Status: cmmeta.ConditionTrue,
|
||||
Reason: forceTriggeredReason,
|
||||
Message: forceTriggeredMessage,
|
||||
LastTransitionTime: &metaNow,
|
||||
"should do nothing if Certificate already has 'Issuing' condition": {
|
||||
existingCertificate: gen.Certificate("cert-1", gen.SetCertificateNamespace("testns"),
|
||||
gen.SetCertificateGeneration(42),
|
||||
gen.SetCertificateStatusCondition(cmapi.CertificateCondition{
|
||||
Type: "Issuing",
|
||||
Status: "True",
|
||||
ObservedGeneration: 3,
|
||||
},
|
||||
}),
|
||||
),
|
||||
},
|
||||
"should call shouldReissue with the correct cert, secret and current CR": {
|
||||
existingCertificate: gen.Certificate("cert-1", gen.SetCertificateNamespace("testns"),
|
||||
gen.SetCertificateSecretName("secret-1"),
|
||||
gen.SetCertificateGeneration(42),
|
||||
gen.SetCertificateUID("cert-1-uid"),
|
||||
gen.SetCertificateRevision(2),
|
||||
),
|
||||
wantDataForCertificateCalled: true,
|
||||
mockDataForCertificateReturn: policies.Input{
|
||||
Secret: gen.Secret("secret-1", gen.SetSecretNamespace("testns")),
|
||||
CurrentRevisionRequest: gen.CertificateRequest("cr-1", gen.SetCertificateRequestNamespace("testns"),
|
||||
gen.AddCertificateRequestOwnerReferences(gen.CertificateRef("cert-1", "cert-1-uid")),
|
||||
gen.SetCertificateRequestAnnotations(map[string]string{"cert-manager.io/certificate-revision": "2"}),
|
||||
),
|
||||
},
|
||||
wantShouldReissueCalled: true,
|
||||
mockShouldReissue: func(t *testing.T) policies.Func {
|
||||
return func(gotInput policies.Input) (string, string, bool) {
|
||||
expectInput := policies.Input{
|
||||
Certificate: gen.Certificate("cert-1", gen.SetCertificateNamespace("testns"),
|
||||
gen.SetCertificateSecretName("secret-1"),
|
||||
gen.SetCertificateGeneration(42),
|
||||
gen.SetCertificateUID("cert-1-uid"),
|
||||
gen.SetCertificateRevision(2),
|
||||
),
|
||||
CurrentRevisionRequest: gen.CertificateRequest("cr-1", gen.SetCertificateRequestNamespace("testns"),
|
||||
gen.AddCertificateRequestOwnerReferences(gen.CertificateRef("cert-1", "cert-1-uid")),
|
||||
gen.SetCertificateRequestAnnotations(map[string]string{"cert-manager.io/certificate-revision": "2"}),
|
||||
),
|
||||
Secret: gen.Secret("secret-1", gen.SetSecretNamespace("testns")),
|
||||
}
|
||||
assert.Equal(t, expectInput, gotInput)
|
||||
return "", "", false
|
||||
}
|
||||
},
|
||||
},
|
||||
"should not set the 'Issuing' status condition if the chain indicates an issuance is required if the last failure time is within the last hour": {
|
||||
certificate: &cmapi.Certificate{
|
||||
ObjectMeta: metav1.ObjectMeta{Namespace: "testns", Name: "test", Generation: 3},
|
||||
Status: cmapi.CertificateStatus{
|
||||
LastFailureTime: func(m metav1.Time) *metav1.Time { return &m }(metav1.NewTime(now.Add(-59 * time.Minute))),
|
||||
},
|
||||
},
|
||||
chainShouldEvaluate: false,
|
||||
chainShouldTriggerIssuance: false,
|
||||
"should log error when dataForCertificate errors": {
|
||||
existingCertificate: gen.Certificate("cert-1", gen.SetCertificateNamespace("testns")),
|
||||
wantDataForCertificateCalled: true,
|
||||
mockDataForCertificateReturnErr: fmt.Errorf("dataForCertificate failed"),
|
||||
wantErr: "dataForCertificate failed",
|
||||
},
|
||||
"should set the 'Issuing' status condition if the chain indicates an issuance is required if the last failure time is older than the last hour": {
|
||||
certificate: &cmapi.Certificate{
|
||||
ObjectMeta: metav1.ObjectMeta{Namespace: "testns", Name: "test", Generation: 3},
|
||||
Status: cmapi.CertificateStatus{
|
||||
LastFailureTime: func(m metav1.Time) *metav1.Time { return &m }(metav1.NewTime(now.Add(-61 * time.Minute))),
|
||||
},
|
||||
"should set Issuing=True if shouldReissue tells us to reissue": {
|
||||
existingCertificate: gen.Certificate("cert-1", gen.SetCertificateNamespace("testns"),
|
||||
gen.SetCertificateGeneration(42),
|
||||
),
|
||||
wantDataForCertificateCalled: true,
|
||||
mockDataForCertificateReturn: policies.Input{},
|
||||
wantShouldReissueCalled: true,
|
||||
mockShouldReissue: func(*testing.T) policies.Func {
|
||||
return func(policies.Input) (string, string, bool) {
|
||||
return "ForceTriggered", "Re-issuance forced by unit test case", true
|
||||
}
|
||||
},
|
||||
chainShouldEvaluate: true,
|
||||
chainShouldTriggerIssuance: true,
|
||||
expectedEvent: "Normal Issuing Re-issuance forced by unit test case",
|
||||
expectedConditions: []cmapi.CertificateCondition{
|
||||
{
|
||||
Type: cmapi.CertificateConditionIssuing,
|
||||
Status: cmmeta.ConditionTrue,
|
||||
Reason: forceTriggeredReason,
|
||||
Message: forceTriggeredMessage,
|
||||
LastTransitionTime: &metaNow,
|
||||
ObservedGeneration: 3,
|
||||
},
|
||||
wantEvent: "Normal Issuing Re-issuance forced by unit test case",
|
||||
wantConditions: []cmapi.CertificateCondition{{
|
||||
Type: "Issuing",
|
||||
Status: "True",
|
||||
Reason: "ForceTriggered",
|
||||
Message: "Re-issuance forced by unit test case",
|
||||
LastTransitionTime: &fixedNow,
|
||||
ObservedGeneration: 42,
|
||||
}},
|
||||
},
|
||||
"should not set Issuing=True when cert has been failing for 59 minutes": {
|
||||
existingCertificate: gen.Certificate("cert-1", gen.SetCertificateNamespace("testns"),
|
||||
gen.SetCertificateLastFailureTime(metav1.NewTime(fixedNow.Add(-59*time.Minute))),
|
||||
),
|
||||
wantDataForCertificateCalled: true,
|
||||
mockDataForCertificateReturn: policies.Input{},
|
||||
wantShouldReissueCalled: false,
|
||||
},
|
||||
"should set Issuing=True when cert has been failing for 61 minutes and shouldReissue returns true": {
|
||||
existingCertificate: gen.Certificate("cert-1", gen.SetCertificateNamespace("testns"),
|
||||
gen.SetCertificateGeneration(42),
|
||||
gen.SetCertificateLastFailureTime(metav1.NewTime(fixedNow.Add(-61*time.Minute))),
|
||||
),
|
||||
wantDataForCertificateCalled: true,
|
||||
mockDataForCertificateReturn: policies.Input{},
|
||||
wantShouldReissueCalled: true,
|
||||
mockShouldReissue: func(*testing.T) policies.Func {
|
||||
return func(policies.Input) (string, string, bool) {
|
||||
return "ForceTriggered", "Re-issuance forced by unit test case", true
|
||||
}
|
||||
},
|
||||
wantEvent: "Normal Issuing Re-issuance forced by unit test case",
|
||||
wantConditions: []cmapi.CertificateCondition{{
|
||||
Type: "Issuing",
|
||||
Status: "True",
|
||||
Reason: "ForceTriggered",
|
||||
Message: "Re-issuance forced by unit test case",
|
||||
LastTransitionTime: &fixedNow,
|
||||
ObservedGeneration: 42,
|
||||
}},
|
||||
},
|
||||
}
|
||||
for name, test := range tests {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
// Create and initialise a new unit test builder
|
||||
builder := &testpkg.Builder{
|
||||
T: t,
|
||||
Clock: fakeclock.NewFakeClock(now),
|
||||
Clock: fakeclock.NewFakeClock(fixedNow.Time),
|
||||
}
|
||||
if test.certificate != nil {
|
||||
builder.CertManagerObjects = append(builder.CertManagerObjects, test.certificate)
|
||||
}
|
||||
if test.secret != nil {
|
||||
builder.KubeObjects = append(builder.KubeObjects, test.secret)
|
||||
}
|
||||
for _, req := range test.requests {
|
||||
builder.CertManagerObjects = append(builder.CertManagerObjects, req)
|
||||
if test.existingCertificate != nil {
|
||||
builder.CertManagerObjects = append(builder.CertManagerObjects, test.existingCertificate)
|
||||
}
|
||||
builder.Init()
|
||||
|
||||
// Register informers used by the controller using the registration wrapper
|
||||
w := &controllerWrapper{}
|
||||
_, _, err := w.Register(builder.Context)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// Fake out the default policy chain
|
||||
w.policyChain = []policies.Func{}
|
||||
// Record whether the policy chain was evaluated
|
||||
evaluated := false
|
||||
w.policyChain = append(w.policyChain, func(_ policies.Input) (string, string, bool) {
|
||||
evaluated = true
|
||||
return "", "", false
|
||||
})
|
||||
// Add any test-specific policies to the chain
|
||||
w.policyChain = append(w.policyChain, buildTestPolicyChain(t, test.policyFuncs...)...)
|
||||
// If the chain should trigger an issuance, inject an 'always reissue'
|
||||
// policyFunc at the end of the chain
|
||||
if test.chainShouldTriggerIssuance {
|
||||
w.policyChain = append(w.policyChain, func(_ policies.Input) (string, string, bool) {
|
||||
return forceTriggeredReason, forceTriggeredMessage, true
|
||||
})
|
||||
|
||||
gotShouldReissueCalled := false
|
||||
w.shouldReissue = func(i policies.Input) (string, string, bool) {
|
||||
gotShouldReissueCalled = true
|
||||
if test.mockShouldReissue == nil {
|
||||
t.Fatal("no mock set for shouldReissue, but shouldReissue has been called")
|
||||
return "", "", false
|
||||
} else {
|
||||
return test.mockShouldReissue(t)(i)
|
||||
}
|
||||
}
|
||||
if test.expectedConditions != nil {
|
||||
if test.certificate == nil {
|
||||
|
||||
// TODO(mael): we should really remove the Certificate field from
|
||||
// DataForCertificate since the input certificate is always expected
|
||||
// to be the same as the output certiticate.
|
||||
test.mockDataForCertificateReturn.Certificate = test.existingCertificate
|
||||
|
||||
gotDataForCertificateCalled := false
|
||||
w.dataForCertificate = func(context.Context, *cmapi.Certificate) (policies.Input, error) {
|
||||
gotDataForCertificateCalled = true
|
||||
return test.mockDataForCertificateReturn, test.mockDataForCertificateReturnErr
|
||||
}
|
||||
|
||||
if test.wantConditions != nil {
|
||||
if test.existingCertificate == nil {
|
||||
t.Fatal("cannot expect an Update operation if test.certificate is nil")
|
||||
}
|
||||
expectedCert := test.certificate.DeepCopy()
|
||||
expectedCert.Status.Conditions = test.expectedConditions
|
||||
expectedCert := test.existingCertificate.DeepCopy()
|
||||
expectedCert.Status.Conditions = test.wantConditions
|
||||
builder.ExpectedActions = append(builder.ExpectedActions,
|
||||
testpkg.NewAction(coretesting.NewUpdateSubresourceAction(
|
||||
cmapi.SchemeGroupVersion.WithResource("certificates"),
|
||||
"status",
|
||||
test.certificate.Namespace,
|
||||
test.existingCertificate.Namespace,
|
||||
expectedCert,
|
||||
)),
|
||||
)
|
||||
}
|
||||
if test.expectedEvent != "" {
|
||||
builder.ExpectedEvents = []string{test.expectedEvent}
|
||||
if test.wantEvent != "" {
|
||||
builder.ExpectedEvents = []string{test.wantEvent}
|
||||
}
|
||||
// Start the informers and begin processing updates
|
||||
|
||||
builder.Start()
|
||||
defer builder.Stop()
|
||||
|
||||
key := test.key
|
||||
if key == "" && test.certificate != nil {
|
||||
key, err = controllerpkg.KeyFunc(test.certificate)
|
||||
if key == "" && test.existingCertificate != nil {
|
||||
key, err = controllerpkg.KeyFunc(test.existingCertificate)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
|
||||
// Call ProcessItem
|
||||
err = w.controller.ProcessItem(context.Background(), key)
|
||||
gotErr := w.controller.ProcessItem(context.Background(), key)
|
||||
switch {
|
||||
case err != nil:
|
||||
if test.err != err.Error() {
|
||||
t.Errorf("error text did not match, got=%s, exp=%s", err.Error(), test.err)
|
||||
case gotErr != nil:
|
||||
if test.wantErr != gotErr.Error() {
|
||||
t.Errorf("error text did not match, got=%s, exp=%s", gotErr.Error(), test.wantErr)
|
||||
}
|
||||
default:
|
||||
if test.err != "" {
|
||||
t.Errorf("got no error but expected: %s", test.err)
|
||||
}
|
||||
}
|
||||
if evaluated != test.chainShouldEvaluate {
|
||||
if test.chainShouldEvaluate {
|
||||
t.Error("expected policy chain to be evaluated but it was not")
|
||||
} else {
|
||||
t.Error("expected policy chain to NOT be evaluated but it was")
|
||||
if test.wantErr != "" {
|
||||
t.Errorf("got no error but expected: %s", test.wantErr)
|
||||
}
|
||||
}
|
||||
|
||||
if err := builder.AllEventsCalled(); err != nil {
|
||||
builder.T.Error(err)
|
||||
}
|
||||
if err := builder.AllActionsExecuted(); err != nil {
|
||||
builder.T.Error(err)
|
||||
}
|
||||
if err := builder.AllReactorsCalled(); err != nil {
|
||||
builder.T.Error(err)
|
||||
}
|
||||
assert.Equal(t, test.wantDataForCertificateCalled, gotDataForCertificateCalled, "dataForCertificate func call")
|
||||
assert.Equal(t, test.wantShouldReissueCalled, gotShouldReissueCalled, "shouldReissue func call")
|
||||
|
||||
builder.CheckAndFinish()
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func buildTestPolicyChain(t *testing.T, funcs ...policyFuncBuilder) policies.Chain {
|
||||
c := policies.Chain{}
|
||||
for _, f := range funcs {
|
||||
c = append(c, f(t))
|
||||
}
|
||||
return c
|
||||
}
|
||||
|
||||
func Test_shouldBackoffReissuingOnFailure(t *testing.T) {
|
||||
clock := fakeclock.NewFakeClock(time.Date(2020, 11, 20, 16, 05, 00, 0000, time.UTC))
|
||||
tests := []struct {
|
||||
name string
|
||||
clock := fakeclock.NewFakeClock(time.Date(2020, 11, 20, 16, 05, 00, 0000, time.Local))
|
||||
|
||||
tests := map[string]struct {
|
||||
givenCert *cmapi.Certificate
|
||||
wantBackoff bool
|
||||
wantDelay time.Duration
|
||||
}{
|
||||
{
|
||||
name: "no need to back off from reissuing when there is no previous failure",
|
||||
givenCert: gen.Certificate("test",
|
||||
gen.SetCertificateNamespace("testns"),
|
||||
gen.SetCertificateUID("test-uid"),
|
||||
gen.SetCertificateDNSNames("example2.com"),
|
||||
"should not back off from reissuing when the input request is nil": {
|
||||
givenCert: gen.Certificate("cert-1", gen.SetCertificateNamespace("testns")),
|
||||
wantBackoff: false,
|
||||
},
|
||||
"should not back off from reissuing when cert is not failing": {
|
||||
givenCert: gen.Certificate("cert-1", gen.SetCertificateNamespace("testns"),
|
||||
gen.SetCertificateUID("cert-1-uid"),
|
||||
gen.SetCertificateRevision(1),
|
||||
gen.SetCertificateDNSNames("example2.com"),
|
||||
// LastFailureTime is not set here.
|
||||
),
|
||||
wantBackoff: false,
|
||||
},
|
||||
{
|
||||
name: "no need to back off from reissuing when the failure is more than an hour ago, reissuance can happen now",
|
||||
givenCert: gen.Certificate("test",
|
||||
gen.SetCertificateNamespace("testns"),
|
||||
gen.SetCertificateUID("test-uid"),
|
||||
gen.SetCertificateDNSNames("example2.com"),
|
||||
"should not back off from reissuing when the failure happened 61 minutes ago": {
|
||||
givenCert: gen.Certificate("cert-1", gen.SetCertificateNamespace("testns"),
|
||||
gen.SetCertificateUID("cert-1-uid"),
|
||||
gen.SetCertificateRevision(1),
|
||||
gen.SetCertificateDNSNames("example2.com"),
|
||||
gen.SetCertificateLastFailureTime(metav1.NewTime(clock.Now().Add(-61*time.Minute))),
|
||||
),
|
||||
wantBackoff: false,
|
||||
},
|
||||
{
|
||||
name: "needs to back off from reissuing when the failure happened less than an hour ago",
|
||||
givenCert: gen.Certificate("test",
|
||||
gen.SetCertificateNamespace("testns"),
|
||||
gen.SetCertificateUID("test-uid"),
|
||||
gen.SetCertificateDNSNames("example2.com"),
|
||||
"should not back off from reissuing when the failure happened 60 minutes ago": {
|
||||
givenCert: gen.Certificate("cert-1", gen.SetCertificateNamespace("testns"),
|
||||
gen.SetCertificateUID("cert-1-uid"),
|
||||
gen.SetCertificateRevision(1),
|
||||
gen.SetCertificateDNSNames("example2.com"),
|
||||
gen.SetCertificateLastFailureTime(metav1.NewTime(clock.Now().Add(-60*time.Minute))),
|
||||
),
|
||||
wantBackoff: false,
|
||||
},
|
||||
"should back off from reissuing when the failure happened 59 minutes ago": {
|
||||
givenCert: gen.Certificate("cert-1", gen.SetCertificateNamespace("testns"),
|
||||
gen.SetCertificateLastFailureTime(metav1.NewTime(clock.Now().Add(-59*time.Minute))),
|
||||
gen.SetCertificateUID("cert-1-uid"),
|
||||
gen.SetCertificateRevision(1),
|
||||
gen.SetCertificateDNSNames("example2.com"),
|
||||
),
|
||||
wantBackoff: true,
|
||||
wantDelay: 1 * time.Minute,
|
||||
},
|
||||
{
|
||||
name: "no need to back off from reissuing when the failure happened exactly an hour ago",
|
||||
givenCert: gen.Certificate("test",
|
||||
gen.SetCertificateNamespace("testns"),
|
||||
gen.SetCertificateUID("test-uid"),
|
||||
gen.SetCertificateDNSNames("example2.com"),
|
||||
gen.SetCertificateRevision(1),
|
||||
gen.SetCertificateLastFailureTime(metav1.NewTime(clock.Now().Add(-60*time.Minute))),
|
||||
),
|
||||
wantBackoff: false,
|
||||
wantDelay: 0,
|
||||
},
|
||||
{
|
||||
name: "needs to back off from reissuing for the maximum of 1 hour when failure just happened",
|
||||
givenCert: gen.Certificate("test",
|
||||
gen.SetCertificateNamespace("testns"),
|
||||
gen.SetCertificateUID("test-uid"),
|
||||
gen.SetCertificateDNSNames("example2.com"),
|
||||
"should back off from reissuing when the failure happened 0 minutes ago": {
|
||||
givenCert: gen.Certificate("cert-1", gen.SetCertificateNamespace("testns"),
|
||||
gen.SetCertificateUID("cert-1-uid"),
|
||||
gen.SetCertificateRevision(1),
|
||||
gen.SetCertificateLastFailureTime(metav1.NewTime(clock.Now())),
|
||||
gen.SetCertificateDNSNames("example2.com"),
|
||||
),
|
||||
wantBackoff: true,
|
||||
wantDelay: 1 * time.Hour,
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
gotBackoff, gotDelay := shouldBackoffReissuingOnFailure(logtest.TestLogger{T: t}, clock, tt.givenCert)
|
||||
assert.Equal(t, tt.wantBackoff, gotBackoff)
|
||||
assert.Equal(t, tt.wantDelay, gotDelay)
|
||||
for name, test := range tests {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
gotBackoff, gotDelay := shouldBackoffReissuingOnFailure(logtest.TestLogger{T: t}, clock, test.givenCert)
|
||||
assert.Equal(t, test.wantBackoff, gotBackoff)
|
||||
assert.Equal(t, test.wantDelay, gotDelay)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@ -67,7 +67,8 @@ func TestTriggerController(t *testing.T) {
|
||||
}
|
||||
// default certificate renewBefore period
|
||||
defaultRenewBefore := time.Hour * 24
|
||||
ctrl, queue, mustSync := trigger.NewController(logf.Log, cmCl, factory, cmFactory, framework.NewEventRecorder(t), fakeClock, policies.NewTriggerPolicyChain(fakeClock, defaultRenewBefore))
|
||||
shouldReissue := policies.NewTriggerPolicyChain(fakeClock, defaultRenewBefore).Evaluate
|
||||
ctrl, queue, mustSync := trigger.NewController(logf.Log, cmCl, factory, cmFactory, framework.NewEventRecorder(t), fakeClock, shouldReissue)
|
||||
c := controllerpkg.NewController(
|
||||
context.Background(),
|
||||
"trigger_test",
|
||||
@ -127,7 +128,7 @@ func TestTriggerController_RenewNearExpiry(t *testing.T) {
|
||||
// Only use the 'current certificate nearing expiry' policy chain during the
|
||||
// test as we want to test the very specific cases of triggering/not
|
||||
// triggering depending on whether a renewal is required.
|
||||
policyChain := policies.Chain{policies.CurrentCertificateNearingExpiry(fakeClock, defaultRenewBefore)}
|
||||
shoudReissue := policies.Chain{policies.CurrentCertificateNearingExpiry(fakeClock, defaultRenewBefore)}.Evaluate
|
||||
// Build, instantiate and run the trigger controller.
|
||||
kubeClient, factory, cmCl, cmFactory := framework.NewClients(t, config)
|
||||
|
||||
@ -182,7 +183,7 @@ func TestTriggerController_RenewNearExpiry(t *testing.T) {
|
||||
}
|
||||
|
||||
// Start the trigger controller
|
||||
ctrl, queue, mustSync := trigger.NewController(logf.Log, cmCl, factory, cmFactory, framework.NewEventRecorder(t), fakeClock, policyChain)
|
||||
ctrl, queue, mustSync := trigger.NewController(logf.Log, cmCl, factory, cmFactory, framework.NewEventRecorder(t), fakeClock, shoudReissue)
|
||||
c := controllerpkg.NewController(
|
||||
logf.NewContext(context.Background(), logf.Log, "trigger_controller_RenewNearExpiry"),
|
||||
"trigger_test",
|
||||
|
||||
Loading…
Reference in New Issue
Block a user