Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
mvbrock committed Dec 17, 2024
1 parent 38b6f65 commit 98b65ba
Show file tree
Hide file tree
Showing 13 changed files with 494 additions and 564 deletions.
3 changes: 1 addition & 2 deletions api/proto/teleport/legacy/types/types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -8025,7 +8025,7 @@ message OktaOptions {
message AccessGraphSync {
// AWS is a configuration for AWS Access Graph service poll service.
repeated AccessGraphAWSSync AWS = 1 [(gogoproto.jsontag) = "aws,omitempty"];
// PollInterval is the frequency at which to poll for AWS resources
// PollInterval is the frequency at which to poll for resources
google.protobuf.Duration PollInterval = 2 [
(gogoproto.jsontag) = "poll_interval,omitempty",
(gogoproto.nullable) = false,
Expand All @@ -8047,7 +8047,6 @@ message AccessGraphAWSSync {

// AccessGraphAzureSync is a configuration for Azure Access Graph service poll service.
message AccessGraphAzureSync {
repeated string Regions = 1 [(gogoproto.jsontag) = "regions,omitempty"];
string SubscriptionID = 2 [(gogoproto.jsontag) = "subscription_id,omitempty"];
string Integration = 3 [(gogoproto.jsontag) = "integration,omitempty"];
}
939 changes: 429 additions & 510 deletions api/types/types.pb.go

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,8 @@ github.com/Azure/azure-sdk-for-go/sdk/internal v1.0.0/go.mod h1:eWRD7oawr1Mu1sLC
github.com/Azure/azure-sdk-for-go/sdk/internal v1.1.1/go.mod h1:eWRD7oawr1Mu1sLCawqVc0CUiF43ia3qQMxLscsKQ9w=
github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0 h1:ywEEhmNahHBihViHepv3xPBn1663uRv2t2q/ESv9seY=
github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0/go.mod h1:iZDifYGJTIgIIkYRNWPENUnqx6bJ2xnSDFI2tjwZNuY=
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2 v2.2.0 h1:Hp+EScFOu9HeCbeW8WU2yQPJd4gGwhMgKxWe+G6jNzw=
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2 v2.2.0/go.mod h1:/pz8dyNQe+Ey3yBp/XuYz7oqX8YDNWVpPB0hH3XWfbc=
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v6 v6.1.0 h1:zDeQI/PaWztI2tcrGO/9RIMey9NvqYbnyttf/0P3QWM=
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v6 v6.1.0/go.mod h1:zflC9v4VfViJrSvcvplqws/yGXVbUEMZi/iHpZdSPWA=
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v5 v5.0.0 h1:5n7dPVqsWfVKw+ZiEKSd3Kzu7gwBkbEBkeXb8rgaE9Q=
Expand Down
1 change: 1 addition & 0 deletions integrations/event-handler/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,7 @@ github.com/Azure/azure-sdk-for-go/sdk/azidentity/cache v0.3.0 h1:+m0M/LFxN43KvUL
github.com/Azure/azure-sdk-for-go/sdk/azidentity/cache v0.3.0/go.mod h1:PwOyop78lveYMRs6oCxjiVyBdyCgIYH6XHIVZO9/SFQ=
github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0 h1:ywEEhmNahHBihViHepv3xPBn1663uRv2t2q/ESv9seY=
github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0/go.mod h1:iZDifYGJTIgIIkYRNWPENUnqx6bJ2xnSDFI2tjwZNuY=
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2 v2.2.0/go.mod h1:/pz8dyNQe+Ey3yBp/XuYz7oqX8YDNWVpPB0hH3XWfbc=
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v6 v6.1.0 h1:zDeQI/PaWztI2tcrGO/9RIMey9NvqYbnyttf/0P3QWM=
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v6 v6.1.0/go.mod h1:zflC9v4VfViJrSvcvplqws/yGXVbUEMZi/iHpZdSPWA=
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v5 v5.0.0 h1:5n7dPVqsWfVKw+ZiEKSd3Kzu7gwBkbEBkeXb8rgaE9Q=
Expand Down
1 change: 1 addition & 0 deletions integrations/terraform/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,7 @@ github.com/Azure/azure-sdk-for-go/sdk/azidentity/cache v0.3.0 h1:+m0M/LFxN43KvUL
github.com/Azure/azure-sdk-for-go/sdk/azidentity/cache v0.3.0/go.mod h1:PwOyop78lveYMRs6oCxjiVyBdyCgIYH6XHIVZO9/SFQ=
github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0 h1:ywEEhmNahHBihViHepv3xPBn1663uRv2t2q/ESv9seY=
github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0/go.mod h1:iZDifYGJTIgIIkYRNWPENUnqx6bJ2xnSDFI2tjwZNuY=
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2 v2.2.0/go.mod h1:/pz8dyNQe+Ey3yBp/XuYz7oqX8YDNWVpPB0hH3XWfbc=
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v6 v6.1.0 h1:zDeQI/PaWztI2tcrGO/9RIMey9NvqYbnyttf/0P3QWM=
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v6 v6.1.0/go.mod h1:zflC9v4VfViJrSvcvplqws/yGXVbUEMZi/iHpZdSPWA=
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v5 v5.0.0 h1:5n7dPVqsWfVKw+ZiEKSd3Kzu7gwBkbEBkeXb8rgaE9Q=
Expand Down
2 changes: 1 addition & 1 deletion lib/cloud/azure/roleassignments.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func NewRoleAssignmentsClient(subscription string, cred azcore.TokenCredential,
// ListRoleAssignments returns role assignments for a given scope
func (c *RoleAssignmentsClient) ListRoleAssignments(ctx context.Context, scope string) ([]*armauthorization.RoleAssignment, error) {
pager := c.cli.NewListForScopePager(scope, nil)
roleDefs := make([]*armauthorization.RoleAssignment, 0, 128)
var roleDefs []*armauthorization.RoleAssignment
for pager.More() {
page, err := pager.NextPage(ctx)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion lib/cloud/azure/roledefinitions.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func NewRoleDefinitionsClient(subscription string, cred azcore.TokenCredential,
// ListRoleDefinitions returns role definitions for a given scope
func (c *RoleDefinitionsClient) ListRoleDefinitions(ctx context.Context, scope string) ([]*armauthorization.RoleDefinition, error) {
pager := c.cli.NewListPager(scope, nil)
roleDefs := make([]*armauthorization.RoleDefinition, 0, 128)
var roleDefs []*armauthorization.RoleDefinition
for pager.More() {
page, err := pager.NextPage(ctx)
if err != nil {
Expand Down
1 change: 0 additions & 1 deletion lib/srv/discovery/access_graph_azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,6 @@ func (s *Server) accessGraphAzureFetchersFromMatchers(
fetcherCfg := azure_sync.Config{
CloudClients: s.CloudClients,
SubscriptionID: matcher.SubscriptionID,
Regions: matcher.Regions,
Integration: matcher.Integration,
DiscoveryConfigName: discoveryConfigName,
}
Expand Down
8 changes: 8 additions & 0 deletions lib/srv/discovery/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -1745,6 +1745,14 @@ func (s *Server) upsertDynamicMatchers(ctx context.Context, dc *discoveryconfig.
s.dynamicTAGAWSFetchers[dc.GetName()] = awsSyncMatchers
s.muDynamicTAGAWSFetchers.Unlock()

azureSyncMatchers, err := s.accessGraphAzureFetchersFromMatchers(matchers, dc.GetName())
if err != nil {
return trace.Wrap(err)
}
s.muDynamicTAGAzureFetchers.Lock()
s.dynamicTAGAzureFetchers[dc.GetName()] = azureSyncMatchers
s.muDynamicTAGAzureFetchers.Unlock()

kubeFetchers, err := s.kubeFetchersFromMatchers(matchers, dc.GetName())
if err != nil {
return trace.Wrap(err)
Expand Down
55 changes: 27 additions & 28 deletions lib/srv/discovery/fetchers/azure-sync/azure-sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (

"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v3"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v6"
"github.com/gravitational/trace"
"golang.org/x/sync/errgroup"

Expand All @@ -41,13 +41,14 @@ const (
featNameVms = "azure/virtualmachines"
)

const FetcherConcurrency = 5
// FetcherConcurrency is an arbitrary per-resource type concurrency to ensure significant throughput. As we increase
// the number of resource types, we may increase this value or use some other approach to fetching concurrency.
const FetcherConcurrency = 4

// Config defines parameters required for fetching resources from Azure
type Config struct {
CloudClients cloud.Clients
SubscriptionID string
Regions []string
Integration string
DiscoveryConfigName string
}
Expand Down Expand Up @@ -82,9 +83,9 @@ type Fetcher struct {
lastDiscoveredResources uint64
lastResult *Resources

vmClient VirtualMachinesClient
roleDefClient RoleDefinitionsClient
roleAssignClient RoleAssignmentsClient
roleDefClient RoleDefinitionsClient
vmClient VirtualMachinesClient
}

// NewFetcher returns a new fetcher based on configuration parameters
Expand All @@ -96,15 +97,15 @@ func NewFetcher(cfg Config, ctx context.Context) (*Fetcher, error) {
}

// Create the clients
vmClient, err := azure.NewVirtualMachinesClient(cfg.SubscriptionID, cred, nil)
roleAssignClient, err := azure.NewRoleAssignmentsClient(cfg.SubscriptionID, cred, nil)
if err != nil {
return nil, trace.Wrap(err)
}
roleDefClient, err := azure.NewRoleDefinitionsClient(cfg.SubscriptionID, cred, nil)
if err != nil {
return nil, trace.Wrap(err)
}
roleAssignClient, err := azure.NewRoleAssignmentsClient(cfg.SubscriptionID, cred, nil)
vmClient, err := azure.NewVirtualMachinesClient(cfg.SubscriptionID, cred, nil)
if err != nil {
return nil, trace.Wrap(err)
}
Expand All @@ -113,35 +114,33 @@ func NewFetcher(cfg Config, ctx context.Context) (*Fetcher, error) {
return &Fetcher{
Config: cfg,
lastResult: &Resources{},
vmClient: vmClient,
roleDefClient: roleDefClient,
roleAssignClient: roleAssignClient,
roleDefClient: roleDefClient,
vmClient: vmClient,
}, nil
}

// Features is a set of booleans that are received from the Access Graph to indicate which resources it can receive
type Features struct {
Principals bool
RoleDefinitions bool
RoleAssignments bool
VirtualMachines bool
ManagedDatabases bool
AKSClusters bool
Principals bool
RoleDefinitions bool
RoleAssignments bool
VirtualMachines bool
}

// BuildFeatures builds the feature flags based on supported types returned by Access Graph Azure endpoints.
func BuildFeatures(values ...string) Features {
features := Features{}
for _, value := range values {
switch value {
case featNameVms:
features.VirtualMachines = true
case featNamePrincipals:
features.Principals = true
case featNameRoleDefinitions:
features.RoleDefinitions = true
case featNameRoleAssignments:
features.RoleAssignments = true
features.AKSClusters = true
case featNameRoleDefinitions:
features.RoleDefinitions = true
case featNameVms:
features.VirtualMachines = true
}
}
return features
Expand All @@ -153,10 +152,10 @@ func (a *Fetcher) Poll(ctx context.Context, feats Features) (*Resources, error)
if res == nil {
return nil, err
}
res.VirtualMachines = common.DeduplicateSlice(res.VirtualMachines, azureVmKey)
res.Principals = common.DeduplicateSlice(res.Principals, azurePrincipalsKey)
res.RoleDefinitions = common.DeduplicateSlice(res.RoleDefinitions, azureRoleDefKey)
res.RoleAssignments = common.DeduplicateSlice(res.RoleAssignments, azureRoleAssignKey)
res.RoleDefinitions = common.DeduplicateSlice(res.RoleDefinitions, azureRoleDefKey)
res.VirtualMachines = common.DeduplicateSlice(res.VirtualMachines, azureVmKey)
return res, trace.Wrap(err)
}

Expand All @@ -179,25 +178,25 @@ func (a *Fetcher) fetch(ctx context.Context, feats Features) (*Resources, error)
return nil
})
}
if feats.RoleDefinitions {
if feats.RoleAssignments {
eg.Go(func() error {
roleDefs, err := a.fetchRoleDefinitions(ctx)
roleAssigns, err := a.fetchRoleAssignments(ctx)
if err != nil {
errsCh <- err
return err
}
result.RoleDefinitions = roleDefs
result.RoleAssignments = roleAssigns
return nil
})
}
if feats.RoleAssignments {
if feats.RoleDefinitions {
eg.Go(func() error {
roleAssigns, err := a.fetchRoleAssignments(ctx)
roleDefs, err := a.fetchRoleDefinitions(ctx)
if err != nil {
errsCh <- err
return err
}
result.RoleAssignments = roleAssigns
result.RoleDefinitions = roleDefs
return nil
})
}
Expand Down
4 changes: 2 additions & 2 deletions lib/srv/discovery/fetchers/azure-sync/azure-sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
"testing"

"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v3"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v6"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -57,7 +57,7 @@ type testVmCli struct {
vms []*armcompute.VirtualMachine
}

func (t testVmCli) ListVirtualMachines(ctx context.Context, scope string) ([]*armcompute.VirtualMachine, error) {
func (t testVmCli) ListVirtualMachines(ctx context.Context, resourceGroup string) ([]*armcompute.VirtualMachine, error) {
if t.returnErr {
return nil, fmt.Errorf("error")
}
Expand Down
36 changes: 18 additions & 18 deletions lib/srv/discovery/fetchers/azure-sync/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,14 @@ func MergeResources(results ...*Resources) *Resources {
result := &Resources{}
for _, r := range results {
result.Principals = append(result.Principals, r.Principals...)
result.VirtualMachines = append(result.VirtualMachines, r.VirtualMachines...)
result.RoleDefinitions = append(result.RoleDefinitions, r.RoleDefinitions...)
result.RoleAssignments = append(result.RoleAssignments, r.RoleAssignments...)
result.RoleDefinitions = append(result.RoleDefinitions, r.RoleDefinitions...)
result.VirtualMachines = append(result.VirtualMachines, r.VirtualMachines...)
}
result.Principals = common.DeduplicateSlice(result.Principals, azurePrincipalsKey)
result.VirtualMachines = common.DeduplicateSlice(result.VirtualMachines, azureVmKey)
result.RoleDefinitions = common.DeduplicateSlice(result.RoleDefinitions, azureRoleDefKey)
result.RoleAssignments = common.DeduplicateSlice(result.RoleAssignments, azureRoleAssignKey)
result.RoleDefinitions = common.DeduplicateSlice(result.RoleDefinitions, azureRoleDefKey)
result.VirtualMachines = common.DeduplicateSlice(result.VirtualMachines, azureVmKey)
return result
}

Expand All @@ -61,10 +61,10 @@ func newResourceList() *accessgraphv1alpha.AzureResourceList {
func ReconcileResults(old *Resources, new *Resources) (upsert, delete *accessgraphv1alpha.AzureResourceList) {
upsert, delete = newResourceList(), newResourceList()
reconciledResources := []*reconcilePair{
reconcile(old.VirtualMachines, new.VirtualMachines, azureVmKey, azureVmWrap),
reconcile(old.Principals, new.Principals, azurePrincipalsKey, azurePrincipalsWrap),
reconcile(old.RoleDefinitions, new.RoleDefinitions, azureRoleDefKey, azureRoleDefWrap),
reconcile(old.RoleAssignments, new.RoleAssignments, azureRoleAssignKey, azureRoleAssignWrap),
reconcile(old.RoleDefinitions, new.RoleDefinitions, azureRoleDefKey, azureRoleDefWrap),
reconcile(old.VirtualMachines, new.VirtualMachines, azureVmKey, azureVmWrap),
}
for _, res := range reconciledResources {
upsert.Resources = append(upsert.Resources, res.upsert.Resources...)
Expand Down Expand Up @@ -132,14 +132,6 @@ func reconcile[T proto.Message](
return &reconcilePair{upsertRes, deleteRes}
}

func azureVmKey(vm *accessgraphv1alpha.AzureVirtualMachine) string {
return fmt.Sprintf("%s:%s", vm.SubscriptionId, vm.Id)
}

func azureVmWrap(vm *accessgraphv1alpha.AzureVirtualMachine) *accessgraphv1alpha.AzureResource {
return &accessgraphv1alpha.AzureResource{Resource: &accessgraphv1alpha.AzureResource_VirtualMachine{VirtualMachine: vm}}
}

func azurePrincipalsKey(user *accessgraphv1alpha.AzurePrincipal) string {
return fmt.Sprintf("%s:%s", user.SubscriptionId, user.Id)
}
Expand All @@ -148,6 +140,14 @@ func azurePrincipalsWrap(principal *accessgraphv1alpha.AzurePrincipal) *accessgr
return &accessgraphv1alpha.AzureResource{Resource: &accessgraphv1alpha.AzureResource_Principal{Principal: principal}}
}

func azureRoleAssignKey(roleAssign *accessgraphv1alpha.AzureRoleAssignment) string {
return fmt.Sprintf("%s:%s", roleAssign.SubscriptionId, roleAssign.Id)
}

func azureRoleAssignWrap(roleAssign *accessgraphv1alpha.AzureRoleAssignment) *accessgraphv1alpha.AzureResource {
return &accessgraphv1alpha.AzureResource{Resource: &accessgraphv1alpha.AzureResource_RoleAssignment{RoleAssignment: roleAssign}}
}

func azureRoleDefKey(roleDef *accessgraphv1alpha.AzureRoleDefinition) string {
return fmt.Sprintf("%s:%s", roleDef.SubscriptionId, roleDef.Id)
}
Expand All @@ -156,10 +156,10 @@ func azureRoleDefWrap(roleDef *accessgraphv1alpha.AzureRoleDefinition) *accessgr
return &accessgraphv1alpha.AzureResource{Resource: &accessgraphv1alpha.AzureResource_RoleDefinition{RoleDefinition: roleDef}}
}

func azureRoleAssignKey(roleAssign *accessgraphv1alpha.AzureRoleAssignment) string {
return fmt.Sprintf("%s:%s", roleAssign.SubscriptionId, roleAssign.Id)
func azureVmKey(vm *accessgraphv1alpha.AzureVirtualMachine) string {
return fmt.Sprintf("%s:%s", vm.SubscriptionId, vm.Id)
}

func azureRoleAssignWrap(roleAssign *accessgraphv1alpha.AzureRoleAssignment) *accessgraphv1alpha.AzureResource {
return &accessgraphv1alpha.AzureResource{Resource: &accessgraphv1alpha.AzureResource_RoleAssignment{RoleAssignment: roleAssign}}
func azureVmWrap(vm *accessgraphv1alpha.AzureVirtualMachine) *accessgraphv1alpha.AzureResource {
return &accessgraphv1alpha.AzureResource{Resource: &accessgraphv1alpha.AzureResource_VirtualMachine{VirtualMachine: vm}}
}
4 changes: 3 additions & 1 deletion lib/srv/discovery/fetchers/azure-sync/virtualmachines.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,10 @@ import (
accessgraphv1alpha "github.com/gravitational/teleport/gen/proto/go/accessgraph/v1alpha"
)

const allResourceGroups = "*"

func (a *Fetcher) fetchVirtualMachines(ctx context.Context) ([]*accessgraphv1alpha.AzureVirtualMachine, error) {
vms, err := a.vmClient.ListVirtualMachines(ctx, "*")
vms, err := a.vmClient.ListVirtualMachines(ctx, allResourceGroups)
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down

0 comments on commit 98b65ba

Please sign in to comment.