mirrors/velero
1
0
Fork 0
mirror of https://github.com/vmware-tanzu/velero.git synced 2026-08-15 19:56:06 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Implement default resource modifier in restore controller

Browse Source 
Thread DefaultResourceModifierConfigMap from server config through to
restoreReconciler. Refactor validateAndComplete to use a shared
loadResourceModifierConfigMap helper that handles both default and
per-restore ConfigMap loading.

Precedence: per-restore modifier takes exclusive precedence over the
default. Default ConfigMap errors are non-fatal (warn and proceed).
SkipDefaultResourceModifier opt-out is respected.

Includes unit tests covering: default-only, per-restore override,
skip flag, missing default (non-fatal), missing per-restore (fatal),
and no modifier configured.

Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
This commit is contained in:
Shubham Pampattiwar
2026-08-03 13:19:47 -07:00
parent 70e70f14e2
commit 8ef8ab3b1d
3 changed files with 200 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/controller/restore_controller.go
+60 -19
View File
@@ -55,6 +55,7 @@ import (
"github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt"
"github.com/vmware-tanzu/velero/pkg/plugin/framework"
pkgrestore "github.com/vmware-tanzu/velero/pkg/restore"
"github.com/vmware-tanzu/velero/pkg/util/boolptr"
"github.com/vmware-tanzu/velero/pkg/util/collections"
kubeutil "github.com/vmware-tanzu/velero/pkg/util/kube"
"github.com/vmware-tanzu/velero/pkg/util/logging"
@@ -109,10 +110,11 @@ type restoreReconciler struct {
defaultItemOperationTimeout time.Duration
disableInformerCache bool
newPluginManager func(logger logrus.FieldLogger) clientmgmt.Manager
backupStoreGetter persistence.ObjectBackupStoreGetter
globalCrClient client.Client
resourceTimeout time.Duration
newPluginManager func(logger logrus.FieldLogger) clientmgmt.Manager
backupStoreGetter persistence.ObjectBackupStoreGetter
globalCrClient client.Client
resourceTimeout time.Duration
defaultResourceModifierConfigMap string
}
type backupInfo struct {
@@ -135,6 +137,7 @@ func NewRestoreReconciler(
disableInformerCache bool,
globalCrClient client.Client,
resourceTimeout time.Duration,
defaultResourceModifierConfigMap string,
) *restoreReconciler {
r := &restoreReconciler{
ctx: ctx,
@@ -154,8 +157,9 @@ func NewRestoreReconciler(
newPluginManager: newPluginManager,
backupStoreGetter: backupStoreGetter,
globalCrClient: globalCrClient,
resourceTimeout: resourceTimeout,
globalCrClient: globalCrClient,
resourceTimeout: resourceTimeout,
defaultResourceModifierConfigMap: defaultResourceModifierConfigMap,
}
// Move the periodical backup and restore metrics computing logic from controllers to here.
@@ -432,26 +436,63 @@ func (r *restoreReconciler) validateAndComplete(ctx context.Context, restore *ap
var resourceModifiers *resourcemodifiers.ResourceModifiers
if restore.Spec.ResourceModifier != nil && strings.EqualFold(restore.Spec.ResourceModifier.Kind, resourcemodifiers.ConfigmapRefType) {
ResourceModifierConfigMap := &corev1api.ConfigMap{}
err := r.kbClient.Get(ctx, client.ObjectKey{Namespace: restore.Namespace, Name: restore.Spec.ResourceModifier.Name}, ResourceModifierConfigMap)
if err != nil {
restore.Status.ValidationErrors = append(restore.Status.ValidationErrors, fmt.Sprintf("failed to get resource modifiers configmap %s/%s", restore.Namespace, restore.Spec.ResourceModifier.Name))
resourceModifiers = r.loadResourceModifierConfigMap(ctx, restore, restore.Spec.ResourceModifier.Name, false)
if resourceModifiers == nil && len(restore.Status.ValidationErrors) > 0 {
return backupInfo{}, nil, nil
}
resourceModifiers, err = resourcemodifiers.GetResourceModifiersFromConfig(ResourceModifierConfigMap)
if err != nil {
restore.Status.ValidationErrors = append(restore.Status.ValidationErrors, errors.Wrapf(err, "Error in parsing resource modifiers provided in configmap %s/%s", restore.Namespace, restore.Spec.ResourceModifier.Name).Error())
return backupInfo{}, nil, nil
} else if err = resourceModifiers.Validate(); err != nil {
restore.Status.ValidationErrors = append(restore.Status.ValidationErrors, errors.Wrapf(err, "Validation error in resource modifiers provided in configmap %s/%s", restore.Namespace, restore.Spec.ResourceModifier.Name).Error())
return backupInfo{}, nil, nil
}
r.logger.Infof("Retrieved Resource modifiers provided in configmap %s/%s", restore.Namespace, restore.Spec.ResourceModifier.Name)
} else if r.defaultResourceModifierConfigMap != "" && !boolptr.IsSetToTrue(restore.Spec.SkipDefaultResourceModifier) {
resourceModifiers = r.loadResourceModifierConfigMap(ctx, restore, r.defaultResourceModifierConfigMap, true)
}
return info, resourceModifiers, restoreResPolicies
}
// loadResourceModifierConfigMap loads and validates a resource modifier ConfigMap.
// When isDefault is true, errors are non-fatal (logged as warnings, returns nil).
// When isDefault is false, errors are added to restore.Status.ValidationErrors.
func (r *restoreReconciler) loadResourceModifierConfigMap(
ctx context.Context, restore *api.Restore, cmName string, isDefault bool,
) *resourcemodifiers.ResourceModifiers {
cm := &corev1api.ConfigMap{}
if err := r.kbClient.Get(ctx, client.ObjectKey{Namespace: restore.Namespace, Name: cmName}, cm); err != nil {
if isDefault {
r.logger.WithError(err).Warnf("Failed to retrieve default resource modifier configmap %s/%s, skipping", restore.Namespace, cmName)
return nil
}
restore.Status.ValidationErrors = append(restore.Status.ValidationErrors,
fmt.Sprintf("failed to get resource modifiers configmap %s/%s", restore.Namespace, cmName))
return nil
}
modifiers, err := resourcemodifiers.GetResourceModifiersFromConfig(cm)
if err != nil {
if isDefault {
r.logger.WithError(err).Warnf("Error parsing default resource modifier configmap %s/%s, skipping", restore.Namespace, cmName)
return nil
}
restore.Status.ValidationErrors = append(restore.Status.ValidationErrors,
errors.Wrapf(err, "Error in parsing resource modifiers provided in configmap %s/%s", restore.Namespace, cmName).Error())
return nil
}
if err = modifiers.Validate(); err != nil {
if isDefault {
r.logger.WithError(err).Warnf("Validation error in default resource modifier configmap %s/%s, skipping", restore.Namespace, cmName)
return nil
}
restore.Status.ValidationErrors = append(restore.Status.ValidationErrors,
errors.Wrapf(err, "Validation error in resource modifiers provided in configmap %s/%s", restore.Namespace, cmName).Error())
return nil
}
source := "per-restore"
if isDefault {
source = "default"
}
r.logger.Infof("Retrieved %s resource modifiers from configmap %s/%s", source, restore.Namespace, cmName)
return modifiers
}
// backupXorScheduleProvided returns true if exactly one of BackupName and
// ScheduleName are non-empty for the restore, or false otherwise.
func backupXorScheduleProvided(restore *api.Restore) bool {
pkg/controller/restore_controller_test.go
+139
View File
@@ -116,6 +116,7 @@ func TestFetchBackupInfo(t *testing.T) {
false,
fakeGlobalClient,
10*time.Minute,
"",
)
if test.backupStoreError == nil {
@@ -197,6 +198,7 @@ func TestProcessQueueItemSkips(t *testing.T) {
false,
fakeGlobalClient,
10*time.Minute,
"",
)
_, err := r.Reconcile(t.Context(), ctrl.Request{NamespacedName: types.NamespacedName{
@@ -579,6 +581,7 @@ func TestRestoreReconcile(t *testing.T) {
false,
fakeGlobalClient,
10*time.Minute,
"",
)
r.clock = clocktesting.NewFakeClock(now)
@@ -767,6 +770,7 @@ func TestValidateAndCompleteWhenScheduleNameSpecified(t *testing.T) {
false,
fakeGlobalClient,
10*time.Minute,
"",
)
restore := &velerov1api.Restore{
@@ -863,6 +867,7 @@ func TestValidateAndCompleteWithResourcePolicySpecified(t *testing.T) {
false,
fakeGlobalClient,
10*time.Minute,
"",
)
restore := &velerov1api.Restore{
@@ -992,6 +997,7 @@ func TestValidateAndCompleteWithResourceModifierSpecified(t *testing.T) {
false,
fakeGlobalClient,
10*time.Minute,
"",
)
restore := &velerov1api.Restore{
@@ -1110,6 +1116,139 @@ func TestValidateAndCompleteWithResourceModifierSpecified(t *testing.T) {
assert.Contains(t, restore3.Status.ValidationErrors[0], "Validation error in resource modifiers provided in configmap")
}
func TestValidateAndCompleteWithDefaultResourceModifier(t *testing.T) {
formatFlag := logging.FormatText
validCMData := map[string]string{
"modifiers.yaml": "version: v1\nresourceModifierRules:\n- conditions:\n groupResource: pods\n mergePatches:\n - patchData: |\n metadata:\n annotations:\n k8s.ovn.org/pod-networks: null\n",
}
setupReconciler := func(t *testing.T, defaultCM string) *restoreReconciler {
t.Helper()
fakeClient := velerotest.NewFakeControllerRuntimeClient(t)
fakeGlobalClient := velerotest.NewFakeControllerRuntimeClient(t)
pluginManager := &pluginmocks.Manager{}
backupStore := &persistencemocks.BackupStore{}
r := NewRestoreReconciler(
t.Context(),
velerov1api.DefaultNamespace,
nil,
fakeClient,
velerotest.NewLogger(),
logrus.DebugLevel,
func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager },
NewFakeSingleObjectBackupStoreGetter(backupStore),
metrics.NewServerMetrics(),
formatFlag,
60*time.Minute,
false,
fakeGlobalClient,
10*time.Minute,
defaultCM,
)
location := builder.ForBackupStorageLocation("velero", "default").Provider("myCloud").Bucket("bucket").Phase(velerov1api.BackupStorageLocationPhaseAvailable).Result()
require.NoError(t, r.kbClient.Create(t.Context(), location))
require.NoError(t, r.kbClient.Create(t.Context(),
defaultBackup().ObjectMeta(builder.WithName("backup-1")).StorageLocation("default").Phase(velerov1api.BackupPhaseCompleted).Result(),
))
return r
}
newRestore := func(perRestoreCM string, skip *bool) *velerov1api.Restore {
restore := &velerov1api.Restore{
ObjectMeta: metav1.ObjectMeta{
Namespace: velerov1api.DefaultNamespace,
Name: "restore-1",
},
Spec: velerov1api.RestoreSpec{
BackupName: "backup-1",
SkipDefaultResourceModifier: skip,
},
}
if perRestoreCM != "" {
restore.Spec.ResourceModifier = &corev1api.TypedLocalObjectReference{
Kind: resourcemodifiers.ConfigmapRefType,
Name: perRestoreCM,
}
}
return restore
}
t.Run("default modifier applied when no per-restore modifier", 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)
_, rm, _ := r.validateAndComplete(t.Context(), restore)
assert.NotNil(t, rm)
assert.Empty(t, restore.Status.ValidationErrors)
})
t.Run("per-restore modifier takes exclusive precedence", 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,
}))
require.NoError(t, r.kbClient.Create(t.Context(), &corev1api.ConfigMap{
ObjectMeta: metav1.ObjectMeta{Name: "per-restore-rm", Namespace: velerov1api.DefaultNamespace},
Data: validCMData,
}))
restore := newRestore("per-restore-rm", nil)
_, rm, _ := r.validateAndComplete(t.Context(), restore)
assert.NotNil(t, rm)
assert.Empty(t, restore.Status.ValidationErrors)
})
t.Run("skip default modifier when SkipDefaultResourceModifier is true", 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,
}))
skipTrue := true
restore := newRestore("", &skipTrue)
_, rm, _ := r.validateAndComplete(t.Context(), restore)
assert.Nil(t, rm)
assert.Empty(t, restore.Status.ValidationErrors)
})
t.Run("default modifier missing is non-fatal", func(t *testing.T) {
r := setupReconciler(t, "nonexistent-cm")
restore := newRestore("", nil)
_, rm, _ := r.validateAndComplete(t.Context(), restore)
assert.Nil(t, rm)
assert.Empty(t, restore.Status.ValidationErrors)
})
t.Run("per-restore modifier missing is fatal", func(t *testing.T) {
r := setupReconciler(t, "")
restore := newRestore("nonexistent-cm", nil)
_, rm, _ := r.validateAndComplete(t.Context(), restore)
assert.Nil(t, rm)
assert.NotEmpty(t, restore.Status.ValidationErrors)
assert.Contains(t, restore.Status.ValidationErrors[0], "failed to get resource modifiers configmap")
})
t.Run("no default configured and no per-restore modifier", func(t *testing.T) {
r := setupReconciler(t, "")
restore := newRestore("", nil)
_, rm, _ := r.validateAndComplete(t.Context(), restore)
assert.Nil(t, rm)
assert.Empty(t, restore.Status.ValidationErrors)
})
}
func TestBackupXorScheduleProvided(t *testing.T) {
r := &velerov1api.Restore{}
assert.False(t, backupXorScheduleProvided(r))