Skip to content

Bump version for envd, require it for nfs #2385

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dobrac merged 3 commits into from
Apr 14, 2026
+4 −4
Merged

Bump version for envd, require it for nfs #2385

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8206f36
bump version for envd, require it for nfs
djeebus Apr 14, 2026
c1920c3
fix the test
djeebus Apr 14, 2026
a9febb3
use the right version
djeebus Apr 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/api/internal/handlers/sandbox_create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,10 +313,10 @@
}

var errVolumesNotSupported = errors.New("volumes are not supported")

var errNoEnvdVersion = errors.New("no envd version provided")

const minEnvdVersionForVolumes = "0.5.8"
const minEnvdVersionForVolumes = "0.5.12"

Check failure on line 319 in packages/api/internal/handlers/sandbox_create.go

View check run for this annotation

Claude / Claude Code Review

Tests use stale envd version 0.5.8, now below new minimum 0.5.12 

After bumping minEnvdVersionForVolumes to 0.5.12, the test file sandbox_create_test.go still uses EnvdVersion: utils.ToPtr("0.5.8") at lines 457 and 487, causing test failures. Update both occurrences to "0.5.12" so that version-gated table-driven tests and the standalone 'existing volumes are returned' test pass again.

Check notice on line 319 in packages/api/internal/handlers/sandbox_create.go

View check run for this annotation

Claude / Claude Code Review

errNoEnvdVersion unhandled in PostSandboxes causes opaque 500 instead of helpful 400 

**Pre-existing bug:** When a template has no recorded envd version (nil/empty `EnvdVersion`), `convertAPIVolumesToOrchestratorVolumes` returns `errNoEnvdVersion`, but `PostSandboxes` does not check for this error — it falls through to the generic handler and returns HTTP 500 with an opaque "failed to convert volume mounts" message instead of a helpful HTTP 400 asking the user to rebuild their template. The fix is to add `errors.Is(err, errNoEnvdVersion)` alongside the existing `errVolumesNotSupp
Comment on lines 316 to +319
Copy link
Copy Markdown

@claude claude bot Apr 14, 2026

Choose a reason for hiding this comment

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

🟣 Pre-existing bug: When a template has no recorded envd version (nil/empty EnvdVersion), convertAPIVolumesToOrchestratorVolumes returns errNoEnvdVersion, but PostSandboxes does not check for this error — it falls through to the generic handler and returns HTTP 500 with an opaque "failed to convert volume mounts" message instead of a helpful HTTP 400 asking the user to rebuild their template. The fix is to add errors.Is(err, errNoEnvdVersion) alongside the existing errVolumesNotSupported check and return a 400 with a clear rebuild message.

Extended reasoning...

Bug Analysis

What the bug is: In convertAPIVolumesToOrchestratorVolumes, when env.EnvdVersion is nil or an empty string, the function returns the sentinel error errNoEnvdVersion (line 336). The caller, PostSandboxes, has a dedicated error-handling block for the result of this function (lines ~197–218), but it only checks for three specific errors: errVolumesNotSupported, ErrVolumeMountsDisabled, and InvalidVolumeMountsError. errNoEnvdVersion is not among them.

The specific code path: When a user submits a sandbox creation request with volume mounts and their template's EnvdVersion field is nil or empty, execution reaches convertAPIVolumesToOrchestratorVolumes → hits the envdVersion == "" branch → returns (nil, errNoEnvdVersion) → back in PostSandboxes, none of the three if branches match → falls to the final else branch → telemetry.ReportError is called (treating it as an unexpected internal error) → http.StatusInternalServerError is returned with the message "failed to convert volume mounts".

Why existing code doesn't prevent it: The three error checks are each exact sentinel comparisons or type assertions. errNoEnvdVersion is a distinct sentinel defined on line 317, and it is never wrapped when returned (line 336 is a bare return nil, errNoEnvdVersion), so errors.Is would match it — but no such check exists in the handler.

Impact: Users whose templates were built without a recorded envd version receive an opaque HTTP 500 that gives no hint about the root cause or what to do. The correct behavior — returning HTTP 400 with a message like "template must be rebuilt" — already exists for the analogous errVolumesNotSupported case; errNoEnvdVersion simply needs the same treatment.

How to fix it: Add a check immediately after (or before) the errVolumesNotSupported check:

if errors.Is(err, errNoEnvdVersion) {
    a.sendAPIStoreError(c, http.StatusBadRequest, "Template must be rebuilt to support volume mounts (no envd version recorded).")
    return
}

Step-by-step proof:

  1. User calls POST /sandboxes with volumeMounts: [{name: "my-vol", path: "/data"}] using a very old template where build.EnvdVersion == nil.
  2. PostSandboxes calls convertAPIVolumesToOrchestratorVolumes(..., build).
  3. Inside that function: envdVersion := sharedUtils.DerefOrDefault(env.EnvdVersion, "") // returns "".
  4. The condition envdVersion == "" is true → function returns nil, errNoEnvdVersion.
  5. Back in PostSandboxes: errors.Is(err, errVolumesNotSupported) → false. errors.Is(err, ErrVolumeMountsDisabled) → false. errors.As(err, &vne) for InvalidVolumeMountsError → false.
  6. Execution falls to telemetry.ReportError(...) and a.sendAPIStoreError(c, http.StatusInternalServerError, "failed to convert volume mounts").
  7. User receives a 500 with no actionable information.


func convertAPIVolumesToOrchestratorVolumes(ctx context.Context, sqlClient *sqlcdb.Client, featureFlags featureFlagsClient, teamID uuid.UUID, volumeMounts []api.SandboxVolumeMount, env *queries.EnvBuild) ([]*orchestrator.SandboxVolumeMount, error) {
// are any volumes configured?
Expand Down
2 changes: 1 addition & 1 deletion packages/envd/pkg/version.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
package pkg

const Version = "0.5.11"
const Version = "0.5.12"
Loading