Update with object that is gotten in APIUpdatingApplicator

This changes APIUpdatingApplicator to make its Update call
with the object it gets rather than the one passed so that
the default behavior is a no-op rather than a guaranteed
error on mismatched resource versions.

Signed-off-by: hasheddan <georgedanielmangum@gmail.com>

pkg/resource/api.go

@@ -123,7 +123,8 @@ func NewAPIPatchingApplicator(c client.Client) *APIPatchingApplicator {
 }
 
 // Apply changes to the supplied object. The object will be created if it does
-// not exist, or patched if it does.
+// not exist, or patched if it does. If the object does it exist it will always
+// be patched, regardless of resource version.
 func (a *APIPatchingApplicator) Apply(ctx context.Context, o runtime.Object, ao ...ApplyOption) error {
 	m, ok := o.(metav1.Object)
 	if !ok {
@@ -173,7 +174,8 @@ func NewAPIUpdatingApplicator(c client.Client) *APIUpdatingApplicator {
 }
 
 // Apply changes to the supplied object. The object will be created if it does
-// not exist, or updated if it does.
+// not exist, or updated if it does. If the object does exist and no
+// ApplyOptions are passed, the update will be a no-op.
 func (a *APIUpdatingApplicator) Apply(ctx context.Context, o runtime.Object, ao ...ApplyOption) error {
 	m, ok := o.(metav1.Object)
 	if !ok {
@@ -201,7 +203,7 @@ func (a *APIUpdatingApplicator) Apply(ctx context.Context, o runtime.Object, ao
 		}
 	}
 
-	return errors.Wrap(a.client.Update(ctx, o), "cannot update object")
+	return errors.Wrap(a.client.Update(ctx, current), "cannot update object")
 }
 
 // An APIFinalizer adds and removes finalizers to and from a resource.

pkg/resource/api_test.go

@@ -18,6 +18,7 @@ package resource
 
 import (
 	"context"
+	"fmt"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -230,8 +231,8 @@ func TestPropagateConnection(t *testing.T) {
 
 func TestAPIPatchingApplicator(t *testing.T) {
 	errBoom := errors.New("boom")
-	named := &object{}
-	named.SetName("barry")
+	desired := &object{}
+	desired.SetName("desired")
 
 	type args struct {
 		ctx context.Context
@@ -317,15 +318,15 @@ func TestAPIPatchingApplicator(t *testing.T) {
 			c: &test.MockClient{
 				MockGet: test.NewMockGetFn(kerrors.NewNotFound(schema.GroupResource{}, "")),
 				MockCreate: test.NewMockCreateFn(nil, func(o runtime.Object) error {
-					*o.(*object) = *named
+					*o.(*object) = *desired
 					return nil
 				}),
 			},
 			args: args{
-				o: &object{},
+				o: desired,
 			},
 			want: want{
-				o: named,
+				o: desired,
 			},
 		},
 		"Patched": {
@@ -333,15 +334,15 @@ func TestAPIPatchingApplicator(t *testing.T) {
 			c: &test.MockClient{
 				MockGet: test.NewMockGetFn(nil),
 				MockPatch: test.NewMockPatchFn(nil, func(o runtime.Object) error {
-					*o.(*object) = *named
+					*o.(*object) = *desired
 					return nil
 				}),
 			},
 			args: args{
-				o: &object{},
+				o: desired,
 			},
 			want: want{
-				o: named,
+				o: desired,
 			},
 		},
 	}
@@ -362,8 +363,10 @@ func TestAPIPatchingApplicator(t *testing.T) {
 
 func TestAPIUpdatingApplicator(t *testing.T) {
 	errBoom := errors.New("boom")
-	named := &object{}
-	named.SetName("barry")
+	desired := &object{}
+	desired.SetName("desired")
+	current := &object{}
+	current.SetName("current")
 
 	type args struct {
 		ctx context.Context
@@ -449,31 +452,39 @@ func TestAPIUpdatingApplicator(t *testing.T) {
 			c: &test.MockClient{
 				MockGet: test.NewMockGetFn(kerrors.NewNotFound(schema.GroupResource{}, "")),
 				MockCreate: test.NewMockCreateFn(nil, func(o runtime.Object) error {
-					*o.(*object) = *named
+					*o.(*object) = *desired
 					return nil
 				}),
 			},
 			args: args{
-				o: &object{},
+				o: desired,
 			},
 			want: want{
-				o: named,
+				o: desired,
 			},
 		},
 		"Updated": {
-			reason: "No error should be returned if we successfully update an existing object",
+			reason: "No error should be returned if we successfully update an existing object. If no ApplyOption is passed the existing should not be modified",
 			c: &test.MockClient{
-				MockGet: test.NewMockGetFn(nil),
+				MockGet: test.NewMockGetFn(nil, func(o runtime.Object) error {
+					*o.(*object) = *current
+					fmt.Println(current.Name)
+					return nil
+				}),
 				MockUpdate: test.NewMockUpdateFn(nil, func(o runtime.Object) error {
-					*o.(*object) = *named
+					fmt.Println(current.Name)
+					fmt.Println(o.(*object).Name)
+					if diff := cmp.Diff(*current, *o.(*object)); diff != "" {
+						t.Errorf("r: -want, +got:\n%s", diff)
+					}
 					return nil
 				}),
 			},
 			args: args{
-				o: &object{},
+				o: desired,
 			},
 			want: want{
-				o: named,
+				o: desired,
 			},
 		},
 	}