fix(agent): reject deployment selectors for prebuilt images - #974
Open
herefindalex wants to merge 2 commits into
Open
herefindalex wants to merge 2 commits into
herefindalex wants to merge 2 commits into
Conversation
Prebuilt image deploys return before reading --deployment, allowing a requested staging deploy to upload an image and report success through the default production workflow. Secrets may also be updated first. Validate prebuilt deployment intent in a deploy-specific Before hook, before client setup, secret updates, push-target acquisition, or image loading. Explain that named deployments require deploying from source. Preserve omitted/empty deployment selectors and source-based deployment. Reject every nonempty prebuilt selector, including literal production: the public client uses an empty string for the default deployment and does not establish production as an equivalent selector alias. Add command-boundary regression tests for both image flags, -d, secrets, missing paths, custom/case/whitespace selectors, quiet mode, and rejection before project validation and client creation. Accepted inputs continue through normal client setup without changing their deployment selector. Validation: - New rejection tests fail on the base and pass with this change. - Full go test -race ./... passes (438 test/subtest passes, 9 skips). - CI-pinned golangci-lint v2.11.4 passes with Go 1.26.3 (0 issues). - Local loopback integration verifies zero requests for rejected inputs, successful default tar upload, and source staging propagation. - Binary rejection and unchanged generated fish completion verified. Fixes livekit#971
Replace the deployment flag matrix with four explicit tests covering named image and image-tar rejection, default prebuilt deployment, and named source deployment. Keep one assertion that rejection precedes client creation. Use fresh definitions for the three relevant flags and the registered Before hook. Remove reflection, global HTTP interception, and assertions about aliases, secrets, quiet mode, and unrelated project validation. Production behavior is unchanged. Validation: - Four focused tests pass. - Both rejection tests fail when the registered guard is bypassed. - Complete cmd/lk package passes with the race detector on Go 1.26.3.
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
1 participant
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.
Problem
lk agent deploytakes an early return for--imageand--image-tar, bypassing the handling of--deployment. An explicit deployment selector is therefore silently ignored, potentially updating production when the user intended to deploy to staging.Related to #971.
Changes
Reject a nonempty
--deploymentwhen combined with--imageor--image-tar. Validation runs before client initialization, secret updates, push-target requests, or image uploads.Source deployments continue to support
--deployment. Prebuilt deployments with an omitted or empty selector retain their existing behavior.This is a guard against silently ignoring the selector; it does not implement named deployment support for prebuilt images.
Compatibility
All nonempty selectors are rejected for prebuilt images, including the literal
production. The existing default path uses an empty selector, and this change does not assume thatproductionis a supported alias.Validation
Current regression coverage consists of four focused tests for the deploy validation hook:
--imagewith a named deployment is rejected.--image-tarwith a named deployment is rejected.--image-tar).Rejection occurs before agent-client creation; the
--image-tarrejection test explicitly asserts this. These tests exercise the registeredBeforehook without loading or deploying an image.Previously performed validation and integration checks:
golangci-lintreported no issues.