diff --git a/changelogs/unreleased/10012-chlins b/changelogs/unreleased/10012-chlins new file mode 100644 index 000000000..e43456ac1 --- /dev/null +++ b/changelogs/unreleased/10012-chlins @@ -0,0 +1 @@ +Add support for matching PVCs by volume mode and access mode in resource policies, and introduce the `--global-backup-volume-policies-configmap` server flag to merge cluster-wide backup volume policies into every backup. diff --git a/design/global-backup-volume-policies.md b/design/global-backup-volume-policies.md new file mode 100644 index 000000000..3f0ae3722 --- /dev/null +++ b/design/global-backup-volume-policies.md @@ -0,0 +1,162 @@ +# Global Backup Volume Policies for Velero + +## Background + +Velero supports [resource policies](./Implemented/handle-backup-of-volumes-by-resources-filters.md) (commonly referred to as "volume policies") that let a user control how volumes are handled during a backup — for example, whether a volume is skipped, backed up via file-system backup (`fs-backup`), snapshotted, or handled by a custom plugin. + +Today these policies are defined per-backup: + +1. A user creates a ConfigMap in the Velero install namespace whose single data key holds a `ResourcePolicies` YAML document (`volumePolicies` and the related include/exclude and fine-grained filter policies). +2. The user opts a specific backup into that ConfigMap with the CLI flag `--resource-policies-configmap`, which sets `Backup.Spec.ResourcePolicy` as a reference to the ConfigMap. +3. When the backup is processed, velero loads the referenced ConfigMap, unmarshals the YAML, builds a `Policies` object, and applies it when performing the backup. + +The limitation today is that volume policies are strictly opt-in **per backup**. An administrator, who usually has the best knowledge of the environment, may want a baseline behavior to apply to *every* backup in the cluster (for example, "always skip volumes from the `gp2` storage class", or "always use `fs-backup` for NFS volumes"). However, today they must remember to attach the same ConfigMap to every backup and every schedule. There is no way to express a cluster-wide default volume policy that is enforced regardless of what an individual backup requests. + +## Goals + +- Introduce "global backup volume policies" that an administrator configures once when the Velero server starts. +- Expose it as a Velero server CLI parameter that points to a ConfigMap in the Velero install namespace. +- When a backup runs, merge the global backup volume policies with the backup's own resource policies ConfigMap (if any) and use the merged result as the effective resource policies for that backup. +- Keep the existing per-backup `--resource-policies-configmap` behavior fully backward compatible when no global policy is configured. + +## Non Goals + +- Changing the schema of the `ResourcePolicies`/`volumePolicies` YAML itself. +- Defining global defaults for anything other than resource policies (e.g. it does not introduce new global backup spec defaults). +- Supporting per-namespace or per-schedule global policy overrides. The "global policies" is a single, server-wide configuration. +- Hot-reloading the global policies ConfigMap without a server restart is out of scope for the initial implementation. +- Support setting other filters in "resource policies" (e.g. include/exclude or fine-grained filters) in the global policy is out of scope for the initial implementation. Only `volumePolicies` will be supported in the global policy for now. + +## Design + +A new Velero server flag, `--global-backup-volume-policies-configmap`, accepts the name of a ConfigMap that lives in the Velero install namespace. The ConfigMap has the exact same format as an existing per-backup resource policies ConfigMap (a single data key holding a `ResourcePolicies` YAML document). + +The flag value is plumbed from the server `Config` into the `backupReconciler`. During `prepareBackupRequest`, in addition to loading the backup's own resource policy (referenced by `Backup.Spec.ResourcePolicy`), Velero loads the global policy ConfigMap. The two `ResourcePolicies` documents are then **merged** into a single effective `ResourcePolicies`, which is compiled into a `Policies` object, validated, and stored on `request.ResPolicies` exactly as today. The rest of the backup pipeline is unchanged because it only consumes `request.ResPolicies`. + +``` + server flag --global-backup-volume-policies-configmap + | + v + Backup.Spec.ResourcePolicy global policies ConfigMap (install ns) + | | + v v + backup-level ResourcePolicies global ResourcePolicies + \ / + \ / + v v + merge() -> effective ResourcePolicies + | + v + Policies (compiled + validated) + | + v + request.ResPolicies (unchanged consumers) +``` + +### Volume Policy only + +The resource policies ConfigMap schema includes both volume policies and include/exclude/fine-grained filter policies. The global backup volume policy only applies to the `volumePolicies` section of the schema. If the global ConfigMap includes any include/exclude/fine-grained filter policies, they are ignored and not merged into the effective policy. In this case, a warning message will be printed in the Velero server logs. +This is a design choice because only the volume policies are more tied to the environment where velero runs, and are more likely to be something an administrator would want to enforce globally. The include/exclude/fine-grained filter policies are more tied to the specific backup use case, and it would be less intuitive for an administrator to have those apply globally across all backups. + +### Validation + +Velero will validate the global backup volume policies ConfigMap at server startup. If the ConfigMap is missing or invalid, the server will fail to start and log an error. This ensures any mistakes in configuration will be caught early. +It should also make sure the validation happens for each backup, because the ConfigMap could be updated or removed after the server starts. If the global policies ConfigMap is missing or invalid at backup time, the backup CR will be put into "FailedValidation" phase, with an appropriate error message in the logs. + +### Merge semantics + +The merge combines two `ResourcePolicies` documents: the global policy (`G`) and the backup-level policy (`B`). The guiding principle is that the global policy provides a baseline, and the backup-level policy is layered with it. + +- **`volumePolicies`**: `volumePolicies` is an ordered list where the *first* matching policy wins (per the existing `Policies.match` logic). The merged list is the concatenation of the backup-level policies followed by the global policies: + + ``` + merged.volumePolicies = B.volumePolicies ++ G.volumePolicies + ``` + + This gives a backup the ability to override the global baseline for a specific volume (because its policy is evaluated first), while still inheriting all global rules that the backup does not override. + +When only the global policy is configured (the backup does not reference a resource policy), the effective policy is the global policy alone. When only the backup policy exists (no global policy configured), behavior is identical to today. + +#### Example + +Global policy ConfigMap (set on the server with `--global-backup-volume-policies-configmap=global-volume-policy`): + +```yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: global-volume-policy + namespace: velero +data: + policies.yaml: | + version: v1 + volumePolicies: + - conditions: + storageClass: + - gp2 + action: + type: skip +``` + +Backup-level policy ConfigMap (referenced with `velero backup create --resource-policies-configmap backup01`): + +```yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: backup01 + namespace: velero +data: + policies.yaml: | + version: v1 + volumePolicies: + - conditions: + nfs: {} + action: + type: fs-backup +``` + +Effective (merged) volume policies used for the backup — backup rules first, then global: + +```yaml +version: v1 +volumePolicies: + - conditions: + nfs: {} + action: + type: fs-backup + - conditions: + storageClass: + - gp2 + action: + type: skip +``` + +### Output of `velero backup describe` + +Currently, the `velero backup describe` command shows the backup-level resource policy. We should update the CLI to make sure the global volume policies are also shown in the output, so that user will not need to check the parameter of velero server. + +## Implementation + +- **Server flag and config.** Add a new field (e.g. `GlobalBackupVolumePoliciesConfigMap`) to the server `Config` struct in `pkg/cmd/server/config/config.go`, register the `--global-backup-volume-policies-configmap` flag in `Config.BindFlags`, and leave its default empty in `GetDefaultConfig` so the feature stays opt-in. +- **Plumb the value into the reconciler.** In `pkg/cmd/server/server.go`, pass the configured ConfigMap name (along with the Velero install namespace) into `controller.NewBackupReconciler`. Add a corresponding parameter and store it as a field on the `backupReconciler` struct in `pkg/controller/backup_controller.go`. +- **Load and merge the policies.** In `internal/resourcepolicies/resource_policies.go`, add a new function (e.g. `GetResourcePoliciesFromBackupWithGlobal`) that, in addition to loading the backup-referenced ConfigMap as `GetResourcePoliciesFromBackup` does today, also loads the global ConfigMap from the install namespace via the existing `getResourcePoliciesFromConfig` helper. After that the function merges the two `ResourcePolicies` documents according to the semantics described above. +- **Call site.** Update `prepareBackupRequest` in `pkg/controller/backup_controller.go` (currently calling `GetResourcePoliciesFromBackup`) to apply the merged policies from the new function. The rest of the backup pipeline remains unchanged. +- **CLI describe output.** Update `DescribeResourcePolicies` in `pkg/cmd/util/output/backup_describer.go` and `DescribeResourcePoliciesInSF` in `pkg/cmd/util/output/backup_structured_describer.go` to also surface the global volume policy ConfigMap that contributed to the backup. + +## Security Considerations + +The Global Backup Volume Policy is read from a ConfigMap in the Velero install namespace, the same trust boundary as existing resource policy ConfigMaps and Velero's own configuration. Setting it requires the ability to pass server flags / edit the Velero deployment, which is already an administrative privilege. No new data is exposed and no new external access patterns are introduced. + +## Compatibility + +- The feature is fully opt-in. If `--global-backup-volume-policies-configmap` is not set (the default), behavior is byte-for-byte identical to today. +- Existing per-backup `--resource-policies-configmap` usage is unchanged; it is simply merged with the global baseline when one is configured. +- Backups created before this feature, and backups that reference no resource policy, transparently start honoring the global policy once it is configured. This is the intended behavior of a "global" policy, but operators should be aware that introducing a global policy changes the effective behavior of backups that previously had no resource policy. +- The behavior of scheduled backup may change when a global backup volume policy is introduced, because the scheduled backup will start honoring the global volume policies. This is an expected change, but administrators should be aware of this when introducing a global policy to an existing velero instance with scheduled backups. +- The merged policy is computed at backup time and is reflected wherever `request.ResPolicies` is consumed. `velero backup describe` should be updated to indicate when a global policy contributed to a backup. + +## Alternatives Considered + +- **Global policies applied only when a backup has no policy of its own.** Simpler, but it makes the global policy a fallback default rather than an enforced baseline, and it cannot express "always do X in addition to whatever the backup wants". Merging is more expressive. +- **Global precedence over backup-level policies** (global volume policies evaluated first). Rejected as the default because it would prevent backups from overriding the baseline for specific volumes. diff --git a/design/volume-policy-pvc-volume-mode-access-modes.md b/design/volume-policy-pvc-volume-mode-access-modes.md new file mode 100644 index 000000000..1c5abba94 --- /dev/null +++ b/design/volume-policy-pvc-volume-mode-access-modes.md @@ -0,0 +1,359 @@ +# Add PVC VolumeMode and AccessModes as Criteria for Volume Policy + +## Abstract +This proposal extends Velero VolumePolicy conditions with two PVC-based criteria, `pvcVolumeMode` and `pvcAccessModes`. +These conditions allow users to select volumes according to the `volumeMode` and `accessModes` of the associated PersistentVolumeClaim (PVC), enabling backup behavior such as skipping block-mode PVCs or choosing a specific backup method for volumes with selected access modes. + +## Background +Velero VolumePolicy already supports selecting volumes by attributes such as capacity, storage class, volume source, volume type, PVC labels, and PVC phase. +PVC metadata and spec fields are often the most direct way for users to express the intended storage semantics of a workload. + +Kubernetes PVCs include a `spec.volumeMode` field that describes whether the volume is exposed as a filesystem or as a raw block device. +The field supports values such as `Filesystem` and `Block`. + +Kubernetes PVCs also include a `spec.accessModes` field that describes how the volume can be mounted. +Common values are `ReadWriteOnce`, `ReadOnlyMany`, `ReadWriteMany`, and `ReadWriteOncePod`. +For resource policies, `pvcAccessModes` uses an exact set match against the PVC's `spec.accessModes`, so a policy does not match PVCs that have missing or additional access modes. + +## Goals +- Add a `pvcVolumeMode` VolumePolicy condition to match volumes by a single `spec.volumeMode` value of their associated PVC. +- Add a `pvcAccessModes` VolumePolicy condition to match volumes whose associated PVC has exactly the configured `spec.accessModes` values, regardless of order. +- Keep the new conditions consistent with existing VolumePolicy behavior, where all conditions in a policy must match and the first matching policy wins. + +## Non-Goals +- This proposal does not add new VolumePolicy actions. +- This proposal does not change how PVCs are discovered or passed into the resource policy matching code. +- This proposal does not add set-based or negative matching operators such as `NotIn`, `Exists`, or `DoesNotExist`. +- This proposal does not change Kubernetes PVC semantics or validate storage provider capabilities. + +## Use-cases/Scenarios + +### Skip block-mode PVCs +A user wants to skip volumes whose associated PVC is configured with raw block volume mode. + +```yaml +version: v1 +volumePolicies: +- conditions: + pvcVolumeMode: Block + action: + type: skip +``` + +### Snapshot filesystem PVCs +A user wants to use snapshots only for volumes whose associated PVC has filesystem mode. + +```yaml +version: v1 +volumePolicies: +- conditions: + pvcVolumeMode: Filesystem + action: + type: snapshot +``` + +### Match PVCs by access mode +A user wants to apply a policy only to PVCs whose `spec.accessModes` is exactly `ReadWriteOnce`. + +```yaml +version: v1 +volumePolicies: +- conditions: + pvcAccessModes: + - ReadWriteOnce + action: + type: skip +``` + +### Match an exact access mode set +A user wants to match volumes whose associated PVC access modes are exactly `ReadOnlyMany` and `ReadWriteMany`. +A PVC that includes only one of these modes, or includes additional modes, does not match. + +```yaml +version: v1 +volumePolicies: +- conditions: + pvcAccessModes: + - ReadOnlyMany + - ReadWriteMany + action: + type: snapshot +``` + +### Combine PVC spec criteria +A user wants to select block-mode PVCs whose access modes are exactly `ReadWriteOnce`. +Because VolumePolicy conditions are conjunctive, the volume must satisfy both conditions. + +```yaml +version: v1 +volumePolicies: +- conditions: + pvcVolumeMode: Block + pvcAccessModes: + - ReadWriteOnce + action: + type: snapshot +``` + +## High-Level Design +The VolumePolicy condition schema is extended with two optional fields, `pvcVolumeMode` and `pvcAccessModes`. +`pvcVolumeMode` is represented as a single string value in the resource policy YAML. +`pvcAccessModes` is represented as a string list in the resource policy YAML. + +The internal `structuredVolume` representation is extended to store the associated PVC's volume mode and access modes. +The existing PVC parsing path populates these fields when a PVC is available in `VolumeFilterData`. + +The policy builder creates a `pvcVolumeModeCondition` when `pvcVolumeMode` is specified and creates a `pvcAccessModesCondition` when `pvcAccessModes` is specified. +The existing matching flow remains unchanged: each condition implements the `volumeCondition` interface, all conditions in a policy must match, and the first matching policy's action is returned. + +## Detailed Design + +### Resource policy YAML schema +Two new fields are added under `volumePolicies[].conditions`. + +```yaml +version: v1 +volumePolicies: +- conditions: + pvcVolumeMode: Block + pvcAccessModes: + - ReadWriteOnce + - ReadWriteMany + action: + type: snapshot +``` + +`pvcVolumeMode` is a string. +The intended values are Kubernetes PVC volume mode values, including `Filesystem` and `Block`. +The condition matches only when the PVC volume mode value observed by Velero exactly equals the configured value. +Matching is case-sensitive, so `block` does not match `Block`. + +`pvcAccessModes` is a list of strings. +The intended values are Kubernetes PVC access mode values, including `ReadWriteOnce`, `ReadOnlyMany`, `ReadWriteMany`, and `ReadWriteOncePod`. +The condition matches only when the configured access modes exactly equal the PVC's `spec.accessModes`, ignoring order. +Matching is case-sensitive, so `readwriteonce` does not match `ReadWriteOnce`. + +The implementation validates that `pvcVolumeMode`, when present, is a string. +The implementation validates that `pvcAccessModes`, when present, is a list of strings. +The implementation does not strictly reject unknown string values so that the condition format remains tolerant of Kubernetes additions or storage-provider-specific behavior. +Unknown `pvcVolumeMode` values match only when the PVC has the same string value, and unknown `pvcAccessModes` values match only as part of the same exact access-mode set. + +### Volume condition struct +The parsed condition struct is extended as follows. + +```go +type volumeConditions struct { + Capacity string `yaml:"capacity,omitempty"` + StorageClass []string `yaml:"storageClass,omitempty"` + NFS *nFSVolumeSource `yaml:"nfs,omitempty"` + CSI *csiVolumeSource `yaml:"csi,omitempty"` + VolumeTypes []SupportedVolume `yaml:"volumeTypes,omitempty"` + PVCLabels map[string]string `yaml:"pvcLabels,omitempty"` + PVCPhase []string `yaml:"pvcPhase,omitempty"` + PVCVolumeMode string `yaml:"pvcVolumeMode,omitempty"` + PVCAccessModes []string `yaml:"pvcAccessModes,omitempty"` +} +``` + +### Structured volume data +The internal `structuredVolume` is extended with `pvcVolumeMode` and `pvcAccessModes`. + +```go +type structuredVolume struct { + capacity resource.Quantity + storageClass string + nfs *nFSVolumeSource + csi *csiVolumeSource + volumeType SupportedVolume + pvcLabels map[string]string + pvcPhase string + pvcVolumeMode string + pvcAccessModes []string +} +``` + +When a PVC is available, `parsePVC` extracts PVC attributes into `structuredVolume` for later condition evaluation. +This parsing step does not create or imply a `pvcVolumeMode` policy condition; `pvcVolumeMode` only constrains matching when the user explicitly configures `conditions.pvcVolumeMode` in the VolumePolicy. +Velero uses `pvc.Spec.VolumeMode` as-is when it is present. +If `pvc.Spec.VolumeMode` is nil, `pvcVolumeMode` remains empty and does not match any non-empty `pvcVolumeMode` condition. +If `pvc.Spec.AccessModes` is empty, `pvcAccessModes` remains empty and does not match any non-empty `pvcAccessModes` condition. + +```go +func (s *structuredVolume) parsePVC(pvc *corev1api.PersistentVolumeClaim) { + if pvc != nil { + if len(pvc.GetLabels()) > 0 { + s.pvcLabels = pvc.Labels + } + s.pvcPhase = string(pvc.Status.Phase) + if pvc.Spec.VolumeMode != nil { + s.pvcVolumeMode = string(*pvc.Spec.VolumeMode) + } + if len(pvc.Spec.AccessModes) > 0 { + s.pvcAccessModes = make([]string, 0, len(pvc.Spec.AccessModes)) + for _, accessMode := range pvc.Spec.AccessModes { + s.pvcAccessModes = append(s.pvcAccessModes, string(accessMode)) + } + } + } +} +``` + +### PVC volume mode condition +`pvcVolumeModeCondition` matches when the associated PVC's parsed volume mode exactly equals the configured value. +The comparison is case-sensitive and does not normalize values. +An empty configured value is treated as no constraint and always matches, consistent with other VolumePolicy conditions. +A non-empty configured value does not match if no PVC volume mode is available. + +```go +type pvcVolumeModeCondition struct { + volumeMode string +} + +func (c *pvcVolumeModeCondition) match(v *structuredVolume) bool { + if c.volumeMode == "" { + return true + } + if v.pvcVolumeMode == "" { + return false + } + return v.pvcVolumeMode == c.volumeMode +} +``` + +### PVC access modes condition +`pvcAccessModesCondition` matches when the configured access modes exactly equal the associated PVC's access modes, ignoring order. +The comparison is case-sensitive and does not normalize values. +An empty configured list is treated as no constraint and always matches. +A non-empty configured list does not match if the structured volume has no PVC access modes, has a different number of access modes, or has a different access-mode set. + +```go +type pvcAccessModesCondition struct { + accessModes []string +} + +func (c *pvcAccessModesCondition) match(v *structuredVolume) bool { + if len(c.accessModes) == 0 { + return true + } + if len(v.pvcAccessModes) == 0 || len(v.pvcAccessModes) != len(c.accessModes) { + return false + } + + return sets.New(c.accessModes...).Equal(sets.New(v.pvcAccessModes...)) +} +``` + +### Condition validation +Both `pvcVolumeModeCondition` and `pvcAccessModesCondition` implement the `validate()` method required by the `volumeCondition` interface. +The `validate()` method returns nil for both conditions. + +```go +func (c *pvcVolumeModeCondition) validate() error { + return nil +} + +func (c *pvcAccessModesCondition) validate() error { + return nil +} +``` + +YAML shape validation is handled when resource policy conditions are unmarshaled. +`pvcVolumeMode` must be a string, and `pvcAccessModes` must be a list of strings. +Condition-level validation intentionally does not reject unknown string values. +This keeps the policy format forward-compatible with future Kubernetes values and consistent with other string-based VolumePolicy conditions. +Unknown values simply do not match normal PVCs unless the evaluated PVC has the same exact value or access-mode set. + +### Policy builder integration +The policy builder appends the new conditions only when the corresponding YAML fields are present. + +```go +func (p *Policies) BuildPolicy(resPolicies *ResourcePolicies) error { + for _, vp := range resPolicies.VolumePolicies { + con, err := unmarshalVolConditions(vp.Conditions) + if err != nil { + return errors.WithStack(err) + } + + // Existing conditions are appended here. + + if con.PVCVolumeMode != "" { + volP.conditions = append(volP.conditions, &pvcVolumeModeCondition{volumeMode: con.PVCVolumeMode}) + } + if len(con.PVCAccessModes) > 0 { + volP.conditions = append(volP.conditions, &pvcAccessModesCondition{accessModes: con.PVCAccessModes}) + } + } + return nil +} +``` + +### Matching behavior with other conditions +The new conditions follow the existing VolumePolicy matching behavior. +Within a single policy, every configured condition must match. +If `pvcVolumeMode` is omitted from a policy, Velero does not add a volume mode condition and the policy does not restrict volume mode. +`pvcVolumeMode` and `pvcAccessModes` are PVC-specific conditions and only match when the volume policy evaluation has associated PVC data. +For non-PVC volumes such as `emptyDir`, `configMap`, or inline volumes without an associated PVC, the parsed PVC fields are empty and policies requiring `pvcVolumeMode` or `pvcAccessModes` do not match. +Across multiple policies, the first matching policy wins. + +For example, this policy matches only PVC-backed volumes that are both `Block` mode and have exactly `ReadWriteOnce` as their access modes. + +```yaml +version: v1 +volumePolicies: +- conditions: + pvcVolumeMode: Block + pvcAccessModes: + - ReadWriteOnce + action: + type: snapshot +``` + +## Alternatives Considered + +### A single `pvcSpec` condition object +One alternative is to add a nested object such as `pvcSpec.volumeMode` and `pvcSpec.accessModes`. +This was not chosen because existing PVC-based VolumePolicy conditions use flat field names such as `pvcLabels` and `pvcPhase`. +Flat names keep the YAML concise and consistent with existing conditions. + +### List-based `pvcVolumeMode` +One alternative is to make `pvcVolumeMode` a list, similar to `pvcPhase`. +This was not chosen because Kubernetes PVC `spec.volumeMode` is a single value and the policy condition is intended to describe an exact match against that value. +Using a string avoids implying that multiple volume modes can apply to one PVC. + +### Contains-based access mode matching +Another alternative is to make `pvcAccessModes` match when any or all configured access modes are present on the PVC. +This was not chosen because contains-based matching would also select PVCs with additional access modes. +Using exact set matching keeps `pvcAccessModes` consistent with `pvcVolumeMode`'s exact-match behavior and avoids matching PVCs whose access mode set differs from the policy. + +### Strict validation of allowed Kubernetes values +Another alternative is to reject `pvcVolumeMode` or `pvcAccessModes` values that are not currently known Kubernetes constants. +This was not chosen because accepting strings is more forward-compatible and keeps behavior consistent with other string-based resource policy conditions. +Invalid or unknown values naturally fail to match unless a PVC has the same value. + +## Security Considerations +This proposal does not introduce new privileges or access to additional Kubernetes resources. +It only uses PVC data already available to the volume policy matching path. + +The new conditions can cause Velero to skip or choose different backup actions for matched volumes. +Users should review policy configuration carefully because an overly broad policy can exclude data from backup or select an unintended backup method. + +## Compatibility +The new fields are optional and do not affect existing resource policy files. +Existing VolumePolicy behavior remains unchanged when `pvcVolumeMode` and `pvcAccessModes` are not configured. + +PVCs without a parsed `spec.volumeMode` value do not match non-empty `pvcVolumeMode` conditions. +PVCs without `spec.accessModes` do not match non-empty `pvcAccessModes` conditions. + +Unknown `pvcVolumeMode` or `pvcAccessModes` string values in a policy are accepted as strings but will not match normal Kubernetes PVCs unless the evaluated PVC has the same exact value or access-mode set. + +## Implementation +Implementation requires changes in the resource policies package and documentation. + +- Extend `volumeConditions` with `PVCVolumeMode string` and `PVCAccessModes []string`. +- Extend `structuredVolume` with `pvcVolumeMode string` and `pvcAccessModes []string`. +- Update `parsePVC` to populate the new fields from the PVC spec. +- Add `pvcVolumeModeCondition` and `pvcAccessModesCondition` implementations. +- Update `Policies.BuildPolicy` to append the new conditions. +- Add YAML type validation to ensure `pvcVolumeMode` is a string and `pvcAccessModes` is a string list. +- Add unit tests for parsing, validation, condition matching, and end-to-end `GetMatchAction` behavior. +- Update user documentation in `site/content/docs/main/resource-filtering.md`. diff --git a/internal/resourcepolicies/resource_policies.go b/internal/resourcepolicies/resource_policies.go index 9d7ed25f6..e14cb59f6 100644 --- a/internal/resourcepolicies/resource_policies.go +++ b/internal/resourcepolicies/resource_policies.go @@ -155,10 +155,35 @@ func unmarshalResourcePolicies(yamlData *string) (*ResourcePolicies, error) { return nil, fmt.Errorf("pvcLabels must be a map of string to string, got %T", raw) } } + if raw, ok := vp.Conditions["pvcVolumeMode"]; ok { + if _, ok := raw.(string); !ok { + return nil, fmt.Errorf("pvcVolumeMode must be a string, got %T", raw) + } + } + if raw, ok := vp.Conditions["pvcAccessModes"]; ok { + if err := validateStringSliceCondition("pvcAccessModes", raw); err != nil { + return nil, err + } + } } return resPolicies, nil } +func validateStringSliceCondition(name string, raw any) error { + switch values := raw.(type) { + case []any: + for _, value := range values { + if _, ok := value.(string); !ok { + return fmt.Errorf("%s must be a list of strings, got element %T", name, value) + } + } + case []string: + default: + return fmt.Errorf("%s must be a list of strings, got %T", name, raw) + } + return nil +} + func (p *Policies) BuildPolicy(resPolicies *ResourcePolicies) error { for _, vp := range resPolicies.VolumePolicies { con, err := unmarshalVolConditions(vp.Conditions) @@ -182,6 +207,12 @@ func (p *Policies) BuildPolicy(resPolicies *ResourcePolicies) error { if len(con.PVCPhase) > 0 { volP.conditions = append(volP.conditions, &pvcPhaseCondition{phases: con.PVCPhase}) } + if con.PVCVolumeMode != "" { + volP.conditions = append(volP.conditions, &pvcVolumeModeCondition{volumeMode: con.PVCVolumeMode}) + } + if len(con.PVCAccessModes) > 0 { + volP.conditions = append(volP.conditions, &pvcAccessModesCondition{accessModes: con.PVCAccessModes}) + } p.volumePolicies = append(p.volumePolicies, volP) } @@ -320,6 +351,80 @@ func GetResourcePoliciesFromBackup( return resourcePolicies, nil } +// GetGlobalResourcePolicies loads and validates the cluster-wide global backup volume +// policies from a ConfigMap in the Velero install namespace. Only the volumePolicies +// section is honored globally; any include/exclude or fine-grained filter policies are +// ignored (a warning is logged), as those are tied to a specific backup use case. +func GetGlobalResourcePolicies( + client crclient.Client, + namespace string, + configMapName string, + logger logrus.FieldLogger, +) (*Policies, error) { + cm := &corev1api.ConfigMap{} + if err := client.Get(context.Background(), crclient.ObjectKey{Namespace: namespace, Name: configMapName}, cm); err != nil { + return nil, fmt.Errorf("fail to get global backup volume policies ConfigMap %s/%s: %w", namespace, configMapName, err) + } + + policies, err := getResourcePoliciesFromConfig(cm) + if err != nil { + return nil, fmt.Errorf("fail to read global backup volume policies from ConfigMap %s/%s: %w", namespace, configMapName, err) + } + if err := policies.Validate(); err != nil { + return nil, fmt.Errorf("fail to validate global backup volume policies in ConfigMap %s/%s: %w", namespace, configMapName, err) + } + + // Only volumePolicies apply globally; warn about any other filter policies that will be ignored. + if policies.includeExcludePolicy != nil || + policies.clusterScopedFilterPolicy != nil || + len(policies.namespacedFilterPolicies) > 0 { + logger.Warnf("Global backup volume policies ConfigMap %s/%s contains include/exclude or fine-grained "+ + "filter policies; these are ignored, only volumePolicies apply globally.", namespace, configMapName) + } + + // Return a fresh Policies carrying only the globally-applicable fields. Using an allowlist here + // (rather than nil-ing out the ignored fields) means any filter field added to Policies in the + // future is excluded from the global policies by default, without needing to update this code. + return &Policies{ + version: policies.version, + volumePolicies: policies.volumePolicies, + }, nil +} + +// GetResourcePoliciesFromBackupWithGlobal builds the effective resource policies for a backup +// by merging the backup-referenced resource policies with the global backup volume policies +// (when globalConfigMapName is set). The merged volumePolicies list is the backup-level +// policies followed by the global ones, so the first match wins and a backup can override the +// global baseline for a specific volume while still inheriting the rest of the global rules. +func GetResourcePoliciesFromBackupWithGlobal( + backup velerov1api.Backup, + client crclient.Client, + globalConfigMapName string, + installNamespace string, + logger logrus.FieldLogger, +) (*Policies, error) { + backupPolicies, err := GetResourcePoliciesFromBackup(backup, client, logger) + if err != nil { + return nil, err + } + + if globalConfigMapName == "" { + return backupPolicies, nil + } + + globalPolicies, err := GetGlobalResourcePolicies(client, installNamespace, globalConfigMapName, logger) + if err != nil { + return nil, err + } + + if backupPolicies == nil { + return globalPolicies, nil + } + // Backup-level policies first, then global, so backups can override the global baseline. + backupPolicies.volumePolicies = append(backupPolicies.volumePolicies, globalPolicies.volumePolicies...) + return backupPolicies, nil +} + func getResourcePoliciesFromConfig(cm *corev1api.ConfigMap) (*Policies, error) { if cm == nil { return nil, fmt.Errorf("could not parse config from nil configmap") diff --git a/internal/resourcepolicies/resource_policies_test.go b/internal/resourcepolicies/resource_policies_test.go index e5736a0e8..cea7ed2cf 100644 --- a/internal/resourcepolicies/resource_policies_test.go +++ b/internal/resourcepolicies/resource_policies_test.go @@ -18,13 +18,21 @@ package resourcepolicies import ( "testing" + "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1api "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + velerotest "github.com/vmware-tanzu/velero/pkg/test" ) +func pvcVolumeMode(mode corev1api.PersistentVolumeMode) *corev1api.PersistentVolumeMode { + return &mode +} + func TestLoadResourcePolicies(t *testing.T) { testCases := []struct { name string @@ -158,6 +166,52 @@ volumePolicies: `, wantErr: false, }, + { + name: "supported format pvcVolumeMode", + yamlData: `version: v1 +volumePolicies: + - conditions: + pvcVolumeMode: Block + action: + type: skip +`, + wantErr: false, + }, + { + name: "error format of pvcVolumeMode (not a string)", + yamlData: `version: v1 +volumePolicies: + - conditions: + pvcVolumeMode: + - Block + action: + type: skip +`, + wantErr: true, + }, + { + name: "supported format pvcAccessModes", + yamlData: `version: v1 +volumePolicies: + - conditions: + pvcAccessModes: + - ReadWriteOnce + action: + type: skip +`, + wantErr: false, + }, + { + name: "error format of pvcAccessModes (not a list)", + yamlData: `version: v1 +volumePolicies: + - conditions: + pvcAccessModes: ReadWriteOnce + action: + type: skip +`, + wantErr: true, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { @@ -1046,6 +1100,271 @@ volumePolicies: }, skip: true, }, + { + name: "PVC volume mode matching - Block volume mode should skip", + yamlData: `version: v1 +volumePolicies: +- conditions: + pvcVolumeMode: Block + action: + type: skip`, + vol: nil, + podVol: nil, + pvc: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "pvc-block", + }, + Spec: corev1api.PersistentVolumeClaimSpec{ + VolumeMode: pvcVolumeMode(corev1api.PersistentVolumeBlock), + }, + }, + skip: true, + }, + { + name: "PVC volume mode matching - Filesystem volume mode should not skip", + yamlData: `version: v1 +volumePolicies: +- conditions: + pvcVolumeMode: Block + action: + type: skip`, + vol: nil, + podVol: nil, + pvc: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "pvc-filesystem", + }, + Spec: corev1api.PersistentVolumeClaimSpec{ + VolumeMode: pvcVolumeMode(corev1api.PersistentVolumeFilesystem), + }, + }, + skip: false, + }, + { + name: "PVC volume mode matching - nil volume mode should not match Filesystem", + yamlData: `version: v1 +volumePolicies: +- conditions: + pvcVolumeMode: Filesystem + action: + type: skip`, + vol: nil, + podVol: nil, + pvc: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "pvc-without-volume-mode", + }, + }, + skip: false, + }, + { + name: "PVC volume mode matching - unknown condition value should not match empty volume mode", + yamlData: `version: v1 +volumePolicies: +- conditions: + pvcVolumeMode: foo + action: + type: skip`, + vol: nil, + podVol: nil, + pvc: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "pvc-without-volume-mode", + }, + }, + skip: false, + }, + { + name: "PVC volume mode matching - omitted condition should not restrict volume mode", + yamlData: `version: v1 +volumePolicies: +- conditions: + pvcAccessModes: ["ReadWriteOnce"] + action: + type: skip`, + vol: nil, + podVol: nil, + pvc: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "pvc-block-rwo", + }, + Spec: corev1api.PersistentVolumeClaimSpec{ + VolumeMode: pvcVolumeMode(corev1api.PersistentVolumeBlock), + AccessModes: []corev1api.PersistentVolumeAccessMode{corev1api.ReadWriteOnce}, + }, + }, + skip: true, + }, + { + name: "PVC volume mode matching - non-PVC volume should not match", + yamlData: `version: v1 +volumePolicies: +- conditions: + pvcVolumeMode: Filesystem + action: + type: skip`, + vol: nil, + podVol: &corev1api.Volume{ + Name: "empty-dir-volume", + VolumeSource: corev1api.VolumeSource{ + EmptyDir: &corev1api.EmptyDirVolumeSource{}, + }, + }, + pvc: nil, + skip: false, + }, + { + name: "PVC access modes matching - non-PVC volume should not match", + yamlData: `version: v1 +volumePolicies: +- conditions: + pvcAccessModes: ["ReadWriteOnce"] + action: + type: skip`, + vol: nil, + podVol: &corev1api.Volume{ + Name: "configmap-volume", + VolumeSource: corev1api.VolumeSource{ + ConfigMap: &corev1api.ConfigMapVolumeSource{}, + }, + }, + pvc: nil, + skip: false, + }, + + { + name: "PVC access modes matching - ReadWriteOnce should skip", + yamlData: `version: v1 +volumePolicies: +- conditions: + pvcAccessModes: ["ReadWriteOnce"] + action: + type: skip`, + vol: nil, + podVol: nil, + pvc: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "pvc-rwo", + }, + Spec: corev1api.PersistentVolumeClaimSpec{ + AccessModes: []corev1api.PersistentVolumeAccessMode{corev1api.ReadWriteOnce}, + }, + }, + skip: true, + }, + { + name: "PVC access modes matching - extra PVC access mode should not skip", + yamlData: `version: v1 +volumePolicies: +- conditions: + pvcAccessModes: ["ReadWriteOnce"] + action: + type: skip`, + vol: nil, + podVol: nil, + pvc: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "pvc-rwo-rom", + }, + Spec: corev1api.PersistentVolumeClaimSpec{ + AccessModes: []corev1api.PersistentVolumeAccessMode{corev1api.ReadWriteOnce, corev1api.ReadOnlyMany}, + }, + }, + skip: false, + }, + { + name: "PVC access modes matching - ReadWriteMany should not skip", + yamlData: `version: v1 +volumePolicies: +- conditions: + pvcAccessModes: ["ReadWriteOnce"] + action: + type: skip`, + vol: nil, + podVol: nil, + pvc: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "pvc-rwx", + }, + Spec: corev1api.PersistentVolumeClaimSpec{ + AccessModes: []corev1api.PersistentVolumeAccessMode{corev1api.ReadWriteMany}, + }, + }, + skip: false, + }, + { + name: "PVC access modes matching - exact access mode set should match regardless of order", + yamlData: `version: v1 +volumePolicies: +- conditions: + pvcAccessModes: ["ReadWriteMany", "ReadOnlyMany"] + action: + type: skip`, + vol: nil, + podVol: nil, + pvc: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "pvc-rom-rwx", + }, + Spec: corev1api.PersistentVolumeClaimSpec{ + AccessModes: []corev1api.PersistentVolumeAccessMode{corev1api.ReadOnlyMany, corev1api.ReadWriteMany}, + }, + }, + skip: true, + }, + { + name: "PVC access modes matching - missing one configured access mode should not skip", + yamlData: `version: v1 +volumePolicies: +- conditions: + pvcAccessModes: ["ReadOnlyMany", "ReadWriteMany"] + action: + type: skip`, + vol: nil, + podVol: nil, + pvc: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "pvc-rwx", + }, + Spec: corev1api.PersistentVolumeClaimSpec{ + AccessModes: []corev1api.PersistentVolumeAccessMode{corev1api.ReadWriteMany}, + }, + }, + skip: false, + }, + { + name: "PVC access modes matching - Combined with volume mode", + yamlData: `version: v1 +volumePolicies: +- conditions: + pvcVolumeMode: Block + pvcAccessModes: ["ReadWriteOnce"] + action: + type: skip`, + vol: nil, + podVol: nil, + pvc: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "pvc-block-rwo", + }, + Spec: corev1api.PersistentVolumeClaimSpec{ + VolumeMode: pvcVolumeMode(corev1api.PersistentVolumeBlock), + AccessModes: []corev1api.PersistentVolumeAccessMode{corev1api.ReadWriteOnce}, + }, + }, + skip: true, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { @@ -1119,28 +1438,36 @@ func TestGetMatchAction_Errors(t *testing.T) { func TestParsePVC(t *testing.T) { tests := []struct { - name string - pvc *corev1api.PersistentVolumeClaim - expectedLabels map[string]string - expectedPhase string - expectErr bool + name string + pvc *corev1api.PersistentVolumeClaim + expectedLabels map[string]string + expectedPhase string + expectedVolumeMode string + expectedAccessModes []string + expectErr bool }{ { - name: "valid PVC with labels and Pending phase", + name: "valid PVC with labels, Pending phase, Block volume mode, and access modes", pvc: &corev1api.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{"env": "prod"}, }, + Spec: corev1api.PersistentVolumeClaimSpec{ + VolumeMode: pvcVolumeMode(corev1api.PersistentVolumeBlock), + AccessModes: []corev1api.PersistentVolumeAccessMode{corev1api.ReadWriteOnce, corev1api.ReadOnlyMany}, + }, Status: corev1api.PersistentVolumeClaimStatus{ Phase: corev1api.ClaimPending, }, }, - expectedLabels: map[string]string{"env": "prod"}, - expectedPhase: "Pending", - expectErr: false, + expectedLabels: map[string]string{"env": "prod"}, + expectedPhase: "Pending", + expectedVolumeMode: "Block", + expectedAccessModes: []string{"ReadWriteOnce", "ReadOnlyMany"}, + expectErr: false, }, { - name: "valid PVC with Bound phase", + name: "valid PVC with Bound phase and nil volume mode", pvc: &corev1api.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{}, @@ -1149,27 +1476,52 @@ func TestParsePVC(t *testing.T) { Phase: corev1api.ClaimBound, }, }, - expectedLabels: nil, - expectedPhase: "Bound", - expectErr: false, + expectedLabels: nil, + expectedPhase: "Bound", + expectedVolumeMode: "", + expectedAccessModes: nil, + expectErr: false, }, { - name: "valid PVC with Lost phase", + name: "valid PVC with Lost phase and Filesystem volume mode", pvc: &corev1api.PersistentVolumeClaim{ + Spec: corev1api.PersistentVolumeClaimSpec{ + VolumeMode: pvcVolumeMode(corev1api.PersistentVolumeFilesystem), + }, Status: corev1api.PersistentVolumeClaimStatus{ Phase: corev1api.ClaimLost, }, }, - expectedLabels: nil, - expectedPhase: "Lost", - expectErr: false, + expectedLabels: nil, + expectedPhase: "Lost", + expectedVolumeMode: "Filesystem", + expectedAccessModes: nil, + expectErr: false, }, { - name: "nil PVC pointer", - pvc: (*corev1api.PersistentVolumeClaim)(nil), - expectedLabels: nil, - expectedPhase: "", - expectErr: false, + name: "valid PVC with unknown non-nil volume mode", + pvc: &corev1api.PersistentVolumeClaim{ + Spec: corev1api.PersistentVolumeClaimSpec{ + VolumeMode: pvcVolumeMode(corev1api.PersistentVolumeMode("foo")), + }, + Status: corev1api.PersistentVolumeClaimStatus{ + Phase: corev1api.ClaimBound, + }, + }, + expectedLabels: nil, + expectedPhase: "Bound", + expectedVolumeMode: "foo", + expectedAccessModes: nil, + expectErr: false, + }, + { + name: "nil PVC pointer", + pvc: (*corev1api.PersistentVolumeClaim)(nil), + expectedLabels: nil, + expectedPhase: "", + expectedVolumeMode: "", + expectedAccessModes: nil, + expectErr: false, }, } @@ -1180,6 +1532,8 @@ func TestParsePVC(t *testing.T) { assert.Equal(t, tc.expectedLabels, s.pvcLabels) assert.Equal(t, tc.expectedPhase, s.pvcPhase) + assert.Equal(t, tc.expectedVolumeMode, s.pvcVolumeMode) + assert.Equal(t, tc.expectedAccessModes, s.pvcAccessModes) }) } } @@ -1509,7 +1863,7 @@ namespacedFilterPolicies: - namespaces: ["team-frontend-*", "specific-ns"] resourceFilters: - kinds: ["Pod", "ConfigMap", "Secret"] -- namespaces: ["team-*", "another-pattern"] +- namespaces: ["team-*", "another-pattern"] resourceFilters: - kinds: ["Deployment", "Service"]` @@ -1680,3 +2034,334 @@ clusterScopedFilterPolicy: }) } } + +func TestPVCVolumeModeMatch(t *testing.T) { + tests := []struct { + name string + condition *pvcVolumeModeCondition + volume *structuredVolume + expectedMatch bool + }{ + { + name: "match Block volume mode", + condition: &pvcVolumeModeCondition{volumeMode: "Block"}, + volume: &structuredVolume{pvcVolumeMode: "Block"}, + expectedMatch: true, + }, + { + name: "match Filesystem volume mode", + condition: &pvcVolumeModeCondition{volumeMode: "Filesystem"}, + volume: &structuredVolume{pvcVolumeMode: "Filesystem"}, + expectedMatch: true, + }, + { + name: "no match for different volume mode", + condition: &pvcVolumeModeCondition{volumeMode: "Block"}, + volume: &structuredVolume{pvcVolumeMode: "Filesystem"}, + expectedMatch: false, + }, + { + name: "case-sensitive no match for lowercase volume mode", + condition: &pvcVolumeModeCondition{volumeMode: "block"}, + volume: &structuredVolume{pvcVolumeMode: "Block"}, + expectedMatch: false, + }, + { + name: "no match for unknown condition value against Filesystem", + condition: &pvcVolumeModeCondition{volumeMode: "foo"}, + volume: &structuredVolume{pvcVolumeMode: "Filesystem"}, + expectedMatch: false, + }, + { + name: "match unknown condition value only when volume has same value", + condition: &pvcVolumeModeCondition{volumeMode: "foo"}, + volume: &structuredVolume{pvcVolumeMode: "foo"}, + expectedMatch: true, + }, + { + name: "no match for empty volume mode", + condition: &pvcVolumeModeCondition{volumeMode: "Block"}, + volume: &structuredVolume{pvcVolumeMode: ""}, + expectedMatch: false, + }, + { + name: "match with empty volume mode condition (always match)", + condition: &pvcVolumeModeCondition{volumeMode: ""}, + volume: &structuredVolume{pvcVolumeMode: "Block"}, + expectedMatch: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := tc.condition.match(tc.volume) + assert.Equal(t, tc.expectedMatch, result) + }) + } +} + +func TestPVCAccessModesMatch(t *testing.T) { + tests := []struct { + name string + condition *pvcAccessModesCondition + volume *structuredVolume + expectedMatch bool + }{ + { + name: "match ReadWriteOnce access mode", + condition: &pvcAccessModesCondition{accessModes: []string{"ReadWriteOnce"}}, + volume: &structuredVolume{pvcAccessModes: []string{"ReadWriteOnce"}}, + expectedMatch: true, + }, + { + name: "match exact multiple access modes", + condition: &pvcAccessModesCondition{accessModes: []string{"ReadWriteOnce", "ReadOnlyMany"}}, + volume: &structuredVolume{pvcAccessModes: []string{"ReadWriteOnce", "ReadOnlyMany"}}, + expectedMatch: true, + }, + { + name: "match exact multiple access modes regardless of order", + condition: &pvcAccessModesCondition{accessModes: []string{"ReadOnlyMany", "ReadWriteOnce"}}, + volume: &structuredVolume{pvcAccessModes: []string{"ReadWriteOnce", "ReadOnlyMany"}}, + expectedMatch: true, + }, + { + name: "no match when one of multiple access modes is missing", + condition: &pvcAccessModesCondition{accessModes: []string{"ReadWriteOnce", "ReadOnlyMany"}}, + volume: &structuredVolume{pvcAccessModes: []string{"ReadOnlyMany"}}, + expectedMatch: false, + }, + { + name: "no match when PVC has extra access modes", + condition: &pvcAccessModesCondition{accessModes: []string{"ReadWriteMany"}}, + volume: &structuredVolume{pvcAccessModes: []string{"ReadWriteOnce", "ReadWriteMany"}}, + expectedMatch: false, + }, + { + name: "no match for different access mode", + condition: &pvcAccessModesCondition{accessModes: []string{"ReadWriteOnce"}}, + volume: &structuredVolume{pvcAccessModes: []string{"ReadWriteMany"}}, + expectedMatch: false, + }, + { + name: "case-sensitive no match for lowercase access mode", + condition: &pvcAccessModesCondition{accessModes: []string{"readwriteonce"}}, + volume: &structuredVolume{pvcAccessModes: []string{"ReadWriteOnce"}}, + expectedMatch: false, + }, + { + name: "no match for empty PVC access modes", + condition: &pvcAccessModesCondition{accessModes: []string{"ReadWriteOnce"}}, + volume: &structuredVolume{pvcAccessModes: []string{}}, + expectedMatch: false, + }, + { + name: "match with empty access modes list (always match)", + condition: &pvcAccessModesCondition{accessModes: []string{}}, + volume: &structuredVolume{pvcAccessModes: []string{"ReadWriteOnce"}}, + expectedMatch: true, + }, + { + name: "match with nil access modes list (always match)", + condition: &pvcAccessModesCondition{accessModes: nil}, + volume: &structuredVolume{pvcAccessModes: []string{"ReadWriteOnce"}}, + expectedMatch: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := tc.condition.match(tc.volume) + assert.Equal(t, tc.expectedMatch, result) + }) + } +} + +// ---- Global backup volume policies ---- + +func globalPolicyConfigMap(name, data string) *corev1api.ConfigMap { + return &corev1api.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Namespace: "velero", Name: name}, + Data: map[string]string{"policies.yaml": data}, + } +} + +func backupWithPolicy(ref string) velerov1api.Backup { + b := velerov1api.Backup{ObjectMeta: metav1.ObjectMeta{Namespace: "velero", Name: "backup"}} + if ref != "" { + b.Spec.ResourcePolicy = &corev1api.TypedLocalObjectReference{Kind: ConfigmapRefType, Name: ref} + } + return b +} + +// firstActionFor returns the action type the policies select for a PV with the given storage +// class, or "" when nothing matches. It exercises the compiled match logic so the tests verify +// merge ordering rather than internal field layout. +func firstActionFor(p *Policies, storageClass string) VolumeActionType { + pv := &corev1api.PersistentVolume{Spec: corev1api.PersistentVolumeSpec{StorageClassName: storageClass}} + vol := &structuredVolume{} + vol.parsePV(pv) + if a := p.match(vol); a != nil { + return a.Type + } + return "" +} + +func TestGetResourcePoliciesFromBackupWithGlobal(t *testing.T) { + gp2Skip := `version: v1 +volumePolicies: + - conditions: + storageClass: + - gp2 + action: + type: skip +` + gp2Snapshot := `version: v1 +volumePolicies: + - conditions: + storageClass: + - gp2 + action: + type: snapshot +` + otherFsBackup := `version: v1 +volumePolicies: + - conditions: + storageClass: + - other + action: + type: fs-backup +` + + tests := []struct { + name string + backupCM *corev1api.ConfigMap + globalCMName string + globalCM *corev1api.ConfigMap + backupRef string + expectErr bool + expectedGp2Action VolumeActionType + expectedNumPolicies int + }{ + { + name: "no global, backup only - unchanged behavior", + backupRef: "backup01", + backupCM: globalPolicyConfigMap("backup01", gp2Snapshot), + expectedGp2Action: Snapshot, + expectedNumPolicies: 1, + }, + { + name: "global only, backup has no policy", + globalCMName: "global", + globalCM: globalPolicyConfigMap("global", gp2Skip), + expectedGp2Action: Skip, + expectedNumPolicies: 1, + }, + { + name: "no global configured and no backup policy", + expectedGp2Action: "", + expectedNumPolicies: 0, + }, + { + name: "merge - backup policy overrides global for gp2", + backupRef: "backup01", + backupCM: globalPolicyConfigMap("backup01", gp2Snapshot), + globalCMName: "global", + globalCM: globalPolicyConfigMap("global", gp2Skip), + expectedGp2Action: Snapshot, // backup-level wins (evaluated first) + expectedNumPolicies: 2, + }, + { + name: "merge - backup inherits non-overlapping global rule", + backupRef: "backup01", + backupCM: globalPolicyConfigMap("backup01", otherFsBackup), + globalCMName: "global", + globalCM: globalPolicyConfigMap("global", gp2Skip), + expectedGp2Action: Skip, // only global matches gp2 + expectedNumPolicies: 2, + }, + { + name: "global configmap missing - error", + globalCMName: "global", + expectErr: true, + }, + { + name: "global configmap invalid - error", + globalCMName: "global", + globalCM: globalPolicyConfigMap("global", "not: [valid"), + expectErr: true, + }, + { + // Parses cleanly but fails Policies.Validate() due to the unsupported version. + name: "global configmap fails validation - error", + globalCMName: "global", + globalCM: globalPolicyConfigMap("global", "version: v2\nvolumePolicies: []\n"), + expectErr: true, + }, + { + // Backup references a ResourcePolicy ConfigMap that does not exist, so resolving the + // backup-level policies fails before the global ones are consulted. + name: "backup configmap missing - error", + backupRef: "missing-backup-cm", + globalCMName: "global", + globalCM: globalPolicyConfigMap("global", gp2Skip), + expectErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := velerotest.NewFakeControllerRuntimeClient(t) + if tc.backupCM != nil { + require.NoError(t, client.Create(t.Context(), tc.backupCM)) + } + if tc.globalCM != nil { + require.NoError(t, client.Create(t.Context(), tc.globalCM)) + } + + b := backupWithPolicy(tc.backupRef) + + p, err := GetResourcePoliciesFromBackupWithGlobal(b, client, tc.globalCMName, "velero", logrus.New()) + if tc.expectErr { + require.Error(t, err) + return + } + require.NoError(t, err) + + if tc.expectedNumPolicies == 0 { + assert.Nil(t, p) + return + } + require.NotNil(t, p) + assert.Len(t, p.volumePolicies, tc.expectedNumPolicies) + assert.Equal(t, tc.expectedGp2Action, firstActionFor(p, "gp2")) + }) + } +} + +func TestGetGlobalResourcePoliciesIgnoresNonVolumePolicies(t *testing.T) { + data := `version: v1 +volumePolicies: + - conditions: + storageClass: + - gp2 + action: + type: skip +namespacedFilterPolicies: +- namespaces: ["frontend"] + resourceFilters: + - kinds: ["Pod"] +` + client := velerotest.NewFakeControllerRuntimeClient(t) + require.NoError(t, client.Create(t.Context(), globalPolicyConfigMap("global", data))) + + p, err := GetGlobalResourcePolicies(client, "velero", "global", logrus.New()) + require.NoError(t, err) + require.NotNil(t, p) + + // Only volumePolicies are kept; the namespaced filter policy is dropped. + assert.Len(t, p.volumePolicies, 1) + assert.Empty(t, p.GetNamespacedFilterPolicies()) + assert.Nil(t, p.GetIncludeExcludePolicy()) + assert.Nil(t, p.GetClusterScopedFilterPolicy()) +} diff --git a/internal/resourcepolicies/volume_resources.go b/internal/resourcepolicies/volume_resources.go index 65f15f54e..29514b9ee 100644 --- a/internal/resourcepolicies/volume_resources.go +++ b/internal/resourcepolicies/volume_resources.go @@ -18,9 +18,11 @@ package resourcepolicies import ( "bytes" "fmt" + "slices" "strings" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/util/sets" "github.com/cockroachdb/errors" "go.yaml.in/yaml/v3" @@ -45,13 +47,15 @@ type capacity struct { } type structuredVolume struct { - capacity resource.Quantity - storageClass string - nfs *nFSVolumeSource - csi *csiVolumeSource - volumeType SupportedVolume - pvcLabels map[string]string - pvcPhase string + capacity resource.Quantity + storageClass string + nfs *nFSVolumeSource + csi *csiVolumeSource + volumeType SupportedVolume + pvcLabels map[string]string + pvcPhase string + pvcVolumeMode string + pvcAccessModes []string } func (s *structuredVolume) parsePV(pv *corev1api.PersistentVolume) { @@ -76,6 +80,15 @@ func (s *structuredVolume) parsePVC(pvc *corev1api.PersistentVolumeClaim) { s.pvcLabels = pvc.Labels } s.pvcPhase = string(pvc.Status.Phase) + if pvc.Spec.VolumeMode != nil { + s.pvcVolumeMode = string(*pvc.Spec.VolumeMode) + } + if len(pvc.Spec.AccessModes) > 0 { + s.pvcAccessModes = make([]string, 0, len(pvc.Spec.AccessModes)) + for _, accessMode := range pvc.Spec.AccessModes { + s.pvcAccessModes = append(s.pvcAccessModes, string(accessMode)) + } + } } } @@ -127,18 +140,55 @@ func (c *pvcPhaseCondition) match(v *structuredVolume) bool { if v.pvcPhase == "" { return false } - for _, phase := range c.phases { - if v.pvcPhase == phase { - return true - } - } - return false + return slices.Contains(c.phases, v.pvcPhase) } func (c *pvcPhaseCondition) validate() error { return nil } +// pvcVolumeModeCondition defines a condition that matches if the PVC's volume mode matches the provided volume mode. +type pvcVolumeModeCondition struct { + volumeMode string +} + +func (c *pvcVolumeModeCondition) match(v *structuredVolume) bool { + // No volume mode specified: always match. + if c.volumeMode == "" { + return true + } + + // Here allows unknown strings for forward compatibility. If Kubernetes adds another volume mode later, + // Velero would not reject the policy just because the string is unfamiliar. + return v.pvcVolumeMode == c.volumeMode +} + +func (c *pvcVolumeModeCondition) validate() error { + return nil +} + +// pvcAccessModesCondition defines a condition that matches if the PVC has exactly the provided access modes. +type pvcAccessModesCondition struct { + accessModes []string +} + +func (c *pvcAccessModesCondition) match(v *structuredVolume) bool { + // No access modes specified: always match. + if len(c.accessModes) == 0 { + return true + } + + if len(v.pvcAccessModes) != len(c.accessModes) { + return false + } + + return sets.New(c.accessModes...).Equal(sets.New(v.pvcAccessModes...)) +} + +func (c *pvcAccessModesCondition) validate() error { + return nil +} + type capacityCondition struct { capacity capacity } diff --git a/internal/resourcepolicies/volume_resources_test.go b/internal/resourcepolicies/volume_resources_test.go index b55692326..02850bb7f 100644 --- a/internal/resourcepolicies/volume_resources_test.go +++ b/internal/resourcepolicies/volume_resources_test.go @@ -430,6 +430,38 @@ func TestUnmarshalVolumeConditions(t *testing.T) { }, expectedError: "!!str `production` into map[string]string", }, + { + name: "Valid pvcVolumeMode input", + input: map[string]any{ + "capacity": "1Gi,10Gi", + "pvcVolumeMode": "Block", + }, + expectedError: "", + }, + { + name: "Invalid pvcVolumeMode input: not a string", + input: map[string]any{ + "capacity": "1Gi,10Gi", + "pvcVolumeMode": []string{"Filesystem", "Block"}, + }, + expectedError: "cannot unmarshal !!seq", + }, + { + name: "Valid pvcAccessModes input", + input: map[string]any{ + "capacity": "1Gi,10Gi", + "pvcAccessModes": []string{"ReadWriteOnce", "ReadWriteMany"}, + }, + expectedError: "", + }, + { + name: "Invalid pvcAccessModes input: not a list", + input: map[string]any{ + "capacity": "1Gi,10Gi", + "pvcAccessModes": "ReadWriteOnce", + }, + expectedError: "cannot unmarshal !!str", + }, } for _, tc := range testCases { diff --git a/internal/resourcepolicies/volume_resources_validator.go b/internal/resourcepolicies/volume_resources_validator.go index e144e8281..928e17df6 100644 --- a/internal/resourcepolicies/volume_resources_validator.go +++ b/internal/resourcepolicies/volume_resources_validator.go @@ -40,13 +40,15 @@ type nFSVolumeSource struct { // volumeConditions defined the current format of conditions we parsed type volumeConditions struct { - Capacity string `yaml:"capacity,omitempty"` - StorageClass []string `yaml:"storageClass,omitempty"` - NFS *nFSVolumeSource `yaml:"nfs,omitempty"` - CSI *csiVolumeSource `yaml:"csi,omitempty"` - VolumeTypes []SupportedVolume `yaml:"volumeTypes,omitempty"` - PVCLabels map[string]string `yaml:"pvcLabels,omitempty"` - PVCPhase []string `yaml:"pvcPhase,omitempty"` + Capacity string `yaml:"capacity,omitempty"` + StorageClass []string `yaml:"storageClass,omitempty"` + NFS *nFSVolumeSource `yaml:"nfs,omitempty"` + CSI *csiVolumeSource `yaml:"csi,omitempty"` + VolumeTypes []SupportedVolume `yaml:"volumeTypes,omitempty"` + PVCLabels map[string]string `yaml:"pvcLabels,omitempty"` + PVCPhase []string `yaml:"pvcPhase,omitempty"` + PVCVolumeMode string `yaml:"pvcVolumeMode,omitempty"` + PVCAccessModes []string `yaml:"pvcAccessModes,omitempty"` } func (c *capacityCondition) validate() error { diff --git a/pkg/apis/velero/v1/labels_annotations.go b/pkg/apis/velero/v1/labels_annotations.go index 921af498e..13da279d8 100644 --- a/pkg/apis/velero/v1/labels_annotations.go +++ b/pkg/apis/velero/v1/labels_annotations.go @@ -80,6 +80,11 @@ const ( // timeout value for backup to plugins. ResourceTimeoutAnnotation = "velero.io/resource-timeout" + // GlobalBackupVolumePolicyConfigMapAnnotation is the annotation key used to record the + // name of the cluster-wide global backup volume policies ConfigMap that contributed to a + // backup, so that `velero backup describe` can surface it. + GlobalBackupVolumePolicyConfigMapAnnotation = "velero.io/global-backup-volume-policy-configmap" + // AsyncOperationIDLabel is the label key used to identify the async operation ID AsyncOperationIDLabel = "velero.io/async-operation-id" diff --git a/pkg/cmd/server/config/config.go b/pkg/cmd/server/config/config.go index e19086217..c8080da21 100644 --- a/pkg/cmd/server/config/config.go +++ b/pkg/cmd/server/config/config.go @@ -145,42 +145,43 @@ var ( ) type Config struct { - PluginDir string - MetricsAddress string - DefaultBackupLocation string // TODO(2.0) Deprecate defaultBackupLocation - BackupSyncPeriod time.Duration - PodVolumeOperationTimeout time.Duration - ResourceTerminatingTimeout time.Duration - DefaultBackupTTL time.Duration - DefaultVGSLabelKey string - StoreValidationFrequency time.Duration - DefaultCSISnapshotTimeout time.Duration - DefaultItemOperationTimeout time.Duration - ResourceTimeout time.Duration - RestoreResourcePriorities types.Priorities - DefaultVolumeSnapshotLocations flag.Map - RestoreOnly bool - DisabledControllers []string - ClientQPS float32 - ClientBurst int - ClientPageSize int - ProfilerAddress string - LogLevel *logging.LevelFlag - LogFormat *logging.FormatFlag - RepoMaintenanceFrequency time.Duration - GarbageCollectionFrequency time.Duration - ItemOperationSyncFrequency time.Duration - DefaultVolumesToFsBackup bool - UploaderType string - MaxConcurrentK8SConnections int - DefaultSnapshotMoveData bool - DisableInformerCache bool - ScheduleSkipImmediately bool - CredentialsDirectory string - BackupRepoConfig string - RepoMaintenanceJobConfig string - ItemBlockWorkerCount int - ConcurrentBackups int + PluginDir string + MetricsAddress string + DefaultBackupLocation string // TODO(2.0) Deprecate defaultBackupLocation + BackupSyncPeriod time.Duration + PodVolumeOperationTimeout time.Duration + ResourceTerminatingTimeout time.Duration + DefaultBackupTTL time.Duration + DefaultVGSLabelKey string + StoreValidationFrequency time.Duration + DefaultCSISnapshotTimeout time.Duration + DefaultItemOperationTimeout time.Duration + ResourceTimeout time.Duration + RestoreResourcePriorities types.Priorities + DefaultVolumeSnapshotLocations flag.Map + RestoreOnly bool + DisabledControllers []string + ClientQPS float32 + ClientBurst int + ClientPageSize int + ProfilerAddress string + LogLevel *logging.LevelFlag + LogFormat *logging.FormatFlag + RepoMaintenanceFrequency time.Duration + GarbageCollectionFrequency time.Duration + ItemOperationSyncFrequency time.Duration + DefaultVolumesToFsBackup bool + UploaderType string + MaxConcurrentK8SConnections int + DefaultSnapshotMoveData bool + DisableInformerCache bool + ScheduleSkipImmediately bool + CredentialsDirectory string + BackupRepoConfig string + RepoMaintenanceJobConfig string + ItemBlockWorkerCount int + ConcurrentBackups int + GlobalBackupVolumePoliciesConfigMap string } func GetDefaultConfig() *Config { @@ -275,4 +276,10 @@ func (c *Config) BindFlags(flags *pflag.FlagSet) { c.ConcurrentBackups, "Number of backups to process concurrently. Default is one. Optional.", ) + flags.StringVar( + &c.GlobalBackupVolumePoliciesConfigMap, + "global-backup-volume-policies-configmap", + c.GlobalBackupVolumePoliciesConfigMap, + "The name of a ConfigMap in the Velero install namespace holding global backup volume policies that are merged into every backup. Optional.", + ) } diff --git a/pkg/cmd/server/config/config_test.go b/pkg/cmd/server/config/config_test.go index ba17437f1..a0e33c413 100644 --- a/pkg/cmd/server/config/config_test.go +++ b/pkg/cmd/server/config/config_test.go @@ -5,6 +5,7 @@ import ( "github.com/spf13/pflag" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestGetDefaultConfig(t *testing.T) { @@ -17,3 +18,14 @@ func TestBindFlags(t *testing.T) { config.BindFlags(pflag.CommandLine) assert.Equal(t, 1, config.ItemBlockWorkerCount) } + +func TestGlobalBackupVolumePoliciesConfigMapFlag(t *testing.T) { + config := GetDefaultConfig() + // Opt-in: defaults to empty. + assert.Empty(t, config.GlobalBackupVolumePoliciesConfigMap) + + flags := pflag.NewFlagSet("test", pflag.ContinueOnError) + config.BindFlags(flags) + require.NoError(t, flags.Parse([]string{"--global-backup-volume-policies-configmap", "global-volume-policy"})) + assert.Equal(t, "global-volume-policy", config.GlobalBackupVolumePoliciesConfigMap) +} diff --git a/pkg/cmd/server/server.go b/pkg/cmd/server/server.go index d382a912a..5cceceafe 100644 --- a/pkg/cmd/server/server.go +++ b/pkg/cmd/server/server.go @@ -57,6 +57,7 @@ import ( "github.com/vmware-tanzu/velero/internal/credentials" "github.com/vmware-tanzu/velero/internal/hook" + "github.com/vmware-tanzu/velero/internal/resourcepolicies" "github.com/vmware-tanzu/velero/internal/storage" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" velerov2alpha1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v2alpha1" @@ -390,6 +391,14 @@ func (s *server) setupBeforeControllerRun() error { if err := setDefaultBackupLocation(s.ctx, client, s.namespace, s.config.DefaultBackupLocation, s.logger); err != nil { return err } + + // Validate the global backup volume policies ConfigMap early, so misconfigurations fail fast. + if s.config.GlobalBackupVolumePoliciesConfigMap != "" { + if _, err := resourcepolicies.GetGlobalResourcePolicies(client, s.namespace, s.config.GlobalBackupVolumePoliciesConfigMap, s.logger); err != nil { + return err + } + s.logger.WithField("configmap", s.config.GlobalBackupVolumePoliciesConfigMap).Info("Loaded global backup volume policies") + } return nil } @@ -671,6 +680,7 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string s.config.ItemBlockWorkerCount, s.config.ConcurrentBackups, s.crClient, + s.config.GlobalBackupVolumePoliciesConfigMap, ).SetupWithManager(s.mgr); err != nil { s.logger.Fatal(err, "unable to create controller", "controller", constant.ControllerBackup) } diff --git a/pkg/cmd/util/output/backup_describer.go b/pkg/cmd/util/output/backup_describer.go index 89fc74a70..4c8222f81 100644 --- a/pkg/cmd/util/output/backup_describer.go +++ b/pkg/cmd/util/output/backup_describer.go @@ -99,6 +99,8 @@ func DescribeBackup( DescribeFineGrainedFilterPolicies(ctx, kbClient, d, backup) } + DescribeGlobalVolumePolicy(d, backup) + if backup.Spec.UploaderConfig != nil && backup.Spec.UploaderConfig.ParallelFilesUpload > 0 { d.Println() DescribeUploaderConfigForBackup(d, backup.Spec) @@ -136,6 +138,19 @@ func DescribeResourcePolicies(d *Describer, resPolicies *corev1api.TypedLocalObj d.Printf("\tName:\t%s\n", resPolicies.Name) } +// DescribeGlobalVolumePolicy describes the cluster-wide global backup volume policies +// ConfigMap that contributed to the backup, if any. +func DescribeGlobalVolumePolicy(d *Describer, backup *velerov1api.Backup) { + name := backup.Annotations[velerov1api.GlobalBackupVolumePolicyConfigMapAnnotation] + if name == "" { + return + } + d.Println() + d.Printf("Global volume policies:\n") + d.Printf("\tType:\t%s\n", resourcepolicies.ConfigmapRefType) + d.Printf("\tName:\t%s\n", name) +} + // DescribeFineGrainedFilterPolicies describes cluster-scoped and namespace-scoped filter policies if present func DescribeFineGrainedFilterPolicies(ctx context.Context, kbClient kbclient.Client, d *Describer, backup *velerov1api.Backup) { if backup.Spec.ResourcePolicy == nil { diff --git a/pkg/cmd/util/output/backup_describer_test.go b/pkg/cmd/util/output/backup_describer_test.go index 936b19422..248b0a45b 100644 --- a/pkg/cmd/util/output/backup_describer_test.go +++ b/pkg/cmd/util/output/backup_describer_test.go @@ -72,6 +72,34 @@ func TestDescribeResourcePolicies(t *testing.T) { assert.Equal(t, expect, d.buf.String()) } +func TestDescribeGlobalVolumePolicy(t *testing.T) { + newDescriber := func() *Describer { + d := &Describer{out: &tabwriter.Writer{}, buf: &bytes.Buffer{}} + d.out.Init(d.buf, 0, 8, 2, ' ', 0) + return d + } + + // No annotation: nothing is printed. + d := newDescriber() + DescribeGlobalVolumePolicy(d, builder.ForBackup("velero", "b").Result()) + d.out.Flush() + assert.Empty(t, d.buf.String()) + + // Annotation present: ConfigMap name is surfaced. + d = newDescriber() + backup := builder.ForBackup("velero", "b"). + ObjectMeta(builder.WithAnnotations(velerov1api.GlobalBackupVolumePolicyConfigMapAnnotation, "global-volume-policy")). + Result() + DescribeGlobalVolumePolicy(d, backup) + d.out.Flush() + expect := ` +Global volume policies: + Type: configmap + Name: global-volume-policy +` + assert.Equal(t, expect, d.buf.String()) +} + func TestDescribeBackupSpec(t *testing.T) { input1 := builder.ForBackup("test-ns", "test-backup-1"). IncludedNamespaces("inc-ns-1", "inc-ns-2"). diff --git a/pkg/cmd/util/output/backup_structured_describer.go b/pkg/cmd/util/output/backup_structured_describer.go index 8ec31b72c..dfffcda06 100644 --- a/pkg/cmd/util/output/backup_structured_describer.go +++ b/pkg/cmd/util/output/backup_structured_describer.go @@ -60,6 +60,8 @@ func DescribeBackupInSF( DescribeFineGrainedFilterPoliciesInSF(ctx, kbClient, d, backup) } + DescribeGlobalVolumePolicyInSF(d, backup) + status := backup.Status if len(status.ValidationErrors) > 0 { d.Describe("validationErrors", status.ValidationErrors) @@ -699,6 +701,19 @@ func DescribeResourcePoliciesInSF(d *StructuredDescriber, resPolicies *corev1api d.Describe("resourcePolicies", policiesInfo) } +// DescribeGlobalVolumePolicyInSF describes the global backup volume policies ConfigMap that +// contributed to the backup, if any, in structured format. +func DescribeGlobalVolumePolicyInSF(d *StructuredDescriber, backup *velerov1api.Backup) { + name := backup.Annotations[velerov1api.GlobalBackupVolumePolicyConfigMapAnnotation] + if name == "" { + return + } + d.Describe("globalVolumePolicies", map[string]any{ + "type": resourcepolicies.ConfigmapRefType, + "name": name, + }) +} + func describeResultInSF(m map[string]any, result results.Result) { m["velero"], m["cluster"], m["namespace"] = []string{}, []string{}, []string{} diff --git a/pkg/cmd/util/output/backup_structured_describer_test.go b/pkg/cmd/util/output/backup_structured_describer_test.go index 77d219f49..cb46a4676 100644 --- a/pkg/cmd/util/output/backup_structured_describer_test.go +++ b/pkg/cmd/util/output/backup_structured_describer_test.go @@ -627,6 +627,27 @@ func TestDescribeResourcePoliciesInSF(t *testing.T) { assert.True(t, reflect.DeepEqual(sd.output, expect)) } +func TestDescribeGlobalVolumePolicyInSF(t *testing.T) { + // No annotation: nothing is added to the output. + sd := &StructuredDescriber{output: make(map[string]any), format: ""} + DescribeGlobalVolumePolicyInSF(sd, builder.ForBackup("velero", "b").Result()) + assert.Empty(t, sd.output) + + // Annotation present: the ConfigMap name is surfaced. + sd = &StructuredDescriber{output: make(map[string]any), format: ""} + backup := builder.ForBackup("velero", "b"). + ObjectMeta(builder.WithAnnotations(velerov1api.GlobalBackupVolumePolicyConfigMapAnnotation, "global-volume-policy")). + Result() + DescribeGlobalVolumePolicyInSF(sd, backup) + expectGlobal := map[string]any{ + "globalVolumePolicies": map[string]any{ + "type": "configmap", + "name": "global-volume-policy", + }, + } + assert.True(t, reflect.DeepEqual(sd.output, expectGlobal)) +} + func TestDescribeBackupResultInSF(t *testing.T) { input := results.Result{ Velero: []string{"msg-1", "msg-2"}, diff --git a/pkg/controller/backup_controller.go b/pkg/controller/backup_controller.go index 7bb9ee5f9..161119d0c 100644 --- a/pkg/controller/backup_controller.go +++ b/pkg/controller/backup_controller.go @@ -84,32 +84,33 @@ var autoExcludeClusterScopedResources = []string{ } type backupReconciler struct { - ctx context.Context - logger logrus.FieldLogger - discoveryHelper discovery.Helper - backupper pkgbackup.Backupper - kbClient kbclient.Client - clock clock.WithTickerAndDelayedExecution - backupLogLevel logrus.Level - newPluginManager func(logrus.FieldLogger) clientmgmt.Manager - backupTracker BackupTracker - defaultBackupLocation string - defaultVolumesToFsBackup bool - defaultBackupTTL time.Duration - defaultVGSLabelKey string - defaultCSISnapshotTimeout time.Duration - resourceTimeout time.Duration - defaultItemOperationTimeout time.Duration - defaultSnapshotLocations map[string]string - metrics *metrics.ServerMetrics - backupStoreGetter persistence.ObjectBackupStoreGetter - formatFlag logging.Format - credentialFileStore credentials.FileStore - maxConcurrentK8SConnections int - defaultSnapshotMoveData bool - globalCRClient kbclient.Client - itemBlockWorkerCount int - concurrentBackups int + ctx context.Context + logger logrus.FieldLogger + discoveryHelper discovery.Helper + backupper pkgbackup.Backupper + kbClient kbclient.Client + clock clock.WithTickerAndDelayedExecution + backupLogLevel logrus.Level + newPluginManager func(logrus.FieldLogger) clientmgmt.Manager + backupTracker BackupTracker + defaultBackupLocation string + defaultVolumesToFsBackup bool + defaultBackupTTL time.Duration + defaultVGSLabelKey string + defaultCSISnapshotTimeout time.Duration + resourceTimeout time.Duration + defaultItemOperationTimeout time.Duration + defaultSnapshotLocations map[string]string + metrics *metrics.ServerMetrics + backupStoreGetter persistence.ObjectBackupStoreGetter + formatFlag logging.Format + credentialFileStore credentials.FileStore + maxConcurrentK8SConnections int + defaultSnapshotMoveData bool + globalCRClient kbclient.Client + itemBlockWorkerCount int + concurrentBackups int + globalVolumePoliciesConfigMap string } func NewBackupReconciler( @@ -138,34 +139,36 @@ func NewBackupReconciler( itemBlockWorkerCount int, concurrentBackups int, globalCRClient kbclient.Client, + globalVolumePoliciesConfigMap string, ) *backupReconciler { b := &backupReconciler{ - ctx: ctx, - discoveryHelper: discoveryHelper, - backupper: backupper, - clock: &clock.RealClock{}, - logger: logger, - backupLogLevel: backupLogLevel, - newPluginManager: newPluginManager, - backupTracker: backupTracker, - kbClient: kbClient, - defaultBackupLocation: defaultBackupLocation, - defaultVolumesToFsBackup: defaultVolumesToFsBackup, - defaultBackupTTL: defaultBackupTTL, - defaultVGSLabelKey: defaultVGSLabelKey, - defaultCSISnapshotTimeout: defaultCSISnapshotTimeout, - resourceTimeout: resourceTimeout, - defaultItemOperationTimeout: defaultItemOperationTimeout, - defaultSnapshotLocations: defaultSnapshotLocations, - metrics: metrics, - backupStoreGetter: backupStoreGetter, - formatFlag: formatFlag, - credentialFileStore: credentialStore, - maxConcurrentK8SConnections: maxConcurrentK8SConnections, - defaultSnapshotMoveData: defaultSnapshotMoveData, - itemBlockWorkerCount: itemBlockWorkerCount, - concurrentBackups: max(concurrentBackups, 1), - globalCRClient: globalCRClient, + ctx: ctx, + discoveryHelper: discoveryHelper, + backupper: backupper, + clock: &clock.RealClock{}, + logger: logger, + backupLogLevel: backupLogLevel, + newPluginManager: newPluginManager, + backupTracker: backupTracker, + kbClient: kbClient, + defaultBackupLocation: defaultBackupLocation, + defaultVolumesToFsBackup: defaultVolumesToFsBackup, + defaultBackupTTL: defaultBackupTTL, + defaultVGSLabelKey: defaultVGSLabelKey, + defaultCSISnapshotTimeout: defaultCSISnapshotTimeout, + resourceTimeout: resourceTimeout, + defaultItemOperationTimeout: defaultItemOperationTimeout, + defaultSnapshotLocations: defaultSnapshotLocations, + metrics: metrics, + backupStoreGetter: backupStoreGetter, + formatFlag: formatFlag, + credentialFileStore: credentialStore, + maxConcurrentK8SConnections: maxConcurrentK8SConnections, + defaultSnapshotMoveData: defaultSnapshotMoveData, + itemBlockWorkerCount: itemBlockWorkerCount, + concurrentBackups: max(concurrentBackups, 1), + globalCRClient: globalCRClient, + globalVolumePoliciesConfigMap: globalVolumePoliciesConfigMap, } b.updateTotalBackupMetric() return b @@ -595,9 +598,13 @@ func (b *backupReconciler) prepareBackupRequest(ctx context.Context, backup *vel request.Status.ValidationErrors = append(request.Status.ValidationErrors, "encountered labelSelector as well as orLabelSelectors in backup spec, only one can be specified") } - resourcePolicies, err := resourcepolicies.GetResourcePoliciesFromBackup(*request.Backup, b.kbClient, logger) + resourcePolicies, err := resourcepolicies.GetResourcePoliciesFromBackupWithGlobal( + *request.Backup, b.kbClient, b.globalVolumePoliciesConfigMap, request.Namespace, logger) if err != nil { request.Status.ValidationErrors = append(request.Status.ValidationErrors, err.Error()) + } else if b.globalVolumePoliciesConfigMap != "" { + // Record the contributing global volume policies ConfigMap so `velero backup describe` can surface it. + request.Annotations[velerov1api.GlobalBackupVolumePolicyConfigMapAnnotation] = b.globalVolumePoliciesConfigMap } if resourcePolicies != nil && resourcePolicies.GetIncludeExcludePolicy() != nil && collections.UseOldResourceFilters(request.Spec) { request.Status.ValidationErrors = append(request.Status.ValidationErrors, "include-resources, exclude-resources and include-cluster-resources are old filter parameters.\n"+ diff --git a/pkg/controller/backup_controller_test.go b/pkg/controller/backup_controller_test.go index 0ff76cf63..6a7e68114 100644 --- a/pkg/controller/backup_controller_test.go +++ b/pkg/controller/backup_controller_test.go @@ -45,6 +45,7 @@ import ( kbclient "sigs.k8s.io/controller-runtime/pkg/client" fakeClient "sigs.k8s.io/controller-runtime/pkg/client/fake" + "github.com/vmware-tanzu/velero/internal/resourcepolicies" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" pkgbackup "github.com/vmware-tanzu/velero/pkg/backup" "github.com/vmware-tanzu/velero/pkg/builder" @@ -2075,6 +2076,100 @@ namespacedFilterPolicies: assert.True(t, hasTargetError, "expected validation error about namespacedFilterPolicies incompatibility with old-style filters, got: %v", res.Status.ValidationErrors) } +// TestPrepareBackupRequest_GlobalVolumePolicies verifies that the cluster-wide global backup +// volume policies are merged into the request and that the contributing ConfigMap is recorded +// on the backup so `velero backup describe` can surface it. +func TestPrepareBackupRequest_GlobalVolumePolicies(t *testing.T) { + formatFlag := logging.FormatText + logger := logging.DefaultLogger(logrus.DebugLevel, formatFlag) + + globalCM := &corev1api.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "global-volume-policy", Namespace: velerov1api.DefaultNamespace}, + Data: map[string]string{"policies.yaml": `version: v1 +volumePolicies: + - conditions: + storageClass: + - gp2 + action: + type: skip +`}, + } + + fakeClient := velerotest.NewFakeControllerRuntimeClient(t, globalCM, + builder.ForBackupStorageLocation(velerov1api.DefaultNamespace, "loc-1").Result()) + apiServer := velerotest.NewAPIServer(t) + discoveryHelper, err := discovery.NewHelper(apiServer.DiscoveryClient, logger) + require.NoError(t, err) + + c := &backupReconciler{ + logger: logger, + discoveryHelper: discoveryHelper, + kbClient: fakeClient, + clock: &clock.RealClock{}, + formatFlag: formatFlag, + defaultBackupLocation: "loc-1", + globalVolumePoliciesConfigMap: "global-volume-policy", + } + + backup := defaultBackup().StorageLocation("loc-1").Result() + res := c.prepareBackupRequest(ctx, backup, logger) + defer res.WorkerPool.Stop() + + // The global volume policies must load cleanly (no policy-related validation error). + for _, e := range res.Status.ValidationErrors { + assert.NotContains(t, e, "global backup volume policies") + } + require.NotNil(t, res.ResPolicies) + assert.Equal(t, "global-volume-policy", res.Annotations[velerov1api.GlobalBackupVolumePolicyConfigMapAnnotation]) + + action, err := res.ResPolicies.GetMatchAction(resourcepolicies.VolumeFilterData{ + PersistentVolume: &corev1api.PersistentVolume{Spec: corev1api.PersistentVolumeSpec{StorageClassName: "gp2"}}, + }) + require.NoError(t, err) + require.NotNil(t, action) + assert.Equal(t, resourcepolicies.Skip, action.Type) +} + +// TestPrepareBackupRequest_GlobalVolumePolicies_LoadError verifies that when the configured +// global backup volume policies ConfigMap cannot be loaded, a validation error is recorded and +// the contributing-ConfigMap annotation is not set on the backup. +func TestPrepareBackupRequest_GlobalVolumePolicies_LoadError(t *testing.T) { + formatFlag := logging.FormatText + logger := logging.DefaultLogger(logrus.DebugLevel, formatFlag) + + // No ConfigMap with this name exists, so loading the global policies fails. + fakeClient := velerotest.NewFakeControllerRuntimeClient(t, + builder.ForBackupStorageLocation(velerov1api.DefaultNamespace, "loc-1").Result()) + apiServer := velerotest.NewAPIServer(t) + discoveryHelper, err := discovery.NewHelper(apiServer.DiscoveryClient, logger) + require.NoError(t, err) + + c := &backupReconciler{ + logger: logger, + discoveryHelper: discoveryHelper, + kbClient: fakeClient, + clock: &clock.RealClock{}, + formatFlag: formatFlag, + defaultBackupLocation: "loc-1", + globalVolumePoliciesConfigMap: "missing-global-volume-policy", + } + + backup := defaultBackup().StorageLocation("loc-1").Result() + res := c.prepareBackupRequest(ctx, backup, logger) + defer res.WorkerPool.Stop() + + // The failure to load the global policies must surface as a validation error. + var hasGlobalPolicyError bool + for _, e := range res.Status.ValidationErrors { + if strings.Contains(e, "global backup volume policies") { + hasGlobalPolicyError = true + } + } + assert.True(t, hasGlobalPolicyError, "expected a validation error about global backup volume policies, got: %v", res.Status.ValidationErrors) + // The annotation is only set when the policies load successfully. + assert.Empty(t, res.Annotations[velerov1api.GlobalBackupVolumePolicyConfigMapAnnotation]) +} + // TestPrepareBackupRequest_ClusterScopedFilterPolicyIncompatibleWithOldFilters verifies // that a backup referencing a ResourcePolicy ConfigMap with clusterScopedFilterPolicy // produces a validation error when old-style resource filters are also set on the spec. diff --git a/site/content/docs/main/resource-filtering.md b/site/content/docs/main/resource-filtering.md index cbfdb2816..88584b362 100644 --- a/site/content/docs/main/resource-filtering.md +++ b/site/content/docs/main/resource-filtering.md @@ -287,6 +287,11 @@ The policies YAML config file would look like this: # pvc matches specific phase(s) pvcPhase: - Pending + # pvc matches specific volume mode + pvcVolumeMode: Block + # pvc matches specific access mode(s) + pvcAccessModes: + - ReadWriteOnce action: type: skip - conditions: @@ -380,6 +385,8 @@ Currently, Velero supports the volume attributes listed below: - storageClass: matching volumes those with specified `storageClass`, such as `gp2`, `ebs-sc` in eks - volume sources: matching volumes that used specified volume sources. Currently we support nfs or csi backend volume source - pvcPhase: matching volumes based on the phase of their associated PVCs (Pending, Bound, Lost) +- pvcVolumeMode: matching volumes based on the volume mode of their associated PVCs (Filesystem, Block) +- pvcAccessModes: matching volumes based on the access modes of their associated PVCs (ReadWriteOnce, ReadOnlyMany, ReadWriteMany, ReadWriteOncePod). All configured access modes must be present on the PVC. Velero supported conditions and format listed below: - capacity @@ -521,6 +528,72 @@ Velero supported conditions and format listed below: type: skip ``` +- pvc VolumeMode + + This condition filters PVC-backed volumes based on the volume mode of their associated PVCs. The condition is specified as a single volume mode to match. The volume matches this condition if the PVC's volume mode exactly matches the configured value. Matching is case-sensitive, so `block` does not match `Block`. Supported volume modes are: `Filesystem` and `Block`. If `pvcVolumeMode` is omitted from a policy, volume mode is not restricted. Non-PVC volumes, such as `emptyDir`, `configMap`, or inline volumes without an associated PVC, do not match policies that require this condition. + ```yaml + pvcVolumeMode: Block + ``` + + Some examples: + - Skip Block PVCs: Skip backup of volumes whose associated PVC uses `Block` volume mode. + ```yaml + volumePolicies: + - conditions: + pvcVolumeMode: Block + action: + type: skip + ``` + - Combine with other conditions: You can combine PVC volume mode conditions with other conditions like PVC phase, storage class, or labels. + ```yaml + volumePolicies: + - conditions: + pvcVolumeMode: Block + pvcPhase: + - Bound + action: + type: snapshot + ``` + +- pvc AccessModes + + This condition filters PVC-backed volumes based on the access modes of their associated PVCs. The condition is specified as a list of access modes to match. The volume matches this condition only if the PVC has all of the access modes in the list. Matching is case-sensitive, so `readwriteonce` does not match `ReadWriteOnce`. Supported access modes are: `ReadWriteOnce`, `ReadOnlyMany`, `ReadWriteMany`, and `ReadWriteOncePod`. Non-PVC volumes, such as `emptyDir`, `configMap`, or inline volumes without an associated PVC, do not match policies that require this condition. + ```yaml + pvcAccessModes: + - ReadWriteOnce + ``` + + Some examples: + - Skip ReadWriteOnce PVCs: Skip backup of volumes whose associated PVC includes the `ReadWriteOnce` access mode. + ```yaml + volumePolicies: + - conditions: + pvcAccessModes: + - ReadWriteOnce + action: + type: skip + ``` + - Match multiple access modes: Apply an action to volumes whose associated PVC includes both `ReadOnlyMany` and `ReadWriteMany`. + ```yaml + volumePolicies: + - conditions: + pvcAccessModes: + - ReadOnlyMany + - ReadWriteMany + action: + type: snapshot + ``` + - Combine with other conditions: You can combine PVC access mode conditions with other conditions like PVC volume mode, PVC phase, storage class, or labels. + ```yaml + volumePolicies: + - conditions: + pvcAccessModes: + - ReadWriteOnce + pvcVolumeMode: Block + action: + type: snapshot + ``` + ### Resource policies rules @@ -631,3 +704,85 @@ volumePolicies: 3. The outcome would be that velero would perform `fs-backup` operation on both the volumes - `fs-backup` on `Volume 1` because `Volume 1` satisfies the criteria for `fs-backup` action. - Also, for Volume 2 as no matching action was found so legacy approach will be used as a fallback option for this volume (`fs-backup` operation will be done as `defaultVolumesToFSBackup: true` is specified by the user). + +### Global backup volume policies + +Resource policies (volume policies) are normally opt-in per backup via `--resource-policies-configmap`. An administrator can instead configure a cluster-wide baseline that applies to **every** backup by starting the Velero server with the `--global-backup-volume-policies-configmap` flag, pointing at a ConfigMap in the Velero install namespace: + +```bash +velero server --global-backup-volume-policies-configmap global-volume-policy +``` + +The ConfigMap uses the exact same format as a per-backup resource policies ConfigMap (a single data key holding a `ResourcePolicies` YAML document): + +```yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: global-volume-policy + namespace: velero +data: + policies.yaml: | + version: v1 + volumePolicies: + - conditions: + storageClass: + - gp2 + action: + type: skip +``` + +#### Behavior + +- **Only `volumePolicies` apply globally.** If the global ConfigMap contains `includeExcludePolicy`, `clusterScopedFilterPolicy`, or `namespacedFilterPolicies`, those sections are ignored and a warning is logged. Those filters are tied to a specific backup use case, so they remain per-backup only. +- **Merge semantics.** When a backup runs, the effective `volumePolicies` list is the backup-level policies followed by the global policies: + + ``` + merged.volumePolicies = backup.volumePolicies ++ global.volumePolicies + ``` + + Because the first matching policy wins, a backup can override the global baseline for a specific volume while still inheriting every global rule it does not override. If a backup references no resource policy, the global policy applies on its own. +- **Validation.** The global ConfigMap is validated at server startup (the server fails to start if it is missing or invalid) and again on each backup (a backup whose global policy has become missing or invalid is moved to the `FailedValidation` phase). + +#### Example + +Global policy (`--global-backup-volume-policies-configmap=global-volume-policy`): skip `gp2` volumes. + +```yaml +version: v1 +volumePolicies: + - conditions: + storageClass: + - gp2 + action: + type: skip +``` + +Backup-level policy (`--resource-policies-configmap backup01`): `fs-backup` NFS volumes. + +```yaml +version: v1 +volumePolicies: + - conditions: + nfs: {} + action: + type: fs-backup +``` + +Effective (merged) policy used for the backup — backup rules first, then global: + +```yaml +version: v1 +volumePolicies: + - conditions: + nfs: {} + action: + type: fs-backup + - conditions: + storageClass: + - gp2 + action: + type: skip +``` + +When a global policy contributes to a backup, `velero backup describe` surfaces the contributing ConfigMap under a `Global volume policies` section.