PR feedback

gravitational · Dec 6, 2024 · a386665 · a386665
1 parent d8e28f9
commit a386665
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 13 deletions.
diff --git a/api/types/discoveryconfig/derived.gen.go b/api/types/discoveryconfig/derived.gen.go
diff --git a/lib/srv/discovery/access_graph_azure.go b/lib/srv/discovery/access_graph_azure.go
@@ -305,6 +305,20 @@ func (s *Server) initializeAndWatchAzureAccessGraph(ctx context.Context, reloadC
 		}
 	}()
 
+	// Configure the poll interval
+	tickerInterval := defaultPollInterval
+	if s.Config.Matchers.AccessGraph != nil {
+		if s.Config.Matchers.AccessGraph.PollInterval > defaultPollInterval {
+			tickerInterval = s.Config.Matchers.AccessGraph.PollInterval
+		} else {
+			s.Log.WarnContext(ctx,
+				"Access graph Azure service poll interval cannot be less than the default",
+				"default_poll_interval",
+				defaultPollInterval)
+		}
+	}
+	s.Log.InfoContext(ctx, "Access graph Azure service poll interval", "poll_interval", tickerInterval)
+
 	// Reconciles the resources as they're imported from Azure
 	azureResources := &azure_sync.Resources{}
 	ticker := time.NewTicker(15 * time.Minute)

diff --git a/lib/srv/discovery/discovery.go b/lib/srv/discovery/discovery.go
@@ -1669,6 +1669,10 @@ func (s *Server) deleteDynamicFetchers(name string) {
 	delete(s.dynamicTAGAWSFetchers, name)
 	s.muDynamicTAGAWSFetchers.Unlock()
 
+	s.muDynamicTAGAzureFetchers.Lock()
+	delete(s.dynamicTAGAzureFetchers, name)
+	s.muDynamicTAGAzureFetchers.Unlock()
+
 	s.muDynamicKubeFetchers.Lock()
 	delete(s.dynamicKubeFetchers, name)
 	s.muDynamicKubeFetchers.Unlock()

diff --git a/lib/srv/discovery/fetchers/azure-sync/azure-sync_test.go b/lib/srv/discovery/fetchers/azure-sync/azure-sync_test.go
@@ -66,12 +66,22 @@ func (t testVmCli) ListVirtualMachines(ctx context.Context, scope string) ([]*ar
 
 func newRoleDef(id string, name string) *armauthorization.RoleDefinition {
 	role_name := "test_role_name"
+	action1 := "Microsoft.Compute/virtualMachines/read"
+	action2 := "Microsoft.Compute/virtualMachines/*"
+	action3 := "Microsoft.Compute/*"
 	return &armauthorization.RoleDefinition{
 		ID:   &id,
 		Name: &name,
 		Properties: &armauthorization.RoleDefinitionProperties{
-			Permissions: []*armauthorization.Permission{},
-			RoleName:    &role_name,
+			Permissions: []*armauthorization.Permission{
+				{
+					Actions: []*string{&action1, &action2},
+				},
+				{
+					Actions: []*string{&action3},
+				},
+			},
+			RoleName: &role_name,
 		},
 	}
 }
@@ -112,7 +122,7 @@ func TestPoll(t *testing.T) {
 	roleAssignClient := testRoleAssignCli{}
 	vmClient := testVmCli{}
 	fetcher := Fetcher{
-		Config:           Config{},
+		Config:           Config{SubscriptionID: "1234567890"},
 		lastResult:       &Resources{},
 		roleDefClient:    &roleDefClient,
 		roleAssignClient: &roleAssignClient,
@@ -189,14 +199,29 @@ func TestPoll(t *testing.T) {
 
 		// Verify the results, based on the features set
 		require.NotNil(t, resources)
-		if tt.feats.RoleDefinitions {
-			require.Len(t, resources.RoleDefinitions, len(tt.roleDefs))
+		require.Equal(t, tt.feats.RoleDefinitions == false || len(tt.roleDefs) == 0, len(resources.RoleDefinitions) == 0)
+		for idx, resource := range resources.RoleDefinitions {
+			roleDef := tt.roleDefs[idx]
+			require.Equal(t, *roleDef.ID, resource.Id)
+			require.Equal(t, fetcher.SubscriptionID, resource.SubscriptionId)
+			require.Equal(t, *roleDef.Properties.RoleName, resource.Name)
+			require.Len(t, roleDef.Properties.Permissions, len(resource.Permissions))
 		}
-		if tt.feats.RoleAssignments {
-			require.Len(t, resources.RoleAssignments, len(tt.roleAssigns))
+		require.Equal(t, tt.feats.RoleAssignments == false || len(tt.roleAssigns) == 0, len(resources.RoleAssignments) == 0)
+		for idx, resource := range resources.RoleAssignments {
+			roleAssign := tt.roleAssigns[idx]
+			require.Equal(t, *roleAssign.ID, resource.Id)
+			require.Equal(t, fetcher.SubscriptionID, resource.SubscriptionId)
+			require.Equal(t, *roleAssign.Properties.PrincipalID, resource.PrincipalId)
+			require.Equal(t, *roleAssign.Properties.RoleDefinitionID, resource.RoleDefinitionId)
+			require.Equal(t, *roleAssign.Properties.Scope, resource.Scope)
 		}
-		if tt.feats.VirtualMachines {
-			require.Len(t, resources.VirtualMachines, len(tt.vms))
+		require.Equal(t, tt.feats.VirtualMachines == false || len(tt.vms) == 0, len(resources.VirtualMachines) == 0)
+		for idx, resource := range resources.VirtualMachines {
+			vm := tt.vms[idx]
+			require.Equal(t, *vm.ID, resource.Id)
+			require.Equal(t, fetcher.SubscriptionID, resource.SubscriptionId)
+			require.Equal(t, *vm.Name, resource.Name)
 		}
 	}
 }
diff --git a/lib/srv/discovery/fetchers/azure-sync/roledefinitions.go b/lib/srv/discovery/fetchers/azure-sync/roledefinitions.go
@@ -38,10 +38,11 @@ func (a *Fetcher) fetchRoleDefinitions(ctx context.Context) ([]*accessgraphv1alp
 	// Convert to protobuf format
 	pbRoleDefs := make([]*accessgraphv1alpha.AzureRoleDefinition, 0, len(roleDefs))
 	for _, roleDef := range roleDefs {
-		pbPerms := make([]*accessgraphv1alpha.AzureRBACPermission, len(roleDef.Properties.Permissions))
+		pbPerms := make([]*accessgraphv1alpha.AzureRBACPermission, 0, len(roleDef.Properties.Permissions))
 		for _, perm := range roleDef.Properties.Permissions {
 			pbPerm := accessgraphv1alpha.AzureRBACPermission{
-				Actions: ptrsToList(perm.Actions),
+				Actions:    ptrsToList(perm.Actions),
+				NotActions: ptrsToList(perm.NotActions),
 			}
 			pbPerms = append(pbPerms, &pbPerm)
 		}
@@ -58,7 +59,7 @@ func (a *Fetcher) fetchRoleDefinitions(ctx context.Context) ([]*accessgraphv1alp
 }
 
 func ptrsToList(ptrs []*string) []string {
-	strList := make([]string, len(ptrs))
+	strList := make([]string, 0, len(ptrs))
 	for _, ptr := range ptrs {
 		strList = append(strList, *ptr)
 	}