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

Merge pull request #5121 from wallrj/3640-finalizer-cleanup

Browse Source 
Refactor the update and updateStatus to a single deferred function
This commit is contained in:
jetstack-bot 2022-05-16 12:39:01 +01:00 committed by GitHub
commit b0caedffd9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 418 additions and 147 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
pkg/controller/acmechallenges/BUILD.bazel
View File

@ -7,6 +7,7 @@ go_library(
"controller.go",
"finalizer.go",
"sync.go",
"update.go",
],
importpath = "github.com/cert-manager/cert-manager/pkg/controller/acmechallenges",
visibility = ["//visibility:public"],
@ -30,7 +31,9 @@ go_library(
"//pkg/issuer/acme/http:go_default_library",
"//pkg/logs:go_default_library",
"//pkg/util/feature:go_default_library",
"//test/unit/gen:go_default_library",
"@com_github_go_logr_logr//:go_default_library",
"@com_github_pkg_errors//:go_default_library",
"@io_k8s_api//core/v1:go_default_library",
"@io_k8s_apimachinery//pkg/api/equality:go_default_library",
"@io_k8s_apimachinery//pkg/api/errors:go_default_library",
@ -51,22 +54,27 @@ go_test(
"controller_test.go",
"finalizer_test.go",
"sync_test.go",
"update_test.go",
],
embed = [":go_default_library"],
deps = [
"//internal/controller/feature:go_default_library",
"//pkg/acme/accounts/test:go_default_library",
"//pkg/acme/client:go_default_library",
"//pkg/apis/acme/v1:go_default_library",
"//pkg/apis/certmanager/v1:go_default_library",
"//pkg/apis/meta/v1:go_default_library",
"//pkg/client/clientset/versioned/fake:go_default_library",
"//pkg/controller/test:go_default_library",
"//pkg/issuer:go_default_library",
"//pkg/util/feature:go_default_library",
"//test/unit/gen:go_default_library",
"@com_github_stretchr_testify//assert:go_default_library",
"@com_github_stretchr_testify//require:go_default_library",
"@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library",
"@io_k8s_apimachinery//pkg/runtime:go_default_library",
"@io_k8s_client_go//testing:go_default_library",
"@io_k8s_component_base//featuregate/testing:go_default_library",
"@org_golang_x_crypto//acme:go_default_library",
],
)

26
pkg/controller/acmechallenges/controller.go
View File

