Skip to content

Commit 05051db

Browse files
kanskans
committed
fixed infinite loop
1 parent 50a3065 commit 05051db

File tree

3 files changed

+181
-212
lines changed

3 files changed

+181
-212
lines changed

‎pkg/sync/expand/graph.go‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -39,17 +39,17 @@ type Node struct {
3939
// This is because the graph can have cycles, and we address them by reducing
4040
// _all_ nodes in a cycle into a single node.
4141
type EntitlementGraph struct {
42-
NextNodeID int `json:"next_node_id"` // Automatically incremented so that each node has a unique ID.
43-
NextEdgeID int `json:"next_edge_id"` // Automatically incremented so that each edge has a unique ID.
44-
Nodes map[int]Node `json:"nodes"` // The mapping of all node IDs to nodes.
45-
EntitlementsToNodes map[string]int `json:"entitlements_to_nodes"` // Internal mapping of entitlements to nodes for quicker lookup.
46-
SourcesToDestinations map[int]map[int]int `json:"sources_to_destinations"` // Internal mapping of outgoing edges by node ID.
47-
DestinationsToSources map[int]map[int]int `json:"destinations_to_sources"` // Internal mapping of incoming edges by node ID.
48-
Edges map[int]Edge `json:"edges"` // Adjacency list. Source node -> descendant node
49-
Loaded bool `json:"loaded"`
50-
Depth int `json:"depth"`
51-
Actions []EntitlementGraphAction `json:"actions"`
52-
HasNoCycles bool `json:"has_no_cycles"`
42+
NextNodeID int `json:"next_node_id"` // Automatically incremented so that each node has a unique ID.
43+
NextEdgeID int `json:"next_edge_id"` // Automatically incremented so that each edge has a unique ID.
44+
Nodes map[int]Node `json:"nodes"` // The mapping of all node IDs to nodes.
45+
EntitlementsToNodes map[string]int `json:"entitlements_to_nodes"` // Internal mapping of entitlements to nodes for quicker lookup.
46+
SourcesToDestinations map[int]map[int]int `json:"sources_to_destinations"` // Internal mapping of outgoing edges by node ID.
47+
DestinationsToSources map[int]map[int]int `json:"destinations_to_sources"` // Internal mapping of incoming edges by node ID.
48+
Edges map[int]Edge `json:"edges"` // Adjacency list. Source node -> descendant node
49+
Loaded bool `json:"loaded"`
50+
Depth int `json:"depth"`
51+
Actions []*EntitlementGraphAction `json:"actions"`
52+
HasNoCycles bool `json:"has_no_cycles"`
5353
}
5454

5555
func NewEntitlementGraph(_ context.Context) *EntitlementGraph {

‎pkg/sync/syncer.go‎

Lines changed: 96 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -852,9 +852,99 @@ func (s *syncer) SyncAssets(ctx context.Context) error {
852852
func (s *syncer) SyncGrantExpansion(ctx context.Context) error {
853853
l := ctxzap.Extract(ctx)
854854
entitlementGraph := s.state.EntitlementGraph(ctx)
855-
fmt.Printf("%v\n", entitlementGraph)
855+
if !entitlementGraph.Loaded {
856+
pageToken := s.state.PageToken(ctx)
857+
858+
if pageToken == "" {
859+
l.Info("Expanding grants...")
860+
s.handleInitialActionForStep(ctx, *s.state.Current())
861+
}
862+
863+
resp, err := s.store.ListGrants(ctx, &v2.GrantsServiceListGrantsRequest{PageToken: pageToken})
864+
if err != nil {
865+
return err
866+
}
867+
868+
// We want to take action on the next page before we push any new actions
869+
if resp.NextPageToken != "" {
870+
err = s.state.NextPage(ctx, resp.NextPageToken)
871+
if err != nil {
872+
return err
873+
}
874+
} else {
875+
l.Info("Finished loading entitlement graph", zap.Int("edges", len(entitlementGraph.Edges)))
876+
entitlementGraph.Loaded = true
877+
}
878+
879+
for _, grant := range resp.List {
880+
annos := annotations.Annotations(grant.Annotations)
881+
expandable := &v2.GrantExpandable{}
882+
_, err := annos.Pick(expandable)
883+
if err != nil {
884+
return err
885+
}
886+
if len(expandable.GetEntitlementIds()) == 0 {
887+
continue
888+
}
889+
890+
principalID := grant.GetPrincipal().GetId()
891+
if principalID == nil {
892+
return fmt.Errorf("principal id was nil")
893+
}
894+
895+
// FIXME(morgabra) Log and skip some of the error paths here?
896+
for _, srcEntitlementID := range expandable.EntitlementIds {
897+
l.Debug(
898+
"Expandable entitlement found",
899+
zap.String("src_entitlement_id", srcEntitlementID),
900+
zap.String("dst_entitlement_id", grant.GetEntitlement().GetId()),
901+
)
902+
903+
srcEntitlement, err := s.store.GetEntitlement(ctx, &reader_v2.EntitlementsReaderServiceGetEntitlementRequest{
904+
EntitlementId: srcEntitlementID,
905+
})
906+
if err != nil {
907+
l.Error("error fetching source entitlement",
908+
zap.String("src_entitlement_id", srcEntitlementID),
909+
zap.String("dst_entitlement_id", grant.GetEntitlement().GetId()),
910+
zap.Error(err),
911+
)
912+
continue
913+
}
914+
915+
// The expand annotation points at entitlements by id. Those entitlements' resource should match
916+
// the current grant's principal, so we don't allow expanding arbitrary entitlements.
917+
sourceEntitlementResourceID := srcEntitlement.GetEntitlement().GetResource().GetId()
918+
if sourceEntitlementResourceID == nil {
919+
return fmt.Errorf("source entitlement resource id was nil")
920+
}
921+
if principalID.ResourceType != sourceEntitlementResourceID.ResourceType ||
922+
principalID.Resource != sourceEntitlementResourceID.Resource {
923+
l.Error(
924+
"source entitlement resource id did not match grant principal id",
925+
zap.String("grant_principal_id", principalID.String()),
926+
zap.String("source_entitlement_resource_id", sourceEntitlementResourceID.String()))
927+
928+
return fmt.Errorf("source entitlement resource id did not match grant principal id")
929+
}
930+
931+
entitlementGraph.AddEntitlement(grant.Entitlement)
932+
entitlementGraph.AddEntitlement(srcEntitlement.GetEntitlement())
933+
err = entitlementGraph.AddEdge(ctx,
934+
srcEntitlement.GetEntitlement().GetId(),
935+
grant.GetEntitlement().GetId(),
936+
expandable.Shallow,
937+
expandable.ResourceTypeIds,
938+
)
939+
if err != nil {
940+
return fmt.Errorf("error adding edge to graph: %w", err)
941+
}
942+
}
943+
}
944+
return nil
945+
}
946+
856947
if entitlementGraph.Loaded {
857-
fmt.Printf("getting cycles1\n")
858948
cycle := entitlementGraph.GetFirstCycle()
859949
if cycle != nil {
860950
l.Warn(
@@ -865,105 +955,19 @@ func (s *syncer) SyncGrantExpansion(ctx context.Context) error {
865955
if dontFixCycles {
866956
return fmt.Errorf("cycles detected in entitlement graph")
867957
}
868-
fmt.Printf("fixing cycles1\n")
958+
869959
err := entitlementGraph.FixCycles()
870-
fmt.Printf("fixed cycles1\n")
871960
if err != nil {
872961
return err
873962
}
874963
}
875-
fmt.Printf("expandGrantsForEntitlements\n")
876-
return s.expandGrantsForEntitlements(ctx)
877964
}
878965

879-
pageToken := s.state.PageToken(ctx)
880-
881-
if pageToken == "" {
882-
l.Info("Expanding grants...")
883-
s.handleInitialActionForStep(ctx, *s.state.Current())
884-
}
885-
886-
resp, err := s.store.ListGrants(ctx, &v2.GrantsServiceListGrantsRequest{PageToken: pageToken})
966+
err := s.expandGrantsForEntitlements(ctx)
887967
if err != nil {
888968
return err
889969
}
890970

891-
// We want to take action on the next page before we push any new actions
892-
if resp.NextPageToken != "" {
893-
err = s.state.NextPage(ctx, resp.NextPageToken)
894-
if err != nil {
895-
return err
896-
}
897-
} else {
898-
l.Info("Finished loading entitlement graph", zap.Int("edges", len(entitlementGraph.Edges)))
899-
entitlementGraph.Loaded = true
900-
}
901-
902-
for _, grant := range resp.List {
903-
annos := annotations.Annotations(grant.Annotations)
904-
expandable := &v2.GrantExpandable{}
905-
_, err := annos.Pick(expandable)
906-
if err != nil {
907-
return err
908-
}
909-
if len(expandable.GetEntitlementIds()) == 0 {
910-
continue
911-
}
912-
913-
principalID := grant.GetPrincipal().GetId()
914-
if principalID == nil {
915-
return fmt.Errorf("principal id was nil")
916-
}
917-
918-
// FIXME(morgabra) Log and skip some of the error paths here?
919-
for _, srcEntitlementID := range expandable.EntitlementIds {
920-
l.Debug(
921-
"Expandable entitlement found",
922-
zap.String("src_entitlement_id", srcEntitlementID),
923-
zap.String("dst_entitlement_id", grant.GetEntitlement().GetId()),
924-
)
925-
926-
srcEntitlement, err := s.store.GetEntitlement(ctx, &reader_v2.EntitlementsReaderServiceGetEntitlementRequest{
927-
EntitlementId: srcEntitlementID,
928-
})
929-
if err != nil {
930-
l.Error("error fetching source entitlement",
931-
zap.String("src_entitlement_id", srcEntitlementID),
932-
zap.String("dst_entitlement_id", grant.GetEntitlement().GetId()),
933-
zap.Error(err),
934-
)
935-
continue
936-
}
937-
938-
// The expand annotation points at entitlements by id. Those entitlements' resource should match
939-
// the current grant's principal, so we don't allow expanding arbitrary entitlements.
940-
sourceEntitlementResourceID := srcEntitlement.GetEntitlement().GetResource().GetId()
941-
if sourceEntitlementResourceID == nil {
942-
return fmt.Errorf("source entitlement resource id was nil")
943-
}
944-
if principalID.ResourceType != sourceEntitlementResourceID.ResourceType ||
945-
principalID.Resource != sourceEntitlementResourceID.Resource {
946-
l.Error(
947-
"source entitlement resource id did not match grant principal id",
948-
zap.String("grant_principal_id", principalID.String()),
949-
zap.String("source_entitlement_resource_id", sourceEntitlementResourceID.String()))
950-
951-
return fmt.Errorf("source entitlement resource id did not match grant principal id")
952-
}
953-
954-
entitlementGraph.AddEntitlement(grant.Entitlement)
955-
entitlementGraph.AddEntitlement(srcEntitlement.GetEntitlement())
956-
err = entitlementGraph.AddEdge(ctx,
957-
srcEntitlement.GetEntitlement().GetId(),
958-
grant.GetEntitlement().GetId(),
959-
expandable.Shallow,
960-
expandable.ResourceTypeIds,
961-
)
962-
if err != nil {
963-
return fmt.Errorf("error adding edge to graph: %w", err)
964-
}
965-
}
966-
}
967971
return nil
968972
}
969973

@@ -1426,7 +1430,6 @@ func (s *syncer) expandGrantsForEntitlements(ctx context.Context) error {
14261430
actions := len(graph.Actions)
14271431
if actions%250 == 0 || actions < 10 {
14281432
l.Info("Expanding grants", zap.Int("actions", actions))
1429-
fmt.Printf("actions: %v: %v\n", actions, graph.Actions)
14301433
}
14311434

14321435
actionsDone, err := s.runGrantExpandActions(ctx)
@@ -1437,7 +1440,7 @@ func (s *syncer) expandGrantsForEntitlements(ctx context.Context) error {
14371440
if !actionsDone {
14381441
return nil
14391442
}
1440-
fmt.Printf("not done: %v\n", graph.Depth)
1443+
14411444
if graph.Depth > maxDepth {
14421445
l.Error(
14431446
"expandGrantsForEntitlements: exceeded max depth",
@@ -1466,7 +1469,7 @@ func (s *syncer) expandGrantsForEntitlements(ctx context.Context) error {
14661469
if grantInfo.IsExpanded {
14671470
continue
14681471
}
1469-
graph.Actions = append(graph.Actions, expand.EntitlementGraphAction{
1472+
graph.Actions = append(graph.Actions, &expand.EntitlementGraphAction{
14701473
SourceEntitlementID: sourceEntitlementID,
14711474
DescendantEntitlementID: descendantEntitlementID,
14721475
PageToken: "",

0 commit comments

Comments
 (0)