Repository: kubernetes-sigs/aws-ebs-csi-driver
Pull request #2014 (2024-04-19 17:55:48)

Add CLI options to run metrics endpoint over HTTPS
**Is this a bug fix or adding new feature?**

New feature (and related bugfix)

**What is this PR about? / Why do we need it?**

_[Add options to run metrics endpoint over HTTPS](https://github.com/kubernetes-sigs/aws-ebs-csi-driver/commit/88e1b6bc4e3c5b0be93fd059b31a1e120580ee40)_

Adds the ability to run the metrics server via HTTPS by supplying a certificate and key.

_[Validate options in main.go](https://github.com/kubernetes-sigs/aws-ebs-csi-driver/commit/1ba86c8e2461bb990ef86654501c64dc02b00cc9)_

Adds missing `Validate()` call to `main.go`

**What testing is done?** 

CI/manual/Added unit tests

```
$ AWS_REGION="us-west-2" ./bin/aws-ebs-csi-driver controller --http-endpoint=":6443" --metrics-cert-file=`pwd`/server.crt --metrics-key-file=`pwd`/server.key 
I0419 18:04:46.442557   29484 metrics.go:96] "Metric server listening" address=":6443" path="/metrics"
I0419 18:04:46.444976   29484 driver.go:68] "Driver Information" Driver="ebs.csi.aws.com" Version="v1.29.1"

$ curl -kv https://localhost:6443
*   Trying 127.0.0.1:6443...
* Connected to localhost (127.0.0.1) port 6443
* ALPN: curl offers h2,http/1.1
* Cipher selection: ALL:!EXPORT:!EXPORT40:!EXPORT56:!aNULL:!LOW:!RC4:@STRENGTH
* TLSv1.2 (OUT), TLS handshake, Client hello (1):
* TLSv1.2 (IN), TLS handshake, Server hello (2):
* TLSv1.2 (IN), TLS handshake, Certificate (11):
* TLSv1.2 (IN), TLS handshake, Server key exchange (12):
* TLSv1.2 (IN), TLS handshake, Server finished (14):
* TLSv1.2 (OUT), TLS handshake, Client key exchange (16):
* TLSv1.2 (OUT), TLS change cipher, Change cipher spec (1):
* TLSv1.2 (OUT), TLS handshake, Finished (20):
* TLSv1.2 (IN), TLS change cipher, Change cipher spec (1):
* TLSv1.2 (IN), TLS handshake, Finished (20):
* SSL connection using TLSv1.2 / ECDHE-RSA-AES128-GCM-SHA256
* ALPN: server accepted h2
* Server certificate:
*  subject: C=XX; L=Default City; O=Default Company Ltd
*  start date: Apr 19 18:02:41 2024 GMT
*  expire date: Apr 17 18:02:41 2034 GMT
*  issuer: C=XX; L=Default City; O=Default Company Ltd
*  SSL certificate verify result: self signed certificate (18), continuing anyway.
* using HTTP/2
* [HTTP/2] [1] OPENED stream for https://localhost:6443/
* [HTTP/2] [1] [:method: GET]
* [HTTP/2] [1] [:scheme: https]
* [HTTP/2] [1] [:authority: localhost:6443]
* [HTTP/2] [1] [:path: /]
* [HTTP/2] [1] [user-agent: curl/8.3.0]
* [HTTP/2] [1] [accept: */*]
> GET / HTTP/2
> Host: localhost:6443
> User-Agent: curl/8.3.0
> Accept: */*
> 
< HTTP/2 404 
< content-type: text/plain; charset=utf-8
< x-content-type-options: nosniff
< content-length: 19
< date: Fri, 19 Apr 2024 18:05:05 GMT
< 
404 page not found
```


Patch:
diff --git a/cmd/main.go b/cmd/main.go
index c1c7fbd2..417ec491 100644
--- a/cmd/main.go
+++ b/cmd/main.go
@@ -89,7 +89,12 @@ func main() {
 	options.AddFlags(fs)
 
 	if err = fs.Parse(args); err != nil {
-		panic(err)
+		klog.ErrorS(err, "Failed to parse options")
+		klog.FlushAndExit(klog.ExitFlushTimeout, 0)
+	}
+	if err = options.Validate(); err != nil {
+		klog.ErrorS(err, "Invalid options")
+		klog.FlushAndExit(klog.ExitFlushTimeout, 0)
 	}
 
 	err = logsapi.ValidateAndApply(c, featureGate)
@@ -130,7 +135,7 @@ func main() {
 
 	if options.HttpEndpoint != "" {
 		r := metrics.InitializeRecorder()
-		r.InitializeMetricsHandler(options.HttpEndpoint, "/metrics")
+		r.InitializeMetricsHandler(options.HttpEndpoint, "/metrics", options.MetricsCertFile, options.MetricsKeyFile)
 	}
 
 	cfg := metadata.MetadataServiceConfig{
diff --git a/docs/options.md b/docs/options.md
index 4a7d5f20..05951f2a 100644
--- a/docs/options.md
+++ b/docs/options.md
@@ -6,6 +6,8 @@ There are a couple of driver options that can be passed as arguments when starti
 |-----------------------------|---------------------------------------------------|-----------------------------------------------------|---------------------|
 | endpoint                    | tcp://127.0.0.1:10000/                            | unix:///var/lib/csi/sockets/pluginproxy/csi.sock    | The socket on which the driver will listen for CSI RPCs|
 | http-endpoint               | :8080                                             |                                                     | The TCP network address where the HTTP server for metrics will listen (example: `:8080`). The default is empty string, which means the server is disabled.|
+| metrics-cert-file           | /metrics.crt                                      |                                                     | The path to a certificate to use for serving the metrics server over HTTPS. If the certificate is signed by a certificate authority, this file should be the concatenation of the server's certificate, any intermediates, and the CA's certificate. If this is non-empty, `--http-endpoint` and `--metrics-key-file` MUST also be non-empty.|
+| metrics-key-file            | /metrics.key                                      |                                                     | The path to a key to use for serving the metrics server over HTTPS. If this is non-empty, `--http-endpoint` and `--metrics-cert-file` MUST also be non-empty.|
 | volume-attach-limit         | 1,2,3 ...                                         | -1                                                  | Value for the maximum number of volumes attachable per node. If specified, the limit applies to all nodes. If not specified, the value is approximated from the instance type|
 | extra-tags                  | key1=value1,key2=value2                           |                                                     | Tags attached to each dynamically provisioned resource|
 | k8s-tag-cluster-id          | aws-cluster-id-1                                  |                                                     | ID of the Kubernetes cluster used for tagging provisioned EBS volumes|
diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go
index a0615593..ac6984bc 100644
--- a/pkg/cloud/cloud.go
+++ b/pkg/cloud/cloud.go
@@ -281,7 +281,6 @@ type cloud struct {
 	ec2    EC2API
 	dm     dm.DeviceManager
 	bm     *batcherManager
-	rm     *retryManager
 }
 
 var _ Cloud = &cloud{}
@@ -329,8 +328,6 @@ func newEC2Cloud(region string, awsSdkDebugLog bool, userAgentExtra string, batc
 			RecordRequestsMiddleware(),
 			RecordThrottledRequestsMiddleware(),
 		)
-
-		o.RetryMaxAttempts = retryMaxAttempt //  Retry EC2 API calls at sdk level until request contexts are cancelled
 	})
 
 	var bm *batcherManager
@@ -339,14 +336,11 @@ func newEC2Cloud(region string, awsSdkDebugLog bool, userAgentExtra string, batc
 		bm = newBatcherManager(svc)
 	}
 
-	rm := newRetryManager()
-
 	return &cloud{
 		region: region,
 		dm:     dm.NewDeviceManager(),
 		ec2:    svc,
 		bm:     bm,
-		rm:     rm,
 	}
 }
 
@@ -597,9 +591,7 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions *
 		requestInput.SnapshotId = aws.String(snapshotID)
 	}
 
-	response, err := c.ec2.CreateVolume(ctx, requestInput, func(o *ec2.Options) {
-		o.Retryer = c.rm.createVolumeRetryer
-	})
+	response, err := c.ec2.CreateVolume(ctx, requestInput)
 	if err != nil {
 		if isAWSErrorSnapshotNotFound(err) {
 			return nil, ErrNotFound
@@ -731,9 +723,7 @@ func (c *cloud) ResizeOrModifyDisk(ctx context.Context, volumeID string, newSize
 	if options.Throughput != 0 {
 		req.Throughput = aws.Int32(options.Throughput)
 	}
-	response, err := c.ec2.ModifyVolume(ctx, req, func(o *ec2.Options) {
-		o.Retryer = c.rm.modifyVolumeRetryer
-	})
+	response, err := c.ec2.ModifyVolume(ctx, req)
 	if err != nil {
 		if isAWSErrorInvalidParameter(err) {
 			// Wrap error to preserve original message from AWS as to why this was an invalid argument
@@ -755,9 +745,7 @@ func (c *cloud) ResizeOrModifyDisk(ctx context.Context, volumeID string, newSize
 
 func (c *cloud) DeleteDisk(ctx context.Context, volumeID string) (bool, error) {
 	request := &ec2.DeleteVolumeInput{VolumeId: &volumeID}
-	if _, err := c.ec2.DeleteVolume(ctx, request, func(o *ec2.Options) {
-		o.Retryer = c.rm.deleteVolumeRetryer
-	}); err != nil {
+	if _, err := c.ec2.DeleteVolume(ctx, request); err != nil {
 		if isAWSErrorVolumeNotFound(err) {
 			return false, ErrNotFound
 		}
@@ -862,9 +850,7 @@ func (c *cloud) AttachDisk(ctx context.Context, volumeID, nodeID string) (string
 			VolumeId:   aws.String(volumeID),
 		}
 
-		resp, attachErr := c.ec2.AttachVolume(ctx, request, func(o *ec2.Options) {
-			o.Retryer = c.rm.attachVolumeRetryer
-		})
+		resp, attachErr := c.ec2.AttachVolume(ctx, request)
 		if attachErr != nil {
 			if isAWSErrorBlockDeviceInUse(attachErr) {
 				cacheMutex.Lock()
@@ -937,9 +923,7 @@ func (c *cloud) DetachDisk(ctx context.Context, volumeID, nodeID string) error {
 		VolumeId:   aws.String(volumeID),
 	}
 
-	_, err = c.ec2.DetachVolume(ctx, request, func(o *ec2.Options) {
-		o.Retryer = c.rm.detachVolumeRetryer
-	})
+	_, err = c.ec2.DetachVolume(ctx, request)
 	if err != nil {
 		if isAWSErrorIncorrectState(err) ||
 			isAWSErrorInvalidAttachmentNotFound(err) ||
@@ -1241,9 +1225,7 @@ func (c *cloud) CreateSnapshot(ctx context.Context, volumeID string, snapshotOpt
 		Description:       aws.String(descriptions),
 	}
 
-	res, err := c.ec2.CreateSnapshot(ctx, request, func(o *ec2.Options) {
-		o.Retryer = c.rm.createSnapshotRetryer
-	})
+	res, err := c.ec2.CreateSnapshot(ctx, request)
 	if err != nil {
 		return nil, fmt.Errorf("error creating snapshot of volume %s: %w", volumeID, err)
 	}
@@ -1264,9 +1246,7 @@ func (c *cloud) DeleteSnapshot(ctx context.Context, snapshotID string) (success
 	request := &ec2.DeleteSnapshotInput{}
 	request.SnapshotId = aws.String(snapshotID)
 	request.DryRun = aws.Bool(false)
-	if _, err := c.ec2.DeleteSnapshot(ctx, request, func(o *ec2.Options) {
-		o.Retryer = c.rm.deleteSnapshotRetryer
-	}); err != nil {
+	if _, err := c.ec2.DeleteSnapshot(ctx, request); err != nil {
 		if isAWSErrorSnapshotNotFound(err) {
 			return false, ErrNotFound
 		}
@@ -1373,9 +1353,7 @@ func (c *cloud) EnableFastSnapshotRestores(ctx context.Context, availabilityZone
 		SourceSnapshotIds: []string{snapshotID},
 	}
 	klog.V(4).InfoS("Creating Fast Snapshot Restores", "snapshotID", snapshotID, "availabilityZones", availabilityZones)
-	response, err := c.ec2.EnableFastSnapshotRestores(ctx, request, func(o *ec2.Options) {
-		o.Retryer = c.rm.enableFastSnapshotRestoresRetryer
-	})
+	response, err := c.ec2.EnableFastSnapshotRestores(ctx, request)
 	if err != nil {
 		return nil, err
 	}
@@ -1731,9 +1709,7 @@ func (c *cloud) getLatestVolumeModification(ctx context.Context, volumeID string
 	}
 
 	if c.bm == nil || !isBatchable {
-		mod, err := c.ec2.DescribeVolumesModifications(ctx, request, func(o *ec2.Options) {
-			o.Retryer = c.rm.unbatchableDescribeVolumesModificationsRetryer
-		})
+		mod, err := c.ec2.DescribeVolumesModifications(ctx, request)
 		if err != nil {
 			if isAWSErrorModificationNotFound(err) {
 				return nil, VolumeNotBeingModified
diff --git a/pkg/cloud/retry_manager.go b/pkg/cloud/retry_manager.go
deleted file mode 100644
index 5258d38d..00000000
--- a/pkg/cloud/retry_manager.go
+++ /dev/null
@@ -1,51 +0,0 @@
-package cloud
-
-import (
-	"github.com/aws/aws-sdk-go-v2/aws"
-	"github.com/aws/aws-sdk-go-v2/aws/retry"
-)
-
-const (
-	// retryMaxAttempt sets max number of EC2 API call attempts.
-	// Set high enough to ensure default sidecar timeout will cancel context long before we stop retrying.
-	retryMaxAttempt = 50
-)
-
-// retryManager dictates the retry strategies of EC2 API calls.
-// Each mutating EC2 API has its own retryer because the AWS SDK throttles on a retryer object level, not by API name.
-// While default AWS accounts share request tokens between mutating APIs, users can raise limits for individual APIs.
-// Separate retryers ensures that throttling one API doesn't unintentionally throttle others with separate token buckets.
-type retryManager struct {
-	createVolumeRetryer                            aws.Retryer
-	deleteVolumeRetryer                            aws.Retryer
-	attachVolumeRetryer                            aws.Retryer
-	detachVolumeRetryer                            aws.Retryer
-	modifyVolumeRetryer                            aws.Retryer
-	createSnapshotRetryer                          aws.Retryer
-	deleteSnapshotRetryer                          aws.Retryer
-	enableFastSnapshotRestoresRetryer              aws.Retryer
-	unbatchableDescribeVolumesModificationsRetryer aws.Retryer
-}
-
-func newRetryManager() *retryManager {
-	return &retryManager{
-		createVolumeRetryer:                            newAdaptiveRetryer(),
-		attachVolumeRetryer:                            newAdaptiveRetryer(),
-		deleteVolumeRetryer:                            newAdaptiveRetryer(),
-		detachVolumeRetryer:                            newAdaptiveRetryer(),
-		modifyVolumeRetryer:                            newAdaptiveRetryer(),
-		createSnapshotRetryer:                          newAdaptiveRetryer(),
-		deleteSnapshotRetryer:                          newAdaptiveRetryer(),
-		enableFastSnapshotRestoresRetryer:              newAdaptiveRetryer(),
-		unbatchableDescribeVolumesModificationsRetryer: newAdaptiveRetryer(),
-	}
-}
-
-// newAdaptiveRetryer restricts attempts of API calls that recently hit throttle errors.
-func newAdaptiveRetryer() *retry.AdaptiveMode {
-	return retry.NewAdaptiveMode(func(ao *retry.AdaptiveModeOptions) {
-		ao.StandardOptions = append(ao.StandardOptions, func(so *retry.StandardOptions) {
-			so.MaxAttempts = retryMaxAttempt
-		})
-	})
-}
diff --git a/pkg/driver/options.go b/pkg/driver/options.go
index 1768b951..2a4a8165 100644
--- a/pkg/driver/options.go
+++ b/pkg/driver/options.go
@@ -34,6 +34,10 @@ type Options struct {
 	Endpoint string
 	// HttpEndpoint is the TCP network address where the HTTP server for metrics will listen
 	HttpEndpoint string
+	// MetricsCertFile is the location of the certificate for serving the metrics server over HTTPS
+	MetricsCertFile string
+	// MetricsKeyFile is the location of the key for serving the metrics server over HTTPS
+	MetricsKeyFile string
 	// EnableOtelTracing is a flag to enable opentelemetry tracing for the driver
 	EnableOtelTracing bool
 
@@ -83,6 +87,8 @@ func (o *Options) AddFlags(f *flag.FlagSet) {
 	// Server options
 	f.StringVar(&o.Endpoint, "endpoint", DefaultCSIEndpoint, "Endpoint for the CSI driver server")
 	f.StringVar(&o.HttpEndpoint, "http-endpoint", "", "The TCP network address where the HTTP server for metrics will listen (example: `:8080`). The default is empty string, which means the server is disabled.")
+	f.StringVar(&o.MetricsCertFile, "metrics-cert-file", "", "The path to a certificate to use for serving the metrics server over HTTPS. If the certificate is signed by a certificate authority, this file should be the concatenation of the server's certificate, any intermediates, and the CA's certificate. If this is non-empty, --http-endpoint and --metrics-key-file MUST also be non-empty.")
+	f.StringVar(&o.MetricsKeyFile, "metrics-key-file", "", "The path to a key to use for serving the metrics server over HTTPS. If this is non-empty, --http-endpoint and --metrics-cert-file MUST also be non-empty.")
 	f.BoolVar(&o.EnableOtelTracing, "enable-otel-tracing", false, "To enable opentelemetry tracing for the driver. The tracing is disabled by default. Configure the exporter endpoint with OTEL_EXPORTER_OTLP_ENDPOINT and other env variables, see https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/#general-sdk-configuration.")
 
 	// Controller options
@@ -104,8 +110,21 @@ func (o *Options) AddFlags(f *flag.FlagSet) {
 }
 
 func (o *Options) Validate() error {
-	if o.VolumeAttachLimit != -1 && o.ReservedVolumeAttachments != -1 {
-		return fmt.Errorf("only one of --volume-attach-limit and --reserved-volume-attachments may be specified")
+	if o.Mode == AllMode || o.Mode == NodeMode {
+		if o.VolumeAttachLimit != -1 && o.ReservedVolumeAttachments != -1 {
+			return fmt.Errorf("only one of --volume-attach-limit and --reserved-volume-attachments may be specified")
+		}
+	}
+
+	if o.MetricsCertFile != "" || o.MetricsKeyFile != "" {
+		if o.HttpEndpoint == "" {
+			return fmt.Errorf("--http-endpoint MUST be specififed when using the metrics server with HTTPS")
+		} else if o.MetricsCertFile == "" {
+			return fmt.Errorf("--metrics-cert-file MUST be specififed when using the metrics server with HTTPS")
+		} else if o.MetricsKeyFile == "" {
+			return fmt.Errorf("--metrics-key-file MUST be specififed when using the metrics server with HTTPS")
+		}
 	}
+
 	return nil
 }
diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go
index 4ad4aab7..cfa5a1d3 100644
--- a/pkg/metrics/metrics.go
+++ b/pkg/metrics/metrics.go
@@ -72,7 +72,7 @@ func (m *metricRecorder) ObserveHistogram(name string, value float64, labels map
 }
 
 // InitializeMetricsHandler starts a new HTTP server to expose the metrics.
-func (m *metricRecorder) InitializeMetricsHandler(address, path string) {
+func (m *metricRecorder) InitializeMetricsHandler(address, path, certFile, keyFile string) {
 	if m == nil {
 		klog.InfoS("InitializeMetricsHandler: metric recorder is not initialized")
 		return
@@ -92,9 +92,16 @@ func (m *metricRecorder) InitializeMetricsHandler(address, path string) {
 	}
 
 	go func() {
+		var err error
 		klog.InfoS("Metric server listening", "address", address, "path", path)
 
-		if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed {
+		if certFile != "" {
+			err = server.ListenAndServeTLS(certFile, keyFile)
+		} else {
+			err = server.ListenAndServe()
+		}
+
+		if err != nil && err != http.ErrServerClosed {
 			klog.ErrorS(err, "Failed to start metric server", "address", address, "path", path)
 			klog.FlushAndExit(klog.ExitFlushTimeout, 1)
 		}


Test patch:
diff --git a/pkg/cloud/cloud_test.go b/pkg/cloud/cloud_test.go
index 65fd1cee..48f11f42 100644
--- a/pkg/cloud/cloud_test.go
+++ b/pkg/cloud/cloud_test.go
@@ -1216,7 +1216,7 @@ func TestCreateDisk(t *testing.T) {
 			defer ctxCancel()
 
 			if tc.expCreateVolumeInput != nil {
-				mockEC2.EXPECT().CreateVolume(gomock.Any(), gomock.Any(), gomock.Any()).Return(&ec2.CreateVolumeOutput{
+				mockEC2.EXPECT().CreateVolume(gomock.Any(), gomock.Any()).Return(&ec2.CreateVolumeOutput{
 					VolumeId:   aws.String(tc.diskOptions.Tags[VolumeNameTagKey]),
 					Size:       aws.Int32(util.BytesToGiB(tc.diskOptions.CapacityBytes)),
 					OutpostArn: aws.String(tc.diskOptions.OutpostArn),
@@ -1234,7 +1234,7 @@ func TestCreateDisk(t *testing.T) {
 				}, tc.expDescVolumeErr).AnyTimes()
 				if tc.diskOptions.AvailabilityZone == "snow" {
 					mockEC2.EXPECT().CreateTags(gomock.Any(), gomock.Any()).Return(&ec2.CreateTagsOutput{}, tc.expCreateTagsErr)
-					mockEC2.EXPECT().DeleteVolume(gomock.Any(), gomock.Any(), gomock.Any()).Return(&ec2.DeleteVolumeOutput{}, nil).AnyTimes()
+					mockEC2.EXPECT().DeleteVolume(gomock.Any(), gomock.Any()).Return(&ec2.DeleteVolumeOutput{}, nil).AnyTimes()
 				}
 				if len(tc.diskOptions.SnapshotID) > 0 {
 					mockEC2.EXPECT().DescribeSnapshots(gomock.Any(), gomock.Any()).Return(&ec2.DescribeSnapshotsOutput{Snapshots: []types.Snapshot{snapshot}}, nil).AnyTimes()
@@ -1313,7 +1313,7 @@ func TestDeleteDisk(t *testing.T) {
 			c := newCloud(mockEC2)
 
 			ctx := context.Background()
-			mockEC2.EXPECT().DeleteVolume(gomock.Any(), gomock.Any(), gomock.Any()).Return(&ec2.DeleteVolumeOutput{}, tc.expErr)
+			mockEC2.EXPECT().DeleteVolume(gomock.Any(), gomock.Any()).Return(&ec2.DeleteVolumeOutput{}, tc.expErr)
 
 			ok, err := c.DeleteDisk(ctx, tc.volumeID)
 			if err != nil && tc.expErr == nil {
@@ -1362,7 +1362,7 @@ func TestAttachDisk(t *testing.T) {
 
 				gomock.InOrder(
 					mockEC2.EXPECT().DescribeInstances(gomock.Any(), gomock.Eq(instanceRequest)).Return(newDescribeInstancesOutput(nodeID), nil),
-					mockEC2.EXPECT().AttachVolume(gomock.Any(), gomock.Eq(attachRequest), gomock.Any()).Return(&ec2.AttachVolumeOutput{
+					mockEC2.EXPECT().AttachVolume(gomock.Any(), gomock.Eq(attachRequest)).Return(&ec2.AttachVolumeOutput{
 						Device:     aws.String(path),
 						InstanceId: aws.String(nodeID),
 						VolumeId:   aws.String(volumeID),
@@ -1385,7 +1385,7 @@ func TestAttachDisk(t *testing.T) {
 
 				gomock.InOrder(
 					mockEC2.EXPECT().DescribeInstances(gomock.Any(), gomock.Eq(instanceRequest)).Return(newDescribeInstancesOutput(nodeID), nil),
-					mockEC2.EXPECT().AttachVolume(gomock.Any(), gomock.Eq(attachRequest), gomock.Any()).Return(&ec2.AttachVolumeOutput{
+					mockEC2.EXPECT().AttachVolume(gomock.Any(), gomock.Eq(attachRequest)).Return(&ec2.AttachVolumeOutput{
 						Device:     aws.String(path),
 						InstanceId: aws.String(nodeID),
 						VolumeId:   aws.String(volumeID),
@@ -1435,7 +1435,7 @@ func TestAttachDisk(t *testing.T) {
 
 				gomock.InOrder(
 					mockEC2.EXPECT().DescribeInstances(gomock.Any(), instanceRequest).Return(newDescribeInstancesOutput(nodeID), nil),
-					mockEC2.EXPECT().AttachVolume(gomock.Any(), attachRequest, gomock.Any()).Return(nil, errors.New("AttachVolume error")),
+					mockEC2.EXPECT().AttachVolume(gomock.Any(), attachRequest).Return(nil, errors.New("AttachVolume error")),
 				)
 			},
 			validateFunc: func(t *testing.T) {
@@ -1454,7 +1454,7 @@ func TestAttachDisk(t *testing.T) {
 
 				gomock.InOrder(
 					mockEC2.EXPECT().DescribeInstances(ctx, instanceRequest).Return(newDescribeInstancesOutput(nodeID), nil),
-					mockEC2.EXPECT().AttachVolume(ctx, attachRequest, gomock.Any()).Return(nil, blockDeviceInUseErr),
+					mockEC2.EXPECT().AttachVolume(ctx, attachRequest).Return(nil, blockDeviceInUseErr),
 				)
 			},
 			validateFunc: func(t *testing.T) {
@@ -1500,7 +1500,7 @@ func TestAttachDisk(t *testing.T) {
 
 				gomock.InOrder(
 					mockEC2.EXPECT().DescribeInstances(ctx, gomock.Eq(instanceRequest)).Return(newDescribeInstancesOutput(nodeID), nil),
-					mockEC2.EXPECT().AttachVolume(ctx, gomock.Eq(attachRequest), gomock.Any()).Return(&ec2.AttachVolumeOutput{
+					mockEC2.EXPECT().AttachVolume(ctx, gomock.Eq(attachRequest)).Return(&ec2.AttachVolumeOutput{
 						Device:     aws.String(path),
 						InstanceId: aws.String(nodeID),
 						VolumeId:   aws.String(volumeID),
@@ -1509,7 +1509,7 @@ func TestAttachDisk(t *testing.T) {
 					mockEC2.EXPECT().DescribeVolumes(ctx, gomock.Eq(volumeRequest)).Return(createDescribeVolumesOutput([]*string{&volumeID}, nodeID, path, "attached"), nil),
 
 					mockEC2.EXPECT().DescribeInstances(ctx, gomock.Eq(createInstanceRequest2)).Return(newDescribeInstancesOutput(nodeID2), nil),
-					mockEC2.EXPECT().AttachVolume(ctx, gomock.Eq(attachRequest2), gomock.Any()).Return(&ec2.AttachVolumeOutput{
+					mockEC2.EXPECT().AttachVolume(ctx, gomock.Eq(attachRequest2)).Return(&ec2.AttachVolumeOutput{
 						Device:     aws.String(path),
 						InstanceId: aws.String(nodeID2),
 						VolumeId:   aws.String(volumeID),
@@ -1580,7 +1580,7 @@ func TestDetachDisk(t *testing.T) {
 
 				gomock.InOrder(
 					mockEC2.EXPECT().DescribeInstances(gomock.Any(), instanceRequest).Return(newDescribeInstancesOutput(nodeID), nil),
-					mockEC2.EXPECT().DetachVolume(gomock.Any(), detachRequest, gomock.Any()).Return(nil, nil),
+					mockEC2.EXPECT().DetachVolume(gomock.Any(), detachRequest).Return(nil, nil),
 					mockEC2.EXPECT().DescribeVolumes(gomock.Any(), volumeRequest).Return(createDescribeVolumesOutput([]*string{&volumeID}, nodeID, "", "detached"), nil),
 				)
 			},
@@ -1596,7 +1596,7 @@ func TestDetachDisk(t *testing.T) {
 
 				gomock.InOrder(
 					mockEC2.EXPECT().DescribeInstances(gomock.Any(), instanceRequest).Return(newDescribeInstancesOutput(nodeID), nil),
-					mockEC2.EXPECT().DetachVolume(gomock.Any(), detachRequest, gomock.Any()).Return(nil, errors.New("DetachVolume error")),
+					mockEC2.EXPECT().DetachVolume(gomock.Any(), detachRequest).Return(nil, errors.New("DetachVolume error")),
 				)
 			},
 		},
@@ -1611,7 +1611,7 @@ func TestDetachDisk(t *testing.T) {
 
 				gomock.InOrder(
 					mockEC2.EXPECT().DescribeInstances(gomock.Any(), instanceRequest).Return(newDescribeInstancesOutput(nodeID), nil),
-					mockEC2.EXPECT().DetachVolume(gomock.Any(), detachRequest, gomock.Any()).Return(nil, ErrNotFound),
+					mockEC2.EXPECT().DetachVolume(gomock.Any(), detachRequest).Return(nil, ErrNotFound),
 				)
 			},
 		},
@@ -1984,7 +1984,7 @@ func TestEnableFastSnapshotRestores(t *testing.T) {
 			c := newCloud(mockEC2)
 
 			ctx := context.Background()
-			mockEC2.EXPECT().EnableFastSnapshotRestores(gomock.Any(), gomock.Any(), gomock.Any()).Return(tc.expOutput, tc.expErr).AnyTimes()
+			mockEC2.EXPECT().EnableFastSnapshotRestores(gomock.Any(), gomock.Any()).Return(tc.expOutput, tc.expErr).AnyTimes()
 
 			response, err := c.EnableFastSnapshotRestores(ctx, tc.availabilityZones, tc.snapshotID)
 
@@ -2101,7 +2101,7 @@ func TestDeleteSnapshot(t *testing.T) {
 			c := newCloud(mockEC2)
 
 			ctx := context.Background()
-			mockEC2.EXPECT().DeleteSnapshot(gomock.Any(), gomock.Any(), gomock.Any()).Return(&ec2.DeleteSnapshotOutput{}, tc.expErr)
+			mockEC2.EXPECT().DeleteSnapshot(gomock.Any(), gomock.Any()).Return(&ec2.DeleteSnapshotOutput{}, tc.expErr)
 
 			_, err := c.DeleteSnapshot(ctx, tc.snapshotName)
 			if err != nil {
@@ -2425,13 +2425,13 @@ func TestResizeOrModifyDisk(t *testing.T) {
 				}
 			}
 			if tc.modifiedVolume != nil || tc.modifiedVolumeError != nil {
-				mockEC2.EXPECT().ModifyVolume(gomock.Any(), gomock.Any(), gomock.Any()).Return(tc.modifiedVolume, tc.modifiedVolumeError).AnyTimes()
+				mockEC2.EXPECT().ModifyVolume(gomock.Any(), gomock.Any()).Return(tc.modifiedVolume, tc.modifiedVolumeError).AnyTimes()
 			}
 			if tc.descModVolume != nil {
-				mockEC2.EXPECT().DescribeVolumesModifications(gomock.Any(), gomock.Any(), gomock.Any()).Return(tc.descModVolume, nil).AnyTimes()
+				mockEC2.EXPECT().DescribeVolumesModifications(gomock.Any(), gomock.Any()).Return(tc.descModVolume, nil).AnyTimes()
 			} else {
 				emptyOutput := &ec2.DescribeVolumesModificationsOutput{}
-				mockEC2.EXPECT().DescribeVolumesModifications(gomock.Any(), gomock.Any(), gomock.Any()).Return(emptyOutput, nil).AnyTimes()
+				mockEC2.EXPECT().DescribeVolumesModifications(gomock.Any(), gomock.Any()).Return(emptyOutput, nil).AnyTimes()
 			}
 
 			newSize, err := c.ResizeOrModifyDisk(ctx, tc.volumeID, util.GiBToBytes(tc.reqSizeGiB), tc.modifyDiskOptions)
@@ -3077,7 +3077,6 @@ func newCloud(mockEC2 EC2API) Cloud {
 		region: "test-region",
 		dm:     dm.NewDeviceManager(),
 		ec2:    mockEC2,
-		rm:     newRetryManager(),
 	}
 	return c
 }
diff --git a/pkg/driver/options_test.go b/pkg/driver/options_test.go
index c5fc111a..2c1d1c9e 100644
--- a/pkg/driver/options_test.go
+++ b/pkg/driver/options_test.go
@@ -20,6 +20,12 @@ func TestAddFlags(t *testing.T) {
 	if err := f.Set("http-endpoint", ":8080"); err != nil {
 		t.Errorf("error setting http-endpoint: %v", err)
 	}
+	if err := f.Set("metrics-cert-file", "/https.crt"); err != nil {
+		t.Errorf("error setting metrics-cert-file: %v", err)
+	}
+	if err := f.Set("metrics-key-file", "/https.key"); err != nil {
+		t.Errorf("error setting metrics-key-file: %v", err)
+	}
 	if err := f.Set("enable-otel-tracing", "true"); err != nil {
 		t.Errorf("error setting enable-otel-tracing: %v", err)
 	}
@@ -89,7 +95,7 @@ func TestAddFlags(t *testing.T) {
 	}
 }
 
-func TestValidate(t *testing.T) {
+func TestValidateAttachmentLimits(t *testing.T) {
 	tests := []struct {
 		name                string
 		volumeAttachLimit   int64
@@ -127,6 +133,7 @@ func TestValidate(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			o := &Options{
+				Mode:                      NodeMode,
 				VolumeAttachLimit:         tt.volumeAttachLimit,
 				ReservedVolumeAttachments: tt.reservedAttachments,
 			}
@@ -142,3 +149,61 @@ func TestValidate(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateMetricsHTTPS(t *testing.T) {
+	tests := []struct {
+		name            string
+		httpEndpoint    string
+		metricsCertFile string
+		metricsKeyFile  string
+		expectError     bool
+	}{
+		{
+			name: "disabled",
+		},
+		{
+			name:         "only http",
+			httpEndpoint: ":8080",
+		},
+		{
+			name:            "https with all",
+			httpEndpoint:    ":443",
+			metricsCertFile: "/https.crt",
+			metricsKeyFile:  "/https.key",
+		},
+		{
+			name:            "https with endpoint missing",
+			metricsCertFile: "/https.crt",
+			metricsKeyFile:  "/https.key",
+			expectError:     true,
+		},
+		{
+			name:           "https with cert missing",
+			httpEndpoint:   ":443",
+			metricsKeyFile: "/https.key",
+			expectError:    true,
+		},
+		{
+			name:            "https with key missing",
+			httpEndpoint:    ":443",
+			metricsCertFile: "/https.crt",
+			expectError:     true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			o := &Options{
+				Mode:            ControllerMode,
+				HttpEndpoint:    tt.httpEndpoint,
+				MetricsCertFile: tt.metricsCertFile,
+				MetricsKeyFile:  tt.metricsKeyFile,
+			}
+
+			err := o.Validate()
+			if (err != nil) != tt.expectError {
+				t.Errorf("Options.Validate() error = %v, wantErr %v", err, tt.expectError)
+			}
+		})
+	}
+}