Skip to content
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

Jump to bottom

[WIP] Finish isolating sessions to each client #6916

Closed
wants to merge 4 commits into from
Closed

[WIP] Finish isolating sessions to each client #6916

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e2c5e6e
consolidate module caller digest to be client id
sipsma Mar 21, 2024
416292f
reuse server for nested execs; register them as calls too
sipsma Mar 21, 2024
e0d9376
disentangle dagql server from ModDeps, tie to Query instead
sipsma Mar 25, 2024
698fd71
wip
sipsma Mar 26, 2024
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
30 changes: 13 additions & 17 deletions cmd/shim/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"github.com/dagger/dagger/engine/client"
"github.com/dagger/dagger/network"
"github.com/google/uuid"
"github.com/opencontainers/go-digest"
"github.com/opencontainers/runtime-spec/specs-go"
"github.com/vito/progrock"
"golang.org/x/sys/unix"
Expand Down Expand Up @@ -452,7 +451,7 @@ func setupBundle() int {
keepEnv := []string{}
for _, env := range spec.Process.Env {
switch {
case strings.HasPrefix(env, "_DAGGER_ENABLE_NESTING="):
case strings.HasPrefix(env, "_DAGGER_NESTED_CLIENT_ID="):
// keep the env var; we use it at runtime
keepEnv = append(keepEnv, env)

Expand Down Expand Up @@ -484,7 +483,9 @@ func setupBundle() int {
Options: []string{"rbind"},
Source: execMetadata.ProgSockPath,
})
case strings.HasPrefix(env, "_DAGGER_SERVER_ID="):
case strings.HasPrefix(env, "_DAGGER_ENGINE_VERSION="):
// don't need this at runtime, it is just for invalidating cache, which
// has already happened by now
case strings.HasPrefix(env, aliasPrefix):
// NB: don't keep this env var, it's only for the bundling step
// keepEnv = append(keepEnv, env)
Expand Down Expand Up @@ -611,7 +612,8 @@ func internalEnv(name string) (string, bool) {
}

func runWithNesting(ctx context.Context, cmd *exec.Cmd) error {
if _, found := internalEnv("_DAGGER_ENABLE_NESTING"); !found {
clientID, ok := internalEnv("_DAGGER_NESTED_CLIENT_ID")
if !ok {
// no nesting; run as normal
return execProcess(cmd, true)
}
Expand All @@ -629,27 +631,21 @@ func runWithNesting(ctx context.Context, cmd *exec.Cmd) error {
}
sessionPort := l.Addr().(*net.TCPAddr).Port

serverID, ok := internalEnv("_DAGGER_SERVER_ID")
if !ok {
return errors.New("missing nested client server ID")
}

parentClientIDsVal, _ := internalEnv("_DAGGER_PARENT_CLIENT_IDS")

clientParams := client.Params{
ID: clientID,
ServerID: serverID,
SecretToken: sessionToken.String(),
RunnerHost: "unix:///.runner.sock",
ParentClientIDs: strings.Fields(parentClientIDsVal),
}

if _, ok := internalEnv("_DAGGER_ENABLE_NESTING_IN_SAME_SESSION"); ok {
serverID, ok := internalEnv("_DAGGER_SERVER_ID")
if !ok {
return fmt.Errorf("missing _DAGGER_SERVER_ID")
}
clientParams.ServerID = serverID
}

moduleCallerDigest, ok := internalEnv("_DAGGER_MODULE_CALLER_DIGEST")
if ok {
clientParams.ModuleCallerDigest = digest.Digest(moduleCallerDigest)
}

progW, err := progrock.DialRPC(ctx, "unix:///.progrock.sock")
if err != nil {
return fmt.Errorf("error connecting to progrock: %w", err)
Expand Down
121 changes: 51 additions & 70 deletions core/container.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ type DefaultTerminalCmdOpts struct {

// Container is a content-addressed container.
type Container struct {
Query *Query

// The container's root filesystem.
FS *pb.Definition `json:"fs"`

Expand Down Expand Up @@ -135,12 +133,8 @@ func (container *Container) PBDefinitions(ctx context.Context) ([]*pb.Definition
return defs, nil
}

func NewContainer(root *Query, platform Platform) (*Container, error) {
if root == nil {
panic("query must be non-nil")
}
func NewContainer(platform Platform) (*Container, error) {
return &Container{
Query: root,
Platform: platform,
}, nil
}
Expand All @@ -167,7 +161,9 @@ var _ pipeline.Pipelineable = (*Container)(nil)

// PipelinePath returns the container's pipeline path.
func (container *Container) PipelinePath() pipeline.Path {
return container.Query.Pipeline
//TODO:
return nil
// return container.Query.Pipeline
}

// Ownership contains a UID/GID pair resolved from a user/group name or ID pair
Expand Down Expand Up @@ -281,7 +277,7 @@ func (mnts ContainerMounts) With(newMnt ContainerMount) ContainerMounts {
}

func (container *Container) From(ctx context.Context, addr string) (*Container, error) {
bk := container.Query.Buildkit
bk := CurrentQuery(ctx).Buildkit

container = container.Clone()

Expand Down Expand Up @@ -361,8 +357,8 @@ func (container *Container) Build(
// set image ref to empty string
container.ImageRef = ""

svcs := container.Query.Services
bk := container.Query.Buildkit
svcs := CurrentQuery(ctx).Services
bk := CurrentQuery(ctx).Buildkit

// add a weak group for the docker build vertices
ctx, subRecorder := progrock.WithGroup(ctx, "docker build", progrock.Weak())
Expand Down Expand Up @@ -399,13 +395,7 @@ func (container *Container) Build(
dockerui.DefaultLocalNameDockerfile: contextDir.LLB,
}

// FIXME: ew, this is a terrible way to pass this around
//nolint:staticcheck
solveCtx := context.WithValue(ctx, "secret-translator", func(name string) (string, error) {
return GetLocalSecretAccessor(ctx, container.Query, name)
})

res, err := bk.Solve(solveCtx, bkgw.SolveRequest{
res, err := bk.Solve(ctx, bkgw.SolveRequest{
Frontend: "dockerfile.v0",
FrontendOpt: opts,
FrontendInputs: inputs,
Expand Down Expand Up @@ -455,7 +445,7 @@ func (container *Container) Build(

func (container *Container) RootFS(ctx context.Context) (*Directory, error) {
return &Directory{
Query: container.Query,
Query: CurrentQuery(ctx),
LLB: container.FS,
Dir: "/",
Platform: container.Platform,
Expand Down Expand Up @@ -743,8 +733,8 @@ func (container *Container) Directory(ctx context.Context, dirPath string) (*Dir
return nil, err
}

svcs := container.Query.Services
bk := container.Query.Buildkit
svcs := CurrentQuery(ctx).Services
bk := CurrentQuery(ctx).Buildkit

// check that the directory actually exists so the user gets an error earlier
// rather than when the dir is used
Expand Down Expand Up @@ -811,7 +801,7 @@ func locatePath[T *File | *Directory](
}

return init(
container.Query,
CurrentQuery(ctx),
mnt.Source,
sub,
container.Platform,
Expand All @@ -822,7 +812,7 @@ func locatePath[T *File | *Directory](

// Not found in a mount
return init(
container.Query,
CurrentQuery(ctx),
container.FS,
containerPath,
container.Platform,
Expand Down Expand Up @@ -899,7 +889,7 @@ func (container *Container) chown(
return nil, "", err
}

ref, err := bkRef(ctx, container.Query.Buildkit, def.ToPB())
ref, err := bkRef(ctx, CurrentQuery(ctx).Buildkit, def.ToPB())
if err != nil {
return nil, "", err
}
Expand Down Expand Up @@ -978,7 +968,8 @@ func (container *Container) UpdateImageConfig(ctx context.Context, updateFn func

func (container *Container) WithPipeline(ctx context.Context, name, description string, labels []pipeline.Label) (*Container, error) {
container = container.Clone()
container.Query = container.Query.WithPipeline(name, description, labels)
// TODO:
// container.Query = container.Query.WithPipeline(name, description, labels)
return container, nil
}

Expand All @@ -999,7 +990,7 @@ func (container *Container) WithExec(ctx context.Context, opts ContainerExecOpts
mounts := container.Mounts
platform := container.Platform
if platform.OS == "" {
platform = container.Query.Platform
platform = CurrentQuery(ctx).Platform
}

args, err := container.command(opts)
Expand All @@ -1021,16 +1012,19 @@ func (container *Container) WithExec(ctx context.Context, opts ContainerExecOpts

// this allows executed containers to communicate back to this API
if opts.ExperimentalPrivilegedNesting {
// include the engine version so that these execs get invalidated if the engine/API change
runOpts = append(runOpts, llb.AddEnv("_DAGGER_ENABLE_NESTING", engine.Version))
}

if opts.ModuleCallerDigest != "" {
runOpts = append(runOpts, llb.AddEnv("_DAGGER_MODULE_CALLER_DIGEST", opts.ModuleCallerDigest.String()))
}

if opts.NestedInSameSession {
runOpts = append(runOpts, llb.AddEnv("_DAGGER_ENABLE_NESTING_IN_SAME_SESSION", ""))
newRoot := opts.NestedExecRoot
if newRoot == nil {
var err error
newRoot, err = CurrentQuery(ctx).NewRootForCurrentCall(ctx, nil)
if err != nil {
return nil, fmt.Errorf("register caller: %w", err)
}
}
runOpts = append(runOpts,
llb.AddEnv("_DAGGER_NESTED_CLIENT_ID", newRoot.ClientID),
// include the engine version so that these execs get invalidated if the engine/API change
llb.AddEnv("_DAGGER_ENGINE_VERSION", engine.Version),
)
}

metaSt, metaSourcePath := metaMount(opts.Stdin)
Expand Down Expand Up @@ -1071,13 +1065,7 @@ func (container *Container) WithExec(ctx context.Context, opts ContainerExecOpts
}

// don't pass these through to the container when manually set, they are internal only
if name == "_DAGGER_ENABLE_NESTING" && !opts.ExperimentalPrivilegedNesting {
continue
}
if name == "_DAGGER_MODULE_CALLER_DIGEST" && opts.ModuleCallerDigest == "" {
continue
}
if name == "_DAGGER_ENABLE_NESTING_IN_SAME_SESSION" && !opts.NestedInSameSession {
if name == "_DAGGER_NESTED_CLIENT_ID" && !opts.ExperimentalPrivilegedNesting {
continue
}

Expand All @@ -1094,7 +1082,7 @@ func (container *Container) WithExec(ctx context.Context, opts ContainerExecOpts

secretsToScrub := SecretToScrubInfo{}
for i, secret := range container.Secrets {
secretOpts := []llb.SecretOption{llb.SecretID(secret.Secret.Accessor)}
secretOpts := []llb.SecretOption{llb.SecretID(secret.Secret.Name)}

var secretDest string
switch {
Expand Down Expand Up @@ -1247,7 +1235,7 @@ func (container Container) Evaluate(ctx context.Context) (*buildkit.Result, erro
return nil, nil
}

root := container.Query
root := CurrentQuery(ctx)

detach, _, err := root.Services.StartBindings(ctx, container.Services)
if err != nil {
Expand Down Expand Up @@ -1281,7 +1269,7 @@ func (container *Container) MetaFileContents(ctx context.Context, filePath strin
}

file := NewFile(
container.Query,
CurrentQuery(ctx),
container.Meta,
path.Join(buildkit.MetaSourcePath, filePath),
container.Platform,
Expand Down Expand Up @@ -1351,8 +1339,8 @@ func (container *Container) Publish(
opts[string(exptypes.OptKeyForceCompression)] = strconv.FormatBool(true)
}

svcs := container.Query.Services
bk := container.Query.Buildkit
svcs := CurrentQuery(ctx).Services
bk := CurrentQuery(ctx).Buildkit

detach, _, err := svcs.StartBindings(ctx, services)
if err != nil {
Expand Down Expand Up @@ -1395,8 +1383,8 @@ func (container *Container) Export(
forcedCompression ImageLayerCompression,
mediaTypes ImageMediaTypes,
) error {
svcs := container.Query.Services
bk := container.Query.Buildkit
svcs := CurrentQuery(ctx).Services
bk := CurrentQuery(ctx).Buildkit

if mediaTypes == "" {
// Modern registry implementations support oci types and docker daemons
Expand Down Expand Up @@ -1462,9 +1450,9 @@ func (container *Container) AsTarball(
forcedCompression ImageLayerCompression,
mediaTypes ImageMediaTypes,
) (*File, error) {
bk := container.Query.Buildkit
svcs := container.Query.Services
engineHostPlatform := container.Query.Platform
bk := CurrentQuery(ctx).Buildkit
svcs := CurrentQuery(ctx).Services
engineHostPlatform := CurrentQuery(ctx).Platform

if mediaTypes == "" {
mediaTypes = OCIMediaTypes
Expand Down Expand Up @@ -1520,17 +1508,17 @@ func (container *Container) AsTarball(
if err != nil {
return nil, fmt.Errorf("container image to tarball file conversion failed: %w", err)
}
return NewFile(container.Query, pbDef, fileName, engineHostPlatform, nil), nil
return NewFile(CurrentQuery(ctx), pbDef, fileName, engineHostPlatform, nil), nil
}

func (container *Container) Import(
ctx context.Context,
source *File,
tag string,
) (*Container, error) {
bk := container.Query.Buildkit
store := container.Query.OCIStore
lm := container.Query.LeaseManager
bk := CurrentQuery(ctx).Buildkit
store := CurrentQuery(ctx).OCIStore
lm := CurrentQuery(ctx).LeaseManager

container = container.Clone()

Expand Down Expand Up @@ -1712,7 +1700,7 @@ func (container *Container) Service(ctx context.Context) (*Service, error) {
return nil, err
}
}
return container.Query.NewContainerService(container), nil
return CurrentQuery(ctx).NewContainerService(container), nil
}

func (container *Container) ownership(ctx context.Context, owner string) (*Ownership, error) {
Expand All @@ -1726,7 +1714,7 @@ func (container *Container) ownership(ctx context.Context, owner string) (*Owner
return nil, err
}

return resolveUIDGID(ctx, fsSt, container.Query.Buildkit, container.Platform, owner)
return resolveUIDGID(ctx, fsSt, CurrentQuery(ctx).Buildkit, container.Platform, owner)
}

func (container *Container) command(opts ContainerExecOpts) ([]string, error) {
Expand Down Expand Up @@ -1783,22 +1771,15 @@ type ContainerExecOpts struct {
// Redirect the command's standard error to a file in the container
RedirectStderr 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
// Provide the executed command access back to the Dagger API
ExperimentalPrivilegedNesting bool `default:"false"`

// Grant the process all root capabilities
InsecureRootCapabilities bool `default:"false"`

// (Internal-only) If this exec is for a module function, this digest will be set in the
// grpc context metadata for any api requests back to the engine. It's used by the API
// server to determine which schema to serve and other module context metadata.
ModuleCallerDigest digest.Digest `name:"-"`

// (Internal-only) Used for module function execs to trigger the nested api client to
// be connected back to the same session.
NestedInSameSession bool `name:"-"`
// (Internal-only) If this is a nested exec, use this as the root Query.
// Otherwise, nested execs will get a new root based on the current ID.
NestedExecRoot *Query `name:"-"`
}

type BuildArg struct {
Expand Down
Loading