From d2fbe366e0d1920aef23e31af7af15d5534a25bb Mon Sep 17 00:00:00 2001 From: Jamison Lahman Date: Tue, 8 Sep 2026 23:38:42 +0000 Subject: [PATCH] fix(cli): fetch config files from the release ref for -dev image tags (#14587) Co-authored-by: Claude Fable 5.1 --- cli/cmd/deploy_upgrade.go | 4 +- cli/internal/deploy/install/install.go | 10 +-- cli/internal/deploy/install/install_test.go | 63 +++++++++++++++-- cli/internal/deploy/install/upgrade_test.go | 60 ++++++++++++++++ cli/internal/deploy/release/release.go | 76 ++++++++++++++++----- cli/internal/deploy/release/release_test.go | 57 +++++++++++++++- 6 files changed, 241 insertions(+), 29 deletions(-) diff --git a/cli/cmd/deploy_upgrade.go b/cli/cmd/deploy_upgrade.go index 16bf098582..a517796de9 100644 --- a/cli/cmd/deploy_upgrade.go +++ b/cli/cmd/deploy_upgrade.go @@ -21,7 +21,8 @@ func newDeployUpgradeCmdWithDeps(ios *iostreams.IOStreams, deps *install.Deps) * Only the IMAGE_TAG line (plus SANDBOX_BACKEND when Craft is enabled) is rewritten in .env — every other setting, including your edits, is preserved. Managed files (compose, overlays, nginx config) are refreshed to match the -target version; files you hand-edited are kept unless you confirm the +target version; a -dev image tag (v4.4.6-dev) takes them from the release it +was built from (v4.4.6). Files you hand-edited are kept unless you confirm the overwrite (or pass --force), and a backup is made first. The running services keep serving while images download and are then recreated on the new version. @@ -32,6 +33,7 @@ asserts it for the first adoption of an unmanaged deployment. --project targets a stack that runs under a compose project name other than "onyx".`, Example: ` onyx-cli deploy upgrade onyx-cli deploy upgrade --tag v4.4.6 + onyx-cli deploy upgrade --tag v4.4.6-dev onyx-cli deploy upgrade --tag v4.4.6 --no-prompt --force onyx-cli deploy upgrade --prod --project danswer-stack --dir /opt/onyx --no-prompt`, Args: cobra.NoArgs, diff --git a/cli/internal/deploy/install/install.go b/cli/internal/deploy/install/install.go index 7ca9f34dfa..3ab5d4250d 100644 --- a/cli/internal/deploy/install/install.go +++ b/cli/internal/deploy/install/install.go @@ -455,9 +455,10 @@ func (in *installer) runInstall(ctx context.Context) error { // exists BEFORE anything is written, so a typo fails here instead of // surfacing minutes later as a pull error with the bad version already in // .env. Only release versions are looked up: floating and hand-built image -// tags are pullable without a matching git ref. Verification is best-effort -// by design — an unreachable (or lying) GitHub must never be able to block -// an install that would otherwise work. +// tags are pullable without a matching git ref. A -dev twin is looked up +// through the release it was built from. Verification is best-effort by +// design — an unreachable (or lying) GitHub must never be able to block an +// install that would otherwise work. func (in *installer) validateTag(ctx context.Context, tag string) (string, error) { tag = strings.TrimSpace(tag) if tag == "" { @@ -472,7 +473,8 @@ func (in *installer) validateTag(ctx context.Context, tag string) (string, error return normalized, nil } - exists, err := in.deps.Release.RefExists(ctx, normalized) + // A -dev twin has no ref of its own: its plain tag is what must exist. + exists, err := in.deps.Release.RefExists(ctx, release.ConfigRef(normalized)) if err != nil { in.warnf("Could not verify that version %s exists (%v) — continuing", normalized, err) return normalized, nil diff --git a/cli/internal/deploy/install/install_test.go b/cli/internal/deploy/install/install_test.go index 2ec280e1d6..a9d30b02b2 100644 --- a/cli/internal/deploy/install/install_test.go +++ b/cli/internal/deploy/install/install_test.go @@ -691,17 +691,70 @@ func TestInstallProceedsWhenExistenceCannotBeConfirmed(t *testing.T) { } } -// Image tags that aren't git refs (hand-built images, -dev twins) are -// pullable and must skip the repo lookup rather than be rejected. -func TestInstallAcceptsNonReleaseImageTag(t *testing.T) { +// Image tags that aren't git refs (hand-built images) are pullable and must +// skip the repo lookup rather than be rejected. +func TestInstallAcceptsHandBuiltImageTag(t *testing.T) { isolateEnv(t) shimDockerOnPath(t) root := t.TempDir() deps := testDeps(t, &fakeRunner{handler: healthyDockerHandler}, refServer(t)) if err := RunInstall(context.Background(), deps, Options{ - NoPrompt: true, Tag: "v4.0.0-dev", Dir: root, NoWait: true, + NoPrompt: true, Tag: "mybuild", Dir: root, NoWait: true, }); err != nil { - t.Fatalf("non-release image tag must be accepted: %v", err) + t.Fatalf("hand-built image tag must be accepted: %v", err) + } +} + +// A -dev twin has no git ref of its own; it is verified through the release +// it was built from, and kept as the image tag. +func TestInstallAcceptsDevTwinOfKnownVersion(t *testing.T) { + cases := []struct { + tag string + ref string + want string + }{ + {"4.0.0-dev", "v4.0.0", "v4.0.0-dev"}, + {"v4.7.0-cloud.3-dev", "v4.7.0-cloud.3", "v4.7.0-cloud.3-dev"}, + } + for _, c := range cases { + t.Run(c.tag, func(t *testing.T) { + isolateEnv(t) + shimDockerOnPath(t) + root := t.TempDir() + deps := testDeps(t, &fakeRunner{handler: healthyDockerHandler}, refServer(t, c.ref)) + if err := RunInstall(context.Background(), deps, Options{ + NoPrompt: true, Tag: c.tag, Dir: root, NoWait: true, + }); err != nil { + t.Fatalf("-dev twin of an existing release must be accepted: %v\noutput:\n%s", err, outBuf(deps).String()) + } + env, _ := os.ReadFile(filepath.Join(root, "deployment", ".env")) + if got := Var(string(env), "IMAGE_TAG"); got != c.want { + t.Errorf("IMAGE_TAG = %q, want the v-prefixed -dev image tag %q", got, c.want) + } + }) + } +} + +// A -dev twin can only exist when its release does, so a typo in the version +// part fails as early as it would without the suffix. Pre-release twins are +// checked the same way: their config ref is the plain pre-release tag. +func TestInstallRejectsDevTwinOfUnknownVersion(t *testing.T) { + for _, tag := range []string{"v9.9.9-dev", "v4.7.0-cloud.999-dev"} { + t.Run(tag, func(t *testing.T) { + isolateEnv(t) + shimDockerOnPath(t) + root := t.TempDir() + deps := testDeps(t, &fakeRunner{handler: healthyDockerHandler}, refServer(t, "v4.7.0-cloud.3")) + err := RunInstall(context.Background(), deps, Options{ + NoPrompt: true, Tag: tag, Dir: root, NoWait: true, + }) + if err == nil || !strings.Contains(err.Error(), "not found") { + t.Fatalf("err = %v, want unknown-version rejection", err) + } + if _, statErr := os.Stat(filepath.Join(root, "deployment", ".env")); !os.IsNotExist(statErr) { + t.Error("rejected install must not create .env") + } + }) } } diff --git a/cli/internal/deploy/install/upgrade_test.go b/cli/internal/deploy/install/upgrade_test.go index 5171e9137f..da79190ec2 100644 --- a/cli/internal/deploy/install/upgrade_test.go +++ b/cli/internal/deploy/install/upgrade_test.go @@ -3,9 +3,12 @@ package install import ( "context" "errors" + "net/http" + "net/http/httptest" "os" "path/filepath" "strings" + "sync" "testing" "github.com/onyx-dot-app/onyx/cli/internal/deploy/dockercmd" @@ -85,6 +88,63 @@ func TestUpgradeRewritesOnlyImageTag(t *testing.T) { } } +// A -dev image tag is the release's image with debugging tools added, and its +// config files live at the release's ref. The upgrade must fetch them from +// there rather than fall back to the embedded copies for a ref that does not +// exist, while .env and the manifest keep naming the -dev image. +func TestUpgradeDevTagFetchesConfigFromReleaseRef(t *testing.T) { + runner := &fakeRunner{handler: healthyDockerHandler} + root := installFixture(t, runner, "v4.0.0") + + upstream := "# compose at v4.2.0\nname: onyx\n" + var mu sync.Mutex + var fetched []string + raw := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + fetched = append(fetched, r.URL.Path) + mu.Unlock() + if r.Method == http.MethodHead { + return + } + _, _ = w.Write([]byte(upstream)) + })) + t.Cleanup(raw.Close) + + deps := testDeps(t, runner, raw) + err := RunUpgrade(context.Background(), deps, Options{ + NoPrompt: true, Tag: "v4.2.0-dev", Dir: root, NoWait: true, + }) + if err != nil { + t.Fatalf("RunUpgrade: %v\noutput:\n%s", err, outBuf(deps).String()) + } + + mu.Lock() + defer mu.Unlock() + if len(fetched) == 0 { + t.Fatal("no config files were fetched") + } + for _, p := range fetched { + if !strings.Contains(p, "/v4.2.0/") { + t.Errorf("fetched %s, want every file from the release ref v4.2.0", p) + } + } + compose, _ := os.ReadFile(filepath.Join(root, "deployment", "docker-compose.yml")) + if string(compose) != upstream { + t.Errorf("compose = %q, want the copy fetched from v4.2.0", compose) + } + env, _ := os.ReadFile(filepath.Join(root, "deployment", ".env")) + if got := Var(string(env), "IMAGE_TAG"); got != "v4.2.0-dev" { + t.Errorf("IMAGE_TAG = %q, want the -dev image tag kept", got) + } + m, merr := state.Load(root) + if merr != nil || m == nil { + t.Fatalf("manifest: %+v, %v", m, merr) + } + if m.InstalledTag != "v4.2.0-dev" { + t.Errorf("manifest tag = %q, want v4.2.0-dev", m.InstalledTag) + } +} + func TestUpgradeRefusesDowngradeNonInteractively(t *testing.T) { runner := &fakeRunner{handler: healthyDockerHandler} root := installFixture(t, runner, "v4.2.0") diff --git a/cli/internal/deploy/release/release.go b/cli/internal/deploy/release/release.go index 50d2783e55..28e528c1e0 100644 --- a/cli/internal/deploy/release/release.go +++ b/cli/internal/deploy/release/release.go @@ -46,26 +46,64 @@ func checkRef(ref string) error { // releaseVersionPattern matches a release version as a user would type it, // with or without the conventional "v" prefix. Image tags that are not git -// refs (beta, nightly, vX.Y.Z-dev, locally built tags) deliberately don't -// match: they are pullable but can't be looked up in the repo. +// refs (beta, nightly, locally built tags) deliberately don't match: they are +// pullable but can't be looked up in the repo. var releaseVersionPattern = regexp.MustCompile(`^v?\d+\.\d+\.\d+$`) -// NormalizeVersionTag adds the conventional "v" prefix to a bare release -// version ("4.4.6" → "v4.4.6") and reports whether the result is a release -// version, i.e. one whose existence can be checked with RefExists. -func NormalizeVersionTag(tag string) (string, bool) { - if !releaseVersionPattern.MatchString(tag) { - return tag, false +// devSuffix marks the -dev twin of an image tag. CI publishes every tag set +// twice from the same git ref — vX.Y.Z and vX.Y.Z-dev, edge and edge-dev — +// with the twin adding a shell and debugging tools to the backend image. The +// suffix names an image variant, never a git ref of its own. +const devSuffix = "-dev" + +// releaseShapedPattern matches a whole tag that names a release, pre-release +// forms included (v4.7.0-beta.1, v4.7.0-cloud.3), with or without the "v" +// prefix. It bounds which -dev tags count as twins: the repo also carries +// unrelated refs that happen to end in -dev, and a hand-built tag may start +// with a version (v4.7.1.2-dev) without naming one. +var releaseShapedPattern = regexp.MustCompile(`^v?\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?$`) + +// SplitDevSuffix separates a -dev twin into its plain tag and a flag. Only +// twins of floating or release-shaped tags split; any other tag comes back +// unchanged with dev=false. +func SplitDevSuffix(tag string) (base string, dev bool) { + base, dev = strings.CutSuffix(tag, devSuffix) + if dev && (isFloating(base) || releaseShapedPattern.MatchString(base)) { + return base, true } - if !strings.HasPrefix(tag, "v") { - return "v" + tag, true - } - return tag, true + return tag, false } -// FloatingTags are rolling image tags that track main rather than a pinned -// release. +// NormalizeVersionTag adds the conventional "v" prefix to a bare release +// version ("4.4.6" → "v4.4.6", "4.4.6-dev" → "v4.4.6-dev") and reports +// whether the result can be checked with RefExists at ConfigRef(tag). That +// holds for release versions and for every -dev twin SplitDevSuffix +// recognizes: a twin's config ref is its plain tag, so a pre-release twin +// (v4.7.0-beta.1-dev) is checked even though a bare pre-release is not. +func NormalizeVersionTag(tag string) (string, bool) { + base, dev := SplitDevSuffix(tag) + checkable := releaseVersionPattern.MatchString(base) || + (dev && releaseShapedPattern.MatchString(base)) + if !checkable { + return tag, false + } + if !strings.HasPrefix(base, "v") { + base = "v" + base + } + if dev { + base += devSuffix + } + return base, true +} + +// IsFloatingTag reports whether tag (or the plain tag of a -dev twin) is a +// rolling image tag that tracks main rather than a pinned release. func IsFloatingTag(tag string) bool { + base, _ := SplitDevSuffix(tag) + return isFloating(base) +} + +func isFloating(tag string) bool { return tag == "edge" || tag == "latest" } @@ -91,13 +129,15 @@ func IsImmutableTag(tag string) bool { } // ConfigRef maps an image tag to the git ref its deployment files ship at: -// floating tags track main, pinned tags use their own ref (mirrors -// install.sh's CONFIG_REF logic). +// floating tags track main, pinned tags use their own ref, and a -dev twin +// uses the ref of the plain tag it was built from (there is no vX.Y.Z-dev +// ref in the repo). func ConfigRef(tag string) string { - if IsFloatingTag(tag) { + base, _ := SplitDevSuffix(tag) + if isFloating(base) { return "main" } - return tag + return base } // Client talks to GitHub. The zero-ish defaults from NewClient hit the real diff --git a/cli/internal/deploy/release/release_test.go b/cli/internal/deploy/release/release_test.go index 984f4e9eed..e1c5a0f78d 100644 --- a/cli/internal/deploy/release/release_test.go +++ b/cli/internal/deploy/release/release_test.go @@ -148,6 +148,14 @@ func TestConfigRef(t *testing.T) { "latest": "main", "v4.4.6": "v4.4.6", "main": "main", + // -dev twins ship their config files at the plain tag's ref. + "v4.7.1-dev": "v4.7.1", + "v4.7.0-cloud.3-dev": "v4.7.0-cloud.3", + "edge-dev": "main", + "latest-dev": "main", + // An unrelated ref that happens to end in -dev is not a twin. + "sandbox-dev": "sandbox-dev", + "v4.7.1.2-dev": "v4.7.1.2-dev", } for tag, want := range cases { if got := ConfigRef(tag); got != want { @@ -157,6 +165,42 @@ func TestConfigRef(t *testing.T) { if !IsFloatingTag("edge") || !IsFloatingTag("latest") || IsFloatingTag("v4.4.6") { t.Error("IsFloatingTag misclassifies") } + if !IsFloatingTag("edge-dev") || !IsFloatingTag("latest-dev") || + IsFloatingTag("v4.7.1-dev") || IsFloatingTag("sandbox-dev") { + t.Error("IsFloatingTag misclassifies -dev twins") + } +} + +// The -dev suffix is an image variant of a release or floating tag, built +// from the same ref. Anything else ending in -dev is left alone. +func TestSplitDevSuffix(t *testing.T) { + cases := []struct { + in string + base string + dev bool + }{ + {"v4.7.1-dev", "v4.7.1", true}, + {"4.7.1-dev", "4.7.1", true}, + {"v4.7.0-beta.1-dev", "v4.7.0-beta.1", true}, + {"edge-dev", "edge", true}, + {"latest-dev", "latest", true}, + {"v4.7.1", "v4.7.1", false}, + {"edge", "edge", false}, + {"sandbox-dev", "sandbox-dev", false}, + {"beta-dev", "beta-dev", false}, + // A version prefix alone does not make a release: the whole tag must + // have the shape. + {"v4.7.1.2-dev", "v4.7.1.2-dev", false}, + {"v4.7.1rc-dev", "v4.7.1rc-dev", false}, + {"-dev", "-dev", false}, + {"", "", false}, + } + for _, c := range cases { + base, dev := SplitDevSuffix(c.in) + if base != c.base || dev != c.dev { + t.Errorf("SplitDevSuffix(%q) = (%q, %t), want (%q, %t)", c.in, base, dev, c.base, c.dev) + } + } } // Only released versions are safe to treat as already-pulled: everything else @@ -220,7 +264,18 @@ func TestNormalizeVersionTag(t *testing.T) { {"edge", "edge", false}, {"latest", "latest", false}, {"main", "main", false}, - {"v4.4.6-dev", "v4.4.6-dev", false}, // pullable image, not a git ref + // A -dev twin keeps its suffix and is checkable through its plain tag, + // pre-release twins included. + {"v4.4.6-dev", "v4.4.6-dev", true}, + {"4.4.6-dev", "v4.4.6-dev", true}, + {"v4.7.0-cloud.3-dev", "v4.7.0-cloud.3-dev", true}, + {"4.7.0-beta.1-dev", "v4.7.0-beta.1-dev", true}, + {"edge-dev", "edge-dev", false}, + {"sandbox-dev", "sandbox-dev", false}, + {"v4.7.1.2-dev", "v4.7.1.2-dev", false}, + // A bare pre-release is pullable but not looked up (hand-built tags + // share its shape). + {"v4.7.0-cloud.3", "v4.7.0-cloud.3", false}, {"beta", "beta", false}, {"v4.4", "v4.4", false}, }