Skip to content

Commit 525faab

Browse files
im-tylerclaude
andcommitted
fix(deploy): take the version from the image being deployed, not from git HEAD
Observed in production: `teploy deploy --image .../fylun-web:462d7a7` printed "Deployed fylun-web version fe82bf0", named the containers `fylun-web-web-fe82bf0`, and wrote fe82bf0 into `teploy log` — while running the image tagged 462d7a7. fe82bf0 was simply the local git HEAD at that moment. During a rollback that is worse than cosmetic. `teploy log` is where an operator picks a target, and every version in it named a commit that had no relationship to the image that ran. Cause, in deployAppConfig: gitShortHash() was called FIRST and unconditionally, with the image consulted only in its *error* branch. So inside any git repo the version was always HEAD and the image tag was never read. And because step 3 resolves `image = appCfg.Image` beforehand, a teploy.yml pinning a prebuilt image hit the same path with no flag passed — this was never about --image. versionFromImage derives the label from the reference. A digest wins over an accompanying tag, because Docker resolves the digest and ignores the tag, so labelling `repo:1.2.3@sha256:...` as 1.2.3 would be the same class of lie. The tag is read only when the last colon falls after the last slash, or a registry port (100.108.123.49:49152/tyler/app) parses as a tag. Output is gated on Docker's tag grammar, not merely parsed. Versions are interpolated UNQUOTED into remote shell commands through ContainerName, and until now a version could only originate from git output or an operator's own --version; an image reference is a less-trusted source and is treated as one. ":latest" and untagged references are refused, falling back to the existing timestamp. Reusing a floating tag as the version gives every deploy the same container name and leaves CurrentHash == PreviousHash, which disables rollback outright. A timestamp is at least unique. This is a behaviour change for build-with-pinned-yml-image deploys, documented in the CHANGELOG. plan.go carried a comment promising it resolved the version "exactly as deploy does" and no longer did; left alone it would predict container names the deploy never creates. Now shares the same rule. LogEntry gains `image`, so the log can be checked against what ran instead of being taken on faith. AppState already carried ImageRef and ImageDigest; the log was the gap. Checked the four places a version string travels: container naming (needs the grammar gate above), Caddy routing (name-only, unaffected), rollback target resolution (matches the teploy.version label — opaque strings are fine, but a NON-UNIQUE version would break it, which is why :latest is refused), and keep_versions pruning (groups by the same label, unaffected). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CfMxUBzUH4UP3reJ71j59K
1 parent 9d669a5 commit 525faab

5 files changed

