Skip to content

Commit d8d503a

Browse files
timbastintimbastin
authored
Merge pull request #1992 from l3montree-dev/overhaul-component-dependencies-table
Overhaul component dependencies table
2 parents e86b035 + fb3e93b commit d8d503a

12 files changed

Lines changed: 101 additions & 76 deletions

‎.gitignore‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,5 @@ key.pem
3232
cache
3333
CLAUDE.md
3434
.claudeignore
35-
result
35+
result
36+
instance_admin_pub_key_new.pem

‎database/migrations/20260518100544_remove_obsolete_indexes_and_columns_from_component_dependencies.up.sql‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
ALTER TABLE public.component_dependencies
2+
DROP COLUMN IF EXISTS semver_start,
3+
DROP COLUMN IF EXISTS semver_end;
4+
5+
DROP INDEX IF EXISTS idx_component_dependencies_component_purl;
6+
DROP INDEX IF EXISTS component_purl_idx;
7+
DROP INDEX IF EXISTS asset_version_name_idx;
8+
DROP INDEX IF EXISTS idx_component_dependencies_dependency_purl;
9+
DROP INDEX IF EXISTS idx_component_dependencies_null_roots;
10+
DROP INDEX IF EXISTS asset_id_idx;
11+
DROP INDEX IF EXISTS idx_component_dependencies_component_id;
12+
DROP INDEX IF EXISTS idx_component_dependencies_dependency_id;
13+
DROP INDEX IF EXISTS idx_component_dependencies_recursive_lookup;
14+
DROP INDEX IF EXISTS dependency_purl_idx;
15+
16+
-- remove the current id column and replace it with a composite key on all columns to make enforce deduplication on a data level and reduce complexity (also on indexes)
17+
18+
-- currently component_id can be NULL if its a root node; but primary keys cannot contain NULL values, so we replace it with a ROOT constant
19+
INSERT INTO public.components (id, component_type, license, published, project_key)
20+
VALUES('ROOT','',NULL,NULL,NULL) ON CONFLICT (id) DO NOTHING; -- add a ROOT component to the components table to be referenced
21+
22+
UPDATE public.component_dependencies SET component_id = 'ROOT' WHERE component_id IS NULL; -- use an explicit value for ROOT component dependencies instead of NULL
23+
24+
ALTER TABLE public.component_dependencies DROP CONSTRAINT component_dependencies_pkey, DROP COLUMN id; -- drop primary key constraint and the primary key column
25+
26+
-- remove all duplicate entries from the table so that the new primary key does not fail on creation
27+
DELETE FROM public.component_dependencies a
28+
USING public.component_dependencies b
29+
WHERE a.ctid < b.ctid -- use the internal column id to choose only 1 candidate per duplicate row
30+
AND a.asset_id = b.asset_id
31+
AND a.asset_version_name = b.asset_version_name
32+
AND a.dependency_id = b.dependency_id
33+
AND a.component_id = b.component_id;
34+
35+
ALTER TABLE public.component_dependencies ADD PRIMARY KEY (asset_id, asset_version_name, dependency_id, component_id); -- add the new primary key consisting of all 4 attributes

‎database/models/component_model.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,11 @@ func (c Component) GetID() (packageurl.PackageURL, error) {
6565
}
6666

