Repository: clouditor/clouditor
Pull request #330 (2021-10-04 10:11:20)

Azure storage fix
Split Azure storage into object, file and block storage. 

Based on issue #285 

codecov[bot]: # [Codecov](https://codecov.io/gh/clouditor/clouditor/pull/330?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor) Report
> Merging [#330](https://codecov.io/gh/clouditor/clouditor/pull/330?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor) (941cb84) into [main](https://codecov.io/gh/clouditor/clouditor/commit/55697319591eaf5b69ac560a759a6e24b7d300c9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor) (5569731) will **decrease** coverage by `0.07%`.
> The diff coverage is `87.23%`.

> :exclamation: Current head 941cb84 differs from pull request most recent head 0c33e94. Consider uploading reports for the commit 0c33e94 to get more accurate results
[![Impacted file tree graph](https://codecov.io/gh/clouditor/clouditor/pull/330/graphs/tree.svg?width=650&height=150&src=pr&token=syrXsw5LTX&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor)](https://codecov.io/gh/clouditor/clouditor/pull/330?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor)

```diff
@@            Coverage Diff             @@
##             main     #330      +/-   ##
==========================================
- Coverage   63.17%   63.09%   -0.08%     
==========================================
  Files          35       35              
  Lines        1974     2065      +91     
==========================================
+ Hits         1247     1303      +56     
- Misses        618      647      +29     
- Partials      109      115       +6     
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `63.09% <87.23%> (-0.08%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/clouditor/clouditor/pull/330?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor) | Coverage Δ | |
|---|---|---|
| [service/discovery/azure/storage.go](https://codecov.io/gh/clouditor/clouditor/pull/330/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor#diff-c2VydmljZS9kaXNjb3ZlcnkvYXp1cmUvc3RvcmFnZS5nbw==) | `69.28% <87.23%> (-14.39%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/clouditor/clouditor/pull/330?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/clouditor/clouditor/pull/330?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor). Last update [5569731...0c33e94](https://codecov.io/gh/clouditor/clouditor/pull/330?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor).


Patch:
diff --git a/service/discovery/azure/storage.go b/service/discovery/azure/storage.go
index 5de57f12..6e225282 100644
--- a/service/discovery/azure/storage.go
+++ b/service/discovery/azure/storage.go
@@ -26,11 +26,11 @@
 package azure
 
 import (
-	"context"
-	"fmt"
-
 	"clouditor.io/clouditor/api/discovery"
 	"clouditor.io/clouditor/voc"
+	"context"
+	"fmt"
+	"github.com/Azure/azure-sdk-for-go/profiles/2020-09-01/compute/mgmt/compute"
 	"github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2021-02-01/storage"
 	"github.com/Azure/go-autorest/autorest/to"
 )
@@ -90,23 +90,123 @@ func (d *azureStorageDiscovery) discoverStorageAccounts() ([]voc.IsCloudResource
 
 	accounts := result.Values()
 	for i := range accounts {
-		s := handleStorageAccount(&accounts[i])
+		// Discover object storages
+		objectStorages, err := d.discoverObjectStorages(&accounts[i])
+		if err != nil {
+			return nil, fmt.Errorf("could not handle object storages: %w", err)
+		}
+		
+		// Discover file storages
+		fileStorages, err := d.discoverFileStorages(&accounts[i])
+		if err != nil {
+			return nil, fmt.Errorf("could not handle file storages: %w", err)
+		}
+
+		//Discover block storages
+		blockStorages, err := d.discoverBlockStorages()
+		if err != nil {
+			return nil, fmt.Errorf("could not handle block storages: %w", err)
+		}
 
-		log.Infof("Adding storage account %+v", s)
 
-		list = append(list, s)
+		log.Infof("Adding storage account %+v", objectStorages)
+
+		list = append(list, objectStorages...)
+		list = append(list, fileStorages...)
+		list = append(list, blockStorages...)
 	}
 
 	return list, err
 }
 
-func handleStorageAccount(account *storage.Account) voc.IsStorage {
+func (d *azureStorageDiscovery) discoverBlockStorages() ([]voc.IsCloudResource, error) {
+	var list []voc.IsCloudResource
+
+	client := compute.NewDisksClient(to.String(d.sub.SubscriptionID))
+	d.apply(&client.Client)
+
+	result, err := client.ListComplete(context.Background())
+	if err != nil {
+		return nil, fmt.Errorf("could not list block storages: %w", err)
+	}
+
+	for _, disk := range *result.Response().Value {
+		blockStorages := handleBlockStorage(disk)
+		log.Infof("Adding block storage %+v", blockStorages)
+
+		list = append(list, blockStorages)
+	}
+
+	return list, nil
+}
+
+func (d *azureStorageDiscovery) discoverFileStorages(account *storage.Account) ([]voc.IsCloudResource, error) {
+	var list []voc.IsCloudResource
+
+	client := storage.NewFileSharesClient(to.String(d.sub.SubscriptionID))
+	d.apply(&client.Client)
 
+	result, err := client.List(context.Background(), GetResourceGroupName(*account.ID), *account.Name, "", "", "")
+	if err != nil {
+	return nil, fmt.Errorf("could not list file storages: %w", err)
+	}
+
+	for _, value := range result.Values() {
+		fileStorages := handleFileStorage(account, value)
+		log.Infof("Adding file storage %+v", fileStorages)
+
+		list = append(list, fileStorages)
+	}
+
+	return list, nil
+}
+
+func (d *azureStorageDiscovery) discoverObjectStorages(account *storage.Account) ([]voc.IsCloudResource, error) {
+	var list []voc.IsCloudResource
+
+	client := storage.NewBlobContainersClient(to.String(d.sub.SubscriptionID))
+	d.apply(&client.Client)
+
+	result, err := client.List(context.Background(), GetResourceGroupName(*account.ID), *account.Name, "", "", "")
+	if err != nil {
+		return nil, fmt.Errorf("could not list object storages: %w", err)
+	}
+
+	for _, value := range result.Values() {
+		objectStorages := handleObjectStorage(account, value)
+		log.Infof("Adding object storage %+v", objectStorages)
+
+		list = append(list, objectStorages)
+	}
+
+	return list, nil
+}
+
+func handleBlockStorage(disk compute.Disk) voc.IsStorage {
+
+	return &voc.BlockStorage{
+		Storage: &voc.Storage{
+			CloudResource: &voc.CloudResource{
+				ID:           voc.ResourceID(to.String(disk.ID)),
+				Name:         to.String(disk.Name),
+				CreationTime: disk.TimeCreated.Unix(),
+				Type:         []string{"BlockStorage", "Storage", "Resource"},
+			},
+			AtRestEncryption: &voc.AtRestEncryption{
+				KeyManager: string(disk.Encryption.Type), // TODO(all): What do we do with the encryption type? Do we leave it like that?
+				Enabled:    true, // is always enabled
+				Algorithm: "", // not available
+			},
+		},
+	}
+}
+
+func handleObjectStorage(account *storage.Account, container storage.ListContainerItem) voc.IsStorage {
 	return &voc.ObjectStorage{
 		Storage: &voc.Storage{
 			CloudResource: &voc.CloudResource{
-				ID:           voc.ResourceID(to.String(account.ID)),
-				Name:         to.String(account.Name),
+				ID:           voc.ResourceID(to.String(container.ID)),
+				Name:         to.String(container.Name),
 				CreationTime: account.CreationTime.Unix(),
 				Type:         []string{"ObjectStorage", "Storage", "Resource"},
 			},
@@ -117,7 +217,7 @@ func handleStorageAccount(account *storage.Account) voc.IsStorage {
 			},
 		},
 		HttpEndpoint: &voc.HttpEndpoint{
-			Url: to.String(account.PrimaryEndpoints.Blob),
+			Url: to.String(account.PrimaryEndpoints.Blob) + to.String(container.Name),
 			TransportEncryption: &voc.TransportEncryption{
 				Enforced:   to.Bool(account.EnableHTTPSTrafficOnly),
 				Enabled:    true, // cannot be disabled
@@ -127,3 +227,31 @@ func handleStorageAccount(account *storage.Account) voc.IsStorage {
 		},
 	}
 }
+
+func handleFileStorage(account *storage.Account, fileshare storage.FileShareItem) voc.IsStorage {
+	return &voc.FileStorage{
+		Storage: &voc.Storage{
+			CloudResource: &voc.CloudResource{
+				ID:           voc.ResourceID(to.String(fileshare.ID)),
+				Name:         to.String(fileshare.Name),
+				CreationTime: account.CreationTime.Unix(),
+				Type:         []string{"FileStorage", "Storage", "Resource"},
+			},
+			AtRestEncryption: &voc.AtRestEncryption{
+				KeyManager: string(account.Encryption.KeySource),
+				Algorithm:  "AES-265", // seems to be always AES-256
+				Enabled:    to.Bool(account.Encryption.Services.File.Enabled),
+			},
+		},
+		// TODO(all) Uncomment as soon as the HttpEndpoint is added to voc/file_storage.go
+		//HttpEndpoint: &voc.HttpEndpoint{
+		//	Url: to.String(account.PrimaryEndpoints.File) + to.String(fileshare.Name),
+		//	TransportEncryption: &voc.TransportEncryption{
+		//		Enforced:   to.Bool(account.EnableHTTPSTrafficOnly),
+		//		Enabled:    true, // cannot be disabled
+		//		TlsVersion: string(account.MinimumTLSVersion),
+		//		Algorithm:  "", // TBD
+		//	},
+		//},
+	}
+}


Test patch:
diff --git a/service/discovery/azure/storage_test.go b/service/discovery/azure/storage_test.go
index 09bfe20b..49cabf57 100644
--- a/service/discovery/azure/storage_test.go
+++ b/service/discovery/azure/storage_test.go
@@ -26,11 +26,11 @@
 package azure_test
 
 import (
+	"clouditor.io/clouditor/voc"
 	"net/http"
 	"testing"
 
 	"clouditor.io/clouditor/service/discovery/azure"
-	"clouditor.io/clouditor/voc"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -70,6 +70,65 @@ func (m mockStorageSender) Do(req *http.Request) (res *http.Response, err error)
 				},
 			},
 		}, 200)
+	} else if req.URL.Path == "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/res1/providers/Microsoft.Storage/storageAccounts/account1/blobServices/default/containers" {
+		return createResponse(map[string]interface{}{
+			"value": &[]map[string]interface{}{
+				{
+					"id":   "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/res1/providers/Microsoft.Storage/storageAccounts/account1/blobServices/default/containers/container1",
+					"name": "container1",
+					"type": "Microsoft.Storage/storageAccounts/blobServices/containers",
+				},
+				{
+					"id":   "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/res1/providers/Microsoft.Storage/storageAccounts/account1/blobServices/default/containers/container2",
+					"name": "container2",
+					"type": "Microsoft.Storage/storageAccounts/blobServices/containers",
+				},
+			},
+		}, 200)
+	} else if req.URL.Path == "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/res1/providers/Microsoft.Storage/storageAccounts/account1/fileServices/default/shares" {
+		return createResponse(map[string]interface{}{
+			"value": &[]map[string]interface{}{
+				{
+					"id":   "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/res1/providers/Microsoft.Storage/storageAccounts/account1/blobServices/default/shares/fileshare1",
+					"name": "fileshare1",
+					"type": "Microsoft.Storage/storageAccounts/fileServices/shares",
+				},
+				{
+					"id":   "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/res1/providers/Microsoft.Storage/storageAccounts/account1/blobServices/default/containers/fileshare2",
+					"name": "fileshare2",
+					"type": "Microsoft.Storage/storageAccounts/fileServices/shares",
+				},
+			},
+		}, 200)
+	} else if req.URL.Path == "/subscriptions/00000000-0000-0000-0000-000000000000/providers/Microsoft.Compute/disks" {
+		return createResponse(map[string]interface{}{
+			"value": &[]map[string]interface{}{
+				{
+					"id":   "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/res1/providers/Microsoft.Compute/disks/disk1",
+					"name": "disk1",
+					"type": "Microsoft.Compute/disks",
+					"properties": map[string]interface{}{
+						"timeCreated": "2017-05-24T13:28:53.4540398Z",
+						"encryption": map[string]interface{}{
+							"diskEncryptionSetID": nil,
+							"type":                "EncryptionAtRestWithPlatformKey",
+						},
+					},
+				},
+				{
+					"id":   "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/res1/providers/Microsoft.Compute/disks/disk2",
+					"name": "disk2",
+					"type": "Microsoft.Compute/disks",
+					"properties": map[string]interface{}{
+						"timeCreated": "2017-05-24T13:28:53.4540398Z",
+						"encryption": map[string]interface{}{
+							"diskEncryptionSetID": nil,
+							"type":                "EncryptionAtRestWithPlatformKey",
+						},
+					},
+				},
+			},
+		}, 200)
 	}
 
 	return m.mockSender.Do(req)
@@ -85,10 +144,112 @@ func TestListStorage(t *testing.T) {
 
 	assert.Nil(t, err)
 	assert.NotNil(t, list)
-	assert.Equal(t, 1, len(list))
+	assert.Equal(t, 6, len(list))
+}
+
+func TestListObjectStorage(t *testing.T) {
+	d := azure.NewAzureStorageDiscovery(
+		azure.WithSender(&mockStorageSender{}),
+		azure.WithAuthorizer(&mockAuthorizer{}),
+	)
+
+	list, err := d.List()
+	assert.Nil(t, err)
+	assert.NotNil(t, list)
+
+	var counter int
+	for _, item := range list {
+		if item.GetType()[0] == "ObjectStorage" {
+			counter++
+		}
+	}
+	assert.Equal(t, 2, counter)
+}
+
+func TestGetObjectStorage(t *testing.T) {
+	d := azure.NewAzureStorageDiscovery(
+		azure.WithSender(&mockStorageSender{}),
+		azure.WithAuthorizer(&mockAuthorizer{}),
+	)
+
+	list, err := d.List()
+	assert.Nil(t, err)
+	assert.NotNil(t, list)
 
 	storage, ok := list[0].(*voc.ObjectStorage)
 
 	assert.True(t, ok)
-	assert.Equal(t, "account1", storage.Name)
+	assert.Equal(t, "container1", storage.Name)
+}
+
+func TestListFileStorage(t *testing.T) {
+	d := azure.NewAzureStorageDiscovery(
+		azure.WithSender(&mockStorageSender{}),
+		azure.WithAuthorizer(&mockAuthorizer{}),
+	)
+
+	list, err := d.List()
+	assert.Nil(t, err)
+	assert.NotNil(t, list)
+
+	var counter int
+	for _, item := range list {
+		if item.GetType()[0] == "FileStorage" {
+			counter++
+		}
+	}
+	assert.Equal(t, 2, counter)
+}
+
+func TestGetFileStorage(t *testing.T) {
+	d := azure.NewAzureStorageDiscovery(
+		azure.WithSender(&mockStorageSender{}),
+		azure.WithAuthorizer(&mockAuthorizer{}),
+	)
+
+	list, err := d.List()
+	assert.Nil(t, err)
+	assert.NotNil(t, list)
+
+	storage, ok := list[2].(*voc.FileStorage)
+
+	assert.True(t, ok)
+	assert.Equal(t, "fileshare1", storage.Name)
 }
+
+func TestListBlockStorage(t *testing.T) {
+	d := azure.NewAzureStorageDiscovery(
+		azure.WithSender(&mockStorageSender{}),
+		azure.WithAuthorizer(&mockAuthorizer{}),
+	)
+
+	list, err := d.List()
+	assert.Nil(t, err)
+	assert.NotNil(t, list)
+
+	var counter int
+	for _, item := range list {
+		if item.GetType()[0] == "BlockStorage" {
+			counter++
+		}
+	}
+	assert.Equal(t, 2, counter)
+}
+
+
+func TestGetBlockStorage(t *testing.T) {
+	d := azure.NewAzureStorageDiscovery(
+		azure.WithSender(&mockStorageSender{}),
+		azure.WithAuthorizer(&mockAuthorizer{}),
+	)
+
+	list, err := d.List()
+	assert.Nil(t, err)
+	assert.NotNil(t, list)
+
+	storage, ok := list[4].(*voc.BlockStorage)
+
+	assert.True(t, ok)
+	assert.Equal(t, "disk1", storage.Name)
+}
+