@ -30,7 +30,6 @@ import (
"github.com/cert-manager/cert-manager/internal/ingress"
"github.com/cert-manager/cert-manager/pkg/acme/accounts"
cmclient "github.com/cert-manager/cert-manager/pkg/client/clientset/versioned"
cmacmelisters "github.com/cert-manager/cert-manager/pkg/client/listers/acme/v1"
cmlisters "github.com/cert-manager/cert-manager/pkg/client/listers/certmanager/v1"
controllerpkg "github.com/cert-manager/cert-manager/pkg/controller"
@ -54,9 +53,6 @@ type controller struct {
clusterIssuerLister cmlisters.ClusterIssuerLister
secretLister corelisters.SecretLister
// fieldManager is the manager name used for the Apply operations.
fieldManager string
// ACME challenge solvers are instantiated once at the time of controller
// construction.
// This also allows for easy mocking of the different challenge mechanisms.
@ -69,8 +65,6 @@ type controller struct {
// used to record Events about resources to the API
recorder record.EventRecorder
// clientset used to update cert-manager API resources
cmClient cmclient.Interface
// maintain a reference to the workqueue for this controller
// so the handleOwnedResource method can enqueue resources
@ -82,6 +76,10 @@ type controller struct {
dns01Nameservers []string
DNS01CheckRetryPeriod time.Duration
// objectUpdater implements the updateObject function which is used to save
// changes to the Challenge.Status and Challenge.Finalizers
objectUpdater
}
func (c *controller) Register(ctx *controllerpkg.Context) (workqueue.RateLimitingInterface, []cache.InformerSynced, error) {
@ -140,8 +138,6 @@ func (c *controller) Register(ctx *controllerpkg.Context) (workqueue.RateLimitin
c.helper = issuer.NewHelper(c.issuerLister, c.clusterIssuerLister)
c.scheduler = scheduler.New(logf.NewContext(ctx.RootContext, c.log), c.challengeLister, ctx.SchedulerOptions.MaxConcurrentChallenges)
c.recorder = ctx.Recorder
c.cmClient = ctx.CMClient
c.fieldManager = ctx.FieldManager
c.accountRegistry = ctx.ACMEOptions.AccountRegistry
c.httpSolver, err = http.NewSolver(ctx)
@ -157,6 +153,10 @@ func (c *controller) Register(ctx *controllerpkg.Context) (workqueue.RateLimitin
c.dns01Nameservers = ctx.ACMEOptions.DNS01Nameservers
c.DNS01CheckRetryPeriod = ctx.ACMEOptions.DNS01CheckRetryPeriod
// Construct an objectUpdater which is used to save changes to the Challenge
// object, either using Update or using Patch + Server Side Apply.
c.objectUpdater = newObjectUpdater(ctx.CMClient, ctx.FieldManager)
return c.queue, mustSync, nil
}
@ -181,16 +181,14 @@ func (c *controller) runScheduler(ctx context.Context) {
return
}
for _, ch := range toSchedule {
log := logf.WithResource(log, ch)
ch = ch.DeepCopy()
for _, chOriginal := range toSchedule {
log := logf.WithResource(log, chOriginal)
ch := chOriginal.DeepCopy()
ch.Status.Processing = true
_, err := c.updateStatusOrApply(ctx, ch)
if err != nil {
if err := c.updateObject(ctx, chOriginal, ch); err != nil {
log.Error(err, "error scheduling challenge for processing")
return
}
c.recorder.Event(ch, corev1.EventTypeNormal, "Started", "Challenge scheduled for processing")
}

19
pkg/controller/acmechallenges/finalizer.go
View File

@ -17,8 +17,6 @@ limitations under the License.
package acmechallenges
import (
"context"
"k8s.io/apimachinery/pkg/util/sets"
cmacme "github.com/cert-manager/cert-manager/pkg/apis/acme/v1"
@ -31,24 +29,7 @@ import (
// deployed ("presented") resources and if successful, removes this finalizer
// allowing the garbage collector to remove the challenge.
// challengeUpdater allows the spec and metadata of a Challenge to be modified.
type challengeUpdater interface {
updateOrApply(context.Context, *cmacme.Challenge) (*cmacme.Challenge, error)
}
// finalizerRequired returns true if the finalizer is not found on the challenge.
func finalizerRequired(ch *cmacme.Challenge) bool {
return !sets.NewString(ch.Finalizers...).Has(cmacme.ACMEFinalizer)
}
// addFinalizer adds the finalizer to the challenge and saves the change to the API
// server.
func addFinalizer(client challengeUpdater, ctx context.Context, ch *cmacme.Challenge) (*cmacme.Challenge, error) {
ch = ch.DeepCopy()
ch.Finalizers = append(ch.Finalizers, cmacme.ACMEFinalizer)
ch, err := client.updateOrApply(ctx, ch)
if err != nil {
return nil, err
}
return ch, nil
}

60
pkg/controller/acmechallenges/finalizer_test.go
View File

@ -17,8 +17,6 @@ limitations under the License.
package acmechallenges
import (
"context"
"errors"
"testing"
"github.com/stretchr/testify/assert"
@ -66,61 +64,3 @@ func Test_finalizerRequired(t *testing.T) {
})
}
}
type fakeChallengeUpdater struct {
fakeUpdateOrApply func(context.Context, *cmacme.Challenge) (*cmacme.Challenge, error)
}
func (o *fakeChallengeUpdater) updateOrApply(ctx context.Context, ch *cmacme.Challenge) (*cmacme.Challenge, error) {
return o.fakeUpdateOrApply(ctx, ch)
}
func Test_addFinalizer(t *testing.T) {
simulatedError := errors.New("simulated-error")
tests := []struct {
name string
updateErr error
wantErr error
}{
{
name: "update-success",
updateErr: nil,
wantErr: nil,
},
{
name: "update-failure",
updateErr: simulatedError,
wantErr: simulatedError,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ch := gen.Challenge("example")
originalCh := ch.DeepCopy()
newCh, err := addFinalizer(
&fakeChallengeUpdater{
fakeUpdateOrApply: func(_ context.Context, ch *cmacme.Challenge) (*cmacme.Challenge, error) {
if tt.updateErr != nil {
return nil, tt.updateErr
}
ch = ch.DeepCopy()
// Update the generation to simulate the sort of change
// that the API server will return.
ch.Generation += 1
return ch, nil
},
},
context.TODO(),
ch,
)
assert.Equal(t, tt.wantErr, err)
assert.Equal(t, originalCh, ch, "the supplied challenge should never be modified")
if err == nil {
assert.EqualValues(t, 1, newCh.Generation-originalCh.Generation,
"if the update succeeds the supplied challenge pointer should be updated with the updated challenge")
} else {
assert.Nil(t, newCh, "if the update fails the returned challenge should be nil")
}
})
}
}

59
pkg/controller/acmechallenges/sync.go
View File

@ -23,11 +23,8 @@ import (
acmeapi "golang.org/x/crypto/acme"
corev1 "k8s.io/api/core/v1"
apiequality "k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
internalchallenges "github.com/cert-manager/cert-manager/internal/controller/challenges"
"github.com/cert-manager/cert-manager/internal/controller/feature"
"github.com/cert-manager/cert-manager/pkg/acme"
acmecl "github.com/cert-manager/cert-manager/pkg/acme/client"
@ -62,26 +59,25 @@ type solver interface {
// Sync will process this ACME Challenge.
// It is the core control function for ACME challenges.
func (c *controller) Sync(ctx context.Context, ch *cmacme.Challenge) (err error) {
log := logf.FromContext(ctx).WithValues("dnsName", ch.Spec.DNSName, "type", ch.Spec.Type)
func (c *controller) Sync(ctx context.Context, chOriginal *cmacme.Challenge) (err error) {
log := logf.FromContext(ctx).WithValues("dnsName", chOriginal.Spec.DNSName, "type", chOriginal.Spec.Type)
ctx = logf.NewContext(ctx, log)
ch := chOriginal.DeepCopy()
oldChal := ch
ch = ch.DeepCopy()
defer func() {
if updateError := c.updateObject(ctx, chOriginal, ch); updateError != nil {
if errors.Is(updateError, argumentError) {
log.Error(updateError, "If this error occurs there is a bug in cert-manager. Please report it. Not retrying.")
return
}
err = utilerrors.NewAggregate([]error{err, updateError})
}
}()
if !ch.DeletionTimestamp.IsZero() {
return c.handleFinalizer(ctx, ch)
}
defer func() {
if apiequality.Semantic.DeepEqual(oldChal.Status, ch.Status) {
return
}
if _, updateErr := c.updateStatusOrApply(ctx, ch); updateErr != nil {
err = utilerrors.NewAggregate([]error{err, updateErr})
}
}()
// bail out early on if processing=false, as this challenge has not been
// scheduled yet.
if !ch.Status.Processing {
@ -92,8 +88,8 @@ func (c *controller) Sync(ctx context.Context, ch *cmacme.Challenge) (err error)
// cert-manager has a chance to clean up resources created for the
// challenge.
if finalizerRequired(ch) {
ch, err = addFinalizer(c, ctx, ch)
return err
ch.Finalizers = append(ch.Finalizers, cmacme.ACMEFinalizer)
return nil
}
genericIssuer, err := c.helper.GetGenericIssuer(ch.Spec.IssuerRef, ch.Namespace)
@ -260,19 +256,8 @@ func (c *controller) handleFinalizer(ctx context.Context, ch *cmacme.Challenge)
}
defer func() {
// call UpdateStatus first as we may have updated the challenge.status.reason field
ch, updateErr := c.updateStatusOrApply(ctx, ch)
if updateErr != nil {
err = utilerrors.NewAggregate([]error{err, updateErr})
return
}
// call Update to remove the metadata.finalizers entry
ch.Finalizers = ch.Finalizers[1:]
_, updateErr = c.updateOrApply(ctx, ch)
if updateErr != nil {
err = utilerrors.NewAggregate([]error{err, updateErr})
return
}
}()
if !ch.Status.Processing {
@ -432,19 +417,3 @@ func (c *controller) solverFor(challengeType cmacme.ACMEChallengeType) (solver,
}
return nil, fmt.Errorf("no solver for %q implemented", challengeType)
}
func (c *controller) updateOrApply(ctx context.Context, challenge *cmacme.Challenge) (*cmacme.Challenge, error) {
if utilfeature.DefaultFeatureGate.Enabled(feature.ServerSideApply) {
return internalchallenges.Apply(ctx, c.cmClient, c.fieldManager, challenge)
} else {
return c.cmClient.AcmeV1().Challenges(challenge.Namespace).Update(ctx, challenge, metav1.UpdateOptions{})
}
}
func (c *controller) updateStatusOrApply(ctx context.Context, challenge *cmacme.Challenge) (*cmacme.Challenge, error) {
if utilfeature.DefaultFeatureGate.Enabled(feature.ServerSideApply) {
return internalchallenges.ApplyStatus(ctx, c.cmClient, c.fieldManager, challenge)
} else {
return c.cmClient.AcmeV1().Challenges(challenge.Namespace).UpdateStatus(ctx, challenge, metav1.UpdateOptions{})
}
}

58
pkg/controller/acmechallenges/sync_test.go
View File

@ -18,6 +18,7 @@ package acmechallenges
import (
"context"
"errors"
"fmt"
"testing"
@ -88,6 +89,8 @@ func TestSyncHappyPath(t *testing.T) {
)
deletedChallenge := gen.ChallengeFrom(baseChallenge,
gen.SetChallengeDeletionTimestamp(metav1.Now()))
simulatedCleanupError := errors.New("simulated-cleanup-error")
tests := map[string]testT{
"cleanup if the challenge is deleted and remove the finalizer": {
challenge: gen.ChallengeFrom(deletedChallenge,
@ -110,14 +113,6 @@ func TestSyncHappyPath(t *testing.T) {
testIssuerHTTP01Enabled,
},
ExpectedActions: []testpkg.Action{
testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("challenges"),
"status",
gen.DefaultTestNamespace,
gen.ChallengeFrom(deletedChallenge,
gen.SetChallengeProcessing(true),
gen.SetChallengeURL("testurl"),
gen.SetChallengeType(cmacme.ACMEChallengeTypeHTTP01),
))),
testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("challenges"),
gen.DefaultTestNamespace,
gen.ChallengeFrom(deletedChallenge,
@ -129,6 +124,53 @@ func TestSyncHappyPath(t *testing.T) {
},
},
},
"if the challenge is deleted and the cleanup fails, set the reason (and remove the finalizer, which is a bug)": {
challenge: gen.ChallengeFrom(deletedChallenge,
gen.SetChallengeProcessing(true),
gen.SetChallengeURL("testurl"),
gen.SetChallengeType(cmacme.ACMEChallengeTypeHTTP01),
),
httpSolver: &fakeSolver{
fakeCleanUp: func(ctx context.Context, issuer v1.GenericIssuer, ch *cmacme.Challenge) error {
return simulatedCleanupError
},
},
builder: &testpkg.Builder{
CertManagerObjects: []runtime.Object{
gen.ChallengeFrom(deletedChallenge,
gen.SetChallengeProcessing(true),
gen.SetChallengeURL("testurl"),
gen.SetChallengeType(cmacme.ACMEChallengeTypeHTTP01),
),
testIssuerHTTP01Enabled,
},
ExpectedActions: []testpkg.Action{
testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("challenges"),
gen.DefaultTestNamespace,
gen.ChallengeFrom(deletedChallenge,
gen.SetChallengeProcessing(true),
gen.SetChallengeURL("testurl"),
gen.SetChallengeType(cmacme.ACMEChallengeTypeHTTP01),
gen.SetChallengeFinalizers([]string{}),
gen.SetChallengeReason(simulatedCleanupError.Error()),
))),
testpkg.NewAction(
coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("challenges"),
"status",
gen.DefaultTestNamespace,
gen.ChallengeFrom(deletedChallenge,
gen.SetChallengeProcessing(true),
gen.SetChallengeType(cmacme.ACMEChallengeTypeHTTP01),
gen.SetChallengeURL("testurl"),
gen.SetChallengeFinalizers([]string{}),
gen.SetChallengeReason(simulatedCleanupError.Error()),
))),
},
ExpectedEvents: []string{
fmt.Sprintf("Warning CleanUpError Error cleaning up challenge: %s", simulatedCleanupError),
},
},
},
"if finalizer is missing, add it": {
challenge: gen.ChallengeFrom(baseChallenge,
gen.SetChallengeProcessing(true),

145
pkg/controller/acmechallenges/update.go Normal file
View File

@ -0,0 +1,145 @@
/*
Copyright 2022 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 acmechallenges
import (
"context"
stderrors "errors"
"fmt"
"github.com/pkg/errors"
apiequality "k8s.io/apimachinery/pkg/api/equality"
k8sErrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
internalchallenges "github.com/cert-manager/cert-manager/internal/controller/challenges"
"github.com/cert-manager/cert-manager/internal/controller/feature"
cmacme "github.com/cert-manager/cert-manager/pkg/apis/acme/v1"
"github.com/cert-manager/cert-manager/pkg/client/clientset/versioned"
utilfeature "github.com/cert-manager/cert-manager/pkg/util/feature"
"github.com/cert-manager/cert-manager/test/unit/gen"
)
var argumentError = stderrors.New("invalid arguments")
type objectUpdateClient interface {
update(context.Context, *cmacme.Challenge) (*cmacme.Challenge, error)
updateStatus(context.Context, *cmacme.Challenge) (*cmacme.Challenge, error)
}
type objectUpdater interface {
updateObject(context.Context, *cmacme.Challenge, *cmacme.Challenge) error
}
type defaultObjectUpdater struct {
objectUpdateClient
}
func newObjectUpdater(cl versioned.Interface, fieldManager string) objectUpdater {
o := &defaultObjectUpdater{
objectUpdateClient: &objectUpdateClientDefault{cl: cl},
}
if utilfeature.DefaultFeatureGate.Enabled(feature.ServerSideApply) {
o.objectUpdateClient = &objectUpdateClientSSA{
fieldManager: fieldManager,
cl: cl,
}
}
return o
}
// updateObject updates the Finalizers if they have changed and updates the Status if it has changed.
// Finalizers are updated using the Update method while Status is updated using
// the UpdateStatus method.
// Both updates will be attempted, even if one fails, except in the case where
// one of the updates fails with a Not Found error.
// If the any of the API operations results in a Not Found error, updateObject
// will exit without error and the remaining operations will be skipped.
// Only the Finalizers and Status fields may be modified. If there are any
// modifications to new object, outside of the Finalizers and Status fields,
// this function return an error.
func (o *defaultObjectUpdater) updateObject(ctx context.Context, old, new *cmacme.Challenge) error {
if !apiequality.Semantic.DeepEqual(
gen.ChallengeFrom(old, gen.SetChallengeFinalizers(nil), gen.ResetChallengeStatus()),
gen.ChallengeFrom(new, gen.SetChallengeFinalizers(nil), gen.ResetChallengeStatus()),
) {
return errors.WithStack(
fmt.Errorf(
"%w: in updateObject: unexpected differences between old and new: only the finalizers and status fields may be modified",
argumentError,
),
)
}
var updateFunctions []func() (*cmacme.Challenge, error)
if !apiequality.Semantic.DeepEqual(old.Status, new.Status) {
updateFunctions = append(
updateFunctions,
func() (*cmacme.Challenge, error) {
obj, err := o.updateStatus(ctx, new)
return obj, errors.Wrap(err, "when updating the status")
},
)
}
if !apiequality.Semantic.DeepEqual(old.Finalizers, new.Finalizers) {
updateFunctions = append(
updateFunctions,
func() (*cmacme.Challenge, error) {
obj, err := o.update(ctx, new)
return obj, errors.Wrap(err, "when updating the finalizers")
},
)
}
var errors []error
for _, f := range updateFunctions {
if o, err := f(); err != nil {
errors = append(errors, err)
if k8sErrors.IsNotFound(err) {
return nil
}
} else {
new = o
}
}
return utilerrors.NewAggregate(errors)
}
type objectUpdateClientDefault struct {
cl versioned.Interface
}
func (o *objectUpdateClientDefault) update(ctx context.Context, new *cmacme.Challenge) (*cmacme.Challenge, error) {
return o.cl.AcmeV1().Challenges(new.Namespace).Update(ctx, new, metav1.UpdateOptions{})
}
func (o *objectUpdateClientDefault) updateStatus(ctx context.Context, new *cmacme.Challenge) (*cmacme.Challenge, error) {
return o.cl.AcmeV1().Challenges(new.Namespace).UpdateStatus(ctx, new, metav1.UpdateOptions{})
}
type objectUpdateClientSSA struct {
cl versioned.Interface
fieldManager string
}
func (o *objectUpdateClientSSA) update(ctx context.Context, new *cmacme.Challenge) (*cmacme.Challenge, error) {
return internalchallenges.Apply(ctx, o.cl, o.fieldManager, new)
}
func (o *objectUpdateClientSSA) updateStatus(ctx context.Context, new *cmacme.Challenge) (*cmacme.Challenge, error) {
return internalchallenges.ApplyStatus(ctx, o.cl, o.fieldManager, new)
}

182
pkg/controller/acmechallenges/update_test.go Normal file
View File

@ -0,0 +1,182 @@
/*
Copyright 2022 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 acmechallenges
import (
"context"
"errors"
"fmt"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
clienttesting "k8s.io/client-go/testing"
featuretesting "k8s.io/component-base/featuregate/testing"
"github.com/cert-manager/cert-manager/internal/controller/feature"
"github.com/cert-manager/cert-manager/pkg/client/clientset/versioned/fake"
utilfeature "github.com/cert-manager/cert-manager/pkg/util/feature"
"github.com/cert-manager/cert-manager/test/unit/gen"
)
func TestUpdateObjectStandard(t *testing.T) {
runUpdateObjectTests(t)
}
func TestUpdateObjectSSA(t *testing.T) {
t.Skip(
"Server Side Apply cannot be tested because PatchType is not supported by the fake versioned client. See:",
"https://github.com/kubernetes/client-go/issues/970",
"https://github.com/kubernetes/client-go/issues/992",
)
defer featuretesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, feature.ServerSideApply, true)()
runUpdateObjectTests(t)
}
func runUpdateObjectTests(t *testing.T) {
simulatedUpdateError := errors.New("simulated-update-error")
simulatedUpdateStatusError := errors.New("simulated-update-status-error")
tests := []struct {
name string
mods []gen.ChallengeModifier
notFound bool
updateError error
updateStatusError error
errorMessage string
}{
// Modifying the finalizers and any status fields results in both
// finalizers and status being updated.
{
name: "success",
mods: []gen.ChallengeModifier{
gen.SetChallengeFinalizers([]string{"example.com/another-finalizer"}),
gen.SetChallengePresented(true),
},
},
// If the API server responds with a NOT FOUND error, the error is
// ignored. Presumably the object has been deleted since the Sync
// function began executing.
{
name: "not-found",
mods: []gen.ChallengeModifier{
gen.SetChallengePresented(true),
},
notFound: true,
},
// Only the Finalizers and Status fields can be updated. Updates to any
// other fields suggests a programming error an argument error.
{
name: "error-on-non-finalizer-non-status-modifications",
mods: []gen.ChallengeModifier{
gen.SetChallengeDNSName("new-dns-name"),
},
errorMessage: fmt.Sprintf(
"%s: in updateObject: unexpected differences between old and new: only the finalizers and status fields may be modified",
argumentError,
),
},
// If the Update API call fails, that error is returned.
{
name: "update-error-only",
mods: []gen.ChallengeModifier{
gen.SetChallengeFinalizers([]string{"example.com/another-finalizer"}),
},
updateError: simulatedUpdateError,
errorMessage: fmt.Sprintf("when updating the finalizers: %s", simulatedUpdateError),
},
// If the UpdateStatus API call fails, that error is returned.
{
name: "update-status-error-only",
mods: []gen.ChallengeModifier{
gen.SetChallengePresented(true),
},
updateStatusError: simulatedUpdateStatusError,
errorMessage: fmt.Sprintf("when updating the status: %s", simulatedUpdateStatusError),
},
// If both Update and UpdateStatus API calls fail, both errors are returned.
{
name: "all-updates-fail",
mods: []gen.ChallengeModifier{
gen.SetChallengeFinalizers([]string{"example.com/another-finalizer"}),
gen.SetChallengePresented(true),
},
updateError: simulatedUpdateError,
updateStatusError: simulatedUpdateStatusError,
errorMessage: fmt.Sprintf(
"[when updating the status: %s, when updating the finalizers: %s]",
simulatedUpdateStatusError,
simulatedUpdateError,
),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx := context.TODO()
old := gen.Challenge("c1")
new := gen.ChallengeFrom(old, tt.mods...)
objects := []runtime.Object{old}
if tt.notFound {
t.Log("Simulating a situation where the target object has been deleted")
objects = nil
}
cl := fake.NewSimpleClientset(objects...)
if tt.updateError != nil {
cl.PrependReactor("update", "challenges", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) {
t.Log("Simulating a challenge update error")
return true, nil, tt.updateError
})
}
if tt.updateStatusError != nil {
cl.PrependReactor("update", "challenges/status", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) {
t.Log("Simulating a challenge/status update error")
return true, nil, tt.updateStatusError
})
}
updater := newObjectUpdater(cl, "test-fieldmanager")
t.Log("Calling updateObject")
updateObjectErr := updater.updateObject(ctx, old, new)
if tt.errorMessage == "" {
assert.NoError(t, updateObjectErr)
} else {
assert.EqualError(t, updateObjectErr, tt.errorMessage)
}
if len(tt.mods) == 0 {
assert.Empty(t, cl.Actions(), "There should not be any API interactions unless the object was modified")
}
if !tt.notFound {
t.Log("Checking whether the object was updated")
actual, err := cl.AcmeV1().Challenges(old.Namespace).Get(ctx, old.Name, metav1.GetOptions{})
require.NoError(t, err)
if updateObjectErr == nil {
assert.Equal(t, new, actual, "updateObject did not return an error so the object in the API should have been updated")
} else {
if !errors.Is(updateObjectErr, simulatedUpdateError) {
assert.Equal(t, new.Finalizers, actual.Finalizers, "The Update did not fail so the Finalizers of the API object should have been updated")
}
if !errors.Is(updateObjectErr, simulatedUpdateStatusError) {
assert.Equal(t, new.Status, actual.Status, "The UpdateStatus did not fail so the Status of the API object should have been updated")
}
}
}
})
}
}

2
pkg/controller/test/context_builder.go
View File

@ -294,7 +294,7 @@ func (b *Builder) AllActionsExecuted() error {
}
func actionToString(a coretesting.Action) string {
return fmt.Sprintf("%s %q in namespace %s", a.GetVerb(), a.GetResource(), a.GetNamespace())
return fmt.Sprintf("%s %s %q in namespace %s", a.GetVerb(), a.GetSubresource(), a.GetResource(), a.GetNamespace())
}
// Stop will signal the informers to stop watching changes

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

@ -120,3 +120,9 @@ func SetChallengeDeletionTimestamp(ts metav1.Time) ChallengeModifier {
ch.DeletionTimestamp = &ts
}
}
func ResetChallengeStatus() ChallengeModifier {
return func(ch *cmacme.Challenge) {
ch.Status = cmacme.ChallengeStatus{}
}
}