diff --git a/internal/controller/kubevirt_dataupload_controller.go b/internal/controller/kubevirt_dataupload_controller.go index bcb32ab8..792dec8d 100644 --- a/internal/controller/kubevirt_dataupload_controller.go +++ b/internal/controller/kubevirt_dataupload_controller.go @@ -1585,27 +1585,31 @@ func (r *KubeVirtDataUploadReconciler) buildDatamoverPodConfig( } return &DatamoverPodConfig{ - Name: du.Name, // Used as a prefix for GenerateName - Namespace: vmRef.Namespace, - Image: image, - ImagePullPolicy: pullPolicy, - BSLProvider: cfg.Provider, - BSLBucket: cfg.Bucket, - BSLPrefix: cfg.Prefix, - BSLRegion: cfg.Region, - CredentialSecretName: cfg.CredentialName, - CredentialSecretKey: cfg.CredentialKey, - VMName: vmRef.Name, - VMNamespace: vmRef.Namespace, - CheckpointName: checkpointName, - BackupType: backupType, - VeleroBackupName: getVeleroBackupName(du), - DataUploadName: du.Name, - DataUploadUID: string(du.UID), - VMBName: vmb.Name, - VMBTName: vmbtName, - SourcePVCName: "", // overridden by handlePrepared with the rebound PVC name - Labels: make(map[string]string), + Name: du.Name, // Used as a prefix for GenerateName + Namespace: vmRef.Namespace, + Image: image, + ImagePullPolicy: pullPolicy, + BSLProvider: cfg.Provider, + BSLBucket: cfg.Bucket, + BSLPrefix: cfg.Prefix, + BSLRegion: cfg.Region, + BSLS3URL: cfg.S3URL, + BSLS3ForcePathStyle: strconv.FormatBool(cfg.S3ForcePathStyle), + BSLInsecureSkipTLSVerify: strconv.FormatBool(cfg.InsecureSkipTLSVerify), + BSLCACert: cfg.CACert, + CredentialSecretName: cfg.CredentialName, + CredentialSecretKey: cfg.CredentialKey, + VMName: vmRef.Name, + VMNamespace: vmRef.Namespace, + CheckpointName: checkpointName, + BackupType: backupType, + VeleroBackupName: getVeleroBackupName(du), + DataUploadName: du.Name, + DataUploadUID: string(du.UID), + VMBName: vmb.Name, + VMBTName: vmbtName, + SourcePVCName: "", // overridden by handlePrepared with the rebound PVC name + Labels: make(map[string]string), }, nil } @@ -1647,6 +1651,12 @@ type bslConfig struct { Region string CredentialName string CredentialKey string + + // S3-compatible storage provider settings + S3URL string + S3ForcePathStyle bool + InsecureSkipTLSVerify bool + CACert string } // extractBSLConfig extracts and validates common BSL configuration fields. @@ -1670,8 +1680,16 @@ func extractBSLConfig(bsl *velerov1.BackupStorageLocation) (*bslConfig, error) { } region := "" + s3URL := "" + s3ForcePathStyle := false + insecureSkipTLSVerify := false + caCert := "" if bsl.Spec.Config != nil { region = bsl.Spec.Config["region"] + s3URL = bsl.Spec.Config["s3Url"] + s3ForcePathStyle = strings.EqualFold(bsl.Spec.Config["s3ForcePathStyle"], "true") + insecureSkipTLSVerify = strings.EqualFold(bsl.Spec.Config["insecureSkipTLSVerify"], "true") + caCert = bsl.Spec.Config["caCert"] } credName := "" @@ -1684,12 +1702,16 @@ func extractBSLConfig(bsl *velerov1.BackupStorageLocation) (*bslConfig, error) { } return &bslConfig{ - Provider: bsl.Spec.Provider, - Bucket: bucket, - Prefix: prefix, - Region: region, - CredentialName: credName, - CredentialKey: credKey, + Provider: bsl.Spec.Provider, + Bucket: bucket, + Prefix: prefix, + Region: region, + CredentialName: credName, + CredentialKey: credKey, + S3URL: s3URL, + S3ForcePathStyle: s3ForcePathStyle, + InsecureSkipTLSVerify: insecureSkipTLSVerify, + CACert: caCert, }, nil } @@ -1737,11 +1759,15 @@ func (r *KubeVirtDataUploadReconciler) initObjectStoreFromBSL( } store, err := factory(&uploader.UploaderConfig{ - BSLProvider: cfg.Provider, - BSLBucket: cfg.Bucket, - BSLPrefix: cfg.Prefix, - BSLRegion: cfg.Region, - CredentialsData: credData, + BSLProvider: cfg.Provider, + BSLBucket: cfg.Bucket, + BSLPrefix: cfg.Prefix, + BSLRegion: cfg.Region, + BSLS3URL: cfg.S3URL, + BSLS3ForcePathStyle: cfg.S3ForcePathStyle, + BSLInsecureSkipTLSVerify: cfg.InsecureSkipTLSVerify, + BSLCACert: cfg.CACert, + CredentialsData: credData, }) if err != nil { return nil, nil, fmt.Errorf("failed to initialize object store: %w", err) diff --git a/internal/controller/kubevirt_dataupload_controller_test.go b/internal/controller/kubevirt_dataupload_controller_test.go index 05925e27..408c08de 100644 --- a/internal/controller/kubevirt_dataupload_controller_test.go +++ b/internal/controller/kubevirt_dataupload_controller_test.go @@ -4372,6 +4372,107 @@ func TestExtractBSLConfig(t *testing.T) { } }, }, + { + name: "S3-compatible config keys are extracted", + bsl: &velerov1.BackupStorageLocation{ + ObjectMeta: metav1.ObjectMeta{Name: "minio-bsl"}, + Spec: velerov1.BackupStorageLocationSpec{ + Provider: "aws", + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + Bucket: "my-bucket", + }, + }, + Config: map[string]string{ + "region": "us-east-1", + "s3Url": "https://minio.example.com", + "s3ForcePathStyle": "true", + "insecureSkipTLSVerify": "true", + "caCert": "-----BEGIN CERTIFICATE-----\nMIIBxTCCAW...\n-----END CERTIFICATE-----", + }, + Credential: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "creds"}, + }, + }, + }, + validate: func(t *testing.T, cfg *bslConfig) { + if cfg.S3URL != "https://minio.example.com" { + t.Errorf("S3URL = %q, want %q", cfg.S3URL, "https://minio.example.com") + } + if !cfg.S3ForcePathStyle { + t.Error("S3ForcePathStyle = false, want true") + } + if !cfg.InsecureSkipTLSVerify { + t.Error("InsecureSkipTLSVerify = false, want true") + } + if cfg.CACert != "-----BEGIN CERTIFICATE-----\nMIIBxTCCAW...\n-----END CERTIFICATE-----" { + t.Errorf("CACert = %q, want PEM content", cfg.CACert) + } + }, + }, + { + name: "S3-compatible booleans are case-insensitive", + bsl: &velerov1.BackupStorageLocation{ + ObjectMeta: metav1.ObjectMeta{Name: "bsl"}, + Spec: velerov1.BackupStorageLocationSpec{ + Provider: "aws", + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + Bucket: "bucket", + }, + }, + Config: map[string]string{ + "s3ForcePathStyle": "TRUE", + "insecureSkipTLSVerify": "True", + }, + Credential: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "creds"}, + }, + }, + }, + validate: func(t *testing.T, cfg *bslConfig) { + if !cfg.S3ForcePathStyle { + t.Error("S3ForcePathStyle = false, want true (case-insensitive)") + } + if !cfg.InsecureSkipTLSVerify { + t.Error("InsecureSkipTLSVerify = false, want true (case-insensitive)") + } + }, + }, + { + name: "S3-compatible fields default when Config has no S3 keys", + bsl: &velerov1.BackupStorageLocation{ + ObjectMeta: metav1.ObjectMeta{Name: "bsl"}, + Spec: velerov1.BackupStorageLocationSpec{ + Provider: "aws", + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + Bucket: "bucket", + }, + }, + Config: map[string]string{ + "region": "us-east-1", + }, + Credential: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "creds"}, + }, + }, + }, + validate: func(t *testing.T, cfg *bslConfig) { + if cfg.S3URL != "" { + t.Errorf("S3URL = %q, want empty", cfg.S3URL) + } + if cfg.S3ForcePathStyle { + t.Error("S3ForcePathStyle = true, want false") + } + if cfg.InsecureSkipTLSVerify { + t.Error("InsecureSkipTLSVerify = true, want false") + } + if cfg.CACert != "" { + t.Errorf("CACert = %q, want empty", cfg.CACert) + } + }, + }, } for _, tt := range tests { diff --git a/internal/controller/pod_builder.go b/internal/controller/pod_builder.go index 29bc4e06..6e65a812 100644 --- a/internal/controller/pod_builder.go +++ b/internal/controller/pod_builder.go @@ -41,6 +41,12 @@ type DatamoverPodConfig struct { BSLPrefix string BSLRegion string + // S3-compatible storage provider settings + BSLS3URL string + BSLS3ForcePathStyle string + BSLInsecureSkipTLSVerify string + BSLCACert string + // Credentials CredentialSecretName string CredentialSecretKey string @@ -85,6 +91,10 @@ func buildDatamoverPod(config *DatamoverPodConfig) *corev1.Pod { {Name: uploader.EnvBSLBucket, Value: config.BSLBucket}, {Name: uploader.EnvBSLPrefix, Value: config.BSLPrefix}, {Name: uploader.EnvBSLRegion, Value: config.BSLRegion}, + {Name: uploader.EnvBSLS3URL, Value: config.BSLS3URL}, + {Name: uploader.EnvBSLS3ForcePathStyle, Value: config.BSLS3ForcePathStyle}, + {Name: uploader.EnvBSLInsecureSkipTLSVerify, Value: config.BSLInsecureSkipTLSVerify}, + {Name: uploader.EnvBSLCACert, Value: config.BSLCACert}, {Name: uploader.EnvCredentialsFile, Value: uploader.DefaultCredentialsPath}, {Name: uploader.EnvVMName, Value: config.VMName}, {Name: uploader.EnvVMNamespace, Value: config.VMNamespace}, diff --git a/internal/controller/pod_builder_test.go b/internal/controller/pod_builder_test.go index 72c31deb..05dfd7e2 100644 --- a/internal/controller/pod_builder_test.go +++ b/internal/controller/pod_builder_test.go @@ -17,6 +17,7 @@ limitations under the License. package controller import ( + "strings" "testing" "github.com/migtools/kubevirt-datamover-controller/pkg/common" @@ -103,24 +104,28 @@ func TestBuildDatamoverPod(t *testing.T) { { name: "environment variables are set correctly", config: &DatamoverPodConfig{ - Name: "test-pod", - Namespace: "default", - Image: "test-image", - BSLProvider: "aws", - BSLBucket: "my-bucket", - BSLPrefix: "my-prefix", - BSLRegion: "eu-west-1", - CredentialSecretName: "secret", - CredentialSecretKey: "key", - VMName: "my-vm", - VMNamespace: "my-ns", - CheckpointName: "cp-001", - BackupType: "incremental", - VeleroBackupName: "velero-backup", - DataUploadName: "du-001", - DataUploadUID: "uid-001", - VMBName: "vmb-001", - SourcePVCName: "pvc-001", + Name: "test-pod", + Namespace: "default", + Image: "test-image", + BSLProvider: "aws", + BSLBucket: "my-bucket", + BSLPrefix: "my-prefix", + BSLRegion: "eu-west-1", + BSLS3URL: "https://minio.example.com", + BSLS3ForcePathStyle: "true", + BSLInsecureSkipTLSVerify: "false", + BSLCACert: "-----BEGIN CERTIFICATE-----\ntest\n-----END CERTIFICATE-----", + CredentialSecretName: "secret", + CredentialSecretKey: "key", + VMName: "my-vm", + VMNamespace: "my-ns", + CheckpointName: "cp-001", + BackupType: "incremental", + VeleroBackupName: "velero-backup", + DataUploadName: "du-001", + DataUploadUID: "uid-001", + VMBName: "vmb-001", + SourcePVCName: "pvc-001", }, validate: func(t *testing.T, pod *corev1.Pod) { container := pod.Spec.Containers[0] @@ -130,20 +135,24 @@ func TestBuildDatamoverPod(t *testing.T) { } expectedEnvs := map[string]string{ - uploader.EnvBSLProvider: "aws", - uploader.EnvBSLBucket: "my-bucket", - uploader.EnvBSLPrefix: "my-prefix", - uploader.EnvBSLRegion: "eu-west-1", - uploader.EnvCredentialsFile: uploader.DefaultCredentialsPath, - uploader.EnvVMName: "my-vm", - uploader.EnvVMNamespace: "my-ns", - uploader.EnvCheckpointName: "cp-001", - uploader.EnvBackupType: "incremental", - uploader.EnvVeleroBackupName: "velero-backup", - uploader.EnvSourcePVCPath: uploader.DefaultSourcePVCPath, - uploader.EnvDataUploadName: "du-001", - uploader.EnvDataUploadUID: "uid-001", - uploader.EnvVMBName: "vmb-001", + uploader.EnvBSLProvider: "aws", + uploader.EnvBSLBucket: "my-bucket", + uploader.EnvBSLPrefix: "my-prefix", + uploader.EnvBSLRegion: "eu-west-1", + uploader.EnvBSLS3URL: "https://minio.example.com", + uploader.EnvBSLS3ForcePathStyle: "true", + uploader.EnvBSLInsecureSkipTLSVerify: "false", + uploader.EnvBSLCACert: "-----BEGIN CERTIFICATE-----\ntest\n-----END CERTIFICATE-----", + uploader.EnvCredentialsFile: uploader.DefaultCredentialsPath, + uploader.EnvVMName: "my-vm", + uploader.EnvVMNamespace: "my-ns", + uploader.EnvCheckpointName: "cp-001", + uploader.EnvBackupType: "incremental", + uploader.EnvVeleroBackupName: "velero-backup", + uploader.EnvSourcePVCPath: uploader.DefaultSourcePVCPath, + uploader.EnvDataUploadName: "du-001", + uploader.EnvDataUploadUID: "uid-001", + uploader.EnvVMBName: "vmb-001", } for key, expected := range expectedEnvs { @@ -334,3 +343,82 @@ func TestDatamoverPodConfigDefaults(t *testing.T) { t.Errorf("source pvc path = %q, want default %q", envMap[uploader.EnvSourcePVCPath], uploader.DefaultSourcePVCPath) } } + +// TestS3CompatibleBooleanRoundtrip verifies the strconv.FormatBool → env var → +// strings.EqualFold roundtrip. The controller converts bools to "true"/"false" +// strings for env vars; the uploader parses them back. Only "true" (case- +// insensitive) should produce true; "false" must not. +func TestS3CompatibleBooleanRoundtrip(t *testing.T) { + tests := []struct { + name string + s3ForcePathStyle string + insecureSkipTLSVerify string + wantForcePathStyle bool + wantInsecureSkip bool + }{ + { + name: "both true", + s3ForcePathStyle: "true", + insecureSkipTLSVerify: "true", + wantForcePathStyle: true, + wantInsecureSkip: true, + }, + { + name: "both false (from strconv.FormatBool)", + s3ForcePathStyle: "false", + insecureSkipTLSVerify: "false", + wantForcePathStyle: false, + wantInsecureSkip: false, + }, + { + name: "both empty", + s3ForcePathStyle: "", + insecureSkipTLSVerify: "", + wantForcePathStyle: false, + wantInsecureSkip: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + config := &DatamoverPodConfig{ + Name: "test", + Namespace: "default", + Image: "test", + CredentialSecretName: "secret", + CredentialSecretKey: "key", + SourcePVCName: "pvc", + BSLS3ForcePathStyle: tt.s3ForcePathStyle, + BSLInsecureSkipTLSVerify: tt.insecureSkipTLSVerify, + } + + pod := buildDatamoverPod(config) + container := pod.Spec.Containers[0] + envMap := make(map[string]string) + for _, env := range container.Env { + envMap[env.Name] = env.Value + } + + // Verify env var values match input + if envMap[uploader.EnvBSLS3ForcePathStyle] != tt.s3ForcePathStyle { + t.Errorf("env %s = %q, want %q", + uploader.EnvBSLS3ForcePathStyle, envMap[uploader.EnvBSLS3ForcePathStyle], tt.s3ForcePathStyle) + } + if envMap[uploader.EnvBSLInsecureSkipTLSVerify] != tt.insecureSkipTLSVerify { + t.Errorf("env %s = %q, want %q", + uploader.EnvBSLInsecureSkipTLSVerify, envMap[uploader.EnvBSLInsecureSkipTLSVerify], tt.insecureSkipTLSVerify) + } + + // Simulate uploader-side parsing (same logic as LoadConfigFromEnv) + gotForcePathStyle := strings.EqualFold(envMap[uploader.EnvBSLS3ForcePathStyle], "true") + gotInsecureSkip := strings.EqualFold(envMap[uploader.EnvBSLInsecureSkipTLSVerify], "true") + + if gotForcePathStyle != tt.wantForcePathStyle { + t.Errorf("parsed s3ForcePathStyle = %v, want %v", gotForcePathStyle, tt.wantForcePathStyle) + } + if gotInsecureSkip != tt.wantInsecureSkip { + t.Errorf("parsed insecureSkipTLSVerify = %v, want %v", gotInsecureSkip, tt.wantInsecureSkip) + } + }) + } +} diff --git a/pkg/uploader/objectstore.go b/pkg/uploader/objectstore.go index 0111762b..9dada0e7 100644 --- a/pkg/uploader/objectstore.go +++ b/pkg/uploader/objectstore.go @@ -19,9 +19,13 @@ package uploader import ( "bytes" "context" + "crypto/tls" + "crypto/x509" "errors" "fmt" "io" + "net/http" + "net/url" "os" "strings" "time" @@ -46,24 +50,13 @@ type S3ObjectStore struct { prefix string } -// NewS3ObjectStore creates a new S3ObjectStore. -// credentialsFile is the path to an AWS credentials file (INI-style). -// Pass empty string to use the default AWS credential chain (env vars, -// instance profile, etc.). The file is parsed by the AWS SDK which handles -// quoted values, profiles, session tokens, and role assumption. -func NewS3ObjectStore(bucket, prefix, region, credentialsFile string) (*S3ObjectStore, error) { +// NewS3ObjectStore creates a new S3ObjectStore from a config map. +// Supported keys: bucket, prefix, region, credentialsFile, credentialsData, +// s3Url, s3ForcePathStyle, insecureSkipTLSVerify, caCert. +func NewS3ObjectStore(configMap map[string]string) (*S3ObjectStore, error) { store := &S3ObjectStore{ - bucket: bucket, - prefix: prefix, - } - - configMap := map[string]string{ - "bucket": bucket, - "prefix": prefix, - "region": region, - } - if credentialsFile != "" { - configMap["credentialsFile"] = credentialsFile + bucket: configMap["bucket"], + prefix: configMap["prefix"], } if err := store.Init(configMap); err != nil { @@ -75,10 +68,10 @@ func NewS3ObjectStore(bucket, prefix, region, credentialsFile string) (*S3Object // Init initializes the ObjectStore with the provided config. // This implements velero.ObjectStore.Init(). -// Expected config keys: bucket, prefix, region, and optionally credentialsFile -// (path to credentials file) or credentialsData (raw credential content that -// will be written to a temp file). Credential parsing is delegated to the AWS -// SDK via config.WithSharedCredentialsFiles, matching Velero's approach. +// Expected config keys: bucket, prefix, region, credentialsFile, credentialsData, +// s3Url, s3ForcePathStyle, insecureSkipTLSVerify, caCert. +// Credential parsing is delegated to the AWS SDK via +// config.WithSharedCredentialsFiles, matching Velero's approach. func (s *S3ObjectStore) Init(configMap map[string]string) error { bucket := configMap["bucket"] prefix := configMap["prefix"] @@ -128,12 +121,41 @@ func (s *S3ObjectStore) Init(configMap map[string]string) error { }) })) + // Configure custom TLS for S3-compatible endpoints with private CAs + // or when TLS verification needs to be skipped. + insecureSkipTLSVerify := strings.EqualFold(configMap["insecureSkipTLSVerify"], "true") + caCert := configMap["caCert"] + if insecureSkipTLSVerify || caCert != "" { + httpClient, err := buildTLSHTTPClient(insecureSkipTLSVerify, caCert) + if err != nil { + return fmt.Errorf("failed to configure TLS: %w", err) + } + opts = append(opts, config.WithHTTPClient(httpClient)) + } + cfg, err := config.LoadDefaultConfig(ctx, opts...) if err != nil { return fmt.Errorf("failed to load AWS config: %w", err) } - s.client = s3.NewFromConfig(cfg) + // Apply S3-specific options: custom endpoint URL and path-style addressing. + s3URL := configMap["s3Url"] + forcePathStyle := strings.EqualFold(configMap["s3ForcePathStyle"], "true") + + if s3URL != "" { + if !isValidS3URLScheme(s3URL) { + return fmt.Errorf("invalid s3Url %q: must start with http:// or https://", s3URL) + } + } + + s.client = s3.NewFromConfig(cfg, func(o *s3.Options) { + if s3URL != "" { + o.BaseEndpoint = aws.String(s3URL) + } + if forcePathStyle { + o.UsePathStyle = true + } + }) s.uploader = manager.NewUploader(s.client) return nil @@ -306,6 +328,38 @@ func (s *S3ObjectStore) GetObjectBytes(key string) ([]byte, error) { return io.ReadAll(reader) } +// isValidS3URLScheme checks if the URL has a valid scheme (http or https). +func isValidS3URLScheme(s3URL string) bool { + u, err := url.Parse(s3URL) + if err != nil { + return false + } + return u.Scheme == "http" || u.Scheme == "https" +} + +// buildTLSHTTPClient creates an *http.Client with custom TLS settings +// for S3-compatible endpoints with private CAs or disabled TLS verification. +func buildTLSHTTPClient(insecureSkipTLSVerify bool, caCert string) (*http.Client, error) { + tlsConfig := &tls.Config{ + InsecureSkipVerify: insecureSkipTLSVerify, //nolint:gosec // User-configured via BSL for private endpoints + } + + if caCert != "" { + certPool, err := x509.SystemCertPool() + if err != nil { + certPool = x509.NewCertPool() + } + if !certPool.AppendCertsFromPEM([]byte(caCert)) { + return nil, fmt.Errorf("failed to parse CA certificate PEM") + } + tlsConfig.RootCAs = certPool + } + + tr := http.DefaultTransport.(*http.Transport).Clone() + tr.TLSClientConfig = tlsConfig + return &http.Client{Transport: tr}, nil +} + // writeCredentialsToTempFile writes credential data to a temporary file and returns // the file path. The caller is responsible for removing the file when done. // The file is created with restrictive permissions (0600) to protect credentials. @@ -330,33 +384,44 @@ func writeCredentialsToTempFile(data []byte) (string, error) { } // InitObjectStore creates an ObjectStore based on the provider type. -// Credentials are resolved to a file path for the AWS SDK's built-in parser: -// - If CredentialsData is set, it is written to a temp file. -// - If CredentialsFile is set, the path is used directly. +// Credentials and S3-compatible settings are passed through the config map +// to Init(), which handles temp file creation for in-memory credentials. func InitObjectStore(cfg *UploaderConfig) (velero.ObjectStore, error) { if cfg.BSLProvider == "" { return nil, fmt.Errorf("BSL provider is required but was empty") } - // Resolve credentials to a file path for the AWS SDK. - // Prefer in-memory data (controller path), fall back to file (uploader pod path). - credentialsFile := "" + // Build config map for the object store. + configMap := map[string]string{ + "bucket": cfg.BSLBucket, + "prefix": cfg.BSLPrefix, + "region": cfg.BSLRegion, + } + + // Credentials: prefer in-memory data (controller path), fall back to file (pod path). if len(cfg.CredentialsData) > 0 { - tmpFile, err := writeCredentialsToTempFile(cfg.CredentialsData) - if err != nil { - return nil, fmt.Errorf("failed to write credentials: %w", err) - } - defer func() { _ = os.Remove(tmpFile) }() - credentialsFile = tmpFile + configMap["credentialsData"] = string(cfg.CredentialsData) } else if cfg.CredentialsFile != "" { - credentialsFile = cfg.CredentialsFile + configMap["credentialsFile"] = cfg.CredentialsFile + } + + // S3-compatible storage settings + if cfg.BSLS3URL != "" { + configMap["s3Url"] = cfg.BSLS3URL + } + if cfg.BSLS3ForcePathStyle { + configMap["s3ForcePathStyle"] = "true" + } + if cfg.BSLInsecureSkipTLSVerify { + configMap["insecureSkipTLSVerify"] = "true" + } + if cfg.BSLCACert != "" { + configMap["caCert"] = cfg.BSLCACert } switch strings.ToLower(cfg.BSLProvider) { case "aws": - return NewS3ObjectStore( - cfg.BSLBucket, cfg.BSLPrefix, cfg.BSLRegion, credentialsFile, - ) + return NewS3ObjectStore(configMap) case "gcp": // TODO: Implement GCP Cloud Storage support (issue #11) return nil, fmt.Errorf("gcp object store not yet implemented") @@ -365,8 +430,6 @@ func InitObjectStore(cfg *UploaderConfig) (velero.ObjectStore, error) { return nil, fmt.Errorf("azure object store not yet implemented") default: // Try S3-compatible for unknown providers - return NewS3ObjectStore( - cfg.BSLBucket, cfg.BSLPrefix, cfg.BSLRegion, credentialsFile, - ) + return NewS3ObjectStore(configMap) } } diff --git a/pkg/uploader/objectstore_test.go b/pkg/uploader/objectstore_test.go index e4f1efc3..1b07b0c3 100644 --- a/pkg/uploader/objectstore_test.go +++ b/pkg/uploader/objectstore_test.go @@ -17,9 +17,18 @@ limitations under the License. package uploader import ( + "bytes" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" "os" "path/filepath" "testing" + "time" ) func TestS3ObjectStoreFullKey(t *testing.T) { @@ -345,3 +354,240 @@ func TestInitObjectStore(t *testing.T) { }) } } + +func TestIsValidS3URLScheme(t *testing.T) { + tests := []struct { + url string + want bool + }{ + {"https://minio.example.com", true}, + {"http://minio.example.com", true}, + {"https://minio.example.com:9000", true}, + {"http://10.0.0.1:9000", true}, + {"ftp://minio.example.com", false}, + {"s3://my-bucket", false}, + {"", false}, + {"not-a-url", false}, + } + + for _, tt := range tests { + t.Run(tt.url, func(t *testing.T) { + if got := isValidS3URLScheme(tt.url); got != tt.want { + t.Errorf("isValidS3URLScheme(%q) = %v, want %v", tt.url, got, tt.want) + } + }) + } +} + +func TestBuildTLSHTTPClient(t *testing.T) { + t.Run("insecure skip verify only", func(t *testing.T) { + client, err := buildTLSHTTPClient(true, "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if client == nil { + t.Fatal("expected non-nil client") + } + }) + + t.Run("valid CA cert", func(t *testing.T) { + caCert := generateTestCACertPEM(t) + client, err := buildTLSHTTPClient(false, caCert) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if client == nil { + t.Fatal("expected non-nil client") + } + }) + + t.Run("invalid CA cert PEM", func(t *testing.T) { + _, err := buildTLSHTTPClient(false, "not-a-valid-pem") + if err == nil { + t.Error("expected error for invalid PEM, got none") + } + }) + + t.Run("empty CA cert with no insecure", func(t *testing.T) { + client, err := buildTLSHTTPClient(false, "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if client == nil { + t.Fatal("expected non-nil client") + } + }) +} + +func TestInitWithS3URL(t *testing.T) { + store := &S3ObjectStore{} + err := store.Init(map[string]string{ + "bucket": "test-bucket", + "region": "us-east-1", + "s3Url": "https://minio.example.com", + }) + if err != nil { + t.Errorf("unexpected error: %v", err) + } +} + +func TestInitWithInvalidS3URLScheme(t *testing.T) { + tests := []struct { + name string + s3URL string + }{ + {"ftp scheme", "ftp://minio.example.com"}, + {"s3 scheme", "s3://my-bucket"}, + {"no scheme", "minio.example.com"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + store := &S3ObjectStore{} + err := store.Init(map[string]string{ + "bucket": "test-bucket", + "s3Url": tt.s3URL, + }) + if err == nil { + t.Error("expected error for invalid s3Url scheme, got none") + } + }) + } +} + +func TestInitWithForcePathStyle(t *testing.T) { + store := &S3ObjectStore{} + err := store.Init(map[string]string{ + "bucket": "test-bucket", + "region": "us-east-1", + "s3Url": "https://minio.example.com", + "s3ForcePathStyle": "true", + }) + if err != nil { + t.Errorf("unexpected error: %v", err) + } +} + +func TestInitWithCACert(t *testing.T) { + caCert := generateTestCACertPEM(t) + store := &S3ObjectStore{} + err := store.Init(map[string]string{ + "bucket": "test-bucket", + "region": "us-east-1", + "caCert": caCert, + }) + if err != nil { + t.Errorf("unexpected error: %v", err) + } +} + +func TestInitWithInvalidCACert(t *testing.T) { + store := &S3ObjectStore{} + err := store.Init(map[string]string{ + "bucket": "test-bucket", + "region": "us-east-1", + "caCert": "not-a-valid-pem", + }) + if err == nil { + t.Error("expected error for invalid CA cert, got none") + } +} + +func TestInitWithAllS3CompatibleOptions(t *testing.T) { + caCert := generateTestCACertPEM(t) + store := &S3ObjectStore{} + err := store.Init(map[string]string{ + "bucket": "test-bucket", + "region": "us-east-1", + "s3Url": "https://minio.example.com:9000", + "s3ForcePathStyle": "true", + "insecureSkipTLSVerify": "false", + "caCert": caCert, + }) + if err != nil { + t.Errorf("unexpected error: %v", err) + } +} + +func TestInitWithInsecureSkipTLSVerify(t *testing.T) { + store := &S3ObjectStore{} + err := store.Init(map[string]string{ + "bucket": "test-bucket", + "region": "us-east-1", + "insecureSkipTLSVerify": "true", + }) + if err != nil { + t.Errorf("unexpected error: %v", err) + } +} + +func TestInitObjectStoreWithS3Settings(t *testing.T) { + caCert := generateTestCACertPEM(t) + + cfg := &UploaderConfig{ + BSLProvider: "aws", + BSLBucket: "test-bucket", + BSLRegion: "us-east-1", + BSLS3URL: "https://minio.example.com:9000", + BSLS3ForcePathStyle: true, + BSLInsecureSkipTLSVerify: false, + BSLCACert: caCert, + } + + store, err := InitObjectStore(cfg) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if store == nil { + t.Fatal("expected non-nil store") + } +} + +func TestInitObjectStoreWithS3SettingsDefaultProvider(t *testing.T) { + // Unknown providers should fall through to S3-compatible + cfg := &UploaderConfig{ + BSLProvider: "minio", + BSLBucket: "test-bucket", + BSLRegion: "us-east-1", + BSLS3URL: "https://minio.example.com", + BSLS3ForcePathStyle: true, + } + + store, err := InitObjectStore(cfg) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if store == nil { + t.Fatal("expected non-nil store for unknown provider (S3-compatible fallback)") + } +} + +// generateTestCACertPEM creates a self-signed CA certificate PEM at runtime for testing. +func generateTestCACertPEM(t *testing.T) string { + t.Helper() + + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("failed to generate key: %v", err) + } + + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "test-ca"}, + NotBefore: time.Now(), + NotAfter: time.Now().Add(time.Hour), + IsCA: true, + BasicConstraintsValid: true, + } + + certDER, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key) + if err != nil { + t.Fatalf("failed to create certificate: %v", err) + } + + var buf bytes.Buffer + if err := pem.Encode(&buf, &pem.Block{Type: "CERTIFICATE", Bytes: certDER}); err != nil { + t.Fatalf("failed to encode PEM: %v", err) + } + return buf.String() +} diff --git a/pkg/uploader/run.go b/pkg/uploader/run.go index 1c446d95..466ea82b 100644 --- a/pkg/uploader/run.go +++ b/pkg/uploader/run.go @@ -136,21 +136,25 @@ func Run(ctx context.Context, logger logr.Logger) error { // LoadConfigFromEnv parses environment variables into UploaderConfig. func LoadConfigFromEnv() (*UploaderConfig, error) { config := &UploaderConfig{ - BSLProvider: os.Getenv(EnvBSLProvider), - BSLBucket: os.Getenv(EnvBSLBucket), - BSLPrefix: os.Getenv(EnvBSLPrefix), - BSLRegion: os.Getenv(EnvBSLRegion), - CredentialsFile: os.Getenv(EnvCredentialsFile), - VMName: os.Getenv(EnvVMName), - VMNamespace: os.Getenv(EnvVMNamespace), - CheckpointName: os.Getenv(EnvCheckpointName), - BackupType: os.Getenv(EnvBackupType), - VeleroBackupName: os.Getenv(EnvVeleroBackupName), - DataUploadName: os.Getenv(EnvDataUploadName), - DataUploadUID: os.Getenv(EnvDataUploadUID), - VMBName: os.Getenv(EnvVMBName), - VMBTName: os.Getenv(EnvVMBTName), - SourcePVCPath: os.Getenv(EnvSourcePVCPath), + BSLProvider: os.Getenv(EnvBSLProvider), + BSLBucket: os.Getenv(EnvBSLBucket), + BSLPrefix: os.Getenv(EnvBSLPrefix), + BSLRegion: os.Getenv(EnvBSLRegion), + BSLS3URL: os.Getenv(EnvBSLS3URL), + BSLS3ForcePathStyle: strings.EqualFold(os.Getenv(EnvBSLS3ForcePathStyle), "true"), + BSLInsecureSkipTLSVerify: strings.EqualFold(os.Getenv(EnvBSLInsecureSkipTLSVerify), "true"), + BSLCACert: os.Getenv(EnvBSLCACert), + CredentialsFile: os.Getenv(EnvCredentialsFile), + VMName: os.Getenv(EnvVMName), + VMNamespace: os.Getenv(EnvVMNamespace), + CheckpointName: os.Getenv(EnvCheckpointName), + BackupType: os.Getenv(EnvBackupType), + VeleroBackupName: os.Getenv(EnvVeleroBackupName), + DataUploadName: os.Getenv(EnvDataUploadName), + DataUploadUID: os.Getenv(EnvDataUploadUID), + VMBName: os.Getenv(EnvVMBName), + VMBTName: os.Getenv(EnvVMBTName), + SourcePVCPath: os.Getenv(EnvSourcePVCPath), } // Apply defaults diff --git a/pkg/uploader/run_test.go b/pkg/uploader/run_test.go index 40bd7ebb..f8003ac4 100644 --- a/pkg/uploader/run_test.go +++ b/pkg/uploader/run_test.go @@ -137,18 +137,23 @@ func TestExtractDiskName(t *testing.T) { } } +//nolint:gocyclo // Table-driven test with many validation cases func TestLoadConfigFromEnv(t *testing.T) { // Save original env and restore after test originalEnv := map[string]string{ - EnvBSLBucket: os.Getenv(EnvBSLBucket), - EnvVMName: os.Getenv(EnvVMName), - EnvVMNamespace: os.Getenv(EnvVMNamespace), - EnvBSLProvider: os.Getenv(EnvBSLProvider), - EnvBSLPrefix: os.Getenv(EnvBSLPrefix), - EnvBSLRegion: os.Getenv(EnvBSLRegion), - EnvBackupType: os.Getenv(EnvBackupType), - EnvSourcePVCPath: os.Getenv(EnvSourcePVCPath), - EnvCheckpointName: os.Getenv(EnvCheckpointName), + EnvBSLBucket: os.Getenv(EnvBSLBucket), + EnvVMName: os.Getenv(EnvVMName), + EnvVMNamespace: os.Getenv(EnvVMNamespace), + EnvBSLProvider: os.Getenv(EnvBSLProvider), + EnvBSLPrefix: os.Getenv(EnvBSLPrefix), + EnvBSLRegion: os.Getenv(EnvBSLRegion), + EnvBackupType: os.Getenv(EnvBackupType), + EnvSourcePVCPath: os.Getenv(EnvSourcePVCPath), + EnvCheckpointName: os.Getenv(EnvCheckpointName), + EnvBSLS3URL: os.Getenv(EnvBSLS3URL), + EnvBSLS3ForcePathStyle: os.Getenv(EnvBSLS3ForcePathStyle), + EnvBSLInsecureSkipTLSVerify: os.Getenv(EnvBSLInsecureSkipTLSVerify), + EnvBSLCACert: os.Getenv(EnvBSLCACert), } defer func() { for k, v := range originalEnv { @@ -279,6 +284,78 @@ func TestLoadConfigFromEnv(t *testing.T) { } }, }, + { + name: "S3-compatible settings are parsed", + envVars: map[string]string{ + EnvBSLBucket: "test-bucket", + EnvVMName: "test-vm", + EnvVMNamespace: "test-ns", + EnvCheckpointName: "cp-001", + EnvBSLS3URL: "https://minio.example.com", + EnvBSLS3ForcePathStyle: "true", + EnvBSLInsecureSkipTLSVerify: "true", + EnvBSLCACert: "-----BEGIN CERTIFICATE-----\ntest\n-----END CERTIFICATE-----", + }, + expectError: false, + validate: func(t *testing.T, cfg *UploaderConfig) { + if cfg.BSLS3URL != "https://minio.example.com" { + t.Errorf("BSLS3URL = %q, want %q", cfg.BSLS3URL, "https://minio.example.com") + } + if !cfg.BSLS3ForcePathStyle { + t.Error("BSLS3ForcePathStyle = false, want true") + } + if !cfg.BSLInsecureSkipTLSVerify { + t.Error("BSLInsecureSkipTLSVerify = false, want true") + } + if cfg.BSLCACert != "-----BEGIN CERTIFICATE-----\ntest\n-----END CERTIFICATE-----" { + t.Errorf("BSLCACert = %q, want PEM content", cfg.BSLCACert) + } + }, + }, + { + name: "S3-compatible booleans default to false when unset", + envVars: map[string]string{ + EnvBSLBucket: "test-bucket", + EnvVMName: "test-vm", + EnvVMNamespace: "test-ns", + EnvCheckpointName: "cp-001", + }, + expectError: false, + validate: func(t *testing.T, cfg *UploaderConfig) { + if cfg.BSLS3URL != "" { + t.Errorf("BSLS3URL = %q, want empty", cfg.BSLS3URL) + } + if cfg.BSLS3ForcePathStyle { + t.Error("BSLS3ForcePathStyle = true, want false") + } + if cfg.BSLInsecureSkipTLSVerify { + t.Error("BSLInsecureSkipTLSVerify = true, want false") + } + if cfg.BSLCACert != "" { + t.Errorf("BSLCACert = %q, want empty", cfg.BSLCACert) + } + }, + }, + { + name: "S3-compatible booleans reject non-true values", + envVars: map[string]string{ + EnvBSLBucket: "test-bucket", + EnvVMName: "test-vm", + EnvVMNamespace: "test-ns", + EnvCheckpointName: "cp-001", + EnvBSLS3ForcePathStyle: "false", + EnvBSLInsecureSkipTLSVerify: "yes", + }, + expectError: false, + validate: func(t *testing.T, cfg *UploaderConfig) { + if cfg.BSLS3ForcePathStyle { + t.Error("BSLS3ForcePathStyle = true for \"false\", want false") + } + if cfg.BSLInsecureSkipTLSVerify { + t.Error("BSLInsecureSkipTLSVerify = true for \"yes\", want false") + } + }, + }, } for _, tt := range tests { diff --git a/pkg/uploader/types.go b/pkg/uploader/types.go index c8f72232..4df8727b 100644 --- a/pkg/uploader/types.go +++ b/pkg/uploader/types.go @@ -94,6 +94,12 @@ const ( EnvDataUploadUID = "KUBEVIRT_DM_DATAUPLOAD_UID" EnvVMBName = "KUBEVIRT_DM_VMB_NAME" EnvVMBTName = "KUBEVIRT_DM_VMBT_NAME" + + // S3-compatible storage provider settings + EnvBSLS3URL = "KUBEVIRT_DM_BSL_S3_URL" + EnvBSLS3ForcePathStyle = "KUBEVIRT_DM_BSL_S3_FORCE_PATH_STYLE" + EnvBSLInsecureSkipTLSVerify = "KUBEVIRT_DM_BSL_INSECURE_SKIP_TLS_VERIFY" + EnvBSLCACert = "KUBEVIRT_DM_BSL_CA_CERT" ) // Default paths and values @@ -116,6 +122,12 @@ type UploaderConfig struct { BSLPrefix string BSLRegion string + // S3-compatible storage provider settings + BSLS3URL string // Custom S3 endpoint URL (e.g., "https://minio.example.com") + BSLS3ForcePathStyle bool // Use path-style URLs (required by most S3-compatible stores) + BSLInsecureSkipTLSVerify bool // Skip TLS certificate verification + BSLCACert string // PEM-encoded custom CA certificate + // CredentialsData holds raw credential content (INI-style). // Used by the controller to pass credentials from K8s Secrets directly // without writing to a temp file. Takes precedence over CredentialsFile.