diff --git a/cmd/server/app/webhook_update.go b/cmd/server/app/webhook_update.go index a0bca61138..34d3a65457 100644 --- a/cmd/server/app/webhook_update.go +++ b/cmd/server/app/webhook_update.go @@ -33,7 +33,7 @@ import ( ) func cmdWebhookUpdate() *cobra.Command { - var updateCmd = &cobra.Command{ + updateCmd := &cobra.Command{ Use: "update", Short: "update the webhook configuration", Long: `Command to upgrade webhook configuration`, diff --git a/database/mock/store.go b/database/mock/store.go index 4ee5419eb5..d449069333 100644 --- a/database/mock/store.go +++ b/database/mock/store.go @@ -1031,6 +1031,21 @@ func (mr *MockStoreMockRecorder) GetEntitiesByType(ctx, arg any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetEntitiesByType", reflect.TypeOf((*MockStore)(nil).GetEntitiesByType), ctx, arg) } +// GetEntitiesWithProps mocks base method. +func (m *MockStore) GetEntitiesWithProps(ctx context.Context, arg db.GetEntitiesWithPropsParams) ([]db.GetEntitiesWithPropsRow, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetEntitiesWithProps", ctx, arg) + ret0, _ := ret[0].([]db.GetEntitiesWithPropsRow) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetEntitiesWithProps indicates an expected call of GetEntitiesWithProps. +func (mr *MockStoreMockRecorder) GetEntitiesWithProps(ctx, arg any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetEntitiesWithProps", reflect.TypeOf((*MockStore)(nil).GetEntitiesWithProps), ctx, arg) +} + // GetEntitlementFeaturesByProjectID mocks base method. func (m *MockStore) GetEntitlementFeaturesByProjectID(ctx context.Context, projectID uuid.UUID) ([]string, error) { m.ctrl.T.Helper() diff --git a/database/query/entities.sql b/database/query/entities.sql index 771a008251..2b82a6850c 100644 --- a/database/query/entities.sql +++ b/database/query/entities.sql @@ -70,6 +70,27 @@ WHERE entity_instances.entity_type = $1 AND entity_instances.provider_id = sqlc.arg(provider_id) AND entity_instances.project_id = ANY(sqlc.arg(projects)::uuid[]); +-- GetEntitiesWithProps retrieves all entities of a given type with their properties for a project or hierarchy of projects. + +-- name: GetEntitiesWithProps :many +SELECT entity_instances.*, + providers.name AS provider_name, + providers.class AS provider_class, + JSON_OBJECT_AGG( + p.key, + p.value + ) AS properties +FROM entity_instances + LEFT JOIN providers ON entity_instances.provider_id = providers.id + LEFT JOIN properties p ON entity_instances.id = p.entity_id +WHERE (entity_instances.entity_type = sqlc.narg(entity_type) OR sqlc.narg(entity_type) IS NULL) + AND entity_instances.id >= sqlc.arg('cursor')::uuid + AND entity_instances.provider_id = sqlc.arg(provider_id) + AND entity_instances.project_id = ANY(sqlc.arg(projects)::uuid[]) +GROUP BY entity_instances.id, providers.id +ORDER BY entity_instances.id +LIMIT sqlc.arg('limit')::bigint; + -- ListEntitiesAfterID retrieves entities of a given type after a cursor ID, for pagination. -- This is used for cursor-based iteration over all entities (e.g., in the reminder service). diff --git a/docs/docs/ref/proto.mdx b/docs/docs/ref/proto.mdx index 62b218d0fb..eb04299de7 100644 --- a/docs/docs/ref/proto.mdx +++ b/docs/docs/ref/proto.mdx @@ -961,6 +961,7 @@ used for parsing resources in ruletypes | name | string | | name is the name of the entity. | | type | Entity | | type is the type of the entity. DISCUSSION: If we're aiming for a BYO entity type, we should probably have this be a string, and have the user provide the type. | | properties | google.protobuf.Struct | | properties is a map of properties of the entity. | +| originating_id | string | | originating_id is the id of the entity which originated this resource. If the originating entity is cleaned up, this entity will also be removed. | diff --git a/internal/controlplane/handlers_entity_instances.go b/internal/controlplane/handlers_entity_instances.go index d2af091c9a..244af8f05d 100644 --- a/internal/controlplane/handlers_entity_instances.go +++ b/internal/controlplane/handlers_entity_instances.go @@ -42,43 +42,39 @@ func (s *Server) ListEntities( return nil, fmt.Errorf("error getting provider: %w", err) } - // Get entity type from request - entityType := in.GetEntityType() - if entityType == pb.Entity_ENTITY_UNSPECIFIED { - return nil, util.UserVisibleError(codes.InvalidArgument, "entity type must be specified") - } - // Get limit from request limit := in.GetCursor().GetSize() if limit == 0 { - limit = 20 // Default limit + limit = 100 // Default limit } - // Get cursor from request - cursor := in.GetCursor().GetCursor() - // Call service to get entities outentities, nextCursor, err := s.entityService.ListEntities( ctx, projectID, provider.ID, - entityType, - cursor, + in.GetEntityType(), + in.GetCursor().GetCursor(), int64(limit), ) if err != nil { - return nil, err + return nil, fmt.Errorf("error listing entities: %w", err) } - // Create response - resp := &pb.ListEntitiesResponse{ - Results: outentities, - Page: &pb.CursorPage{ + var pageInfo *pb.CursorPage + if nextCursor != "" { + pageInfo = &pb.CursorPage{ Next: &pb.Cursor{ Cursor: nextCursor, Size: limit, }, - }, + } + } + + // Create response + resp := &pb.ListEntitiesResponse{ + Results: outentities, + Page: pageInfo, } return resp, nil diff --git a/internal/controlplane/handlers_entity_instances_test.go b/internal/controlplane/handlers_entity_instances_test.go index a7ca695937..ac699dcc16 100644 --- a/internal/controlplane/handlers_entity_instances_test.go +++ b/internal/controlplane/handlers_entity_instances_test.go @@ -410,3 +410,184 @@ func TestParseIdentifyingProperties(t *testing.T) { }) } } + +func TestServer_ListEntities(t *testing.T) { + t.Parallel() + + projectID := uuid.New() + providerID := uuid.New() + providerName := "github" + + tests := []struct { + name string + request *pb.ListEntitiesRequest + setupContext func(context.Context) context.Context + setupMocks func(*mockproviders.MockProviderStore, *mockentitysvc.MockEntityService) + wantCode codes.Code + errContains string + validateResp func(*testing.T, *pb.ListEntitiesResponse) + }{ + { + name: "successfully lists entities", + request: &pb.ListEntitiesRequest{ + EntityType: pb.Entity_ENTITY_REPOSITORIES, + }, + setupMocks: func(provStore *mockproviders.MockProviderStore, svc *mockentitysvc.MockEntityService) { + provStore.EXPECT(). + GetByName(gomock.Any(), projectID, providerName). + Return(&db.Provider{ + ID: providerID, + Name: providerName, + ProjectID: projectID, + }, nil) + + svc.EXPECT(). + ListEntities(gomock.Any(), projectID, providerID, pb.Entity_ENTITY_REPOSITORIES, "", int64(100)). + Return([]*pb.EntityInstance{ + {Id: uuid.New().String(), Name: "repo1", Type: pb.Entity_ENTITY_REPOSITORIES}, + {Id: uuid.New().String(), Name: "repo2", Type: pb.Entity_ENTITY_REPOSITORIES}, + }, "", nil) + }, + validateResp: func(t *testing.T, resp *pb.ListEntitiesResponse) { + t.Helper() + assert.Len(t, resp.Results, 2) + assert.Nil(t, resp.Page) + }, + }, + { + name: "lists multiple entity types", + request: &pb.ListEntitiesRequest{}, + setupMocks: func(provStore *mockproviders.MockProviderStore, svc *mockentitysvc.MockEntityService) { + provStore.EXPECT(). + GetByName(gomock.Any(), projectID, providerName). + Return(&db.Provider{ + ID: providerID, + Name: providerName, + ProjectID: projectID, + }, nil) + + svc.EXPECT(). + ListEntities(gomock.Any(), projectID, providerID, pb.Entity_ENTITY_UNSPECIFIED, "", int64(100)). + Return([]*pb.EntityInstance{ + {Id: uuid.New().String(), Name: "repo1", Type: pb.Entity_ENTITY_REPOSITORIES}, + {Id: uuid.New().String(), Name: "repo1/pull/2", Type: pb.Entity_ENTITY_PULL_REQUESTS}, + }, "", nil) + }, + validateResp: func(t *testing.T, resp *pb.ListEntitiesResponse) { + t.Helper() + assert.Len(t, resp.Results, 2) + assert.Nil(t, resp.Page) + }, + }, + { + name: "successfully lists entities with pagination", + request: &pb.ListEntitiesRequest{ + EntityType: pb.Entity_ENTITY_REPOSITORIES, + Cursor: &pb.Cursor{ + Size: 1, + }, + }, + setupMocks: func(provStore *mockproviders.MockProviderStore, svc *mockentitysvc.MockEntityService) { + provStore.EXPECT(). + GetByName(gomock.Any(), projectID, providerName). + Return(&db.Provider{ + ID: providerID, + Name: providerName, + ProjectID: projectID, + }, nil) + + svc.EXPECT(). + ListEntities(gomock.Any(), projectID, providerID, pb.Entity_ENTITY_REPOSITORIES, "", int64(1)). + Return([]*pb.EntityInstance{ + {Id: uuid.New().String(), Name: "repo1", Type: pb.Entity_ENTITY_REPOSITORIES}, + }, "next-cursor", nil) + }, + validateResp: func(t *testing.T, resp *pb.ListEntitiesResponse) { + t.Helper() + assert.Len(t, resp.Results, 1) + assert.NotNil(t, resp.Page) + assert.Equal(t, "next-cursor", resp.Page.Next.Cursor) + assert.Equal(t, uint32(1), resp.Page.Next.Size) + }, + }, + { + name: "fails when provider not found", + request: &pb.ListEntitiesRequest{ + EntityType: pb.Entity_ENTITY_REPOSITORIES, + }, + setupMocks: func(provStore *mockproviders.MockProviderStore, _ *mockentitysvc.MockEntityService) { + provStore.EXPECT(). + GetByName(gomock.Any(), projectID, providerName). + Return(nil, sql.ErrNoRows) + }, + wantCode: codes.NotFound, + errContains: "provider not found", + }, + { + name: "handles service error", + request: &pb.ListEntitiesRequest{ + EntityType: pb.Entity_ENTITY_REPOSITORIES, + }, + setupMocks: func(provStore *mockproviders.MockProviderStore, svc *mockentitysvc.MockEntityService) { + provStore.EXPECT(). + GetByName(gomock.Any(), projectID, providerName). + Return(&db.Provider{ + ID: providerID, + Name: providerName, + ProjectID: projectID, + }, nil) + + svc.EXPECT(). + ListEntities(gomock.Any(), projectID, providerID, pb.Entity_ENTITY_REPOSITORIES, "", int64(100)). + Return(nil, "", errors.New("service error")) + }, + wantCode: codes.Internal, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockProvStore := mockproviders.NewMockProviderStore(ctrl) + mockEntitySvc := mockentitysvc.NewMockEntityService(ctrl) + + if tt.setupMocks != nil { + tt.setupMocks(mockProvStore, mockEntitySvc) + } + + server := &Server{ + providerStore: mockProvStore, + entityService: mockEntitySvc, + } + + ctx := engcontext.WithEntityContext(context.Background(), &engcontext.EntityContext{ + Project: engcontext.Project{ID: projectID}, + Provider: engcontext.Provider{Name: providerName}, + }) + + resp, err := server.ListEntities(ctx, tt.request) + + if tt.wantCode != codes.OK { + require.Error(t, err) + + if tt.wantCode != codes.Internal { + st, ok := status.FromError(err) + require.True(t, ok, "error should be a gRPC status error") + assert.Equal(t, tt.wantCode, st.Code()) + } + assert.Contains(t, err.Error(), tt.errContains) + return + } + + require.NoError(t, err) + require.NotNil(t, resp) + if tt.validateResp != nil { + tt.validateResp(t, resp) + } + }) + } +} diff --git a/internal/db/entities.sql.go b/internal/db/entities.sql.go index 740810353f..13f4401a72 100644 --- a/internal/db/entities.sql.go +++ b/internal/db/entities.sql.go @@ -7,7 +7,9 @@ package db import ( "context" + "database/sql" "encoding/json" + "time" "github.com/google/uuid" "github.com/lib/pq" @@ -403,6 +405,89 @@ func (q *Queries) GetEntitiesByType(ctx context.Context, arg GetEntitiesByTypePa return items, nil } +const getEntitiesWithProps = `-- name: GetEntitiesWithProps :many + +SELECT entity_instances.id, entity_instances.entity_type, entity_instances.name, entity_instances.project_id, entity_instances.provider_id, entity_instances.created_at, entity_instances.originated_from, + providers.name AS provider_name, + providers.class AS provider_class, + JSON_OBJECT_AGG( + p.key, + p.value + ) AS properties +FROM entity_instances + LEFT JOIN providers ON entity_instances.provider_id = providers.id + LEFT JOIN properties p ON entity_instances.id = p.entity_id +WHERE (entity_instances.entity_type = $1 OR $1 IS NULL) + AND entity_instances.id >= $2::uuid + AND entity_instances.provider_id = $3 + AND entity_instances.project_id = ANY($4::uuid[]) +GROUP BY entity_instances.id, providers.id +ORDER BY entity_instances.id +LIMIT $5::bigint +` + +type GetEntitiesWithPropsParams struct { + EntityType NullEntities `json:"entity_type"` + Cursor uuid.UUID `json:"cursor"` + ProviderID uuid.UUID `json:"provider_id"` + Projects []uuid.UUID `json:"projects"` + Limit int64 `json:"limit"` +} + +type GetEntitiesWithPropsRow struct { + ID uuid.UUID `json:"id"` + EntityType Entities `json:"entity_type"` + Name string `json:"name"` + ProjectID uuid.UUID `json:"project_id"` + ProviderID uuid.UUID `json:"provider_id"` + CreatedAt time.Time `json:"created_at"` + OriginatedFrom uuid.NullUUID `json:"originated_from"` + ProviderName sql.NullString `json:"provider_name"` + ProviderClass NullProviderClass `json:"provider_class"` + Properties json.RawMessage `json:"properties"` +} + +// GetEntitiesWithProps retrieves all entities of a given type with their properties for a project or hierarchy of projects. +func (q *Queries) GetEntitiesWithProps(ctx context.Context, arg GetEntitiesWithPropsParams) ([]GetEntitiesWithPropsRow, error) { + rows, err := q.db.QueryContext(ctx, getEntitiesWithProps, + arg.EntityType, + arg.Cursor, + arg.ProviderID, + pq.Array(arg.Projects), + arg.Limit, + ) + if err != nil { + return nil, err + } + defer rows.Close() + items := []GetEntitiesWithPropsRow{} + for rows.Next() { + var i GetEntitiesWithPropsRow + if err := rows.Scan( + &i.ID, + &i.EntityType, + &i.Name, + &i.ProjectID, + &i.ProviderID, + &i.CreatedAt, + &i.OriginatedFrom, + &i.ProviderName, + &i.ProviderClass, + &i.Properties, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const getEntityByID = `-- name: GetEntityByID :one SELECT id, entity_type, name, project_id, provider_id, created_at, originated_from FROM entity_instances WHERE entity_instances.id = $1 diff --git a/internal/db/querier.go b/internal/db/querier.go index 039a77b2e6..700c2db3c8 100644 --- a/internal/db/querier.go +++ b/internal/db/querier.go @@ -117,6 +117,8 @@ type Querier interface { // GetEntitiesByType retrieves all entities of a given type for a project or hierarchy of projects. // this is how one would get all repositories, artifacts, etc. GetEntitiesByType(ctx context.Context, arg GetEntitiesByTypeParams) ([]EntityInstance, error) + // GetEntitiesWithProps retrieves all entities of a given type with their properties for a project or hierarchy of projects. + GetEntitiesWithProps(ctx context.Context, arg GetEntitiesWithPropsParams) ([]GetEntitiesWithPropsRow, error) GetEntitlementFeaturesByProjectID(ctx context.Context, projectID uuid.UUID) ([]string, error) // GetEntityByID retrieves an entity by its ID for a project or hierarchy of projects. GetEntityByID(ctx context.Context, id uuid.UUID) (EntityInstance, error) diff --git a/internal/entities/handlers/handler_test.go b/internal/entities/handlers/handler_test.go index f80ffd0492..096b64f9a9 100644 --- a/internal/entities/handlers/handler_test.go +++ b/internal/entities/handlers/handler_test.go @@ -6,6 +6,7 @@ package handlers import ( "encoding/json" "errors" + "maps" "testing" watermill "github.com/ThreeDotsLabs/watermill/message" @@ -13,7 +14,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" - "golang.org/x/exp/maps" "google.golang.org/protobuf/reflect/protoreflect" df "github.com/mindersec/minder/database/mock/fixtures" diff --git a/internal/entities/properties/service/entitycache.go b/internal/entities/properties/service/entitycache.go index e6dd0a98d2..671bc6f7e0 100644 --- a/internal/entities/properties/service/entitycache.go +++ b/internal/entities/properties/service/entitycache.go @@ -5,11 +5,13 @@ package service import ( "context" + "encoding/json" "fmt" "github.com/google/uuid" "github.com/puzpuzpuz/xsync/v3" + "github.com/mindersec/minder/internal/db" "github.com/mindersec/minder/internal/entities/models" ) @@ -66,3 +68,24 @@ func (ps *propertyServiceWithPersistentEntityCache) EntityWithPropertiesByID( return ent, nil } + +// PropertiesFromJSON converts pre-fetched {"key": value} data from JSON_OBJECT_AGG into an EntityWithProperties +func (ps *propertyServiceWithPersistentEntityCache) PropertiesFromJSON( + entity db.EntityInstance, props json.RawMessage, opts *CallOptions, +) (*models.EntityWithProperties, error) { + // Check the cache first. + if ent, ok := ps.entCache.Load(entity.ID); ok { + return ent, nil + } + + // If not in the cache, call the underlying service. + ewp, err := ps.PropertiesService.PropertiesFromJSON(entity, props, opts) + if err != nil { + return nil, err + } + + // Store the entity in the cache. + ps.entCache.Store(entity.ID, ewp) + + return ewp, nil +} diff --git a/internal/entities/properties/service/mock/service.go b/internal/entities/properties/service/mock/service.go index 7d047db7a1..d552d32de8 100644 --- a/internal/entities/properties/service/mock/service.go +++ b/internal/entities/properties/service/mock/service.go @@ -11,9 +11,11 @@ package mock_service import ( context "context" + json "encoding/json" reflect "reflect" uuid "github.com/google/uuid" + db "github.com/mindersec/minder/internal/db" models "github.com/mindersec/minder/internal/entities/models" service "github.com/mindersec/minder/internal/entities/properties/service" manager "github.com/mindersec/minder/internal/providers/manager" @@ -93,6 +95,21 @@ func (mr *MockPropertiesServiceMockRecorder) EntityWithPropertiesByUpstreamHint( return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EntityWithPropertiesByUpstreamHint", reflect.TypeOf((*MockPropertiesService)(nil).EntityWithPropertiesByUpstreamHint), ctx, entType, getByProps, hint, opts) } +// PropertiesFromJSON mocks base method. +func (m *MockPropertiesService) PropertiesFromJSON(entity db.EntityInstance, props json.RawMessage, opts *service.CallOptions) (*models.EntityWithProperties, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "PropertiesFromJSON", entity, props, opts) + ret0, _ := ret[0].(*models.EntityWithProperties) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// PropertiesFromJSON indicates an expected call of PropertiesFromJSON. +func (mr *MockPropertiesServiceMockRecorder) PropertiesFromJSON(entity, props, opts any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PropertiesFromJSON", reflect.TypeOf((*MockPropertiesService)(nil).PropertiesFromJSON), entity, props, opts) +} + // ReplaceAllProperties mocks base method. func (m *MockPropertiesService) ReplaceAllProperties(ctx context.Context, entityID uuid.UUID, props *properties.Properties, opts *service.CallOptions) error { m.ctrl.T.Helper() diff --git a/internal/entities/properties/service/service.go b/internal/entities/properties/service/service.go index 72de9042e7..1ff8475195 100644 --- a/internal/entities/properties/service/service.go +++ b/internal/entities/properties/service/service.go @@ -7,6 +7,7 @@ package service import ( "context" "database/sql" + "encoding/json" "errors" "fmt" "time" @@ -52,6 +53,10 @@ type PropertiesService interface { EntityWithPropertiesByID( ctx context.Context, entityID uuid.UUID, opts *CallOptions, ) (*models.EntityWithProperties, error) + // PropertiesFromJSON converts pre-fetched {"key": value} data from JSON_OBJECT_AGG into an EntityWithProperties + PropertiesFromJSON( + entity db.EntityInstance, props json.RawMessage, opts *CallOptions, + ) (*models.EntityWithProperties, error) // EntityWithPropertiesByUpstreamHint fetches an entity by upstream properties // and returns the entity with its properties. It is expected that the caller // does NOT know the project or provider ID. Whatever hints it may have @@ -301,6 +306,29 @@ func (ps *propertiesService) EntityWithPropertiesByID( return ps.getEntityWithProperties(ctx, ent, opts) } +// PropertiesFromJSON converts pre-fetched {"key": propertyvalue} data from JSON_OBJECT_AGG +// into an EntityWithProperties +func (*propertiesService) PropertiesFromJSON( + entity db.EntityInstance, propsData json.RawMessage, _ *CallOptions, +) (*models.EntityWithProperties, error) { + var propData map[string]db.PropertyWrapper + + err := json.Unmarshal(propsData, &propData) + if err != nil { + return nil, fmt.Errorf("error unmarshaling properties: %w", err) + } + + // TODO: check that the version is "v1" (db.propValueV1) for each PropertyWrapper + propsMap := make(map[string]any, len(propData)) + for k, v := range propData { + propsMap[k] = v.Value + } + props := properties.NewProperties(propsMap) + + ewp := models.NewEntityWithProperties(entity, props) + return ewp, nil +} + // ByUpstreamHint is a hint to help find an entity by upstream ID // API-wise it should only be exposed in those methods of the service layer // that do not know the project or provider ID and are searching for an entity diff --git a/internal/entities/service/service.go b/internal/entities/service/service.go index 7b3ff171fb..a082d606ad 100644 --- a/internal/entities/service/service.go +++ b/internal/entities/service/service.go @@ -84,7 +84,11 @@ func NewEntityService( } } -//nolint:gocyclo // Let's refactor this later +// ListEntities retrieves entities for the specific project and provider. +// If entityType is unspecified, all entity types will be returned. +// Pagination is supported via the cursor and limit parameters; an empty +// input cursor will return the first page, and an empty next cursor +// indicates the end of the list. func (s *entityService) ListEntities( ctx context.Context, projectID uuid.UUID, @@ -99,29 +103,35 @@ func (s *entityService) ListEntities( Str("entityType", entityType.String()). Logger() - // Convert pb.Entity to db.Entities - dbEntityType, err := entities.EntityTypeToDBType(entityType) - if err != nil { - return nil, "", fmt.Errorf("failed to convert entity type: %w", err) + var err error + dbEntityType := db.NullEntities{ + Valid: false, + } + if entityType != pb.Entity_ENTITY_UNSPECIFIED { + dbEntityType.Valid = true + // Convert pb.Entity to db.Entities + dbEntityType.Entities, err = entities.EntityTypeToDBType(entityType) + if err != nil { + return nil, "", fmt.Errorf("failed to convert entity type: %w", err) + } } // Parse cursor if provided + firstElement := uuid.Nil if cursor != "" { - _, err := uuid.Parse(cursor) + firstElement, err = uuid.Parse(cursor) if err != nil { return nil, "", util.UserVisibleError(codes.InvalidArgument, "invalid cursor format") } - // TODO: Use lastEntityID for pagination } // Set up query parameters - queryLimit := sql.NullInt64{Valid: false, Int64: 0} - if limit > 0 { - const maxFetchLimit = 100 - if limit > maxFetchLimit { - return nil, "", util.UserVisibleError(codes.InvalidArgument, "limit too high, max is %d", maxFetchLimit) - } - queryLimit = sql.NullInt64{Valid: true, Int64: limit + 1} // +1 to check if there are more results + const maxFetchLimit = 100 + if limit <= 0 { + limit = maxFetchLimit + } + if limit > maxFetchLimit { + return nil, "", util.UserVisibleError(codes.InvalidArgument, "limit too high, max is %d", maxFetchLimit) } // Get entities from database @@ -137,11 +147,12 @@ func (s *entityService) ListEntities( qtx := s.store.GetQuerierWithTransaction(tx) - // TODO: Implement proper pagination with cursor - outentities, err := qtx.GetEntitiesByType(ctx, db.GetEntitiesByTypeParams{ + found, err := qtx.GetEntitiesWithProps(ctx, db.GetEntitiesWithPropsParams{ EntityType: dbEntityType, ProviderID: providerID, Projects: []uuid.UUID{projectID}, + Cursor: firstElement, + Limit: limit + 1, }) if err != nil { return nil, "", fmt.Errorf("error fetching entities: %w", err) @@ -151,36 +162,35 @@ func (s *entityService) ListEntities( var results []*pb.EntityInstance var nextCursor string - for i, entity := range outentities { - // Apply limit if specified - if queryLimit.Valid && int64(i) >= queryLimit.Int64-1 { - nextCursor = entity.ID.String() - break - } + // We fetch one extra row so that we can determine if there are more results + // This saves the client from doing a zero-row fetch later, at a cost of slightly + // more row fetches. + if len(found) > int(limit) { + nextCursor = found[limit].ID.String() + found = found[:limit] + } - ewp, err := s.propSvc.EntityWithPropertiesByID(ctx, entity.ID, - propService.CallBuilder().WithStoreOrTransaction(qtx)) - if err != nil { - return nil, "", fmt.Errorf("error fetching properties for entity: %w", err) + for i, entity := range found { + ei := db.EntityInstance{ + ID: entity.ID, + EntityType: entity.EntityType, + Name: entity.Name, + ProjectID: entity.ProjectID, + ProviderID: entity.ProviderID, + CreatedAt: entity.CreatedAt, + OriginatedFrom: entity.OriginatedFrom, } - - if err := s.propSvc.RetrieveAllPropertiesForEntity(ctx, ewp, s.providerManager, - propService.ReadBuilder().WithStoreOrTransaction(qtx).TolerateStaleData()); err != nil { + ewp, err := s.propSvc.PropertiesFromJSON(ei, found[i].Properties, propService.CallBuilder().WithStoreOrTransaction(qtx)) + if err != nil { return nil, "", fmt.Errorf("error fetching properties for entity: %w", err) } - // Convert to protobuf pbEntity := entityInstanceToProto(ewp) + pbEntity.Context.Provider = entity.ProviderName.String // If null, will store empty string, which is fine results = append(results, pbEntity) } - // Rollback transaction - if err := tx.Rollback(); err != nil { - // This is not fatal, but we should log it - l.Error().Err(err).Msg("error rolling back transaction") - } - return results, nextCursor, nil } @@ -324,10 +334,11 @@ func entityInstanceToProto(ewp *models.EntityWithProperties) *pb.EntityInstance Id: ewp.Entity.ID.String(), Context: &pb.ContextV2{ ProjectId: ewp.Entity.ProjectID.String(), - Provider: "", // This would need to be filled in from the provider name + // Provider: "", // This would need to be filled in from the provider name }, - Name: ewp.Entity.Name, - Type: ewp.Entity.Type, - Properties: propsStruct, + Name: ewp.Entity.Name, + Type: ewp.Entity.Type, + Properties: propsStruct, + OriginatingId: ewp.Entity.OriginatedFrom.String(), } } diff --git a/internal/entities/service/service_test.go b/internal/entities/service/service_test.go new file mode 100644 index 0000000000..0c34d52f40 --- /dev/null +++ b/internal/entities/service/service_test.go @@ -0,0 +1,277 @@ +// SPDX-FileCopyrightText: Copyright 2024 The Minder Authors +// SPDX-License-Identifier: Apache-2.0 + +package service_test + +import ( + "context" + "fmt" + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + "github.com/mindersec/minder/internal/db" + "github.com/mindersec/minder/internal/db/embedded" + propService "github.com/mindersec/minder/internal/entities/properties/service" + "github.com/mindersec/minder/internal/entities/service" + mockprov "github.com/mindersec/minder/internal/providers/manager/mock" + pb "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" + "github.com/mindersec/minder/pkg/entities/properties" +) + +func setupEntityService(ctx context.Context, t *testing.T) ( + service.EntityService, db.Store, propService.PropertiesService, uuid.UUID, db.Provider, *mockprov.MockProviderManager, +) { + t.Helper() + + store, cancel, err := embedded.GetFakeStore() + require.NoError(t, err) + t.Cleanup(cancel) + + propSvc := propService.NewPropertiesService(store) + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + providerManager := mockprov.NewMockProviderManager(ctrl) + svc := service.NewEntityService(store, propSvc, providerManager) + + projectID := uuid.New() + + // Create project + _, err = store.CreateProjectWithID(ctx, db.CreateProjectWithIDParams{ + ID: projectID, + Name: "test-project-list", + Metadata: []byte("{}"), + }) + require.NoError(t, err) + + // Create provider + provider, err := store.CreateProvider(ctx, db.CreateProviderParams{ + Name: "github-list", + ProjectID: projectID, + Class: db.ProviderClassGithub, + Implements: []db.ProviderType{db.ProviderTypeGithub, db.ProviderTypeRepoLister}, + Definition: []byte("{}"), + AuthFlows: []db.AuthorizationFlow{db.AuthorizationFlowOauth2AuthorizationCodeFlow}, + }) + require.NoError(t, err) + + return svc, store, propSvc, projectID, provider, providerManager +} + +func TestEntityService_ListEntities(t *testing.T) { + t.Parallel() + ctx := context.Background() + + svc, store, propSvc, projectID, provider, _ := setupEntityService(ctx, t) + + // Seed some entities + nRepos := 5 + for i := range nRepos { + ei, err := store.CreateEntity(ctx, db.CreateEntityParams{ + EntityType: db.EntitiesRepository, + Name: fmt.Sprintf("repo-%d", i), + ProjectID: projectID, + ProviderID: provider.ID, + }) + require.NoError(t, err) + + // Add some properties + props := properties.NewProperties(map[string]any{ + "name": fmt.Sprintf("test-owner/repo-%d", i), + "github/repo_id": int64(i), + "github/repo_name": fmt.Sprintf("repo-%d", i), + "github/repo_owner": "test-owner", + }) + err = propSvc.SaveAllProperties(ctx, ei.ID, props, nil) + require.NoError(t, err) + } + + nArtifacts := 3 + for i := range nArtifacts { + ei, err := store.CreateEntity(ctx, db.CreateEntityParams{ + EntityType: db.EntitiesArtifact, + Name: fmt.Sprintf("artifact-%d", i), + ProjectID: projectID, + ProviderID: provider.ID, + }) + require.NoError(t, err) + + // Add some properties + props := properties.NewProperties(map[string]any{ + "name": fmt.Sprintf("test-owner/repo-1/artifact-%d", i), + }) + err = propSvc.SaveAllProperties(ctx, ei.ID, props, nil) + require.NoError(t, err) + } + + t.Run("List all entities", func(t *testing.T) { + t.Parallel() + results, nextCursor, err := svc.ListEntities(ctx, projectID, provider.ID, pb.Entity_ENTITY_UNSPECIFIED, "", 0) + assert.NoError(t, err) + assert.Len(t, results, nRepos+nArtifacts) + assert.Empty(t, nextCursor) + }) + + t.Run("List repositories only", func(t *testing.T) { + t.Parallel() + results, _, err := svc.ListEntities(ctx, projectID, provider.ID, pb.Entity_ENTITY_REPOSITORIES, "", 0) + assert.NoError(t, err) + assert.Len(t, results, nRepos) + for _, r := range results { + assert.Equal(t, pb.Entity_ENTITY_REPOSITORIES, r.Type) + } + }) + + t.Run("List artifacts only", func(t *testing.T) { + t.Parallel() + results, _, err := svc.ListEntities(ctx, projectID, provider.ID, pb.Entity_ENTITY_ARTIFACTS, "", 0) + assert.NoError(t, err) + assert.Len(t, results, nArtifacts) + for _, r := range results { + assert.Equal(t, pb.Entity_ENTITY_ARTIFACTS, r.Type) + } + }) + + t.Run("Pagination with limit", func(t *testing.T) { + t.Parallel() + limit := 3 + results, nextCursor, err := svc.ListEntities(ctx, projectID, provider.ID, pb.Entity_ENTITY_UNSPECIFIED, "", int64(limit)) + assert.NoError(t, err) + assert.Len(t, results, limit) + assert.NotEmpty(t, nextCursor) + + // Get next page + results2, nextCursor2, err := svc.ListEntities(ctx, projectID, provider.ID, pb.Entity_ENTITY_UNSPECIFIED, nextCursor, int64(limit)) + assert.NoError(t, err) + assert.Len(t, results2, limit) + assert.NotEmpty(t, nextCursor2) + + // Get last page + results3, nextCursor3, err := svc.ListEntities(ctx, projectID, provider.ID, pb.Entity_ENTITY_UNSPECIFIED, nextCursor2, int64(limit)) + assert.NoError(t, err) + assert.Len(t, results3, (nRepos+nArtifacts)-(2*limit)) + assert.Empty(t, nextCursor3) + }) + + t.Run("Empty results for non-existent provider", func(t *testing.T) { + t.Parallel() + results, nextCursor, err := svc.ListEntities(ctx, projectID, uuid.New(), pb.Entity_ENTITY_UNSPECIFIED, "", 0) + assert.NoError(t, err) + assert.Empty(t, results) + assert.Empty(t, nextCursor) + }) +} + +func TestEntityService_GetEntity(t *testing.T) { + t.Parallel() + ctx := context.Background() + + svc, store, propSvc, projectID, provider, providerManager := setupEntityService(ctx, t) + + // Seed an entity + name := "test-owner/repo-to-get" + ei, err := store.CreateEntity(ctx, db.CreateEntityParams{ + EntityType: db.EntitiesRepository, + Name: name, + ProjectID: projectID, + ProviderID: provider.ID, + }) + require.NoError(t, err) + + // Add some properties + props := properties.NewProperties(map[string]any{ + "name": name, + "github/repo_id": 123, + "github/repo_name": "repo-to-get", + "github/repo_owner": "test-owner", + }) + err = propSvc.SaveAllProperties(ctx, ei.ID, props, nil) + require.NoError(t, err) + + t.Run("GetEntityByID", func(t *testing.T) { + t.Parallel() + providerManager.EXPECT(). + InstantiateFromID(gomock.Any(), provider.ID). + Return(nil, nil) + + result, err := svc.GetEntityByID(ctx, ei.ID, projectID) + assert.NoError(t, err) + assert.NotNil(t, result) + assert.Equal(t, ei.ID.String(), result.Id) + assert.Equal(t, name, result.Name) + }) + + t.Run("GetEntityByName", func(t *testing.T) { + t.Parallel() + providerManager.EXPECT(). + InstantiateFromID(gomock.Any(), provider.ID). + Return(nil, nil) + + result, err := svc.GetEntityByName(ctx, name, projectID, provider.ID, pb.Entity_ENTITY_REPOSITORIES) + assert.NoError(t, err) + assert.NotNil(t, result) + assert.Equal(t, ei.ID.String(), result.Id) + assert.Equal(t, name, result.Name) + }) + + t.Run("GetEntityByID - Not Found", func(t *testing.T) { + t.Parallel() + result, err := svc.GetEntityByID(ctx, uuid.New(), projectID) + assert.Error(t, err) + assert.Nil(t, result) + }) + + t.Run("GetEntityByName - Not Found", func(t *testing.T) { + t.Parallel() + result, err := svc.GetEntityByName(ctx, "my-repo/non-existent", projectID, provider.ID, pb.Entity_ENTITY_REPOSITORIES) + assert.Error(t, err) + assert.Nil(t, result) + }) +} + +func TestEntityService_DeleteEntity(t *testing.T) { + t.Parallel() + ctx := context.Background() + + svc, store, propSvc, projectID, provider, _ := setupEntityService(ctx, t) + + // Seed an entity + name := "test-owner/repo-to-delete" + ei, err := store.CreateEntity(ctx, db.CreateEntityParams{ + EntityType: db.EntitiesRepository, + Name: name, + ProjectID: projectID, + ProviderID: provider.ID, + }) + require.NoError(t, err) + + // Add some properties + props := properties.NewProperties(map[string]any{ + "name": name, + "github/repo_id": 12345, + "github/repo_name": "repo-to-delete", + "github/repo_owner": "test-owner", + }) + err = propSvc.SaveAllProperties(ctx, ei.ID, props, nil) + require.NoError(t, err) + + t.Run("DeleteEntityByID", func(t *testing.T) { + t.Parallel() + err := svc.DeleteEntityByID(ctx, ei.ID, projectID) + assert.NoError(t, err) + + // Verify it's gone + _, err = store.GetEntityByID(ctx, ei.ID) + assert.Error(t, err) + }) + + t.Run("DeleteEntityByID - Not Found", func(t *testing.T) { + t.Parallel() + err := svc.DeleteEntityByID(ctx, uuid.New(), projectID) + assert.Error(t, err) + }) +} diff --git a/pkg/api/openapi/minder/v1/minder.swagger.json b/pkg/api/openapi/minder/v1/minder.swagger.json index 82244171d1..2bcaae771a 100644 --- a/pkg/api/openapi/minder/v1/minder.swagger.json +++ b/pkg/api/openapi/minder/v1/minder.swagger.json @@ -4570,6 +4570,10 @@ "properties": { "type": "object", "description": "properties is a map of properties of the entity." + }, + "originatingId": { + "type": "string", + "description": "originating_id is the id of the entity which originated this resource.\nIf the originating entity is cleaned up, this entity will also be removed." } }, "title": "used for parsing resources in ruletypes" diff --git a/pkg/api/protobuf/go/minder/v1/minder.pb.go b/pkg/api/protobuf/go/minder/v1/minder.pb.go index d6e76a62e0..b33624c09b 100644 --- a/pkg/api/protobuf/go/minder/v1/minder.pb.go +++ b/pkg/api/protobuf/go/minder/v1/minder.pb.go @@ -12043,7 +12043,10 @@ type EntityInstance struct { // have this be a string, and have the user provide the type. Type Entity `protobuf:"varint,4,opt,name=type,proto3,enum=minder.v1.Entity" json:"type,omitempty"` // properties is a map of properties of the entity. - Properties *structpb.Struct `protobuf:"bytes,5,opt,name=properties,proto3" json:"properties,omitempty"` + Properties *structpb.Struct `protobuf:"bytes,5,opt,name=properties,proto3" json:"properties,omitempty"` + // originating_id is the id of the entity which originated this resource. + // If the originating entity is cleaned up, this entity will also be removed. + OriginatingId string `protobuf:"bytes,6,opt,name=originating_id,json=originatingId,proto3" json:"originating_id,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -12113,6 +12116,13 @@ func (x *EntityInstance) GetProperties() *structpb.Struct { return nil } +func (x *EntityInstance) GetOriginatingId() string { + if x != nil { + return x.OriginatingId + } + return "" +} + // ListEntitiesRequest is the request message for the ListEntities method type ListEntitiesRequest struct { state protoimpl.MessageState `protogen:"open.v1"` @@ -15965,7 +15975,7 @@ const file_minder_v1_minder_proto_rawDesc = "" + "\adetails\x18\x02 \x01(\tR\adetails\"O\n" + "\x16EvaluationHistoryAlert\x12\x1b\n" + "\x06status\x18\x01 \x01(\tB\x03\xe0A\x02R\x06status\x12\x18\n" + - "\adetails\x18\x02 \x01(\tR\adetails\"\xc4\x01\n" + + "\adetails\x18\x02 \x01(\tR\adetails\"\xeb\x01\n" + "\x0eEntityInstance\x12\x0e\n" + "\x02id\x18\x01 \x01(\tR\x02id\x12.\n" + "\acontext\x18\x02 \x01(\v2\x14.minder.v1.ContextV2R\acontext\x12\x12\n" + @@ -15973,7 +15983,8 @@ const file_minder_v1_minder_proto_rawDesc = "" + "\x04type\x18\x04 \x01(\x0e2\x11.minder.v1.EntityR\x04type\x127\n" + "\n" + "properties\x18\x05 \x01(\v2\x17.google.protobuf.StructR\n" + - "properties\"\xa9\x01\n" + + "properties\x12%\n" + + "\x0eoriginating_id\x18\x06 \x01(\tR\roriginatingId\"\xa9\x01\n" + "\x13ListEntitiesRequest\x12.\n" + "\acontext\x18\x01 \x01(\v2\x14.minder.v1.ContextV2R\acontext\x127\n" + "\ventity_type\x18\x02 \x01(\x0e2\x11.minder.v1.EntityB\x03\xe0A\x02R\n" + diff --git a/proto/minder/v1/minder.proto b/proto/minder/v1/minder.proto index 183ee4b01b..acec4c829d 100644 --- a/proto/minder/v1/minder.proto +++ b/proto/minder/v1/minder.proto @@ -4156,6 +4156,10 @@ message EntityInstance { // properties is a map of properties of the entity. google.protobuf.Struct properties = 5; + + // originating_id is the id of the entity which originated this resource. + // If the originating entity is cleaned up, this entity will also be removed. + string originating_id = 6; } // EntityInstanceService provides API endpoints for managing entity instances