diff --git a/pkg/reconciler/managed/api.go b/pkg/reconciler/managed/api.go index 7bf6a1514..08c1c9af8 100644 --- a/pkg/reconciler/managed/api.go +++ b/pkg/reconciler/managed/api.go @@ -17,14 +17,20 @@ limitations under the License. package managed import ( + "bytes" "context" "encoding/json" + "maps" + "slices" + "strings" jsonpatch "github.com/evanphx/json-patch" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" corev1 "k8s.io/api/core/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" + kmeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" @@ -50,6 +56,10 @@ const ( errMarshalExisting = "cannot marshal the existing object into JSON" errMarshalResolved = "cannot marshal the object with the resolved references into JSON" errPreparePatch = "cannot prepare the JSON merge patch for the resolved object" + errExtractOwnedFields = "cannot extract the fields owned by the reference resolver" + errUnmarshalOwnedFields = "cannot unmarshal the managedFields entry of the reference resolver" + errMarshalOwnedFields = "cannot marshal the fields owned by the reference resolver into JSON" + errMergeOwnedFields = "cannot merge the resolved references into the fields owned by the reference resolver" errUpdateManagedStatus = "cannot update managed resource status" errResolveReferences = "cannot resolve references" errUpdateCriticalAnnotations = "cannot update critical annotations" @@ -232,6 +242,238 @@ func prepareJSONMerge(existing, resolved runtime.Object) ([]byte, error) { return patch, errors.Wrap(err, errPreparePatch) } +// withOwnedFields returns the supplied server-side apply document extended with +// every field the reference resolver already owns on the existing object. +// +// A server-side apply request is the complete list of fields its field manager +// wants to own: a field the manager owned before but leaves out of the request +// is removed from the object. prepareJSONMerge only yields the fields that +// changed during this resolution, so applying it as is on an object with two or +// more resolved references makes the API server drop the references resolved +// earlier, and the next reconcile resolves those again while dropping this one. +// Carrying the owned fields along keeps the request complete. Fields changed by +// this resolution take precedence over the values recorded on the object. +func withOwnedFields(existing runtime.Object, patch []byte) ([]byte, error) { + entry, ok := ownedFieldsEntry(existing) + if !ok { + return patch, nil + } + + fields := map[string]any{} + if err := json.Unmarshal(entry.FieldsV1.GetRawBytes(), &fields); err != nil { + return nil, errors.Wrap(err, errUnmarshalOwnedFields) + } + + u, err := runtime.DefaultUnstructuredConverter.ToUnstructured(existing) + if err != nil { + return nil, errors.Wrap(err, errExtractOwnedFields) + } + + owned, ok := pickOwned(fields, u) + if !ok { + return patch, nil + } + + oBuff, err := json.Marshal(owned) + if err != nil { + return nil, errors.Wrap(err, errMarshalOwnedFields) + } + + merged, err := jsonpatch.MergePatch(oBuff, patch) + + return merged, errors.Wrap(err, errMergeOwnedFields) +} + +// ownedFieldsEntry returns the managedFields entry recorded for the reference +// resolver's server-side apply operations, if any. +func ownedFieldsEntry(obj runtime.Object) (metav1.ManagedFieldsEntry, bool) { + accessor, err := kmeta.Accessor(obj) + if err != nil { + return metav1.ManagedFieldsEntry{}, false + } + + for _, e := range accessor.GetManagedFields() { + if e.Manager == fieldOwnerAPISimpleRefResolver && e.Operation == metav1.ManagedFieldsOperationApply && e.Subresource == "" && e.FieldsV1 != nil { + return e, true + } + } + + return metav1.ManagedFieldsEntry{}, false +} + +// pickOwned returns the part of the supplied unstructured value that the +// supplied FieldsV1 set covers. It follows the FieldsV1 encoding: "f:name" +// descends into a field, "k:{...}" selects an associative list item by its key +// fields, "v:..." selects a set list item by value, "i:n" selects a list item by +// index, "." marks the node itself and an empty set covers the whole value. +// +// Ownership of a list item is carried as that item alone, together with its key +// fields, so that the apply document neither claims nor resurrects items owned +// by other field managers. Paths the value no longer has are skipped. +func pickOwned(fields map[string]any, value any) (any, bool) { + if isLeaf(fields) { + return value, true + } + + switch v := value.(type) { + case map[string]any: + return pickOwnedMap(fields, v) + case []any: + return pickOwnedList(fields, v) + default: + return value, true + } +} + +func isLeaf(fields map[string]any) bool { + for k := range fields { + if k != "." { + return false + } + } + + return true +} + +func pickOwnedMap(fields map[string]any, value map[string]any) (any, bool) { + out := map[string]any{} + + for k, sub := range fields { + name, ok := strings.CutPrefix(k, "f:") + if !ok { + continue + } + + child, ok := value[name] + if !ok { + continue + } + + subFields, _ := sub.(map[string]any) + + picked, ok := pickOwned(subFields, child) + if ok { + out[name] = picked + } + } + + return out, len(out) > 0 +} + +func pickOwnedList(fields map[string]any, value []any) (any, bool) { + out := []any{} + + // Walk the set in a stable order so that equal ownership always yields + // the same apply document. + for _, k := range slices.Sorted(maps.Keys(fields)) { + subFields, _ := fields[k].(map[string]any) + + switch { + case strings.HasPrefix(k, "k:"): + out = append(out, pickKeyedItems(k[2:], subFields, value)...) + case strings.HasPrefix(k, "v:"): + out = append(out, pickSetItems(k[2:], value)...) + case strings.HasPrefix(k, "i:"): + out = append(out, pickIndexedItem(k[2:], subFields, value)...) + } + } + + return out, len(out) > 0 +} + +// pickKeyedItems returns the owned parts of the associative list items whose +// key fields match the supplied JSON key, each carrying its key fields. +func pickKeyedItems(rawKey string, fields map[string]any, value []any) []any { + key := map[string]any{} + if err := json.Unmarshal([]byte(rawKey), &key); err != nil { + return nil + } + + out := []any{} + + for _, item := range value { + m, ok := item.(map[string]any) + if !ok || !hasKeyFields(m, key) { + continue + } + + picked, ok := pickOwned(fields, m) + if !ok { + continue + } + + if pm, ok := picked.(map[string]any); ok { + maps.Copy(pm, key) + } + + out = append(out, picked) + } + + return out +} + +// pickSetItems returns the set list items equal to the supplied JSON value. +func pickSetItems(rawValue string, value []any) []any { + var want any + if err := json.Unmarshal([]byte(rawValue), &want); err != nil { + return nil + } + + out := []any{} + + for _, item := range value { + if sameJSON(item, want) { + out = append(out, item) + } + } + + return out +} + +// pickIndexedItem returns the owned part of the list item at the supplied +// index, if the list still has one. +func pickIndexedItem(rawIndex string, fields map[string]any, value []any) []any { + var i int + if err := json.Unmarshal([]byte(rawIndex), &i); err != nil || i < 0 || i >= len(value) { + return nil + } + + picked, ok := pickOwned(fields, value[i]) + if !ok { + return nil + } + + return []any{picked} +} + +func hasKeyFields(item, key map[string]any) bool { + for k, want := range key { + got, ok := item[k] + if !ok || !sameJSON(got, want) { + return false + } + } + + return true +} + +// sameJSON compares two values by their JSON encoding, so that numbers decoded +// from FieldsV1 compare equal to the numbers held by the unstructured object +// regardless of their Go type. +func sameJSON(a, b any) bool { + ab, err := json.Marshal(a) + if err != nil { + return false + } + + bb, err := json.Marshal(b) + if err != nil { + return false + } + + return bytes.Equal(ab, bb) +} + // ResolveReferences of the supplied managed resource by calling its // ResolveReferences method, if any. func (a *APISimpleReferenceResolver) ResolveReferences(ctx context.Context, mg resource.Managed) error { @@ -259,6 +501,11 @@ func (a *APISimpleReferenceResolver) ResolveReferences(ctx context.Context, mg r return err } + patch, err = withOwnedFields(existing, patch) + if err != nil { + return err + } + return errors.Wrap(a.client.Patch(ctx, mg, client.RawPatch(types.ApplyPatchType, patch), client.FieldOwner(fieldOwnerAPISimpleRefResolver), client.ForceOwnership), errPatchManaged) } diff --git a/pkg/reconciler/managed/api_test.go b/pkg/reconciler/managed/api_test.go index c1d5952fd..9338cbd5b 100644 --- a/pkg/reconciler/managed/api_test.go +++ b/pkg/reconciler/managed/api_test.go @@ -26,6 +26,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/crossplane/crossplane-runtime/v2/pkg/errors" @@ -388,11 +389,56 @@ func (r *mockSimpleReferencer) Equal(s *mockSimpleReferencer) bool { return cmp.Equal(r.Managed, s.Managed) } +// mockStructuredReferencer embeds the fake managed resource instead of wrapping +// it, so that it marshals to the managed resource's own JSON shape. The field +// ownership tests rely on that shape to address fields from managedFields. +type mockStructuredReferencer struct { + fake.LegacyManaged + + MockResolveReferences func(context.Context, client.Reader) error `json:"-"` +} + +func (r *mockStructuredReferencer) ResolveReferences(ctx context.Context, c client.Reader) error { + return r.MockResolveReferences(ctx, c) +} + +func (r *mockStructuredReferencer) DeepCopyObject() runtime.Object { + out := *r + out.LegacyManaged = *r.LegacyManaged.DeepCopyObject().(*fake.LegacyManaged) + + return &out +} + +func (r *mockStructuredReferencer) Equal(s *mockStructuredReferencer) bool { + return cmp.Equal(r.LegacyManaged, s.LegacyManaged) +} + +func ownedBy(manager string, op metav1.ManagedFieldsOperationType, fieldsV1 string) []metav1.ManagedFieldsEntry { + return []metav1.ManagedFieldsEntry{{ + Manager: manager, + Operation: op, + FieldsType: "FieldsV1", + FieldsV1: &metav1.FieldsV1{Raw: []byte(fieldsV1)}, + }} +} + func TestResolveReferences(t *testing.T) { errBoom := errors.New("boom") different := &fake.LegacyManaged{} + owned := &mockStructuredReferencer{ + LegacyManaged: fake.LegacyManaged{ObjectMeta: metav1.ObjectMeta{ + Name: "owned", + Annotations: map[string]string{"resolved-a": "1"}, + ManagedFields: ownedBy(fieldOwnerAPISimpleRefResolver, metav1.ManagedFieldsOperationApply, `{"f:annotations":{"f:resolved-a":{}}}`), + }}, + } + owned.MockResolveReferences = func(context.Context, client.Reader) error { + owned.Annotations["resolved-b"] = "2" + return nil + } + type args struct { ctx context.Context mg resource.Managed @@ -461,6 +507,32 @@ func TestResolveReferences(t *testing.T) { }, want: nil, }, + "PatchKeepsOwnedFields": { + reason: "Should keep the fields the resolver already owns in the server-side apply document next to the newly resolved ones.", + c: &test.MockClient{ + MockPatch: func(_ context.Context, obj client.Object, patch client.Patch, _ ...client.PatchOption) error { + if patch.Type() != types.ApplyPatchType { + return errors.Errorf("unexpected patch type %q", patch.Type()) + } + + got, err := patch.Data(obj) + if err != nil { + return err + } + + if diff := cmp.Diff(`{"annotations":{"resolved-a":"1","resolved-b":"2"}}`, string(got)); diff != "" { + return errors.Errorf("unexpected patch: -want, +got:\n%s", diff) + } + + return nil + }, + }, + args: args{ + ctx: context.Background(), + mg: owned, + }, + want: nil, + }, "PatchError": { reason: "Should return an error when the managed resource cannot be updated.", c: &test.MockClient{ @@ -538,6 +610,170 @@ func TestPrepareJSONMerge(t *testing.T) { } } +func TestWithOwnedFields(t *testing.T) { + type args struct { + existing runtime.Object + patch string + } + + type want struct { + patch string + err error + } + + cases := map[string]struct { + reason string + args args + want want + }{ + "NoManagedFields": { + reason: "Should return the patch unchanged when the object records no field ownership.", + args: args{ + existing: &fake.LegacyManaged{}, + patch: `{"annotations":{"b":"2"}}`, + }, + want: want{patch: `{"annotations":{"b":"2"}}`}, + }, + "OtherManager": { + reason: "Should ignore fields owned by other field managers.", + args: args{ + existing: &fake.LegacyManaged{ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"a": "1"}, + ManagedFields: ownedBy("someone-else", metav1.ManagedFieldsOperationApply, `{"f:annotations":{"f:a":{}}}`), + }}, + patch: `{"annotations":{"b":"2"}}`, + }, + want: want{patch: `{"annotations":{"b":"2"}}`}, + }, + "UpdateOperationIgnored": { + reason: "Should only consider fields the resolver owns through an apply operation.", + args: args{ + existing: &fake.LegacyManaged{ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"a": "1"}, + ManagedFields: ownedBy(fieldOwnerAPISimpleRefResolver, metav1.ManagedFieldsOperationUpdate, `{"f:annotations":{"f:a":{}}}`), + }}, + patch: `{"annotations":{"b":"2"}}`, + }, + want: want{patch: `{"annotations":{"b":"2"}}`}, + }, + "OwnedFieldsKept": { + reason: "Should carry the fields the resolver already owns along with the newly resolved ones.", + args: args{ + existing: &fake.LegacyManaged{ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"a": "1", "unowned": "x"}, + ManagedFields: ownedBy(fieldOwnerAPISimpleRefResolver, metav1.ManagedFieldsOperationApply, `{"f:annotations":{"f:a":{}}}`), + }}, + patch: `{"annotations":{"b":"2"}}`, + }, + want: want{patch: `{"annotations":{"a":"1","b":"2"}}`}, + }, + "ResolvedValueWins": { + reason: "Should prefer the value resolved now over the value recorded on the object for a field the resolver owns.", + args: args{ + existing: &fake.LegacyManaged{ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"a": "1"}, + ManagedFields: ownedBy(fieldOwnerAPISimpleRefResolver, metav1.ManagedFieldsOperationApply, `{"f:annotations":{"f:a":{}}}`), + }}, + patch: `{"annotations":{"a":"9"}}`, + }, + want: want{patch: `{"annotations":{"a":"9"}}`}, + }, + "OwnedListKept": { + reason: "Should carry a list the resolver owns as a whole.", + args: args{ + existing: &fake.LegacyManaged{ObjectMeta: metav1.ObjectMeta{ + Finalizers: []string{"a", "b"}, + ManagedFields: ownedBy(fieldOwnerAPISimpleRefResolver, metav1.ManagedFieldsOperationApply, `{"f:finalizers":{}}`), + }}, + patch: `{"annotations":{"b":"2"}}`, + }, + want: want{patch: `{"annotations":{"b":"2"},"finalizers":["a","b"]}`}, + }, + "OwnedKeyedListItem": { + reason: "Should carry only the associative list item the resolver owns, with its key fields, leaving the other items to their owners.", + args: args{ + existing: &fake.LegacyManaged{ObjectMeta: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "v1", Kind: "A", Name: "a", UID: "1"}, + {APIVersion: "v1", Kind: "B", Name: "b", UID: "2"}, + }, + ManagedFields: ownedBy(fieldOwnerAPISimpleRefResolver, metav1.ManagedFieldsOperationApply, `{"f:ownerReferences":{"k:{\"uid\":\"1\"}":{".":{},"f:name":{}}}}`), + }}, + patch: `{"annotations":{"b":"2"}}`, + }, + want: want{patch: `{"annotations":{"b":"2"},"ownerReferences":[{"name":"a","uid":"1"}]}`}, + }, + "OwnedKeyedListItemGone": { + reason: "Should skip an associative list item the resolver owns but the object no longer has.", + args: args{ + existing: &fake.LegacyManaged{ObjectMeta: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{{APIVersion: "v1", Kind: "A", Name: "a", UID: "1"}}, + ManagedFields: ownedBy(fieldOwnerAPISimpleRefResolver, metav1.ManagedFieldsOperationApply, `{"f:ownerReferences":{"k:{\"uid\":\"9\"}":{".":{},"f:name":{}}}}`), + }}, + patch: `{"annotations":{"b":"2"}}`, + }, + want: want{patch: `{"annotations":{"b":"2"}}`}, + }, + "OwnedSetListItem": { + reason: "Should carry only the set list item the resolver owns.", + args: args{ + existing: &fake.LegacyManaged{ObjectMeta: metav1.ObjectMeta{ + Finalizers: []string{"a", "b"}, + ManagedFields: ownedBy(fieldOwnerAPISimpleRefResolver, metav1.ManagedFieldsOperationApply, `{"f:finalizers":{"v:\"b\"":{}}}`), + }}, + patch: `{}`, + }, + want: want{patch: `{"finalizers":["b"]}`}, + }, + "OwnedIndexedListItem": { + reason: "Should carry only the list item at the index the resolver owns.", + args: args{ + existing: &fake.LegacyManaged{ObjectMeta: metav1.ObjectMeta{ + Finalizers: []string{"a", "b"}, + ManagedFields: ownedBy(fieldOwnerAPISimpleRefResolver, metav1.ManagedFieldsOperationApply, `{"f:finalizers":{"i:0":{}}}`), + }}, + patch: `{}`, + }, + want: want{patch: `{"finalizers":["a"]}`}, + }, + "OwnedMapItself": { + reason: "Should carry a map as a whole when the resolver owns the map itself and nothing beneath it.", + args: args{ + existing: &fake.LegacyManaged{ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"a": "1"}, + ManagedFields: ownedBy(fieldOwnerAPISimpleRefResolver, metav1.ManagedFieldsOperationApply, `{"f:annotations":{".":{}}}`), + }}, + patch: `{}`, + }, + want: want{patch: `{"annotations":{"a":"1"}}`}, + }, + "OwnedFieldMissingFromObject": { + reason: "Should tolerate ownership of a field the object no longer has.", + args: args{ + existing: &fake.LegacyManaged{ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"a": "1"}, + ManagedFields: ownedBy(fieldOwnerAPISimpleRefResolver, metav1.ManagedFieldsOperationApply, `{"f:annotations":{"f:gone":{}},"f:labels":{"f:gone":{}}}`), + }}, + patch: `{"annotations":{"b":"2"}}`, + }, + want: want{patch: `{"annotations":{"b":"2"}}`}, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + patch, err := withOwnedFields(tc.args.existing, []byte(tc.args.patch)) + if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { + t.Errorf("\n%s\nwithOwnedFields(...): -wantErr, +gotErr:\n%s", tc.reason, diff) + } + + if diff := cmp.Diff(tc.want.patch, string(patch)); diff != "" { + t.Errorf("\n%s\nwithOwnedFields(...): -want, +got:\n%s", tc.reason, diff) + } + }) + } +} + func TestRetryingCriticalAnnotationUpdater(t *testing.T) { errBoom := errors.New("boom")