feat(OpenGraph): DB - Create/Read Extension Edge Kind Schema Entry - BED-6790 #2107
Conversation
LawsonWillard
added
enhancement
WalkthroughAdds a new SchemaEdgeKind model and DB table plus migration, implements create/get DB methods (with duplicate-name error mapping), updates gomock mocks, and adds integration tests covering creation, retrieval, not-found, and duplicate-name handling. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller
participant DB as BloodhoundDB
participant PG as Postgres
rect rgb(220,240,255)
Note over Caller,DB: CreateSchemaEdgeKind
Caller->>DB: CreateSchemaEdgeKind(ctx, name, extId, desc, isTraversable)
DB->>PG: INSERT INTO schema_edge_kinds (...) RETURNING *
alt success
PG-->>DB: inserted row
DB-->>Caller: model.SchemaEdgeKind (created)
else unique violation
PG-->>DB: unique constraint error
DB-->>Caller: ErrDuplicateSchemaEdgeKindName
end
end
rect rgb(240,255,220)
Note over Caller,DB: GetSchemaEdgeKindById
Caller->>DB: GetSchemaEdgeKindById(ctx, id)
DB->>PG: SELECT ... FROM schema_edge_kinds WHERE id = $1
alt found
PG-->>DB: row
DB-->>Caller: model.SchemaEdgeKind
else not found
PG-->>DB: no rows
DB-->>Caller: ErrEntityNotFound (via CheckError)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-11-25T22:11:53.518ZApplied to files:
📚 Learning: 2025-06-06T23:12:14.181ZApplied to files:
🧬 Code graph analysis (2)cmd/api/src/database/graphschema.go (3)
cmd/api/src/database/mocks/db.go (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
cmd/api/src/model/graphschema.go (1)
42-54: SchemaEdgeKind mapping looks correct; tighten comments and consider tagsThe struct and
TableName()correctly align withschema_edge_kinds(includingschema_extension_id INT→SchemaExtensionId int32and the other columns), so the DB mapping looks good.Two small nits you may want to address:
- The comments say “node kind” and
// indicates, which is a bit misleading/incomplete for an edge kind; updating to “edge kind” and a full description would avoid confusion.GraphSchemaExtensionusesjson/validatetags; ifSchemaEdgeKindis ever surfaced via the API, mirroring that style (e.g.,json:"name" validate:"required") would keep things consistent. This can be deferred until you expose it beyond the DB layer.cmd/api/src/database/graphschema_test.go (1)
80-145: Integration coverage for SchemaEdgeKind is solid; a couple of minor polish opportunitiesThe test flow (create two edge kinds, read-by-id, assert not-found, then assert duplicate-name maps to
ErrDuplicateSchemaEdgeKindName) lines up well with the new DB methods and migration. ThecompareSchemaEdgeKindhelper keeps the assertions focused on the important fields. No functional issues here.A few small, optional refinements you might consider:
- For the not-found case (Line 132–133), using
require.ErrorIs(t, err, database.ErrNotFound)would be a bit more robust than asserting the string"entity not found", and would match how you’re already handling the duplicate-name sentinel.- The
compareSchemaEdgeKindmessages all sayCreateSchemaEdgeKind - ...even when used afterGetSchemaEdgeKindById; if you ever hit a failure while reading, that message might be slightly confusing. Renaming to something generic likeSchemaEdgeKind - name - ...would make it reusable across both paths.These are small test-quality tweaks; the current tests are still clear and effective.
cmd/api/src/database/migration/migrations/v8.5.0.sql (1)
42-54: DDL forschema_edge_kindsmatches model and usage; only comment text is slightly offThe table definition looks consistent with the Go model and queries:
- Column names/types align with
model.SchemaEdgeKindand theINSERT/SELECTstatements.schema_extension_idFK withON DELETE CASCADEand a separate index on that column are sensible for lookups by extension.- The uniqueness and regex constraint on
namematch the intent described in the comment and the duplicate-name error handling.Two tiny nits:
- The inline comment for
schema_extension_idsays “node kind”; updating it to “edge kind” would avoid confusion.- If you ever decide that edge-kind names should be unique only within an extension, you may want to change
UNIQUE (name)to a composite unique over(schema_extension_id, name). If global uniqueness is the goal, the current definition is correct.Nothing blocking here.
cmd/api/src/database/graphschema.go (3)
27-30: Consider exposing edge-kind operations via the OpenGraphSchema interfaceYou’ve added
CreateSchemaEdgeKind/GetSchemaEdgeKindByIdon*BloodhoundDB, butOpenGraphSchema(and thereforeDatabase) still only exposes the extension methods. If callers are meant to depend on theOpenGraphSchema/Databaseabstraction rather than the concrete*BloodhoundDB, it may be worth adding the new methods to the interface now so they’re available to services in the same way as the extension operations.If you’re intentionally keeping these as internal helpers for now, then this can wait, but it’s an easy place for future drift between the interface and the concrete implementation.
Also applies to: 82-105
82-97: CreateSchemaEdgeKind implementation is correct and consistent with existing patternsThe insert logic looks good:
- Uses parameterized
INSERT ... RETURNINGwithschemaEdgeKind.TableName()so there’s no SQL injection risk.- Maps the Postgres unique-constraint string to
ErrDuplicateSchemaEdgeKindName, matching how extensions handleErrDuplicateGraphSchemaExtensionName.- Falls back to
CheckError(result)for other DB errors, which will appropriately surface non-not-found failures.One optional consideration:
CreateGraphSchemaExtensionruns insideAuditableTransactionwith anAuditEntry, while edge-kind creation currently does not. If edge-kind definitions are considered configuration that should be audited, you might want to mirror that pattern later for consistency. Functionally, this is fine as-is.
99-105: GetSchemaEdgeKindById is functionally correct; style is slightly different from the extension versionThis method correctly:
- Queries by
idusing the same set of columns defined in the migration/model.- Relies on
CheckErrorto translategorm.ErrRecordNotFoundintoErrNotFound, which aligns with how the tests assert the not-found case.It’s a bit more compact than
GetGraphSchemaExtensionById(which uses an explicitif result := ...; result.Error != nil { ... }block), but behavior is equivalent. If you prefer consistency, you could mirror that style; otherwise, this is fine.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
cmd/api/src/database/db.go(1 hunks)cmd/api/src/database/graphschema.go(1 hunks)cmd/api/src/database/graphschema_test.go(1 hunks)cmd/api/src/database/migration/migrations/v8.5.0.sql(1 hunks)cmd/api/src/model/graphschema.go(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-22T20:30:34.839Z
Learnt from: LawsonWillard
Repo: SpecterOps/BloodHound PR: 1700
File: cmd/api/src/api/v2/saved_queries_test.go:3182-3182
Timestamp: 2025-07-22T20:30:34.839Z
Learning: In Go table-driven tests in cmd/api/src/api/v2/saved_queries_test.go, subtest parallelization with t.Parallel() is acceptable when tests are self-contained, each creating their own mock controller (gomock.NewController(t)) and having isolated mock expectations without shared state between subtests.
Applied to files:
cmd/api/src/database/graphschema_test.go
📚 Learning: 2025-06-06T23:12:14.181Z
Learnt from: elikmiller
Repo: SpecterOps/BloodHound PR: 1563
File: packages/go/graphschema/azure/azure.go:24-24
Timestamp: 2025-06-06T23:12:14.181Z
Learning: In BloodHound, files in packages/go/graphschema/*/`*.go` are generated from CUE schemas. When `just prepare-for-codereview` is run, it triggers code generation that may automatically add import aliases or other formatting changes. These changes are legitimate outputs of the generation process, not manual edits that would be overwritten.
Applied to files:
cmd/api/src/database/graphschema.go
🧬 Code graph analysis (3)
cmd/api/src/database/graphschema_test.go (3)
cmd/api/src/model/graphschema.go (2)
SchemaEdgeKind(43-50)SchemaEdgeKind(52-54)packages/go/graphschema/common/common.go (1)
Description(55-55)cmd/api/src/database/db.go (1)
ErrDuplicateSchemaEdgeKindName(56-56)
cmd/api/src/model/graphschema.go (1)
packages/go/graphschema/common/common.go (1)
Description(55-55)
cmd/api/src/database/graphschema.go (3)
cmd/api/src/database/db.go (2)
BloodhoundDB(189-192)ErrDuplicateSchemaEdgeKindName(56-56)cmd/api/src/model/graphschema.go (2)
SchemaEdgeKind(43-50)SchemaEdgeKind(52-54)cmd/api/src/database/helper.go (1)
CheckError(26-32)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build BloodHound Container Image / Build and Package Container
- GitHub Check: run-tests
- GitHub Check: run-analysis
- GitHub Check: build-ui
🔇 Additional comments (1)
cmd/api/src/database/db.go (1)
45-57: New duplicate edge-kind error aligns with existing patterns
ErrDuplicateSchemaEdgeKindNameis consistent with other duplicate* sentinels and cleanly supports mapping the unique-constraint violation fromschema_edge_kinds. No issues here.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cmd/api/src/database/graphschema.go(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-06T23:12:14.181Z
Learnt from: elikmiller
Repo: SpecterOps/BloodHound PR: 1563
File: packages/go/graphschema/azure/azure.go:24-24
Timestamp: 2025-06-06T23:12:14.181Z
Learning: In BloodHound, files in packages/go/graphschema/*/`*.go` are generated from CUE schemas. When `just prepare-for-codereview` is run, it triggers code generation that may automatically add import aliases or other formatting changes. These changes are legitimate outputs of the generation process, not manual edits that would be overwritten.
Applied to files:
cmd/api/src/database/graphschema.go
🧬 Code graph analysis (1)
cmd/api/src/database/graphschema.go (3)
cmd/api/src/model/graphschema.go (2)
SchemaEdgeKind(43-50)SchemaEdgeKind(52-54)cmd/api/src/database/db.go (2)
BloodhoundDB(189-192)ErrDuplicateSchemaEdgeKindName(56-56)cmd/api/src/database/helper.go (1)
CheckError(26-32)
🪛 golangci-lint (2.5.0)
cmd/api/src/database/graphschema.go
[major] 103-103: : # github.com/specterops/bloodhound/cmd/api/src/api [github.com/specterops/bloodhound/cmd/api/src/api.test]
cmd/api/src/api/auth_internal_test.go:103:32: cannot use mockDB (variable of type *"github.com/specterops/bloodhound/cmd/api/src/database/mocks".MockDatabase) as database.Database value in struct literal: *"github.com/specterops/bloodhound/cmd/api/src/database/mocks".MockDatabase does not implement database.Database (missing method CreateSchemaEdgeKind)
cmd/api/src/api/auth_internal_test.go:118:32: cannot use mockDB (variable of type *"github.com/specterops/bloodhound/cmd/api/src/database/mocks".MockDatabase) as database.Database value in struct literal: *"github.com/specterops/bloodhound/cmd/api/src/database/mocks".MockDatabase does not implement database.Database (missing method CreateSchemaEdgeKind)
cmd/api/src/api/auth_internal_test.go:136:21: cannot use dbMocks.NewMockDatabase(ctrl) (value of type *"github.com/specterops/bloodhound/cmd/api/src/database/mocks".MockDatabase) as database.Database value in struct literal: *"github.com/specterops/bloodhound/cmd/api/src/database/mocks".MockDatabase does not implement database.Database (missing method CreateSchemaEdgeKind)
cmd/api/src/api/auth_internal_test.go:219:9: impossible type assertion: authenticator.db.(*dbMocks.MockDatabase)
*"github.com/specterops/bloodhound/cmd/api/src/database/mocks".MockDatabase does not implement database.Database (missing method CreateSchemaEdgeKind)
cmd/api/src/api/auth_internal_test.go:241:9: impossible type assertion: authenticator.db.(*dbMocks.MockDatabase)
*"github.com/specterops/bloodhound/cmd/api/src/database/mocks".MockDatabase does not implement database.Database (missing method CreateSchemaEdgeKind)
cmd/api/src/api/auth_internal_test.go:266:9: impossible type assertion: authenticator.db.(*dbMocks.MockDatabase)
*"github.com/specterops/bloodhound/cmd/api/src/database/mocks".MockDatabase does not implement database.Database (missing method CreateSchemaEdgeKind)
cmd/api/src/api/auth_internal_test.go:296:9: impossible type assertion: authenticator.db.(*dbMocks.MockDatabase)
*"github.com/specterops/bloodhound/cmd/api/src/database/mocks".MockDatabase does not implement database.Database (missing method CreateSchemaEdgeKind)
cmd/api/src/api/auth_internal_test.go:323:9: impossible type assertion: authenticator.db.(*dbMocks.MockDatabase)
*"github.com/specterops/bloodhound/cmd/api/src/database/mocks".MockDatabase does not implement database.Database (missing method CreateSchemaEdgeKind)
cmd/api/src/api/auth_internal_test.go:365:9: impossible type assertion: authenticator.db.(*dbMocks.MockDatabase)
*"github.com/specterops/bloodhound/cmd/api/src/database/mocks".MockDatabase does not implement database.Database (missing method CreateSchemaEdgeKind)
cmd/api/src/api/auth_internal_test.go:398:9: impossible type assertion: authenticator.db.(*dbMocks.MockDatabase)
*"github.com/specterops/bloodhound/cmd/api/src/database/mocks".MockDatabase does not implement database.Database (missing method CreateSchemaEdgeKind)
cmd/api/src/api/auth_internal_test.go:398:9: too many errors
(typecheck)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build BloodHound Container Image / Build and Package Container
- GitHub Check: run-analysis
- GitHub Check: build-ui
- GitHub Check: run-tests
🔇 Additional comments (3)
cmd/api/src/database/graphschema.go (3)
31-32: LGTM!The interface method signatures are well-structured and follow consistent patterns with the existing
OpenGraphSchemamethods.
103-108: LGTM!The implementation follows the established pattern from
GetGraphSchemaExtensionByIdand correctly handles errors throughCheckError.
31-32: Regenerate mocks to include new interface methods.The Database interface now embeds the OpenGraphSchema interface, which defines two new methods:
CreateSchemaEdgeKindandGetSchemaEdgeKindById. The mock implementation atcmd/api/src/database/mocks/db.godoes not yet include these methods.Regenerate mocks by running:
go generate ./cmd/api/src/database/db.goThis will update
mocks/db.gowith the missing interface methods.
# Conflicts: # cmd/api/src/database/db.go # cmd/api/src/database/graphschema.go # cmd/api/src/database/graphschema_test.go # cmd/api/src/database/migration/migrations/v8.5.0.sql # cmd/api/src/database/mocks/db.go # cmd/api/src/model/graphschema.go
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cmd/api/src/model/graphschema.go (1)
67-67: Complete the inline documentation.The comment on Line 67 is incomplete:
// indicatesshould explain whatIsTraversableindicates (e.g., "indicates whether the edge kind can be traversed in graph queries" or similar).Apply this diff to complete the comment:
- IsTraversable bool // indicates + IsTraversable bool // indicates whether the edge kind is traversable in graph queries
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
cmd/api/src/database/db.go(1 hunks)cmd/api/src/database/graphschema.go(2 hunks)cmd/api/src/database/graphschema_test.go(1 hunks)cmd/api/src/database/migration/migrations/v8.5.0.sql(1 hunks)cmd/api/src/database/mocks/db.go(2 hunks)cmd/api/src/model/graphschema.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/api/src/database/db.go
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: LawsonWillard
Repo: SpecterOps/BloodHound PR: 2107
File: cmd/api/src/database/graphschema.go:86-100
Timestamp: 2025-11-25T22:11:53.509Z
Learning: In cmd/api/src/database/graphschema.go, the CreateSchemaEdgeKind method intentionally does not use AuditableTransaction or audit logging because it would create too much noise in the audit log, unlike CreateGraphSchemaExtension which does use auditing.
📚 Learning: 2025-11-25T22:11:53.509Z
Learnt from: LawsonWillard
Repo: SpecterOps/BloodHound PR: 2107
File: cmd/api/src/database/graphschema.go:86-100
Timestamp: 2025-11-25T22:11:53.509Z
Learning: In cmd/api/src/database/graphschema.go, the CreateSchemaEdgeKind method intentionally does not use AuditableTransaction or audit logging because it would create too much noise in the audit log, unlike CreateGraphSchemaExtension which does use auditing.
Applied to files:
cmd/api/src/database/migration/migrations/v8.5.0.sqlcmd/api/src/model/graphschema.gocmd/api/src/database/graphschema.gocmd/api/src/database/mocks/db.gocmd/api/src/database/graphschema_test.go
📚 Learning: 2025-06-06T23:12:14.181Z
Learnt from: elikmiller
Repo: SpecterOps/BloodHound PR: 1563
File: packages/go/graphschema/azure/azure.go:24-24
Timestamp: 2025-06-06T23:12:14.181Z
Learning: In BloodHound, files in packages/go/graphschema/*/`*.go` are generated from CUE schemas. When `just prepare-for-codereview` is run, it triggers code generation that may automatically add import aliases or other formatting changes. These changes are legitimate outputs of the generation process, not manual edits that would be overwritten.
Applied to files:
cmd/api/src/database/graphschema.go
🧬 Code graph analysis (3)
cmd/api/src/model/graphschema.go (1)
packages/go/graphschema/common/common.go (1)
Description(55-55)
cmd/api/src/database/mocks/db.go (1)
cmd/api/src/model/graphschema.go (2)
SchemaEdgeKind(61-68)SchemaEdgeKind(70-72)
cmd/api/src/database/graphschema_test.go (2)
cmd/api/src/model/graphschema.go (2)
SchemaEdgeKind(61-68)SchemaEdgeKind(70-72)cmd/api/src/database/db.go (1)
ErrDuplicateSchemaEdgeKindName(57-57)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build BloodHound Container Image / Build and Package Container
- GitHub Check: run-analysis
- GitHub Check: build-ui
- GitHub Check: run-tests
# Conflicts: # cmd/api/src/database/db.go # cmd/api/src/database/graphschema.go # cmd/api/src/database/graphschema_test.go # cmd/api/src/database/migration/migrations/v8.5.0.sql # cmd/api/src/model/graphschema.go
Description
Added a new schema_edge_kinds table and the create and read by id methods used to interact with it.
Added integration tests to cover these methods.
Motivation and Context
Resolves: BED-6790
This feature is required to enable OpenGraph schemas
How Has This Been Tested?
Types of changes
Checklist:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.