Lines changed: 209 additions & 13 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,36 @@ All notable changes to teploy are documented here. Format follows [Keep a Change
44

55
## [Unreleased]
66

7+
### Fixed
8+
9+
- `deploy` labelled the deploy with the local git HEAD even when deploying a
10+
prebuilt image, so the container name, `teploy log` and the operator's screen
11+
all named a commit whose code was not running. Observed as "Deployed version
12+
fe82bf0" while running the image tagged `462d7a7`. The version now comes from
13+
the image when one is given; `--version` still wins over everything.
14+
15+
This was never specific to the `--image` flag — a `teploy.yml` pinning a
16+
prebuilt image hit the identical problem with no flag passed.
17+
18+
**Behaviour change:** with an image that carries no usable version (untagged,
19+
or `:latest`), `deploy` now labels with a timestamp rather than the git hash.
20+
A git hash there asserts a commit that may not be what `:latest` resolves to;
21+
a timestamp asserts nothing except uniqueness, which is the honest answer when
22+
the reference genuinely does not identify a build. `:latest` is refused
23+
outright as a version: reused, it gives every deploy the same container name
24+
and leaves `CurrentHash == PreviousHash`, disabling rollback.
25+
26+
- `plan` resolved the target version by the old rule, so it predicted container
27+
names `deploy` would not create. It now uses the same resolution.
28+
29+
### Added
30+
31+
- `LogEntry.image`, so `teploy log` records the image a deploy actually ran.
32+
The version alone could not be checked against anything, and the log is
33+
precisely where a rollback target gets chosen. Omitted for entries with no
34+
image (heal, lifecycle, static); old log lines parse unchanged.
35+
36+
737
### Added
838
- `teploy lock status` — report the current deploy lock without touching it.
939
Locks were writable from both ends (`deploy` takes an auto lock, `lock` takes

‎internal/cli/deploy.go‎

Lines changed: 69 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"os"
1111
"os/exec"
1212
"os/signal"
13+
"regexp"
1314
"strings"
1415
"time"
1516

@@ -305,13 +306,24 @@ func deployAppConfig(flags *Flags, appCfg *config.AppConfig, serverName, image,
305306
}
306307

307308
// 4. Resolve version.
309+
//
310+
// Order matters. An explicit --version always wins. Otherwise, when a
311+
// prebuilt image is being deployed, the version comes from THE IMAGE, not
312+
// from git: the image is what actually runs, and git HEAD is merely
313+
// whatever the operator's working directory happened to be on. Only a
314+
// build-from-source deploy falls through to the git hash, which is correct
315+
// there because the source and the artifact are the same thing.
308316
if version == "" {
309-
version, err = gitShortHash()
310-
if err != nil {
311-
if image != "" {
312-
// Pre-built image with no git repo — use a timestamp.
317+
if image != "" {
318+
version = versionFromImage(image)
319+
if version == "" {
320+
// A floating or digest-less reference carries no usable
321+
// version; a timestamp is at least unique per deploy.
313322
version = fmt.Sprintf("%d", time.Now().Unix())
314-
} else {
323+
}
324+
} else {
325+
version, err = gitShortHash()
326+
if err != nil {
315327
return fmt.Errorf("could not determine version from git: %w (use --version flag)", err)
316328
}
317329
}
@@ -645,6 +657,58 @@ func deployBuiltImage(ctx context.Context, executor ssh.Executor, appCfg *config
645657
return nil
646658
}
647659

660+
// imageTagPattern is Docker's tag grammar. Version strings are interpolated
661+
// UNQUOTED into remote shell commands via ContainerName (docker rename, docker
662+
// rm -f), and until now a version could only come from git output or an
663+
// operator's own --version. Deriving it from an image reference introduces a
664+
// less-trusted source, so it is gated here rather than trusted.
665+
var imageTagPattern = regexp.MustCompile(`^[A-Za-z0-9_][A-Za-z0-9._-]{0,127}$`)
666+
667+
// versionFromImage derives a deploy version from an image reference, or ""
668+
// when the reference carries no meaningful one.
669+
//
670+
// Why this exists: `teploy deploy --image repo/app:462d7a7` used to label the
671+
// deploy with the local git HEAD instead — so the container name, `teploy log`
672+
// and the operator's screen all asserted a commit whose code was not running.
673+
// Observed in production as "Deployed version fe82bf0" while running the image
674+
// tagged 462d7a7. During a rollback that is worse than cosmetic: the version
675+
// someone picks out of the log never corresponded to that image.
676+
//
677+
// Note this is NOT specific to the --image flag. The caller resolves
678+
// `image = appCfg.Image` first, so a teploy.yml pinning a prebuilt image hit
679+
// the identical problem with no flag passed.
680+
func versionFromImage(image string) string {
681+
// A digest pins exact content, so it is the most truthful label available,
682+
// and it wins over any tag beside it: Docker resolves the digest and
683+
// ignores the tag, so labelling `repo:1.2.3@sha256:...` as 1.2.3 would be
684+
// the same class of lie this function exists to remove.
685+
if _, digest, ok := strings.Cut(image, "@"); ok {
686+
const prefix = "sha256:"
687+
if strings.HasPrefix(digest, prefix) && len(digest) == len(prefix)+64 {
688+
// Colons are invalid in a container name, so the prefix is dashed.
689+
return "sha256-" + digest[len(prefix):len(prefix)+12]
690+
}
691+
return ""
692+
}
693+
694+
// The tag follows the last ':', but only when that colon comes after the
695+
// last '/' — otherwise a registry port (100.108.123.49:49152/tyler/app)
696+
// parses as the tag.
697+
tag := ""
698+
if i := strings.LastIndex(image, ":"); i > strings.LastIndex(image, "/") {
699+
tag = image[i+1:]
700+
}
701+
702+
// "" and "latest" are refused deliberately. A floating tag reused as the
703+
// version gives every deploy the same container name AND leaves
704+
// CurrentHash == PreviousHash, which disables rollback entirely. The
705+
// caller's timestamp fallback is at least unique per deploy.
706+
if tag == "" || tag == "latest" || !imageTagPattern.MatchString(tag) {
707+
return ""
708+
}
709+
return tag
710+
}
711+
648712
// runMultiDeploy handles deploying to multiple servers in parallel.
649713
func runMultiDeploy(flags *Flags, appCfg *config.AppConfig, image, version string, skipDNSCheck bool, parallel int, migrateVolumes bool) error {
650714
// Resolve parallel setting.

‎internal/cli/plan.go‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -128,18 +128,23 @@ func runPlan(flags *Flags, version string) error {
128128
return planStatic(ctx, flags, appCfg, executor)
129129
}
130130

131-
// Resolve the target version exactly as `deploy` does: --version wins, else
132-
// the local git short hash. When neither is available (no git repo) but a
133-
// pre-built image is configured, deploy falls back to a timestamp — a
134-
// non-deterministic version — so we fall back to an indicative diff.
131+
// Resolve the target version exactly as `deploy` does — and it has to stay
132+
// exactly, or plan predicts container names the deploy will not create.
133+
// --version wins; else a prebuilt image supplies it; else the git hash.
134+
// A floating tag (:latest, untagged) yields a timestamp at deploy time,
135+
// which is non-deterministic, so the diff can only be indicative.
135136
versionKnown := true
136137
if version == "" {
137-
version, err = gitShortHash()
138-
if err != nil {
139-
if appCfg.Image == "" {
138+
if appCfg.Image != "" {
139+
version = versionFromImage(appCfg.Image)
140+
if version == "" {
141+
versionKnown = false
142+
}
143+
} else {
144+
version, err = gitShortHash()
145+
if err != nil {
140146
return fmt.Errorf("could not determine target version from git: %w (pass --version)", err)
141147
}
142-
versionKnown = false
143148
}
144149
}
145150

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
package cli
2+
3+
import "testing"
4+
5+
// The bug this guards: `teploy deploy --image repo/app:462d7a7` labelled the
6+
// deploy with the local git HEAD (fe82bf0), so the container name, the deploy
7+
// log and the operator's screen all named a commit whose code was not running.
8+
func TestVersionFromImage(t *testing.T) {
9+
cases := []struct {
10+
name string
11+
image string
12+
want string
13+
}{
14+
{
15+
// The production case. The registry port contains a colon, so a
16+
// naive "text after the last colon" split returns the wrong thing
17+
// for a registry-qualified reference without a tag.
18+
name: "registry with port and tag",
19+
image: "100.108.123.49:49152/tyler/fylun-web:462d7a7",
20+
want: "462d7a7",
21+
},
22+
{name: "simple tag", image: "nginx:1.25", want: "1.25"},
23+
{name: "namespaced tag", image: "black-forest-labs/flux:v2", want: "v2"},
24+
25+
// A registry port must not be mistaken for a tag.
26+
{name: "registry port, no tag", image: "100.108.123.49:49152/tyler/app", want: ""},
27+
{name: "untagged", image: "nginx", want: ""},
28+
29+
// Reusing a floating tag as the version gives every deploy the same
30+
// container name and leaves CurrentHash == PreviousHash, which disables
31+
// rollback. The caller falls back to a timestamp instead.
32+
{name: "latest is refused", image: "nginx:latest", want: ""},
33+
34+
// A digest is content-addressed and the most truthful label available.
35+
// Colons are invalid in container names, hence the dashed prefix.
36+
{
37+
name: "digest",
38+
image: "repo/app@sha256:" + "ab12cd34ef56" + "0000000000000000000000000000000000000000000000000000",
39+
want: "sha256-ab12cd34ef56",
40+
},
41+
{
42+
// Docker resolves the digest and ignores the tag, so labelling this
43+
// "1.2.3" would be the same lie this function exists to remove.
44+
name: "digest wins over an accompanying tag",
45+
image: "repo/app:1.2.3@sha256:" + "ab12cd34ef56" + "0000000000000000000000000000000000000000000000000000",
46+
want: "sha256-ab12cd34ef56",
47+
},
48+
{name: "malformed digest", image: "repo/app@sha256:tooshort", want: ""},
49+
50+
// Versions are interpolated UNQUOTED into remote docker commands via
51+
// ContainerName, and an image reference is a less-trusted source than
52+
// git output. Anything outside Docker's tag grammar is refused.
53+
{name: "shell metacharacters refused", image: "repo/app:v1;rm -rf /", want: ""},
54+
{name: "backtick refused", image: "repo/app:`whoami`", want: ""},
55+
{name: "leading dot refused", image: "repo/app:.hidden", want: ""},
56+
{name: "empty", image: "", want: ""},
57+
}
58+
59+
for _, tc := range cases {
60+
t.Run(tc.name, func(t *testing.T) {
61+
if got := versionFromImage(tc.image); got != tc.want {
62+
t.Errorf("versionFromImage(%q) = %q, want %q", tc.image, got, tc.want)
63+
}
64+
})
65+
}
66+
}
67+
68+
// Every accepted version must be safe to concatenate into a container name and
69+
// into the remote shell commands that manipulate it.
70+
func TestVersionFromImage_OutputIsContainerNameSafe(t *testing.T) {
71+
images := []string{
72+
"100.108.123.49:49152/tyler/fylun-web:462d7a7",
73+
"nginx:1.25",
74+
"repo/app@sha256:ab12cd34ef560000000000000000000000000000000000000000000000000000",
75+
}
76+
for _, image := range images {
77+
v := versionFromImage(image)
78+
if v == "" {
79+
t.Fatalf("expected a version for %q", image)
80+
}
81+
for _, r := range v {
82+
ok := (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') ||
83+
(r >= '0' && r <= '9') || r == '.' || r == '_' || r == '-'
84+
if !ok {
85+
t.Errorf("versionFromImage(%q) = %q contains %q, unsafe in a container name", image, v, r)
86+
}
87+
}
88+
}
89+
}

‎internal/state/state.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,14 @@ type LogEntry struct {
103103
Success bool `json:"success"`
104104
DurationMs int64 `json:"duration_ms"`
105105
Message string `json:"message,omitempty"`
106+
// Image the deploy actually ran, when one was known.
107+
//
108+
// The version alone was not enough: a deploy labelled with a version that
109+
// did not come from the image it ran could not be told apart afterwards,
110+
// and `teploy log` is exactly where a rollback target gets chosen. Omitted
111+
// for entries with no image (heal, lifecycle, static), so old log lines
112+
// parse unchanged.
113+
Image string `json:"image,omitempty"`
106114
}
107115

108116
// Read reads canonical v2 state first. A missing v2 file falls back to the

0 commit comments

Comments
 (0)