feat(engine): manage verified engine lifecycle - #102
Conversation
rjckkkkk
left a comment
rjckkkkk
left a comment
There was a problem hiding this comment.
The lifecycle structure and atomic DB transitions are solid, and the full suite, vet, and targeted race tests pass. I found four lifecycle/import correctness issues that should be addressed before merge.
| func imageContentDigest(id string) string { | ||
| id = strings.TrimSpace(id) | ||
| if strings.HasPrefix(strings.ToLower(id), "sha256:") { | ||
| return id |
There was a problem hiding this comment.
[P1] Do not treat the runtime image ID as the Catalog OCI digest. Docker's docker images --no-trunc ID (and the CRI image id) identifies the image/config object; it is not the repository/manifest digest exposed through RepoDigests. ImportEngine later compares this value with asset.Image.Digest, so a normal imported image whose Catalog digest is a manifest digest will never be promoted to imported/verified and therefore cannot be activated. The existing VerifyImageDigest code already inspects RepoDigests; scanning/import should collect or verify that same evidence instead of passing img.id through as ContentDigest.
| if img.RuntimeType == "native" && img.Origin == "preinstalled" && img.ContentDigest != "" && | ||
| (img.VersionMatch == "exact" || img.VersionMatch == "compatible") { | ||
| entry.LifecycleStatus = "verified" | ||
| entry.VerificationStatus = "verified" |
There was a problem hiding this comment.
[P1] Do not mark a preinstalled executable verified merely because it has a locally computed hash and reports a compatible version. ContentDigest here is just the SHA256 of whatever file scanning found; it is never compared with Catalog trust evidence, and DetectedVersion may even come from probe.fallback_version when no successful version command ran. This turns an arbitrary/tampered binary from AIMA_ENGINE_DIR, PATH, or a probe path into an inactive verified record that engine.ensure may activate and the deployment overlay will execute. Keep it unverified unless its digest is checked against trusted Catalog evidence (or introduce an explicit operator trust action distinct from passive scan).
| continue | ||
| } | ||
| if current != nil { | ||
| return result, fmt.Errorf("engine asset %s has multiple active runtime versions on platform %s", result.AssetName, s.inventoryPlatform) |
There was a problem hiding this comment.
[P2] Make rollback address the same activation scope that the DB permits. The v21 unique index and ActivateEngineVersion scope activity by (asset_name, platform, runtime_type), so one asset may legitimately have both a Native and a container version active (for example after a Catalog update changes the preferred runtime). This API accepts only the asset name and rejects that valid state as ambiguous, leaving neither chain rollbackable. Either enforce one active version per asset/platform across runtimes, or add/select runtime_type in the rollback request and query that exact activation group.
| if err != nil { | ||
| return fmt.Errorf("resolve path %s: %w", path, err) | ||
| } | ||
| if looksLikeNativeEngineBundle(absPath) { |
There was a problem hiding this comment.
[P2] Detect package type by content or fall back to OCI import before returning. This extension gate sends every .tar.gz/.tgz directly to the Native importer; if the file is a gzip-compressed docker save/OCI archive, the native layout check fails and the existing container importer is never attempted. docker load and containerd import accept compressed image archives, so this regresses a common air-gapped container format. The later branch already demonstrates the safer order by trying one importer and then the other.
1d0fb3a to
6038ca1
Compare
2 participants
Summary
Stack
This PR is intentionally stacked on codex/deployment-recovery-clean. Review and merge the deployment recovery PR first; then retarget this PR to develop.
Regression coverage
Validation