Skip to content

Commit

Permalink
Azure: Send at least one batch resource per subscription (#1484)
Browse files Browse the repository at this point in the history 
This fixes issue with Azure Bastion and Activity Log Alerts where
failing findings are not generated because there are no resources to be
evaluated when the corresponding Azure resources do not exist.

This has already been backported in 8.11 in #1475.

Fixes #1474
  • Loading branch information
@orestisfl
orestisfl authored Nov 9, 2023
1 parent 0b3a133 commit 807348d
Show file tree
Hide file tree
Showing 4 changed files with 225 additions and 59 deletions.
58 changes: 33 additions & 25 deletions resources/fetching/fetchers/azure/batch_fetcher.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,30 +57,36 @@ func (f *AzureBatchAssetFetcher) Fetch(ctx context.Context, cMetadata fetching.C
return err
}

if len(allAssets) == 0 {
return nil
}

assetGroups := lo.GroupBy(allAssets, func(item inventory.AzureAsset) string {
return fmt.Sprintf("%s-%s", item.SubscriptionId, item.Type)
subscriptionGroups := lo.GroupBy(allAssets, func(item inventory.AzureAsset) string {
return item.SubscriptionId
})

for _, assets := range assetGroups {
pair := AzureBatchAssets[assets[0].Type]

select {
case <-ctx.Done():
f.log.Infof("AzureBatchAssetFetcher.Fetch context err: %s", ctx.Err().Error())
return nil
case f.resourceCh <- fetching.ResourceInfo{
CycleMetadata: cMetadata,
Resource: &AzureBatchResource{
// Every asset in the list has the same type and subtype
Type: pair.Type,
SubType: pair.SubType,
Assets: assets,
},
}:
for subId, subName := range f.provider.GetSubscriptions() {
assetGroups := lo.GroupBy(subscriptionGroups[subId], func(item inventory.AzureAsset) string {
return item.Type
})
for assetType, pair := range AzureBatchAssets {
assets := assetGroups[assetType]
if assets == nil {
assets = []inventory.AzureAsset{} // Use empty array instead of nil
}

select {
case <-ctx.Done():
f.log.Infof("AzureBatchAssetFetcher.Fetch context err: %s", ctx.Err().Error())
return nil
case f.resourceCh <- fetching.ResourceInfo{
CycleMetadata: cMetadata,
Resource: &AzureBatchResource{
// Every asset in the list has the same type and subtype
Type: pair.Type,
SubType: pair.SubType,
SubId: subId,
SubName: subName,
Assets: assets,
},
}:
}
}
}

Expand All @@ -92,6 +98,8 @@ func (f *AzureBatchAssetFetcher) Stop() {}
type AzureBatchResource struct {
Type string
SubType string
SubId string
SubName string
Assets []inventory.AzureAsset `json:"assets,omitempty"`
}

Expand All @@ -101,7 +109,7 @@ func (r *AzureBatchResource) GetData() any {

func (r *AzureBatchResource) GetMetadata() (fetching.ResourceMetadata, error) {
// Assuming all batch in not empty includes assets of the same subscription
id := fmt.Sprintf("%s-%s", r.SubType, r.Assets[0].SubscriptionId)
id := fmt.Sprintf("%s-%s", r.SubType, r.SubId)
return fetching.ResourceMetadata{
ID: id,
Type: r.Type,
Expand All @@ -117,8 +125,8 @@ func (r *AzureBatchResource) GetElasticCommonData() (map[string]any, error) {
"cloud": map[string]any{
"provider": "azure",
"account": map[string]any{
"id": r.Assets[0].SubscriptionId,
"name": r.Assets[0].SubscriptionName,
"id": r.SubId,
"name": r.SubName,
},
// TODO: Organization fields
},
Expand Down
178 changes: 144 additions & 34 deletions resources/fetching/fetchers/azure/batch_fetcher_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ package fetchers
import (
"context"
"fmt"
"sort"
"strconv"
"testing"

"github.com/samber/lo"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/suite"
"golang.org/x/exp/maps"
Expand Down Expand Up @@ -108,6 +110,9 @@ func (s *AzureBatchAssetFetcherTestSuite) TestFetcher_Fetch() {
}
return result, nil
}).Once()
mockInventoryService.EXPECT().GetSubscriptions().Return(map[string]string{
"subId1": "subName1",
}).Once()

fetcher := AzureBatchAssetFetcher{
log: testhelper.NewLogger(s.T()),
Expand Down Expand Up @@ -161,6 +166,12 @@ func (s *AzureBatchAssetFetcherTestSuite) TestFetcher_Fetch() {
}

func (s *AzureBatchAssetFetcherTestSuite) TestFetcher_Fetch_Batches() {
subs := map[string]string{
"1": "one",
"2": "two",
"3": "three",
"4": "four",
}
var mockAssets []inventory.AzureAsset
for i, variableFields := range []struct {
sub string
Expand Down Expand Up @@ -209,75 +220,174 @@ func (s *AzureBatchAssetFetcherTestSuite) TestFetcher_Fetch_Batches() {
} {
id := strconv.Itoa(i)
mockAssets = append(mockAssets, inventory.AzureAsset{
Id: "id" + id,
Name: "name" + id,
Location: "loc" + id,
Properties: map[string]any{"key" + id: "value" + id},
ResourceGroup: "rg" + id,
SubscriptionId: variableFields.sub,
TenantId: "tenant",
Type: variableFields.tpe,
Sku: "sku" + id,
Id: "id" + id,
Name: "name" + id,
Location: "loc" + id,
Properties: map[string]any{"key" + id: "value" + id},
ResourceGroup: "rg" + id,
SubscriptionId: variableFields.sub,
SubscriptionName: subs[variableFields.sub],
TenantId: "tenant",
Type: variableFields.tpe,
Sku: "sku" + id,
})
}

mockInventoryService := inventory.NewMockServiceAPI(s.T())
mockInventoryService.EXPECT().
ListAllAssetTypesByName(mock.Anything, mock.AnythingOfType("[]string")).
Return(mockAssets, nil)
Return(mockAssets, nil).Once()
mockInventoryService.EXPECT().GetSubscriptions().Return(subs).Once()
fetcher := AzureBatchAssetFetcher{
log: testhelper.NewLogger(s.T()),
resourceCh: s.resourceCh,
provider: mockInventoryService,
}

err := fetcher.Fetch(context.Background(), fetching.CycleMetadata{})
err := fetcher.Fetch(context.Background(), fetching.CycleMetadata{Sequence: 111})
s.Require().NoError(err)
results := testhelper.CollectResources(s.resourceCh)

s.Len(results, 5)
s.ElementsMatch([]fetching.ResourceInfo{
{ // sub 1
Resource: &AzureBatchResource{
Type: fetching.MonitoringIdentity,
SubType: fetching.AzureActivityLogAlertType,
Assets: []inventory.AzureAsset{mockAssets[0], mockAssets[1]},
},
CycleMetadata: fetching.CycleMetadata{Sequence: 0},
},
{ // sub 2
sort.Slice(results, func(i, j int) bool {
a := results[i].Resource.(*AzureBatchResource)
b := results[j].Resource.(*AzureBatchResource)
if a.Type == b.Type {
s.NotEqualf(a.SubId, b.SubId, "two resources of same type %s and SubId %s", a.Type, a.SubId)
return a.SubId < b.SubId
}
return a.Type < b.Type
})
expected := []fetching.ResourceInfo{
{
Resource: &AzureBatchResource{
Type: fetching.MonitoringIdentity,
SubType: fetching.AzureActivityLogAlertType,
Assets: []inventory.AzureAsset{mockAssets[2], mockAssets[5]},
Type: fetching.CloudDns,
SubType: fetching.AzureBastionType,
SubId: "1",
SubName: "one",
Assets: []inventory.AzureAsset{mockAssets[4]},
},
CycleMetadata: fetching.CycleMetadata{Sequence: 0},
CycleMetadata: fetching.CycleMetadata{Sequence: 111},
},
{ // sub 1
{
Resource: &AzureBatchResource{
Type: fetching.CloudDns,
SubType: fetching.AzureBastionType,
Assets: []inventory.AzureAsset{mockAssets[4]},
SubId: "2",
SubName: "two",
Assets: []inventory.AzureAsset{},
},
CycleMetadata: fetching.CycleMetadata{Sequence: 0},
CycleMetadata: fetching.CycleMetadata{Sequence: 111},
},
{ // sub 3
{
Resource: &AzureBatchResource{
Type: fetching.CloudDns,
SubType: fetching.AzureBastionType,
SubId: "3",
SubName: "three",
Assets: []inventory.AzureAsset{mockAssets[3], mockAssets[6]},
},
CycleMetadata: fetching.CycleMetadata{Sequence: 0},
CycleMetadata: fetching.CycleMetadata{Sequence: 111},
},
{ // sub 4
{
Resource: &AzureBatchResource{
Type: fetching.CloudDns,
SubType: fetching.AzureBastionType,
SubId: "4",
SubName: "four",
Assets: []inventory.AzureAsset{mockAssets[7]},
},
CycleMetadata: fetching.CycleMetadata{Sequence: 0},
CycleMetadata: fetching.CycleMetadata{Sequence: 111},
},
{
Resource: &AzureBatchResource{
Type: fetching.MonitoringIdentity,
SubType: fetching.AzureActivityLogAlertType,
SubId: "1",
SubName: "one",
Assets: []inventory.AzureAsset{mockAssets[0], mockAssets[1]},
},
CycleMetadata: fetching.CycleMetadata{Sequence: 111},
},
{
Resource: &AzureBatchResource{
Type: fetching.MonitoringIdentity,
SubType: fetching.AzureActivityLogAlertType,
SubId: "2",
SubName: "two",
Assets: []inventory.AzureAsset{mockAssets[2], mockAssets[5]},
},
CycleMetadata: fetching.CycleMetadata{Sequence: 111},
},
{
Resource: &AzureBatchResource{
Type: fetching.MonitoringIdentity,
SubType: fetching.AzureActivityLogAlertType,
SubId: "3",
SubName: "three",
Assets: []inventory.AzureAsset{},
},
CycleMetadata: fetching.CycleMetadata{Sequence: 111},
},
{
Resource: &AzureBatchResource{
Type: fetching.MonitoringIdentity,
SubType: fetching.AzureActivityLogAlertType,
SubId: "4",
SubName: "four",
Assets: []inventory.AzureAsset{},
},
CycleMetadata: fetching.CycleMetadata{Sequence: 111},
},
}, results)
}
s.Equal(expected, results)

var expectedMetadata []fetching.ResourceMetadata
for i := 0; i < 8; i++ {
subType := "azure-bastion"
tpe := "cloud-dns"
if i >= 4 {
subType = "azure-activity-log-alert"
tpe = "monitoring"
}
id := fmt.Sprintf("%s-%d", subType, i%4+1)
expectedMetadata = append(expectedMetadata, fetching.ResourceMetadata{
ID: id,
Type: tpe,
SubType: subType,
Name: id,
Region: "global",
AwsAccountId: "",
AwsAccountAlias: "",
AwsOrganizationId: "",
AwsOrganizationName: "",
})
}
metadata := lo.Map(results, func(item fetching.ResourceInfo, index int) fetching.ResourceMetadata {
metadata, err := item.GetMetadata()
s.Require().NoError(err)
return metadata
})
s.Equal(expectedMetadata, metadata)

var expectedECS []map[string]any
for i := 0; i < 8; i++ {
subId := strconv.Itoa(i%4 + 1)
expectedECS = append(expectedECS, map[string]any{
"cloud": map[string]any{
"provider": "azure",
"account": map[string]any{
"id": subId,
"name": subs[subId],
},
},
})
}
ecs := lo.Map(results, func(item fetching.ResourceInfo, _ int) map[string]any {
data, err := item.GetElasticCommonData()
s.Require().NoError(err)
return data
})
s.Equal(expectedECS, ecs)
}

func findResult(results []fetching.ResourceInfo, assetType string) *fetching.ResourceInfo {
Expand Down
43 changes: 43 additions & 0 deletions resources/providers/azurelib/inventory/mock_service_api.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 807348d

Please sign in to comment.