6767
type ComponentDependency struct {
68-
ID uuid.UUID `gorm:"primarykey;type:uuid;default:gen_random_uuid()" json:"id"`
6968
// the provided sbom from cyclondx only contains the transitive dependencies, which do really get used
7069
// this means, that the dependency graph between people using the same library might differ, since they use it differently
7170
// we use edges, which provide the information, that a component is used by another component in one asset
7271
Component Component `json:"component" gorm:"foreignKey:ComponentID;references:ID;constraint:OnDelete:CASCADE;"`
73-
ComponentID *string `json:"componentPurl" gorm:"column:component_id;index:component_purl_idx"` // will be nil, for direct dependencies
72+
ComponentID string `json:"componentPurl" gorm:"column:component_id;index:component_purl_idx"` // will be ROOT for direct dependencies
7473
Dependency Component `json:"dependency" gorm:"foreignKey:DependencyID;references:ID;constraint:OnDelete:CASCADE;"`
7574
DependencyID string `json:"dependencyPurl" gorm:"column:dependency_id;index:dependency_purl_idx"`
7675

@@ -93,7 +92,7 @@ func (c ComponentDependencyNode) GetID() string {
9392
func (c ComponentDependency) ToNodes() []ComponentDependencyNode {
9493
// a component dependency represents an edge in the dependency tree
9594
// thus we can represent it as two nodes
96-
return []ComponentDependencyNode{{ID: utils.SafeDereference(c.ComponentID)}, {ID: c.DependencyID}}
95+
return []ComponentDependencyNode{{ID: c.ComponentID}, {ID: c.DependencyID}}
9796
}
9897

9998
func resolveLicense(component ComponentDependency, componentLicenseOverwrites map[string]string) cyclonedx.Licenses {
@@ -242,7 +241,7 @@ func (c ComponentDependency) GetID() string {
242241
return c.DependencyID
243242
}
244243

245-
func (c ComponentDependency) GetDependentID() *string {
244+
func (c ComponentDependency) GetDependentID() string {
246245
return c.ComponentID
247246
}
248247

‎database/repositories/component_repository.go‎

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"github.com/l3montree-dev/devguard/utils"
2929
"github.com/pkg/errors"
3030
"gorm.io/gorm"
31+
"gorm.io/gorm/clause"
3132
)
3233

3334
type componentRepository struct {
@@ -55,7 +56,7 @@ func (c *componentRepository) CreateComponents(ctx context.Context, tx *gorm.DB,
5556
return nil
5657
}
5758

58-
return c.GetDB(ctx, tx).Create(&components).Error
59+
return c.GetDB(ctx, tx).Clauses(clause.OnConflict{DoNothing: true}).Create(&components).Error
5960
}
6061

6162
// LoadComponents loads all component dependencies for an asset version.
@@ -147,8 +148,8 @@ func (c *componentRepository) LoadComponentsWithProject(ctx context.Context, tx
147148
componentDependencies[i].Dependency.License = &license
148149
componentDependencies[i].Dependency.IsLicenseOverwritten = true
149150
}
150-
if component.ComponentID != nil {
151-
if license, ok := isPurlOverwrittenMap[*component.ComponentID]; ok {
151+
if component.ComponentID != "ROOT" {
152+
if license, ok := isPurlOverwrittenMap[component.ComponentID]; ok {
152153
componentDependencies[i].Component.License = &license
153154
componentDependencies[i].Component.IsLicenseOverwritten = true
154155
}
@@ -196,15 +197,9 @@ func (c *componentRepository) HandleStateDiff(ctx context.Context, tx *gorm.DB,
196197
if len(diff.RemovedEdges) > 0 {
197198
var valueClauses []string
198199
for _, edge := range diff.RemovedEdges {
199-
var componentID string
200-
if edge[0] == normalize.GraphRootNodeID {
201-
componentID = "NULL"
202-
} else {
203-
componentID = fmt.Sprintf("'%s'", strings.ReplaceAll(edge[0], "'", "''"))
204-
}
205-
200+
escapedComp := strings.ReplaceAll(edge[0], "'", "''")
206201
escapedDep := strings.ReplaceAll(edge[1], "'", "''")
207-
valueClauses = append(valueClauses, fmt.Sprintf("(%s, '%s')", componentID, escapedDep))
202+
valueClauses = append(valueClauses, fmt.Sprintf("('%s', '%s')", escapedComp, escapedDep))
208203
}
209204
// Join the value clauses with commas
210205
values := strings.Join(valueClauses, ",")
@@ -232,12 +227,12 @@ func (c *componentRepository) HandleStateDiff(ctx context.Context, tx *gorm.DB,
232227
for _, edge := range diff.AddedEdges {
233228
c1 := wholeAssetGraph.Node(edge[0])
234229
c2 := wholeAssetGraph.Node(edge[1])
235-
var componentID *string
230+
var componentID string
236231
if c1.Type == normalize.GraphNodeTypeRoot {
237-
// set to nil for root nodes
238-
componentID = nil
232+
// set to ROOT for root nodes
233+
componentID = "ROOT"
239234
} else {
240-
componentID = utils.Ptr(c1.Component.PackageURL)
235+
componentID = c1.Component.PackageURL
241236
}
242237

243238
componentDependency := models.ComponentDependency{

‎database/repositories/gorm_repository.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -234,10 +234,10 @@ WHERE NOT EXISTS (SELECT artifact_dependency_vulns.dependency_vuln_id FROM artif
234234
DELETE FROM license_risks lr
235235
WHERE NOT EXISTS (SELECT artifact_license_risks.license_risk_id FROM artifact_license_risks WHERE artifact_license_risks.license_risk_id = lr.id);
236236
237-
-- Clean up artifact root nodes (component_id IS NULL, dependency_id LIKE 'artifact:%')
237+
-- Clean up artifact root nodes (component_id = 'ROOT', dependency_id LIKE 'artifact:%')
238238
-- where the artifact no longer exists
239239
DELETE FROM component_dependencies cd
240-
WHERE cd.component_id IS NULL
240+
WHERE cd.component_id = 'ROOT'
241241
AND cd.dependency_id LIKE 'artifact:%'
242242
AND NOT EXISTS (
243243
SELECT 1 FROM artifacts a

‎integrations/commonint/integration_helper_test.go‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -395,13 +395,13 @@ func TestRenderPathToComponent(t *testing.T) {
395395
}
396396

397397
})
398-
t.Run("Everything works as expeted with a non empty component list", func(t *testing.T) {
398+
t.Run("Everything works as expected with a non empty component list", func(t *testing.T) {
399399
// Create a chain of actual components (all with pkg: prefix) to have a path with edges
400400
components := []models.ComponentDependency{
401-
{ComponentID: nil, DependencyID: "artifact:test-artifact", Dependency: models.Component{ID: "artifact:test-artifact"}}, // root --> artifact
402-
{ComponentID: utils.Ptr("artifact:test-artifact"), DependencyID: "sbom:test@test-artifact", Dependency: models.Component{ID: "sbom:test@test-artifact"}}, // artifact -> sbom
403-
{ComponentID: utils.Ptr("sbom:test@test-artifact"), DependencyID: "pkg:npm/root-dep@1.0.0", Dependency: models.Component{ID: "pkg:npm/root-dep@1.0.0"}}, // sbom -> root-dep (component)
404-
{ComponentID: utils.Ptr("pkg:npm/root-dep@1.0.0"), DependencyID: "pkg:npm/test-package@1.0.0", Dependency: models.Component{ID: "pkg:npm/test-package@1.0.0"}}, // root-dep -> test-package (component)
401+
{ComponentID: "ROOT", DependencyID: "artifact:test-artifact", Dependency: models.Component{ID: "artifact:test-artifact"}}, // root --> artifact
402+
{ComponentID: "artifact:test-artifact", DependencyID: "sbom:test@test-artifact", Dependency: models.Component{ID: "sbom:test@test-artifact"}}, // artifact -> sbom
403+
{ComponentID: "sbom:test@test-artifact", DependencyID: "pkg:npm/root-dep@1.0.0", Dependency: models.Component{ID: "pkg:npm/root-dep@1.0.0"}}, // sbom -> root-dep (component)
404+
{ComponentID: "pkg:npm/root-dep@1.0.0", DependencyID: "pkg:npm/test-package@1.0.0", Dependency: models.Component{ID: "pkg:npm/test-package@1.0.0"}}, // root-dep -> test-package (component)
405405
}
406406
componentRepository := mocks.NewComponentRepository(t)
407407
componentRepository.On("LoadComponents", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(components, nil)
@@ -423,10 +423,10 @@ func TestRenderPathToComponent(t *testing.T) {
423423
t.Run("should escape @ symbols", func(t *testing.T) {
424424
// Create a chain of actual components to verify @ escaping in mermaid output
425425
components := []models.ComponentDependency{
426-
{ComponentID: nil, DependencyID: "artifact:test-artifact", Dependency: models.Component{ID: "artifact:test-artifact"}}, // root --> artifact
427-
{ComponentID: utils.Ptr("artifact:test-artifact"), DependencyID: "sbom:test@test-artifact", Dependency: models.Component{ID: "sbom:test@test-artifact"}}, // artifact -> sbom
428-
{ComponentID: utils.Ptr("sbom:test@test-artifact"), DependencyID: "pkg:npm/root-dep@1.0.0", Dependency: models.Component{ID: "pkg:npm/root-dep@1.0.0"}}, // sbom -> root-dep (component)
429-
{ComponentID: utils.Ptr("pkg:npm/root-dep@1.0.0"), DependencyID: "pkg:npm/test-package@1.0.0", Dependency: models.Component{ID: "pkg:npm/test-package@1.0.0"}}, // root-dep -> test-package (component)
426+
{ComponentID: "ROOT", DependencyID: "artifact:test-artifact", Dependency: models.Component{ID: "artifact:test-artifact"}}, // root --> artifact
427+
{ComponentID: "artifact:test-artifact", DependencyID: "sbom:test@test-artifact", Dependency: models.Component{ID: "sbom:test@test-artifact"}}, // artifact -> sbom
428+
{ComponentID: "sbom:test@test-artifact", DependencyID: "pkg:npm/root-dep@1.0.0", Dependency: models.Component{ID: "pkg:npm/root-dep@1.0.0"}}, // sbom -> root-dep (component)
429+
{ComponentID: "pkg:npm/root-dep@1.0.0", DependencyID: "pkg:npm/test-package@1.0.0", Dependency: models.Component{ID: "pkg:npm/test-package@1.0.0"}}, // root-dep -> test-package (component)
430430
}
431431
componentRepository := mocks.NewComponentRepository(t)
432432
componentRepository.On("LoadComponents", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(components, nil)
@@ -452,9 +452,9 @@ func TestRenderPathToComponent(t *testing.T) {
452452
// The graph requires an artifact and sbom info-source node above the component
453453
// so that FindAllComponentOnlyPathsToPURL can terminate the BFS correctly.
454454
components := []models.ComponentDependency{
455-
{ComponentID: nil, DependencyID: "artifact:test-artifact", Dependency: models.Component{ID: "artifact:test-artifact"}},
456-
{ComponentID: utils.Ptr("artifact:test-artifact"), DependencyID: "sbom:test@test-artifact", Dependency: models.Component{ID: "sbom:test@test-artifact"}},
457-
{ComponentID: utils.Ptr("sbom:test@test-artifact"), DependencyID: "pkg:npm/single@1.0.0", Dependency: models.Component{ID: "pkg:npm/single@1.0.0"}},
455+
{ComponentID: "ROOT", DependencyID: "artifact:test-artifact", Dependency: models.Component{ID: "artifact:test-artifact"}},
456+
{ComponentID: "artifact:test-artifact", DependencyID: "sbom:test@test-artifact", Dependency: models.Component{ID: "sbom:test@test-artifact"}},
457+
{ComponentID: "sbom:test@test-artifact", DependencyID: "pkg:npm/single@1.0.0", Dependency: models.Component{ID: "pkg:npm/single@1.0.0"}},
458458
}
459459
componentRepository := mocks.NewComponentRepository(t)
460460
componentRepository.On("LoadComponents", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(components, nil)
@@ -711,12 +711,12 @@ func TestTicketContentBitwiseReproducibility(t *testing.T) {
711711
// Previously, map iteration randomness caused the two path edges to appear
712712
// in non-deterministic order in the Mermaid output.
713713
components := []models.ComponentDependency{
714-
{ComponentID: nil, DependencyID: "artifact:art", Dependency: models.Component{ID: "artifact:art"}},
715-
{ComponentID: utils.Ptr("artifact:art"), DependencyID: "sbom:s@art", Dependency: models.Component{ID: "sbom:s@art"}},
716-
{ComponentID: utils.Ptr("sbom:s@art"), DependencyID: "pkg:npm/route-b@1.0", Dependency: models.Component{ID: "pkg:npm/route-b@1.0"}},
717-
{ComponentID: utils.Ptr("sbom:s@art"), DependencyID: "pkg:npm/route-a@1.0", Dependency: models.Component{ID: "pkg:npm/route-a@1.0"}},
718-
{ComponentID: utils.Ptr("pkg:npm/route-a@1.0"), DependencyID: "pkg:npm/target@1.0", Dependency: models.Component{ID: "pkg:npm/target@1.0"}},
719-
{ComponentID: utils.Ptr("pkg:npm/route-b@1.0"), DependencyID: "pkg:npm/target@1.0", Dependency: models.Component{ID: "pkg:npm/target@1.0"}},
714+
{ComponentID: "ROOT", DependencyID: "artifact:art", Dependency: models.Component{ID: "artifact:art"}},
715+
{ComponentID: "artifact:art", DependencyID: "sbom:s@art", Dependency: models.Component{ID: "sbom:s@art"}},
716+
{ComponentID: "sbom:s@art", DependencyID: "pkg:npm/route-b@1.0", Dependency: models.Component{ID: "pkg:npm/route-b@1.0"}},
717+
{ComponentID: "sbom:s@art", DependencyID: "pkg:npm/route-a@1.0", Dependency: models.Component{ID: "pkg:npm/route-a@1.0"}},
718+
{ComponentID: "pkg:npm/route-a@1.0", DependencyID: "pkg:npm/target@1.0", Dependency: models.Component{ID: "pkg:npm/target@1.0"}},
719+
{ComponentID: "pkg:npm/route-b@1.0", DependencyID: "pkg:npm/target@1.0", Dependency: models.Component{ID: "pkg:npm/target@1.0"}},
720720
}
721721

722722
assetID := uuid.New()

‎mocks/mock_GraphComponent.go‎

Lines changed: 6 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎normalize/sbom_graph.go‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1831,7 +1831,7 @@ func ParseGraphNodeID(id string) (prefix, name string) {
18311831
// GraphComponent represents a component that can be loaded from the database.
18321832
type GraphComponent interface {
18331833
GetID() string
1834-
GetDependentID() *string
1834+
GetDependentID() string
18351835
ToCdxComponent(componentLicenseOverwrites map[string]string) (cdx.Component, error)
18361836
}
18371837

@@ -1845,10 +1845,8 @@ func SBOMGraphFromComponents[T GraphComponent](components []T, licenseOverwrites
18451845
// Build dependency map: parent -> children
18461846
dependencyMap := make(map[string][]string, len(components))
18471847
for _, c := range components {
1848-
var parentID string
1849-
if c.GetDependentID() != nil {
1850-
parentID = *c.GetDependentID()
1851-
} else {
1848+
parentID := c.GetDependentID()
1849+
if parentID == "ROOT" {
18521850
parentID = GraphRootNodeID
18531851
}
18541852
dependencyMap[parentID] = append(dependencyMap[parentID], c.GetID())

‎tests/component_service_integration_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ func TestGetAndSaveLicenseInformation(t *testing.T) {
9797
err = f.DB.Create(&models.ComponentDependency{
9898
AssetVersionName: assetVersion.Name,
9999
AssetID: assetVersion.AssetID,
100-
ComponentID: nil,
100+
ComponentID: "ROOT",
101101
DependencyID: artifactRoot,
102102
}).Error
103103
assert.NoError(t, err)
@@ -107,21 +107,21 @@ func TestGetAndSaveLicenseInformation(t *testing.T) {
107107
{
108108
AssetVersionName: assetVersion.Name,
109109
AssetID: assetVersion.AssetID,
110-
ComponentID: &artifactRoot,
110+
ComponentID: artifactRoot,
111111
DependencyID: componentWithInvalidLicense.ID,
112112
Dependency: componentWithInvalidLicense,
113113
},
114114
{
115115
AssetVersionName: assetVersion.Name,
116116
AssetID: assetVersion.AssetID,
117-
ComponentID: &artifactRoot,
117+
ComponentID: artifactRoot,
118118
DependencyID: componentWithValidLicense.ID,
119119
Dependency: componentWithValidLicense,
120120
},
121121
{
122122
AssetVersionName: assetVersion.Name,
123123
AssetID: assetVersion.AssetID,
124-
ComponentID: &artifactRoot,
124+
ComponentID: artifactRoot,
125125
DependencyID: componentWithoutLicense.ID,
126126
Dependency: componentWithoutLicense,
127127
},
@@ -219,7 +219,7 @@ func TestGetAndSaveLicenseInformation(t *testing.T) {
219219
err = f.DB.Create(&models.ComponentDependency{
220220
AssetVersionName: assetVersion.Name,
221221
AssetID: assetVersion.AssetID,
222-
ComponentID: nil,
222+
ComponentID: "ROOT",
223223
DependencyID: artifactRoot,
224224
}).Error
225225
assert.NoError(t, err)
@@ -228,7 +228,7 @@ func TestGetAndSaveLicenseInformation(t *testing.T) {
228228
componentDep := models.ComponentDependency{
229229
AssetVersionName: assetVersion.Name,
230230
AssetID: assetVersion.AssetID,
231-
ComponentID: &artifactRoot,
231+
ComponentID: artifactRoot,
232232
DependencyID: componentWithInvalidLicense.ID,
233233
Dependency: componentWithInvalidLicense,
234234
}

0 commit comments

Comments
 (0)