launchpad-reviewers team mailing list archive

[Enrique Sánchez <mp+494096@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> diff --git a/lib/lp/bugs/scripts/cveimport.py b/lib/lp/bugs/scripts/cveimport.py
> index a1c27d5..e019d07 100644
> --- a/lib/lp/bugs/scripts/cveimport.py
> +++ b/lib/lp/bugs/scripts/cveimport.py
> @@ -723,10 +723,11 @@ class CVEUpdater(LaunchpadCronScript):
>              modified = True
>  
>          # handle affected
> -        metadata = cve.metadata or {}
> -        if metadata.get("affected", {}) != affected:
> -            metadata["affected"] = affected
> -            cve.metadata = metadata
> +        if (
> +            not isinstance(cve.metadata, dict)
> +            or cve.metadata.get("affected", {}) != affected
> +        ):
> +            cve.metadata = {"affected": affected}

It is true that we will be overwriting them, but we want to overwrite.

As I see this, MITRE is the source of truth so let's imagine that we add a new key called `new_key`.

When doing an import we will check if the content of MITRE `affected` is the same that we have in `cve.metadata['affected']`, and we will check the same with the `new_key`.

I any of them has changed, we will do:
```
cve.metadata = {"affected": affected, "new_key": new_key}
```

We don't want to let there data that is not in MITRE and is not for the `affected` or `new_key` keys. MITRE will always have everything we need to populate the entire `metadata`. 

In case that there is other keys there, we don't want them.

I've also thought about this (same) approach (maybe easier to understand), what do you think?

```
# Build metadata dict
metadata = {"affected": affected, "new_key": new_value}

# If anything changed, update cve.metadata
if metadata != cve.metadata:
    cve.metadata = metadata
    modified = True
```

>              modified = True
>  
>          if modified:


-- 
https://code.launchpad.net/~enriqueesanchz/launchpad/+git/launchpad/+merge/494096
Your team Launchpad code reviewers is requested to review the proposed merge of ~enriqueesanchz/launchpad:fix-update-cve into launchpad:master.