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

Address review feedback on default resource modifier

Browse Source 
- Fix fallthrough bug: when ResourceModifier is set with a non-ConfigMap
  kind, do not fall through to applying the server default. The outer
  check on ResourceModifier != nil now prevents default application
  regardless of the Kind value.
- Include underlying error in fatal validation message for ConfigMap
  retrieval failures.
- Strengthen exclusive precedence test: default ConfigMap intentionally
  does not exist while per-restore does, proving the default is never
  consulted.
- Add test for invalid default ConfigMap data (non-fatal, warn and
  proceed).

Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
This commit is contained in:
Shubham Pampattiwar
2026-08-03 13:19:48 -07:00
parent d87a66393d
commit a6f800c591
2 changed files with 26 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/controller/restore_controller.go
+7 -5
View File
@@ -435,10 +435,12 @@ 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) {
resourceModifiers = r.loadResourceModifierConfigMap(ctx, restore, restore.Spec.ResourceModifier.Name, false)
if resourceModifiers == nil && len(restore.Status.ValidationErrors) > 0 {
return backupInfo{}, nil, nil
if restore.Spec.ResourceModifier != nil {
if strings.EqualFold(restore.Spec.ResourceModifier.Kind, resourcemodifiers.ConfigmapRefType) {
resourceModifiers = r.loadResourceModifierConfigMap(ctx, restore, restore.Spec.ResourceModifier.Name, false)
if resourceModifiers == nil && len(restore.Status.ValidationErrors) > 0 {
return backupInfo{}, nil, nil
}
}
} else if r.defaultResourceModifierConfigMap != "" && !boolptr.IsSetToTrue(restore.Spec.SkipDefaultResourceModifier) {
resourceModifiers = r.loadResourceModifierConfigMap(ctx, restore, r.defaultResourceModifierConfigMap, true)
@@ -460,7 +462,7 @@ func (r *restoreReconciler) loadResourceModifierConfigMap(
return nil
}
restore.Status.ValidationErrors = append(restore.Status.ValidationErrors,
fmt.Sprintf("failed to get resource modifiers configmap %s/%s", restore.Namespace, cmName))
fmt.Sprintf("failed to get resource modifiers configmap %s/%s: %v", restore.Namespace, cmName, err))
return nil
}
pkg/controller/restore_controller_test.go
+19 -6
View File
@@ -1189,12 +1189,10 @@ func TestValidateAndCompleteWithDefaultResourceModifier(t *testing.T) {
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,
}))
t.Run("per-restore modifier takes exclusive precedence over default", func(t *testing.T) {
// Default ConfigMap does NOT exist, but per-restore does.
// If default were applied, it would fail. Per-restore should succeed.
r := setupReconciler(t, "nonexistent-default")
require.NoError(t, r.kbClient.Create(t.Context(), &corev1api.ConfigMap{
ObjectMeta: metav1.ObjectMeta{Name: "per-restore-rm", Namespace: velerov1api.DefaultNamespace},
Data: validCMData,
@@ -1220,6 +1218,21 @@ func TestValidateAndCompleteWithDefaultResourceModifier(t *testing.T) {
assert.Empty(t, restore.Status.ValidationErrors)
})
t.Run("default modifier with invalid data is non-fatal", func(t *testing.T) {
r := setupReconciler(t, "invalid-default")
require.NoError(t, r.kbClient.Create(t.Context(), &corev1api.ConfigMap{
ObjectMeta: metav1.ObjectMeta{Name: "invalid-default", Namespace: velerov1api.DefaultNamespace},
Data: map[string]string{
"modifiers.yaml": "not-valid-yaml: [",
},
}))
restore := newRestore("", nil)
_, 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")