Skip to content

feat(image): validate ref shape against ImageType before auto-pull#40

Merged
CMGS merged 2 commits into
masterfrom
feat-ensure-image-ref-shape-guard
May 11, 2026
Merged

feat(image): validate ref shape against ImageType before auto-pull#40
CMGS merged 2 commits into
masterfrom
feat-ensure-image-ref-shape-guard

Conversation

@CMGS
Copy link
Copy Markdown
Contributor

@CMGS CMGS commented May 11, 2026

Summary

EnsureImage drives pulls based on vmCfg.ImageType but never validated that vmCfg.Image actually has the shape the chosen backend can handle. Mismatch surfaces deep:

  • A cloudimg backend with a bare OCI ref (`simular/win10:22h2-20260510` — the issue 38 failure mode) ran all the way down to `http.Get()` and returned `unsupported protocol scheme ""`.
  • An OCI backend with a malformed ref reported a similarly downstream-looking error from `go-containerregistry`.

This PR adds `validateRefShape(ref, imageType)` ahead of `Pull`:

  • `cloudimg` → must be a URL with scheme (`http`/`https`); bare refs or local names are imported images that auto-pull can't materialize anyway
  • `oci` → must be parseable by `name.ParseReference`

Malformed refs short-circuit with an actionable warning instead of a wasted HTTP round-trip.

Scope

This is a defensive guard, not the issue 38 fix proper. The real issue 38 fix lives on the vk-cocoon side: stop writing bare OCI refs into `cocoonstack.snapshot.baseimage` annotations for cloudimg bases. Once vk-cocoon writes fully-qualified `/dl/{name}/{ref}` URLs (which the cocoonstack/epoch#4 routes now support), this guard becomes a no-op on the happy path — it only fires when something has gone wrong upstream.

Two complementary benefits independent of issue 38:

  1. Better error messages: `cocoon vm clone` against mis-annotated snapshots fails fast with "is not an http(s) URL (imported or bare OCI ref?)" instead of "unsupported protocol scheme".
  2. Cross-registry parity: `validateRefShape` doesn't assume epoch — the URL check just requires a scheme. Snapshots referencing arbitrary cloudimg sources (`https://cloud-images.ubuntu.com/...\`, internal mirrors) all pass; only truly malformed refs are rejected.

Test plan

  • New unit test `TestEnsureImage_SkipsBadShape` covers three malformed combinations:
    • cloudimg + bare OCI ref → skipped
    • cloudimg + local image name → skipped
    • OCI + unparseable ref → skipped
  • `go test ./...` clean
  • `go build ./...` clean
  • Existing `TestEnsureImage_ForceWhenDigestPinned` + `TestEnsureImage_SkipsPullWhenDigestLocal` unchanged

Followups

  • vk-cocoon side fix for issue 38: write `baseimage` annotation as fully-qualified URL with explicit ref (tag-pinned or manifest-digest-pinned form)

EnsureImage drives pulls based on vmCfg.ImageType, but never checked that
vmCfg.Image actually has the shape the chosen backend can handle. A
cloudimg backend with a bare OCI ref (`simular/win10:22h2-20260510`) ran
all the way down to http.Get(), which returned the cryptic `unsupported
protocol scheme ""` — the issue 38 failure mode.

Add validateRefShape() ahead of Pull:

- cloudimg: must be a URL with scheme (http/https); bare refs or local
  names are imported images that auto-pull can't materialize anyway
- oci: must be parseable by name.ParseReference

Malformed refs trigger a clear warn-and-return instead of a wasted HTTP
round-trip with a misleading error.

This is a defensive guard, not the issue 38 fix proper — vk-cocoon still
needs to stop writing bare OCI refs into `cocoonstack.snapshot.baseimage`
annotations for cloudimg bases. But the guard makes mis-annotated
snapshots fail fast with an actionable signal at the cocoon CLI layer.
/code pass on the validateRefShape change:

- helpers.go: drop the standalone url.Parse(ref) + Scheme!="" check in
  favor of the existing IsURL helper. url.Parse accepts non-http schemes
  (file://, git://) that fail at cloudimg.Pull anyway; IsURL restricts
  to http(s):// directly, matching the comment and the cloudimg backend's
  actual contract. Also drops the net/url import.

- ensure_image_test.go: add positive-case TestEnsureImage_AcceptsGoodShape
  to lock in that well-formed cloudimg URLs and tagged OCI refs still
  reach Pull; add a "cloudimg non-http scheme" (file://) reject case to
  cover the url.Parse-passes-but-shouldn't trap.

- Drop "issue 38" references from godoc and test comment per "don't
  reference current task/fix" rule; the surrounding behavior stands on
  its own.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an early “ref shape” validation guard in EnsureImage so auto-pull decisions fail fast with a clear warning when vmCfg.ImageType and vmCfg.Image are incompatible (e.g., cloudimg backend receiving a bare OCI-style ref), avoiding misleading downstream errors.

Changes:

  • Validate base-image reference shape (cloudimg must be http(s) URL; oci must parse via name.ParseReference) before calling Pull.
  • Warn and skip auto-pull on malformed/mismatched refs to prevent wasted network work and confusing errors.
  • Add unit tests covering both rejected malformed shapes and accepted well-formed shapes.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
cmd/core/helpers.go Adds validateRefShape and invokes it in EnsureImage before auto-pull.
cmd/core/ensure_image_test.go Adds tests asserting bad ref shapes skip pull and good shapes reach pull.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@CMGS CMGS merged commit 92329f9 into master May 11, 2026
10 checks passed
@CMGS CMGS deleted the feat-ensure-image-ref-shape-guard branch May 11, 2026 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants