From 21da3f3400614e5f16a55b1c9d3a346207f87571 Mon Sep 17 00:00:00 2001 From: Matt Brock Date: Thu, 5 Dec 2024 22:59:19 -0600 Subject: [PATCH] PR feedback --- api/types/discoveryconfig/derived.gen.go | 1 - lib/srv/discovery/access_graph_azure.go | 14 ++++++ lib/srv/discovery/discovery.go | 4 ++ .../fetchers/azure-sync/azure-sync_test.go | 43 +++++++++++++++---- .../fetchers/azure-sync/roledefinitions.go | 7 +-- 5 files changed, 56 insertions(+), 13 deletions(-) diff --git a/api/types/discoveryconfig/derived.gen.go b/api/types/discoveryconfig/derived.gen.go index 9279ab6d4d824..0855486f8af8c 100644 --- a/api/types/discoveryconfig/derived.gen.go +++ b/api/types/discoveryconfig/derived.gen.go @@ -302,7 +302,6 @@ func deriveTeleportEqual_20(this, that *types.AccessGraphAzureSync) bool { this != nil && that != nil && deriveTeleportEqual_14(this.Regions, that.Regions) && this.SubscriptionID == that.SubscriptionID && - this.UMIClientID == that.UMIClientID && this.Integration == that.Integration } diff --git a/lib/srv/discovery/access_graph_azure.go b/lib/srv/discovery/access_graph_azure.go index 9ea76d1be74e1..93896c808fbdf 100644 --- 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 index c39b934b1e15d..f52a4d2255617 100644 --- a/lib/srv/discovery/discovery.go +++ b/lib/srv/discovery/discovery.go @@ -1685,6 +1685,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 index 013e7dba1b51a..7a7b709e45b35 100644 --- 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 index d47ce7623b9ea..050cb0cadc816 100644 --- 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) }