fix(types): allow ':' and '/' in snapshot names for OCI tag-aware refs#65
Merged
Conversation
7a490c0 to
f99340e
Compare
CMGS
reviewed
May 26, 2026
CMGS
reviewed
May 26, 2026
The validName regex added in #61 (`^[a-zA-Z0-9][a-zA-Z0-9._-]{0,62}$`) rejects vk-cocoon's tag-aware local-snapshot naming (`repo:tag`, e.g. `simular/ubuntu-hot-testing:v1`), breaking PullSnapshot for any cocoon VM create that goes through vk-cocoon. The cocoon subprocess exits with name validation error before reading stdin; vk-cocoon then writes the blob into a closed pipe and surfaces the symptom as "stream snapshot: ... broken pipe" with no visible upstream hint (its `command()` discards subprocess stderr). Split: keep validName strict for VMConfig (hostname / DNS-1123 / cidata constraints apply) and introduce validSnapshotName that additionally allows ':' and '/' for SnapshotConfig. Snapshot names are local cocoon DB keys and never propagate to hostname / DNS / cidata, so the strict charset isn't load-bearing there. Shell-unsafe chars and leading non-alnum remain rejected.
f99340e to
8f0a19a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
types.SnapshotConfig.Validate()rejects names containing:or/, which breaks vk-cocoon's tag-aware local-snapshot naming (repo:tag, e.g.simular/ubuntu-hot-testing:v1).validNamestrict forVMConfig(hostname / DNS-1123 / cidata constraints apply), introducevalidSnapshotNameallowing:and/forSnapshotConfig.Failure mode this fixes
Real downstream symptom in vk-cocoon's
PullSnapshot(vk-cocoon/snapshots/puller.go):What actually happens on a
cocoon snapshot import --name simular/ubuntu-hot-testing:v1(vk-cocoon's exec):The cocoon subprocess hits validation before reading stdin → exits non-zero → vk-cocoon writes the blob into a closed pipe → EPIPE bubbles up as "broken pipe". The real validation error is invisible to upstream because vk-cocoon's
command()doesn't capture subprocess stderr — that's a separate vk-cocoon bug worth a follow-up.Affects every cocoon VM create that goes through vk-cocoon
PullSnapshot, not just Linux — the only reason Windows wasn't flagged yet is the existing CocoonSets all predate the cocoon binary rebuild that picked up #61.Why the split (vs. keeping a single regex)
#61's rationale — LinuxHOST_NAME_MAX=64, DNS-1123 labels, cidata YAML — applies to VM names, which propagate into the guest hostname, network identity, and cidata. Snapshot names are local cocoon DB keys: they are matched inlocalfileby string equality, never written into a filesystem path (data dirs key off the generated ID, not the name), and never reach hostname / DNS / cidata. So the strict charset is correct forVMConfig.Nameand over-restrictive forSnapshotConfig.Name. The split is intentional rather than relaxingvalidNameglobally.Test plan
go test ./types/...— passes (15 cases inTestSnapshotConfig_Validate, including newsimular/ubuntu-hot-testing:v1,repo:tag, leading:/ leading/, semicolon, backtick).go vet ./...— clean.go build ./...— clean.make lint— not run locally (golangci-lintnot installed on this host); expecting CI to vet.Follow-ups (not in this PR)
vm/cocoon_cli.go:command()should capturecmd.Stderrso futurecocoonsubprocess failures surface the real error instead of an upstream EPIPE symptom. Would have made this bug a 5-minute fix instead of an afternoon.