feat(pack): add optional OCI subject linkage for adapters#1133
feat(pack): add optional OCI subject linkage for adapters#1133rishi-jat wants to merge 1 commit intokitops-ml:mainfrom
Conversation
Signed-off-by: Rishi Jat <rishijat098@gmail.com>
rishi-jat
force-pushed
the
feat/oci-subject-linkage
branch
from
c8281f2 to
3287e13
Compare
There was a problem hiding this comment.
Pull request overview
Adds optional OCI subject linkage when packing adapter modelkits by attempting to resolve the base model’s manifest descriptor (local-first, remote fallback) and wiring it into the generated manifest.
Changes:
- Extend
filesystem.SaveModelOptionsto accept an optional OCISubjectdescriptor and apply it to the manifest. - During
kit pack, attempt to resolve the base model’s manifest descriptor and pass it through as the subject (best-effort; warnings only).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/lib/filesystem/local-storage.go | Adds an optional Subject field to SaveModelOptions and sets manifest.Subject when provided. |
| pkg/cmd/pack/pack.go | Resolves a base manifest descriptor (local-first, remote fallback) and passes it into SaveModel to enable OCI subject linkage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| parsedDigest, err := digest.Parse(baseRef) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to parse digest-only base reference: %w", err) | ||
| } | ||
| return &ocispec.Descriptor{MediaType: ocispec.MediaTypeImageManifest, Digest: parsedDigest}, nil |
There was a problem hiding this comment.
In the digest-only baseRef case ("sha256:..."), this returns an OCI descriptor without a valid Size (defaults to 0). OCI descriptors require an accurate size, and emitting a subject with size 0 can break interoperability/validation. Consider skipping subject linkage for digest-only references (return nil) or resolving the full descriptor (digest+size) from an actual repository/tag/@digest reference instead of constructing it manually.
| parsedDigest, err := digest.Parse(baseRef) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to parse digest-only base reference: %w", err) | |
| } | |
| return &ocispec.Descriptor{MediaType: ocispec.MediaTypeImageManifest, Digest: parsedDigest}, nil | |
| _, err := digest.Parse(baseRef) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to parse digest-only base reference: %w", err) | |
| } | |
| // For digest-only references, we cannot reliably construct a complete OCI descriptor | |
| // (including accurate Size), so we skip subject linkage to avoid emitting an invalid descriptor. | |
| output.Logf(output.LogLevelWarn, | |
| "base reference %q is a digest-only reference; skipping subject linkage", baseRef) | |
| return nil, nil |
| desc, _, _, err := util.ResolveManifestAndConfig(ctx, localRepo, ref.Reference) | ||
| if err == nil { | ||
| return &desc, nil | ||
| } |
There was a problem hiding this comment.
resolveBaseManifestDescriptor uses util.ResolveManifestAndConfig, which fetches both the manifest and Kitfile config blob. Since pack() already called ResolveKitfile (which fetches the base Kitfile) just above, this adds redundant local/remote reads and network round-trips. Consider resolving only the manifest descriptor (e.g., util.ResolveManifest / store.Resolve) to get digest+size for the OCI subject, and rely on the prior ResolveKitfile result for modelkit validation.
Adds optional OCI subject linkage during pack.
Part of #899