Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 3 additions & 12 deletions cmd/internal/agentworkspace/logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,6 @@ func NewLogsCmd(flags *flags.GlobalFlags) *cobra.Command {
}

func (cmd *LogsCmd) Run(ctx context.Context) error {
// `agent workspace logs` returns devcontainer logs from inside the
// workspace container/machine. Reject host invocations explicitly
// to avoid silently reading from the wrong place.
if agent.IsHostAgentInvocation(cmd.AgentDir) {
return fmt.Errorf(
"`devsy internal agent workspace logs` is only valid inside the workspace container or machine",
)
}

// get workspace info
shouldExit, workspaceInfo, err := agent.ReadAgentWorkspaceInfo(
cmd.AgentDir,
cmd.Context,
Expand All @@ -59,8 +49,10 @@ func (cmd *LogsCmd) Run(ctx context.Context) error {
} else if shouldExit {
return nil
}
if workspaceInfo == nil {
return fmt.Errorf("workspace %q not found", cmd.ID)
}

// create new runner
runner, err := devcontainer.NewRunner(
config.ContainerDevsyHelperLocation,
config.DefaultAgentDownloadURL(),
Expand All @@ -70,6 +62,5 @@ func (cmd *LogsCmd) Run(ctx context.Context) error {
return fmt.Errorf("create runner: %w", err)
}

// write devcontainer logs to stdout
return runner.Logs(ctx, os.Stdout)
}
4 changes: 1 addition & 3 deletions cmd/internal/logs_daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"os"

"github.com/devsy-org/devsy/cmd/flags"
"github.com/devsy-org/devsy/pkg/agent"
"github.com/devsy-org/devsy/pkg/client"
"github.com/devsy-org/devsy/pkg/config"
provider2 "github.com/devsy-org/devsy/pkg/provider"
Expand Down Expand Up @@ -66,8 +65,7 @@ func (cmd *LogsDaemonCmd) Run(ctx context.Context, args []string) error {
}

command := fmt.Sprintf(
"%s%q internal agent workspace logs-daemon --context %q --id %q",
agent.ContainerAgentEnvPrefix,
"%q internal agent workspace logs-daemon --context %q --id %q",
workspaceClient.AgentPath(),
workspaceClient.Context(),
workspaceClient.Workspace(),
Expand Down
3 changes: 1 addition & 2 deletions cmd/workspace/logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,7 @@ func (cmd *LogsCmd) Run(ctx context.Context, args []string) error {
defer func() { _ = session.Close() }()

agentCommand := fmt.Sprintf(
"%s%q internal agent workspace logs --context %q --id %q",
agent.ContainerAgentEnvPrefix,
"%q internal agent workspace logs --context %q --id %q",
client.AgentPath(),
client.Context(),
client.Workspace(),
Expand Down
3 changes: 1 addition & 2 deletions cmd/workspace/up/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,7 @@ func (cmd *UpCmd) devsyUpMachineSSH(
}

agentCommand := fmt.Sprintf(
"%s%q internal agent workspace up --workspace-info %q",
agent.ContainerAgentEnvPrefix,
"%q internal agent workspace up --workspace-info %q",
client.AgentPath(),
workspaceInfo,
)
Expand Down
65 changes: 4 additions & 61 deletions pkg/agent/workspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,42 +35,10 @@ var extraSearchLocations = []string{
config.ContainerDataDir + "/agent",
}

// EnvAgentInContainer is set to "1" by every SSH command builder that
// launches `devsy agent ...` inside the workspace container or machine.
// It is the single signal used by IsHostAgentInvocation to distinguish a
// container-side invocation from a host-side one. The host never sets it.
const EnvAgentInContainer = "DEVSY_AGENT_IN_CONTAINER"

// EnvAgentInContainerTrue is the value the env var takes when set.
const EnvAgentInContainerTrue = "1"

// ContainerAgentEnvPrefix is the inline env-var assignment SSH command
// builders prepend to the shell snippet that launches `devsy agent ...`
// inside the workspace container or machine. POSIX shells parse leading
// `NAME=value cmd ...` as a one-shot environment assignment for `cmd`,
// so this works without requiring the SSH server to honour SetEnv (which
// is often disabled).
const ContainerAgentEnvPrefix = EnvAgentInContainer + "=" + EnvAgentInContainerTrue + " "

// AgentCommandEnvPrefix omits the container env prefix when the command
// will be executed on the host (provider agent.local=true); prepending it
// there would mislabel the host process and trip IsHostAgentInvocation's
// stale-env warning.
func AgentCommandEnvPrefix(isLocal bool) string {
if isLocal {
return ""
}
return ContainerAgentEnvPrefix
}

var ErrFindAgentHomeFolder = fmt.Errorf("couldn't find devsy home directory")

// GetAgentDaemonLogFolder returns the folder that holds agent-daemon.log.
// The daemon is a container/machine-side process: it is started by the
// inject-and-run path during `devsy up` and the SSH command builder sets
// DEVSY_AGENT_IN_CONTAINER=1. A host-side invocation has no daemon to
// inspect and therefore is rejected explicitly instead of silently
// resolving to the legacy `<DEVSY_HOME>/agent` glob.
// GetAgentDaemonLogFolder returns the path to the agent daemon log folder, which is only
// available inside the workspace container or machine.
func GetAgentDaemonLogFolder(agentFolder string) (string, error) {
if IsHostAgentInvocation(agentFolder) {
return "", fmt.Errorf(
Expand Down Expand Up @@ -229,39 +197,14 @@ func GetAgentBinariesDirFromWorkspaceDir(workspaceDir string) (string, error) {
return "", os.ErrNotExist
}

// IsHostAgentInvocation reports whether this `devsy agent ...` call is
// running on the user's host (as opposed to inside the workspace container
// or machine). The signal is explicit: every SSH command builder that
// launches `devsy agent ...` inside the container sets
// DEVSY_AGENT_IN_CONTAINER=1; the host never sets it. An explicit
// --agent-dir (agentFolder != "") also forces a non-host (legacy/explicit)
// routing, so unit tests and unusual deployments can still pin a folder.
//
// When this returns true, per-workspace agent state lives under the
// canonical PathManager.WorkspaceAgentDir so a single
// os.RemoveAll(WorkspaceDir) wipes it on delete. DEVSY_HOME is NOT
// consulted here — it is purely a host-side config-dir relocation knob
// and must not double as a container marker.
// containerDetector is the package-level seam used by IsHostAgentInvocation
// to detect whether the process is running inside a container. Tests
// override this to make the predicate deterministic across platforms.
var containerDetector = isLikelyContainer

// IsHostAgentInvocation determines whether the current invocation is a host-side invocation of the agent.
func IsHostAgentInvocation(agentFolder string) bool {
if agentFolder != "" {
return false
}
if os.Getenv(EnvAgentInContainer) != EnvAgentInContainerTrue {
return true
}
if !containerDetector() {
log.Debugf(
"%s=1 is set but no container indicator file found; treating as host invocation",
EnvAgentInContainer,
)
return true
}
return false
return !containerDetector()
}

func GetAgentBinariesDir(agentFolder, context, workspaceID string) (string, error) {
Expand Down
73 changes: 6 additions & 67 deletions pkg/agent/workspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,6 @@ import (

const explicitAgentDir = "/some/dir"

func TestAgentCommandEnvPrefix(t *testing.T) {
if got := AgentCommandEnvPrefix(true); got != "" {
t.Errorf("AgentCommandEnvPrefix(true) = %q, want \"\"", got)
}
if got := AgentCommandEnvPrefix(false); got != ContainerAgentEnvPrefix {
t.Errorf(
"AgentCommandEnvPrefix(false) = %q, want %q",
got,
ContainerAgentEnvPrefix,
)
}
}

// withContainerDetector swaps the package-level container detector for
// the duration of the test, restoring the previous value on cleanup.
func withContainerDetector(t *testing.T, fn func() bool) {
t.Helper()
prev := containerDetector
Expand All @@ -33,106 +18,60 @@ func withContainerDetector(t *testing.T, fn func() bool) {
type hostInvocationCase struct {
name string
agentFolder string
inContainer string // empty == unset; "1" == container marker
containerSeen bool
want bool
}

func hostInvocationCases() []hostInvocationCase {
return []hostInvocationCase{
{
name: "host: no agentFolder, no marker, no indicator",
agentFolder: "",
inContainer: "",
containerSeen: false,
want: true,
},
{
name: "container: no agentFolder, marker set, indicator present",
agentFolder: "",
inContainer: "1",
containerSeen: true,
want: false,
},
{
name: "host with stale env: marker set but no indicator",
name: "host: no agentFolder, not in a container",
agentFolder: "",
inContainer: "1",
containerSeen: false,
want: true,
},
{
name: "host with rogue indicator but no env",
name: "container: no agentFolder, running in a container",
agentFolder: "",
inContainer: "",
containerSeen: true,
want: true,
},
{
name: "explicit agentFolder, no marker (legacy/explicit)",
agentFolder: explicitAgentDir,
inContainer: "",
containerSeen: false,
want: false,
},
{
name: "explicit agentFolder beats stale env",
name: "explicit agentFolder, not in a container (legacy/explicit)",
agentFolder: explicitAgentDir,
inContainer: "1",
containerSeen: false,
want: false,
},
{
name: "explicit agentFolder and marker (container with --agent-dir)",
name: "explicit agentFolder beats container detection",
agentFolder: explicitAgentDir,
inContainer: "1",
containerSeen: true,
want: false,
},
}
}

// TestIsHostAgentInvocation covers the matrix of
// (agentFolder empty/non-empty) x (env unset/"1") x (container indicator yes/no).
func TestIsHostAgentInvocation(t *testing.T) {
for _, tc := range hostInvocationCases() {
t.Run(tc.name, func(t *testing.T) {
t.Setenv(EnvAgentInContainer, tc.inContainer)
withContainerDetector(t, func() bool { return tc.containerSeen })

got := IsHostAgentInvocation(tc.agentFolder)
if got != tc.want {
t.Fatalf(
"IsHostAgentInvocation(%q) with %s=%q indicator=%v = %v, want %v",
tc.agentFolder, EnvAgentInContainer, tc.inContainer,
tc.containerSeen, got, tc.want,
"IsHostAgentInvocation(%q) with container=%v = %v, want %v",
tc.agentFolder, tc.containerSeen, got, tc.want,
)
}
})
}
}

// TestIsHostAgentInvocation_IgnoresDevsyHome guards the regression that
// setting DEVSY_HOME on the host must NOT flip the predicate to
// "container" — only DEVSY_AGENT_IN_CONTAINER + an indicator does that.
func TestIsHostAgentInvocation_IgnoresDevsyHome(t *testing.T) {
t.Setenv("DEVSY_HOME", "/custom/devsy/home")
t.Setenv(EnvAgentInContainer, "")
withContainerDetector(t, func() bool { return false })

if !IsHostAgentInvocation("") {
t.Fatal("IsHostAgentInvocation should still report host when only DEVSY_HOME is set")
}
}

// TestIsHostAgentInvocation_NonStandardMarkerValue ensures that only the
// exact "1" string flips the predicate, mirroring the strict equality
// check in the implementation.
func TestIsHostAgentInvocation_NonStandardMarkerValue(t *testing.T) {
t.Setenv(EnvAgentInContainer, "true")
withContainerDetector(t, func() bool { return true })

if !IsHostAgentInvocation("") {
t.Fatal("only DEVSY_AGENT_IN_CONTAINER=1 should be honoured; got false for value 'true'")
}
}
12 changes: 4 additions & 8 deletions pkg/client/clientimplementation/workspace_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -378,8 +378,7 @@ func (s *workspaceClient) Delete(ctx context.Context, opt client.DeleteOptions)
return fmt.Errorf("agent info")
}
command := fmt.Sprintf(
"%s%q internal agent workspace delete --workspace-info %q",
agent.AgentCommandEnvPrefix(s.agentLocalLocked()),
"%q internal agent workspace delete --workspace-info %q",
info.Agent.Path,
compressed,
)
Expand Down Expand Up @@ -487,8 +486,7 @@ func (s *workspaceClient) Stop(ctx context.Context, opt client.StopOptions) erro
return fmt.Errorf("agent info")
}
command := fmt.Sprintf(
"%s%q internal agent workspace stop --workspace-info %q",
agent.AgentCommandEnvPrefix(s.agentLocalLocked()),
"%q internal agent workspace stop --workspace-info %q",
info.Agent.Path,
compressed,
)
Expand Down Expand Up @@ -667,8 +665,7 @@ func (s *workspaceClient) getContainerStatus(ctx context.Context) (client.Status
return "", fmt.Errorf("get agent info")
}
command := fmt.Sprintf(
"%s%q internal agent workspace status --workspace-info %q",
agent.AgentCommandEnvPrefix(s.agentLocalLocked()),
"%q internal agent workspace status --workspace-info %q",
info.Agent.Path,
compressed,
)
Expand Down Expand Up @@ -1008,8 +1005,7 @@ func buildAgentCommand(
agentCommand, workspaceInfo string,
) string {
command := fmt.Sprintf(
"%s%q internal agent workspace %s --workspace-info %q",
agent.AgentCommandEnvPrefix(workspaceClient.AgentLocal()),
"%q internal agent workspace %s --workspace-info %q",
workspaceClient.AgentPath(),
agentCommand,
workspaceInfo,
Expand Down
6 changes: 2 additions & 4 deletions pkg/tunnel/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,7 @@ func (c *ContainerTunnel) updateConfig(ctx context.Context, sshClient *ssh.Clien
// update workspace remotely
buf := &bytes.Buffer{}
command := fmt.Sprintf(
"%s%q internal agent workspace update-config --workspace-info %q",
agent.ContainerAgentEnvPrefix,
"%q internal agent workspace update-config --workspace-info %q",
c.client.AgentPath(),
workspaceInfo,
)
Expand Down Expand Up @@ -243,8 +242,7 @@ func (c *ContainerTunnel) runContainerTunnel(ctx context.Context, opts container
defer log.Debugf("Container tunnel exited")

command := fmt.Sprintf(
"%s%q internal agent container-tunnel --workspace-info %q",
agent.ContainerAgentEnvPrefix,
"%q internal agent container-tunnel --workspace-info %q",
c.client.AgentPath(),
opts.workspaceInfo,
)
Expand Down
4 changes: 1 addition & 3 deletions pkg/tunnel/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (

"al.essio.dev/pkg/shellescape"
"github.com/devsy-org/api/pkg/devsy"
"github.com/devsy-org/devsy/pkg/agent"
"github.com/devsy-org/devsy/pkg/agent/tunnelserver"
"github.com/devsy-org/devsy/pkg/config"
config2 "github.com/devsy-org/devsy/pkg/devcontainer/config"
Expand Down Expand Up @@ -150,8 +149,7 @@ func addGitSSHSigningKey(
// buildCredentialsCommand builds the credentials server command.
func buildCredentialsCommand(ctx context.Context, opts RunServicesOptions) string {
command := fmt.Sprintf(
"%s%s internal agent container credentials-server --user %s",
agent.ContainerAgentEnvPrefix,
"%s internal agent container credentials-server --user %s",
shellescape.Quote(config.ContainerDevsyHelperLocation),
shellescape.Quote(opts.User),
)
Expand Down
Loading