From 56cbde254b65ac27324a37b9da3a03a6034723e3 Mon Sep 17 00:00:00 2001 From: jaydeep869 Date: Mon, 13 Apr 2026 19:08:33 +0530 Subject: [PATCH 1/7] Add organization entity support Implements ENTITY_ORGANIZATION relying entirely on the new generic entity architecture to solve #5377 and unblock 2FA checks. Includes property fetcher, validator, and async Watermill organization auto-registration upon GitHub App installation. Also implements a backfill migration to synthesize missing organization associations for existing providers. --- cmd/server/app/backfill_organizations.go | 86 ++++++++++++++++++ cmd/server/app/migrate_up.go | 6 ++ .../000117_organization_entity.down.sql | 4 + .../000117_organization_entity.up.sql | 4 + docs/docs/ref/proto.mdx | 1 + internal/controlplane/handlers_entities.go | 31 +++++++ internal/controlplane/handlers_oauth.go | 19 +++- internal/controlplane/handlers_oauth_test.go | 2 +- internal/controlplane/handlers_user.go | 7 +- internal/controlplane/handlers_user_test.go | 6 +- internal/db/models.go | 1 + .../service/validators/organization.go | 30 ++++++ .../providers/github/properties/fetcher.go | 2 + .../github/properties/organization.go | 91 +++++++++++++++++++ .../providers/github/service/mock/service.go | 7 +- internal/providers/github/service/service.go | 16 ++-- .../providers/github/service/service_test.go | 4 +- internal/service/service.go | 2 + pkg/api/openapi/minder/v1/minder.swagger.json | 15 ++- pkg/api/protobuf/go/minder/v1/minder.pb.go | 8 +- pkg/entities/properties/constants_org.go | 14 +++ proto/minder/v1/minder.proto | 1 + 22 files changed, 330 insertions(+), 27 deletions(-) create mode 100644 cmd/server/app/backfill_organizations.go create mode 100644 database/migrations/000117_organization_entity.down.sql create mode 100644 database/migrations/000117_organization_entity.up.sql create mode 100644 internal/entities/service/validators/organization.go create mode 100644 internal/providers/github/properties/organization.go create mode 100644 pkg/entities/properties/constants_org.go diff --git a/cmd/server/app/backfill_organizations.go b/cmd/server/app/backfill_organizations.go new file mode 100644 index 0000000000..799655ca2f --- /dev/null +++ b/cmd/server/app/backfill_organizations.go @@ -0,0 +1,86 @@ +// SPDX-FileCopyrightText: Copyright 2026 The Minder Authors +// SPDX-License-Identifier: Apache-2.0 + +package app + +import ( + "context" + "database/sql" + "encoding/json" + "errors" + "fmt" + "strings" + + "github.com/rs/zerolog" + + "github.com/mindersec/minder/internal/db" + "github.com/mindersec/minder/pkg/entities/properties" +) + +func backfillOrganizations(ctx context.Context, store db.Store) error { + l := zerolog.Ctx(ctx) + l.Info().Msg("Starting backfill for Organization entities...") + + provs, err := store.GlobalListProvidersByClass(ctx, db.ProviderClassGithubApp) + if err != nil { + return fmt.Errorf("failed to list providers: %w", err) + } + + count := 0 + + for _, prov := range provs { + login := strings.TrimPrefix(prov.Name, string(db.ProviderClassGithubApp)+"-") + + _, err = db.WithTransaction(store, func(qtx db.ExtendQuerier) (any, error) { + // Check if organization entity already exists + _, err := qtx.GetEntityByName(ctx, db.GetEntityByNameParams{ + EntityType: db.EntitiesOrganization, + Name: login, + ProviderID: prov.ID, + ProjectID: prov.ProjectID, + }) + + if err == nil { + return nil, nil // already exists + } else if !errors.Is(err, sql.ErrNoRows) { + return nil, err + } + + // Entity doesn't exist, create it + ent, err := qtx.CreateEntity(ctx, db.CreateEntityParams{ + EntityType: db.EntitiesOrganization, + Name: login, + ProviderID: prov.ID, + ProjectID: prov.ProjectID, + }) + if err != nil { + return nil, err + } + + // Set the default property (login name) + propVal := map[string]any{ + "minder.internal.type": "string", + "minder.internal.value": login, + } + propBytes, _ := json.Marshal(propVal) + + _, err = qtx.UpsertProperty(ctx, db.UpsertPropertyParams{ + EntityID: ent.ID, + Key: properties.PropertyName, + Value: propBytes, + }) + + if err == nil { + count++ + } + return nil, err + }) + + if err != nil { + l.Error().Err(err).Str("provider", prov.ID.String()).Msg("Failed to backfill organization for provider") + } + } + + l.Info().Int("count", count).Msg("Completed backfill for Organization entities") + return nil +} diff --git a/cmd/server/app/migrate_up.go b/cmd/server/app/migrate_up.go index ef813cd9ed..aff4a48046 100644 --- a/cmd/server/app/migrate_up.go +++ b/cmd/server/app/migrate_up.go @@ -18,6 +18,7 @@ import ( "github.com/mindersec/minder/database" "github.com/mindersec/minder/internal/authz" + "github.com/mindersec/minder/internal/db" "github.com/mindersec/minder/pkg/config" serverconfig "github.com/mindersec/minder/pkg/config/server" ) @@ -98,6 +99,11 @@ var upCmd = &cobra.Command{ return fmt.Errorf("error preparing authz client: %w", err) } + cmd.Println("Backfilling organizations...") + if err := backfillOrganizations(ctx, db.NewStore(dbConn)); err != nil { + return fmt.Errorf("error while backfilling organizations: %w", err) + } + return nil }, } diff --git a/database/migrations/000117_organization_entity.down.sql b/database/migrations/000117_organization_entity.down.sql new file mode 100644 index 0000000000..6292dc986a --- /dev/null +++ b/database/migrations/000117_organization_entity.down.sql @@ -0,0 +1,4 @@ +-- SPDX-FileCopyrightText: Copyright 2026 The Minder Authors +-- SPDX-License-Identifier: Apache-2.0 + +-- Postgres doesn't easily drop enum values, down migration is a no-op diff --git a/database/migrations/000117_organization_entity.up.sql b/database/migrations/000117_organization_entity.up.sql new file mode 100644 index 0000000000..2680350dcd --- /dev/null +++ b/database/migrations/000117_organization_entity.up.sql @@ -0,0 +1,4 @@ +-- SPDX-FileCopyrightText: Copyright 2026 The Minder Authors +-- SPDX-License-Identifier: Apache-2.0 + +ALTER TYPE entities ADD VALUE 'organization'; diff --git a/docs/docs/ref/proto.mdx b/docs/docs/ref/proto.mdx index 2fd42324b6..59b134c4a1 100644 --- a/docs/docs/ref/proto.mdx +++ b/docs/docs/ref/proto.mdx @@ -3291,6 +3291,7 @@ Entity defines the entity that is supported by the provider. | ENTITY_PIPELINE_RUN | 6 | | | ENTITY_TASK_RUN | 7 | | | ENTITY_BUILD | 8 | | +| ENTITY_ORGANIZATION | 9 | | diff --git a/internal/controlplane/handlers_entities.go b/internal/controlplane/handlers_entities.go index d875647d87..2fee967dd5 100644 --- a/internal/controlplane/handlers_entities.go +++ b/internal/controlplane/handlers_entities.go @@ -127,3 +127,34 @@ func createEntityMessage( return msg, nil } + +func (s *Server) publishOrganizationEntityEvent( + ctx context.Context, + providerID, projectID uuid.UUID, + login string, +) { + l := zerolog.Ctx(ctx) + msg := message.NewMessage(uuid.New().String(), nil) + msg.SetContext(ctx) + + orgProps := properties.NewProperties(map[string]any{ + properties.PropertyName: login, + }) + + event := messages.NewMinderEvent(). + WithProjectID(projectID). + WithProviderID(providerID). + WithEntityType(pb.Entity_ENTITY_ORGANIZATION). + WithProperties(orgProps) + + if err := event.ToMessage(msg); err != nil { + l.Error().Err(err).Msg("error marshalling organization entity event") + return + } + + if err := s.evt.Publish(constants.TopicQueueReconcileEntityAdd, msg); err != nil { + l.Error().Err(err).Msg("error publishing organization entity event") + } else { + l.Info().Str("messageID", msg.UUID).Msg("published organization entity event for execution") + } +} diff --git a/internal/controlplane/handlers_oauth.go b/internal/controlplane/handlers_oauth.go index 05291704c2..b913a670fa 100644 --- a/internal/controlplane/handlers_oauth.go +++ b/internal/controlplane/handlers_oauth.go @@ -431,7 +431,7 @@ func (s *Server) processAppCallback(ctx context.Context, w http.ResponseWriter, logger.BusinessRecord(ctx).Project = stateData.ProjectID var confErr providers.ErrProviderInvalidConfig - _, err = s.ghProviders.CreateGitHubAppProvider(ctx, *token, stateData, installationID, state) + dbProv, err := s.ghProviders.CreateGitHubAppProvider(ctx, *token, stateData, installationID, state) if err != nil { if errors.As(err, &confErr) { return newHttpError(http.StatusBadRequest, "Invalid provider config").SetContents( @@ -444,6 +444,11 @@ func (s *Server) processAppCallback(ctx context.Context, w http.ResponseWriter, return fmt.Errorf("error creating GitHub App provider: %w", err) } + if dbProv != nil { + login := strings.TrimPrefix(dbProv.Name, string(db.ProviderClassGithubApp)+"-") + s.publishOrganizationEntityEvent(ctx, dbProv.ID, dbProv.ProjectID, login) + } + if stateData.RedirectUrl.Valid || stateData.EncryptedRedirect.Valid { redirectURL, err := s.decryptRedirect(&stateData) if err != nil { @@ -535,7 +540,17 @@ func (s *Server) handleAppInstallWithoutInvite(ctx context.Context, token *oauth } _, err = db.WithTransaction(s.store, func(qtx db.ExtendQuerier) (*db.Project, error) { - return s.ghProviders.CreateGitHubAppWithoutInvitation(ctx, qtx, *userID, installationID) + proj, dbProv, err := s.ghProviders.CreateGitHubAppWithoutInvitation(ctx, qtx, *userID, installationID) + if err != nil { + return nil, err + } + if dbProv != nil && proj != nil { + login := strings.TrimPrefix(dbProv.Name, string(db.ProviderClassGithubApp)+"-") + // It is generally safe to publish an event from within a transaction, as long + // as the event handler evaluates the state matching later. + s.publishOrganizationEntityEvent(ctx, dbProv.ID, proj.ID, login) + } + return proj, nil }) return err } diff --git a/internal/controlplane/handlers_oauth_test.go b/internal/controlplane/handlers_oauth_test.go index 4b829a1d9e..69ecc16a06 100644 --- a/internal/controlplane/handlers_oauth_test.go +++ b/internal/controlplane/handlers_oauth_test.go @@ -812,7 +812,7 @@ func TestHandleGitHubAppCallback(t *testing.T) { db.EXPECT().Rollback(gomock.Any()).Return(nil) service.EXPECT(). CreateGitHubAppWithoutInvitation(gomock.Any(), gomock.Any(), userId, installationID). - Return(nil, nil) + Return(nil, nil, nil) }, checkResponse: func(t *testing.T, resp httptest.ResponseRecorder) { t.Helper() diff --git a/internal/controlplane/handlers_user.go b/internal/controlplane/handlers_user.go index 11726299f4..57befff4d0 100644 --- a/internal/controlplane/handlers_user.go +++ b/internal/controlplane/handlers_user.go @@ -12,6 +12,7 @@ import ( "net/http" "path" "strconv" + "strings" "github.com/google/uuid" gauth "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/auth" @@ -134,11 +135,15 @@ func (s *Server) claimGitHubInstalls(ctx context.Context, qtx db.ExtendQuerier) for _, i := range installs { // TODO: if we can get an GitHub auth token for the user, we can do the rest with CreateGitHubAppWithoutInvitation - proj, err := s.ghProviders.CreateGitHubAppWithoutInvitation(ctx, qtx, userID, i.AppInstallationID) + proj, dbProv, err := s.ghProviders.CreateGitHubAppWithoutInvitation(ctx, qtx, userID, i.AppInstallationID) if err != nil { zerolog.Ctx(ctx).Error().Err(err).Int64("org_id", i.OrganizationID).Msg("failed to create GitHub app at first login") continue } + if dbProv != nil { + login := strings.TrimPrefix(dbProv.Name, string(db.ProviderClassGithubApp)+"-") + s.publishOrganizationEntityEvent(ctx, dbProv.ID, proj.ID, login) + } if proj != nil { userProjects = append(userProjects, proj) } diff --git a/internal/controlplane/handlers_user_test.go b/internal/controlplane/handlers_user_test.go index e1a14ea446..88332d2be3 100644 --- a/internal/controlplane/handlers_user_test.go +++ b/internal/controlplane/handlers_user_test.go @@ -145,7 +145,7 @@ func TestCreateUser_gRPC(t *testing.T) { Return(&db.Project{ ID: projectID, Name: "github-org1", - }, nil) + }, nil, nil) store.EXPECT().Commit(gomock.Any()) store.EXPECT().Rollback(gomock.Any()) @@ -206,14 +206,14 @@ func TestCreateUser_gRPC(t *testing.T) { Return(&db.Project{ ID: projectID, Name: "github-org1", - }, nil) + }, nil, nil) prov.EXPECT(). CreateGitHubAppWithoutInvitation(gomock.Any(), gomock.Any(), int64(31337), int64(11)). Return(&db.Project{ ID: uuid.New(), Name: "github-org2", - }, nil) + }, nil, nil) store.EXPECT().Commit(gomock.Any()) store.EXPECT().Rollback(gomock.Any()) diff --git a/internal/db/models.go b/internal/db/models.go index 70d8df5caf..c2f8a128ca 100644 --- a/internal/db/models.go +++ b/internal/db/models.go @@ -158,6 +158,7 @@ const ( EntitiesPipelineRun Entities = "pipeline_run" EntitiesTaskRun Entities = "task_run" EntitiesBuild Entities = "build" + EntitiesOrganization Entities = "organization" ) func (e *Entities) Scan(src interface{}) error { diff --git a/internal/entities/service/validators/organization.go b/internal/entities/service/validators/organization.go new file mode 100644 index 0000000000..2be23c3bcb --- /dev/null +++ b/internal/entities/service/validators/organization.go @@ -0,0 +1,30 @@ +// SPDX-FileCopyrightText: Copyright 2026 The Minder Authors +// SPDX-License-Identifier: Apache-2.0 + +package validators + +import ( +"context" + +"github.com/google/uuid" + +"github.com/mindersec/minder/pkg/entities/properties" +) + +// OrganizationValidator validates organization entity creation +type OrganizationValidator struct{} + +// NewOrganizationValidator creates a new OrganizationValidator +func NewOrganizationValidator() *OrganizationValidator { +return &OrganizationValidator{} +} + +// Validate checks if an organization entity can be created +func (v *OrganizationValidator) Validate( +_ context.Context, +_ *properties.Properties, +_ uuid.UUID, +) error { +// For now, any organization properties that make it this far are valid +return nil +} diff --git a/internal/providers/github/properties/fetcher.go b/internal/providers/github/properties/fetcher.go index 3296106e4d..f6816b8f46 100644 --- a/internal/providers/github/properties/fetcher.go +++ b/internal/providers/github/properties/fetcher.go @@ -54,6 +54,8 @@ func (ghEntityFetcher) EntityPropertyFetcher(entType minderv1.Entity) GhProperty return NewArtifactFetcher() case minderv1.Entity_ENTITY_RELEASE: return NewReleaseFetcher() + case minderv1.Entity_ENTITY_ORGANIZATION: + return NewOrganizationFetcher() } return nil diff --git a/internal/providers/github/properties/organization.go b/internal/providers/github/properties/organization.go new file mode 100644 index 0000000000..93da7e2396 --- /dev/null +++ b/internal/providers/github/properties/organization.go @@ -0,0 +1,91 @@ +// SPDX-FileCopyrightText: Copyright 2026 The Minder Authors +// SPDX-License-Identifier: Apache-2.0 + +package properties + +import ( + "context" + "fmt" + "strconv" + + go_github "github.com/google/go-github/v63/github" + + "github.com/mindersec/minder/pkg/entities/properties" +) + +// OrganizationFetcher is a GhPropertyFetcher for organizations +type OrganizationFetcher struct { + propertyFetcherBase +} + +// NewOrganizationFetcher creates a new OrganizationFetcher +func NewOrganizationFetcher() *OrganizationFetcher { + return &OrganizationFetcher{ + propertyFetcherBase: propertyFetcherBase{ + propertyOrigins: []propertyOrigin{ + { + keys: []string{ + properties.PropertyUpstreamID, + properties.PropertyName, + properties.OrgPropertyIsUser, + properties.OrgPropertyAvatarURL, + properties.OrgPropertyCompany, + }, + wrapper: fetchOrganizationProperties, + }, + }, + }, + } +} + +// GetName returns the name of the organization +func (*OrganizationFetcher) GetName(props *properties.Properties) (string, error) { + name := props.GetProperty(properties.PropertyName).GetString() + if name == "" { + return "", fmt.Errorf("missing property: %s", properties.PropertyName) + } + return name, nil +} + +func fetchOrganizationProperties( + ctx context.Context, ghCli *go_github.Client, _ bool, lookupProperties *properties.Properties, +) (map[string]any, error) { + // We can look up by either exact upstream ID or by name (login). + var user *go_github.User + var err error + + if idStr := lookupProperties.GetProperty(properties.PropertyUpstreamID).GetString(); idStr != "" { + id, parseErr := strconv.ParseInt(idStr, 10, 64) + if parseErr != nil { + return nil, fmt.Errorf("invalid upstream ID: %w", parseErr) + } + user, _, err = ghCli.Users.GetByID(ctx, id) + } else if name := lookupProperties.GetProperty(properties.PropertyName).GetString(); name != "" { + user, _, err = ghCli.Users.Get(ctx, name) + } else { + return nil, fmt.Errorf("either upstream_id or name (login) must be provided to fetch an organization") + } + + if err != nil { + return nil, err + } + + if user == nil { + return nil, fmt.Errorf("organization/user not found") + } + + result := map[string]any{ + properties.PropertyUpstreamID: properties.NumericalValueToUpstreamID(user.GetID()), + properties.PropertyName: user.GetLogin(), + properties.OrgPropertyIsUser: user.GetType() == "User", + } + + if user.AvatarURL != nil { + result[properties.OrgPropertyAvatarURL] = *user.AvatarURL + } + if user.Company != nil { + result[properties.OrgPropertyCompany] = *user.Company + } + + return result, nil +} diff --git a/internal/providers/github/service/mock/service.go b/internal/providers/github/service/mock/service.go index cc74916dc4..c0cbe9f62c 100644 --- a/internal/providers/github/service/mock/service.go +++ b/internal/providers/github/service/mock/service.go @@ -60,12 +60,13 @@ func (mr *MockGitHubProviderServiceMockRecorder) CreateGitHubAppProvider(ctx, to } // CreateGitHubAppWithoutInvitation mocks base method. -func (m *MockGitHubProviderService) CreateGitHubAppWithoutInvitation(ctx context.Context, qtx db.ExtendQuerier, userID, installationID int64) (*db.Project, error) { +func (m *MockGitHubProviderService) CreateGitHubAppWithoutInvitation(ctx context.Context, qtx db.ExtendQuerier, userID, installationID int64) (*db.Project, *db.Provider, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "CreateGitHubAppWithoutInvitation", ctx, qtx, userID, installationID) ret0, _ := ret[0].(*db.Project) - ret1, _ := ret[1].(error) - return ret0, ret1 + ret1, _ := ret[1].(*db.Provider) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 } // CreateGitHubAppWithoutInvitation indicates an expected call of CreateGitHubAppWithoutInvitation. diff --git a/internal/providers/github/service/service.go b/internal/providers/github/service/service.go index 68b1c3ba99..5c0372c208 100644 --- a/internal/providers/github/service/service.go +++ b/internal/providers/github/service/service.go @@ -42,7 +42,7 @@ type GitHubProviderService interface { // // Note that this function may return nil, nil if the installation user is not known to Minder. CreateGitHubAppWithoutInvitation(ctx context.Context, qtx db.ExtendQuerier, userID int64, - installationID int64) (*db.Project, error) + installationID int64) (*db.Project, *db.Provider, error) // ValidateGitHubInstallationId checks if the supplied GitHub token has access to the installation ID ValidateGitHubInstallationId(ctx context.Context, token *oauth2.Token, installationID int64) error // DeleteGitHubAppInstallation deletes the GitHub App installation and provider from the database. @@ -170,10 +170,10 @@ func (p *ghProviderService) CreateGitHubAppWithoutInvitation( qtx db.ExtendQuerier, userID int64, installationID int64, -) (*db.Project, error) { +) (*db.Project, *db.Provider, error) { installationOwner, err := p.getInstallationOwner(ctx, installationID) if err != nil { - return nil, err + return nil, nil, err } isOrg := installationOwner.GetType() == TypeGitHubOrganization @@ -198,22 +198,22 @@ func (p *ghProviderService) CreateGitHubAppWithoutInvitation( IsOrg: isOrg, }) if err != nil { - return nil, fmt.Errorf("error saving installation ID: %w", err) + return nil, nil, fmt.Errorf("error saving installation ID: %w", err) } - return nil, nil + return nil, nil, nil } zerolog.Ctx(ctx).Info().Str("project", project.ID.String()).Int64("owner", installationOwner.GetID()). Msg("Creating GitHub App Provider") - _, err = createGitHubApp( + provider, err := createGitHubApp( ctx, qtx, project.ID, installationOwner, installationID, json.RawMessage(`{"github-app": {}}`), nil, sql.NullString{}) if err != nil { - return nil, fmt.Errorf("error creating GitHub App Provider: %w", err) + return nil, nil, fmt.Errorf("error creating GitHub App Provider: %w", err) } - return project, err + return project, &provider, err } // Internal shared implementation between CreateGitHubAppProvider and CreateGitHubAppWithoutInvitation. diff --git a/internal/providers/github/service/service_test.go b/internal/providers/github/service/service_test.go index 2687d02077..49b6361947 100644 --- a/internal/providers/github/service/service_test.go +++ b/internal/providers/github/service/service_test.go @@ -332,7 +332,7 @@ func TestProviderService_CreateGitHubAppWithNewProject(t *testing.T) { }, }, nil, nil) - project, err := provSvc.CreateGitHubAppWithoutInvitation( + project, _, err := provSvc.CreateGitHubAppWithoutInvitation( context.Background(), mocks.fakeStore, accountID, installationID) require.NoError(t, err) require.NotNil(t, project) @@ -389,7 +389,7 @@ func TestProviderService_CreateUnclaimedGitHubAppInstallation(t *testing.T) { }, }, nil, nil) - project, err := provSvc.CreateGitHubAppWithoutInvitation( + project, _, err := provSvc.CreateGitHubAppWithoutInvitation( context.Background(), mocks.fakeStore, accountID, installationID) require.NoError(t, err) require.Nil(t, project) diff --git a/internal/service/service.go b/internal/service/service.go index 87856e9d82..6a1c6585b6 100644 --- a/internal/service/service.go +++ b/internal/service/service.go @@ -178,6 +178,8 @@ func AllInOneServerService( validatorRegistry := validators.NewValidatorRegistry() repoValidator := validators.NewRepositoryValidator(store) validatorRegistry.AddValidator(pb.Entity_ENTITY_REPOSITORIES, repoValidator) + orgValidator := validators.NewOrganizationValidator() + validatorRegistry.AddValidator(pb.Entity_ENTITY_ORGANIZATION, orgValidator) // Create entity creator entityCreator := entityService.NewEntityCreator( diff --git a/pkg/api/openapi/minder/v1/minder.swagger.json b/pkg/api/openapi/minder/v1/minder.swagger.json index c3fbcc8f72..f3d9b1c61e 100644 --- a/pkg/api/openapi/minder/v1/minder.swagger.json +++ b/pkg/api/openapi/minder/v1/minder.swagger.json @@ -800,7 +800,8 @@ "ENTITY_RELEASE", "ENTITY_PIPELINE_RUN", "ENTITY_TASK_RUN", - "ENTITY_BUILD" + "ENTITY_BUILD", + "ENTITY_ORGANIZATION" ], "default": "ENTITY_UNSPECIFIED" }, @@ -958,7 +959,8 @@ "ENTITY_RELEASE", "ENTITY_PIPELINE_RUN", "ENTITY_TASK_RUN", - "ENTITY_BUILD" + "ENTITY_BUILD", + "ENTITY_ORGANIZATION" ] }, { @@ -1600,7 +1602,8 @@ "ENTITY_RELEASE", "ENTITY_PIPELINE_RUN", "ENTITY_TASK_RUN", - "ENTITY_BUILD" + "ENTITY_BUILD", + "ENTITY_ORGANIZATION" ], "default": "ENTITY_UNSPECIFIED" }, @@ -1846,7 +1849,8 @@ "ENTITY_RELEASE", "ENTITY_PIPELINE_RUN", "ENTITY_TASK_RUN", - "ENTITY_BUILD" + "ENTITY_BUILD", + "ENTITY_ORGANIZATION" ], "default": "ENTITY_UNSPECIFIED" }, @@ -4517,7 +4521,8 @@ "ENTITY_RELEASE", "ENTITY_PIPELINE_RUN", "ENTITY_TASK_RUN", - "ENTITY_BUILD" + "ENTITY_BUILD", + "ENTITY_ORGANIZATION" ], "default": "ENTITY_UNSPECIFIED", "description": "Entity defines the entity that is supported by the provider." diff --git a/pkg/api/protobuf/go/minder/v1/minder.pb.go b/pkg/api/protobuf/go/minder/v1/minder.pb.go index 1a21604a64..7276d2f088 100644 --- a/pkg/api/protobuf/go/minder/v1/minder.pb.go +++ b/pkg/api/protobuf/go/minder/v1/minder.pb.go @@ -322,6 +322,7 @@ const ( Entity_ENTITY_PIPELINE_RUN Entity = 6 Entity_ENTITY_TASK_RUN Entity = 7 Entity_ENTITY_BUILD Entity = 8 + Entity_ENTITY_ORGANIZATION Entity = 9 ) // Enum value maps for Entity. @@ -336,6 +337,7 @@ var ( 6: "ENTITY_PIPELINE_RUN", 7: "ENTITY_TASK_RUN", 8: "ENTITY_BUILD", + 9: "ENTITY_ORGANIZATION", } Entity_value = map[string]int32{ "ENTITY_UNSPECIFIED": 0, @@ -347,6 +349,7 @@ var ( "ENTITY_PIPELINE_RUN": 6, "ENTITY_TASK_RUN": 7, "ENTITY_BUILD": 8, + "ENTITY_ORGANIZATION": 9, } ) @@ -15887,7 +15890,7 @@ const file_minder_v1_minder_proto_rawDesc = "" + "\x1bTARGET_RESOURCE_UNSPECIFIED\x10\x00\x12\x18\n" + "\x14TARGET_RESOURCE_NONE\x10\x01\x12\x18\n" + "\x14TARGET_RESOURCE_USER\x10\x02\x12\x1b\n" + - "\x17TARGET_RESOURCE_PROJECT\x10\x03*\xdc\x01\n" + + "\x17TARGET_RESOURCE_PROJECT\x10\x03*\xf5\x01\n" + "\x06Entity\x12\x16\n" + "\x12ENTITY_UNSPECIFIED\x10\x00\x12\x17\n" + "\x13ENTITY_REPOSITORIES\x10\x01\x12\x1d\n" + @@ -15897,7 +15900,8 @@ const file_minder_v1_minder_proto_rawDesc = "" + "\x0eENTITY_RELEASE\x10\x05\x12\x17\n" + "\x13ENTITY_PIPELINE_RUN\x10\x06\x12\x13\n" + "\x0fENTITY_TASK_RUN\x10\a\x12\x10\n" + - "\fENTITY_BUILD\x10\b*\xf9\x01\n" + + "\fENTITY_BUILD\x10\b\x12\x17\n" + + "\x13ENTITY_ORGANIZATION\x10\t*\xf9\x01\n" + "\x14RuleTypeReleasePhase\x12'\n" + "#RULE_TYPE_RELEASE_PHASE_UNSPECIFIED\x10\x00\x12,\n" + "\x1dRULE_TYPE_RELEASE_PHASE_ALPHA\x10\x01\x1a\t\xea\xdc\x14\x05alpha\x12*\n" + diff --git a/pkg/entities/properties/constants_org.go b/pkg/entities/properties/constants_org.go new file mode 100644 index 0000000000..e5bbe99ede --- /dev/null +++ b/pkg/entities/properties/constants_org.go @@ -0,0 +1,14 @@ +// SPDX-FileCopyrightText: Copyright 2026 The Minder Authors +// SPDX-License-Identifier: Apache-2.0 + +package properties + +// Organization property keys +const ( +// OrgPropertyIsUser represents whether the organization is actually a user account +OrgPropertyIsUser = "is_user" +// OrgPropertyAvatarURL represents the avatar URL of the organization +OrgPropertyAvatarURL = "avatar_url" +// OrgPropertyCompany represents the company name of the organization +OrgPropertyCompany = "company" +) diff --git a/proto/minder/v1/minder.proto b/proto/minder/v1/minder.proto index 8760ccd017..306064972e 100644 --- a/proto/minder/v1/minder.proto +++ b/proto/minder/v1/minder.proto @@ -2030,6 +2030,7 @@ enum Entity { ENTITY_PIPELINE_RUN = 6; ENTITY_TASK_RUN = 7; ENTITY_BUILD = 8; + ENTITY_ORGANIZATION = 9; } message EntityAutoRegistrationConfig { From 4000a990a79b910ee2595222fc7b3a50bda248d8 Mon Sep 17 00:00:00 2001 From: jaydeep869 Date: Mon, 13 Apr 2026 19:40:33 +0530 Subject: [PATCH 2/7] Fix lint, exhaustive cases and test panics - Addresses exhaustive switch cases missed in initial entity setup - Fixes cyclomatic complexity warning in processAppCallback by extracting error handling - Fixes gh provider initialization panic by adding support for organization in RegisterEntity --- internal/controlplane/handlers_evalstatus.go | 2 ++ internal/controlplane/handlers_oauth.go | 24 +++++++++++-------- internal/controlplane/handlers_profile.go | 2 +- internal/eea/eea.go | 2 +- internal/engine/entities/entities.go | 4 ++++ .../engine/entities/entity_type_conversion.go | 2 ++ .../service/validators/organization.go | 20 ++++++++-------- internal/logger/telemetry_store_watermill.go | 2 +- internal/providers/github/entities.go | 2 ++ .../providers/github/service/service_test.go | 2 +- pkg/api/protobuf/go/minder/v1/entities.go | 2 +- pkg/entities/properties/constants_org.go | 12 +++++----- pkg/profiles/util.go | 3 +++ 13 files changed, 48 insertions(+), 31 deletions(-) diff --git a/internal/controlplane/handlers_evalstatus.go b/internal/controlplane/handlers_evalstatus.go index 41a78b8fe9..ae2eca1e3d 100644 --- a/internal/controlplane/handlers_evalstatus.go +++ b/internal/controlplane/handlers_evalstatus.go @@ -768,6 +768,8 @@ func dbEntityToEntity(dbEnt db.Entities) minderv1.Entity { switch dbEnt { case db.EntitiesPullRequest: return minderv1.Entity_ENTITY_PULL_REQUESTS + case db.EntitiesOrganization: + return minderv1.Entity_ENTITY_ORGANIZATION case db.EntitiesArtifact: return minderv1.Entity_ENTITY_ARTIFACTS case db.EntitiesRepository: diff --git a/internal/controlplane/handlers_oauth.go b/internal/controlplane/handlers_oauth.go index 40ec97a392..dfed3b8953 100644 --- a/internal/controlplane/handlers_oauth.go +++ b/internal/controlplane/handlers_oauth.go @@ -436,18 +436,9 @@ func (s *Server) processAppCallback(ctx context.Context, w http.ResponseWriter, logger.BusinessRecord(ctx).Project = stateData.ProjectID - var confErr providers.ErrProviderInvalidConfig dbProv, err := s.ghProviders.CreateGitHubAppProvider(ctx, *token, stateData, installationID, state) if err != nil { - if errors.As(err, &confErr) { - return newHttpError(http.StatusBadRequest, "Invalid provider config").SetContents( - "The provider configuration is invalid: %s", confErr.Details) - } - if errors.Is(err, service.ErrInvalidTokenIdentity) { - return newHttpError(http.StatusForbidden, "User token mismatch").SetContents( - "The provided login token was associated with a different GitHub user.") - } - return fmt.Errorf("error creating GitHub App provider: %w", err) + return handleProviderCreationError(err) } if dbProv != nil { @@ -802,3 +793,16 @@ func (s *Server) decryptRedirect(stateData *db.GetProjectIDBySessionStateRow) (* } return parsedURL, nil } + +func handleProviderCreationError(err error) error { + var confErr providers.ErrProviderInvalidConfig + if errors.As(err, &confErr) { + return newHttpError(http.StatusBadRequest, "Invalid provider config").SetContents( + "The provider configuration is invalid: %s", confErr.Details) + } + if errors.Is(err, service.ErrInvalidTokenIdentity) { + return newHttpError(http.StatusForbidden, "User token mismatch").SetContents( + "The provided login token was associated with a different GitHub user.") + } + return fmt.Errorf("error creating GitHub App provider: %w", err) +} diff --git a/internal/controlplane/handlers_profile.go b/internal/controlplane/handlers_profile.go index 93765fe322..332a49de2e 100644 --- a/internal/controlplane/handlers_profile.go +++ b/internal/controlplane/handlers_profile.go @@ -427,7 +427,7 @@ func (s *Server) getRuleEvalStatus( repoPath = fmt.Sprintf("%s/%s", prRepoOwner, prRepoName) } case db.EntitiesBuildEnvironment, db.EntitiesRelease, db.EntitiesPipelineRun, - db.EntitiesTaskRun, db.EntitiesBuild: + db.EntitiesTaskRun, db.EntitiesBuild, db.EntitiesOrganization: zerolog.Ctx(ctx).Warn().Msgf("attempting to set alerts for unsupported entity type: %v", dbRuleEvalStat.EntityType) default: zerolog.Ctx(ctx).Error().Msgf("unknown entity type: %v", dbRuleEvalStat.EntityType) diff --git a/internal/eea/eea.go b/internal/eea/eea.go index 6f2d0fa55b..0b758ea56f 100644 --- a/internal/eea/eea.go +++ b/internal/eea/eea.go @@ -255,7 +255,7 @@ func (e *EEA) buildEntityWrapper( case db.EntitiesPullRequest: return e.buildPullRequestInfoWrapper(ctx, entityID, projID) case db.EntitiesBuildEnvironment, db.EntitiesRelease, - db.EntitiesPipelineRun, db.EntitiesTaskRun, db.EntitiesBuild: + db.EntitiesPipelineRun, db.EntitiesTaskRun, db.EntitiesBuild, db.EntitiesOrganization: return nil, fmt.Errorf("entity type %q not yet supported", entity) default: return nil, fmt.Errorf("unknown entity type: %q", entity) diff --git a/internal/engine/entities/entities.go b/internal/engine/entities/entities.go index 3f1e1eb8f2..87778a7857 100644 --- a/internal/engine/entities/entities.go +++ b/internal/engine/entities/entities.go @@ -50,6 +50,8 @@ func EntityTypeFromDB(entity db.Entities) minderv1.Entity { return minderv1.Entity_ENTITY_PIPELINE_RUN case db.EntitiesTaskRun: return minderv1.Entity_ENTITY_TASK_RUN + case db.EntitiesOrganization: + return minderv1.Entity_ENTITY_ORGANIZATION case db.EntitiesBuild: return minderv1.Entity_ENTITY_BUILD default: @@ -76,6 +78,8 @@ func EntityTypeToDB(entity minderv1.Entity) db.Entities { dbEnt = db.EntitiesPipelineRun case minderv1.Entity_ENTITY_TASK_RUN: dbEnt = db.EntitiesTaskRun + case minderv1.Entity_ENTITY_ORGANIZATION: + dbEnt = db.EntitiesOrganization case minderv1.Entity_ENTITY_BUILD: dbEnt = db.EntitiesBuild case minderv1.Entity_ENTITY_UNSPECIFIED: diff --git a/internal/engine/entities/entity_type_conversion.go b/internal/engine/entities/entity_type_conversion.go index 1d2b001291..b843fe7dfb 100644 --- a/internal/engine/entities/entity_type_conversion.go +++ b/internal/engine/entities/entity_type_conversion.go @@ -28,6 +28,8 @@ func EntityTypeToDBType(entityType pb.Entity) (db.Entities, error) { return db.EntitiesPipelineRun, nil case pb.Entity_ENTITY_TASK_RUN: return db.EntitiesTaskRun, nil + case pb.Entity_ENTITY_ORGANIZATION: + return db.EntitiesOrganization, nil case pb.Entity_ENTITY_BUILD: return db.EntitiesBuild, nil case pb.Entity_ENTITY_UNSPECIFIED: diff --git a/internal/entities/service/validators/organization.go b/internal/entities/service/validators/organization.go index 2be23c3bcb..e7f4927dc6 100644 --- a/internal/entities/service/validators/organization.go +++ b/internal/entities/service/validators/organization.go @@ -4,11 +4,11 @@ package validators import ( -"context" + "context" -"github.com/google/uuid" + "github.com/google/uuid" -"github.com/mindersec/minder/pkg/entities/properties" + "github.com/mindersec/minder/pkg/entities/properties" ) // OrganizationValidator validates organization entity creation @@ -16,15 +16,15 @@ type OrganizationValidator struct{} // NewOrganizationValidator creates a new OrganizationValidator func NewOrganizationValidator() *OrganizationValidator { -return &OrganizationValidator{} + return &OrganizationValidator{} } // Validate checks if an organization entity can be created -func (v *OrganizationValidator) Validate( -_ context.Context, -_ *properties.Properties, -_ uuid.UUID, +func (*OrganizationValidator) Validate( + _ context.Context, + _ *properties.Properties, + _ uuid.UUID, ) error { -// For now, any organization properties that make it this far are valid -return nil + // For now, any organization properties that make it this far are valid + return nil } diff --git a/internal/logger/telemetry_store_watermill.go b/internal/logger/telemetry_store_watermill.go index 096f734cf2..233a55e7bb 100644 --- a/internal/logger/telemetry_store_watermill.go +++ b/internal/logger/telemetry_store_watermill.go @@ -82,7 +82,7 @@ func newTelemetryStoreFromEntity(inf *entities.EntityInfoWrapper) (*TelemetrySto ts.PullRequest = ent case minderv1.Entity_ENTITY_BUILD_ENVIRONMENTS, minderv1.Entity_ENTITY_RELEASE, minderv1.Entity_ENTITY_PIPELINE_RUN, - minderv1.Entity_ENTITY_TASK_RUN, minderv1.Entity_ENTITY_BUILD: + minderv1.Entity_ENTITY_TASK_RUN, minderv1.Entity_ENTITY_BUILD, minderv1.Entity_ENTITY_ORGANIZATION: // Noop, see https://github.com/mindersec/minder/issues/3838 case minderv1.Entity_ENTITY_UNSPECIFIED: // Do nothing diff --git a/internal/providers/github/entities.go b/internal/providers/github/entities.go index 7e2251b78d..f47048bc3a 100644 --- a/internal/providers/github/entities.go +++ b/internal/providers/github/entities.go @@ -82,6 +82,8 @@ func (c *GitHub) RegisterEntity( case minderv1.Entity_ENTITY_ARTIFACTS: fallthrough case minderv1.Entity_ENTITY_RELEASE: + fallthrough + case minderv1.Entity_ENTITY_ORGANIZATION: // Nothing to do, accept: return props, nil case minderv1.Entity_ENTITY_REPOSITORIES: diff --git a/internal/providers/github/service/service_test.go b/internal/providers/github/service/service_test.go index 49b6361947..56161e9501 100644 --- a/internal/providers/github/service/service_test.go +++ b/internal/providers/github/service/service_test.go @@ -332,7 +332,7 @@ func TestProviderService_CreateGitHubAppWithNewProject(t *testing.T) { }, }, nil, nil) - project, _, err := provSvc.CreateGitHubAppWithoutInvitation( + project, _, err := provSvc.CreateGitHubAppWithoutInvitation( context.Background(), mocks.fakeStore, accountID, installationID) require.NoError(t, err) require.NotNil(t, project) diff --git a/pkg/api/protobuf/go/minder/v1/entities.go b/pkg/api/protobuf/go/minder/v1/entities.go index d1b11c78e8..d5d83daacc 100644 --- a/pkg/api/protobuf/go/minder/v1/entities.go +++ b/pkg/api/protobuf/go/minder/v1/entities.go @@ -67,7 +67,7 @@ func (entity Entity) IsValid() bool { case Entity_ENTITY_REPOSITORIES, Entity_ENTITY_BUILD_ENVIRONMENTS, Entity_ENTITY_ARTIFACTS, Entity_ENTITY_PULL_REQUESTS, Entity_ENTITY_RELEASE, Entity_ENTITY_PIPELINE_RUN, - Entity_ENTITY_TASK_RUN, Entity_ENTITY_BUILD: + Entity_ENTITY_TASK_RUN, Entity_ENTITY_BUILD, Entity_ENTITY_ORGANIZATION: return true case Entity_ENTITY_UNSPECIFIED: return false diff --git a/pkg/entities/properties/constants_org.go b/pkg/entities/properties/constants_org.go index e5bbe99ede..ee06a98f57 100644 --- a/pkg/entities/properties/constants_org.go +++ b/pkg/entities/properties/constants_org.go @@ -5,10 +5,10 @@ package properties // Organization property keys const ( -// OrgPropertyIsUser represents whether the organization is actually a user account -OrgPropertyIsUser = "is_user" -// OrgPropertyAvatarURL represents the avatar URL of the organization -OrgPropertyAvatarURL = "avatar_url" -// OrgPropertyCompany represents the company name of the organization -OrgPropertyCompany = "company" + // OrgPropertyIsUser represents whether the organization is actually a user account + OrgPropertyIsUser = "is_user" + // OrgPropertyAvatarURL represents the avatar URL of the organization + OrgPropertyAvatarURL = "avatar_url" + // OrgPropertyCompany represents the company name of the organization + OrgPropertyCompany = "company" ) diff --git a/pkg/profiles/util.go b/pkg/profiles/util.go index 4a3fcbfff4..8862363720 100644 --- a/pkg/profiles/util.go +++ b/pkg/profiles/util.go @@ -99,6 +99,8 @@ func GetRulesForEntity(p *pb.Profile, entity pb.Entity) ([]*pb.Profile_Rule, err return p.PipelineRun, nil case pb.Entity_ENTITY_TASK_RUN: return p.TaskRun, nil + case pb.Entity_ENTITY_ORGANIZATION: + return nil, nil case pb.Entity_ENTITY_BUILD: return p.Build, nil case pb.Entity_ENTITY_UNSPECIFIED: @@ -424,6 +426,7 @@ func rowInfoToProfileMap( profile.PipelineRun = ruleset case pb.Entity_ENTITY_TASK_RUN: profile.TaskRun = ruleset + case pb.Entity_ENTITY_ORGANIZATION: case pb.Entity_ENTITY_BUILD: profile.Build = ruleset case pb.Entity_ENTITY_UNSPECIFIED: From 3b5eb14936d677bd6bdc537cc76f304598e9affe Mon Sep 17 00:00:00 2001 From: jaydeep869 Date: Fri, 1 May 2026 01:08:55 +0530 Subject: [PATCH 3/7] fix: CR fixes for #6356 Signed-off-by: jaydeep869 --- cmd/server/app/migrate_up.go | 3 +- internal/controlplane/handlers_oauth.go | 5 +-- internal/controlplane/handlers_profile.go | 1 + internal/controlplane/handlers_user.go | 4 +-- internal/eea/eea.go | 1 + .../service/validators/organization.go | 30 ------------------ internal/providers/github/common.go | 7 ++++- .../github/properties/organization.go | 31 +++++++++++++------ .../providers/github/service/backfill.go | 9 +++--- internal/service/service.go | 2 -- pkg/entities/properties/constants_org.go | 10 +++--- pkg/profiles/util.go | 2 ++ 12 files changed, 49 insertions(+), 56 deletions(-) delete mode 100644 internal/entities/service/validators/organization.go rename cmd/server/app/backfill_organizations.go => internal/providers/github/service/backfill.go (86%) diff --git a/cmd/server/app/migrate_up.go b/cmd/server/app/migrate_up.go index aff4a48046..5a3f828c78 100644 --- a/cmd/server/app/migrate_up.go +++ b/cmd/server/app/migrate_up.go @@ -19,6 +19,7 @@ import ( "github.com/mindersec/minder/database" "github.com/mindersec/minder/internal/authz" "github.com/mindersec/minder/internal/db" + "github.com/mindersec/minder/internal/providers/github/service" "github.com/mindersec/minder/pkg/config" serverconfig "github.com/mindersec/minder/pkg/config/server" ) @@ -100,7 +101,7 @@ var upCmd = &cobra.Command{ } cmd.Println("Backfilling organizations...") - if err := backfillOrganizations(ctx, db.NewStore(dbConn)); err != nil { + if err := service.BackfillOrganizations(ctx, db.NewStore(dbConn)); err != nil { return fmt.Errorf("error while backfilling organizations: %w", err) } diff --git a/internal/controlplane/handlers_oauth.go b/internal/controlplane/handlers_oauth.go index dfed3b8953..274ba2f195 100644 --- a/internal/controlplane/handlers_oauth.go +++ b/internal/controlplane/handlers_oauth.go @@ -33,6 +33,7 @@ import ( "github.com/mindersec/minder/internal/logger" "github.com/mindersec/minder/internal/providers" "github.com/mindersec/minder/internal/providers/credentials" + "github.com/mindersec/minder/internal/providers/github" "github.com/mindersec/minder/internal/providers/github/service" "github.com/mindersec/minder/internal/providers/manager" "github.com/mindersec/minder/internal/util" @@ -442,7 +443,7 @@ func (s *Server) processAppCallback(ctx context.Context, w http.ResponseWriter, } if dbProv != nil { - login := strings.TrimPrefix(dbProv.Name, string(db.ProviderClassGithubApp)+"-") + login := github.GetGithubAppOwner(dbProv.Name) s.publishOrganizationEntityEvent(ctx, dbProv.ID, dbProv.ProjectID, login) } @@ -542,7 +543,7 @@ func (s *Server) handleAppInstallWithoutInvite(ctx context.Context, token *oauth return nil, err } if dbProv != nil && proj != nil { - login := strings.TrimPrefix(dbProv.Name, string(db.ProviderClassGithubApp)+"-") + login := github.GetGithubAppOwner(dbProv.Name) // It is generally safe to publish an event from within a transaction, as long // as the event handler evaluates the state matching later. s.publishOrganizationEntityEvent(ctx, dbProv.ID, proj.ID, login) diff --git a/internal/controlplane/handlers_profile.go b/internal/controlplane/handlers_profile.go index 332a49de2e..5e53dae763 100644 --- a/internal/controlplane/handlers_profile.go +++ b/internal/controlplane/handlers_profile.go @@ -428,6 +428,7 @@ func (s *Server) getRuleEvalStatus( } case db.EntitiesBuildEnvironment, db.EntitiesRelease, db.EntitiesPipelineRun, db.EntitiesTaskRun, db.EntitiesBuild, db.EntitiesOrganization: + // TODO: Alert URLs for organizations are incorrect zerolog.Ctx(ctx).Warn().Msgf("attempting to set alerts for unsupported entity type: %v", dbRuleEvalStat.EntityType) default: zerolog.Ctx(ctx).Error().Msgf("unknown entity type: %v", dbRuleEvalStat.EntityType) diff --git a/internal/controlplane/handlers_user.go b/internal/controlplane/handlers_user.go index 57befff4d0..5cafb7ce62 100644 --- a/internal/controlplane/handlers_user.go +++ b/internal/controlplane/handlers_user.go @@ -12,7 +12,6 @@ import ( "net/http" "path" "strconv" - "strings" "github.com/google/uuid" gauth "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/auth" @@ -27,6 +26,7 @@ import ( "github.com/mindersec/minder/internal/db" "github.com/mindersec/minder/internal/logger" "github.com/mindersec/minder/internal/projects" + "github.com/mindersec/minder/internal/providers/github" "github.com/mindersec/minder/internal/util" pb "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" ) @@ -141,7 +141,7 @@ func (s *Server) claimGitHubInstalls(ctx context.Context, qtx db.ExtendQuerier) continue } if dbProv != nil { - login := strings.TrimPrefix(dbProv.Name, string(db.ProviderClassGithubApp)+"-") + login := github.GetGithubAppOwner(dbProv.Name) s.publishOrganizationEntityEvent(ctx, dbProv.ID, proj.ID, login) } if proj != nil { diff --git a/internal/eea/eea.go b/internal/eea/eea.go index 0b758ea56f..0756056d34 100644 --- a/internal/eea/eea.go +++ b/internal/eea/eea.go @@ -256,6 +256,7 @@ func (e *EEA) buildEntityWrapper( return e.buildPullRequestInfoWrapper(ctx, entityID, projID) case db.EntitiesBuildEnvironment, db.EntitiesRelease, db.EntitiesPipelineRun, db.EntitiesTaskRun, db.EntitiesBuild, db.EntitiesOrganization: + // TODO: Support evaluate policy on organizations return nil, fmt.Errorf("entity type %q not yet supported", entity) default: return nil, fmt.Errorf("unknown entity type: %q", entity) diff --git a/internal/entities/service/validators/organization.go b/internal/entities/service/validators/organization.go deleted file mode 100644 index e7f4927dc6..0000000000 --- a/internal/entities/service/validators/organization.go +++ /dev/null @@ -1,30 +0,0 @@ -// SPDX-FileCopyrightText: Copyright 2026 The Minder Authors -// SPDX-License-Identifier: Apache-2.0 - -package validators - -import ( - "context" - - "github.com/google/uuid" - - "github.com/mindersec/minder/pkg/entities/properties" -) - -// OrganizationValidator validates organization entity creation -type OrganizationValidator struct{} - -// NewOrganizationValidator creates a new OrganizationValidator -func NewOrganizationValidator() *OrganizationValidator { - return &OrganizationValidator{} -} - -// Validate checks if an organization entity can be created -func (*OrganizationValidator) Validate( - _ context.Context, - _ *properties.Properties, - _ uuid.UUID, -) error { - // For now, any organization properties that make it this far are valid - return nil -} diff --git a/internal/providers/github/common.go b/internal/providers/github/common.go index 1a7a8fdf09..9564e0cd64 100644 --- a/internal/providers/github/common.go +++ b/internal/providers/github/common.go @@ -995,11 +995,16 @@ func IsMinderHook(hook *github.Hook, hostURL string) (bool, error) { return false, nil } +// GetGithubAppOwner returns the owner of the GitHub App given a provider name. +func GetGithubAppOwner(provName string) string { + return strings.TrimPrefix(provName, string(db.ProviderClassGithubApp)+"-") +} + // CanHandleOwner checks if the GitHub provider has the right credentials to handle the owner func CanHandleOwner(_ context.Context, prov db.Provider, owner string) bool { // TODO: this is fragile and does not handle organization renames, in the future we can make sure the credential // has admin permissions on the owner - if prov.Name == fmt.Sprintf("%s-%s", db.ProviderClassGithubApp, owner) { + if prov.Class == db.ProviderClassGithubApp && GetGithubAppOwner(prov.Name) == owner { return true } if prov.Class == db.ProviderClassGithub { diff --git a/internal/providers/github/properties/organization.go b/internal/providers/github/properties/organization.go index 93da7e2396..8b199781be 100644 --- a/internal/providers/github/properties/organization.go +++ b/internal/providers/github/properties/organization.go @@ -6,7 +6,6 @@ package properties import ( "context" "fmt" - "strconv" go_github "github.com/google/go-github/v63/github" @@ -28,8 +27,9 @@ func NewOrganizationFetcher() *OrganizationFetcher { properties.PropertyUpstreamID, properties.PropertyName, properties.OrgPropertyIsUser, - properties.OrgPropertyAvatarURL, - properties.OrgPropertyCompany, + properties.OrgPropertyHasOrganizationProjects, + properties.OrgPropertyCreatedAt, + properties.OrgPropertyPlanName, }, wrapper: fetchOrganizationProperties, }, @@ -54,13 +54,16 @@ func fetchOrganizationProperties( var user *go_github.User var err error - if idStr := lookupProperties.GetProperty(properties.PropertyUpstreamID).GetString(); idStr != "" { - id, parseErr := strconv.ParseInt(idStr, 10, 64) + upstreamIDProp := lookupProperties.GetProperty(properties.PropertyUpstreamID) + nameProp := lookupProperties.GetProperty(properties.PropertyName) + + if upstreamIDProp != nil { + id, parseErr := upstreamIDProp.AsInt64() if parseErr != nil { return nil, fmt.Errorf("invalid upstream ID: %w", parseErr) } user, _, err = ghCli.Users.GetByID(ctx, id) - } else if name := lookupProperties.GetProperty(properties.PropertyName).GetString(); name != "" { + } else if name := nameProp.GetString(); name != "" { user, _, err = ghCli.Users.Get(ctx, name) } else { return nil, fmt.Errorf("either upstream_id or name (login) must be provided to fetch an organization") @@ -80,11 +83,19 @@ func fetchOrganizationProperties( properties.OrgPropertyIsUser: user.GetType() == "User", } - if user.AvatarURL != nil { - result[properties.OrgPropertyAvatarURL] = *user.AvatarURL + if user.GetType() == "Organization" { + org, _, err := ghCli.Organizations.GetByID(ctx, user.GetID()) + if err == nil { + if org.HasOrganizationProjects != nil { + result[properties.OrgPropertyHasOrganizationProjects] = org.GetHasOrganizationProjects() + } + } + } + if user.CreatedAt != nil { + result[properties.OrgPropertyCreatedAt] = user.GetCreatedAt().Time } - if user.Company != nil { - result[properties.OrgPropertyCompany] = *user.Company + if user.Plan != nil { + result[properties.OrgPropertyPlanName] = user.GetPlan().GetName() } return result, nil diff --git a/cmd/server/app/backfill_organizations.go b/internal/providers/github/service/backfill.go similarity index 86% rename from cmd/server/app/backfill_organizations.go rename to internal/providers/github/service/backfill.go index 799655ca2f..6a70bee3ed 100644 --- a/cmd/server/app/backfill_organizations.go +++ b/internal/providers/github/service/backfill.go @@ -1,7 +1,7 @@ // SPDX-FileCopyrightText: Copyright 2026 The Minder Authors // SPDX-License-Identifier: Apache-2.0 -package app +package service import ( "context" @@ -9,15 +9,16 @@ import ( "encoding/json" "errors" "fmt" - "strings" "github.com/rs/zerolog" "github.com/mindersec/minder/internal/db" + "github.com/mindersec/minder/internal/providers/github" "github.com/mindersec/minder/pkg/entities/properties" ) -func backfillOrganizations(ctx context.Context, store db.Store) error { +// BackfillOrganizations loops through GitHub app providers and ensures an organization entity is tracked for each +func BackfillOrganizations(ctx context.Context, store db.Store) error { l := zerolog.Ctx(ctx) l.Info().Msg("Starting backfill for Organization entities...") @@ -29,7 +30,7 @@ func backfillOrganizations(ctx context.Context, store db.Store) error { count := 0 for _, prov := range provs { - login := strings.TrimPrefix(prov.Name, string(db.ProviderClassGithubApp)+"-") + login := github.GetGithubAppOwner(prov.Name) _, err = db.WithTransaction(store, func(qtx db.ExtendQuerier) (any, error) { // Check if organization entity already exists diff --git a/internal/service/service.go b/internal/service/service.go index 6a1c6585b6..87856e9d82 100644 --- a/internal/service/service.go +++ b/internal/service/service.go @@ -178,8 +178,6 @@ func AllInOneServerService( validatorRegistry := validators.NewValidatorRegistry() repoValidator := validators.NewRepositoryValidator(store) validatorRegistry.AddValidator(pb.Entity_ENTITY_REPOSITORIES, repoValidator) - orgValidator := validators.NewOrganizationValidator() - validatorRegistry.AddValidator(pb.Entity_ENTITY_ORGANIZATION, orgValidator) // Create entity creator entityCreator := entityService.NewEntityCreator( diff --git a/pkg/entities/properties/constants_org.go b/pkg/entities/properties/constants_org.go index ee06a98f57..8211b2667e 100644 --- a/pkg/entities/properties/constants_org.go +++ b/pkg/entities/properties/constants_org.go @@ -7,8 +7,10 @@ package properties const ( // OrgPropertyIsUser represents whether the organization is actually a user account OrgPropertyIsUser = "is_user" - // OrgPropertyAvatarURL represents the avatar URL of the organization - OrgPropertyAvatarURL = "avatar_url" - // OrgPropertyCompany represents the company name of the organization - OrgPropertyCompany = "company" + // OrgPropertyHasOrganizationProjects represents whether the organization has organization projects + OrgPropertyHasOrganizationProjects = "has_organization_projects" + // OrgPropertyCreatedAt represents the creation date of the organization + OrgPropertyCreatedAt = "created_at" + // OrgPropertyPlanName represents the plan name of the organization + OrgPropertyPlanName = "plan_name" ) diff --git a/pkg/profiles/util.go b/pkg/profiles/util.go index 8862363720..55e47b51c2 100644 --- a/pkg/profiles/util.go +++ b/pkg/profiles/util.go @@ -100,6 +100,7 @@ func GetRulesForEntity(p *pb.Profile, entity pb.Entity) ([]*pb.Profile_Rule, err case pb.Entity_ENTITY_TASK_RUN: return p.TaskRun, nil case pb.Entity_ENTITY_ORGANIZATION: + // Profile evaluation for organizations is not currently supported return nil, nil case pb.Entity_ENTITY_BUILD: return p.Build, nil @@ -427,6 +428,7 @@ func rowInfoToProfileMap( case pb.Entity_ENTITY_TASK_RUN: profile.TaskRun = ruleset case pb.Entity_ENTITY_ORGANIZATION: + // Profile evaluation for organizations is not currently supported case pb.Entity_ENTITY_BUILD: profile.Build = ruleset case pb.Entity_ENTITY_UNSPECIFIED: From bda676fccfa56d91b1e36942a622048dec19533f Mon Sep 17 00:00:00 2001 From: jaydeep869 Date: Fri, 1 May 2026 01:47:47 +0530 Subject: [PATCH 4/7] fix(org): save org properties and correct migration to 118 Signed-off-by: jaydeep869 --- ...ion_entity.down.sql => 000118_organization_entity.down.sql} | 0 ...ization_entity.up.sql => 000118_organization_entity.up.sql} | 0 internal/providers/github/properties/organization.go | 3 ++- 3 files changed, 2 insertions(+), 1 deletion(-) rename database/migrations/{000117_organization_entity.down.sql => 000118_organization_entity.down.sql} (100%) rename database/migrations/{000117_organization_entity.up.sql => 000118_organization_entity.up.sql} (100%) diff --git a/database/migrations/000117_organization_entity.down.sql b/database/migrations/000118_organization_entity.down.sql similarity index 100% rename from database/migrations/000117_organization_entity.down.sql rename to database/migrations/000118_organization_entity.down.sql diff --git a/database/migrations/000117_organization_entity.up.sql b/database/migrations/000118_organization_entity.up.sql similarity index 100% rename from database/migrations/000117_organization_entity.up.sql rename to database/migrations/000118_organization_entity.up.sql diff --git a/internal/providers/github/properties/organization.go b/internal/providers/github/properties/organization.go index 8b199781be..03a122a621 100644 --- a/internal/providers/github/properties/organization.go +++ b/internal/providers/github/properties/organization.go @@ -6,6 +6,7 @@ package properties import ( "context" "fmt" + "time" go_github "github.com/google/go-github/v63/github" @@ -92,7 +93,7 @@ func fetchOrganizationProperties( } } if user.CreatedAt != nil { - result[properties.OrgPropertyCreatedAt] = user.GetCreatedAt().Time + result[properties.OrgPropertyCreatedAt] = user.GetCreatedAt().Time.Format(time.RFC3339) } if user.Plan != nil { result[properties.OrgPropertyPlanName] = user.GetPlan().GetName() From 34c66c259901b9e8457aeed64ef55357d2efbbd1 Mon Sep 17 00:00:00 2001 From: jaydeep869 Date: Fri, 1 May 2026 05:36:59 +0530 Subject: [PATCH 5/7] fix(org): address Evan's review comments Signed-off-by: jaydeep869 --- internal/controlplane/handlers_oauth.go | 7 ++--- internal/controlplane/handlers_oauth_test.go | 2 +- internal/controlplane/handlers_user.go | 4 +-- .../github/properties/organization.go | 2 +- internal/providers/github/service/backfill.go | 11 ++------ .../providers/github/service/mock/service.go | 9 ++++--- internal/providers/github/service/service.go | 26 ++++++++++++++----- .../providers/github/service/service_test.go | 15 ++++++----- 8 files changed, 39 insertions(+), 37 deletions(-) diff --git a/internal/controlplane/handlers_oauth.go b/internal/controlplane/handlers_oauth.go index 274ba2f195..2213180f3c 100644 --- a/internal/controlplane/handlers_oauth.go +++ b/internal/controlplane/handlers_oauth.go @@ -33,7 +33,6 @@ import ( "github.com/mindersec/minder/internal/logger" "github.com/mindersec/minder/internal/providers" "github.com/mindersec/minder/internal/providers/credentials" - "github.com/mindersec/minder/internal/providers/github" "github.com/mindersec/minder/internal/providers/github/service" "github.com/mindersec/minder/internal/providers/manager" "github.com/mindersec/minder/internal/util" @@ -443,8 +442,7 @@ func (s *Server) processAppCallback(ctx context.Context, w http.ResponseWriter, } if dbProv != nil { - login := github.GetGithubAppOwner(dbProv.Name) - s.publishOrganizationEntityEvent(ctx, dbProv.ID, dbProv.ProjectID, login) + s.publishOrganizationEntityEvent(ctx, dbProv.Provider.ID, dbProv.Provider.ProjectID, dbProv.InstallationOwner) } if stateData.RedirectUrl.Valid || stateData.EncryptedRedirect.Valid { @@ -543,10 +541,9 @@ func (s *Server) handleAppInstallWithoutInvite(ctx context.Context, token *oauth return nil, err } if dbProv != nil && proj != nil { - login := github.GetGithubAppOwner(dbProv.Name) // It is generally safe to publish an event from within a transaction, as long // as the event handler evaluates the state matching later. - s.publishOrganizationEntityEvent(ctx, dbProv.ID, proj.ID, login) + s.publishOrganizationEntityEvent(ctx, dbProv.Provider.ID, proj.ID, dbProv.InstallationOwner) } return proj, nil }) diff --git a/internal/controlplane/handlers_oauth_test.go b/internal/controlplane/handlers_oauth_test.go index 081fb3410e..9d51dbc230 100644 --- a/internal/controlplane/handlers_oauth_test.go +++ b/internal/controlplane/handlers_oauth_test.go @@ -829,7 +829,7 @@ func TestHandleGitHubAppCallback(t *testing.T) { }, nil) service.EXPECT(). CreateGitHubAppProvider(gomock.Any(), gomock.Any(), gomock.Any(), installationID, gomock.Any()). - Return(&db.Provider{}, nil) + Return(&ghService.GitHubProviderFacet{Provider: &db.Provider{}}, nil) }, checkResponse: func(t *testing.T, resp httptest.ResponseRecorder) { t.Helper() diff --git a/internal/controlplane/handlers_user.go b/internal/controlplane/handlers_user.go index 5cafb7ce62..f58da002af 100644 --- a/internal/controlplane/handlers_user.go +++ b/internal/controlplane/handlers_user.go @@ -26,7 +26,6 @@ import ( "github.com/mindersec/minder/internal/db" "github.com/mindersec/minder/internal/logger" "github.com/mindersec/minder/internal/projects" - "github.com/mindersec/minder/internal/providers/github" "github.com/mindersec/minder/internal/util" pb "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" ) @@ -141,8 +140,7 @@ func (s *Server) claimGitHubInstalls(ctx context.Context, qtx db.ExtendQuerier) continue } if dbProv != nil { - login := github.GetGithubAppOwner(dbProv.Name) - s.publishOrganizationEntityEvent(ctx, dbProv.ID, proj.ID, login) + s.publishOrganizationEntityEvent(ctx, dbProv.Provider.ID, proj.ID, dbProv.InstallationOwner) } if proj != nil { userProjects = append(userProjects, proj) diff --git a/internal/providers/github/properties/organization.go b/internal/providers/github/properties/organization.go index 03a122a621..bc5fade8f9 100644 --- a/internal/providers/github/properties/organization.go +++ b/internal/providers/github/properties/organization.go @@ -93,7 +93,7 @@ func fetchOrganizationProperties( } } if user.CreatedAt != nil { - result[properties.OrgPropertyCreatedAt] = user.GetCreatedAt().Time.Format(time.RFC3339) + result[properties.OrgPropertyCreatedAt] = user.GetCreatedAt().Format(time.RFC3339) } if user.Plan != nil { result[properties.OrgPropertyPlanName] = user.GetPlan().GetName() diff --git a/internal/providers/github/service/backfill.go b/internal/providers/github/service/backfill.go index 6a70bee3ed..38c40d2265 100644 --- a/internal/providers/github/service/backfill.go +++ b/internal/providers/github/service/backfill.go @@ -6,7 +6,6 @@ package service import ( "context" "database/sql" - "encoding/json" "errors" "fmt" @@ -59,16 +58,10 @@ func BackfillOrganizations(ctx context.Context, store db.Store) error { } // Set the default property (login name) - propVal := map[string]any{ - "minder.internal.type": "string", - "minder.internal.value": login, - } - propBytes, _ := json.Marshal(propVal) - - _, err = qtx.UpsertProperty(ctx, db.UpsertPropertyParams{ + _, err = qtx.UpsertPropertyValueV1(ctx, db.UpsertPropertyValueV1Params{ EntityID: ent.ID, Key: properties.PropertyName, - Value: propBytes, + Value: login, }) if err == nil { diff --git a/internal/providers/github/service/mock/service.go b/internal/providers/github/service/mock/service.go index c0cbe9f62c..f55767e57e 100644 --- a/internal/providers/github/service/mock/service.go +++ b/internal/providers/github/service/mock/service.go @@ -16,6 +16,7 @@ import ( uuid "github.com/google/uuid" db "github.com/mindersec/minder/internal/db" + service "github.com/mindersec/minder/internal/providers/github/service" gomock "go.uber.org/mock/gomock" oauth2 "golang.org/x/oauth2" ) @@ -45,10 +46,10 @@ func (m *MockGitHubProviderService) EXPECT() *MockGitHubProviderServiceMockRecor } // CreateGitHubAppProvider mocks base method. -func (m *MockGitHubProviderService) CreateGitHubAppProvider(ctx context.Context, token oauth2.Token, stateData db.GetProjectIDBySessionStateRow, installationID int64, state string) (*db.Provider, error) { +func (m *MockGitHubProviderService) CreateGitHubAppProvider(ctx context.Context, token oauth2.Token, stateData db.GetProjectIDBySessionStateRow, installationID int64, state string) (*service.GitHubProviderFacet, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "CreateGitHubAppProvider", ctx, token, stateData, installationID, state) - ret0, _ := ret[0].(*db.Provider) + ret0, _ := ret[0].(*service.GitHubProviderFacet) ret1, _ := ret[1].(error) return ret0, ret1 } @@ -60,11 +61,11 @@ func (mr *MockGitHubProviderServiceMockRecorder) CreateGitHubAppProvider(ctx, to } // CreateGitHubAppWithoutInvitation mocks base method. -func (m *MockGitHubProviderService) CreateGitHubAppWithoutInvitation(ctx context.Context, qtx db.ExtendQuerier, userID, installationID int64) (*db.Project, *db.Provider, error) { +func (m *MockGitHubProviderService) CreateGitHubAppWithoutInvitation(ctx context.Context, qtx db.ExtendQuerier, userID, installationID int64) (*db.Project, *service.GitHubProviderFacet, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "CreateGitHubAppWithoutInvitation", ctx, qtx, userID, installationID) ret0, _ := ret[0].(*db.Project) - ret1, _ := ret[1].(*db.Provider) + ret1, _ := ret[1].(*service.GitHubProviderFacet) ret2, _ := ret[2].(error) return ret0, ret1, ret2 } diff --git a/internal/providers/github/service/service.go b/internal/providers/github/service/service.go index cac80c17bb..6ed816c8c1 100644 --- a/internal/providers/github/service/service.go +++ b/internal/providers/github/service/service.go @@ -32,17 +32,23 @@ import ( //go:generate go run go.uber.org/mock/mockgen -package mock_$GOPACKAGE -destination=./mock/$GOFILE -source=./$GOFILE +// GitHubProviderFacet encapsulates the created db.Provider and additional GitHub specific properties +type GitHubProviderFacet struct { + Provider *db.Provider + InstallationOwner string +} + // GitHubProviderService encapsulates methods for creating and updating providers type GitHubProviderService interface { // CreateGitHubAppProvider creates a GitHub App provider with an installation ID in a known project CreateGitHubAppProvider(ctx context.Context, token oauth2.Token, stateData db.GetProjectIDBySessionStateRow, - installationID int64, state string) (*db.Provider, error) + installationID int64, state string) (*GitHubProviderFacet, error) // CreateGitHubAppWithoutInvitation either creates a new project for the selected app, or stores // the installation in preparation for creating a new project when the authorizing user logs in. // // Note that this function may return nil, nil if the installation user is not known to Minder. CreateGitHubAppWithoutInvitation(ctx context.Context, qtx db.ExtendQuerier, userID int64, - installationID int64) (*db.Project, *db.Provider, error) + installationID int64) (*db.Project, *GitHubProviderFacet, error) // ValidateGitHubInstallationId checks if the supplied GitHub token has access to the installation ID ValidateGitHubInstallationId(ctx context.Context, token *oauth2.Token, installationID int64) error // DeleteGitHubAppInstallation deletes the GitHub App installation and provider from the database. @@ -106,13 +112,13 @@ func (p *ghProviderService) CreateGitHubAppProvider( stateData db.GetProjectIDBySessionStateRow, installationID int64, state string, -) (*db.Provider, error) { +) (*GitHubProviderFacet, error) { installationOwner, err := p.getInstallationOwner(ctx, installationID) if err != nil { return nil, err } - return db.WithTransaction(p.store, func(qtx db.ExtendQuerier) (*db.Provider, error) { + return db.WithTransaction(p.store, func(qtx db.ExtendQuerier) (*GitHubProviderFacet, error) { validateOwnership := func(ctx context.Context) error { // Older enrollments may not have a RemoteUser stored; these should age out fairly quickly. p.mt.AddTokenOpCount(ctx, "check", stateData.RemoteUser.Valid) @@ -157,7 +163,10 @@ func (p *ghProviderService) CreateGitHubAppProvider( }, ) - return &provider, err + return &GitHubProviderFacet{ + Provider: &provider, + InstallationOwner: installationOwner.GetLogin(), + }, err }) } @@ -170,7 +179,7 @@ func (p *ghProviderService) CreateGitHubAppWithoutInvitation( qtx db.ExtendQuerier, userID int64, installationID int64, -) (*db.Project, *db.Provider, error) { +) (*db.Project, *GitHubProviderFacet, error) { installationOwner, err := p.getInstallationOwner(ctx, installationID) if err != nil { return nil, nil, err @@ -213,7 +222,10 @@ func (p *ghProviderService) CreateGitHubAppWithoutInvitation( } - return project, &provider, err + return project, &GitHubProviderFacet{ + Provider: &provider, + InstallationOwner: installationOwner.GetLogin(), + }, err } // Internal shared implementation between CreateGitHubAppProvider and CreateGitHubAppWithoutInvitation. diff --git a/internal/providers/github/service/service_test.go b/internal/providers/github/service/service_test.go index 56161e9501..03d4310e1e 100644 --- a/internal/providers/github/service/service_test.go +++ b/internal/providers/github/service/service_test.go @@ -271,15 +271,16 @@ func TestProviderService_CreateGitHubAppProvider(t *testing.T) { require.NoError(t, err) require.NotNil(t, dbProv) - require.Equal(t, dbProv.ProjectID, dbproj.ID) - require.Equal(t, dbProv.AuthFlows, clients.AppAuthorizationFlows) - require.Equal(t, dbProv.Implements, clients.AppImplements) - require.Equal(t, dbProv.Class, db.ProviderClassGithubApp) - require.Contains(t, dbProv.Name, db.ProviderClassGithubApp) - require.Contains(t, dbProv.Name, accountLogin) + require.Equal(t, dbProv.Provider.ProjectID, dbproj.ID) + require.Equal(t, dbProv.Provider.AuthFlows, clients.AppAuthorizationFlows) + require.Equal(t, dbProv.Provider.Implements, clients.AppImplements) + require.Equal(t, dbProv.Provider.Class, db.ProviderClassGithubApp) + require.Contains(t, dbProv.Provider.Name, db.ProviderClassGithubApp) + require.Contains(t, dbProv.Provider.Name, accountLogin) + require.Equal(t, accountLogin, dbProv.InstallationOwner) dbInstall, err := mocks.fakeStore.GetInstallationIDByProviderID(context.Background(), - uuid.NullUUID{UUID: dbProv.ID, Valid: true}, + uuid.NullUUID{UUID: dbProv.Provider.ID, Valid: true}, ) require.NoError(t, err) require.Equal(t, dbInstall.AppInstallationID, int64(installationID)) From ab5e202e548e5bce6a7ed569eadbf7d0ae73ee3a Mon Sep 17 00:00:00 2001 From: jaydeep869 Date: Fri, 1 May 2026 05:53:34 +0530 Subject: [PATCH 6/7] test(org): add unit tests for organization fetcher and backfill Signed-off-by: jaydeep869 --- .../github/properties/organization_test.go | 79 ++++++++++ .../providers/github/service/backfill_test.go | 136 ++++++++++++++++++ 2 files changed, 215 insertions(+) create mode 100644 internal/providers/github/properties/organization_test.go create mode 100644 internal/providers/github/service/backfill_test.go diff --git a/internal/providers/github/properties/organization_test.go b/internal/providers/github/properties/organization_test.go new file mode 100644 index 0000000000..4b17b303a5 --- /dev/null +++ b/internal/providers/github/properties/organization_test.go @@ -0,0 +1,79 @@ +// SPDX-FileCopyrightText: Copyright 2026 The Minder Authors +// SPDX-License-Identifier: Apache-2.0 + +// Package properties provides utility functions for fetching and managing properties +package properties + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/mindersec/minder/pkg/entities/properties" +) + +func TestNewOrganizationFetcher(t *testing.T) { + t.Parallel() + fetcher := NewOrganizationFetcher() + assert.NotNil(t, fetcher) + assert.Len(t, fetcher.propertyOrigins, 1) + assert.Len(t, fetcher.propertyOrigins[0].keys, 6) + // all entities should have these properties + assert.Contains(t, fetcher.propertyOrigins[0].keys, properties.PropertyName) + assert.Contains(t, fetcher.propertyOrigins[0].keys, properties.PropertyUpstreamID) + // org-specific properties + assert.Contains(t, fetcher.propertyOrigins[0].keys, properties.OrgPropertyIsUser) + assert.Contains(t, fetcher.propertyOrigins[0].keys, properties.OrgPropertyHasOrganizationProjects) + assert.Contains(t, fetcher.propertyOrigins[0].keys, properties.OrgPropertyCreatedAt) + assert.Contains(t, fetcher.propertyOrigins[0].keys, properties.OrgPropertyPlanName) + assert.Empty(t, fetcher.operationalProperties) +} + +func TestOrganizationFetcherGetName(t *testing.T) { + t.Parallel() + + fetcher := NewOrganizationFetcher() + tests := []struct { + name string + props map[string]any + expected string + expectedErrMsg string + }{ + { + name: "Valid properties with name", + props: map[string]any{ + properties.PropertyName: "my-org", + }, + expected: "my-org", + }, + { + name: "Missing name property", + props: map[string]any{}, + expectedErrMsg: "missing property", + }, + { + name: "Empty name property", + props: map[string]any{ + properties.PropertyName: "", + }, + expectedErrMsg: "missing property", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + props := properties.NewProperties(tt.props) + + result, err := fetcher.GetName(props) + if tt.expectedErrMsg != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.expectedErrMsg) + } else { + require.NoError(t, err) + assert.Equal(t, tt.expected, result) + } + }) + } +} diff --git a/internal/providers/github/service/backfill_test.go b/internal/providers/github/service/backfill_test.go new file mode 100644 index 0000000000..a0d599cc44 --- /dev/null +++ b/internal/providers/github/service/backfill_test.go @@ -0,0 +1,136 @@ +// SPDX-FileCopyrightText: Copyright 2026 The Minder Authors +// SPDX-License-Identifier: Apache-2.0 + +package service + +import ( + "context" + "encoding/json" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/mindersec/minder/internal/db" + "github.com/mindersec/minder/internal/db/embedded" + "github.com/mindersec/minder/pkg/entities/properties" +) + +func TestBackfillOrganizations_NoProviders(t *testing.T) { + t.Parallel() + + store, cancelFunc, err := embedded.GetFakeStore() + if cancelFunc != nil { + t.Cleanup(cancelFunc) + } + require.NoError(t, err) + + // No providers in the DB, so backfill should succeed and do nothing + err = BackfillOrganizations(context.Background(), store) + require.NoError(t, err) +} + +func TestBackfillOrganizations_CreatesEntity(t *testing.T) { + t.Parallel() + + store, cancelFunc, err := embedded.GetFakeStore() + if cancelFunc != nil { + t.Cleanup(cancelFunc) + } + require.NoError(t, err) + + // Create a project first + proj, err := store.CreateProject(context.Background(), db.CreateProjectParams{ + Name: "test-backfill", + Metadata: []byte(`{}`), + }) + require.NoError(t, err) + + // Create a GitHub App provider + prov, err := store.CreateProvider(context.Background(), db.CreateProviderParams{ + Name: "github-app-test-org", + ProjectID: proj.ID, + Class: db.ProviderClassGithubApp, + Implements: []db.ProviderType{db.ProviderTypeGithub, db.ProviderTypeGit}, + AuthFlows: []db.AuthorizationFlow{db.AuthorizationFlowUserInput}, + Definition: json.RawMessage(`{}`), + }) + require.NoError(t, err) + + // Run the backfill + err = BackfillOrganizations(context.Background(), store) + require.NoError(t, err) + + // Verify organization entity was created + ent, err := store.GetEntityByName(context.Background(), db.GetEntityByNameParams{ + EntityType: db.EntitiesOrganization, + Name: "test-org", + ProviderID: prov.ID, + ProjectID: proj.ID, + }) + require.NoError(t, err) + require.Equal(t, "test-org", ent.Name) + require.Equal(t, db.EntitiesOrganization, ent.EntityType) + + // Verify property was set + prop, err := store.GetProperty(context.Background(), db.GetPropertyParams{ + EntityID: ent.ID, + Key: properties.PropertyName, + }) + require.NoError(t, err) + + val, err := db.PropValueFromDbV1(prop.Value) + require.NoError(t, err) + require.Equal(t, "test-org", val) +} + +func TestBackfillOrganizations_Idempotent(t *testing.T) { + t.Parallel() + + store, cancelFunc, err := embedded.GetFakeStore() + if cancelFunc != nil { + t.Cleanup(cancelFunc) + } + require.NoError(t, err) + + // Create a project + proj, err := store.CreateProject(context.Background(), db.CreateProjectParams{ + Name: "test-backfill-idempotent", + Metadata: []byte(`{}`), + }) + require.NoError(t, err) + + // Create a GitHub App provider + _, err = store.CreateProvider(context.Background(), db.CreateProviderParams{ + Name: "github-app-my-idempotent-org", + ProjectID: proj.ID, + Class: db.ProviderClassGithubApp, + Implements: []db.ProviderType{db.ProviderTypeGithub, db.ProviderTypeGit}, + AuthFlows: []db.AuthorizationFlow{db.AuthorizationFlowUserInput}, + Definition: json.RawMessage(`{}`), + }) + require.NoError(t, err) + + // Run backfill twice - second time should not fail + err = BackfillOrganizations(context.Background(), store) + require.NoError(t, err) + + err = BackfillOrganizations(context.Background(), store) + require.NoError(t, err) +} + +func TestGitHubProviderFacet(t *testing.T) { + t.Parallel() + + facet := &GitHubProviderFacet{ + Provider: &db.Provider{ + Name: "github-app-my-org", + Class: db.ProviderClassGithubApp, + }, + InstallationOwner: "my-org", + } + + require.NotNil(t, facet.Provider) + require.Equal(t, "my-org", facet.InstallationOwner) + require.Equal(t, "github-app-my-org", facet.Provider.Name) + require.Equal(t, db.ProviderClassGithubApp, facet.Provider.Class) +} From 9a4b2be9a0ed985b8c1aa8120f306ea63a8af91b Mon Sep 17 00:00:00 2001 From: jaydeep869 Date: Fri, 1 May 2026 06:03:39 +0530 Subject: [PATCH 7/7] ci: retrigger CI (flaky eventer test) Signed-off-by: jaydeep869