Skip to content

Commit e723e72

Browse files
committed
Incorporate feedback from review
Signed-off-by: Renan Magagnin <renanmag@amazon.co.uk>
1 parent 5443228 commit e723e72

7 files changed

Lines changed: 44 additions & 40 deletions

File tree

examples/kubernetes/static_provisioning/static_provisioning.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ spec:
1414
mountOptions:
1515
- allow-delete
1616
- region us-west-2
17+
- prefix some-s3-prefix/
1718
csi:
1819
driver: s3.csi.aws.com # Required
1920
volumeHandle: s3-csi-driver-volume # Must be unique

pkg/driver/node/credentialprovider/provider_driver.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ func (c *Provider) provideFromDriver(provideCtx ProvideContext) (envprovider.Env
2727
accessKeyID := os.Getenv(envprovider.EnvAccessKeyID)
2828
secretAccessKey := os.Getenv(envprovider.EnvSecretAccessKey)
2929
if accessKeyID != "" && secretAccessKey != "" {
30+
klog.V(4).Infof("Providing credentials from driver with Long-term AWS credentials")
3031
sessionToken := os.Getenv(envprovider.EnvSessionToken)
3132
longTermCredsEnv, err := provideLongTermCredentialsFromDriver(provideCtx, accessKeyID, secretAccessKey, sessionToken)
3233
if err != nil {
@@ -38,6 +39,7 @@ func (c *Provider) provideFromDriver(provideCtx ProvideContext) (envprovider.Env
3839
} else {
3940
// Profile provider
4041
// TODO: This is not officially supported and won't work by default with containerization.
42+
klog.V(4).Infof("Providing credentials from driver with Profile provider")
4143
configFile := os.Getenv(envprovider.EnvConfigFile)
4244
sharedCredentialsFile := os.Getenv(envprovider.EnvSharedCredentialsFile)
4345
if configFile != "" && sharedCredentialsFile != "" {
@@ -50,7 +52,7 @@ func (c *Provider) provideFromDriver(provideCtx ProvideContext) (envprovider.Env
5052
webIdentityTokenFile := os.Getenv(envprovider.EnvWebIdentityTokenFile)
5153
roleARN := os.Getenv(envprovider.EnvRoleARN)
5254
if webIdentityTokenFile != "" && roleARN != "" {
53-
klog.Info("STS Web Identity provider (IRSA)")
55+
klog.V(4).Infof("Providing credentials from driver with STS Web Identity provider (IRSA)")
5456
stsWebIdentityCredsEnv, err := provideStsWebIdentityCredentialsFromDriver(provideCtx)
5557
if err != nil {
5658
klog.V(4).ErrorS(err, "credentialprovider: Failed to provide STS Web Identity credentials from driver")
@@ -63,8 +65,8 @@ func (c *Provider) provideFromDriver(provideCtx ProvideContext) (envprovider.Env
6365
// Container credential provider (EKS Pod Identity)
6466
containerAuthorizationTokenFile := os.Getenv(envprovider.EnvContainerAuthorizationTokenFile)
6567
containerCredentialsFullURI := os.Getenv(envprovider.EnvContainerCredentialsFullURI)
66-
if containerAuthorizationTokenFile != "" && containerCredentialsFullURI != "" {
67-
klog.Info("Container credential provider (EKS Pod Identity)")
68+
if util.UsePodMounter() && containerAuthorizationTokenFile != "" && containerCredentialsFullURI != "" {
69+
klog.V(4).Infof("Providing credentials from driver with Container credential provider (EKS Pod Identity)")
6870
containerCredsEnv, err := provideContainerCredentialsFromDriver(provideCtx, containerAuthorizationTokenFile, containerCredentialsFullURI)
6971
if err != nil {
7072
klog.V(4).ErrorS(err, "credentialprovider: Failed to provide container credentials from driver")

pkg/driver/node/credentialprovider/provider_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -953,6 +953,8 @@ func setEnvForStsWebIdentityCredentials(t *testing.T) {
953953
func setEnvForContainerCredentials(t *testing.T) {
954954
t.Helper()
955955

956+
t.Setenv("MOUNTER_KIND", "pod")
957+
956958
tokenPath := filepath.Join(t.TempDir(), "token")
957959
assert.NoError(t, os.WriteFile(tokenPath, []byte(testContainerAuthorizationToken), 0600))
958960

tests/e2e-kubernetes/go.mod

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ require (
2323
github.com/NYTimes/gziphandler v1.1.1 // indirect
2424
github.com/antlr/antlr4/runtime/Go/antlr/v4 v4.0.0-20230305170008-8188dc5388df // indirect
2525
github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a // indirect
26-
github.com/aws/aws-sdk-go v1.55.7
2726
github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.5.3 // indirect
2827
github.com/aws/aws-sdk-go-v2/credentials v1.16.10 // indirect
2928
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.14.9 // indirect

tests/e2e-kubernetes/go.sum

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,6 @@ github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPd
1313
github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs=
1414
github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a h1:idn718Q4B6AGu/h5Sxe66HYVdqdGu2l9Iebqhi/AEoA=
1515
github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a/go.mod h1:lB+ZfQJz7igIIfQNfa7Ml4HSf2uFQQRzpGGRXenZAgY=
16-
github.com/aws/aws-sdk-go v1.55.7 h1:UJrkFq7es5CShfBwlWAC8DA077vp8PyVbQd3lqLiztE=
17-
github.com/aws/aws-sdk-go v1.55.7/go.mod h1:eRwEWoyTWFMVYVQzKMNHWP5/RV4xIUGMQfXQHfHkpNU=
18-
github.com/aws/aws-sdk-go-v2 v1.30.3 h1:jUeBtG0Ih+ZIFH0F4UkmL9w3cSpaMv9tYYDbzILP8dY=
19-
github.com/aws/aws-sdk-go-v2 v1.30.3/go.mod h1:nIQjQVp5sfpQcTc9mPSr1B0PaWK5ByX9MOoDadSN4lc=
2016
github.com/aws/aws-sdk-go-v2 v1.36.3 h1:mJoei2CxPutQVxaATCzDUjcZEjVRdpsiiXi2o38yqWM=
2117
github.com/aws/aws-sdk-go-v2 v1.36.3/go.mod h1:LLXuLpgzEbD766Z5ECcRmi8AzSwfZItDtmABVkRLGzg=
2218
github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.5.3 h1:Zx9+31KyB8wQna6SXFWOewlgoY5uGdDAu6PTOEU3OQI=
@@ -27,12 +23,8 @@ github.com/aws/aws-sdk-go-v2/credentials v1.16.10 h1:VmRkuoKaGl2ZDNGkkRQgw80Hxj1
2723
github.com/aws/aws-sdk-go-v2/credentials v1.16.10/go.mod h1:WEn22lpd50buTs/TDqywytW5xQ2zPOMbYipIlqI6xXg=
2824
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.14.9 h1:FZVFahMyZle6WcogZCOxo6D/lkDA2lqKIn4/ueUmVXw=
2925
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.14.9/go.mod h1:kjq7REMIkxdtcEC9/4BVXjOsNY5isz6jQbEgk6osRTU=
30-
github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.15 h1:SoNJ4RlFEQEbtDcCEt+QG56MY4fm4W8rYirAmq+/DdU=
31-
github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.15/go.mod h1:U9ke74k1n2bf+RIgoX1SXFed1HLs51OgUSs+Ph0KJP8=
3226
github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.34 h1:ZK5jHhnrioRkUNOc+hOgQKlUL5JeC3S6JgLxtQ+Rm0Q=
3327
github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.34/go.mod h1:p4VfIceZokChbA9FzMbRGz5OV+lekcVtHlPKEO0gSZY=
34-
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.15 h1:C6WHdGnTDIYETAm5iErQUiVNsclNx9qbJVPIt03B6bI=
35-
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.15/go.mod h1:ZQLZqhcu+JhSrA9/NXRm8SkDvsycE+JkV3WGY41e+IM=
3628
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.34 h1:SZwFm17ZUNNg5Np0ioo/gq8Mn6u9w19Mri8DnJ15Jf0=
3729
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.34/go.mod h1:dFZsC0BLo346mvKQLWmoJxT+Sjp+qcVR1tRVHQGOH9Q=
3830
github.com/aws/aws-sdk-go-v2/internal/ini v1.7.1 h1:uR9lXYjdPX0xY+NhvaJ4dD8rpSRz5VY81ccIIoNG+lw=
@@ -61,8 +53,6 @@ github.com/aws/aws-sdk-go-v2/service/ssooidc v1.21.3 h1:CxAHBS0BWSUqI7qzXHc2ZpTe
6153
github.com/aws/aws-sdk-go-v2/service/ssooidc v1.21.3/go.mod h1:7Lt5mjQ8x5rVdKqg+sKKDeuwoszDJIIPmkd8BVsEdS0=
6254
github.com/aws/aws-sdk-go-v2/service/sts v1.26.3 h1:KfREzajmHCSYjCaMRtdLr9boUMA7KPpoPApitPlbNeo=
6355
github.com/aws/aws-sdk-go-v2/service/sts v1.26.3/go.mod h1:7Ld9eTqocTvJqqJ5K/orbSDwmGcpRdlDiLjz2DO+SL8=
64-
github.com/aws/smithy-go v1.20.3 h1:ryHwveWzPV5BIof6fyDvor6V3iUL7nTfiTKXHiW05nE=
65-
github.com/aws/smithy-go v1.20.3/go.mod h1:krry+ya/rV9RDcV/Q16kpu6ypI4K2czasz0NC3qS14E=
6656
github.com/aws/smithy-go v1.22.2 h1:6D9hW43xKFrRx/tXXfAlIZc4JI+yQe6snnWcQyxSyLQ=
6757
github.com/aws/smithy-go v1.22.2/go.mod h1:irrKGvNn1InZwb2d7fkIRNucdfwR8R+Ts3wxYa/cJHg=
6858
github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA=

tests/e2e-kubernetes/testsuites/credentials.go

Lines changed: 35 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,12 @@ const (
5656
stsAssumeRoleRetryMaxBackoffDelay = 10 * time.Second
5757
)
5858

59+
const (
60+
eksauthAssumeRoleRetryCode = "AccessDeniedException"
61+
eksauthAssumeRoleRetryMaxAttemps = 0 // This will cause SDK to retry indefinetly, but we do have a timeout on the operation
62+
eksauthAssumeRoleRetryMaxBackoffDelay = 10 * time.Second
63+
)
64+
5965
const serviceAccountTokenAudienceSTS = "sts.amazonaws.com"
6066
const roleARNAnnotation = "eks.amazonaws.com/role-arn"
6167
const credentialSecretName = "aws-secret"
@@ -360,7 +366,7 @@ func (t *s3CSICredentialsTestSuite) DefineTests(driver storageframework.TestDriv
360366
removeAssociation := createPodIdentityAssociation(ctx, f, sa, *role.Arn)
361367
deferCleanup(removeAssociation)
362368

363-
pod := CSIDriverPod(ctx, f)
369+
pod := csiDriverPod(ctx, f)
364370
waitUntilRoleIsAssumableWithEKS(ctx, f, sa, pod)
365371

366372
// Trigger recreation of our pods to use the new IAM role
@@ -752,7 +758,7 @@ func eksPodIdentityRoleTrustPolicyDocument() string {
752758
{
753759
"Effect": "Allow",
754760
"Principal": jsonMap{
755-
"Service": "pods.eks.amazonaws.com",
761+
"Service": serviceAccountTokenAudienceEKS,
756762
},
757763
"Action": []string{"sts:AssumeRole", "sts:TagSession"},
758764
},
@@ -818,7 +824,7 @@ func assumeRole(ctx context.Context, f *framework.Framework, roleArn string) *st
818824
framework.Logf("Assuming IAM role %s", roleArn)
819825

820826
client := sts.NewFromConfig(awsConfig(ctx))
821-
return waitUntilRoleIsAssumable(ctx, client.AssumeRole, &sts.AssumeRoleInput{
827+
return waitUntilRoleIsAssumableSTS(ctx, client.AssumeRole, &sts.AssumeRoleInput{
822828
RoleArn: ptr.To(roleArn),
823829
RoleSessionName: ptr.To(f.BaseName),
824830
DurationSeconds: ptr.To(int32(stsAssumeRoleCredentialDuration.Seconds())),
@@ -828,38 +834,44 @@ func assumeRole(ctx context.Context, f *framework.Framework, roleArn string) *st
828834
// waitUntilRoleIsAssumable waits until the given role is assumable.
829835
// This is needed because we're creating new roles in our test cases and then trying to assume those roles,
830836
// but there is a delay between IAM and STS services and newly created roles/policies does not appear on STS immediately.
831-
func waitUntilRoleIsAssumable[Input any, Output any](ctx context.Context, assumeFunc func(context.Context, *Input, ...func(*sts.Options)) (*Output, error), input *Input) *Output {
837+
func waitUntilRoleIsAssumable[Input any, Output any, O any](
838+
ctx context.Context,
839+
assumeFunc func(context.Context, *Input, ...func(O)) (*Output, error),
840+
input *Input,
841+
optionsFunc func(O),
842+
) *Output {
832843
ctx, cancel := context.WithTimeout(ctx, stsAssumeRoleTimeout)
833844
defer cancel()
834845

835-
output, err := assumeFunc(ctx, input, func(o *sts.Options) {
836-
o.Retryer = retry.AddWithErrorCodes(o.Retryer, stsAssumeRoleRetryCode)
837-
o.Retryer = retry.AddWithMaxAttempts(o.Retryer, stsAssumeRoleRetryMaxAttemps)
838-
o.Retryer = retry.AddWithMaxBackoffDelay(o.Retryer, stsAssumeRoleRetryMaxBackoffDelay)
839-
})
846+
output, err := assumeFunc(ctx, input, optionsFunc)
840847
framework.ExpectNoError(err)
841848
gomega.Expect(output).ToNot(gomega.BeNil())
842849

843850
return output
844851
}
845852

846-
// waitUntilRoleIsAssumableEKS waits until the given role is assumable.
847-
// This is needed because we're creating new roles in our test cases and then trying to assume those roles,
848-
// but there is a delay between IAM and STS services and newly created roles/policies does not appear on STS immediately.
849-
func waitUntilRoleIsAssumableEKS[Input any, Output any](ctx context.Context, assumeFunc func(context.Context, *Input, ...func(*eksauth.Options)) (*Output, error), input *Input) *Output {
850-
ctx, cancel := context.WithTimeout(ctx, stsAssumeRoleTimeout)
851-
defer cancel()
852-
853-
output, err := assumeFunc(ctx, input, func(o *eksauth.Options) {
854-
o.Retryer = retry.AddWithErrorCodes(o.Retryer, stsAssumeRoleRetryCode, "AccessDeniedException")
853+
func waitUntilRoleIsAssumableSTS[Input any, Output any](
854+
ctx context.Context,
855+
assumeFunc func(context.Context, *Input, ...func(*sts.Options)) (*Output, error),
856+
input *Input,
857+
) *Output {
858+
return waitUntilRoleIsAssumable(ctx, assumeFunc, input, func(o *sts.Options) {
859+
o.Retryer = retry.AddWithErrorCodes(o.Retryer, stsAssumeRoleRetryCode)
855860
o.Retryer = retry.AddWithMaxAttempts(o.Retryer, stsAssumeRoleRetryMaxAttemps)
856861
o.Retryer = retry.AddWithMaxBackoffDelay(o.Retryer, stsAssumeRoleRetryMaxBackoffDelay)
857862
})
863+
}
858864

859-
framework.ExpectNoError(err)
860-
gomega.Expect(output).ToNot(gomega.BeNil())
861-
862-
return output
865+
func waitUntilRoleIsAssumableEKS[Input any, Output any](
866+
ctx context.Context,
867+
assumeFunc func(context.Context, *Input, ...func(*eksauth.Options)) (*Output, error),
868+
input *Input,
869+
) *Output {
870+
return waitUntilRoleIsAssumable(ctx, assumeFunc, input, func(o *eksauth.Options) {
871+
o.Retryer = retry.AddWithErrorCodes(o.Retryer, eksauthAssumeRoleRetryCode)
872+
o.Retryer = retry.AddWithMaxAttempts(o.Retryer, eksauthAssumeRoleRetryMaxAttemps)
873+
o.Retryer = retry.AddWithMaxBackoffDelay(o.Retryer, eksauthAssumeRoleRetryMaxBackoffDelay)
874+
})
863875
}
864876

865877
func waitUntilRoleIsAssumableWithWebIdentity(ctx context.Context, f *framework.Framework, sa *v1.ServiceAccount) {
@@ -875,7 +887,7 @@ func waitUntilRoleIsAssumableWithWebIdentity(ctx context.Context, f *framework.F
875887
framework.ExpectNoError(err)
876888

877889
client := sts.NewFromConfig(awsConfig(ctx))
878-
waitUntilRoleIsAssumable(ctx, client.AssumeRoleWithWebIdentity, &sts.AssumeRoleWithWebIdentityInput{
890+
waitUntilRoleIsAssumableSTS(ctx, client.AssumeRoleWithWebIdentity, &sts.AssumeRoleWithWebIdentityInput{
879891
RoleArn: ptr.To(roleARN),
880892
RoleSessionName: ptr.To(f.BaseName),
881893
WebIdentityToken: ptr.To(serviceAccountToken.Status.Token),

tests/e2e-kubernetes/testsuites/util.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -250,8 +250,6 @@ func deletePodIdentityAssociations(ctx context.Context, sa *v1.ServiceAccount) {
250250
})
251251
framework.ExpectNoError(listErr)
252252

253-
framework.Logf("listOutput.Associations: %s", listOutput.Associations)
254-
255253
for _, association := range listOutput.Associations {
256254
_, deleteErr := eksClient.DeletePodIdentityAssociation(ctx, &eks.DeletePodIdentityAssociationInput{
257255
ClusterName: &ClusterName,
@@ -261,7 +259,7 @@ func deletePodIdentityAssociations(ctx context.Context, sa *v1.ServiceAccount) {
261259
}
262260
}
263261

264-
func CSIDriverPod(ctx context.Context, f *framework.Framework) *v1.Pod {
262+
func csiDriverPod(ctx context.Context, f *framework.Framework) *v1.Pod {
265263
ds := csiDriverDaemonSet(ctx, f)
266264
client := f.ClientSet.CoreV1().Pods(csiDriverDaemonSetNamespace)
267265

0 commit comments

Comments
 (0)