feat(graphschema): Implement basic schema relationship findings CRUD BED-6868 #2207
Conversation
WalkthroughThis PR extends the database layer with transaction support and new CRUD operations. It adds a generic Transaction method enabling functions to execute within database transactions, implements five new methods for managing schema environments and relationship findings, introduces a SchemaRelationshipFinding model, and includes comprehensive integration tests and mock implementations. Changes
Sequence Diagram(s)sequenceDiagram
actor Caller
participant BloodhoundDB
participant GORM
participant Database
rect rgb(200, 220, 240)
note over Caller,Database: Transaction Success Path
Caller->>BloodhoundDB: Transaction(ctx, fn, opts)
BloodhoundDB->>GORM: BeginTx(ctx, opts)
GORM->>Database: START TRANSACTION
Database-->>GORM: ✓ Connection established
BloodhoundDB->>BloodhoundDB: fn(txDB) executes
BloodhoundDB->>Database: Query operations
Database-->>BloodhoundDB: Results
BloodhoundDB->>GORM: Commit()
GORM->>Database: COMMIT
Database-->>GORM: ✓ Committed
BloodhoundDB-->>Caller: nil (success)
end
rect rgb(240, 200, 200)
note over Caller,Database: Transaction Error Path
Caller->>BloodhoundDB: Transaction(ctx, fn, opts)
BloodhoundDB->>GORM: BeginTx(ctx, opts)
GORM->>Database: START TRANSACTION
Database-->>GORM: ✓ Connection established
BloodhoundDB->>BloodhoundDB: fn(txDB) executes
BloodhoundDB->>Database: Query operations
Database-->>BloodhoundDB: ✗ Error returned
BloodhoundDB->>GORM: Rollback()
GORM->>Database: ROLLBACK
Database-->>GORM: ✓ Rolled back
BloodhoundDB-->>Caller: error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
cmd/api/src/model/graphschema.go (1)
112-125: SchemaRelationshipFinding shape looks correct; consider documenting why it diverges from other modelsThe struct matches the selected/inserted columns and the table name, but unlike other graphschema models it doesn’t embed
Serial(noUpdatedAt/DeletedAt). If this is intentional (created‑at‑only, no soft delete), consider a brief comment or usingSerialand ignoring unused fields for consistency with the rest of the schema models.cmd/api/src/database/graphschema_integration_test.go (2)
1152-1330: SchemaEnvironment integration tests are solid; consider avoiding hard‑coded IDsThe new environment tests (CRUD + transaction scenarios) exercise the happy paths, not‑found behavior, uniqueness rollback, and create+delete within a single transaction. That’s good coverage.
The only fragility is relying on specific numeric IDs (e.g., extensionId/environmentId
1or2) and on the implicit ordering ofGetSchemaEnvironments. For future‑proofing, you could instead:
- Use the IDs returned from
CreateGraphSchemaExtension/CreateSchemaEnvironmentin both the calls and the expected structs.- Keep
wantstructs in sync by fillingSchemaExtensionIdandSerial.IDfrom those returned values rather than literals.Not urgent, but would make these tests more resilient to changes in migrations or sequence behavior.
Also applies to: 1332-1583
1585-1686: Relationship finding tests cover key behaviors but also rely on fixed IDsThe relationship‑finding tests nicely exercise creation, duplicate‑name handling via
ErrDuplicateSchemaRelationshipFindingName, retrieval by ID, deletion, and not‑found cases. The pattern of zeroingCreatedAtbefore equality checks keeps the assertions stable across runs.Similar to the environment tests, they assume specific IDs (extensionId/environmentId/findingId
1) rather than using the IDs returned from the create calls. If migrations ever seed data or sequences change, this could fail unexpectedly. Using the returned IDs in both the calls and expectations would harden these tests without changing behavior.Also applies to: 1688-1761, 1763-1828
cmd/api/src/database/graphschema.go (1)
526-530: Consider explicit ordering in GetSchemaEnvironments for deterministic results
GetSchemaEnvironmentscurrently does a bareFind(&result)with no ORDER BY. That’s fine functionally, but it leaves the row order up to the database. Since tests and potential callers appear to expect environments to come back in a stable order (by ID insertion), you might want to add.Order("id")(or whatever key you intend) to make the behavior deterministic.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
cmd/api/src/database/database_integration_test.gocmd/api/src/database/db.gocmd/api/src/database/graphschema.gocmd/api/src/database/graphschema_integration_test.gocmd/api/src/database/mocks/db.gocmd/api/src/model/appcfg/parameter_integration_test.gocmd/api/src/model/graphschema.golocal-harnesses/integration.config.json.template
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-08-12T15:30:00.877Z
Learnt from: bsheth711
Repo: SpecterOps/BloodHound PR: 1766
File: cmd/api/src/database/access_control_list.go:55-103
Timestamp: 2025-08-12T15:30:00.877Z
Learning: In BloodHound's database layer, s.UpdateUser uses nested transactions within GORM. When called inside an AuditableTransaction, error propagation from the nested transaction causes rollback of the parent transaction, maintaining atomicity across the entire operation.
Applied to files:
cmd/api/src/database/db.gocmd/api/src/database/database_integration_test.go
📚 Learning: 2025-06-25T17:52:33.291Z
Learnt from: superlinkx
Repo: SpecterOps/BloodHound PR: 1606
File: cmd/api/src/analysis/azure/post.go:33-35
Timestamp: 2025-06-25T17:52:33.291Z
Learning: In BloodHound Go code, prefer using explicit slog type functions like slog.Any(), slog.String(), slog.Int(), etc. over simple key-value pairs for structured logging. This provides better type safety and makes key-value pairs more visually distinct. For error types, use slog.Any("key", err) or slog.String("key", err.Error()).
Applied to files:
cmd/api/src/database/db.go
📚 Learning: 2025-05-29T18:24:23.227Z
Learnt from: superlinkx
Repo: SpecterOps/BloodHound PR: 1509
File: packages/go/stbernard/command/license/internal/cmd.go:49-51
Timestamp: 2025-05-29T18:24:23.227Z
Learning: In the stbernard package, use slog for all logging prints instead of fmt.Printf/fmt.Fprintf to maintain consistency with the codebase's logging standards.
Applied to files:
cmd/api/src/database/db.go
📚 Learning: 2025-11-25T22:11:53.518Z
Learnt from: LawsonWillard
Repo: SpecterOps/BloodHound PR: 2107
File: cmd/api/src/database/graphschema.go:86-100
Timestamp: 2025-11-25T22:11:53.518Z
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/db.gocmd/api/src/model/graphschema.gocmd/api/src/database/graphschema_integration_test.gocmd/api/src/database/graphschema.gocmd/api/src/database/mocks/db.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/model/graphschema.gocmd/api/src/database/graphschema_integration_test.gocmd/api/src/database/graphschema.go
📚 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/model/appcfg/parameter_integration_test.gocmd/api/src/database/database_integration_test.gocmd/api/src/database/graphschema_integration_test.go
📚 Learning: 2025-08-07T16:45:34.764Z
Learnt from: specterops-avongluck
Repo: SpecterOps/BloodHound PR: 1765
File: debian/bhapi.json.default:7-9
Timestamp: 2025-08-07T16:45:34.764Z
Learning: In BloodHound Debian packaging, the database connection string "postgresql://" in debian/bhapi.json.default is intentionally minimal because it uses PostgreSQL peer authentication. The bloodhound system user (created in postinst) can connect to the bloodhound PostgreSQL role without a password, similar to how the postgresql system user works.
Applied to files:
local-harnesses/integration.config.json.template
🧬 Code graph analysis (6)
cmd/api/src/model/graphschema.go (2)
packages/go/schemagen/tsgen/tsgen.go (1)
ID(216-220)packages/go/graphschema/common/common.go (1)
DisplayName(54-54)
cmd/api/src/model/appcfg/parameter_integration_test.go (2)
cmd/api/src/model/appcfg/parameter.go (1)
GetCitrixRDPSupport(263-273)cmd/api/src/test/integration/database.go (1)
SetupDB(126-132)
cmd/api/src/database/database_integration_test.go (2)
packages/go/graphschema/common/common.go (1)
Enabled(64-64)cmd/api/src/database/db.go (1)
BloodhoundDB(194-197)
cmd/api/src/database/graphschema_integration_test.go (4)
cmd/api/src/model/graphschema.go (4)
SchemaEnvironment(101-106)SchemaEnvironment(108-110)SchemaRelationshipFinding(113-121)SchemaRelationshipFinding(123-125)packages/go/schemagen/tsgen/tsgen.go (1)
ID(216-220)cmd/api/src/database/db.go (3)
ErrNotFound(42-42)BloodhoundDB(194-197)ErrDuplicateSchemaRelationshipFindingName(61-61)packages/go/graphschema/common/common.go (1)
DisplayName(54-54)
cmd/api/src/database/graphschema.go (3)
cmd/api/src/model/graphschema.go (4)
SchemaEnvironment(101-106)SchemaEnvironment(108-110)SchemaRelationshipFinding(113-121)SchemaRelationshipFinding(123-125)cmd/api/src/database/helper.go (1)
CheckError(39-45)cmd/api/src/database/db.go (2)
ErrNotFound(42-42)ErrDuplicateSchemaRelationshipFindingName(61-61)
cmd/api/src/database/mocks/db.go (1)
cmd/api/src/model/graphschema.go (4)
SchemaRelationshipFinding(113-121)SchemaRelationshipFinding(123-125)SchemaEnvironment(101-106)SchemaEnvironment(108-110)
⏰ 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: build-ui
- GitHub Check: run-analysis
🔇 Additional comments (9)
local-harnesses/integration.config.json.template (1)
11-11: No action needed—the change is appropriate for this template context.The switch from "neo4j" to "pg" is correct. The "pg" driver value is fully supported throughout the codebase, and the Neo4j connection configuration should remain because the system is designed to support both database drivers with runtime switching capability. The integration test framework uses the configured driver value, so setting it to "pg" in the template aligns with the PR's PostgreSQL-based graph schema operations.
cmd/api/src/model/appcfg/parameter_integration_test.go (2)
25-27: LGTM! Missing imports added.The neo4j and require imports were missing but are used in the test file (neo4j constants at line 112, require assertions throughout). This fixes a compilation issue.
119-119: The test expectation is correct. The database migrationv8.5.0.sqlexplicitly enables Citrix RDP support by updating the configuration from the defaultfalsetotrue. Whenintegration.SetupDB(t)is called, it runsdb.Migrate()which applies all migrations, including the v8.5.0 update that sets the CitrixRDP parameter to enabled. The test assertion is now properly aligned with this configuration change.cmd/api/src/database/db.go (2)
45-62: Duplicate finding error constant aligns with existing error taxonomyAdding
ErrDuplicateSchemaRelationshipFindingNamenext to the other duplicate‑key errors keeps error handling consistent and lets callers distinguish this constraint cleanly. No issues here.
229-239: Transaction helper is a clean, reusable wrapper over DB transactionsThe
Transactionmethod correctly wrapsgorm.DB.Transaction, passes throughsql.TxOptions, and exposes a tx‑scoped*BloodhoundDBso existing methods participate in the transaction without code changes. The integration tests cover commit, rollback, isolation, and read‑only behavior, so this looks solid.cmd/api/src/database/database_integration_test.go (1)
21-34: Transaction integration tests are comprehensive and exercise key pathsThe new
TestTransactionsubtests cover commit, rollback (explicit and via error), nested calls, isolation level, and read‑only enforcement against real DB state. Structure and teardown look good; this should give strong safety around the newBloodhoundDB.Transactionhelper. Only implicit dependency is on theopengraph_searchflag existing post‑migration, which seems intentional.Also applies to: 120-238
cmd/api/src/database/graphschema.go (2)
27-62: New OpenGraphSchema surface and environment CRUD match existing DAO patternsAdding
GetSchemaEnvironmentById/DeleteSchemaEnvironmenttoOpenGraphSchemaand implementing them with the same Raw+Scan / Exec +ErrNotFoundconventions used elsewhere keeps the API coherent.CreateSchemaEnvironment’s duplicate‑key handling viaErrDuplicateSchemaEnvironmentalso follows the existing duplicate‑error pattern. No functional issues spotted.Also applies to: 508-560
562-608: SchemaRelationshipFinding DAO methods look correct and align with the modelThe new relationship‑finding methods:
- Insert all and only the columns present in
SchemaRelationshipFindingand return the full row.- Map duplicate‑key violations to
ErrDuplicateSchemaRelationshipFindingName.- Use Raw SELECT + Scan with explicit
ErrNotFoundwhen no rows are found.- Mirror the delete‑semantics used for other graphschema entities (error on 0 rows affected).
Given the integration tests around creation, duplicate detection, get‑by‑id, and deletion, this implementation looks sound.
cmd/api/src/database/mocks/db.go (1)
573-587: Mocks correctly track new schema environment and relationship‑finding methodsThe added mock methods for
CreateSchemaRelationshipFinding,GetSchemaEnvironmentById,DeleteSchemaEnvironment,GetSchemaRelationshipFindingById, andDeleteSchemaRelationshipFindingmatch the database interface signatures and follow the established gomock patterns in this file. Since this is generated code, keeping it in sync viamockgenlooks appropriate.Also applies to: 946-972, 2102-2115, 2132-2145
cweidenkeller
force-pushed
the
BED-6868
branch
from
7e6fb1e to
1f9e919
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
cmd/api/src/model/graphschema.go (1)
19-19: SchemaRelationshipFinding shape matches DB usage; consider Serial if you later add more timestampsThe fields and
TableNameline up with the SQL indatabase/graphschema.go, so CRUD should marshal/unmarshal cleanly. If you ever addupdated_at/deleted_attoschema_relationship_findings, it may be worth embeddingmodel.Serial(like other schema models) instead of hand-writingID/CreatedAt, but as-is this is consistent with a “created_at-only” table.Also applies to: 112-125
cmd/api/src/database/db.go (1)
229-239: Transaction helper is correct; consider binding context into the transactional DBThe
Transactionwrapper cleanly exposes a transactional*BloodhoundDB, passes through*sql.TxOptions, and correctly lets gorm manage commit/rollback based on the callback error. This is a good addition.To make sure all methods (including any that might not reapply
WithContext) always see the intendedctx, you might consider constructing the transactional instance fromtx.WithContext(ctx)instead of baretx:Optional refactor
func (s *BloodhoundDB) Transaction(ctx context.Context, fn func(tx *BloodhoundDB) error, opts ...*sql.TxOptions) error { - return s.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error { - return fn(NewBloodhoundDB(tx, s.idResolver)) - }, opts...) + return s.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error { + return fn(NewBloodhoundDB(tx.WithContext(ctx), s.idResolver)) + }, opts...) }cmd/api/src/database/graphschema_integration_test.go (4)
1332-1410: Schema environment GetById tests look good but rely on hard-coded IDsThe success and not-found cases exercise
GetSchemaEnvironmentByIdcorrectly, includingErrNotFoundpropagation and zeroing time fields before comparison.To make the tests more robust against future schema or seed-data changes, consider capturing the created environment’s ID from
CreateSchemaEnvironmentinsetupand passing that throughargsinstead of hard-codingenvironmentId: 1.
1412-1480: Delete schema environment tests are correct; same note on hard-coded IDsThe delete tests validate both the happy path (including verifying deletion via a follow-up
GetSchemaEnvironmentById) and the not-found behavior.As with the GetById tests, using the ID returned from
CreateSchemaEnvironmentrather than literals like1/9999would decouple these tests from assumptions about sequence state and make them less brittle.
1482-1583: Transaction tests provide solid coverage of commit and rollback semantics
TestTransaction_SchemaEnvironmentnicely covers:
- Successful multi-row commit.
- Manual rollback via an application error.
- Rollback on a database constraint violation.
- Create-then-delete within a single transaction.
This gives good confidence in the new
BloodhoundDB.Transactionhelper and the schema environment methods. If you want additional assurance later, you could add an assertion that the constraint-rollback case returnsErrDuplicateSchemaEnvironment(not just any error), but that’s not strictly necessary for this PR.
1585-1828: Relationship finding CRUD tests are correct but depend on magic IDsThe relationship-finding tests correctly exercise:
- Creation with expected data.
- Duplicate-name handling via
ErrDuplicateSchemaRelationshipFindingName.- Get-by-id success and not-found behavior.
- Delete and post-delete not-found verification.
Two potential robustness improvements:
Avoid hard-coded IDs
All tests assume specific IDs (e.g., finding/environment/extension ID of1). Using the IDs returned fromCreateGraphSchemaExtension,CreateSchemaEnvironment, andCreateSchemaRelationshipFindingwould make these tests resilient to changes in seed data or sequence behavior.Make relationshipKindId explicit
Tests currently hard-coderelationshipKindId: 1without creating a corresponding schema edge kind. If the schema enforces a foreign key onrelationship_kind_id, these tests will implicitly depend on specific seed data. Creating aGraphSchemaEdgeKindinsetupand using its ID would make the relationship clearer and the tests more self-contained.These are forward-looking test-hardening tweaks; functionally the tests exercise the new API surface well.
cmd/api/src/database/graphschema.go (1)
59-61: Relationship finding CRUD implementation looks correct and matches existing patternsFor
SchemaRelationshipFinding:
CreateSchemaRelationshipFinding:
- Inserts via
INSERT ... RETURNINGusing the model’sTableName().- Detects duplicate-key errors by checking for
DuplicateKeyValueErrorStringand wraps them asErrDuplicateSchemaRelationshipFindingNamewith%w, soerrors.Isworks.- Otherwise delegates to
CheckError.
GetSchemaRelationshipFindingByIdandDeleteSchemaRelationshipFinding:
- Use
Raw/ScanandExecrespectively, withCheckErrorfor DB failures.- Map
RowsAffected == 0toErrNotFound, consistent with other getters/deleters.Everything is wired cleanly into the
OpenGraphSchemainterface and should behave correctly within or outside explicit transactions. If you want to align even more with earlier helpers likeGetGraphSchemaExtensionById, you could useRaw(...).First(&finding)and rely solely onCheckErrorfor not-found detection, but the current explicitRowsAffectedchecks are clear and correct.Also applies to: 562-608
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
cmd/api/src/database/database_integration_test.gocmd/api/src/database/db.gocmd/api/src/database/graphschema.gocmd/api/src/database/graphschema_integration_test.gocmd/api/src/database/mocks/db.gocmd/api/src/model/graphschema.go
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/api/src/database/database_integration_test.go
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-08-12T15:30:00.877Z
Learnt from: bsheth711
Repo: SpecterOps/BloodHound PR: 1766
File: cmd/api/src/database/access_control_list.go:55-103
Timestamp: 2025-08-12T15:30:00.877Z
Learning: In BloodHound's database layer, s.UpdateUser uses nested transactions within GORM. When called inside an AuditableTransaction, error propagation from the nested transaction causes rollback of the parent transaction, maintaining atomicity across the entire operation.
Applied to files:
cmd/api/src/database/db.go
📚 Learning: 2025-11-25T22:11:53.518Z
Learnt from: LawsonWillard
Repo: SpecterOps/BloodHound PR: 2107
File: cmd/api/src/database/graphschema.go:86-100
Timestamp: 2025-11-25T22:11:53.518Z
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/db.gocmd/api/src/database/graphschema.gocmd/api/src/model/graphschema.go
📚 Learning: 2025-06-25T17:52:33.291Z
Learnt from: superlinkx
Repo: SpecterOps/BloodHound PR: 1606
File: cmd/api/src/analysis/azure/post.go:33-35
Timestamp: 2025-06-25T17:52:33.291Z
Learning: In BloodHound Go code, prefer using explicit slog type functions like slog.Any(), slog.String(), slog.Int(), etc. over simple key-value pairs for structured logging. This provides better type safety and makes key-value pairs more visually distinct. For error types, use slog.Any("key", err) or slog.String("key", err.Error()).
Applied to files:
cmd/api/src/database/db.go
📚 Learning: 2025-05-29T18:24:23.227Z
Learnt from: superlinkx
Repo: SpecterOps/BloodHound PR: 1509
File: packages/go/stbernard/command/license/internal/cmd.go:49-51
Timestamp: 2025-05-29T18:24:23.227Z
Learning: In the stbernard package, use slog for all logging prints instead of fmt.Printf/fmt.Fprintf to maintain consistency with the codebase's logging standards.
Applied to files:
cmd/api/src/database/db.go
📚 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_integration_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 (1)
cmd/api/src/database/graphschema.go (3)
cmd/api/src/model/graphschema.go (4)
SchemaEnvironment(101-106)SchemaEnvironment(108-110)SchemaRelationshipFinding(113-121)SchemaRelationshipFinding(123-125)cmd/api/src/database/helper.go (1)
CheckError(39-45)cmd/api/src/database/db.go (3)
BloodhoundDB(194-197)ErrNotFound(42-42)ErrDuplicateSchemaRelationshipFindingName(61-61)
⏰ 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). (5)
- GitHub Check: Build BloodHound Container Image / Build and Package Container
- GitHub Check: run-tests
- GitHub Check: run-go-unit-tests
- GitHub Check: run-analysis
- GitHub Check: build-ui
🔇 Additional comments (3)
cmd/api/src/database/db.go (1)
45-62: New duplicate-name error follows existing pattern
ErrDuplicateSchemaRelationshipFindingNameis consistent with the other duplicate-* sentinels and is correctly exported for reuse by callers and tests. No issues here.cmd/api/src/database/graphschema.go (1)
54-60: Schema environment data-access methods are consistent and error-safeThe additions for schema environments:
OpenGraphSchemagainsGetSchemaEnvironmentByIdandDeleteSchemaEnvironmentalongsideCreateSchemaEnvironment/GetSchemaEnvironments.GetSchemaEnvironmentsusesFindwithCheckError, correctly returning an empty slice andnilerror when there are no rows.GetSchemaEnvironmentByIdandDeleteSchemaEnvironmentboth:
- Use the model’s
TableName()to build SQL.- Map DB errors via
CheckError.- Explicitly translate
RowsAffected == 0toErrNotFound.These behaviors align with the rest of the graphschema accessors and work correctly in transactional contexts (when called on a transactional
*BloodhoundDB).Also applies to: 526-560
cmd/api/src/database/mocks/db.go (1)
573-586: New mock methods correctly mirror the extended Database interfaceThe added gomock methods and recorders:
CreateSchemaRelationshipFindingGetSchemaEnvironmentByIdDeleteSchemaEnvironmentGetSchemaRelationshipFindingByIdDeleteSchemaRelationshipFindingall match the signatures of the newly added
OpenGraphSchema/Databasemethods (argument order and return types) and follow the same pattern as existing mocks (call-through plus type assertions).This generated code looks consistent and ready for use in unit tests.
Also applies to: 946-972, 2102-2115, 2132-2145
Description
Describe your changes in detail
Motivation and Context
Resolves BED-6868
Why is this change required? What problem does it solve?
Added CRUD operations for relationship findings table
How Has This Been Tested?
Added integration tests
Please describe in detail how you tested your changes.
Include details of your testing environment, and the tests you ran to
see how your change affects other areas of the code, etc.
Screenshots (optional):
Types of changes
Checklist:
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.