diff --git a/changelogs/unreleased/9982-shubham-pampattiwar b/changelogs/unreleased/9982-shubham-pampattiwar new file mode 100644 index 000000000..aeb80e8da --- /dev/null +++ b/changelogs/unreleased/9982-shubham-pampattiwar @@ -0,0 +1 @@ +Validate user-provided labels and annotations in maintenance job diff --git a/pkg/repository/maintenance/maintenance.go b/pkg/repository/maintenance/maintenance.go index 2c33c83e2..33c3fb1f8 100644 --- a/pkg/repository/maintenance/maintenance.go +++ b/pkg/repository/maintenance/maintenance.go @@ -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 { diff --git a/pkg/repository/maintenance/maintenance_test.go b/pkg/repository/maintenance/maintenance_test.go index 97eee1148..05fce89e9 100644 --- a/pkg/repository/maintenance/maintenance_test.go +++ b/pkg/repository/maintenance/maintenance_test.go @@ -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) } })