From b9525af464a0fe4bf001dc42cd0b8e6f2c4d91b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B8=E5=8F=B8?= Date: Mon, 30 Dec 2024 12:03:38 +0800 Subject: [PATCH 1/2] fix nilpointer panic when decode addlitional item failed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: 司司 --- pkg/restore/restore.go | 1 + pkg/restore/restore_test.go | 38 +++++++++++++++++++++++++++++++++++-- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/pkg/restore/restore.go b/pkg/restore/restore.go index 619db5b87e..4f88104edf 100644 --- a/pkg/restore/restore.go +++ b/pkg/restore/restore.go @@ -1388,6 +1388,7 @@ func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupReso additionalObj, err := archive.Unmarshal(ctx.fileSystem, itemPath) if err != nil { errs.Add(namespace, errors.Wrapf(err, "error restoring additional item %s", additionalResourceID)) + continue } additionalItemNamespace := additionalItem.Namespace diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go index f3da852023..49a4805cc4 100644 --- a/pkg/restore/restore_test.go +++ b/pkg/restore/restore_test.go @@ -955,6 +955,7 @@ func TestInvalidTarballContents(t *testing.T) { backup *velerov1api.Backup apiResources []*test.APIResource tarball io.Reader + actions []riav2.RestoreItemAction want map[*test.APIResource][]string wantErrs Result wantWarnings Result @@ -991,6 +992,39 @@ func TestInvalidTarballContents(t *testing.T) { }, }, }, + { + name: "invalid JSON for additional item is reported as an error and restore continues", + restore: defaultRestore().IncludedNamespaces("ns-1").Result(), + backup: defaultBackup().Result(), + tarball: test.NewTarWriter(t). + AddItems("pods", builder.ForPod("ns-1", "pod-1").Result()). + Add("resources/persistentvolumes/cluster/pv-1.json", []byte("invalid JSON")). + Done(), + apiResources: []*test.APIResource{ + test.Pods(), + test.PVs(), + }, + actions: []riav2.RestoreItemAction{ + &pluggableAction{ + executeFunc: func(input *velero.RestoreItemActionExecuteInput) (*velero.RestoreItemActionExecuteOutput, error) { + return &velero.RestoreItemActionExecuteOutput{ + UpdatedItem: input.Item, + AdditionalItems: []velero.ResourceIdentifier{ + {GroupResource: kuberesource.PersistentVolumes, Name: "pv-1"}, + }, + }, nil + }, + }, + }, + want: map[*test.APIResource][]string{ + test.Pods(): {"ns-1/pod-1"}, + }, + wantErrs: Result{ + Namespaces: map[string][]string{ + "ns-1": {"error restoring additional item persistentvolumes/pv-1"}, + }, + }, + }, } for _, tc := range tests { @@ -1012,8 +1046,8 @@ func TestInvalidTarballContents(t *testing.T) { } warnings, errs := h.restorer.Restore( data, - nil, // restoreItemActions - nil, // volume snapshotter getter + tc.actions, // restoreItemActions + nil, // volume snapshotter getter ) assertWantErrsOrWarnings(t, tc.wantWarnings, warnings) assertWantErrsOrWarnings(t, tc.wantErrs, errs) From 24860e4d50192c47f70433da58d07ba5799b48e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B8=E5=8F=B8?= Date: Mon, 30 Dec 2024 16:22:17 +0800 Subject: [PATCH 2/2] add warning logs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: 司司 --- pkg/restore/restore.go | 8 +++++++- pkg/restore/restore_test.go | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/pkg/restore/restore.go b/pkg/restore/restore.go index 4f88104edf..d67da67f81 100644 --- a/pkg/restore/restore.go +++ b/pkg/restore/restore.go @@ -1387,7 +1387,13 @@ func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupReso additionalResourceID := getResourceID(additionalItem.GroupResource, additionalItem.Namespace, additionalItem.Name) additionalObj, err := archive.Unmarshal(ctx.fileSystem, itemPath) if err != nil { - errs.Add(namespace, errors.Wrapf(err, "error restoring additional item %s", additionalResourceID)) + ctx.log.WithError(err).WithFields(logrus.Fields{ + "additionalResource": additionalItem.GroupResource.String(), + "additionalResourceNamespace": additionalItem.Namespace, + "additionalResourceName": additionalItem.Name, + }).Warn("Could not unmarshal additional item") + errs.Add(namespace, errors.Wrapf(err, "Could not unmarshal additional item: %s", additionalResourceID)) + continue } diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go index 49a4805cc4..1c87d7fb0d 100644 --- a/pkg/restore/restore_test.go +++ b/pkg/restore/restore_test.go @@ -1021,7 +1021,7 @@ func TestInvalidTarballContents(t *testing.T) { }, wantErrs: Result{ Namespaces: map[string][]string{ - "ns-1": {"error restoring additional item persistentvolumes/pv-1"}, + "ns-1": {"Could not unmarshal additional item: persistentvolumes/pv-1"}, }, }, },