Skip to content

Commit d2a6e50

Browse files
committed
fix(assemble): handle root for each entry
`assemble` command cannot be executed as root. Instead, each item can either be processed by a rootful or rootless container manager.
1 parent 5ddceca commit d2a6e50

9 files changed

Lines changed: 248 additions & 65 deletions

File tree

internal/cli/assemble.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99

1010
"github.com/urfave/cli/v3"
1111

12+
"github.com/89luca89/distrobox/internal/rootful"
1213
"github.com/89luca89/distrobox/pkg/commands"
1314
"github.com/89luca89/distrobox/pkg/config"
1415
"github.com/89luca89/distrobox/pkg/containermanager"
@@ -98,6 +99,16 @@ func assembleAction(ctx context.Context, cmd *cli.Command, cfg *config.Values, d
9899
return fmt.Errorf("failed to parse manifest file: %w", err)
99100
}
100101

102+
// if at least one item in the manifest requires root, validate sudo before proceeding
103+
for _, item := range manifest {
104+
if item.Root {
105+
if err := rootful.Validate(ctx); err != nil {
106+
return fmt.Errorf("cannot run in root mode: %w", err)
107+
}
108+
break
109+
}
110+
}
111+
101112
opts := commands.AssembleOptions{
102113
Items: manifest,
103114
Boxname: cmd.String("name"),

internal/cli/root.go

Lines changed: 54 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -56,53 +56,60 @@ func printInvalidContainerManager(p *ui.Printer, containerManagerType string) {
5656
}
5757

5858
func subcommands(cfg *config.Values) []*cli.Command {
59-
return []*cli.Command{
60-
composeCommand(
61-
newListCommand,
62-
withRoot,
63-
withContainerManager,
64-
)(cfg),
65-
composeCommand(
66-
newGenerateEntryCommand,
67-
withRoot,
68-
withContainerManager,
69-
)(cfg),
70-
composeCommand(
71-
newCreateCommand,
72-
withRoot,
73-
withContainerManager,
74-
)(cfg),
75-
composeCommand(
76-
newEnterCommand,
77-
withRoot,
78-
withContainerManager,
79-
)(cfg),
80-
composeCommand(
81-
newAssembleCommand,
82-
withRootSupport,
83-
withContainerManager,
84-
)(cfg),
85-
composeCommand(
86-
newRmCommand,
87-
withRoot,
88-
withContainerManager,
89-
)(cfg),
90-
composeCommand(
91-
newStopCommand,
92-
withRoot,
93-
withContainerManager,
94-
)(cfg),
95-
composeCommand(
96-
newEphemeralCommand,
97-
withRoot,
98-
withContainerManager,
99-
)(cfg),
100-
composeCommand(newUpgradeCommand,
101-
withRoot,
102-
withContainerManager,
103-
)(cfg),
104-
}
105-
}
59+
cc := &CommandComposer[config.Values]{cfg: cfg}
60+
61+
list := cc.apply(
62+
newListCommand,
63+
withRoot,
64+
withContainerManager,
65+
)
66+
67+
generateEntry := cc.apply(
68+
newGenerateEntryCommand,
69+
withRoot,
70+
withContainerManager,
71+
)
72+
73+
create := cc.apply(
74+
newCreateCommand,
75+
withRoot,
76+
withContainerManager,
77+
)
78+
79+
enter := cc.apply(
80+
newEnterCommand,
81+
withRoot,
82+
withContainerManager,
83+
)
84+
85+
assemble := cc.apply(
86+
newAssembleCommand,
87+
withContainerManager,
88+
)
89+
90+
rm := cc.apply(
91+
newRmCommand,
92+
withRoot,
93+
withContainerManager,
94+
)
95+
96+
stop := cc.apply(
97+
newStopCommand,
98+
withRoot,
99+
withContainerManager,
100+
)
101+
102+
ephemeral := cc.apply(
103+
newEphemeralCommand,
104+
withRoot,
105+
withContainerManager,
106+
)
107+
108+
upgrade := cc.apply(
109+
newUpgradeCommand,
110+
withRoot,
111+
withContainerManager,
112+
)
106113

107114
return []*cli.Command{
108115
list,

pkg/commands/assemble.go

Lines changed: 35 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,14 @@ type AssembleOptions struct {
2828
}
2929

3030
type AssembleCommand struct {
31-
cfg *config.Values
32-
containermanager containermanager.ContainerManager
33-
createCmd *CreateCommand
34-
rmCmd *RmCommand
35-
enterCmd *EnterCommand
36-
progress *ui.Progress
31+
cfg *config.Values
32+
createCmd *CreateCommand
33+
rmCmd *RmCommand
34+
enterCmd *EnterCommand
35+
createCmdRoot *CreateCommand
36+
rmCmdRoot *RmCommand
37+
enterCmdRoot *EnterCommand
38+
progress *ui.Progress
3739
}
3840

3941
func NewAssembleCommand(
@@ -43,13 +45,16 @@ func NewAssembleCommand(
4345
progress *ui.Progress,
4446
printer *ui.Printer,
4547
) *AssembleCommand {
48+
cmRoot := cm.CloneAsRoot()
4649
return &AssembleCommand{
47-
cfg: cfg,
48-
containermanager: cm,
49-
createCmd: NewCreateCommand(cfg, cm, ui.NewDevNullProgress(), prompter),
50-
rmCmd: NewRmCommand(cfg, cm, prompter),
51-
enterCmd: NewEnterCommand(cfg, cm, progress, printer),
52-
progress: progress,
50+
cfg: cfg,
51+
createCmd: NewCreateCommand(cfg, cm, ui.NewDevNullProgress(), prompter),
52+
rmCmd: NewRmCommand(cfg, cm, prompter),
53+
enterCmd: NewEnterCommand(cfg, cm, progress, printer),
54+
createCmdRoot: NewCreateCommand(cfg, cmRoot, ui.NewDevNullProgress(), prompter),
55+
rmCmdRoot: NewRmCommand(cfg, cmRoot, prompter),
56+
enterCmdRoot: NewEnterCommand(cfg, cmRoot, progress, printer),
57+
progress: progress,
5358
}
5459
}
5560

@@ -97,7 +102,12 @@ func (ac *AssembleCommand) deleteItem(ctx context.Context, item manifest.Item, d
97102
ContainerNames: []string{item.Name},
98103
}
99104

100-
_, err := ac.rmCmd.Execute(ctx, opts)
105+
rmCmd := ac.rmCmd
106+
if item.Root {
107+
rmCmd = ac.rmCmdRoot
108+
}
109+
110+
_, err := rmCmd.Execute(ctx, opts)
101111
if err != nil {
102112
ac.progress.Fail()
103113
return fmt.Errorf("failed to execute delete item '%s': %w", item.Name, err)
@@ -142,7 +152,11 @@ func (ac *AssembleCommand) createItem(ctx context.Context, item manifest.Item, d
142152
ContainerAlwaysPull: item.AlwaysPull,
143153
}
144154

145-
_, err := ac.createCmd.Execute(ctx, opts)
155+
createCmd := ac.createCmd
156+
if item.Root {
157+
createCmd = ac.createCmdRoot
158+
}
159+
_, err := createCmd.Execute(ctx, opts)
146160
if err != nil {
147161
ac.progress.Fail()
148162
return err
@@ -183,8 +197,12 @@ func (ac *AssembleCommand) joinHooks(hooks []string) string {
183197
}
184198

185199
func (ac *AssembleCommand) setupBox(ctx context.Context, item manifest.Item) error {
200+
enterCmd := ac.enterCmd
201+
if item.Root {
202+
enterCmd = ac.enterCmdRoot
203+
}
186204
if item.StartNow {
187-
_, err := ac.enterCmd.Execute(ctx, EnterOptions{
205+
_, err := enterCmd.Execute(ctx, EnterOptions{
188206
ContainerName: item.Name,
189207
NoTTY: true,
190208
CustomCommand: []string{"true"}, // we just want to run the init hooks, so we can skip the shell
@@ -203,7 +221,7 @@ func (ac *AssembleCommand) setupBox(ctx context.Context, item manifest.Item) err
203221
}
204222
}
205223
for _, app := range item.ExportedApps {
206-
_, err := ac.enterCmd.Execute(ctx, EnterOptions{
224+
_, err := enterCmd.Execute(ctx, EnterOptions{
207225
ContainerName: item.Name,
208226
NoTTY: true,
209227
CustomCommand: []string{"distrobox-export", "--app", app},
@@ -226,7 +244,7 @@ func (ac *AssembleCommand) setupBox(ctx context.Context, item manifest.Item) err
226244
}
227245
}
228246
for _, bin := range item.ExportedBins {
229-
_, err := ac.enterCmd.Execute(ctx, EnterOptions{
247+
_, err := enterCmd.Execute(ctx, EnterOptions{
230248
ContainerName: item.Name,
231249
NoTTY: true,
232250
CustomCommand: []string{"distrobox-export", "--bin", bin, "--export-path", item.ExportedBinsPath},

pkg/commands/assemble_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,53 @@ func TestAssembleCommand_SetupBox_ExportedBins_Invalid(t *testing.T) {
190190
}
191191
}
192192

193+
func TestAssembleCommand_RoutesCreateAndSetupByItemRoot(t *testing.T) {
194+
mock := &testutil.MockContainerManager{}
195+
cmd := newTestAssembleCommand(mock)
196+
197+
require.NotNil(t, mock.RootClone, "AssembleCommand constructor should call CloneAsRoot")
198+
199+
err := cmd.Execute(context.Background(), commands.AssembleOptions{
200+
Items: []manifest.Item{
201+
{Name: "rootless-box", Image: "ubuntu:latest", Root: false, StartNow: true},
202+
{Name: "rootful-box", Image: "ubuntu:latest", Root: true, StartNow: true},
203+
},
204+
})
205+
require.NoError(t, err)
206+
207+
require.Len(t, mock.Spy.Create, 1, "rootless mock should receive exactly one Create")
208+
createOpts := mock.Spy.Create[0][0].(containermanager.CreateOptions)
209+
assert.Equal(t, "rootless-box", createOpts.ContainerName)
210+
211+
require.Len(t, mock.RootClone.Spy.Create, 1, "root mock should receive exactly one Create")
212+
createOptsRoot := mock.RootClone.Spy.Create[0][0].(containermanager.CreateOptions)
213+
assert.Equal(t, "rootful-box", createOptsRoot.ContainerName)
214+
215+
require.Len(t, mock.Spy.Enter, 1, "rootless mock should receive exactly one Enter (StartNow)")
216+
assert.Equal(t, "rootless-box", getEnterOptions(mock.Spy, 0).ContainerName)
217+
218+
require.Len(t, mock.RootClone.Spy.Enter, 1, "root mock should receive exactly one Enter (StartNow)")
219+
assert.Equal(t, "rootful-box", getEnterOptions(mock.RootClone.Spy, 0).ContainerName)
220+
}
221+
222+
func TestAssembleCommand_RootlessOnlyManifest_DoesNotInvokeRootMock(t *testing.T) {
223+
mock := &testutil.MockContainerManager{}
224+
cmd := newTestAssembleCommand(mock)
225+
226+
err := cmd.Execute(context.Background(), commands.AssembleOptions{
227+
Items: []manifest.Item{
228+
{Name: "a", Image: "ubuntu:latest", Root: false},
229+
{Name: "b", Image: "ubuntu:latest", Root: false},
230+
},
231+
})
232+
require.NoError(t, err)
233+
234+
assert.Len(t, mock.Spy.Create, 2)
235+
require.NotNil(t, mock.RootClone, "constructor still calls CloneAsRoot eagerly")
236+
assert.Empty(t, mock.RootClone.Spy.Create, "root mock should not receive Create for rootless items")
237+
assert.Empty(t, mock.RootClone.Spy.Enter, "root mock should not receive Enter for rootless items")
238+
}
239+
193240
func TestAssembleCommand_SetupBox_ExportedBins_InvalidExportPath(t *testing.T) {
194241
invalidPaths := []string{
195242
"",

pkg/containermanager/containermanager.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,9 @@ type ContainerManagerType string
8686

8787
type ContainerManager interface {
8888
Name() string
89+
// CloneAsRoot returns a copy of the manager configured to run in root
90+
// mode. The original instance is not modified.
91+
CloneAsRoot() ContainerManager
8992
Enter(ctx context.Context, options EnterOptions, progress *ui.Progress, printer *ui.Printer) error
9093
ListContainers(ctx context.Context) ([]Container, error)
9194
Create(ctx context.Context, opts CreateOptions) error
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
package providers
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
"github.com/stretchr/testify/require"
8+
)
9+
10+
func TestPodman_CloneAsRoot_PreservesFieldsAndFlipsRoot(t *testing.T) {
11+
original := newPodman(podmanCommandPodman, false, "doas", true)
12+
13+
cloned := original.CloneAsRoot()
14+
15+
require.NotSame(t, original, cloned, "expected a fresh instance")
16+
17+
clone, ok := cloned.(*Podman)
18+
require.True(t, ok, "expected *Podman, got %T", cloned)
19+
20+
assert.True(t, clone.root, "clone should be in root mode")
21+
assert.Equal(t, original.command, clone.command)
22+
assert.Equal(t, original.sudoCommand, clone.sudoCommand)
23+
assert.Equal(t, original.verbose, clone.verbose)
24+
25+
assert.False(t, original.root, "original should remain non-root")
26+
}
27+
28+
func TestPodman_CloneAsRoot_AlreadyRootStillReturnsCopy(t *testing.T) {
29+
original := newPodman(podmanCommandLauncher, true, "sudo", false)
30+
31+
cloned := original.CloneAsRoot()
32+
33+
require.NotSame(t, original, cloned)
34+
35+
clone, ok := cloned.(*Podman)
36+
require.True(t, ok)
37+
38+
assert.True(t, clone.root)
39+
assert.Equal(t, podmanCommandLauncher, clone.command)
40+
}
41+
42+
func TestDocker_CloneAsRoot_PreservesFieldsAndFlipsRoot(t *testing.T) {
43+
original := NewDocker(false, "doas", true)
44+
45+
cloned := original.CloneAsRoot()
46+
47+
require.NotSame(t, original, cloned, "expected a fresh instance")
48+
49+
clone, ok := cloned.(*Docker)
50+
require.True(t, ok, "expected *Docker, got %T", cloned)
51+
52+
assert.True(t, clone.root, "clone should be in root mode")
53+
assert.Equal(t, original.sudoCommand, clone.sudoCommand)
54+
assert.Equal(t, original.verbose, clone.verbose)
55+
56+
assert.False(t, original.root, "original should remain non-root")
57+
}
58+
59+
func TestDocker_CloneAsRoot_AlreadyRootStillReturnsCopy(t *testing.T) {
60+
original := NewDocker(true, "sudo", false)
61+
62+
cloned := original.CloneAsRoot()
63+
64+
require.NotSame(t, original, cloned)
65+
66+
clone, ok := cloned.(*Docker)
67+
require.True(t, ok)
68+
69+
assert.True(t, clone.root)
70+
}

pkg/containermanager/providers/docker.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,12 @@ func NewDocker(root bool, sudoCommand string, verbose bool) *Docker {
3636
}
3737
}
3838

39+
func (d *Docker) CloneAsRoot() containermanager.ContainerManager {
40+
cp := *d
41+
cp.root = true
42+
return &cp
43+
}
44+
3945
func (d *Docker) Name() string {
4046
return "docker"
4147
}

pkg/containermanager/providers/podman.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,12 @@ func NewPodmanLauncher(root bool, sudoCommand string, verbose bool) *Podman {
5454
return newPodman(podmanCommandLauncher, root, sudoCommand, verbose)
5555
}
5656

57+
func (p *Podman) CloneAsRoot() containermanager.ContainerManager {
58+
cp := *p
59+
cp.root = true
60+
return &cp
61+
}
62+
5763
func (p *Podman) Name() string {
5864
return string(p.command)
5965
}

0 commit comments

Comments
 (0)