mirrors/velero
1
0
Fork 0
mirror of https://github.com/vmware-tanzu/velero.git synced 2026-09-17 05:24:26 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Validate user-provided labels and annotations in maintenance job (#9982)
Run the E2E test on kind / get-go-version (push) Failing after 58s
Details
Run the E2E test on kind / build (push) Has been skipped
Details
Run the E2E test on kind / setup-test-matrix (push) Successful in 3s
Details
Run the E2E test on kind / run-e2e-test (push) Has been skipped
Details
Main CI / get-go-version (push) Successful in 13s
Details
Main CI / Build (push) Failing after 26s
Details

Browse Source 
* Validate user-provided labels and annotations in maintenance job

User-provided labels and annotations from maintenance JobConfigs
are now validated before being applied to the maintenance Job pod
template. Invalid label keys, label values, and annotation keys
are skipped with a warning log. This prevents the Kubernetes API
from rejecting the entire Job when a user provides labels or
annotations that violate naming rules.

Additionally, user-provided labels can no longer overwrite the
internal RepositoryNameLabel used for job tracking.

Fixes velero-io/velero#9981

Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>

* Add tests for label and annotation validation in maintenance job

Add test cases to TestBuildJob covering:
- Invalid label key is skipped
- Invalid label value is skipped
- Label value exceeding 63 characters is skipped
- User-provided label cannot overwrite RepositoryNameLabel
- Invalid annotation key is skipped

Also fix a latent test issue where param.BackupRepo was not reset
between test cases, and add the missing assertion for
expectedPodAnnotation which was defined but never checked.

Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>

* Fix gofmt formatting in maintenance test file

Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>

* Add changelog for PR #9982

Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>

