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.
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
feat: support new options in terminal #6805
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
kind: Added | ||
body: Support privileges and nesting in default terminal command | ||
time: 2024-03-04T22:45:13.122651+01:00 | ||
custom: | ||
Author: TomChv | ||
PR: "6805" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,7 +39,19 @@ func (term *Terminal) WebsocketURL() string { | |
return fmt.Sprintf("ws://dagger/%s", term.Endpoint) | ||
} | ||
|
||
func (container *Container) Terminal(svcID *idproto.ID, args []string) (*Terminal, http.Handler, error) { | ||
type TerminalArgs struct { | ||
Cmd []string `default:"[]"` | ||
|
||
// Provide dagger access to the executed command | ||
// Do not use this option unless you trust the command being executed. | ||
// The command being executed WILL BE GRANTED FULL ACCESS TO YOUR HOST FILESYSTEM | ||
ExperimentalPrivilegedNesting *bool `default:"false"` | ||
|
||
// Grant the process all root capabilities | ||
InsecureRootCapabilities *bool `default:"false"` | ||
Comment on lines
+48
to
+51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah fun, I guess these need to be pointers so we can tell if you meant to override back to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Exactly, I was confused at first but then I realised it was the only solution because we're using |
||
} | ||
|
||
func (container *Container) Terminal(svcID *idproto.ID, args *TerminalArgs) (*Terminal, http.Handler, error) { | ||
termID, err := svcID.Digest() | ||
if err != nil { | ||
return nil, nil, err | ||
|
@@ -79,13 +91,15 @@ func (container *Container) runTerminal( | |
svcID *idproto.ID, | ||
conn *websocket.Conn, | ||
clientMetadata *engine.ClientMetadata, | ||
args []string, | ||
args *TerminalArgs, | ||
) error { | ||
container = container.Clone() | ||
|
||
container, err := container.WithExec(ctx, ContainerExecOpts{ | ||
Args: args, | ||
SkipEntrypoint: true, | ||
Args: args.Cmd, | ||
SkipEntrypoint: true, | ||
ExperimentalPrivilegedNesting: *args.ExperimentalPrivilegedNesting, | ||
InsecureRootCapabilities: *args.InsecureRootCapabilities, | ||
}) | ||
if err != nil { | ||
return fmt.Errorf("failed to create container for interactive terminal: %w", err) | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just hit this today - this line can panic if
ctr.Self.DefaultTerminalCmd
isn't set.