launchpad-reviewers team mailing list archive

[Ines Almeida <mp+487018@xxxxxxxxxxxxxxxxxx>]

Left a few very minor comments, looks good to me overall!

About the Enum classes within the SOSSRecord class:
I usually don't love nested classes, and perhaps eventually we will want to add other sorts of non-SOSS records that will use the same sort of Priority, Package, etc, and we might want to re-use them - if so leaving it in a common place makes sense.

That being said, I really like how tidy this looks - it represents exactly a SOSSRecord with all that's nested within it, and I don't need to go and find the definition anything anywhere else. So IMO there is no need to overcomplicate.

Also, this all seems to follow the pattern in UCT (but cleaner with the dataclass, which is a nice touch IMO).

So I'm happy with this approach

Diff comments:

> diff --git a/lib/lp/bugs/scripts/soss/sossrecord.py b/lib/lp/bugs/scripts/soss/sossrecord.py
> new file mode 100644
> index 0000000..466606b
> --- /dev/null
> +++ b/lib/lp/bugs/scripts/soss/sossrecord.py

I would consider naming this files `models.py` to keep consistency with the uct scripts and overall within the repo.

> @@ -0,0 +1,139 @@
> +#  Copyright 2025 Canonical Ltd.  This software is licensed under the
> +#  GNU Affero General Public License version 3 (see the file LICENSE).
> +import re
> +from dataclasses import dataclass
> +from datetime import datetime
> +from enum import Enum
> +from typing import Any, Dict, List, Optional
> +
> +import yaml

I'd add a
```
__all__ = [
    "SOSSRecord",
]
```
to keep consistency of what we do in other files.

> +
> +# From `soss-cve-tracker/git-hooks/check-cve-syntax`
> +VALID_CHANNEL_REGEX = re.compile(r"^(focal|jammy|noble):[^/]+/stable$")
> +
> +
> +@dataclass
> +class SOSSRecord:
> +
> +    class PriorityEnum(Enum):
> +        NEEDS_TRIAGE = "Needs-triage"
> +        NEGLIGIBLE = "Negligible"
> +        LOW = "Low"
> +        MEDIUM = "Medium"
> +        HIGH = "High"
> +        CRITICAL = "Critical"
> +
> +    class PackageStatusEnum(Enum):
> +        IGNORED = "ignored"
> +        NEEDS_TRIAGE = "needs-triage"
> +        RELEASED = "released"
> +        NOT_AFFECTED = "not-affected"
> +        DEFERRED = "deferred"
> +        NEEDED = "needed"
> +
> +    class PackageTypeEnum(Enum):
> +        CONDA = "conda"
> +        PYTHON = "python"
> +        UNPACKAGED = "unpackaged"
> +        MAVEN = "maven"
> +        RUST = "rust"
> +
> +    @dataclass
> +    class Channel:
> +        value: str
> +
> +        def __post_init__(self):
> +            if not VALID_CHANNEL_REGEX.match(self.value):
> +                raise ValueError(f"Invalid channel format: {self.value}")
> +
> +    @dataclass
> +    class CVSS:
> +        source: str
> +        vector: str
> +        base_score: float
> +        base_severity: float
> +
> +        BASE_SEVERITIES = {"LOW", "MEDIUM", "HIGH", "CRITICAL"}
> +
> +        def __post_init__(self):
> +            if not (0.0 <= self.base_score <= 10.0):
> +                raise ValueError(f"Invalid base score: {self.base_score}")
> +            if self.base_severity not in self.BASE_SEVERITIES:
> +                raise ValueError(
> +                    f"Invalid base severity: {self.base_severity}"
> +                )
> +
> +    @dataclass
> +    class Package:
> +        name: str
> +        channel: "SOSSRecord.Channel"
> +        repositories: List[str]
> +        status: "SOSSRecord.PackageStatusEnum"
> +        note: str
> +
> +    references: List[str]
> +    notes: List[str]
> +    priority: PriorityEnum
> +    priority_reason: str
> +    assigned_to: str
> +    packages: Dict[PackageTypeEnum, List[Package]]
> +    candidate: Optional[str] = None
> +    description: Optional[str] = None
> +    cvss: Optional[List[CVSS]] = None
> +    public_date: Optional[datetime] = None
> +
> +    @classmethod
> +    def from_yaml(cls, yaml_str: str) -> "SOSSRecord":
> +        raw: Dict[str, Any] = yaml.safe_load(yaml_str)
> +        return cls.from_dict(raw)
> +
> +    @classmethod
> +    def from_dict(cls, raw: Dict[str, Any]) -> "SOSSRecord":
> +        priority = SOSSRecord.PriorityEnum(raw["Priority"])

I see some fields use `get` and others (like Piority) don't. I'm guessing some are mandatory and others can be empty or non existant?

> +
> +        packages = {}
> +        for enum_key, pkgs in raw.get("Packages", {}).items():
> +            package_type = SOSSRecord.PackageTypeEnum(enum_key.lower())
> +            package_list = [
> +                SOSSRecord.Package(
> +                    name=package["Name"],
> +                    channel=SOSSRecord.Channel(package["Channel"]),
> +                    repositories=package["Repositories"],
> +                    status=SOSSRecord.PackageStatusEnum(
> +                        package["Status"].lower()
> +                    ),
> +                    note=package["Note"],
> +                )
> +                for package in pkgs
> +            ]
> +            packages[package_type] = package_list
> +
> +        cvss_list = [
> +            SOSSRecord.CVSS(
> +                cvss["source"],
> +                cvss["vector"],
> +                cvss["baseScore"],
> +                cvss["baseSeverity"],
> +            )
> +            for cvss in raw.get("CVSS", [])
> +        ]
> +
> +        public_date_str = raw.get("PublicDate")
> +        public_date = (
> +            datetime.fromisoformat(public_date_str)
> +            if public_date_str
> +            else None
> +        )
> +
> +        return cls(
> +            references=raw.get("References", []),
> +            notes=raw.get("Notes", []),
> +            priority=priority,
> +            priority_reason=raw.get("Priority-Reason", ""),
> +            assigned_to=raw.get("Assigned-To", ""),
> +            packages=packages,
> +            candidate=raw.get("Candidate"),
> +            description=raw.get("Description"),
> +            cvss=cvss_list,
> +            public_date=public_date,
> +        )


-- 
https://code.launchpad.net/~enriqueesanchz/launchpad/+git/launchpad/+merge/487018
Your team Launchpad code reviewers is requested to review the proposed merge of ~enriqueesanchz/launchpad:soss-import-parsing into launchpad:master.