Skip to content

Commit

Permalink
Merge pull request #24 from dlipovetsky/fix-create-target-object-muta…
Browse files Browse the repository at this point in the history
…ted-d2iq-v1.7.4

🐛 Ensure move uses mutated metadata when updating a target object
  • Loading branch information
dlipovetsky authored Jul 29, 2024
2 parents 55722b7 + 88476cd commit 05da24c
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 8 deletions.
10 changes: 4 additions & 6 deletions cmd/clusterctl/client/cluster/mover.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down Expand Up @@ -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())
}
Expand Down
50 changes: 48 additions & 2 deletions cmd/clusterctl/client/cluster/mover_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -1956,6 +1955,7 @@ func Test_createTargetObject(t *testing.T) {
fromProxy Proxy
toProxy Proxy
node *node
mutators []ResourceMutatorFunc
}

tests := []struct {
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 05da24c

Please sign in to comment.