diff --git a/go.mod b/go.mod index 4a9b307a4..97a8f0f2e 100644 --- a/go.mod +++ b/go.mod @@ -1,11 +1,14 @@ module github.com/kubernetes-sigs/aws-efs-csi-driver require ( + github.com/aws/aws-sdk-go v1.50.3 github.com/aws/aws-sdk-go-v2 v1.31.0 github.com/aws/aws-sdk-go-v2/config v1.27.35 + github.com/aws/aws-sdk-go-v2/credentials v1.17.33 github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.13 github.com/aws/aws-sdk-go-v2/service/ec2 v1.178.0 github.com/aws/aws-sdk-go-v2/service/efs v1.31.8 + github.com/aws/aws-sdk-go-v2/service/sts v1.30.8 github.com/aws/smithy-go v1.21.0 github.com/container-storage-interface/spec v1.7.0 github.com/golang/mock v1.6.0 @@ -26,8 +29,6 @@ require ( ) require ( - github.com/aws/aws-sdk-go v1.50.3 // indirect - github.com/aws/aws-sdk-go-v2/credentials v1.17.33 // indirect github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.18 // indirect github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.18 // indirect github.com/aws/aws-sdk-go-v2/internal/ini v1.8.1 // indirect @@ -35,7 +36,6 @@ require ( github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.11.20 // indirect github.com/aws/aws-sdk-go-v2/service/sso v1.22.8 // indirect github.com/aws/aws-sdk-go-v2/service/ssooidc v1.26.8 // indirect - github.com/aws/aws-sdk-go-v2/service/sts v1.30.8 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/blang/semver/v4 v4.0.0 // indirect github.com/cenkalti/backoff/v4 v4.2.1 // indirect diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go index ecf5ab731..edb46c65b 100644 --- a/pkg/cloud/cloud.go +++ b/pkg/cloud/cloud.go @@ -51,7 +51,9 @@ var ( ) type FileSystem struct { - FileSystemId string + FileSystemId string + FileSystemArn string + Tags map[string]string } type AccessPoint struct { @@ -105,7 +107,8 @@ type Cloud interface { DescribeAccessPoint(ctx context.Context, accessPointId string) (accessPoint *AccessPoint, err error) FindAccessPointByClientToken(ctx context.Context, clientToken, fileSystemId string) (accessPoint *AccessPoint, err error) ListAccessPoints(ctx context.Context, fileSystemId string) (accessPoints []*AccessPoint, err error) - DescribeFileSystem(ctx context.Context, fileSystemId string) (fs *FileSystem, err error) + DescribeFileSystemById(ctx context.Context, fileSystemId string) (fs *FileSystem, err error) + DescribeFileSystemByToken(ctx context.Context, creationToken string) (fs []*FileSystem, err error) DescribeMountTargets(ctx context.Context, fileSystemId, az string) (fs *MountTarget, err error) } @@ -322,7 +325,41 @@ func (c *cloud) ListAccessPoints(ctx context.Context, fileSystemId string) (acce return } -func (c *cloud) DescribeFileSystem(ctx context.Context, fileSystemId string) (fs *FileSystem, err error) { +func (c *cloud) DescribeFileSystemByToken(ctx context.Context, creationToken string) (fs []*FileSystem, err error) { + var describeFsInput *efs.DescribeFileSystemsInput + if creationToken == "" { + describeFsInput = &efs.DescribeFileSystemsInput{} + } else { + describeFsInput = &efs.DescribeFileSystemsInput{CreationToken: &creationToken} + } + klog.V(5).Infof("Calling DescribeFileSystems with input: %+v", *describeFsInput) + res, err := c.efs.DescribeFileSystems(ctx, describeFsInput) + if err != nil { + if isAccessDenied(err) { + return nil, ErrAccessDenied + } + if isFileSystemNotFound(err) { + return nil, ErrNotFound + } + return nil, fmt.Errorf("Describe File System failed: %v", err) + } + + var efsList = make([]*FileSystem, 0) + for _, fileSystem := range res.FileSystems { + var tagsList = make(map[string]string, 0) + for _, tag := range fileSystem.Tags { + tagsList[*tag.Key] = *tag.Value + } + efsList = append(efsList, &FileSystem{ + FileSystemId: *fileSystem.FileSystemId, + FileSystemArn: *fileSystem.FileSystemArn, + Tags: tagsList, + }) + } + return efsList, nil +} + +func (c *cloud) DescribeFileSystemById(ctx context.Context, fileSystemId string) (fs *FileSystem, err error) { describeFsInput := &efs.DescribeFileSystemsInput{FileSystemId: &fileSystemId} klog.V(5).Infof("Calling DescribeFileSystems with input: %+v", *describeFsInput) res, err := c.efs.DescribeFileSystems(ctx, describeFsInput) diff --git a/pkg/cloud/cloud_test.go b/pkg/cloud/cloud_test.go index d4ad01666..8f1c009cd 100644 --- a/pkg/cloud/cloud_test.go +++ b/pkg/cloud/cloud_test.go @@ -785,7 +785,7 @@ func TestDescribeFileSystem(t *testing.T) { ctx := context.Background() mockEfs.EXPECT().DescribeFileSystems(gomock.Eq(ctx), gomock.Any()).Return(output, nil) - res, err := c.DescribeFileSystem(ctx, fsId) + res, err := c.DescribeFileSystemById(ctx, fsId) if err != nil { t.Fatalf("Describe File System failed: %v", err) } @@ -813,7 +813,7 @@ func TestDescribeFileSystem(t *testing.T) { ctx := context.Background() mockEfs.EXPECT().DescribeFileSystems(gomock.Eq(ctx), gomock.Any()).Return(output, nil) - _, err := c.DescribeFileSystem(ctx, fsId) + _, err := c.DescribeFileSystemById(ctx, fsId) if err == nil { t.Fatalf("DescribeFileSystem did not fail") } @@ -848,7 +848,7 @@ func TestDescribeFileSystem(t *testing.T) { ctx := context.Background() mockEfs.EXPECT().DescribeFileSystems(gomock.Eq(ctx), gomock.Any()).Return(output, nil) - _, err := c.DescribeFileSystem(ctx, fsId) + _, err := c.DescribeFileSystemById(ctx, fsId) if err == nil { t.Fatalf("DescribeFileSystem did not fail") } @@ -867,7 +867,7 @@ func TestDescribeFileSystem(t *testing.T) { &types.FileSystemNotFound{ Message: aws.String("File System not found"), }) - _, err := c.DescribeFileSystem(ctx, fsId) + _, err := c.DescribeFileSystemById(ctx, fsId) if err == nil { t.Fatalf("DescribeFileSystem did not fail") } @@ -890,7 +890,7 @@ func TestDescribeFileSystem(t *testing.T) { Code: AccessDeniedException, Message: "Access Denied", }) - _, err := c.DescribeFileSystem(ctx, fsId) + _, err := c.DescribeFileSystemById(ctx, fsId) if err == nil { t.Fatalf("DescribeFileSystem did not fail") } @@ -909,7 +909,7 @@ func TestDescribeFileSystem(t *testing.T) { ctx := context.Background() mockEfs.EXPECT().DescribeFileSystems(gomock.Eq(ctx), gomock.Any()).Return(nil, errors.New("DescribeFileSystem failed")) - _, err := c.DescribeFileSystem(ctx, fsId) + _, err := c.DescribeFileSystemById(ctx, fsId) if err == nil { t.Fatalf("DescribeFileSystem did not fail") } @@ -1050,6 +1050,214 @@ func TestDescribeMountTargets(t *testing.T) { } } +func TestDescribeFileSystemByToken(t *testing.T) { + var ( + fsId = []string{"fs-abcd1234", "fs-efgh5678"} + fsArn = []string{"arn:aws:elasticfilesystem:us-west-2:1111333322228888:file-system/fs-0123456789abcdef8", "arn:aws:elasticfilesystem:us-west-2:1111333322228888:file-system/fs-987654321abcdef0"} + creationToken = "efs-for-discovery" + az = "us-east-1a" + ) + + testCases := []struct { + name string + testFunc func(t *testing.T) + }{ + { + name: "Success: Normal flow", + testFunc: func(t *testing.T) { + mockctl := gomock.NewController(t) + mockEfs := mocks.NewMockEfs(mockctl) + c := &cloud{efs: mockEfs} + + fs := &efs.DescribeFileSystemsOutput{ + FileSystems: []types.FileSystemDescription{ + { + FileSystemId: aws.String(fsId[0]), + FileSystemArn: aws.String(fsArn[0]), + Encrypted: aws.Bool(true), + CreationToken: aws.String("efs-for-discovery"), + AvailabilityZoneName: aws.String(az), + Tags: []types.Tag{ + { + Key: aws.String("env"), + Value: aws.String("prod"), + }, + { + Key: aws.String("owner"), + Value: aws.String("avanishpatil23@gmail.com"), + }, + }, + }, + { + FileSystemId: aws.String(fsId[1]), + FileSystemArn: aws.String(fsArn[1]), + Encrypted: aws.Bool(true), + CreationToken: aws.String("efs-not-for-discovery"), + AvailabilityZoneName: aws.String(az), + Tags: []types.Tag{ + { + Key: aws.String("env"), + Value: aws.String("prod"), + }, + { + Key: aws.String("owner"), + Value: aws.String("avanishpatil23@gmail.com"), + }, + }, + }, + }, + } + + ctx := context.Background() + mockEfs.EXPECT().DescribeFileSystems(gomock.Eq(ctx), gomock.Any()).DoAndReturn(func(ctx context.Context, input *efs.DescribeFileSystemsInput, opts ...func(*efs.Options)) (*efs.DescribeFileSystemsOutput, error) { + res := &efs.DescribeFileSystemsOutput{} + for _, fileSystem := range fs.FileSystems { + if input.CreationToken != nil && *fileSystem.CreationToken == *input.CreationToken { + res.FileSystems = append(res.FileSystems, fileSystem) + } else if input.CreationToken == nil { + res.FileSystems = append(res.FileSystems, fileSystem) + } + } + return res, nil + }) + + efsList, err := c.DescribeFileSystemByToken(ctx, creationToken) + if err != nil { + t.Fatalf("DescribeFileSystem failed") + } + if len(efsList) != 1 { + t.Fatalf("Expected 1 fileSystems got %d", len(efsList)) + } + mockctl.Finish() + }, + }, + { + name: "Success: Normal flow without creation token", + testFunc: func(t *testing.T) { + mockctl := gomock.NewController(t) + mockEfs := mocks.NewMockEfs(mockctl) + c := &cloud{efs: mockEfs} + + fs := &efs.DescribeFileSystemsOutput{ + FileSystems: []types.FileSystemDescription{ + { + FileSystemId: aws.String(fsId[0]), + FileSystemArn: aws.String(fsArn[0]), + Encrypted: aws.Bool(true), + CreationToken: aws.String("efs-for-discovery"), + AvailabilityZoneName: aws.String(az), + Tags: []types.Tag{ + { + Key: aws.String("env"), + Value: aws.String("prod"), + }, + { + Key: aws.String("owner"), + Value: aws.String("avanishpatil23@gmail.com"), + }, + }, + }, + { + FileSystemId: aws.String(fsId[1]), + FileSystemArn: aws.String(fsArn[1]), + Encrypted: aws.Bool(true), + CreationToken: aws.String("efs-not-for-discovery"), + AvailabilityZoneName: aws.String(az), + Tags: []types.Tag{ + { + Key: aws.String("env"), + Value: aws.String("prod"), + }, + { + Key: aws.String("owner"), + Value: aws.String("avanishpatil23@gmail.com"), + }, + }, + }, + }, + } + + ctx := context.Background() + mockEfs.EXPECT().DescribeFileSystems(gomock.Eq(ctx), gomock.Any()).DoAndReturn(func(ctx context.Context, input *efs.DescribeFileSystemsInput, opts ...func(*efs.Options)) (*efs.DescribeFileSystemsOutput, error) { + res := &efs.DescribeFileSystemsOutput{} + for _, fileSystem := range fs.FileSystems { + if input.CreationToken != nil && *fileSystem.CreationToken == *input.CreationToken { + res.FileSystems = append(res.FileSystems, fileSystem) + } else if input.CreationToken == nil { + res.FileSystems = append(res.FileSystems, fileSystem) + } + } + return res, nil + }) + + efsList, err := c.DescribeFileSystemByToken(ctx, "") + if err != nil { + t.Fatalf("DescribeFileSystem failed") + } + if len(efsList) != len(fs.FileSystems) { + t.Fatalf("Expected 1 fileSystems got %d", len(efsList)) + } + for i, fileSystem := range fs.FileSystems { + for _, v := range fileSystem.Tags { + if val, exists := efsList[i].Tags[*v.Key]; !exists || val != *v.Value { + t.Fatalf("Tags list is corrupted, expected %s for %s but got %s", *v.Value, *v.Key, val) + } + } + } + mockctl.Finish() + }, + }, + { + name: "Fail: Access Denied", + testFunc: func(t *testing.T) { + mockctl := gomock.NewController(t) + mockEfs := mocks.NewMockEfs(mockctl) + c := &cloud{efs: mockEfs} + + ctx := context.Background() + mockEfs.EXPECT().DescribeFileSystems(gomock.Eq(ctx), gomock.Any()).Return(nil, &smithy.GenericAPIError{ + Code: AccessDeniedException, + Message: "Access Denied", + }) + + _, err := c.DescribeFileSystemByToken(ctx, "efs-discovery") + if err == nil { + t.Fatalf("DescribeFileSystemByToken did not fail") + } + if err != ErrAccessDenied { + t.Fatalf("Failed. Expected: %v, Actual:%v", ErrAccessDenied, err) + } + mockctl.Finish() + }, + }, + { + name: "Fail: File System not found", + testFunc: func(t *testing.T) { + mockctl := gomock.NewController(t) + mockEfs := mocks.NewMockEfs(mockctl) + c := &cloud{efs: mockEfs} + + ctx := context.Background() + mockEfs.EXPECT().DescribeFileSystems(gomock.Eq(ctx), gomock.Any()).Return(nil, &types.FileSystemNotFound{ + Message: aws.String("File System not found"), + }) + + _, err := c.DescribeFileSystemByToken(ctx, "efs-discovery") + if err == nil { + t.Fatalf("DescribeFileSystemByToken did not fail") + } + if err != ErrNotFound { + t.Fatalf("Failed. Expected: %v, Actual:%v", ErrNotFound, err) + } + mockctl.Finish() + }, + }, + } + for _, tc := range testCases { + t.Run(tc.name, tc.testFunc) + } +} + func testResult(t *testing.T, funcName string, ret interface{}, err error, expectError errtyp) { if expectError.message == "" { if err != nil { diff --git a/pkg/cloud/fakes.go b/pkg/cloud/fakes.go index 8f910ca88..3f20a51b6 100644 --- a/pkg/cloud/fakes.go +++ b/pkg/cloud/fakes.go @@ -12,6 +12,7 @@ type FakeCloudProvider struct { fileSystems map[string]*FileSystem accessPoints map[string]*AccessPoint mountTargets map[string]*MountTarget + tags map[string]map[string]string } func NewFakeCloudProvider() *FakeCloudProvider { @@ -20,6 +21,7 @@ func NewFakeCloudProvider() *FakeCloudProvider { fileSystems: make(map[string]*FileSystem), accessPoints: make(map[string]*AccessPoint), mountTargets: make(map[string]*MountTarget), + tags: make(map[string]map[string]string), } } @@ -69,7 +71,7 @@ func (c *FakeCloudProvider) DescribeAccessPoint(ctx context.Context, accessPoint // CreateVolume calls DescribeFileSystem and then CreateAccessPoint. // Add file system into the map here to allow CreateVolume sanity tests to succeed. -func (c *FakeCloudProvider) DescribeFileSystem(ctx context.Context, fileSystemId string) (fileSystem *FileSystem, err error) { +func (c *FakeCloudProvider) DescribeFileSystemById(ctx context.Context, fileSystemId string) (fileSystem *FileSystem, err error) { if fs, ok := c.fileSystems[fileSystemId]; ok { return fs, nil } @@ -90,6 +92,37 @@ func (c *FakeCloudProvider) DescribeFileSystem(ctx context.Context, fileSystemId return fs, nil } +func (c *FakeCloudProvider) DescribeFileSystemByToken(ctx context.Context, creationToken string) (fileSystem []*FileSystem, err error) { + var efsList = make([]*FileSystem, 0) + if fs, ok := c.fileSystems[creationToken]; ok { + efsList = append(efsList, fs) + return efsList, nil + } + + tags := map[string]string{ + "env": "prod", + "owner": "avanishpatil23@gmail.com", + } + + fs := &FileSystem{ + FileSystemId: creationToken, + FileSystemArn: "arn:aws:elasticfilesystem:us-west-2:xxxx:file-system/fs-xxxx", + Tags: tags, + } + c.fileSystems[creationToken] = fs + + mt := &MountTarget{ + AZName: "us-east-1a", + AZId: "mock-AZ-id", + MountTargetId: "fsmt-abcd1234", + IPAddress: "127.0.0.1", + } + + c.mountTargets[creationToken] = mt + efsList = append(efsList, c.fileSystems[creationToken]) + return efsList, nil +} + func (c *FakeCloudProvider) DescribeMountTargets(ctx context.Context, fileSystemId, az string) (mountTarget *MountTarget, err error) { if mt, ok := c.mountTargets[fileSystemId]; ok { return mt, nil diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index 86c6baaf1..9aab685f4 100644 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -19,7 +19,9 @@ package driver import ( "context" "crypto/sha256" + "encoding/json" "fmt" + "github.com/aws/aws-sdk-go/aws" "os" "path" "sort" @@ -47,6 +49,7 @@ const ( DirectoryPerms = "directoryPerms" EnsureUniqueDirectory = "ensureUniqueDirectory" FsId = "fileSystemId" + FsDiscovery = "fileSystemDiscovery" Gid = "gid" GidMin = "gidRangeStart" GidMax = "gidRangeEnd" @@ -78,6 +81,11 @@ var ( } ) +type FileSystemDiscovery struct { + CreationToken string `yaml:"creationToken" json:"creationToken"` + Tags map[string]string `yaml:"tags,omitempty" json:"tags,omitempty"` +} + func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) (*csi.CreateVolumeResponse, error) { klog.V(4).Infof("CreateVolume: called with args %+v", util.SanitizeRequest(*req)) @@ -148,18 +156,34 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) CapacityGiB: volSize, } + localCloud, roleArn, crossAccountDNSEnabled, err = getCloud(req.GetSecrets(), d) + if err != nil { + return nil, err + } + if value, ok := volumeParams[FsId]; ok { if strings.TrimSpace(value) == "" { return nil, status.Errorf(codes.InvalidArgument, "Parameter %v cannot be empty", FsId) } accessPointsOptions.FileSystemId = value } else { - return nil, status.Errorf(codes.InvalidArgument, "Missing %v parameter", FsId) - } - - localCloud, roleArn, crossAccountDNSEnabled, err = getCloud(req.GetSecrets(), d) - if err != nil { - return nil, err + var discovery *FileSystemDiscovery + if value, ok := volumeParams[FsDiscovery]; ok { + json.Unmarshal([]byte(value), &discovery) + } else { + return nil, status.Errorf(codes.InvalidArgument, "Both parameters %v and %v missing", FsId, FsDiscovery) + } + FsId, err := discoverVolume(ctx, localCloud, discovery) + if err != nil { + if err == cloud.ErrAccessDenied { + return nil, status.Errorf(codes.Unauthenticated, "Access Denied. Please ensure you have the right AWS permissions: %v", err) + } + if err == cloud.ErrNotFound { + return nil, status.Errorf(codes.InvalidArgument, "File System does not exist: %v", err) + } + return nil, status.Errorf(codes.Internal, "Failed to fetch Access Points or Describe File System: %v", err) + } + accessPointsOptions.FileSystemId = *FsId } var accessPoint *cloud.AccessPoint @@ -272,7 +296,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) if uid == -1 || gid == -1 { accessPoints, err = localCloud.ListAccessPoints(ctx, accessPointsOptions.FileSystemId) } else { - _, err = localCloud.DescribeFileSystem(ctx, accessPointsOptions.FileSystemId) + _, err = localCloud.DescribeFileSystemById(ctx, accessPointsOptions.FileSystemId) } if err != nil { if err == cloud.ErrAccessDenied { @@ -646,3 +670,39 @@ func get64LenHash(text string) string { h.Write([]byte(text)) return fmt.Sprintf("%x", h.Sum(nil)) } + +func discoverVolume(ctx context.Context, localCloud cloud.Cloud, discovery *FileSystemDiscovery) (*string, error) { + efs, err := localCloud.DescribeFileSystemByToken(ctx, discovery.CreationToken) + if err != nil { + if err == cloud.ErrAccessDenied { + return nil, status.Errorf(codes.Unauthenticated, "Access Denied. Please ensure you have the right AWS permissions: %v", err) + } + if err == cloud.ErrNotFound { + return nil, status.Errorf(codes.InvalidArgument, "File System does not exist: %v", err) + } + return nil, status.Errorf(codes.Internal, "Failed to fetch Access Points or Describe File System: %v", err) + } + if len(discovery.Tags) > 0 { + res := make([]string, 0) + for _, fs := range efs { + tags := fs.Tags + foundAllTags := true + for k, v := range discovery.Tags { + if value, exists := tags[k]; !exists || value != v { + foundAllTags = false + } + } + if foundAllTags { + res = append(res, fs.FileSystemId) + } + } + if len(res) != 1 { + return nil, fmt.Errorf("failed to discover volume, found %d file systems", len(res)) + } + return aws.String(res[0]), nil + } + if len(efs) != 1 { + return nil, fmt.Errorf("failed to discover volume, found %d file systems", len(efs)) + } + return aws.String(efs[0].FileSystemId), nil +} diff --git a/pkg/driver/controller_test.go b/pkg/driver/controller_test.go index b625f508b..b7fda05fc 100644 --- a/pkg/driver/controller_test.go +++ b/pkg/driver/controller_test.go @@ -78,7 +78,7 @@ func TestCreateVolume(t *testing.T) { AccessPointId: apId, FileSystemId: fsId, } - mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) + mockCloud.EXPECT().DescribeFileSystemById(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Eq(volumeName), gomock.Any()).Return(accessPoint, nil). Do(func(ctx context.Context, clientToken string, accessPointsOptions *cloud.AccessPointOptions) { if accessPointsOptions.Uid != 1000 { @@ -145,7 +145,7 @@ func TestCreateVolume(t *testing.T) { AccessPointId: apId, FileSystemId: fsId, } - mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) + mockCloud.EXPECT().DescribeFileSystemById(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil). Do(func(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions) { if accessPointOpts.Uid != 1000 { @@ -1669,7 +1669,7 @@ func TestCreateVolume(t *testing.T) { }, }, { - name: "Fail: Missing Parameter FsId", + name: "Fail: FsId and FsDiscovery cannot be missing", testFunc: func(t *testing.T) { mockCtl := gomock.NewController(t) mockCloud := mocks.NewMockCloud(mockCtl) @@ -2131,7 +2131,7 @@ func TestCreateVolume(t *testing.T) { } ctx := context.Background() - mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(nil, cloud.ErrNotFound) + mockCloud.EXPECT().DescribeFileSystemById(gomock.Eq(ctx), gomock.Any()).Return(nil, cloud.ErrNotFound) _, err := driver.CreateVolume(ctx, req) if err == nil { t.Fatal("CreateVolume did not fail") @@ -2209,7 +2209,7 @@ func TestCreateVolume(t *testing.T) { } ctx := context.Background() - mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(nil, cloud.ErrAccessDenied) + mockCloud.EXPECT().DescribeFileSystemById(gomock.Eq(ctx), gomock.Any()).Return(nil, cloud.ErrAccessDenied) _, err := driver.CreateVolume(ctx, req) if err == nil { t.Fatal("CreateVolume did not fail") @@ -2287,7 +2287,7 @@ func TestCreateVolume(t *testing.T) { } ctx := context.Background() - mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(nil, errors.New("DescribeFileSystem failed")) + mockCloud.EXPECT().DescribeFileSystemById(gomock.Eq(ctx), gomock.Any()).Return(nil, errors.New("DescribeFileSystem failed")) _, err := driver.CreateVolume(ctx, req) if err == nil { t.Fatal("CreateVolume did not fail") @@ -3222,6 +3222,344 @@ func TestControllerGetCapabilities(t *testing.T) { } } +func TestDiscoverVolume(t *testing.T) { + var ( + fsId = []string{"fs-abcd1234", "fs-efgh5678"} + fsArn = []string{"arn:aws:elasticfilesystem:us-west-2:1111333322228888:file-system/fs-0123456789abcdef8", "arn:aws:elasticfilesystem:us-west-2:1111333322228888:file-system/fs-987654321abcdef0"} + tags = []map[string]string{ + { + "env": "prod", + "owner": "avanishpatil23@gmail.com", + }, + { + "env": "dev", + "owner": "avanishpatil23@gmail.com", + }, + } + ) + testCases := []struct { + name string + testFunc func(t *testing.T) + }{ + { + name: "Success: Normal flow", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + + ctx := context.Background() + + discovery := &FileSystemDiscovery{ + CreationToken: "efs-discovery", + Tags: map[string]string{ + "env": "prod", + "owner": "avanishpatil23@gmail.com", + }, + } + + fileSystem := []*cloud.FileSystem{ + { + FileSystemId: fsId[0], + FileSystemArn: fsArn[0], + Tags: tags[0], + }, + { + FileSystemId: fsId[1], + FileSystemArn: fsArn[1], + Tags: tags[1], + }, + } + + mockCloud.EXPECT().DescribeFileSystemByToken(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) + + efsId, err := discoverVolume(ctx, mockCloud, discovery) + + if err != nil { + t.Fatalf("EFS Discovery failed: %v", err) + } + if *efsId != "fs-abcd1234" { + t.Fatalf("Expected fsId to be %s but go %s", fsId[0], *efsId) + } + mockCtl.Finish() + }, + }, + { + name: "Success: Normal flow without tags", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + + ctx := context.Background() + discovery := &FileSystemDiscovery{ + CreationToken: "efs-discovery", + } + fileSystem := []*cloud.FileSystem{ + { + FileSystemId: fsId[0], + FileSystemArn: fsArn[0], + Tags: tags[0], + }, + } + mockCloud.EXPECT().DescribeFileSystemByToken(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) + efsId, err := discoverVolume(ctx, mockCloud, discovery) + + if err != nil { + t.Fatalf("EFS Discovery failed: %v", err) + } + if *efsId != "fs-abcd1234" { + t.Fatalf("Expected fsId to be %s but got %s", fsId[0], *efsId) + } + mockCtl.Finish() + }, + }, + { + name: "Fail: Multiple EFS discovered", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + + ctx := context.Background() + + discovery := &FileSystemDiscovery{ + CreationToken: "efs-discovery", + Tags: map[string]string{ + "env": "prod", + "owner": "avanishpatil23@gmail.com", + }, + } + + fileSystem := []*cloud.FileSystem{ + { + FileSystemId: fsId[0], + FileSystemArn: fsArn[0], + Tags: tags[0], + }, + { + FileSystemId: fsId[1], + FileSystemArn: fsArn[1], + Tags: tags[0], + }, + } + + expectedErrMsg := "failed to discover volume, found 2 file systems" + + mockCloud.EXPECT().DescribeFileSystemByToken(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) + + efsId, err := discoverVolume(ctx, mockCloud, discovery) + if err == nil { + t.Fatalf("Expected error during FS Discovery") + } + if err.Error() != expectedErrMsg { + t.Fatalf("Expected error %s but got %s", expectedErrMsg, err.Error()) + } + if efsId != nil { + t.Fatalf("Expected fsId to be nil") + } + mockCtl.Finish() + }, + }, + { + name: "Fail: Multiple EFS discovered without tags", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + + ctx := context.Background() + discovery := &FileSystemDiscovery{ + CreationToken: "efs-discovery", + } + fileSystem := []*cloud.FileSystem{ + { + FileSystemId: fsId[0], + FileSystemArn: fsArn[0], + Tags: tags[0], + }, + { + FileSystemId: fsId[1], + FileSystemArn: fsArn[1], + Tags: tags[1], + }, + } + + expectedErrMsg := "failed to discover volume, found 2 file systems" + + mockCloud.EXPECT().DescribeFileSystemByToken(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) + + efsId, err := discoverVolume(ctx, mockCloud, discovery) + + if err == nil { + t.Fatalf("Expected error during FS Discovery") + } + if err.Error() != expectedErrMsg { + t.Fatalf("Expected error %s but got %s", expectedErrMsg, err.Error()) + } + if efsId != nil { + t.Fatalf("Expected fsId to be nil") + } + mockCtl.Finish() + }, + }, + { + name: "Fail: No EFS discovered", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + + ctx := context.Background() + + discovery := &FileSystemDiscovery{ + CreationToken: "efs-discovery", + Tags: map[string]string{ + "env": "prod", + "owner": "avanishpatil23@gmail.com", + }, + } + + fileSystem := []*cloud.FileSystem{ + { + FileSystemId: fsId[0], + FileSystemArn: fsArn[0], + }, + { + FileSystemId: fsId[1], + FileSystemArn: fsArn[1], + Tags: tags[1], + }, + } + + expectedErrMsg := "failed to discover volume, found 0 file systems" + + mockCloud.EXPECT().DescribeFileSystemByToken(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) + + efsId, err := discoverVolume(ctx, mockCloud, discovery) + if err == nil { + t.Fatalf("Expected error during FS Discovery") + } + if err.Error() != expectedErrMsg { + t.Fatalf("Expected error %s but got %s", expectedErrMsg, err.Error()) + } + if efsId != nil { + t.Fatalf("Expected fsId to be nil") + } + mockCtl.Finish() + }, + }, + { + name: "Fail: No EFS discovered with more discovery tags", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + + ctx := context.Background() + + discovery := &FileSystemDiscovery{ + CreationToken: "efs-discovery", + Tags: map[string]string{ + "env": "prod", + "budget": "engineering", + "owner": "avanishpatil23@gmail.com", + }, + } + + fileSystem := []*cloud.FileSystem{ + { + FileSystemId: fsId[0], + FileSystemArn: fsArn[0], + Tags: tags[0], + }, + { + FileSystemId: fsId[1], + FileSystemArn: fsArn[1], + Tags: tags[1], + }, + } + + expectedErrMsg := "failed to discover volume, found 0 file systems" + + mockCloud.EXPECT().DescribeFileSystemByToken(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) + + efsId, err := discoverVolume(ctx, mockCloud, discovery) + if err == nil { + t.Fatalf("Expected error during FS Discovery") + } + if err.Error() != expectedErrMsg { + t.Fatalf("Expected error %s but got %s", expectedErrMsg, err.Error()) + } + if efsId != nil { + t.Fatalf("Expected fsId to be nil") + } + mockCtl.Finish() + }, + }, + { + name: "Fail: No EFS found by DescribeFileSystemByToken", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + + ctx := context.Background() + + discovery := &FileSystemDiscovery{ + CreationToken: "efs-discovery", + Tags: map[string]string{ + "env": "prod", + "budget": "engineering", + "owner": "avanishpatil23@gmail.com", + }, + } + + fileSystem := []*cloud.FileSystem{} + + expectedErrMsg := "failed to discover volume, found 0 file systems" + + mockCloud.EXPECT().DescribeFileSystemByToken(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) + + efsId, err := discoverVolume(ctx, mockCloud, discovery) + if err == nil { + t.Fatalf("Expected error during FS Discovery") + } + if err.Error() != expectedErrMsg { + t.Fatalf("Expected error %s but got %s", expectedErrMsg, err.Error()) + } + if efsId != nil { + t.Fatalf("Expected fsId to be nil") + } + mockCtl.Finish() + }, + }, + { + name: "Fail: Access Denied", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + + ctx := context.Background() + + discovery := &FileSystemDiscovery{ + CreationToken: "efs-discovery", + Tags: map[string]string{ + "env": "prod", + "owner": "avanishpatil23@gmail.com", + }, + } + + mockCloud.EXPECT().DescribeFileSystemByToken(gomock.Eq(ctx), gomock.Any()).Return(nil, cloud.ErrAccessDenied) + + efsId, err := discoverVolume(ctx, mockCloud, discovery) + if err == nil { + t.Fatalf("Expected error during FS Discovery") + } + if efsId != nil { + t.Fatalf("Expected fsId to be nil") + } + mockCtl.Finish() + }, + }, + } + for _, tc := range testCases { + t.Run(tc.name, tc.testFunc) + } +} func verifyPathWhenUUIDIncluded(pathToVerify string, expectedPathWithoutUUID string) bool { r := regexp.MustCompile("(.*)-([0-9A-fA-F]+-[0-9A-fA-F]+-[0-9A-fA-F]+-[0-9A-fA-F]+-[0-9A-fA-F]+$)") matches := r.FindStringSubmatch(pathToVerify) diff --git a/pkg/driver/mocks/mock_cloud.go b/pkg/driver/mocks/mock_cloud.go index 6385943fa..fe54cc925 100644 --- a/pkg/driver/mocks/mock_cloud.go +++ b/pkg/driver/mocks/mock_cloud.go @@ -1,5 +1,5 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: pkg/cloud/cloud.go +// Source: ./pkg/cloud/cloud.go // Package mocks is a generated GoMock package. package mocks @@ -203,19 +203,34 @@ func (mr *MockCloudMockRecorder) DescribeAccessPoint(ctx, accessPointId interfac return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DescribeAccessPoint", reflect.TypeOf((*MockCloud)(nil).DescribeAccessPoint), ctx, accessPointId) } -// DescribeFileSystem mocks base method. -func (m *MockCloud) DescribeFileSystem(ctx context.Context, fileSystemId string) (*cloud.FileSystem, error) { +// DescribeFileSystemById mocks base method. +func (m *MockCloud) DescribeFileSystemById(ctx context.Context, fileSystemId string) (*cloud.FileSystem, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DescribeFileSystem", ctx, fileSystemId) + ret := m.ctrl.Call(m, "DescribeFileSystemById", ctx, fileSystemId) ret0, _ := ret[0].(*cloud.FileSystem) ret1, _ := ret[1].(error) return ret0, ret1 } -// DescribeFileSystem indicates an expected call of DescribeFileSystem. -func (mr *MockCloudMockRecorder) DescribeFileSystem(ctx, fileSystemId interface{}) *gomock.Call { +// DescribeFileSystemById indicates an expected call of DescribeFileSystemById. +func (mr *MockCloudMockRecorder) DescribeFileSystemById(ctx, fileSystemId interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DescribeFileSystem", reflect.TypeOf((*MockCloud)(nil).DescribeFileSystem), ctx, fileSystemId) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DescribeFileSystemById", reflect.TypeOf((*MockCloud)(nil).DescribeFileSystemById), ctx, fileSystemId) +} + +// DescribeFileSystemByToken mocks base method. +func (m *MockCloud) DescribeFileSystemByToken(ctx context.Context, creationToken string) ([]*cloud.FileSystem, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DescribeFileSystemByToken", ctx, creationToken) + ret0, _ := ret[0].([]*cloud.FileSystem) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// DescribeFileSystemByToken indicates an expected call of DescribeFileSystemByToken. +func (mr *MockCloudMockRecorder) DescribeFileSystemByToken(ctx, creationToken interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DescribeFileSystemByToken", reflect.TypeOf((*MockCloud)(nil).DescribeFileSystemByToken), ctx, creationToken) } // DescribeMountTargets mocks base method. diff --git a/pkg/driver/mocks/mock_mount.go b/pkg/driver/mocks/mock_mount.go index 0a50a1ea5..2afe27d4f 100644 --- a/pkg/driver/mocks/mock_mount.go +++ b/pkg/driver/mocks/mock_mount.go @@ -8,12 +8,12 @@ import ( reflect "reflect" gomock "github.com/golang/mock/gomock" - mount_utils "k8s.io/mount-utils" + mount "k8s.io/mount-utils" ) // MockMounter is a mock of Mounter interface. type MockMounter struct { - mount_utils.Interface + mount.Interface ctrl *gomock.Controller recorder *MockMounterMockRecorder } @@ -97,10 +97,10 @@ func (mr *MockMounterMockRecorder) IsMountPoint(arg0 interface{}) *gomock.Call { } // List mocks base method. -func (m *MockMounter) List() ([]mount_utils.MountPoint, error) { +func (m *MockMounter) List() ([]mount.MountPoint, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "List") - ret0, _ := ret[0].([]mount_utils.MountPoint) + ret0, _ := ret[0].([]mount.MountPoint) ret1, _ := ret[1].(error) return ret0, ret1 } @@ -181,7 +181,7 @@ func (mr *MockMounterMockRecorder) MountSensitiveWithoutSystemdWithMountFlags(ar return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MountSensitiveWithoutSystemdWithMountFlags", reflect.TypeOf((*MockMounter)(nil).MountSensitiveWithoutSystemdWithMountFlags), arg0, arg1, arg2, arg3, arg4, arg5) } -// Unmount_utils mocks base method. +// Unmount mocks base method. func (m *MockMounter) Unmount(arg0 string) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Unmount", arg0) @@ -189,7 +189,7 @@ func (m *MockMounter) Unmount(arg0 string) error { return ret0 } -// Unmount_utils indicates an expected call of Unmount_utils. +// Unmount indicates an expected call of Unmount. func (mr *MockMounterMockRecorder) Unmount(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Unmount", reflect.TypeOf((*MockMounter)(nil).Unmount), arg0)