ROX-18202: Fix language layer error due to unique constraint #1223
Conversation
connorgorman
changed the title
|
Images are ready for the commit at 7186419. To use the images, use the tag |
|
I'll look into the failure |
|
/retest |
1 similar comment
|
/retest |
| RegisterMigration(migrate.Migration{ | ||
| ID: 20, | ||
| Up: migrate.Queries([]string{ | ||
| `ALTER TABLE LanguageLayer DROP CONSTRAINT IF EXISTS languagelayer_pkey`, |
There was a problem hiding this comment.
is this a generic, default constraint? I don't see it in my deployment:
postgres=# \d languagelayer
Table "public.languagelayer"
Column | Type | Collation | Nullable | Default
--------------------+------------------------+-----------+----------+---------
layer_id | integer | | not null |
layer_name | character varying(128) | | not null |
component_data | bytea | | not null |
removed_components | text[] | | |
lineage | character varying | | |
Indexes:
"languagelayer_layer_name_key" UNIQUE CONSTRAINT, btree (layer_name)
There was a problem hiding this comment.
languagelayer_pkey is the name of the constraint after ALTER TABLE LanguageLayer ADD PRIMARY KEY (layer_name, lineage) is applied, the default name of a primary key looks to be <table name>_pkey
Possibly added to the migration so that it would not cause errors if re-ran.
| ID: 20, | ||
| Up: migrate.Queries([]string{ | ||
| `ALTER TABLE LanguageLayer DROP CONSTRAINT IF EXISTS languagelayer_pkey`, | ||
| `ALTER TABLE LanguageLayer DROP CONSTRAINT IF EXISTS languagelayer_layer_name_key`, |
There was a problem hiding this comment.
so dropping this constraint removes the UNIQUE part of layer_name VARCHAR(128) NOT NULL UNIQUE?
api/v1/models.go
Outdated
| for _, dbFeatureVersion := range dbLayer.Features { | ||
| feature := featureFromDatabaseModel(dbFeatureVersion, opts.GetUncertifiedRHEL(), depMap) | ||
| if withFeatures || withVulnerabilities { | ||
| if dbLayer.Features != nil || namespaces.IsRHELNamespace(layer.NamespaceName) { |
There was a problem hiding this comment.
would need to see if our scanning notes accounts for us still finding language vulns without OS-level vulns. Don't think we considered this possibility before
There was a problem hiding this comment.
In the GetComponents flow used via GRPC looks like the OS-level comps vs. language comps are not dependent - FWIW
|
/retest |
connorgorman
force-pushed
the
cgorman-fix-scanner-language-layer-error
branch
from
b53279b to
8b4408a
Compare
|
/retest |
There was a problem hiding this comment.
LGTM
Cloned and retested per the original findings and this indeed fixes the issue. Thanks @connorgorman !
dcaravel
dismissed
their stale review
Dismissing - need to retest - force push'd changes were not brought into my tests.
|
Yeah i separated them because it was causing other issues and is a larger change |
The issue was laid out in the doc by David. Basically what's happening is that the sample images have layers that are identical so they have the same "layer_name" which is the digest, but the lineage is different. This means that if you have identical layers with vulns that have different lineages (or parent layers) then the second insertion is due to the conflict on the layer_name (insert does on conflict do nothing). This change makes it so that table is keyed on layer_name and lineage, effectively only deduping layers with the exact same parents (which is good because we don't need to reinsert the layers). The unit test should test this case
Note: for some reason, this does not work on scratch, but I find that less pressing because it seems to be when the language vuln is simply in the base layerThis is fixed by always evaluating language vulns even if there are no db features