From bd9563396776e3ac8835d541fd14f1e4242a2663 Mon Sep 17 00:00:00 2001 From: Shubham Pampattiwar Date: Tue, 28 Jul 2026 12:22:36 -0700 Subject: [PATCH] Add skip log and improve test coverage - Log when SkipDefaultResourceModifier skips the default modifier - Add test for unsupported ResourceModifier Kind (warns, does not apply default) - Add test for default ConfigMap with invalid rules (validation failure is non-fatal) - loadResourceModifierConfigMap now at 100% coverage Signed-off-by: Shubham Pampattiwar --- pkg/controller/restore_controller.go | 8 ++++-- pkg/controller/restore_controller_test.go | 32 +++++++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/pkg/controller/restore_controller.go b/pkg/controller/restore_controller.go index a7f0f7429..e4eb68144 100644 --- a/pkg/controller/restore_controller.go +++ b/pkg/controller/restore_controller.go @@ -444,8 +444,12 @@ func (r *restoreReconciler) validateAndComplete(ctx context.Context, restore *ap } else { r.logger.Warnf("Unsupported resource modifier kind %q, only %q is supported", restore.Spec.ResourceModifier.Kind, resourcemodifiers.ConfigmapRefType) } - } else if r.defaultResourceModifierConfigMap != "" && !boolptr.IsSetToTrue(restore.Spec.SkipDefaultResourceModifier) { - resourceModifiers = r.loadResourceModifierConfigMap(ctx, restore, r.defaultResourceModifierConfigMap, true) + } else if r.defaultResourceModifierConfigMap != "" { + if boolptr.IsSetToTrue(restore.Spec.SkipDefaultResourceModifier) { + r.logger.Infof("Skipping default resource modifier configmap %s/%s as SkipDefaultResourceModifier is set", restore.Namespace, r.defaultResourceModifierConfigMap) + } else { + resourceModifiers = r.loadResourceModifierConfigMap(ctx, restore, r.defaultResourceModifierConfigMap, true) + } } return info, resourceModifiers, restoreResPolicies diff --git a/pkg/controller/restore_controller_test.go b/pkg/controller/restore_controller_test.go index 26c607efe..738ad43db 100644 --- a/pkg/controller/restore_controller_test.go +++ b/pkg/controller/restore_controller_test.go @@ -1260,6 +1260,38 @@ func TestValidateAndCompleteWithDefaultResourceModifier(t *testing.T) { assert.Nil(t, rm) assert.Empty(t, restore.Status.ValidationErrors) }) + + t.Run("unsupported resource modifier kind does not apply default", func(t *testing.T) { + r := setupReconciler(t, "default-rm") + require.NoError(t, r.kbClient.Create(t.Context(), &corev1api.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "default-rm", Namespace: velerov1api.DefaultNamespace}, + Data: validCMData, + })) + + restore := newRestore("", nil) + restore.Spec.ResourceModifier = &corev1api.TypedLocalObjectReference{ + Kind: "Secret", + Name: "some-secret", + } + _, rm, _ := r.validateAndComplete(t.Context(), restore) + assert.Nil(t, rm) + assert.Empty(t, restore.Status.ValidationErrors) + }) + + t.Run("default modifier validation failure is non-fatal", func(t *testing.T) { + r := setupReconciler(t, "invalid-validation") + require.NoError(t, r.kbClient.Create(t.Context(), &corev1api.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "invalid-validation", Namespace: velerov1api.DefaultNamespace}, + Data: map[string]string{ + "modifiers.yaml": "version: v1\nresourceModifierRules:\n- conditions:\n groupResource: pods\n patches:\n - operation: invalid\n path: \"/spec\"\n value: \"test\"\n", + }, + })) + + restore := newRestore("", nil) + _, rm, _ := r.validateAndComplete(t.Context(), restore) + assert.Nil(t, rm) + assert.Empty(t, restore.Status.ValidationErrors) + }) } func TestBackupXorScheduleProvided(t *testing.T) {