Take into account hierarchy when dealing with rule types

Rule types are usable by projects down the hierarchy, this allows someone in a child project to create a profile that uses rule types approved in a parent project. This is very handy since folks with permission to write profiles don't need to have permissions to write rule types, they can simply use some already approved and created. This also adds the constraint that rule type names are unique down the hierarchy. That is, a user cannot create a rule type name that overrides a rule type that exists in a parent project. Signed-off-by: Juan Antonio Osorio <ozz@stacklok.com>
mindersec · Jun 14, 2024 · 38dcea4 · 38dcea4
1 parent 266f0f4
commit 38dcea4
Show file tree

Hide file tree

Showing 10 changed files with 71 additions and 43 deletions.
diff --git a/database/query/rule_types.sql b/database/query/rule_types.sql
@@ -26,7 +26,7 @@ SELECT * FROM rule_type WHERE project_id = $1;
 SELECT * FROM rule_type WHERE id = $1;
 
 -- name: GetRuleTypeByName :one
-SELECT * FROM rule_type WHERE  project_id = sqlc.arg(project_id) AND lower(name) = lower(sqlc.arg(name));
+SELECT * FROM rule_type WHERE  project_id = ANY(sqlc.arg(projects)::uuid[]) AND lower(name) = lower(sqlc.arg(name));
 
 -- name: DeleteRuleType :exec
 DELETE FROM rule_type WHERE id = $1;

