From 2f7c92a18ebb65861857f0369324abeac4c25778 Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Mon, 29 Jul 2024 10:45:10 -0700 Subject: [PATCH 1/2] Test createTargetObject for object whose namespace is mutated --- cmd/clusterctl/client/cluster/mover_test.go | 50 ++++++++++++++++++++- 1 file changed, 48 insertions(+), 2 deletions(-) diff --git a/cmd/clusterctl/client/cluster/mover_test.go b/cmd/clusterctl/client/cluster/mover_test.go index edc3077143db..e6e04287b2f7 100644 --- a/cmd/clusterctl/client/cluster/mover_test.go +++ b/cmd/clusterctl/client/cluster/mover_test.go @@ -1876,7 +1876,6 @@ func Test_objectMoverService_ensureNamespaces(t *testing.T) { expectedNamespaces: []string{"namespace-1", "namespace-2"}, }, { - name: "ensureNamespaces moves namespace-2 to target which already has namespace-1", fields: fields{ objs: cluster2.Objs(), @@ -1956,6 +1955,7 @@ func Test_createTargetObject(t *testing.T) { fromProxy Proxy toProxy Proxy node *node + mutators []ResourceMutatorFunc } tests := []struct { @@ -2075,6 +2075,52 @@ func Test_createTargetObject(t *testing.T) { g.Expect(c.Annotations).To(BeEmpty()) }, }, + { + name: "updates object whose namespace is mutated, if it already exists and the object is not Global/GlobalHierarchy", + args: args{ + fromProxy: test.NewFakeProxy().WithObjs( + &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "ns1", + }, + }, + ), + toProxy: test.NewFakeProxy().WithObjs( + &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "mutatedns1", + Annotations: map[string]string{"foo": "bar"}, + }, + }, + ), + node: &node{ + identity: corev1.ObjectReference{ + Kind: "Cluster", + Namespace: "ns1", + Name: "foo", + APIVersion: "cluster.x-k8s.io/v1beta1", + }, + }, + mutators: []ResourceMutatorFunc{ + func(u *unstructured.Unstructured) error { + return unstructured.SetNestedField(u.Object, + "mutatedns1", + "metadata", "namespace") + }, + }, + }, + want: func(g *WithT, toClient client.Client) { + c := &clusterv1.Cluster{} + key := client.ObjectKey{ + Namespace: "mutatedns1", + Name: "foo", + } + g.Expect(toClient.Get(context.Background(), key, c)).ToNot(HaveOccurred()) + g.Expect(c.Annotations).To(BeEmpty()) + }, + }, { name: "should not update Global objects", args: args{ @@ -2163,7 +2209,7 @@ func Test_createTargetObject(t *testing.T) { fromProxy: tt.args.fromProxy, } - err := mover.createTargetObject(ctx, tt.args.node, tt.args.toProxy, nil, sets.New[string]()) + err := mover.createTargetObject(ctx, tt.args.node, tt.args.toProxy, tt.args.mutators, sets.New[string]()) if tt.wantErr { g.Expect(err).To(HaveOccurred()) return From 88476cd8ea44a238e6ed1a0a68c0139011d639b3 Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Mon, 29 Jul 2024 10:45:31 -0700 Subject: [PATCH 2/2] Fix createTargetObject for object whose namespace is mutated Previously, createTargetObject updated an object using the metadata (including namespace) of the unmutated object, even when the object was mutated. --- cmd/clusterctl/client/cluster/mover.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/cmd/clusterctl/client/cluster/mover.go b/cmd/clusterctl/client/cluster/mover.go index 0232aba2b8b7..f94248040bb8 100644 --- a/cmd/clusterctl/client/cluster/mover.go +++ b/cmd/clusterctl/client/cluster/mover.go @@ -946,12 +946,10 @@ func (o *objectMover) createTargetObject(ctx context.Context, nodeToCreate *node obj := &unstructured.Unstructured{} obj.SetAPIVersion(nodeToCreate.identity.APIVersion) obj.SetKind(nodeToCreate.identity.Kind) - objKey := client.ObjectKey{ - Namespace: nodeToCreate.identity.Namespace, - Name: nodeToCreate.identity.Name, - } + obj.SetName(nodeToCreate.identity.Name) + obj.SetNamespace(nodeToCreate.identity.Namespace) - if err := cFrom.Get(ctx, objKey, obj); err != nil { + if err := cFrom.Get(ctx, client.ObjectKeyFromObject(obj), obj); err != nil { return errors.Wrapf(err, "error reading %q %s/%s", obj.GroupVersionKind(), obj.GetNamespace(), obj.GetName()) } @@ -1006,7 +1004,7 @@ func (o *objectMover) createTargetObject(ctx context.Context, nodeToCreate *node existingTargetObj := &unstructured.Unstructured{} existingTargetObj.SetAPIVersion(obj.GetAPIVersion()) existingTargetObj.SetKind(obj.GetKind()) - if err := cTo.Get(ctx, objKey, existingTargetObj); err != nil { + if err := cTo.Get(ctx, client.ObjectKeyFromObject(obj), existingTargetObj); err != nil { return errors.Wrapf(err, "error reading resource for %q %s/%s", existingTargetObj.GroupVersionKind(), existingTargetObj.GetNamespace(), existingTargetObj.GetName()) }