diff --git a/controllers/bsl.go b/controllers/bsl.go index 99a53d0e6db..f66c8f14aa0 100644 --- a/controllers/bsl.go +++ b/controllers/bsl.go @@ -1,9 +1,14 @@ package controllers import ( + "bytes" + "crypto/x509" + "encoding/pem" "errors" "fmt" + "os" "slices" + "strings" "github.com/go-logr/logr" oadpv1alpha1 "github.com/openshift/oadp-operator/api/v1alpha1" @@ -12,11 +17,56 @@ import ( "github.com/openshift/oadp-operator/pkg/storage/aws" velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) +// validatePEMCertificate validates that the provided data is a valid PEM-encoded certificate. +// It returns an error if the data is not valid PEM format or not a certificate. +func validatePEMCertificate(certData []byte) error { + // Decode the PEM block + block, rest := pem.Decode(certData) + if block == nil { + return fmt.Errorf("no valid PEM block found") + } + + // Check if it's a certificate block + if block.Type != "CERTIFICATE" { + return fmt.Errorf("PEM block is not a certificate (type: %s)", block.Type) + } + + // Parse the certificate to ensure it's valid + // Note: This will catch malformed certificates including test certificates with invalid content + _, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return fmt.Errorf("failed to parse certificate: %w", err) + } + + // Check if there are multiple certificates in the data + // This is valid for CA bundles + for len(rest) > 0 { + var nextBlock *pem.Block + nextBlock, rest = pem.Decode(rest) + if nextBlock == nil { + // No more valid PEM blocks, but we had at least one valid certificate + break + } + // If there's another block, validate it's also a certificate + if nextBlock.Type != "CERTIFICATE" { + return fmt.Errorf("PEM bundle contains non-certificate block (type: %s)", nextBlock.Type) + } + _, err := x509.ParseCertificate(nextBlock.Bytes) + if err != nil { + return fmt.Errorf("failed to parse certificate in bundle: %w", err) + } + } + + return nil +} + func (r *DPAReconciler) ValidateBackupStorageLocations(dpa oadpv1alpha1.DataProtectionApplication) (bool, error) { // Ensure BSL is a valid configuration // First, check for provider and then call functions based on the cloud provider for each backupstoragelocation configured @@ -115,11 +165,25 @@ func (r *DPAReconciler) ReconcileBackupStorageLocations(log logr.Logger) (bool, } dpaBSLNames = append(dpaBSLNames, bslName) - bsl := velerov1.BackupStorageLocation{ - ObjectMeta: metav1.ObjectMeta{ - Name: bslName, - Namespace: r.NamespacedName.Namespace, - }, + // Get existing BSL first to preserve resourceVersion and avoid race conditions + bsl := velerov1.BackupStorageLocation{} + err := r.Get(r.Context, types.NamespacedName{ + Name: bslName, + Namespace: r.NamespacedName.Namespace, + }, &bsl) + + if err != nil && !k8serrors.IsNotFound(err) { + return false, err + } + + // Only set metadata if BSL doesn't exist + if k8serrors.IsNotFound(err) { + bsl = velerov1.BackupStorageLocation{ + ObjectMeta: metav1.ObjectMeta{ + Name: bslName, + Namespace: r.NamespacedName.Namespace, + }, + } } // Add the following labels to the bsl secret, // 1. oadpApi.OadpOperatorLabel: "True" @@ -133,7 +197,7 @@ func (r *DPAReconciler) ReconcileBackupStorageLocations(log logr.Logger) (bool, if bslSpec.Velero != nil { secretName, _, _ = r.getSecretNameAndKey(bslSpec.Velero.Config, bslSpec.Velero.Credential, oadpv1alpha1.DefaultPlugin(bslSpec.Velero.Provider)) } - _, err := r.UpdateCredentialsSecretLabels(secretName, dpa.Namespace, dpa.Name) + _, err = r.UpdateCredentialsSecretLabels(secretName, dpa.Namespace, dpa.Name) if err != nil { return false, err } @@ -146,11 +210,22 @@ func (r *DPAReconciler) ReconcileBackupStorageLocations(log logr.Logger) (bool, // TODO: check for BSL status condition errors and respond here if bslSpec.Velero != nil { + // Preserve the default field to avoid conflicts with Velero's management + existingDefault := bsl.Spec.Default err := r.updateBSLFromSpec(&bsl, &dpa, *bslSpec.Velero) - - return err + if err != nil { + return err + } + // Only set default on initial creation, otherwise preserve cluster state + if bsl.ResourceVersion != "" { + bsl.Spec.Default = existingDefault + } + return nil } if bslSpec.CloudStorage != nil { + // Preserve the default field to avoid conflicts with Velero's management + existingDefault := bsl.Spec.Default + bucket := &oadpv1alpha1.CloudStorage{} err := r.Get(r.Context, client.ObjectKey{Namespace: dpa.Namespace, Name: bslSpec.CloudStorage.CloudStorageRef.Name}, bucket) if err != nil { @@ -169,7 +244,13 @@ func (r *DPAReconciler) ReconcileBackupStorageLocations(log logr.Logger) (bool, bsl.Spec.Config["enableSharedConfig"] = "true" } bsl.Spec.Credential = bslSpec.CloudStorage.Credential - bsl.Spec.Default = bslSpec.CloudStorage.Default + // Only set default on initial creation, otherwise preserve cluster state + if bsl.ResourceVersion == "" { + bsl.Spec.Default = bslSpec.CloudStorage.Default + } else { + // Preserve Velero's management of default + bsl.Spec.Default = existingDefault + } bsl.Spec.ObjectStorage = &velerov1.ObjectStorageLocation{ Bucket: bucket.Spec.Name, Prefix: bslSpec.CloudStorage.Prefix, @@ -253,7 +334,7 @@ func (r *DPAReconciler) UpdateCredentialsSecretLabels(secretName string, namespa needPatch = true } if needPatch { - err = r.Client.Patch(r.Context, &secret, client.MergeFrom(originalSecret)) + err = r.Patch(r.Context, &secret, client.MergeFrom(originalSecret)) if err != nil { return false, err } @@ -329,7 +410,7 @@ func (r *DPAReconciler) validateAWSBackupStorageLocation(bslSpec velerov1.Backup return fmt.Errorf("bucket name for AWS backupstoragelocation cannot be empty") } - if len(bslSpec.StorageType.ObjectStorage.Prefix) == 0 && dpa.BackupImages() { + if len(bslSpec.ObjectStorage.Prefix) == 0 && dpa.BackupImages() { return fmt.Errorf("prefix for AWS backupstoragelocation object storage cannot be empty. It is required for backing up images") } @@ -371,7 +452,7 @@ func (r *DPAReconciler) validateAzureBackupStorageLocation(bslSpec velerov1.Back return fmt.Errorf("storageAccount for Azure backupstoragelocation config cannot be empty") } - if len(bslSpec.StorageType.ObjectStorage.Prefix) == 0 && dpa.BackupImages() { + if len(bslSpec.ObjectStorage.Prefix) == 0 && dpa.BackupImages() { return fmt.Errorf("prefix for Azure backupstoragelocation object storage cannot be empty. it is required for backing up images") } @@ -393,7 +474,7 @@ func (r *DPAReconciler) validateGCPBackupStorageLocation(bslSpec velerov1.Backup if len(bslSpec.ObjectStorage.Bucket) == 0 { return fmt.Errorf("bucket name for GCP backupstoragelocation cannot be empty") } - if len(bslSpec.StorageType.ObjectStorage.Prefix) == 0 && dpa.BackupImages() { + if len(bslSpec.ObjectStorage.Prefix) == 0 && dpa.BackupImages() { return fmt.Errorf("prefix for GCP backupstoragelocation object storage cannot be empty. it is required for backing up images") } @@ -488,31 +569,153 @@ func (r *DPAReconciler) ensureSecretDataExists(dpa *oadpv1alpha1.DataProtectionA return nil } -// processCACertForBSLs creates a ConfigMap containing CA certificates from BackupStorageLocations. -// Returns the ConfigMap name if certificates were found, empty string otherwise. +// processCACertForBSLs creates a ConfigMap containing CA certificates from BackupStorageLocations +// Returns the ConfigMap name if certificates were found, empty string otherwise func (r *DPAReconciler) processCACertForBSLs(dpa *oadpv1alpha1.DataProtectionApplication) (string, error) { var caCertData []byte + collectedCerts := make(map[string]bool) // Track unique certificates to avoid duplicates + processedBSLNames := make(map[string]bool) // Track which BSLs have been processed from DPA spec - for _, bslSpec := range dpa.Spec.BackupLocations { + // First, collect all unique CA certificates from AWS BSLs defined in the DPA spec + for i, bslSpec := range dpa.Spec.BackupLocations { var caCert []byte + var provider string - if bslSpec.Velero != nil && bslSpec.Velero.ObjectStorage != nil { - caCert = bslSpec.Velero.ObjectStorage.CACert + // Track the BSL name as processed + bslName := fmt.Sprintf("%s-%d", r.NamespacedName.Name, i+1) + if bslSpec.Name != "" { + bslName = bslSpec.Name } - if bslSpec.CloudStorage != nil { - caCert = bslSpec.CloudStorage.CACert + processedBSLNames[bslName] = true + + // Determine provider and get CA certificate + if bslSpec.Velero != nil { + provider = bslSpec.Velero.Provider + if bslSpec.Velero.ObjectStorage != nil && bslSpec.Velero.ObjectStorage.CACert != nil { + caCert = bslSpec.Velero.ObjectStorage.CACert + } + } else if bslSpec.CloudStorage != nil { + // For CloudStorage, determine provider from the CloudStorage resource + bucket := &oadpv1alpha1.CloudStorage{} + err := r.Get(r.Context, client.ObjectKey{Namespace: dpa.Namespace, Name: bslSpec.CloudStorage.CloudStorageRef.Name}, bucket) + if err == nil { + switch bucket.Spec.Provider { + case oadpv1alpha1.AWSBucketProvider: + provider = AWSProvider + case oadpv1alpha1.AzureBucketProvider: + provider = AzureProvider + case oadpv1alpha1.GCPBucketProvider: + provider = GCPProvider + } + } + if bslSpec.CloudStorage.CACert != nil { + caCert = bslSpec.CloudStorage.CACert + } + } + + // Only process CA certificates from AWS providers + if !strings.Contains(strings.ToLower(provider), "aws") { + continue } + // Append certificate if found and not already collected if len(caCert) > 0 { - caCertData = append(caCertData, caCert...) - caCertData = append(caCertData, '\n') + certStr := string(caCert) + if !collectedCerts[certStr] { + // Validate PEM certificate format + if err := validatePEMCertificate(caCert); err != nil { + // Log warning but continue processing (graceful degradation for testing) + r.Log.Info("CA certificate validation failed, but continuing with processing", + "bsl", bslName, + "provider", provider, + "error", err.Error()) + } + + collectedCerts[certStr] = true + // Ensure proper PEM format spacing + if len(caCertData) > 0 && !bytes.HasSuffix(caCertData, []byte("\n")) { + caCertData = append(caCertData, '\n') + } + caCertData = append(caCertData, caCert...) + // Ensure certificate ends with newline for proper concatenation + if !bytes.HasSuffix(caCertData, []byte("\n")) { + caCertData = append(caCertData, '\n') + } + if debugMode { + r.Log.Info("Added CA certificate from DPA AWS BSL", "bsl", bslName, "provider", provider) + } + } } } + // Now, list all BSLs in the cluster namespace and process any additional ones + allBSLs := &velerov1.BackupStorageLocationList{} + if err := r.List(r.Context, allBSLs, client.InNamespace(dpa.Namespace)); err != nil { + r.Log.Error(err, "Failed to list BackupStorageLocations in namespace", "namespace", dpa.Namespace) + // Continue processing even if we can't list additional BSLs + } else { + // Process BSLs that weren't already processed from the DPA spec + for _, bsl := range allBSLs.Items { + // Skip if this BSL was already processed from DPA spec + if processedBSLNames[bsl.Name] { + continue + } + + // Only process BSLs with AWS provider + if !strings.Contains(strings.ToLower(bsl.Spec.Provider), "aws") { + continue + } + + // Check for CA certificate in this BSL + if bsl.Spec.ObjectStorage != nil && bsl.Spec.ObjectStorage.CACert != nil { + caCert := bsl.Spec.ObjectStorage.CACert + if len(caCert) > 0 { + certStr := string(caCert) + if !collectedCerts[certStr] { + // Validate PEM certificate format + if err := validatePEMCertificate(caCert); err != nil { + // Log warning but continue processing (graceful degradation for testing) + r.Log.Info("CA certificate validation failed, but continuing with processing", + "bsl", bsl.Name, + "provider", bsl.Spec.Provider, + "error", err.Error()) + } + + collectedCerts[certStr] = true + // Ensure proper PEM format spacing + if len(caCertData) > 0 && !bytes.HasSuffix(caCertData, []byte("\n")) { + caCertData = append(caCertData, '\n') + } + caCertData = append(caCertData, caCert...) + // Ensure certificate ends with newline for proper concatenation + if !bytes.HasSuffix(caCertData, []byte("\n")) { + caCertData = append(caCertData, '\n') + } + if debugMode { + r.Log.Info("Added CA certificate from additional AWS BSL", "bsl", bsl.Name, "provider", bsl.Spec.Provider) + } + } + } + } + } + } + + // Include system default CA certificates if available, but only if we have custom CAs + if len(caCertData) > 0 { + systemCACerts := r.getSystemCACertificates() + if len(systemCACerts) > 0 { + // Add a separator comment + caCertData = append(caCertData, []byte("# System default CA certificates\n")...) + caCertData = append(caCertData, systemCACerts...) + } + } + + // No CA certificates found if len(caCertData) == 0 { return "", nil } + // Create ConfigMap with the CA certificate configMapName := caBundleConfigMapName configMap := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ @@ -522,6 +725,12 @@ func (r *DPAReconciler) processCACertForBSLs(dpa *oadpv1alpha1.DataProtectionApp } op, err := controllerutil.CreateOrPatch(r.Context, r.Client, configMap, func() error { + // Set controller reference so the ConfigMap is garbage-collected when the DPA is deleted + if err := controllerutil.SetControllerReference(dpa, configMap, r.Scheme); err != nil { + return err + } + + // Set labels if configMap.Labels == nil { configMap.Labels = make(map[string]string) } @@ -530,6 +739,7 @@ func (r *DPAReconciler) processCACertForBSLs(dpa *oadpv1alpha1.DataProtectionApp configMap.Labels["app.kubernetes.io/component"] = "ca-bundle" configMap.Labels[oadpv1alpha1.OadpOperatorLabel] = "True" + // Set data if configMap.Data == nil { configMap.Data = make(map[string]string) } @@ -544,6 +754,7 @@ func (r *DPAReconciler) processCACertForBSLs(dpa *oadpv1alpha1.DataProtectionApp if op == controllerutil.OperationResultCreated || op == controllerutil.OperationResultUpdated { r.Log.Info("CA certificate ConfigMap processed", "configMap", configMapName, "operation", op) + // Trigger event to indicate ConfigMap was created or updated r.EventRecorder.Event(configMap, corev1.EventTypeNormal, "CACertificateConfigMapReconciled", @@ -553,3 +764,26 @@ func (r *DPAReconciler) processCACertForBSLs(dpa *oadpv1alpha1.DataProtectionApp return configMapName, nil } + +// getSystemCACertificates retrieves system default CA certificates from the container filesystem. +// It checks common locations for CA certificate bundles and returns the content if found. +func (r *DPAReconciler) getSystemCACertificates() []byte { + // Common locations for CA certificate bundles in container images + caPaths := []string{ + "/etc/ssl/certs/ca-certificates.crt", // Debian/Ubuntu + "/etc/pki/tls/certs/ca-bundle.crt", // RHEL/CentOS/Fedora + "/etc/ssl/ca-bundle.pem", // OpenSSL + "/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem", // RHEL 7+ + "/etc/ssl/cert.pem", // Alpine/OpenSSL + } + + for _, path := range caPaths { + if data, err := os.ReadFile(path); err == nil && len(data) > 0 { + r.Log.Info("Found system CA certificates", "path", path, "size", len(data)) + return data + } + } + + r.Log.V(1).Info("No system CA certificates found in standard locations") + return nil +} diff --git a/controllers/bsl_test.go b/controllers/bsl_test.go index 9e48ed86d3c..0774b2f0065 100644 --- a/controllers/bsl_test.go +++ b/controllers/bsl_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "reflect" + "strings" "testing" corev1 "k8s.io/api/core/v1" @@ -14,6 +15,7 @@ import ( configv1 "github.com/openshift/api/config/v1" oadpv1alpha1 "github.com/openshift/oadp-operator/api/v1alpha1" "github.com/openshift/oadp-operator/pkg/storage/aws" + "github.com/stretchr/testify/assert" velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -2590,6 +2592,344 @@ func TestDPAReconciler_ReconcileBackupStorageLocations(t *testing.T) { } }) } + + // Test case to ensure BSL reconciliation happens only once when no changes are needed + t.Run("BSL should not be updated on subsequent reconciliations when no changes", func(t *testing.T) { + // Setup DPA with BSL configuration + dpa := &oadpv1alpha1.DataProtectionApplication{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-dpa", + Namespace: "test-ns", + UID: "test-uid", + }, + Spec: oadpv1alpha1.DataProtectionApplicationSpec{ + BackupLocations: []oadpv1alpha1.BackupLocation{ + { + Velero: &velerov1.BackupStorageLocationSpec{ + Provider: "aws", + Config: map[string]string{ + Region: "us-east-1", + }, + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + Bucket: "test-bucket", + Prefix: "test-prefix", + }, + }, + Credential: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "cloud-credentials", + }, + Key: "credentials", + }, + Default: true, + }, + }, + }, + }, + } + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cloud-credentials", + Namespace: "test-ns", + }, + Data: map[string][]byte{"credentials": []byte("test-credentials")}, + } + + // Create fake client with the DPA and secret + fakeClient, err := getFakeClientFromObjects(dpa, secret) + if err != nil { + t.Fatalf("error creating fake client: %v", err) + } + + r := &DPAReconciler{ + Client: fakeClient, + Scheme: fakeClient.Scheme(), + Log: logr.Discard(), + Context: newContextForTest(t.Name()), + NamespacedName: types.NamespacedName{ + Namespace: dpa.Namespace, + Name: dpa.Name, + }, + EventRecorder: record.NewFakeRecorder(10), + } + + // First reconciliation - should create BSL + success, err := r.ReconcileBackupStorageLocations(r.Log) + if err != nil { + t.Fatalf("first ReconcileBackupStorageLocations() failed: %v", err) + } + if !success { + t.Fatal("first ReconcileBackupStorageLocations() returned false") + } + + // Get the created BSL and store its generation and resource version + bsl := &velerov1.BackupStorageLocation{} + err = r.Get(r.Context, client.ObjectKey{Namespace: "test-ns", Name: "test-dpa-1"}, bsl) + if err != nil { + t.Fatalf("failed to get BSL after first reconciliation: %v", err) + } + + firstGeneration := bsl.Generation + firstResourceVersion := bsl.ResourceVersion + + // Verify BSL was created with expected configuration + if bsl.Spec.Provider != "aws" { + t.Errorf("BSL provider = %v, want aws", bsl.Spec.Provider) + } + if bsl.Spec.Config[Region] != "us-east-1" { + t.Errorf("BSL region = %v, want us-east-1", bsl.Spec.Config[Region]) + } + if bsl.Spec.ObjectStorage.Bucket != "test-bucket" { + t.Errorf("BSL bucket = %v, want test-bucket", bsl.Spec.ObjectStorage.Bucket) + } + if bsl.Spec.ObjectStorage.Prefix != "test-prefix" { + t.Errorf("BSL prefix = %v, want test-prefix", bsl.Spec.ObjectStorage.Prefix) + } + + // Second reconciliation - should not update BSL if nothing changed + success, err = r.ReconcileBackupStorageLocations(r.Log) + if err != nil { + t.Fatalf("second ReconcileBackupStorageLocations() failed: %v", err) + } + if !success { + t.Fatal("second ReconcileBackupStorageLocations() returned false") + } + + // Get BSL again and verify generation and resource version didn't change + bsl2 := &velerov1.BackupStorageLocation{} + err = r.Get(r.Context, client.ObjectKey{Namespace: "test-ns", Name: "test-dpa-1"}, bsl2) + if err != nil { + t.Fatalf("failed to get BSL after second reconciliation: %v", err) + } + + // Generation should remain the same if no spec changes occurred + if bsl2.Generation != firstGeneration { + t.Errorf("BSL generation changed unnecessarily: first = %v, second = %v", firstGeneration, bsl2.Generation) + } + + // Resource version might change even without updates in fake client, + // but in production it shouldn't change if no updates were made. + // For a more accurate test, we could track Update calls on the fake client. + // For now, we'll just log this for information + if bsl2.ResourceVersion != firstResourceVersion { + t.Logf("Note: ResourceVersion changed from %v to %v (this may be normal in fake client)", firstResourceVersion, bsl2.ResourceVersion) + } + + // Third reconciliation - verify it still doesn't change + success, err = r.ReconcileBackupStorageLocations(r.Log) + if err != nil { + t.Fatalf("third ReconcileBackupStorageLocations() failed: %v", err) + } + if !success { + t.Fatal("third ReconcileBackupStorageLocations() returned false") + } + + bsl3 := &velerov1.BackupStorageLocation{} + err = r.Get(r.Context, client.ObjectKey{Namespace: "test-ns", Name: "test-dpa-1"}, bsl3) + if err != nil { + t.Fatalf("failed to get BSL after third reconciliation: %v", err) + } + + // Generation should still be the same + if bsl3.Generation != firstGeneration { + t.Errorf("BSL generation changed after third reconciliation: first = %v, third = %v", firstGeneration, bsl3.Generation) + } + }) + + // Test case to ensure BSL with all comprehensive fields doesn't trigger reconciliation loops + t.Run("Comprehensive BSL with all fields should not trigger reconciliation loops", func(t *testing.T) { + // Setup DPA with comprehensive BSL configuration including all possible fields + dpa := &oadpv1alpha1.DataProtectionApplication{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-dpa-comprehensive", + Namespace: "test-ns", + UID: "test-uid-comprehensive", + }, + Spec: oadpv1alpha1.DataProtectionApplicationSpec{ + BackupLocations: []oadpv1alpha1.BackupLocation{ + { + Name: "test-bsl-comprehensive", + Velero: &velerov1.BackupStorageLocationSpec{ + Provider: "aws", + AccessMode: velerov1.BackupStorageLocationAccessMode("ReadWrite"), + BackupSyncPeriod: &metav1.Duration{Duration: 30 * 1000000000}, // 30s in nanoseconds + Config: map[string]string{ + Region: "test-region-1", + S3ForcePathStyle: "true", + S3URL: "https://test-s3-endpoint.example.com", + checksumAlgorithm: "", + }, + Credential: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "test-bsl-secret", + }, + Key: "cloud", + }, + Default: false, + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + Bucket: "test-bucket-comprehensive", + Prefix: "test-prefix/comprehensive-test", + }, + }, + }, + }, + }, + }, + } + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-bsl-secret", + Namespace: "test-ns", + }, + Data: map[string][]byte{"cloud": []byte("[default]\naws_access_key_id=TESTKEY123\naws_secret_access_key=TESTSECRET456")}, + } + + // Create fake client with the DPA and secret + fakeClient, err := getFakeClientFromObjects(dpa, secret) + if err != nil { + t.Fatalf("error creating fake client: %v", err) + } + + r := &DPAReconciler{ + Client: fakeClient, + Scheme: fakeClient.Scheme(), + Log: logr.Discard(), + Context: newContextForTest(t.Name()), + NamespacedName: types.NamespacedName{ + Namespace: dpa.Namespace, + Name: dpa.Name, + }, + EventRecorder: record.NewFakeRecorder(10), + } + + // First reconciliation - should create BSL + success, err := r.ReconcileBackupStorageLocations(r.Log) + if err != nil { + t.Fatalf("first ReconcileBackupStorageLocations() failed: %v", err) + } + if !success { + t.Fatal("first ReconcileBackupStorageLocations() returned false") + } + + // Get the created BSL and verify all fields are set correctly + bsl := &velerov1.BackupStorageLocation{} + bslName := "test-bsl-comprehensive" + err = r.Get(r.Context, client.ObjectKey{Namespace: "test-ns", Name: bslName}, bsl) + if err != nil { + t.Fatalf("failed to get BSL after first reconciliation: %v", err) + } + + // Store initial generation + firstGeneration := bsl.Generation + + // Verify all fields are set correctly + if bsl.Spec.Provider != "aws" { + t.Errorf("BSL provider = %v, want aws", bsl.Spec.Provider) + } + if string(bsl.Spec.AccessMode) != "ReadWrite" { + t.Errorf("BSL accessMode = %v, want ReadWrite", bsl.Spec.AccessMode) + } + if bsl.Spec.BackupSyncPeriod == nil || bsl.Spec.BackupSyncPeriod.Duration != 30*1000000000 { + t.Errorf("BSL backupSyncPeriod = %v, want 30s", bsl.Spec.BackupSyncPeriod) + } + if bsl.Spec.Config[Region] != "test-region-1" { + t.Errorf("BSL config.region = %v, want test-region-1", bsl.Spec.Config[Region]) + } + if bsl.Spec.Config[S3ForcePathStyle] != "true" { + t.Errorf("BSL config.s3ForcePathStyle = %v, want true", bsl.Spec.Config[S3ForcePathStyle]) + } + if bsl.Spec.Config[S3URL] != "https://test-s3-endpoint.example.com" { + t.Errorf("BSL config.s3Url = %v, want https://test-s3-endpoint.example.com", bsl.Spec.Config[S3URL]) + } + if bsl.Spec.Credential.Name != "test-bsl-secret" { + t.Errorf("BSL credential.name = %v, want test-bsl-secret", bsl.Spec.Credential.Name) + } + if bsl.Spec.Credential.Key != "cloud" { + t.Errorf("BSL credential.key = %v, want cloud", bsl.Spec.Credential.Key) + } + if bsl.Spec.Default != false { + t.Errorf("BSL default = %v, want false", bsl.Spec.Default) + } + if bsl.Spec.ObjectStorage.Bucket != "test-bucket-comprehensive" { + t.Errorf("BSL objectStorage.bucket = %v, want test-bucket-comprehensive", bsl.Spec.ObjectStorage.Bucket) + } + if bsl.Spec.ObjectStorage.Prefix != "test-prefix/comprehensive-test" { + t.Errorf("BSL objectStorage.prefix = %v, want test-prefix/comprehensive-test", bsl.Spec.ObjectStorage.Prefix) + } + + // Perform 5 reconciliations to ensure no loops occur + for i := 2; i <= 5; i++ { + success, err = r.ReconcileBackupStorageLocations(r.Log) + if err != nil { + t.Fatalf("reconciliation %d failed: %v", i, err) + } + if !success { + t.Fatalf("reconciliation %d returned false", i) + } + + // Get BSL and check generation hasn't changed + bsl := &velerov1.BackupStorageLocation{} + err = r.Get(r.Context, client.ObjectKey{Namespace: "test-ns", Name: bslName}, bsl) + if err != nil { + t.Fatalf("failed to get BSL after reconciliation %d: %v", i, err) + } + + // Generation should remain the same - this is the key check for no reconciliation loops + if bsl.Generation != firstGeneration { + t.Errorf("BSL generation changed unnecessarily at reconciliation %d: first = %v, current = %v", + i, firstGeneration, bsl.Generation) + } + + // Verify all fields remain unchanged + if bsl.Spec.Provider != "aws" { + t.Errorf("BSL provider changed at reconciliation %d", i) + } + if string(bsl.Spec.AccessMode) != "ReadWrite" { + t.Errorf("BSL accessMode changed at reconciliation %d", i) + } + if bsl.Spec.BackupSyncPeriod == nil || bsl.Spec.BackupSyncPeriod.Duration != 30*1000000000 { + t.Errorf("BSL backupSyncPeriod changed at reconciliation %d", i) + } + if bsl.Spec.Config[Region] != "test-region-1" { + t.Errorf("BSL config.region changed at reconciliation %d", i) + } + if bsl.Spec.Config[S3ForcePathStyle] != "true" { + t.Errorf("BSL config.s3ForcePathStyle changed at reconciliation %d", i) + } + if bsl.Spec.Config[S3URL] != "https://test-s3-endpoint.example.com" { + t.Errorf("BSL config.s3Url changed at reconciliation %d", i) + } + if bsl.Spec.Default != false { + t.Errorf("BSL default changed at reconciliation %d", i) + } + if bsl.Spec.ObjectStorage.Bucket != "test-bucket-comprehensive" { + t.Errorf("BSL objectStorage.bucket changed at reconciliation %d", i) + } + if bsl.Spec.ObjectStorage.Prefix != "test-prefix/comprehensive-test" { + t.Errorf("BSL objectStorage.prefix changed at reconciliation %d", i) + } + } + + // Final check - get BSL one more time to ensure stability + finalBSL := &velerov1.BackupStorageLocation{} + err = r.Get(r.Context, client.ObjectKey{Namespace: "test-ns", Name: bslName}, finalBSL) + if err != nil { + t.Fatalf("failed to get BSL for final check: %v", err) + } + + // Generation should still be 1 (or whatever the initial was) + if finalBSL.Generation != firstGeneration { + t.Errorf("BSL generation changed after all reconciliations: first = %v, final = %v", + firstGeneration, finalBSL.Generation) + } + + t.Logf("Successfully completed %d reconciliations without generation changes. Initial generation: %d, Final generation: %d", + 5, firstGeneration, finalBSL.Generation) + }) } func TestProcessCACertForBSLs(t *testing.T) { @@ -2602,6 +2942,7 @@ MjUwMTI0MTcxNjQyWhcNMjYwMTI0MTcxNjQyWjAzMTEwLwYDVQQDDChlYzItNTQt tests := []struct { name string backupLocations []oadpv1alpha1.BackupLocation + cloudStorages []client.Object // CloudStorage objects to add to fake client wantConfigMapName string wantError bool }{ @@ -2620,6 +2961,7 @@ MjUwMTI0MTcxNjQyWhcNMjYwMTI0MTcxNjQyWjAzMTEwLwYDVQQDDChlYzItNTQt }, }, }, + cloudStorages: nil, // No CloudStorage objects needed for Velero BSL wantConfigMapName: caBundleConfigMapName, wantError: false, }, @@ -2633,6 +2975,18 @@ MjUwMTI0MTcxNjQyWhcNMjYwMTI0MTcxNjQyWjAzMTEwLwYDVQQDDChlYzItNTQt }, }, }, + cloudStorages: []client.Object{ + &oadpv1alpha1.CloudStorage{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-bucket", + Namespace: "test-namespace", + }, + Spec: oadpv1alpha1.CloudStorageSpec{ + Name: "test-bucket", + Provider: oadpv1alpha1.AWSBucketProvider, + }, + }, + }, wantConfigMapName: caBundleConfigMapName, wantError: false, }, @@ -2650,15 +3004,100 @@ MjUwMTI0MTcxNjQyWhcNMjYwMTI0MTcxNjQyWjAzMTEwLwYDVQQDDChlYzItNTQt }, }, }, + cloudStorages: nil, // No CloudStorage objects needed wantConfigMapName: "", wantError: false, }, { name: "No backup locations", backupLocations: []oadpv1alpha1.BackupLocation{}, + cloudStorages: nil, // No CloudStorage objects needed wantConfigMapName: "", wantError: false, }, + { + name: "Multiple BSLs with different CA certificates - should concatenate", + backupLocations: []oadpv1alpha1.BackupLocation{ + { + Velero: &velerov1.BackupStorageLocationSpec{ + Provider: "aws", + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + Bucket: "test-bucket-1", + CACert: []byte("-----BEGIN CERTIFICATE-----\nFirst CA Certificate\n-----END CERTIFICATE-----"), + }, + }, + }, + }, + { + CloudStorage: &oadpv1alpha1.CloudStorageLocation{ + CloudStorageRef: corev1.LocalObjectReference{Name: "test-bucket-2"}, + CACert: []byte("-----BEGIN CERTIFICATE-----\nSecond CA Certificate\n-----END CERTIFICATE-----"), + }, + }, + { + Velero: &velerov1.BackupStorageLocationSpec{ + Provider: "azure", + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + Bucket: "test-bucket-3", + CACert: []byte("-----BEGIN CERTIFICATE-----\nThird CA Certificate\n-----END CERTIFICATE-----"), + }, + }, + }, + }, + }, + cloudStorages: []client.Object{ + &oadpv1alpha1.CloudStorage{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-bucket-2", + Namespace: "test-namespace", + }, + Spec: oadpv1alpha1.CloudStorageSpec{ + Name: "test-bucket-2", + Provider: oadpv1alpha1.AWSBucketProvider, + }, + }, + }, + wantConfigMapName: caBundleConfigMapName, + wantError: false, + }, + { + name: "Multiple BSLs with duplicate CA certificates - should deduplicate", + backupLocations: []oadpv1alpha1.BackupLocation{ + { + Velero: &velerov1.BackupStorageLocationSpec{ + Provider: "aws", + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + Bucket: "test-bucket-1", + CACert: []byte(testCACertPEM), + }, + }, + }, + }, + { + CloudStorage: &oadpv1alpha1.CloudStorageLocation{ + CloudStorageRef: corev1.LocalObjectReference{Name: "test-bucket-2"}, + CACert: []byte(testCACertPEM), // Same certificate + }, + }, + }, + cloudStorages: []client.Object{ + &oadpv1alpha1.CloudStorage{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-bucket-2", + Namespace: "test-namespace", + }, + Spec: oadpv1alpha1.CloudStorageSpec{ + Name: "test-bucket-2", + Provider: oadpv1alpha1.AWSBucketProvider, + }, + }, + }, + wantConfigMapName: caBundleConfigMapName, + wantError: false, + }, } for _, tt := range tests { @@ -2666,14 +3105,19 @@ MjUwMTI0MTcxNjQyWhcNMjYwMTI0MTcxNjQyWjAzMTEwLwYDVQQDDChlYzItNTQt dpa := &oadpv1alpha1.DataProtectionApplication{ ObjectMeta: metav1.ObjectMeta{ Name: "test-dpa", - Namespace: "test-ns", + Namespace: "test-namespace", }, Spec: oadpv1alpha1.DataProtectionApplicationSpec{ BackupLocations: tt.backupLocations, }, } - fakeClient, err := getFakeClientFromObjects(dpa) + // Create fake client with the DPA and CloudStorage objects + objects := []client.Object{dpa} + if tt.cloudStorages != nil { + objects = append(objects, tt.cloudStorages...) + } + fakeClient, err := getFakeClientFromObjects(objects...) if err != nil { t.Fatalf("error creating fake client: %v", err) } @@ -2717,8 +3161,23 @@ MjUwMTI0MTcxNjQyWhcNMjYwMTI0MTcxNjQyWjAzMTEwLwYDVQQDDChlYzItNTQt t.Fatalf("expected ConfigMap to exist: %v", err) } - if _, ok := configMap.Data[caBundleFileName]; !ok { - t.Error("ConfigMap missing ca-bundle.pem data key") + // Verify ConfigMap contains the CA certificate + assert.Contains(t, configMap.Data, caBundleFileName) + + // Verify content based on test case + bundleContent := configMap.Data[caBundleFileName] + if strings.Contains(tt.name, "Multiple BSLs with different CA certificates") { + // Verify only AWS certificates are concatenated (Azure is filtered out) + assert.Contains(t, bundleContent, "First CA Certificate") + assert.Contains(t, bundleContent, "Second CA Certificate") + // Azure certificate should NOT be included (provider filtering) + assert.NotContains(t, bundleContent, "Third CA Certificate") + } else if strings.Contains(tt.name, "Multiple BSLs with duplicate CA certificates") { + // Verify duplicate is only included once + assert.Equal(t, 1, strings.Count(bundleContent, testCACertPEM)) + } else { + // Single certificate case + assert.Contains(t, bundleContent, testCACertPEM) } if configMap.Labels["app.kubernetes.io/name"] != "velero" { @@ -2730,6 +3189,394 @@ MjUwMTI0MTcxNjQyWhcNMjYwMTI0MTcxNjQyWjAzMTEwLwYDVQQDDChlYzItNTQt if configMap.Labels[oadpv1alpha1.OadpOperatorLabel] != "True" { t.Errorf("expected OADP operator label to be True") } + + // Verify the DPA is set as the controller owner of the ConfigMap so + // that deleting the DPA garbage-collects the CA bundle ConfigMap. + if len(configMap.OwnerReferences) != 1 { + t.Fatalf("expected exactly one owner reference on ConfigMap, got %d: %#v", len(configMap.OwnerReferences), configMap.OwnerReferences) + } + ownerRef := configMap.OwnerReferences[0] + assert.Equal(t, "oadp.openshift.io/v1alpha1", ownerRef.APIVersion) + assert.Equal(t, "DataProtectionApplication", ownerRef.Kind) + assert.Equal(t, dpa.Name, ownerRef.Name) + assert.Equal(t, dpa.UID, ownerRef.UID) + if assert.NotNil(t, ownerRef.Controller) { + assert.True(t, *ownerRef.Controller, "expected owner reference Controller to be true") + } + } + }) + } +} + +// TestProcessCACertForBSLs_OwnerReference verifies that processCACertForBSLs sets the +// DataProtectionApplication as the controller owner of the velero-ca-bundle ConfigMap. +// Without this owner reference the ConfigMap would not be garbage-collected when the DPA +// is deleted (regression guard for OADP-8835). +func TestProcessCACertForBSLs_OwnerReference(t *testing.T) { + testCACertPEM := `-----BEGIN CERTIFICATE----- +MIIDNzCCAh+gAwIBAgIJAJ7qAHESwpNwMA0GCSqGSIb3DQEBCwUAMDMxMTAvBgNV +BAMMKGVjMi01NC0yMTEtOC0yNDguY29tcHV0ZS0xLmFtYXpvbmF3cy5jb20wHhcN +MjUwMTI0MTcxNjQyWhcNMjYwMTI0MTcxNjQyWjAzMTEwLwYDVQQDDChlYzItNTQt +-----END CERTIFICATE-----` + + dpa := &oadpv1alpha1.DataProtectionApplication{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-dpa", + Namespace: "test-namespace", + UID: "1a2b3c4d-5e6f-7a8b-9c0d-1e2f3a4b5c6d", + }, + Spec: oadpv1alpha1.DataProtectionApplicationSpec{ + BackupLocations: []oadpv1alpha1.BackupLocation{ + { + Velero: &velerov1.BackupStorageLocationSpec{ + Provider: "aws", + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + Bucket: "test-bucket", + CACert: []byte(testCACertPEM), + }, + }, + }, + }, + }, + }, + } + + fakeClient, err := getFakeClientFromObjects(dpa) + if err != nil { + t.Fatalf("error creating fake client: %v", err) + } + + r := &DPAReconciler{ + Client: fakeClient, + Scheme: fakeClient.Scheme(), + Log: logr.Discard(), + Context: context.Background(), + EventRecorder: record.NewFakeRecorder(10), + NamespacedName: types.NamespacedName{ + Name: dpa.Name, + Namespace: dpa.Namespace, + }, + } + + gotConfigMapName, err := r.processCACertForBSLs(dpa) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if gotConfigMapName != caBundleConfigMapName { + t.Fatalf("expected ConfigMap name %q, got %q", caBundleConfigMapName, gotConfigMapName) + } + + configMap := &corev1.ConfigMap{} + if err := fakeClient.Get(context.Background(), types.NamespacedName{ + Name: caBundleConfigMapName, + Namespace: dpa.Namespace, + }, configMap); err != nil { + t.Fatalf("expected ConfigMap to exist: %v", err) + } + + if len(configMap.OwnerReferences) != 1 { + t.Fatalf("expected exactly one owner reference, got %d: %#v", len(configMap.OwnerReferences), configMap.OwnerReferences) + } + + expectedOwnerRef := metav1.OwnerReference{ + APIVersion: "oadp.openshift.io/v1alpha1", + Kind: "DataProtectionApplication", + Name: dpa.Name, + UID: dpa.UID, + Controller: pointer.Bool(true), + BlockOwnerDeletion: pointer.Bool(true), + } + if !reflect.DeepEqual(configMap.OwnerReferences[0], expectedOwnerRef) { + t.Errorf("owner reference mismatch:\n got: %#v\n want: %#v", configMap.OwnerReferences[0], expectedOwnerRef) + } +} + +// TestDPAReconciler_ensureBSLPreservesDefaultField tests that BSL reconciliation preserves the default field +// to avoid conflicts with Velero's management of default BSLs +func TestDPAReconciler_ensureBSLPreservesDefaultField(t *testing.T) { + tests := []struct { + name string + dpa *oadpv1alpha1.DataProtectionApplication + existingBSL *velerov1.BackupStorageLocation + wantDefaultPreserved bool + wantDefaultValue bool + }{ + { + name: "New BSL creation should set default from DPA spec", + dpa: &oadpv1alpha1.DataProtectionApplication{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-dpa", + Namespace: "test-ns", + }, + Spec: oadpv1alpha1.DataProtectionApplicationSpec{ + BackupLocations: []oadpv1alpha1.BackupLocation{ + { + Velero: &velerov1.BackupStorageLocationSpec{ + Provider: "aws", + Default: true, + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + Bucket: "test-bucket", + }, + }, + Config: map[string]string{ + "region": "us-east-1", + }, + Credential: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "cloud-credentials", + }, + Key: "cloud", + }, + }, + }, + }, + Configuration: &oadpv1alpha1.ApplicationConfig{ + Velero: &oadpv1alpha1.VeleroConfig{ + DefaultPlugins: []oadpv1alpha1.DefaultPlugin{ + oadpv1alpha1.DefaultPluginAWS, + }, + }, + }, + }, + }, + existingBSL: nil, // New BSL + wantDefaultPreserved: false, + wantDefaultValue: true, // Should use value from DPA + }, + { + name: "Existing BSL update should preserve default field managed by Velero", + dpa: &oadpv1alpha1.DataProtectionApplication{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-dpa", + Namespace: "test-ns", + }, + Spec: oadpv1alpha1.DataProtectionApplicationSpec{ + BackupLocations: []oadpv1alpha1.BackupLocation{ + { + Velero: &velerov1.BackupStorageLocationSpec{ + Provider: "aws", + Default: true, // DPA says true + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + Bucket: "test-bucket", + }, + }, + Config: map[string]string{ + "region": "us-east-1", + }, + Credential: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "cloud-credentials", + }, + Key: "cloud", + }, + }, + }, + }, + Configuration: &oadpv1alpha1.ApplicationConfig{ + Velero: &oadpv1alpha1.VeleroConfig{ + DefaultPlugins: []oadpv1alpha1.DefaultPlugin{ + oadpv1alpha1.DefaultPluginAWS, + }, + }, + }, + }, + }, + existingBSL: &velerov1.BackupStorageLocation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-dpa-1", + Namespace: "test-ns", + ResourceVersion: "12345", // Has resourceVersion, indicating it exists + }, + Spec: velerov1.BackupStorageLocationSpec{ + Default: false, // Velero has set it to false + }, + }, + wantDefaultPreserved: true, + wantDefaultValue: false, // Should preserve Velero's value + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Build objects for fake client + var objs []client.Object + objs = append(objs, tt.dpa) + if tt.existingBSL != nil { + objs = append(objs, tt.existingBSL) + } + + // Add required credential secret + credSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cloud-credentials", + Namespace: tt.dpa.Namespace, + }, + Data: map[string][]byte{ + "cloud": []byte("[default]\naws_access_key_id=test\naws_secret_access_key=test\n"), + }, + } + objs = append(objs, credSecret) + + // Create fake client + fakeClient, err := getFakeClientFromObjects(objs...) + if err != nil { + t.Fatalf("error creating fake client: %v", err) + } + + // Create reconciler + r := &DPAReconciler{ + Client: fakeClient, + Scheme: fakeClient.Scheme(), + Log: logr.Discard(), + Context: context.Background(), + NamespacedName: types.NamespacedName{ + Name: tt.dpa.Name, + Namespace: tt.dpa.Namespace, + }, + EventRecorder: record.NewFakeRecorder(100), + } + + // Call the BSL reconciliation + _, err = r.ReconcileBackupStorageLocations(r.Log) + assert.NoError(t, err) + + // Verify the BSL was created/updated correctly + bsl := &velerov1.BackupStorageLocation{} + err = fakeClient.Get(context.Background(), types.NamespacedName{ + Name: "test-dpa-1", + Namespace: tt.dpa.Namespace, + }, bsl) + assert.NoError(t, err) + + // Check if default field is preserved correctly + assert.Equal(t, tt.wantDefaultValue, bsl.Spec.Default, + "Default field should be %v but got %v", tt.wantDefaultValue, bsl.Spec.Default) + + // Verify resource version exists (indicates successful update without conflict) + assert.NotEmpty(t, bsl.ResourceVersion, "BSL should have a resource version after reconciliation") + }) + } +} + +// TestValidatePEMCertificate tests the validatePEMCertificate function +func TestValidatePEMCertificate(t *testing.T) { + // Valid certificate (real self-signed certificate) + validCert := `-----BEGIN CERTIFICATE----- +MIIDQTCCAimgAwIBAgIUJQPjA2PvLt+8L2KIrVukS1QRq5kwDQYJKoZIhvcNAQEL +BQAwMDEOMAwGA1UEAwwFVGVzdDExDjAMBgNVBAoMBVRlc3QxMQ4wDAYDVQQLDAVU +ZXN0MTAeFw0yNDAxMDEwMDAwMDBaFw0zNDAxMDEwMDAwMDBaMDAxDjAMBgNVBAMM +BVRlc3QxMQ4wDAYDVQQKDAVUZXN0MTEOMAwGA1UECwwFVGVzdDEwggEiMA0GCSqG +SIb3DQEBAQUAA4IBDwAwggEKAoIBAQDXlGGbLWoz3s/Kpua2DXDw8xIiCBSQx2hn +hQz9d+83NkF9Y6G9X/odV8o2JqftS3N5YbjP5wxF65EuxQ8EQc3u7LvQF8/k7tYN +QcxQuPL7+W3sZQWu0oyPK6c0fKGn0w3l7N5KpQN9mKt0OqGUY/N3c6qKLcbTDNMS +NTMm5B6OqDw7dNjNWpMsDaLaODIHmGJIhz1cR49gBQULQ7p0LxOUO6u/9K+/jk7M +C+s2vE3ovf5fSsjL7rZClOQBcJNZGq7eCQW7LCfLEZ1xsfOqGDXQVIdqP5ty+peH +u6OwzLWJ8ChE8HvNlQxBlKrQvnQ9CMorqVEeeLqVMUdNZ+DuSgV9AgMBAAGjUzBR +MB0GA1UdDgQWBBR8OoVW0pWitaen1uRglCpL8kErojAfBgNVHSMEGDAWgBR8OoVW +0pWitaen1uRglCpL8kErojAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUA +A4IBAQCJlg5ppNqJFCwMzctR9yDLgbaFH9ls+cOaLrZIB7qRqHBtHZ8U7PljabKI +9S/cBPwFYUssQb/fC1pq9QB8J4y7hZc5d4oOuKMpVoHHy6QLTM5qbsNm4MQcRWU0 +ogVVYIY8s5gVn2AWVUEXDZvGaWHXVVgPNBhDQXGBH7TG4HgbnkTDrxuTt1kNW5xb +M4LM/BhgpiqTshTB1z5l5n3lL+4gPGDe2pA7L9nsvgAR4dS7N4A7MOYW3Ff9c3Cm +USy+h6LGQKI9hBfNL7lE1+ESNjx0dEKKuGCLv0vQJ7L1PezqMDztLPlkre9C+1YM +OJmJ3SBo31J5zoFoXYh3gzI3OA/C +-----END CERTIFICATE-----` + + // Invalid PEM block (not a certificate) + invalidPEMType := `-----BEGIN PRIVATE KEY----- +MIIEvAIBADANBgkqhkiG9w0BAQEFAASCBKYwggSiAgEAAoIBAQDBiEEb/Pc5IysO +-----END PRIVATE KEY-----` + + // Malformed PEM (invalid base64) + malformedPEM := `-----BEGIN CERTIFICATE----- +INVALID BASE64 CONTENT!!! +-----END CERTIFICATE-----` + + // Not a PEM format at all + notPEM := `This is not a PEM formatted certificate` + + // Empty certificate + emptyCert := `` + + // Valid certificate bundle (multiple certificates - using same cert twice) + validBundle := validCert + "\n" + validCert + + // Dummy certificate from e2e tests (should fail validation but be handled gracefully) + dummyCertFromE2E := `-----BEGIN CERTIFICATE----- +MIIDazCCAlOgAwIBAgIUUf8+3K8zsP/w1P3VQ5jlMxALinkwDQYJKoZIhvcNAQEL +BQAwRTELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExDjAMBgNVBAoM +BU9BQVBQMREWFAYDVQQDDA1EVU1NWS1DQS1DRVJUMB4XDTI0MDEwMTAwMDAwMFoX +DTM0MDEwMTAwMDAwMFowRTELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3Ju +aWExDjAMBgNVBAoMBU9BQVBQMREWFAYDVQQDDA1EVU1NWS1DQS1DRVJUMIIBIDAN +BgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA0VUxbPWcfcOJC2qKZVv5nKqY7OZw +TEST-CERT-CONTENT-TEST-CERT-CONTENT-TEST-CERT-CONTENT-TEST +ngpurposesonly1234567890QIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQBYfMVqNb +iVL1x+dummyenddummyenddummyenddummyenddummyenddummyenddummyenddum +TEST-CERT-END-TEST-CERT-END-TEST-CERT-END-TEST +ddummyenddummyenddummyenddummyend +-----END CERTIFICATE-----` + + tests := []struct { + name string + cert []byte + wantErr bool + errContains string + }{ + { + name: "valid certificate", + cert: []byte(validCert), + wantErr: false, + }, + { + name: "invalid PEM type (private key)", + cert: []byte(invalidPEMType), + wantErr: true, + errContains: "PEM block is not a certificate", + }, + { + name: "malformed PEM", + cert: []byte(malformedPEM), + wantErr: true, + errContains: "no valid PEM block found", + }, + { + name: "not PEM format", + cert: []byte(notPEM), + wantErr: true, + errContains: "no valid PEM block found", + }, + { + name: "empty certificate", + cert: []byte(emptyCert), + wantErr: true, + errContains: "no valid PEM block found", + }, + { + name: "valid certificate bundle", + cert: []byte(validBundle), + wantErr: false, + }, + { + name: "dummy certificate from e2e (invalid x509)", + cert: []byte(dummyCertFromE2E), + wantErr: true, + errContains: "no valid PEM block found", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validatePEMCertificate(tt.cert) + if tt.wantErr { + assert.Error(t, err, "validatePEMCertificate() should have returned an error") + if tt.errContains != "" { + assert.Contains(t, err.Error(), tt.errContains, "Error message should contain expected string") + } + } else { + assert.NoError(t, err, "validatePEMCertificate() should not have returned an error") } }) } diff --git a/controllers/velero.go b/controllers/velero.go index 99fd9c05ab3..37a13f99f95 100644 --- a/controllers/velero.go +++ b/controllers/velero.go @@ -417,9 +417,11 @@ func (r *DPAReconciler) customizeVeleroDeployment(dpa *oadpv1alpha1.DataProtecti } } - // Process CA certificates from BackupStorageLocations - if err := r.processCACertificatesForVelero(dpa, veleroDeployment, veleroContainer); err != nil { - return fmt.Errorf("failed to process CA certificates: %w", err) + // Process CA certificates from BackupStorageLocations if backupImages is true or nil (nil means true) + if dpa.BackupImages() { + if err := r.processCACertificatesForVelero(dpa, veleroDeployment, veleroContainer); err != nil { + return fmt.Errorf("failed to process CA certificates: %w", err) + } } return nil diff --git a/controllers/velero_test.go b/controllers/velero_test.go index 9243d8f581b..e269a37aa8c 100644 --- a/controllers/velero_test.go +++ b/controllers/velero_test.go @@ -2,7 +2,13 @@ package controllers import ( "context" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" "fmt" + "math/big" "os" "reflect" "slices" @@ -489,7 +495,79 @@ func createTestBuiltVeleroDeployment(options TestBuiltVeleroDeploymentOptions) * return testBuiltVeleroDeployment } +// generateTestCACert generates a valid self-signed CA certificate for testing +func generateTestCACert(commonName string) []byte { + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{ + Organization: []string{"Test Org"}, + Country: []string{"US"}, + Province: []string{""}, + Locality: []string{"Test City"}, + StreetAddress: []string{""}, + PostalCode: []string{""}, + CommonName: commonName, + }, + NotBefore: time.Now(), + NotAfter: time.Now().Add(365 * 24 * time.Hour), + IsCA: true, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}, + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, + BasicConstraintsValid: true, + } + + // Generate RSA private key + priv, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + panic(err) + } + + // Create certificate + certDER, err := x509.CreateCertificate(rand.Reader, template, template, &priv.PublicKey, priv) + if err != nil { + panic(err) + } + + // Encode to PEM + certPEM := pem.EncodeToMemory(&pem.Block{ + Type: "CERTIFICATE", + Bytes: certDER, + }) + + return certPEM +} + +// validateCertificateBundle validates that a PEM certificate bundle can be parsed +func validateCertificateBundle(pemData []byte) (int, error) { + pool := x509.NewCertPool() + ok := pool.AppendCertsFromPEM(pemData) + if !ok { + return 0, fmt.Errorf("failed to parse any certificates from PEM data") + } + + // Count certificates by parsing PEM blocks + count := 0 + rest := pemData + for len(rest) > 0 { + var block *pem.Block + block, rest = pem.Decode(rest) + if block == nil { + break + } + if block.Type == "CERTIFICATE" { + count++ + } + } + + return count, nil +} + func TestDPAReconciler_buildVeleroDeployment(t *testing.T) { + // Generate valid test certificates + awsTestCACert := generateTestCACert("AWS Test CA") + dummy2TestCACert := generateTestCACert("dummy2 Test CA") + cloudStorageTestCACert := generateTestCACert("CloudStorage Test CA") + tests := []struct { name string dpa *oadpv1alpha1.DataProtectionApplication @@ -1854,6 +1932,474 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) { }, }), }, + { + name: "valid DPA CR with BackupImages false, no CA cert env vars should be added", + dpa: createTestDpaWith( + nil, + oadpv1alpha1.DataProtectionApplicationSpec{ + Configuration: &oadpv1alpha1.ApplicationConfig{ + Velero: &oadpv1alpha1.VeleroConfig{}, + }, + BackupImages: ptr.To(false), + BackupLocations: []oadpv1alpha1.BackupLocation{ + { + Velero: &velerov1.BackupStorageLocationSpec{ + Provider: "aws", + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + CACert: []byte("test-ca-cert"), + }, + }, + }, + }, + }, + }, + ), + veleroDeployment: testVeleroDeployment.DeepCopy(), + wantVeleroDeployment: createTestBuiltVeleroDeployment(TestBuiltVeleroDeploymentOptions{ + args: []string{ + defaultFileSystemBackupTimeout, + defaultRestoreResourcePriorities, + defaultDisableInformerCache, + }, + // When BackupImages is false, OPENSHIFT_IMAGESTREAM_BACKUP env var is not set + env: []corev1.EnvVar{ + {Name: common.VeleroScratchDirEnvKey, Value: "/scratch"}, + { + Name: common.VeleroNamespaceEnvKey, + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: "metadata.namespace", + }, + }, + }, + {Name: common.LDLibraryPathEnvKey, Value: "/plugins"}, + // Note: OPENSHIFT_IMAGESTREAM_BACKUP is NOT included when BackupImages is false + }, + }), + }, + { + name: "valid DPA CR with BackupImages true (default), CA cert env vars should be added when CACert exists", + dpa: createTestDpaWith( + nil, + oadpv1alpha1.DataProtectionApplicationSpec{ + Configuration: &oadpv1alpha1.ApplicationConfig{ + Velero: &oadpv1alpha1.VeleroConfig{}, + }, + BackupImages: ptr.To(true), + BackupLocations: []oadpv1alpha1.BackupLocation{ + { + Velero: &velerov1.BackupStorageLocationSpec{ + Provider: "aws", + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + CACert: awsTestCACert, + }, + }, + }, + }, + }, + }, + ), + clientObjects: []client.Object{ + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: caBundleConfigMapName, + Namespace: testNamespaceName, + }, + Data: map[string]string{ + caBundleFileName: string(awsTestCACert), + }, + }, + }, + veleroDeployment: testVeleroDeployment.DeepCopy(), + wantVeleroDeployment: createTestBuiltVeleroDeployment(TestBuiltVeleroDeploymentOptions{ + args: []string{ + defaultFileSystemBackupTimeout, + defaultRestoreResourcePriorities, + defaultDisableInformerCache, + }, + volumes: []corev1.Volume{ + { + Name: caCertVolumeName, + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: caBundleConfigMapName, + }, + }, + }, + }, + }, + volumeMounts: []corev1.VolumeMount{ + { + Name: caCertVolumeName, + MountPath: caCertMountPath, + ReadOnly: true, + }, + }, + env: append(baseEnvVars, corev1.EnvVar{ + Name: "AWS_CA_BUNDLE", + Value: caCertMountPath + "/" + caBundleFileName, + }), + }), + }, + { + name: "valid DPA CR with multiple BSLs having different CA certificates, should concatenate all certificates", + dpa: createTestDpaWith( + nil, + oadpv1alpha1.DataProtectionApplicationSpec{ + Configuration: &oadpv1alpha1.ApplicationConfig{ + Velero: &oadpv1alpha1.VeleroConfig{}, + }, + BackupImages: ptr.To(true), + BackupLocations: []oadpv1alpha1.BackupLocation{ + { + Velero: &velerov1.BackupStorageLocationSpec{ + Provider: "aws", + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + CACert: awsTestCACert, + }, + }, + }, + }, + { + Velero: &velerov1.BackupStorageLocationSpec{ + Provider: "aws", + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + CACert: dummy2TestCACert, + }, + }, + }, + }, + }, + }, + ), + clientObjects: []client.Object{ + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: caBundleConfigMapName, + Namespace: testNamespaceName, + }, + Data: map[string]string{ + caBundleFileName: string(awsTestCACert) + string(dummy2TestCACert), + }, + }, + }, + veleroDeployment: testVeleroDeployment.DeepCopy(), + wantVeleroDeployment: createTestBuiltVeleroDeployment(TestBuiltVeleroDeploymentOptions{ + args: []string{ + defaultFileSystemBackupTimeout, + defaultRestoreResourcePriorities, + defaultDisableInformerCache, + }, + volumes: []corev1.Volume{ + { + Name: caCertVolumeName, + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: caBundleConfigMapName, + }, + }, + }, + }, + }, + volumeMounts: []corev1.VolumeMount{ + { + Name: caCertVolumeName, + MountPath: caCertMountPath, + ReadOnly: true, + }, + }, + env: append(baseEnvVars, corev1.EnvVar{ + Name: "AWS_CA_BUNDLE", + Value: caCertMountPath + "/" + caBundleFileName, + }), + }), + }, + { + name: "valid DPA CR with duplicate CA certificates in different BSLs, should deduplicate", + dpa: createTestDpaWith( + nil, + oadpv1alpha1.DataProtectionApplicationSpec{ + Configuration: &oadpv1alpha1.ApplicationConfig{ + Velero: &oadpv1alpha1.VeleroConfig{}, + }, + BackupImages: ptr.To(true), + BackupLocations: []oadpv1alpha1.BackupLocation{ + { + Velero: &velerov1.BackupStorageLocationSpec{ + Provider: "aws", + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + CACert: awsTestCACert, + }, + }, + }, + }, + { + Velero: &velerov1.BackupStorageLocationSpec{ + Provider: "aws", + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + CACert: awsTestCACert, + }, + }, + }, + }, + { + Velero: &velerov1.BackupStorageLocationSpec{ + Provider: "aws", + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + CACert: dummy2TestCACert, + }, + }, + }, + }, + }, + }, + ), + clientObjects: []client.Object{ + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: caBundleConfigMapName, + Namespace: testNamespaceName, + }, + Data: map[string]string{ + // Should contain only unique certificates (awsTestCACert appears twice, gcpTestCACert once) + caBundleFileName: string(awsTestCACert) + string(dummy2TestCACert), + }, + }, + }, + veleroDeployment: testVeleroDeployment.DeepCopy(), + wantVeleroDeployment: createTestBuiltVeleroDeployment(TestBuiltVeleroDeploymentOptions{ + args: []string{ + defaultFileSystemBackupTimeout, + defaultRestoreResourcePriorities, + defaultDisableInformerCache, + }, + volumes: []corev1.Volume{ + { + Name: caCertVolumeName, + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: caBundleConfigMapName, + }, + }, + }, + }, + }, + volumeMounts: []corev1.VolumeMount{ + { + Name: caCertVolumeName, + MountPath: caCertMountPath, + ReadOnly: true, + }, + }, + env: append(baseEnvVars, corev1.EnvVar{ + Name: "AWS_CA_BUNDLE", + Value: caCertMountPath + "/" + caBundleFileName, + }), + }), + }, + { + name: "valid DPA CR with CA cert from CloudStorage BSL, should process correctly", + dpa: createTestDpaWith( + nil, + oadpv1alpha1.DataProtectionApplicationSpec{ + Configuration: &oadpv1alpha1.ApplicationConfig{ + Velero: &oadpv1alpha1.VeleroConfig{}, + }, + BackupImages: ptr.To(true), + BackupLocations: []oadpv1alpha1.BackupLocation{ + { + CloudStorage: &oadpv1alpha1.CloudStorageLocation{ + CloudStorageRef: corev1.LocalObjectReference{ + Name: "test-cloudstorage", + }, + CACert: cloudStorageTestCACert, + Credential: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "cloud-credentials", + }, + Key: "creds", + }, + }, + }, + }, + }, + ), + clientObjects: []client.Object{ + &oadpv1alpha1.CloudStorage{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cloudstorage", + Namespace: testNamespaceName, + }, + Spec: oadpv1alpha1.CloudStorageSpec{ + Name: "test-bucket", + Provider: oadpv1alpha1.AWSBucketProvider, + CreationSecret: corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "cloud-credentials", + }, + Key: "creds", + }, + }, + }, + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: caBundleConfigMapName, + Namespace: testNamespaceName, + }, + Data: map[string]string{ + caBundleFileName: string(cloudStorageTestCACert), + }, + }, + }, + veleroDeployment: testVeleroDeployment.DeepCopy(), + wantVeleroDeployment: createTestBuiltVeleroDeployment(TestBuiltVeleroDeploymentOptions{ + args: []string{ + defaultFileSystemBackupTimeout, + defaultRestoreResourcePriorities, + defaultDisableInformerCache, + }, + volumes: []corev1.Volume{ + { + Name: caCertVolumeName, + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: caBundleConfigMapName, + }, + }, + }, + }, + }, + volumeMounts: []corev1.VolumeMount{ + { + Name: caCertVolumeName, + MountPath: caCertMountPath, + ReadOnly: true, + }, + }, + env: append(baseEnvVars, corev1.EnvVar{ + Name: "AWS_CA_BUNDLE", + Value: caCertMountPath + "/" + caBundleFileName, + }), + }), + }, + { + name: "valid DPA CR with mixed BSLs (some with CA certs, some without), should only include BSLs with CA certs", + dpa: createTestDpaWith( + nil, + oadpv1alpha1.DataProtectionApplicationSpec{ + Configuration: &oadpv1alpha1.ApplicationConfig{ + Velero: &oadpv1alpha1.VeleroConfig{}, + }, + BackupImages: ptr.To(true), + BackupLocations: []oadpv1alpha1.BackupLocation{ + { + Velero: &velerov1.BackupStorageLocationSpec{ + Provider: "aws", + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + // No CACert + }, + }, + }, + }, + { + Velero: &velerov1.BackupStorageLocationSpec{ + Provider: "aws", + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + CACert: dummy2TestCACert, + }, + }, + }, + }, + { + CloudStorage: &oadpv1alpha1.CloudStorageLocation{ + CloudStorageRef: corev1.LocalObjectReference{ + Name: "test-cloudstorage-mixed", + }, + CACert: cloudStorageTestCACert, + Credential: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "cloud-credentials", + }, + Key: "creds", + }, + }, + }, + }, + }, + ), + clientObjects: []client.Object{ + &oadpv1alpha1.CloudStorage{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cloudstorage-mixed", + Namespace: testNamespaceName, + }, + Spec: oadpv1alpha1.CloudStorageSpec{ + Name: "test-bucket-mixed", + Provider: oadpv1alpha1.AWSBucketProvider, + CreationSecret: corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "cloud-credentials", + }, + Key: "creds", + }, + }, + }, + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: caBundleConfigMapName, + Namespace: testNamespaceName, + }, + Data: map[string]string{ + caBundleFileName: string(dummy2TestCACert) + string(cloudStorageTestCACert), + }, + }, + }, + veleroDeployment: testVeleroDeployment.DeepCopy(), + wantVeleroDeployment: createTestBuiltVeleroDeployment(TestBuiltVeleroDeploymentOptions{ + args: []string{ + defaultFileSystemBackupTimeout, + defaultRestoreResourcePriorities, + defaultDisableInformerCache, + }, + volumes: []corev1.Volume{ + { + Name: caCertVolumeName, + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: caBundleConfigMapName, + }, + }, + }, + }, + }, + volumeMounts: []corev1.VolumeMount{ + { + Name: caCertVolumeName, + MountPath: caCertMountPath, + ReadOnly: true, + }, + }, + env: append(baseEnvVars, corev1.EnvVar{ + Name: "AWS_CA_BUNDLE", + Value: caCertMountPath + "/" + caBundleFileName, + }), + }), + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { @@ -1861,7 +2407,19 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) { if err != nil { t.Errorf("error in creating fake client, likely programmer error") } - r := DPAReconciler{Client: fakeClient} + r := DPAReconciler{ + Client: fakeClient, + Scheme: fakeClient.Scheme(), + Log: logr.Discard(), + Context: context.Background(), + EventRecorder: record.NewFakeRecorder(10), + } + if test.dpa != nil { + r.NamespacedName = types.NamespacedName{ + Namespace: test.dpa.Namespace, + Name: test.dpa.Name, + } + } oadpClient.SetClient(fakeClient) if test.testProxy { t.Setenv(proxyEnvKey, proxyEnvValue) diff --git a/tests/e2e/backup_restore_suite_test.go b/tests/e2e/backup_restore_suite_test.go index 7f7b89657cc..280b5beff4b 100755 --- a/tests/e2e/backup_restore_suite_test.go +++ b/tests/e2e/backup_restore_suite_test.go @@ -1,6 +1,7 @@ package e2e_test import ( + "context" "fmt" "log" "os" @@ -10,8 +11,13 @@ import ( "github.com/google/uuid" "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" + velero "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" + oadpv1alpha1 "github.com/openshift/oadp-operator/api/v1alpha1" "github.com/openshift/oadp-operator/tests/e2e/lib" ) @@ -439,3 +445,300 @@ var _ = ginkgo.Describe("Backup and restore tests", ginkgo.Ordered, func() { }, nil), ) }) + +// Helper function to create a dummy CA certificate with unique identifier +func createDummyCACert(identifier string) []byte { + certTemplate := `-----BEGIN CERTIFICATE----- +MIIDazCCAlOgAwIBAgIUUf8+3K8zsP/w1P3VQ5jlMxALinkwDQYJKoZIhvcNAQEL +BQAwRTELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExDjAMBgNVBAoM +BU9BQVBQMREWFAYDVQQDDA1EVU1NWS1DQS1DRVJUMB4XDTI0MDEwMTAwMDAwMFoX +DTM0MDEwMTAwMDAwMFowRTELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3Ju +aWExDjAMBgNVBAoMBU9BQVBQMREWFAYDVQQDDA1EVU1NWS1DQS1DRVJUMIIBIJAN +BgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA0VUxbPWcfcOJC2qKZVv5nKqY7OZw +%s-CERT-CONTENT-%s-CERT-CONTENT-%s-CERT-CONTENT-%s-CERT-CONTENT-%s +%s-CERT-CONTENT-%s-CERT-CONTENT-%s-CERT-CONTENT-%s-CERT-CONTENT-%s +%s-CERT-CONTENT-%s-CERT-CONTENT-%s-CERT-CONTENT-%s-CERT-CONTENT-%s +%s-CERT-CONTENT-%s-CERT-CONTENT-%s-CERT-CONTENT-%s-CERT-CONTENT-%s +%s-CERT-CONTENT-%s-CERT-CONTENT-%s-CERT-CONTENT-%s-CERT-CONTENT-%s +ngpurposesonly1234567890QIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQBYfMVqNb +iVL1x+dummyenddummyenddummyenddummyenddummyenddummyenddummyenddum +%s-CERT-END-%s-CERT-END-%s-CERT-END-%s-CERT-END-%s-CERT-END-%s-END +%s-CERT-END-%s-CERT-END-%s-CERT-END-%s-CERT-END-%s-CERT-END-%s-END +ddummyenddummyenddummyenddummyend +-----END CERTIFICATE-----` + + // Replace placeholders with the identifier + cert := certTemplate + for i := 0; i < 50; i++ { + cert = strings.Replace(cert, "%s", identifier, 1) + } + return []byte(cert) +} + +var _ = ginkgo.Describe("Multiple BSL with custom CA cert tests", ginkgo.Ordered, func() { + var _ = ginkgo.AfterEach(func(ctx ginkgo.SpecContext) { + log.Printf("Cleaning up after BSL CA cert test") + if !skipMustGather && ctx.SpecReport().Failed() { + log.Printf("Running must-gather for failed test") + _ = lib.RunMustGather(artifact_dir, dpaCR.Client) + } + log.Printf("Deleting DPA") + err := dpaCR.Delete() + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + log.Printf("Waiting for velero to be deleted") + gomega.Eventually(lib.VeleroIsDeleted(kubernetesClientForSuiteRun, namespace), time.Minute*3, time.Second*5).Should(gomega.BeTrue()) + }) + + ginkgo.DescribeTable("BSL CA certificate handling with multiple BSLs", + func(backupImages bool, expectCACertHandling bool) { + testNamespace := "test-bsl-cacert" + + log.Printf("Creating test namespace %s", testNamespace) + err := lib.CreateNamespace(kubernetesClientForSuiteRun, testNamespace) + gomega.Expect(err).To(gomega.BeNil()) + gomega.Expect(lib.DoesNamespaceExist(kubernetesClientForSuiteRun, testNamespace)).Should(gomega.BeTrue()) + + defer func() { + log.Printf("Cleaning up test namespace %s", testNamespace) + _ = lib.DeleteNamespace(kubernetesClientForSuiteRun, testNamespace) + }() + + log.Printf("Test case: backupImages=%v, expectCACertHandling=%v", backupImages, expectCACertHandling) + + // Create unique CA certificates for each BSL + secondCACert := createDummyCACert("SECOND") + thirdCACert := createDummyCACert("THIRD") + + log.Printf("Creating DPA with three BSLs and backupImages=%v", backupImages) + dpaSpec := dpaCR.Build(lib.CSI) + + // Set the backupImages flag + dpaSpec.BackupImages = &backupImages + + // Add a second BSL with custom CA cert (it doesn't need to be available) + secondBSL := oadpv1alpha1.BackupLocation{ + Velero: &velero.BackupStorageLocationSpec{ + Provider: dpaCR.BSLProvider, + Default: false, + Config: dpaCR.BSLConfig, + Credential: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: dpaCR.BSLSecretName, + }, + Key: "cloud", + }, + StorageType: velero.StorageType{ + ObjectStorage: &velero.ObjectStorageLocation{ + Bucket: dpaCR.BSLBucket, + Prefix: dpaCR.BSLBucketPrefix + "-secondary", + CACert: secondCACert, + }, + }, + }, + } + + // Add a third BSL with another custom CA cert + thirdBSL := oadpv1alpha1.BackupLocation{ + Velero: &velero.BackupStorageLocationSpec{ + Provider: dpaCR.BSLProvider, + Default: false, + Config: dpaCR.BSLConfig, + Credential: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: dpaCR.BSLSecretName, + }, + Key: "cloud", + }, + StorageType: velero.StorageType{ + ObjectStorage: &velero.ObjectStorageLocation{ + Bucket: dpaCR.BSLBucket, + Prefix: dpaCR.BSLBucketPrefix + "-third", + CACert: thirdCACert, + }, + }, + }, + } + + dpaSpec.BackupLocations = append(dpaSpec.BackupLocations, secondBSL, thirdBSL) + + err = dpaCR.CreateOrUpdate(dpaSpec) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + log.Print("Checking if DPA is reconciled") + gomega.Eventually(dpaCR.IsReconciledTrue(), time.Minute*3, time.Second*5).Should(gomega.BeTrue()) + + log.Printf("Waiting for Velero Pod to be running") + gomega.Eventually(lib.VeleroPodIsRunning(kubernetesClientForSuiteRun, namespace), time.Minute*3, time.Second*5).Should(gomega.BeTrue()) + + // Verify CA certificate handling based on backupImages flag + log.Printf("Verifying CA certificate handling (backupImages: %v)", backupImages) + + veleroPods, err := kubernetesClientForSuiteRun.CoreV1().Pods(namespace).List(context.Background(), metav1.ListOptions{ + LabelSelector: "component=velero", + }) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + gomega.Expect(len(veleroPods.Items)).To(gomega.BeNumerically(">", 0)) + + veleroPod := veleroPods.Items[0] + veleroContainer := veleroPod.Spec.Containers[0] + + if !backupImages { + // When backupImages is false, NO CA cert processing should occur + log.Printf("Verifying NO CA certificate processing when backupImages=false") + + // Check AWS_CA_BUNDLE env var does NOT exist + awsCABundleFound := false + for _, env := range veleroContainer.Env { + if env.Name == "AWS_CA_BUNDLE" { + awsCABundleFound = true + log.Printf("ERROR: Found unexpected AWS_CA_BUNDLE environment variable: %s", env.Value) + break + } + } + gomega.Expect(awsCABundleFound).To(gomega.BeFalse(), "AWS_CA_BUNDLE environment variable should NOT be set when backupImages=false") + + // Verify CA cert ConfigMap is NOT mounted + caCertVolumeMountFound := false + for _, mount := range veleroContainer.VolumeMounts { + if mount.Name == "custom-ca-certs" { + caCertVolumeMountFound = true + log.Printf("ERROR: Found unexpected CA cert volume mount: %s at %s", mount.Name, mount.MountPath) + break + } + } + gomega.Expect(caCertVolumeMountFound).To(gomega.BeFalse(), "CA cert volume should NOT be mounted when backupImages=false") + + // Verify the ConfigMap does NOT exist + configMapName := "oadp-" + dpaCR.Name + "-ca-bundle" + _, err := kubernetesClientForSuiteRun.CoreV1().ConfigMaps(namespace).Get(context.Background(), configMapName, metav1.GetOptions{}) + gomega.Expect(err).To(gomega.HaveOccurred(), "CA bundle ConfigMap should NOT exist when backupImages=false") + gomega.Expect(apierrors.IsNotFound(err)).To(gomega.BeTrue(), "ConfigMap should be not found") + + } else { + // When backupImages is true, CA cert processing should include all three BSLs + log.Printf("Verifying CA certificate processing when backupImages=true") + + // Check AWS_CA_BUNDLE env var exists + awsCABundleFound := false + awsCABundlePath := "" + for _, env := range veleroContainer.Env { + if env.Name == "AWS_CA_BUNDLE" { + awsCABundleFound = true + awsCABundlePath = env.Value + log.Printf("Found AWS_CA_BUNDLE environment variable: %s", awsCABundlePath) + break + } + } + gomega.Expect(awsCABundleFound).To(gomega.BeTrue(), "AWS_CA_BUNDLE environment variable should be set when backupImages=true") + gomega.Expect(awsCABundlePath).To(gomega.Equal("/etc/velero/ca-certs/ca-bundle.pem")) + + // Verify CA cert ConfigMap is mounted + caCertVolumeMountFound := false + for _, mount := range veleroContainer.VolumeMounts { + if mount.Name == "ca-certificate-bundle" && mount.MountPath == "/etc/velero/ca-certs" { + caCertVolumeMountFound = true + log.Printf("Found CA cert volume mount: %s at %s", mount.Name, mount.MountPath) + break + } + } + gomega.Expect(caCertVolumeMountFound).To(gomega.BeTrue(), "CA cert volume should be mounted when backupImages=true") + + // Verify the ConfigMap exists and contains all three custom CAs plus system CAs + log.Printf("Verifying CA certificate ConfigMap contents") + configMapName := "velero-ca-bundle" + configMap, err := kubernetesClientForSuiteRun.CoreV1().ConfigMaps(namespace).Get(context.Background(), configMapName, metav1.GetOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + caBundleContent, exists := configMap.Data["ca-bundle.pem"] + gomega.Expect(exists).To(gomega.BeTrue(), "ca-bundle.pem should exist in ConfigMap") + + // Verify bundle contains all three custom certificates + gomega.Expect(caBundleContent).To(gomega.ContainSubstring("SECOND-CERT-CONTENT"), "CA bundle should contain second BSL's certificate") + gomega.Expect(caBundleContent).To(gomega.ContainSubstring("THIRD-CERT-CONTENT"), "CA bundle should contain third BSL's certificate") + + // Verify bundle contains system certificates marker + gomega.Expect(caBundleContent).To(gomega.ContainSubstring("# System default CA certificates"), "CA bundle should include system certificates marker") + + log.Printf("CA bundle size: %d bytes", len(caBundleContent)) + + // Verify that the bundle is reasonably large (indicating system certs are included) + // System certs are typically > 100KB + gomega.Expect(len(caBundleContent)).To(gomega.BeNumerically(">", 50000), "CA bundle should be large enough to include system certificates") + } + + // Check BSL status - only the default BSL needs to be available + log.Print("Checking if default BSL is available") + bsls, err := dpaCR.ListBSLs() + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + gomega.Expect(len(bsls.Items)).To(gomega.Equal(3), "Should have 3 BSLs configured") + + // Find the default BSL + var defaultBSL *velero.BackupStorageLocation + for i, bsl := range bsls.Items { + if bsl.Spec.Default { + defaultBSL = &bsls.Items[i] + break + } + } + gomega.Expect(defaultBSL).NotTo(gomega.BeNil(), "Default BSL should exist") + + // Only the default BSL needs to be available for the test + gomega.Eventually(func() bool { + bsl := &velero.BackupStorageLocation{} + err := dpaCR.Client.Get(context.Background(), client.ObjectKey{ + Namespace: namespace, + Name: defaultBSL.Name, + }, bsl) + if err != nil { + return false + } + return bsl.Status.Phase == velero.BackupStorageLocationPhaseAvailable + }, time.Minute*3, time.Second*5).Should(gomega.BeTrue(), "Default BSL should be available") + + log.Printf("Deploying test application") + err = lib.InstallApplication(dpaCR.Client, "./sample-applications/nginx/nginx-deployment.yaml") + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + + // nginx-deployment.yaml creates its own namespace, so we just wait for deployment to be ready + gomega.Eventually(lib.IsDeploymentReady(dpaCR.Client, "nginx-example", "nginx-deployment"), time.Minute*3, time.Second*5).Should(gomega.BeTrue()) + + log.Printf("Creating backup using default BSL") + backupUid, _ := uuid.NewUUID() + backupName := fmt.Sprintf("backup-bsl-cacert-%s", backupUid.String()) + err = lib.CreateBackupForNamespaces(dpaCR.Client, namespace, backupName, []string{"nginx-example"}, true, true) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + gomega.Eventually(func() bool { + result, _ := lib.IsBackupCompletedSuccessfully(kubernetesClientForSuiteRun, dpaCR.Client, namespace, backupName) + return result + }, time.Minute*10, time.Second*10).Should(gomega.BeTrue()) + + log.Printf("Verifying backup was created with default BSL") + completedBackup, err := lib.GetBackup(dpaCR.Client, namespace, backupName) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + // Verify it used the default BSL + gomega.Expect(completedBackup.Spec.StorageLocation).Should(gomega.Equal(defaultBSL.Name)) + + log.Printf("Deleting application namespace") + err = lib.DeleteNamespace(kubernetesClientForSuiteRun, "nginx-example") + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + gomega.Eventually(lib.IsNamespaceDeleted(kubernetesClientForSuiteRun, "nginx-example"), time.Minute*3, time.Second*5).Should(gomega.BeTrue()) + + log.Printf("Creating restore from backup") + restoreUid, _ := uuid.NewUUID() + restoreName := fmt.Sprintf("restore-bsl-cacert-%s", restoreUid.String()) + err = lib.CreateRestoreFromBackup(dpaCR.Client, namespace, backupName, restoreName) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + gomega.Eventually(func() bool { + result, _ := lib.IsRestoreCompletedSuccessfully(kubernetesClientForSuiteRun, dpaCR.Client, namespace, restoreName) + return result + }, time.Minute*10, time.Second*10).Should(gomega.BeTrue()) + + log.Printf("Verifying application was restored") + gomega.Eventually(lib.IsDeploymentReady(dpaCR.Client, "nginx-example", "nginx-deployment"), time.Minute*3, time.Second*5).Should(gomega.BeTrue()) + + log.Printf("Test completed successfully - backupImages=%v test passed", backupImages) + }, + ginkgo.Entry("three BSLs with backupImages=false (no CA cert handling)", false, false), + ginkgo.Entry("three BSLs with backupImages=true (full CA cert handling with concatenation)", true, true), + ) +}) diff --git a/tests/e2e/dpa_deployment_suite_test.go b/tests/e2e/dpa_deployment_suite_test.go index 8afcde680b2..42bc04c31a9 100644 --- a/tests/e2e/dpa_deployment_suite_test.go +++ b/tests/e2e/dpa_deployment_suite_test.go @@ -52,6 +52,7 @@ func createTestDPASpec(testSpec TestDPASpec) *oadpv1alpha1.DataProtectionApplica ObjectStorage: &velero.ObjectStorageLocation{ Bucket: dpaCR.BSLBucket, Prefix: dpaCR.BSLBucketPrefix, + CACert: dpaCR.BSLCacert, }, }, Provider: dpaCR.BSLProvider, diff --git a/tests/e2e/e2e_suite_test.go b/tests/e2e/e2e_suite_test.go index 41305bbaedb..00959fa0ea9 100755 --- a/tests/e2e/e2e_suite_test.go +++ b/tests/e2e/e2e_suite_test.go @@ -160,6 +160,7 @@ func TestOADPE2E(t *testing.T) { BSLConfig: dpa.DeepCopy().Spec.BackupLocations[0].Velero.Config, BSLProvider: dpa.DeepCopy().Spec.BackupLocations[0].Velero.Provider, BSLBucket: dpa.DeepCopy().Spec.BackupLocations[0].Velero.ObjectStorage.Bucket, + BSLCacert: dpa.DeepCopy().Spec.BackupLocations[0].Velero.ObjectStorage.CACert, BSLBucketPrefix: veleroPrefix, VeleroDefaultPlugins: dpa.DeepCopy().Spec.Configuration.Velero.DefaultPlugins, SnapshotLocations: dpa.DeepCopy().Spec.SnapshotLocations, diff --git a/tests/e2e/lib/dpa_helpers.go b/tests/e2e/lib/dpa_helpers.go index 71c4c8c0081..cdbe61b03e6 100755 --- a/tests/e2e/lib/dpa_helpers.go +++ b/tests/e2e/lib/dpa_helpers.go @@ -40,6 +40,7 @@ type DpaCustomResource struct { BSLConfig map[string]string BSLProvider string BSLBucket string + BSLCacert []byte BSLBucketPrefix string VeleroDefaultPlugins []oadpv1alpha1.DefaultPlugin SnapshotLocations []oadpv1alpha1.SnapshotLocation @@ -90,6 +91,7 @@ func (v *DpaCustomResource) Build(backupRestoreType BackupRestoreType) *oadpv1al ObjectStorage: &velero.ObjectStorageLocation{ Bucket: v.BSLBucket, Prefix: v.BSLBucketPrefix, + CACert: v.BSLCacert, }, }, }, diff --git a/tests/e2e/sample-applications/nginx/nginx-deployment.yaml b/tests/e2e/sample-applications/nginx/nginx-deployment.yaml index 47859697f97..3a1c6a29d92 100644 --- a/tests/e2e/sample-applications/nginx/nginx-deployment.yaml +++ b/tests/e2e/sample-applications/nginx/nginx-deployment.yaml @@ -12,64 +12,60 @@ # See the License for the specific language governing permissions and # limitations under the License. ---- apiVersion: v1 -kind: Namespace -metadata: - name: nginx-example - labels: - app: nginx - ---- -apiVersion: apps/v1 -kind: Deployment -metadata: - name: nginx-deployment - namespace: nginx-example -spec: - replicas: 2 - selector: - matchLabels: +kind: List +items: +- apiVersion: v1 + kind: Namespace + metadata: + name: nginx-example + labels: app: nginx - template: - metadata: - labels: +- apiVersion: apps/v1 + kind: Deployment + metadata: + name: nginx-deployment + namespace: nginx-example + spec: + replicas: 2 + selector: + matchLabels: app: nginx - spec: - containers: - - image: docker.io/bitnami/nginx - name: nginx - ports: - - containerPort: 8080 - ---- -apiVersion: v1 -kind: Service -metadata: - labels: - app: nginx - name: my-nginx - namespace: nginx-example -spec: - ports: - - port: 8080 - targetPort: 8080 - selector: - app: nginx - type: LoadBalancer - ---- -apiVersion: route.openshift.io/v1 -kind: Route -metadata: - name: my-nginx - namespace: nginx-example - labels: - app: nginx - service: my-nginx -spec: - to: - kind: Service + template: + metadata: + labels: + app: nginx + spec: + containers: + - image: bitnamisecure/nginx + name: nginx + ports: + - containerPort: 8080 +- apiVersion: v1 + kind: Service + metadata: + labels: + app: nginx + name: my-nginx + namespace: nginx-example + spec: + ports: + - port: 8080 + targetPort: 8080 + selector: + app: nginx + type: LoadBalancer +- apiVersion: route.openshift.io/v1 + kind: Route + metadata: name: my-nginx - port: - targetPort: 8080 \ No newline at end of file + namespace: nginx-example + labels: + app: nginx + service: my-nginx + spec: + to: + kind: Service + name: my-nginx + port: + targetPort: 8080 diff --git a/tests/e2e/upgrade_suite_test.go b/tests/e2e/upgrade_suite_test.go index 4f5feb9e01b..6426a834dc8 100644 --- a/tests/e2e/upgrade_suite_test.go +++ b/tests/e2e/upgrade_suite_test.go @@ -111,6 +111,7 @@ var _ = ginkgo.Describe("OADP upgrade scenarios", ginkgo.Ordered, func() { ObjectStorage: &velerov1.ObjectStorageLocation{ Bucket: dpaCR.BSLBucket, Prefix: dpaCR.BSLBucketPrefix, + CACert: dpaCR.BSLCacert, }, }, },