From a3199f4b9e652ce08853c90b1cd4d4c28f889da7 Mon Sep 17 00:00:00 2001 From: sarthakkothari <32287416+sarthakkothari@users.noreply.github.com> Date: Fri, 16 Aug 2024 10:43:15 -0700 Subject: [PATCH 1/3] Adding RoleArn option for the driver --- cmd/main.go | 2 +- docs/options.md | 1 + pkg/cloud/cloud.go | 24 ++++++++++++++++++++---- pkg/cloud/cloud_test.go | 3 +-- pkg/driver/options.go | 4 ++++ pkg/driver/options_test.go | 6 ++++++ tests/e2e/dynamic_provisioning.go | 2 +- tests/e2e/pre_provsioning.go | 4 ++-- 8 files changed, 36 insertions(+), 10 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 1428a69dc1..a694b3dc3a 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -168,7 +168,7 @@ func main() { region = md.GetRegion() } - cloud, err := cloud.NewCloud(region, options.AwsSdkDebugLog, options.UserAgentExtra, options.Batching) + cloud, err := cloud.NewCloud(region, options.AwsSdkDebugLog, options.UserAgentExtra, options.Batching, options.RoleARN) if err != nil { klog.ErrorS(err, "failed to create cloud service") klog.FlushAndExit(klog.ExitFlushTimeout, 1) diff --git a/docs/options.md b/docs/options.md index 05951f2ab5..e9b9ae37e5 100644 --- a/docs/options.md +++ b/docs/options.md @@ -19,3 +19,4 @@ There are a couple of driver options that can be passed as arguments when starti | modify-volume-request-handler-timeout | 10s | 2s | Timeout for the window in which volume modification calls must be received in order for them to coalesce into a single volume modification call to AWS. If changing this, be aware that the ebs-csi-controller's csi-resizer and volumemodifier containers both have timeouts on the calls they make, if this value exceeds those timeouts it will cause them to always fail and fall into a retry loop, so adjust those values accordingly. | warn-on-invalid-tag | true | false | To warn on invalid tags, instead of returning an error| |reserved-volume-attachments | 2 | -1 | Number of volume attachments reserved for system use. Not used when --volume-attach-limit is specified. When -1, the amount of reserved attachments is loaded from instance metadata that captured state at node boot and may include not only system disks but also CSI volumes.| +| role-arn | arn:aws:iam::012345678910:role/ExampleRole | | The Role used to interact with AWS EC2 APIs | diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go index 4e946008c8..e1e49d67df 100644 --- a/pkg/cloud/cloud.go +++ b/pkg/cloud/cloud.go @@ -30,8 +30,10 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/config" + "github.com/aws/aws-sdk-go-v2/credentials/stscreds" "github.com/aws/aws-sdk-go-v2/service/ec2" "github.com/aws/aws-sdk-go-v2/service/ec2/types" + "github.com/aws/aws-sdk-go-v2/service/sts" "github.com/aws/smithy-go" "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/batcher" dm "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/cloud/devicemanager" @@ -94,6 +96,8 @@ const ( volumeDetachedState = "detached" volumeAttachedState = "attached" cacheForgetDelay = 1 * time.Hour + + assumeRoleSessionDuration = 1 * time.Hour ) // AWS provisioning limits. @@ -333,12 +337,12 @@ var _ Cloud = &cloud{} // NewCloud returns a new instance of AWS cloud // It panics if session is invalid -func NewCloud(region string, awsSdkDebugLog bool, userAgentExtra string, batching bool) (Cloud, error) { - c := newEC2Cloud(region, awsSdkDebugLog, userAgentExtra, batching) +func NewCloud(region string, awsSdkDebugLog bool, userAgentExtra string, batching bool, roleARN string) (Cloud, error) { + c := newEC2Cloud(region, awsSdkDebugLog, userAgentExtra, batching, roleARN) return c, nil } -func newEC2Cloud(region string, awsSdkDebugLog bool, userAgentExtra string, batchingEnabled bool) Cloud { +func newEC2Cloud(region string, awsSdkDebugLog bool, userAgentExtra string, batchingEnabled bool, roleARN string) Cloud { cfg, err := config.LoadDefaultConfig(context.Background(), config.WithRegion(region)) if err != nil { panic(err) @@ -355,7 +359,19 @@ func newEC2Cloud(region string, awsSdkDebugLog bool, userAgentExtra string, batc os.Setenv("AWS_EXECUTION_ENV", "aws-ebs-csi-driver-"+driverVersion) } - svc := ec2.NewFromConfig(cfg, func(o *ec2.Options) { + ec2Config := cfg + if roleARN != "" { + creds := stscreds.NewAssumeRoleProvider(sts.NewFromConfig(cfg), roleARN, func(aro *stscreds.AssumeRoleOptions) { + aro.Duration = assumeRoleSessionDuration + }) + ec2Config = aws.Config{ + Region: cfg.Region, + DefaultsMode: aws.DefaultsModeStandard, + Credentials: aws.NewCredentialsCache(creds), + } + } + + svc := ec2.NewFromConfig(ec2Config, func(o *ec2.Options) { o.APIOptions = append(o.APIOptions, RecordRequestsMiddleware(), ) diff --git a/pkg/cloud/cloud_test.go b/pkg/cloud/cloud_test.go index f7acef7b61..1c3396b577 100644 --- a/pkg/cloud/cloud_test.go +++ b/pkg/cloud/cloud_test.go @@ -83,7 +83,6 @@ func extractVolumeIdentifiers(volumes []types.Volume) (volumeIDs []string, volum return volumeIDs, volumeNames } func TestNewCloud(t *testing.T) { - testCases := []struct { name string region string @@ -110,7 +109,7 @@ func TestNewCloud(t *testing.T) { }, } for _, tc := range testCases { - ec2Cloud, err := NewCloud(tc.region, tc.awsSdkDebugLog, tc.userAgentExtra, tc.batchingEnabled) + ec2Cloud, err := NewCloud(tc.region, tc.awsSdkDebugLog, tc.userAgentExtra, tc.batchingEnabled, "") if err != nil { t.Fatalf("error %v", err) } diff --git a/pkg/driver/options.go b/pkg/driver/options.go index 5dc13c4b43..478b901f24 100644 --- a/pkg/driver/options.go +++ b/pkg/driver/options.go @@ -32,6 +32,9 @@ type Options struct { // If empty, the in-cluster config will be loaded. Kubeconfig string + //RoleArn is the role driver will use to interact with the AWS EC2 APIs. + RoleARN string + // #### Server options #### //Endpoint is the endpoint for the CSI driver server @@ -91,6 +94,7 @@ type Options struct { func (o *Options) AddFlags(f *flag.FlagSet) { f.StringVar(&o.Kubeconfig, "kubeconfig", "", "Absolute path to a kubeconfig file. The default is the emtpy string, which causes the in-cluster config to be used") + f.StringVar(&o.RoleARN, "role-arn", "", "Arn of the role to be used while interacting with EC2 APIs. The default is the empty string, which causes the role provided by the Pod identity or OIDC to be used.") // Server options f.StringVar(&o.Endpoint, "endpoint", DefaultCSIEndpoint, "Endpoint for the CSI driver server") diff --git a/pkg/driver/options_test.go b/pkg/driver/options_test.go index 21b4fe4a75..c8f3510ea2 100644 --- a/pkg/driver/options_test.go +++ b/pkg/driver/options_test.go @@ -70,6 +70,9 @@ func TestAddFlags(t *testing.T) { if err := f.Set("reserved-volume-attachments", "5"); err != nil { t.Errorf("error setting reserved-volume-attachments: %v", err) } + if err := f.Set("role-arn", "arn:aws:iam::012345678910:role/ExampleRole"); err != nil { + t.Errorf("error setting role-arn: %v", err) + } if o.Endpoint != "custom-endpoint" { t.Errorf("unexpected Endpoint: got %s, want custom-endpoint", o.Endpoint) @@ -107,6 +110,9 @@ func TestAddFlags(t *testing.T) { if o.ReservedVolumeAttachments != 5 { t.Errorf("unexpected ReservedVolumeAttachments: got %d, want 5", o.ReservedVolumeAttachments) } + if o.RoleARN != "arn:aws:iam::012345678910:role/ExampleRole" { + t.Errorf("unexpected role-arn: got %d, want arn:aws:iam::012345678910:role/ExampleRole", o.RoleARN) + } } func TestValidateAttachmentLimits(t *testing.T) { diff --git a/tests/e2e/dynamic_provisioning.go b/tests/e2e/dynamic_provisioning.go index c39de5b51c..0b4b3e41e7 100644 --- a/tests/e2e/dynamic_provisioning.go +++ b/tests/e2e/dynamic_provisioning.go @@ -623,7 +623,7 @@ var _ = Describe("[ebs-csi-e2e] [single-az] Dynamic Provisioning", func() { availabilityZones := strings.Split(os.Getenv(awsAvailabilityZonesEnv), ",") availabilityZone := availabilityZones[rand.Intn(len(availabilityZones))] region := availabilityZone[0 : len(availabilityZone)-1] - cloud, err := awscloud.NewCloud(region, false, "", true) + cloud, err := awscloud.NewCloud(region, false, "", true, "") if err != nil { Fail(fmt.Sprintf("could not get NewCloud: %v", err)) } diff --git a/tests/e2e/pre_provsioning.go b/tests/e2e/pre_provsioning.go index cd77e6e6ca..a7b70bdf3e 100644 --- a/tests/e2e/pre_provsioning.go +++ b/tests/e2e/pre_provsioning.go @@ -88,7 +88,7 @@ var _ = Describe("[ebs-csi-e2e] [single-az] Pre-Provisioned", func() { Tags: map[string]string{awscloud.VolumeNameTagKey: dummyVolumeName, awscloud.AwsEbsDriverTagKey: "true"}, } var err error - cloud, err = awscloud.NewCloud(region, false, "", true) + cloud, err = awscloud.NewCloud(region, false, "", true, "") if err != nil { Fail(fmt.Sprintf("could not get NewCloud: %v", err)) } @@ -261,7 +261,7 @@ var _ = Describe("[ebs-csi-e2e] [single-az] Pre-Provisioned with Multi-Attach", Tags: map[string]string{awscloud.VolumeNameTagKey: dummyVolumeName, awscloud.AwsEbsDriverTagKey: "true"}, } var err error - cloud, err = awscloud.NewCloud(region, false, "", true) + cloud, err = awscloud.NewCloud(region, false, "", true, "") if err != nil { Fail(fmt.Sprintf("could not get NewCloud: %v", err)) } From 8b8f5fb9ec3d893b057d088d24a555ed551fbdac Mon Sep 17 00:00:00 2001 From: Sarthak Kothari Date: Fri, 16 Aug 2024 18:57:07 +0000 Subject: [PATCH 2/3] fix error --- pkg/driver/options_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/driver/options_test.go b/pkg/driver/options_test.go index c8f3510ea2..87a1477d15 100644 --- a/pkg/driver/options_test.go +++ b/pkg/driver/options_test.go @@ -111,7 +111,7 @@ func TestAddFlags(t *testing.T) { t.Errorf("unexpected ReservedVolumeAttachments: got %d, want 5", o.ReservedVolumeAttachments) } if o.RoleARN != "arn:aws:iam::012345678910:role/ExampleRole" { - t.Errorf("unexpected role-arn: got %d, want arn:aws:iam::012345678910:role/ExampleRole", o.RoleARN) + t.Errorf("unexpected role-arn: got %s, want arn:aws:iam::012345678910:role/ExampleRole", o.RoleARN) } } From 318d065d200257009966615da9585d88db1e2698 Mon Sep 17 00:00:00 2001 From: Sarthak Kothari Date: Fri, 16 Aug 2024 20:05:10 +0000 Subject: [PATCH 3/3] Add unit-test --- go.mod | 4 ++-- pkg/cloud/cloud_test.go | 14 +++++++++++++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 5d64269cf0..e410e532af 100644 --- a/go.mod +++ b/go.mod @@ -3,8 +3,10 @@ module github.com/kubernetes-sigs/aws-ebs-csi-driver require ( github.com/aws/aws-sdk-go-v2 v1.30.3 github.com/aws/aws-sdk-go-v2/config v1.27.27 + github.com/aws/aws-sdk-go-v2/credentials v1.17.27 github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.11 github.com/aws/aws-sdk-go-v2/service/ec2 v1.171.0 + github.com/aws/aws-sdk-go-v2/service/sts v1.30.3 github.com/aws/smithy-go v1.20.3 github.com/awslabs/volume-modifier-for-k8s v0.3.1 github.com/container-storage-interface/spec v1.10.0 @@ -43,7 +45,6 @@ require ( github.com/NYTimes/gziphandler v1.1.1 // indirect github.com/antlr/antlr4/runtime/Go/antlr/v4 v4.0.0-20230305170008-8188dc5388df // indirect github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 // indirect - github.com/aws/aws-sdk-go-v2/credentials v1.17.27 // indirect github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.15 // indirect github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.15 // indirect github.com/aws/aws-sdk-go-v2/internal/ini v1.8.0 // indirect @@ -51,7 +52,6 @@ require ( github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.11.17 // indirect github.com/aws/aws-sdk-go-v2/service/sso v1.22.4 // indirect github.com/aws/aws-sdk-go-v2/service/ssooidc v1.26.4 // indirect - github.com/aws/aws-sdk-go-v2/service/sts v1.30.3 // indirect github.com/awslabs/operatorpkg v0.0.0-20240715175203-3fe9588ed7de // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/blang/semver/v4 v4.0.0 // indirect diff --git a/pkg/cloud/cloud_test.go b/pkg/cloud/cloud_test.go index 1c3396b577..80b93f7bf4 100644 --- a/pkg/cloud/cloud_test.go +++ b/pkg/cloud/cloud_test.go @@ -82,10 +82,12 @@ func extractVolumeIdentifiers(volumes []types.Volume) (volumeIDs []string, volum } return volumeIDs, volumeNames } + func TestNewCloud(t *testing.T) { testCases := []struct { name string region string + roleARN string awsSdkDebugLog bool userAgentExtra string batchingEnabled bool @@ -107,9 +109,14 @@ func TestNewCloud(t *testing.T) { name: "success: with only region", region: "us-east-1", }, + { + name: "success: with role & region", + region: "us-east-1", + roleARN: "arn:aws:iam::012345678910:role/ExampleRole", + }, } for _, tc := range testCases { - ec2Cloud, err := NewCloud(tc.region, tc.awsSdkDebugLog, tc.userAgentExtra, tc.batchingEnabled, "") + ec2Cloud, err := NewCloud(tc.region, tc.awsSdkDebugLog, tc.userAgentExtra, tc.batchingEnabled, tc.roleARN) if err != nil { t.Fatalf("error %v", err) } @@ -120,6 +127,11 @@ func TestNewCloud(t *testing.T) { } else { assert.Nil(t, ec2CloudAscloud.bm) } + if tc.roleARN != "" { + ec2, ok := ec2CloudAscloud.ec2.(*ec2.Client) + assert.True(t, ok) + assert.IsType(t, &aws.CredentialsCache{}, ec2.Options().Credentials) + } } } func TestBatchDescribeVolumes(t *testing.T) {