diff --git a/internal/controlplane/handlers_ruletype.go b/internal/controlplane/handlers_ruletype.go
@@ -83,8 +83,9 @@ func (s *Server) GetRuleTypeByName(
 	resp := &minderv1.GetRuleTypeByNameResponse{}
 
 	rtdb, err := s.store.GetRuleTypeByName(ctx, db.GetRuleTypeByNameParams{
-		ProjectID: entityCtx.Project.ID,
-		Name:      in.GetName(),
+		// TODO: Add option to fetch rule types from parent projects too
+		Projects: []uuid.UUID{entityCtx.Project.ID},
+		Name:     in.GetName(),
 	})
 	if errors.Is(err, sql.ErrNoRows) {
 		return nil, util.UserVisibleError(codes.NotFound, "rule type %s not found", in.GetName())

diff --git a/internal/db/rule_types.sql.go b/internal/db/rule_types.sql.go
diff --git a/internal/engine/executor.go b/internal/engine/executor.go
@@ -273,17 +273,12 @@ func (e *Executor) getEvaluator(
 		return nil, nil, fmt.Errorf("error creating eval status params: %w", err)
 	}
 
-	// NOTE: We're only using the first project in the hierarchy for now.
-	// This means that a rule type must exist in the same project as the profile.
-	// This will be revisited in the future.
-	projID := hierarchy[0]
-
 	// Load Rule Class from database
 	// TODO(jaosorior): Rule types should be cached in memory so
 	// we don't have to query the database for each rule.
 	dbrt, err := e.querier.GetRuleTypeByName(ctx, db.GetRuleTypeByNameParams{
-		ProjectID: projID,
-		Name:      rule.Type,
+		Projects: hierarchy,
+		Name:     rule.Type,
 	})
 	if err != nil {
 		return nil, nil, fmt.Errorf("error getting rule type when traversing profile %s: %w", params.ProfileID, err)

diff --git a/internal/engine/executor_test.go b/internal/engine/executor_test.go
@@ -220,8 +220,8 @@ default allow = true`,
 
 	mockStore.EXPECT().
 		GetRuleTypeByName(gomock.Any(), db.GetRuleTypeByNameParams{
-			ProjectID: projectID,
-			Name:      passthroughRuleType,
+			Projects: []uuid.UUID{projectID},
+			Name:     passthroughRuleType,
 		}).Return(db.RuleType{
 		ID:         ruleTypeID,
 		Name:       passthroughRuleType,

diff --git a/internal/profiles/service.go b/internal/profiles/service.go
@@ -563,12 +563,16 @@ func (_ *profileService) getRulesFromProfile(
 	// track them in the db later.
 	rulesInProf := make(RuleMapping)
 
-	err := TraverseAllRulesForPipeline(profile, func(r *minderv1.Profile_Rule) error {
-		// TODO: This will need to be updated to support
-		// the hierarchy tree once that's settled in.
+	// Allows a profile to use rules from parent projects
+	projects, err := qtx.GetParentProjects(ctx, projectID)
+	if err != nil {
+		return nil, fmt.Errorf("error getting parent projects: %w", err)
+	}
+
+	err = TraverseAllRulesForPipeline(profile, func(r *minderv1.Profile_Rule) error {
 		rtdb, err := qtx.GetRuleTypeByName(ctx, db.GetRuleTypeByNameParams{
-			ProjectID: projectID,
-			Name:      r.GetType(),
+			Projects: projects,
+			Name:     r.GetType(),
 		})
 		if err != nil {
 			return fmt.Errorf("error getting rule type %s: %w", r.GetType(), err)

diff --git a/internal/profiles/validator.go b/internal/profiles/validator.go
@@ -97,12 +97,16 @@ func (_ *Validator) validateRuleParams(
 	// track them in the db later.
 	rulesInProfile := make(RuleMapping)
 
-	err := TraverseAllRulesForPipeline(prof, func(profileRule *minderv1.Profile_Rule) error {
-		// TODO: This will need to be updated to support
-		// the hierarchy tree once that's settled in.
+	// Allows a profile to use rules from parent projects
+	projects, err := qtx.GetParentProjects(ctx, projectID)
+	if err != nil {
+		return nil, fmt.Errorf("error getting parent projects: %w", err)
+	}
+
+	err = TraverseAllRulesForPipeline(prof, func(profileRule *minderv1.Profile_Rule) error {
 		ruleType, err := qtx.GetRuleTypeByName(ctx, db.GetRuleTypeByNameParams{
-			ProjectID: projectID,
-			Name:      profileRule.GetType(),
+			Projects: projects,
+			Name:     profileRule.GetType(),
 		})
 
 		if err != nil {
@@ -272,13 +276,16 @@ func validateEntities(
 	profile *minderv1.Profile,
 	projectID uuid.UUID,
 ) error {
+	projects, err := qtx.GetParentProjects(ctx, projectID)
+	if err != nil {
+		return fmt.Errorf("error getting parent projects: %w", err)
+	}
+
 	// validate that the entities in the profile match the entities in the project
-	err := TraverseRuleTypesForEntities(profile, func(entity minderv1.Entity, rule *minderv1.Profile_Rule) error {
-		// TODO: This will need to be updated to support
-		// the hierarchy tree once that's settled in.
+	err = TraverseRuleTypesForEntities(profile, func(entity minderv1.Entity, rule *minderv1.Profile_Rule) error {
 		ruleType, err := qtx.GetRuleTypeByName(ctx, db.GetRuleTypeByNameParams{
-			ProjectID: projectID,
-			Name:      rule.GetType(),
+			Projects: projects,
+			Name:     rule.GetType(),
 		})
 
 		if err != nil {

diff --git a/internal/profiles/validator_test.go b/internal/profiles/validator_test.go
@@ -179,6 +179,10 @@ func withArtifactRules(rules ...*minderv1.Profile_Rule) func(*minderv1.Profile)
 }
 
 func dbReturnsError(store *mockdb.MockStore) {
+	store.EXPECT().
+		GetParentProjects(gomock.Any(), gomock.Any()).
+		Return([]uuid.UUID{uuid.New()}, nil).
+		AnyTimes()
 	store.EXPECT().
 		GetRuleTypeByName(gomock.Any(), gomock.Any()).
 		Return(db.RuleType{}, sql.ErrNoRows).
@@ -193,6 +197,10 @@ func dbMockWithRuleType(rawRuleDefinition json.RawMessage) func(*mockdb.MockStor
 			Definition: rawRuleDefinition,
 		}
 
+		store.EXPECT().
+			GetParentProjects(gomock.Any(), gomock.Any()).
+			Return([]uuid.UUID{uuid.New()}, nil).
+			AnyTimes()
 		store.EXPECT().
 			GetRuleTypeByName(gomock.Any(), gomock.Any()).
 			Return(ruleType, nil).

diff --git a/internal/ruletypes/service.go b/internal/ruletypes/service.go
@@ -113,9 +113,14 @@ func (_ *ruleTypeService) CreateRuleType(
 	ruleTypeName := ruleType.GetName()
 	ruleTypeDef := ruleType.GetDef()
 
-	_, err := qtx.GetRuleTypeByName(ctx, db.GetRuleTypeByNameParams{
-		ProjectID: projectID,
-		Name:      ruleTypeName,
+	projects, err := qtx.GetParentProjects(ctx, projectID)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get parent projects: %w", err)
+	}
+
+	_, err = qtx.GetRuleTypeByName(ctx, db.GetRuleTypeByNameParams{
+		Projects: projects,
+		Name:     ruleTypeName,
 	})
 	if err == nil {
 		return nil, ErrRuleAlreadyExists
@@ -177,8 +182,9 @@ func (_ *ruleTypeService) UpdateRuleType(
 	ruleTypeDef := ruleType.GetDef()
 
 	oldRuleType, err := qtx.GetRuleTypeByName(ctx, db.GetRuleTypeByNameParams{
-		ProjectID: projectID,
-		Name:      ruleTypeName,
+		// we only need to check the project that the rule type is in
+		Projects: []uuid.UUID{projectID},
+		Name:     ruleTypeName,
 	})
 	if err != nil {
 		if errors.Is(err, sql.ErrNoRows) {

diff --git a/internal/ruletypes/service_test.go b/internal/ruletypes/service_test.go
@@ -95,33 +95,33 @@ func TestRuleTypeService(t *testing.T) {
 			Name:          "CreateRuleType rejects attempt to overwrite an existing rule",
 			RuleType:      newRuleType(withBasicStructure),
 			ExpectedError: ruletypes.ErrRuleAlreadyExists.Error(),
-			DBSetup:       dbf.NewDBMock(withSuccessfulGet),
+			DBSetup:       dbf.NewDBMock(withHierarchyGet, withSuccessfulGet),
 			TestMethod:    create,
 		},
 		{
 			Name:          "CreateRuleType returns error on rule type lookup failure",
 			RuleType:      newRuleType(withBasicStructure),
 			ExpectedError: "failed to get rule type",
-			DBSetup:       dbf.NewDBMock(withFailedGet),
+			DBSetup:       dbf.NewDBMock(withHierarchyGet, withFailedGet),
 			TestMethod:    create,
 		},
 		{
 			Name:          "CreateRuleType returns error when unable to create rule type in database",
 			RuleType:      newRuleType(withBasicStructure),
 			ExpectedError: "failed to create rule type",
-			DBSetup:       dbf.NewDBMock(withNotFoundGet, withFailedCreate),
+			DBSetup:       dbf.NewDBMock(withHierarchyGet, withNotFoundGet, withFailedCreate),
 			TestMethod:    create,
 		},
 		{
 			Name:       "CreateRuleType successfully creates a new rule type",
 			RuleType:   newRuleType(withBasicStructure),
-			DBSetup:    dbf.NewDBMock(withNotFoundGet, withSuccessfulCreate),
+			DBSetup:    dbf.NewDBMock(withHierarchyGet, withNotFoundGet, withSuccessfulCreate),
 			TestMethod: create,
 		},
 		{
 			Name:           "CreateRuleType successfully creates a new namespaced rule type",
 			RuleType:       newRuleType(withBasicStructure, withRuleName(namespacedRuleName)),
-			DBSetup:        dbf.NewDBMock(withNotFoundGet, withSuccessfulNamespaceCreate),
+			DBSetup:        dbf.NewDBMock(withHierarchyGet, withNotFoundGet, withSuccessfulNamespaceCreate),
 			SubscriptionID: subscriptionID,
 			TestMethod:     create,
 		},
@@ -205,14 +205,14 @@ func TestRuleTypeService(t *testing.T) {
 		{
 			Name:           "UpsertRuleType successfully creates a new namespaced rule type",
 			RuleType:       newRuleType(withBasicStructure, withRuleName(namespacedRuleName)),
-			DBSetup:        dbf.NewDBMock(withNotFoundGet, withSuccessfulNamespaceCreate),
+			DBSetup:        dbf.NewDBMock(withHierarchyGet, withNotFoundGet, withSuccessfulNamespaceCreate),
 			SubscriptionID: subscriptionID,
 			TestMethod:     upsert,
 		},
 		{
 			Name:           "UpsertRuleType successfully updates an existing rule",
 			RuleType:       newRuleType(withBasicStructure, withRuleName(namespacedRuleName)),
-			DBSetup:        dbf.NewDBMock(withSuccessfulNamespaceGet, withSuccessfulUpdate),
+			DBSetup:        dbf.NewDBMock(withHierarchyGet, withSuccessfulNamespaceGet, withSuccessfulUpdate),
 			TestMethod:     upsert,
 			SubscriptionID: subscriptionID,
 		},
@@ -347,6 +347,12 @@ func withIncompatibleParams(ruleType *pb.RuleType) {
 	ruleType.Def.ParamSchema = incompatibleSchema
 }
 
+func withHierarchyGet(mock dbf.DBMock) {
+	mock.EXPECT().
+		GetParentProjects(gomock.Any(), gomock.Any()).
+		Return([]uuid.UUID{uuid.New()}, nil)
+}
+
 func withSuccessfulGet(mock dbf.DBMock) {
 	mock.EXPECT().
 		GetRuleTypeByName(gomock.Any(), gomock.Any()).