From 41d5dfa5a76918f8f067ccf1e49f7db3f2431f70 Mon Sep 17 00:00:00 2001 From: Shubham Pampattiwar Date: Mon, 11 May 2026 11:21:59 -0700 Subject: [PATCH 1/9] feat: add S3-compatible config constants and UploaderConfig fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add environment variable constants and UploaderConfig struct fields for S3-compatible storage provider support: s3Url, s3ForcePathStyle, insecureSkipTLSVerify, and caCert. These will be wired through the config pipeline in subsequent commits. Refs: #23 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- pkg/uploader/types.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) 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. From ed406ea617615ee7e29bd8d3c474d38f1113d9f1 Mon Sep 17 00:00:00 2001 From: Shubham Pampattiwar Date: Mon, 11 May 2026 11:34:05 -0700 Subject: [PATCH 2/9] feat: plumb S3-compatible config through pod builder and env loading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add S3-compatible storage fields to DatamoverPodConfig and wire them as environment variables in the datamover pod. Update LoadConfigFromEnv() to parse the new env vars, including boolean parsing for s3ForcePathStyle and insecureSkipTLSVerify. Refs: #23 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- internal/controller/pod_builder.go | 10 ++++ internal/controller/pod_builder_test.go | 72 +++++++++++++----------- pkg/uploader/run.go | 4 ++ pkg/uploader/run_test.go | 74 ++++++++++++++++++++++--- 4 files changed, 119 insertions(+), 41 deletions(-) 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..6661bc38 100644 --- a/internal/controller/pod_builder_test.go +++ b/internal/controller/pod_builder_test.go @@ -103,24 +103,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 +134,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 { diff --git a/pkg/uploader/run.go b/pkg/uploader/run.go index 1c446d95..0ea0612a 100644 --- a/pkg/uploader/run.go +++ b/pkg/uploader/run.go @@ -140,6 +140,10 @@ func LoadConfigFromEnv() (*UploaderConfig, error) { 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), diff --git a/pkg/uploader/run_test.go b/pkg/uploader/run_test.go index 40bd7ebb..6092b13f 100644 --- a/pkg/uploader/run_test.go +++ b/pkg/uploader/run_test.go @@ -140,15 +140,19 @@ func TestExtractDiskName(t *testing.T) { 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 +283,58 @@ 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) + } + }, + }, } for _, tt := range tests { From de38fa32b9548891cec07fcb205ade81e94b823f Mon Sep 17 00:00:00 2001 From: Shubham Pampattiwar Date: Mon, 11 May 2026 11:40:23 -0700 Subject: [PATCH 3/9] feat: extract S3-compatible keys from BSL config map MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Read s3Url, s3ForcePathStyle, insecureSkipTLSVerify, and caCert from bsl.Spec.Config in extractBSLConfig(). Propagate the new fields through buildDatamoverPodConfig() and initObjectStoreFromBSL() so both the datamover pod and controller-side object store receive the full config. Refs: #23 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../kubevirt_dataupload_controller.go | 60 ++++++++--- .../kubevirt_dataupload_controller_test.go | 101 ++++++++++++++++++ 2 files changed, 144 insertions(+), 17 deletions(-) diff --git a/internal/controller/kubevirt_dataupload_controller.go b/internal/controller/kubevirt_dataupload_controller.go index bcb32ab8..82d3f764 100644 --- a/internal/controller/kubevirt_dataupload_controller.go +++ b/internal/controller/kubevirt_dataupload_controller.go @@ -1589,12 +1589,16 @@ func (r *KubeVirtDataUploadReconciler) buildDatamoverPodConfig( 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, + 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, @@ -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 { From 880a9e14a25f0f282910295e253d1117ad164124 Mon Sep 17 00:00:00 2001 From: Shubham Pampattiwar Date: Mon, 11 May 2026 11:44:23 -0700 Subject: [PATCH 4/9] refactor: accept config map in NewS3ObjectStore for extensibility MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change NewS3ObjectStore from individual parameters to a config map, removing the intermediate map construction. Simplify InitObjectStore by passing credentialsData and S3-compatible settings through the map, letting Init() handle temp file creation in one place. Refs: #23 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- pkg/uploader/objectstore.go | 70 ++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 36 deletions(-) diff --git a/pkg/uploader/objectstore.go b/pkg/uploader/objectstore.go index 0111762b..0d6139e1 100644 --- a/pkg/uploader/objectstore.go +++ b/pkg/uploader/objectstore.go @@ -46,24 +46,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 { @@ -330,33 +319,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 +365,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) } } From 8c681b0b9362f6227e982ea567d65db927434e05 Mon Sep 17 00:00:00 2001 From: Shubham Pampattiwar Date: Mon, 11 May 2026 11:51:50 -0700 Subject: [PATCH 5/9] feat: apply s3Url, s3ForcePathStyle, caCert, insecureSkipTLSVerify in S3 client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wire the S3-compatible BSL config keys into the S3 client creation: - s3Url sets BaseEndpoint for custom S3 endpoints (MinIO, NooBaa, Ceph) - s3ForcePathStyle enables UsePathStyle addressing - caCert configures custom CA certificate for private endpoints - insecureSkipTLSVerify skips TLS certificate verification URL scheme is validated (http/https only) and invalid CA PEM is rejected with a clear error. TLS configuration follows the same approach as Velero's AWS plugin. Fixes: #23 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- pkg/uploader/objectstore.go | 77 +++++++++++- pkg/uploader/objectstore_test.go | 193 +++++++++++++++++++++++++++++++ 2 files changed, 265 insertions(+), 5 deletions(-) diff --git a/pkg/uploader/objectstore.go b/pkg/uploader/objectstore.go index 0d6139e1..12890e16 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" @@ -64,10 +68,10 @@ func NewS3ObjectStore(configMap map[string]string) (*S3ObjectStore, error) { // 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"] @@ -117,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 @@ -295,6 +328,40 @@ 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 + } + + return &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: tlsConfig, + }, + }, 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. diff --git a/pkg/uploader/objectstore_test.go b/pkg/uploader/objectstore_test.go index e4f1efc3..2ef2335f 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,187 @@ 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) + } +} + +// 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() +} From 8dca52bac3a463a46a6b7987312087bf0285638c Mon Sep 17 00:00:00 2001 From: Shubham Pampattiwar Date: Tue, 12 May 2026 14:41:08 -0700 Subject: [PATCH 6/9] test: add coverage for S3-compatible config gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add tests for coverage gaps identified during review: - InitObjectStore with S3 settings (configMap building) - InitObjectStore with unknown provider (S3-compatible fallback) - Init with insecureSkipTLSVerify alone - Boolean parsing edge cases ("false", "yes" must not parse as true) - strconv.FormatBool roundtrip (controller → env var → uploader) Refs: #23 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- internal/controller/pod_builder_test.go | 80 +++++++++++++++++++++++++ pkg/uploader/objectstore_test.go | 53 ++++++++++++++++ pkg/uploader/run_test.go | 20 +++++++ 3 files changed, 153 insertions(+) diff --git a/internal/controller/pod_builder_test.go b/internal/controller/pod_builder_test.go index 6661bc38..158bb1af 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" @@ -342,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_test.go b/pkg/uploader/objectstore_test.go index 2ef2335f..1b07b0c3 100644 --- a/pkg/uploader/objectstore_test.go +++ b/pkg/uploader/objectstore_test.go @@ -509,6 +509,59 @@ func TestInitWithAllS3CompatibleOptions(t *testing.T) { } } +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() diff --git a/pkg/uploader/run_test.go b/pkg/uploader/run_test.go index 6092b13f..49932be5 100644 --- a/pkg/uploader/run_test.go +++ b/pkg/uploader/run_test.go @@ -335,6 +335,26 @@ func TestLoadConfigFromEnv(t *testing.T) { } }, }, + { + 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 { From 7a8a92ff37d8f7d4ed7b02fb0111a069959e9fb4 Mon Sep 17 00:00:00 2001 From: Shubham Pampattiwar Date: Wed, 13 May 2026 10:58:14 -0700 Subject: [PATCH 7/9] fix: clone default HTTP transport to preserve connection pool settings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use http.DefaultTransport.Clone() instead of a bare http.Transport{} when building the custom TLS client. This preserves Go's default connection pooling, timeouts, keep-alive, and proxy settings. Refs: #23 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- pkg/uploader/objectstore.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/pkg/uploader/objectstore.go b/pkg/uploader/objectstore.go index 12890e16..9dada0e7 100644 --- a/pkg/uploader/objectstore.go +++ b/pkg/uploader/objectstore.go @@ -355,11 +355,9 @@ func buildTLSHTTPClient(insecureSkipTLSVerify bool, caCert string) (*http.Client tlsConfig.RootCAs = certPool } - return &http.Client{ - Transport: &http.Transport{ - TLSClientConfig: tlsConfig, - }, - }, nil + 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 From 2f3350464474ccc3b505072d3558d7a543f7180e Mon Sep 17 00:00:00 2001 From: Shubham Pampattiwar Date: Wed, 13 May 2026 11:02:03 -0700 Subject: [PATCH 8/9] style: align struct field values in LoadConfigFromEnv MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refs: #23 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- pkg/uploader/run.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/uploader/run.go b/pkg/uploader/run.go index 0ea0612a..86a58495 100644 --- a/pkg/uploader/run.go +++ b/pkg/uploader/run.go @@ -140,10 +140,10 @@ func LoadConfigFromEnv() (*UploaderConfig, error) { BSLBucket: os.Getenv(EnvBSLBucket), BSLPrefix: os.Getenv(EnvBSLPrefix), BSLRegion: os.Getenv(EnvBSLRegion), - BSLS3URL: os.Getenv(EnvBSLS3URL), + BSLS3URL: os.Getenv(EnvBSLS3URL), BSLS3ForcePathStyle: strings.EqualFold(os.Getenv(EnvBSLS3ForcePathStyle), "true"), BSLInsecureSkipTLSVerify: strings.EqualFold(os.Getenv(EnvBSLInsecureSkipTLSVerify), "true"), - BSLCACert: os.Getenv(EnvBSLCACert), + BSLCACert: os.Getenv(EnvBSLCACert), CredentialsFile: os.Getenv(EnvCredentialsFile), VMName: os.Getenv(EnvVMName), VMNamespace: os.Getenv(EnvVMNamespace), From b3d4a9e425238270a30726423b7e0d593091bdeb Mon Sep 17 00:00:00 2001 From: Shubham Pampattiwar Date: Wed, 13 May 2026 11:07:46 -0700 Subject: [PATCH 9/9] fix: resolve linter failures (gofmt, gocyclo) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Run gofmt on files with alignment issues and add nolint:gocyclo directive to TestLoadConfigFromEnv (complexity 31, threshold 30) matching the existing pattern in TestBuildDatamoverPod. Refs: #23 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../kubevirt_dataupload_controller.go | 30 ++++++++--------- internal/controller/pod_builder_test.go | 32 +++++++++---------- pkg/uploader/run.go | 30 ++++++++--------- pkg/uploader/run_test.go | 1 + 4 files changed, 47 insertions(+), 46 deletions(-) diff --git a/internal/controller/kubevirt_dataupload_controller.go b/internal/controller/kubevirt_dataupload_controller.go index 82d3f764..792dec8d 100644 --- a/internal/controller/kubevirt_dataupload_controller.go +++ b/internal/controller/kubevirt_dataupload_controller.go @@ -1585,10 +1585,10 @@ func (r *KubeVirtDataUploadReconciler) buildDatamoverPodConfig( } return &DatamoverPodConfig{ - Name: du.Name, // Used as a prefix for GenerateName - Namespace: vmRef.Namespace, - Image: image, - ImagePullPolicy: pullPolicy, + 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, @@ -1599,17 +1599,17 @@ func (r *KubeVirtDataUploadReconciler) buildDatamoverPodConfig( 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), + 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 } diff --git a/internal/controller/pod_builder_test.go b/internal/controller/pod_builder_test.go index 158bb1af..05dfd7e2 100644 --- a/internal/controller/pod_builder_test.go +++ b/internal/controller/pod_builder_test.go @@ -350,32 +350,32 @@ func TestDatamoverPodConfigDefaults(t *testing.T) { // insensitive) should produce true; "false" must not. func TestS3CompatibleBooleanRoundtrip(t *testing.T) { tests := []struct { - name string - s3ForcePathStyle string + name string + s3ForcePathStyle string insecureSkipTLSVerify string - wantForcePathStyle bool - wantInsecureSkip bool + wantForcePathStyle bool + wantInsecureSkip bool }{ { - name: "both true", - s3ForcePathStyle: "true", + name: "both true", + s3ForcePathStyle: "true", insecureSkipTLSVerify: "true", - wantForcePathStyle: true, - wantInsecureSkip: true, + wantForcePathStyle: true, + wantInsecureSkip: true, }, { - name: "both false (from strconv.FormatBool)", - s3ForcePathStyle: "false", + name: "both false (from strconv.FormatBool)", + s3ForcePathStyle: "false", insecureSkipTLSVerify: "false", - wantForcePathStyle: false, - wantInsecureSkip: false, + wantForcePathStyle: false, + wantInsecureSkip: false, }, { - name: "both empty", - s3ForcePathStyle: "", + name: "both empty", + s3ForcePathStyle: "", insecureSkipTLSVerify: "", - wantForcePathStyle: false, - wantInsecureSkip: false, + wantForcePathStyle: false, + wantInsecureSkip: false, }, } diff --git a/pkg/uploader/run.go b/pkg/uploader/run.go index 86a58495..466ea82b 100644 --- a/pkg/uploader/run.go +++ b/pkg/uploader/run.go @@ -136,25 +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), + 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), + 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 49932be5..f8003ac4 100644 --- a/pkg/uploader/run_test.go +++ b/pkg/uploader/run_test.go @@ -137,6 +137,7 @@ 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{