Skip to content

Commit d2b8ab5

Browse files
sseagosseago
committed
include additional volume policy actions in validation
Don't validate volume policies from plugin calls, since those have alreay been validated for the backup. Signed-off-by: Scott Seago <sseago@redhat.com>
1 parent 6382977 commit d2b8ab5

7 files changed

Lines changed: 88 additions & 72 deletions

File tree

‎changelogs/unreleased/9506-sseago‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Add installer/server arg to allow custom volume policy actions

‎internal/resourcepolicies/resource_policies.go‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,10 @@ type ResourcePolicies struct {
101101
}
102102

103103
type Policies struct {
104-
version string
105-
volumePolicies []volPolicy
106-
includeExcludePolicy *IncludeExcludePolicy
104+
version string
105+
volumePolicies []volPolicy
106+
additionalVolumePolicyActions []string
107+
includeExcludePolicy *IncludeExcludePolicy
107108
// OtherPolicies
108109
}
109110

@@ -210,7 +211,7 @@ func (p *Policies) Validate() error {
210211
}
211212

212213
for _, policy := range p.volumePolicies {
213-
if err := policy.action.validate(); err != nil {
214+
if err := policy.action.validate(p.additionalVolumePolicyActions); err != nil {
214215
return errors.WithStack(err)
215216
}
216217
for _, con := range policy.conditions {
@@ -237,6 +238,8 @@ func GetResourcePoliciesFromBackup(
237238
backup velerov1api.Backup,
238239
client crclient.Client,
239240
logger logrus.FieldLogger,
241+
additionalVolumePolicyActions []string,
242+
validate bool,
240243
) (resourcePolicies *Policies, err error) {
241244
if backup.Spec.ResourcePolicy != nil &&
242245
strings.EqualFold(backup.Spec.ResourcePolicy.Kind, ConfigmapRefType) {
@@ -258,11 +261,14 @@ func GetResourcePoliciesFromBackup(
258261
backup.Namespace+"/"+backup.Name, err.Error())
259262
return nil, fmt.Errorf("fail to read the ResourcePolicies from ConfigMap %s with error %s",
260263
backup.Namespace+"/"+backup.Name, err.Error())
261-
} else if err = resourcePolicies.Validate(); err != nil {
262-
logger.Errorf("Fail to validate ResourcePolicies in ConfigMap %s with error %s.",
263-
backup.Namespace+"/"+backup.Name, err.Error())
264-
return nil, fmt.Errorf("fail to validate ResourcePolicies in ConfigMap %s with error %s",
265-
backup.Namespace+"/"+backup.Name, err.Error())
264+
} else if validate {
265+
resourcePolicies.additionalVolumePolicyActions = additionalVolumePolicyActions
266+
if err = resourcePolicies.Validate(); err != nil {
267+
logger.Errorf("Fail to validate ResourcePolicies in ConfigMap %s with error %s.",
268+
backup.Namespace+"/"+backup.Name, err.Error())
269+
return nil, fmt.Errorf("fail to validate ResourcePolicies in ConfigMap %s with error %s",
270+
backup.Namespace+"/"+backup.Name, err.Error())
271+
}
266272
}
267273
}
268274

‎internal/resourcepolicies/volume_resources_validator.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package resourcepolicies
1818
import (
1919
"fmt"
2020
"io"
21+
"slices"
2122

2223
"github.com/pkg/errors"
2324
"gopkg.in/yaml.v3"
@@ -87,12 +88,15 @@ func decodeStruct(r io.Reader, s any) error {
8788
}
8889

8990
// validate check action format
90-
func (a *Action) validate() error {
91+
func (a *Action) validate(additionalVolumePolicyActions []string) error {
9192
// validate Type
9293
valid := false
9394
if a.Type == Skip || a.Type == Snapshot || a.Type == FSBackup {
9495
valid = true
96+
} else if slices.Contains(additionalVolumePolicyActions, string(a.Type)) {
97+
valid = true
9598
}
99+
96100
if !valid {
97101
return fmt.Errorf("invalid action type %s", a.Type)
98102
}

‎internal/volumehelper/volume_policy_helper.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,9 @@ func NewVolumeHelperImplWithCache(
111111
logger logrus.FieldLogger,
112112
pvcPodCache *podvolumeutil.PVCPodCache,
113113
) (VolumeHelper, error) {
114-
resourcePolicies, err := resourcepolicies.GetResourcePoliciesFromBackup(backup, client, logger)
114+
// no need to validate resource policies here since this is called from the plugin. They're already
115+
// validated at the backup level.
116+
resourcePolicies, err := resourcepolicies.GetResourcePoliciesFromBackup(backup, client, logger, nil, false)
115117
if err != nil {
116118
return nil, errors.Wrap(err, "failed to get volume policies from backup")
117119
}

‎pkg/cmd/cli/install/install.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -244,14 +244,14 @@ func NewInstallOptions() *Options {
244244
NodeAgentPodCPULimit: install.DefaultNodeAgentPodCPULimit,
245245
NodeAgentPodMemLimit: install.DefaultNodeAgentPodMemLimit,
246246
// Default to creating a VSL unless we're told otherwise
247-
UseVolumeSnapshots: true,
248-
NoDefaultBackupLocation: false,
249-
CRDsOnly: false,
250-
DefaultVolumesToFsBackup: false,
251-
UploaderType: uploader.KopiaType,
252-
DefaultSnapshotMoveData: false,
253-
DisableInformerCache: false,
254-
ScheduleSkipImmediately: false,
247+
UseVolumeSnapshots: true,
248+
NoDefaultBackupLocation: false,
249+
CRDsOnly: false,
250+
DefaultVolumesToFsBackup: false,
251+
UploaderType: uploader.KopiaType,
252+
DefaultSnapshotMoveData: false,
253+
DisableInformerCache: false,
254+
ScheduleSkipImmediately: false,
255255
kubeletRootDir: install.DefaultKubeletRootDir,
256256
NodeAgentDisableHostPath: false,
257257
AdditionalVolumePolicyActions: flag.NewStringArray(),

‎pkg/controller/backup_controller.go‎

Lines changed: 53 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -84,32 +84,32 @@ var autoExcludeClusterScopedResources = []string{
8484
}
8585

8686
type backupReconciler struct {
87-
ctx context.Context
88-
logger logrus.FieldLogger
89-
discoveryHelper discovery.Helper
90-
backupper pkgbackup.Backupper
91-
kbClient kbclient.Client
92-
clock clock.WithTickerAndDelayedExecution
93-
backupLogLevel logrus.Level
94-
newPluginManager func(logrus.FieldLogger) clientmgmt.Manager
95-
backupTracker BackupTracker
96-
defaultBackupLocation string
97-
defaultVolumesToFsBackup bool
98-
defaultBackupTTL time.Duration
99-
defaultVGSLabelKey string
100-
defaultCSISnapshotTimeout time.Duration
101-
resourceTimeout time.Duration
102-
defaultItemOperationTimeout time.Duration
103-
defaultSnapshotLocations map[string]string
104-
metrics *metrics.ServerMetrics
105-
backupStoreGetter persistence.ObjectBackupStoreGetter
106-
formatFlag logging.Format
107-
credentialFileStore credentials.FileStore
108-
maxConcurrentK8SConnections int
109-
defaultSnapshotMoveData bool
110-
globalCRClient kbclient.Client
111-
itemBlockWorkerCount int
112-
concurrentBackups int
87+
ctx context.Context
88+
logger logrus.FieldLogger
89+
discoveryHelper discovery.Helper
90+
backupper pkgbackup.Backupper
91+
kbClient kbclient.Client
92+
clock clock.WithTickerAndDelayedExecution
93+
backupLogLevel logrus.Level
94+
newPluginManager func(logrus.FieldLogger) clientmgmt.Manager
95+
backupTracker BackupTracker
96+
defaultBackupLocation string
97+
defaultVolumesToFsBackup bool
98+
defaultBackupTTL time.Duration
99+
defaultVGSLabelKey string
100+
defaultCSISnapshotTimeout time.Duration
101+
resourceTimeout time.Duration
102+
defaultItemOperationTimeout time.Duration
103+
defaultSnapshotLocations map[string]string
104+
metrics *metrics.ServerMetrics
105+
backupStoreGetter persistence.ObjectBackupStoreGetter
106+
formatFlag logging.Format
107+
credentialFileStore credentials.FileStore
108+
maxConcurrentK8SConnections int
109+
defaultSnapshotMoveData bool
110+
globalCRClient kbclient.Client
111+
itemBlockWorkerCount int
112+
concurrentBackups int
113113
additionalVolumePolicyActions []string
114114
}
115115

@@ -142,32 +142,32 @@ func NewBackupReconciler(
142142
additionalVolumePolicyActions []string,
143143
) *backupReconciler {
144144
b := &backupReconciler{
145-
ctx: ctx,
146-
discoveryHelper: discoveryHelper,
147-
backupper: backupper,
148-
clock: &clock.RealClock{},
149-
logger: logger,
150-
backupLogLevel: backupLogLevel,
151-
newPluginManager: newPluginManager,
152-
backupTracker: backupTracker,
153-
kbClient: kbClient,
154-
defaultBackupLocation: defaultBackupLocation,
155-
defaultVolumesToFsBackup: defaultVolumesToFsBackup,
156-
defaultBackupTTL: defaultBackupTTL,
157-
defaultVGSLabelKey: defaultVGSLabelKey,
158-
defaultCSISnapshotTimeout: defaultCSISnapshotTimeout,
159-
resourceTimeout: resourceTimeout,
160-
defaultItemOperationTimeout: defaultItemOperationTimeout,
161-
defaultSnapshotLocations: defaultSnapshotLocations,
162-
metrics: metrics,
163-
backupStoreGetter: backupStoreGetter,
164-
formatFlag: formatFlag,
165-
credentialFileStore: credentialStore,
166-
maxConcurrentK8SConnections: maxConcurrentK8SConnections,
167-
defaultSnapshotMoveData: defaultSnapshotMoveData,
168-
itemBlockWorkerCount: itemBlockWorkerCount,
169-
concurrentBackups: max(concurrentBackups, 1),
170-
globalCRClient: globalCRClient,
145+
ctx: ctx,
146+
discoveryHelper: discoveryHelper,
147+
backupper: backupper,
148+
clock: &clock.RealClock{},
149+
logger: logger,
150+
backupLogLevel: backupLogLevel,
151+
newPluginManager: newPluginManager,
152+
backupTracker: backupTracker,
153+
kbClient: kbClient,
154+
defaultBackupLocation: defaultBackupLocation,
155+
defaultVolumesToFsBackup: defaultVolumesToFsBackup,
156+
defaultBackupTTL: defaultBackupTTL,
157+
defaultVGSLabelKey: defaultVGSLabelKey,
158+
defaultCSISnapshotTimeout: defaultCSISnapshotTimeout,
159+
resourceTimeout: resourceTimeout,
160+
defaultItemOperationTimeout: defaultItemOperationTimeout,
161+
defaultSnapshotLocations: defaultSnapshotLocations,
162+
metrics: metrics,
163+
backupStoreGetter: backupStoreGetter,
164+
formatFlag: formatFlag,
165+
credentialFileStore: credentialStore,
166+
maxConcurrentK8SConnections: maxConcurrentK8SConnections,
167+
defaultSnapshotMoveData: defaultSnapshotMoveData,
168+
itemBlockWorkerCount: itemBlockWorkerCount,
169+
concurrentBackups: max(concurrentBackups, 1),
170+
globalCRClient: globalCRClient,
171171
additionalVolumePolicyActions: additionalVolumePolicyActions,
172172
}
173173
b.updateTotalBackupMetric()
@@ -583,7 +583,7 @@ func (b *backupReconciler) prepareBackupRequest(ctx context.Context, backup *vel
583583
request.Status.ValidationErrors = append(request.Status.ValidationErrors, "encountered labelSelector as well as orLabelSelectors in backup spec, only one can be specified")
584584
}
585585

586-
resourcePolicies, err := resourcepolicies.GetResourcePoliciesFromBackup(*request.Backup, b.kbClient, logger)
586+
resourcePolicies, err := resourcepolicies.GetResourcePoliciesFromBackup(*request.Backup, b.kbClient, logger, b.additionalVolumePolicyActions, true)
587587
if err != nil {
588588
request.Status.ValidationErrors = append(request.Status.ValidationErrors, err.Error())
589589
}

‎pkg/plugin/utils/volumehelper/volume_policy_helper.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,13 @@ func ShouldPerformSnapshotWithVolumeHelper(
7474
}
7575

7676
// Otherwise, create a new VolumeHelper (original behavior for third-party plugins)
77+
// No need to validate policies here as this has already happened for the backup.
7778
resourcePolicies, err := resourcepolicies.GetResourcePoliciesFromBackup(
7879
backup,
7980
crClient,
8081
logger,
82+
nil,
83+
false,
8184
)
8285
if err != nil {
8386
return false, err

0 commit comments

Comments
 (0)