---------

Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
This commit is contained in:
Shubham Pampattiwar
2026-07-10 13:00:43 -04:00
committed by GitHub
parent 05aba1e870
commit ae06d40c69
3 changed files with 294 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
changelogs/unreleased/9982-shubham-pampattiwar
+1
View File
@@ -0,0 +1 @@
Validate user-provided labels and annotations in maintenance job
pkg/repository/maintenance/maintenance.go
+17
View File
@@ -34,6 +34,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/wait"
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -610,6 +611,18 @@ func buildJob(
}
if config != nil && len(config.PodLabels) > 0 {
for k, v := range config.PodLabels {
if k == RepositoryNameLabel {
logger.Warnf("Skipping user-provided label with reserved key %q; this label is managed internally by Velero", k)
continue
}
if errs := validation.IsQualifiedName(k); len(errs) > 0 {
logger.Warnf("Skipping user-provided label with invalid key %q: %s", k, strings.Join(errs, "; "))
continue
}
if errs := validation.IsValidLabelValue(v); len(errs) > 0 {
logger.Warnf("Skipping user-provided label %q with invalid value %q: %s", k, v, strings.Join(errs, "; "))
continue
}
podLabels[k] = v
}
} else {
@@ -623,6 +636,10 @@ func buildJob(
podAnnotations := map[string]string{}
if config != nil && len(config.PodAnnotations) > 0 {
for k, v := range config.PodAnnotations {
if errs := validation.IsQualifiedName(k); len(errs) > 0 {
logger.Warnf("Skipping user-provided annotation with invalid key %q: %s", k, strings.Join(errs, "; "))
continue
}
podAnnotations[k] = v
}
} else {
pkg/repository/maintenance/maintenance_test.go
+276
View File
@@ -1224,6 +1224,274 @@ func TestBuildJob(t *testing.T) {
},
},
},
{
name: "Invalid label key is skipped",
m: &velerotypes.JobConfigs{
PodResources: &kube.PodResources{
CPURequest: "100m",
MemoryRequest: "128Mi",
CPULimit: "200m",
MemoryLimit: "256Mi",
},
PodLabels: map[string]string{
"valid-label": "valid-value",
"INVALID KEY!!": "some-value",
},
},
deploy: deploy2,
logLevel: logrus.InfoLevel,
logFormat: logging.NewFormatFlag(),
expectedJobName: "test-123-maintain-job",
expectedError: false,
expectedEnv: []corev1api.EnvVar{
{
Name: "test-name",
Value: "test-value",
},
},
expectedEnvFrom: []corev1api.EnvFromSource{
{
ConfigMapRef: &corev1api.ConfigMapEnvSource{
LocalObjectReference: corev1api.LocalObjectReference{
Name: "test-configmap",
},
},
},
{
SecretRef: &corev1api.SecretEnvSource{
LocalObjectReference: corev1api.LocalObjectReference{
Name: "test-secret",
},
},
},
},
expectedPodLabel: map[string]string{
RepositoryNameLabel: "test-123",
"valid-label": "valid-value",
},
expectedSecurityContext: nil,
expectedPodSecurityContext: nil,
expectedImagePullSecrets: []corev1api.LocalObjectReference{
{
Name: "imagePullSecret1",
},
},
},
{
name: "Invalid label value is skipped",
m: &velerotypes.JobConfigs{
PodResources: &kube.PodResources{
CPURequest: "100m",
MemoryRequest: "128Mi",
CPULimit: "200m",
MemoryLimit: "256Mi",
},
PodLabels: map[string]string{
"valid-label": "valid-value",
"another-label": "this value has spaces and is invalid",
},
},
deploy: deploy2,
logLevel: logrus.InfoLevel,
logFormat: logging.NewFormatFlag(),
expectedJobName: "test-123-maintain-job",
expectedError: false,
expectedEnv: []corev1api.EnvVar{
{
Name: "test-name",
Value: "test-value",
},
},
expectedEnvFrom: []corev1api.EnvFromSource{
{
ConfigMapRef: &corev1api.ConfigMapEnvSource{
LocalObjectReference: corev1api.LocalObjectReference{
Name: "test-configmap",
},
},
},
{
SecretRef: &corev1api.SecretEnvSource{
LocalObjectReference: corev1api.LocalObjectReference{
Name: "test-secret",
},
},
},
},
expectedPodLabel: map[string]string{
RepositoryNameLabel: "test-123",
"valid-label": "valid-value",
},
expectedSecurityContext: nil,
expectedPodSecurityContext: nil,
expectedImagePullSecrets: []corev1api.LocalObjectReference{
{
Name: "imagePullSecret1",
},
},
},
{
name: "Label value exceeding 63 characters is skipped",
m: &velerotypes.JobConfigs{
PodResources: &kube.PodResources{
CPURequest: "100m",
MemoryRequest: "128Mi",
CPULimit: "200m",
MemoryLimit: "256Mi",
},
PodLabels: map[string]string{
"valid-label": "valid-value",
"long-value-label": "this-value-is-way-too-long-for-a-kubernetes-label-value-and-exceeds-sixty-three-characters",
},
},
deploy: deploy2,
logLevel: logrus.InfoLevel,
logFormat: logging.NewFormatFlag(),
expectedJobName: "test-123-maintain-job",
expectedError: false,
expectedEnv: []corev1api.EnvVar{
{
Name: "test-name",
Value: "test-value",
},
},
expectedEnvFrom: []corev1api.EnvFromSource{
{
ConfigMapRef: &corev1api.ConfigMapEnvSource{
LocalObjectReference: corev1api.LocalObjectReference{
Name: "test-configmap",
},
},
},
{
SecretRef: &corev1api.SecretEnvSource{
LocalObjectReference: corev1api.LocalObjectReference{
Name: "test-secret",
},
},
},
},
expectedPodLabel: map[string]string{
RepositoryNameLabel: "test-123",
"valid-label": "valid-value",
},
expectedSecurityContext: nil,
expectedPodSecurityContext: nil,
expectedImagePullSecrets: []corev1api.LocalObjectReference{
{
Name: "imagePullSecret1",
},
},
},
{
name: "User-provided label cannot overwrite RepositoryNameLabel",
m: &velerotypes.JobConfigs{
PodResources: &kube.PodResources{
CPURequest: "100m",
MemoryRequest: "128Mi",
CPULimit: "200m",
MemoryLimit: "256Mi",
},
PodLabels: map[string]string{
RepositoryNameLabel: "user-override-attempt",
"valid-label": "valid-value",
},
},
deploy: deploy2,
logLevel: logrus.InfoLevel,
logFormat: logging.NewFormatFlag(),
expectedJobName: "test-123-maintain-job",
expectedError: false,
expectedEnv: []corev1api.EnvVar{
{
Name: "test-name",
Value: "test-value",
},
},
expectedEnvFrom: []corev1api.EnvFromSource{
{
ConfigMapRef: &corev1api.ConfigMapEnvSource{
LocalObjectReference: corev1api.LocalObjectReference{
Name: "test-configmap",
},
},
},
{
SecretRef: &corev1api.SecretEnvSource{
LocalObjectReference: corev1api.LocalObjectReference{
Name: "test-secret",
},
},
},
},
expectedPodLabel: map[string]string{
RepositoryNameLabel: "test-123",
"valid-label": "valid-value",
},
expectedSecurityContext: nil,
expectedPodSecurityContext: nil,
expectedImagePullSecrets: []corev1api.LocalObjectReference{
{
Name: "imagePullSecret1",
},
},
},
{
name: "Invalid annotation key is skipped",
m: &velerotypes.JobConfigs{
PodResources: &kube.PodResources{
CPURequest: "100m",
MemoryRequest: "128Mi",
CPULimit: "200m",
MemoryLimit: "256Mi",
},
PodAnnotations: map[string]string{
"valid-annotation": "any value is fine for annotations, even with spaces!",
"INVALID KEY ANNO!": "some-value",
},
},
deploy: deploy2,
logLevel: logrus.InfoLevel,
logFormat: logging.NewFormatFlag(),
expectedJobName: "test-123-maintain-job",
expectedError: false,
expectedEnv: []corev1api.EnvVar{
{
Name: "test-name",
Value: "test-value",
},
},
expectedEnvFrom: []corev1api.EnvFromSource{
{
ConfigMapRef: &corev1api.ConfigMapEnvSource{
LocalObjectReference: corev1api.LocalObjectReference{
Name: "test-configmap",
},
},
},
{
SecretRef: &corev1api.SecretEnvSource{
LocalObjectReference: corev1api.LocalObjectReference{
Name: "test-secret",
},
},
},
},
expectedPodLabel: map[string]string{
RepositoryNameLabel: "test-123",
"azure.workload.identity/use": "fake-label-value",
},
expectedPodAnnotation: map[string]string{
"valid-annotation": "any value is fine for annotations, even with spaces!",
},
expectedSecurityContext: nil,
expectedPodSecurityContext: nil,
expectedImagePullSecrets: []corev1api.LocalObjectReference{
{
Name: "imagePullSecret1",
},
},
},
}
param := provider.RepoParam{
@@ -1245,10 +1513,14 @@ func TestBuildJob(t *testing.T) {
},
}
defaultBackupRepo := param.BackupRepo
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
if tc.backupRepository != nil {
param.BackupRepo = tc.backupRepository
} else {
param.BackupRepo = defaultBackupRepo
}
// Create a fake clientset with resources
@@ -1328,6 +1600,10 @@ func TestBuildJob(t *testing.T) {
assert.Equal(t, tc.expectedPodLabel, job.Spec.Template.Labels)
if tc.expectedPodAnnotation != nil {
assert.Equal(t, tc.expectedPodAnnotation, job.Spec.Template.Annotations)
}
assert.Equal(t, tc.expectedImagePullSecrets, job.Spec.Template.Spec.ImagePullSecrets)
}
})