From b0b9269ccc84cbc357bf7fc838597672643fa564 Mon Sep 17 00:00:00 2001 From: "Jonathan Leibiusky @xetorthio" Date: Sat, 27 May 2017 20:04:37 -0300 Subject: [PATCH 1/2] Allow to launch instances with any kind of public image. Images that are not whitelisted will be launched as normal containers. Only whitelisted ones will be launched as privileged. Additionally pull the image if it doesn't exist. --- docker/docker.go | 40 ++++++++++++++- pwd/docker_mock_test.go | 44 +++++++++++++++- pwd/instance.go | 24 ++++++--- pwd/instance_test.go | 111 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 208 insertions(+), 11 deletions(-) diff --git a/docker/docker.go b/docker/docker.go index 65a0f06..a619527 100644 --- a/docker/docker.go +++ b/docker/docker.go @@ -14,10 +14,12 @@ import ( "strings" "time" + "github.com/docker/distribution/reference" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/network" "github.com/docker/docker/client" + "github.com/docker/docker/pkg/jsonmessage" ) const ( @@ -190,6 +192,7 @@ type CreateContainerOpts struct { ServerCert []byte ServerKey []byte CACert []byte + Privileged bool } func (d *docker) CreateContainer(opts CreateContainerOpts) (string, error) { @@ -219,7 +222,7 @@ func (d *docker) CreateContainer(opts CreateContainerOpts) (string, error) { h := &container.HostConfig{ NetworkMode: container.NetworkMode(opts.SessionId), - Privileged: true, + Privileged: opts.Privileged, AutoRemove: true, LogConfig: container.LogConfig{Config: map[string]string{"max-size": "10m", "max-file": "1"}}, } @@ -257,7 +260,18 @@ func (d *docker) CreateContainer(opts CreateContainerOpts) (string, error) { container, err := d.c.ContainerCreate(context.Background(), cf, h, networkConf, opts.ContainerName) if err != nil { - return "", err + if client.IsErrImageNotFound(err) { + log.Printf("Unable to find image '%s' locally\n", opts.Image) + if err = d.pullImage(context.Background(), opts.Image); err != nil { + return "", err + } + container, err = d.c.ContainerCreate(context.Background(), cf, h, networkConf, opts.ContainerName) + if err != nil { + return "", err + } + } else { + return "", err + } } if err := d.copyIfSet(opts.ServerCert, "cert.pem", containerCertDir, opts.ContainerName); err != nil { @@ -283,6 +297,28 @@ func (d *docker) CreateContainer(opts CreateContainerOpts) (string, error) { return cinfo.NetworkSettings.Networks[opts.SessionId].IPAddress, nil } +func (d *docker) pullImage(ctx context.Context, image string) error { + _, err := reference.ParseNormalizedNamed(image) + if err != nil { + return err + } + + options := types.ImageCreateOptions{} + + responseBody, err := d.c.ImageCreate(ctx, image, options) + if err != nil { + return err + } + defer responseBody.Close() + + return jsonmessage.DisplayJSONMessagesStream( + responseBody, + os.Stderr, + os.Stdout.Fd(), + false, + nil) +} + func (d *docker) copyIfSet(content []byte, fileName, path, containerName string) error { if len(content) > 0 { return d.CopyToContainer(containerName, path, fileName, bytes.NewReader(content)) diff --git a/pwd/docker_mock_test.go b/pwd/docker_mock_test.go index 9abfbbf..c84d638 100644 --- a/pwd/docker_mock_test.go +++ b/pwd/docker_mock_test.go @@ -3,6 +3,7 @@ package pwd import ( "io" "net" + "time" "github.com/docker/docker/api/types" "github.com/play-with-docker/play-with-docker/docker" @@ -12,6 +13,7 @@ type mockDocker struct { createNetwork func(string) error connectNetwork func(container, network, ip string) (string, error) containerResize func(string, uint, uint) error + createContainer func(opts docker.CreateContainerOpts) (string, error) } func (m *mockDocker) CreateNetwork(id string) error { @@ -47,7 +49,7 @@ func (m *mockDocker) ContainerResize(name string, rows, cols uint) error { return nil } func (m *mockDocker) CreateAttachConnection(name string) (net.Conn, error) { - return nil, nil + return &mockConn{}, nil } func (m *mockDocker) CopyToContainer(containerName, destination, fileName string, content io.Reader) error { return nil @@ -56,7 +58,10 @@ func (m *mockDocker) DeleteContainer(id string) error { return nil } func (m *mockDocker) CreateContainer(opts docker.CreateContainerOpts) (string, error) { - return "", nil + if m.createContainer != nil { + return m.createContainer(opts) + } + return "10.0.0.1", nil } func (m *mockDocker) ExecAttach(instanceName string, command []string, out io.Writer) (int, error) { return 0, nil @@ -70,3 +75,38 @@ func (m *mockDocker) DeleteNetwork(id string) error { func (m *mockDocker) Exec(instanceName string, command []string) (int, error) { return 0, nil } + +type mockConn struct { +} + +func (m *mockConn) Read(b []byte) (n int, err error) { + return len(b), nil +} + +func (m *mockConn) Write(b []byte) (n int, err error) { + return len(b), nil +} + +func (m *mockConn) Close() error { + return nil +} + +func (m *mockConn) LocalAddr() net.Addr { + return &net.IPAddr{} +} + +func (m *mockConn) RemoteAddr() net.Addr { + return &net.IPAddr{} +} + +func (m *mockConn) SetDeadline(t time.Time) error { + return nil +} + +func (m *mockConn) SetReadDeadline(t time.Time) error { + return nil +} + +func (m *mockConn) SetWriteDeadline(t time.Time) error { + return nil +} diff --git a/pwd/instance.go b/pwd/instance.go index fcf41ba..81ac9ca 100644 --- a/pwd/instance.go +++ b/pwd/instance.go @@ -35,25 +35,26 @@ func (p UInt16Slice) Less(i, j int) bool { return p[i] < p[j] } func (p UInt16Slice) Swap(i, j int) { p[i], p[j] = p[j], p[i] } type Instance struct { - rw sync.Mutex - session *Session `json:"-"` + Image string `json:"image"` Name string `json:"name"` Hostname string `json:"hostname"` IP string `json:"ip"` - conn net.Conn `json:"-"` - ctx context.Context `json:"-"` - docker docker.DockerApi `json:"-"` IsManager *bool `json:"is_manager"` Mem string `json:"mem"` Cpu string `json:"cpu"` Alias string `json:"alias"` - tempPorts []uint16 `json:"-"` ServerCert []byte `json:"server_cert"` ServerKey []byte `json:"server_key"` CACert []byte `json:"ca_cert"` Cert []byte `json:"cert"` Key []byte `json:"key"` + session *Session `json:"-"` + conn net.Conn `json:"-"` + ctx context.Context `json:"-"` + docker docker.DockerApi `json:"-"` + tempPorts []uint16 `json:"-"` Ports UInt16Slice + rw sync.Mutex } type InstanceConfig struct { ImageName string @@ -200,7 +201,7 @@ func (p *pwd) InstanceNew(session *Session, conf InstanceConfig) (*Instance, err } opts := docker.CreateContainerOpts{ - Image: config.GetDindImageName(), + Image: conf.ImageName, SessionId: session.Id, PwdIpAddress: session.PwdIpAddress, ContainerName: containerName, @@ -208,6 +209,14 @@ func (p *pwd) InstanceNew(session *Session, conf InstanceConfig) (*Instance, err ServerCert: conf.ServerCert, ServerKey: conf.ServerKey, CACert: conf.CACert, + Privileged: false, + } + + for _, imageName := range p.InstanceAllowedImages() { + if conf.ImageName == imageName { + opts.Privileged = true + break + } } ip, err := p.docker.CreateContainer(opts) @@ -216,6 +225,7 @@ func (p *pwd) InstanceNew(session *Session, conf InstanceConfig) (*Instance, err } instance := &Instance{} + instance.Image = opts.Image instance.IP = ip instance.Name = containerName instance.Hostname = nodeName diff --git a/pwd/instance_test.go b/pwd/instance_test.go index 99dda16..4ff73ce 100644 --- a/pwd/instance_test.go +++ b/pwd/instance_test.go @@ -1,8 +1,12 @@ package pwd import ( + "fmt" "testing" + "time" + "github.com/play-with-docker/play-with-docker/config" + "github.com/play-with-docker/play-with-docker/docker" "github.com/stretchr/testify/assert" ) @@ -33,3 +37,110 @@ func TestInstanceResizeTerminal(t *testing.T) { assert.Equal(t, uint(24), resizedRows) assert.Equal(t, uint(80), resizedCols) } + +func TestInstanceNew(t *testing.T) { + containerOpts := docker.CreateContainerOpts{} + dock := &mockDocker{} + dock.createContainer = func(opts docker.CreateContainerOpts) (string, error) { + containerOpts = opts + return "10.0.0.1", nil + } + + tasks := &mockTasks{} + broadcast := &mockBroadcast{} + storage := &mockStorage{} + + p := NewPWD(dock, tasks, broadcast, storage) + + session, err := p.SessionNew(time.Hour, "", "") + + assert.Nil(t, err) + + instance, err := p.InstanceNew(session, InstanceConfig{}) + + assert.Nil(t, err) + + expectedInstance := Instance{ + Name: fmt.Sprintf("%s_node1", session.Id[:8]), + Hostname: "node1", + IP: "10.0.0.1", + Alias: "", + Image: config.GetDindImageName(), + session: session, + } + + assert.Equal(t, expectedInstance, *instance) + + expectedContainerOpts := docker.CreateContainerOpts{ + Image: expectedInstance.Image, + SessionId: session.Id, + PwdIpAddress: session.PwdIpAddress, + ContainerName: expectedInstance.Name, + Hostname: expectedInstance.Hostname, + ServerCert: nil, + ServerKey: nil, + CACert: nil, + Privileged: true, + } + assert.Equal(t, expectedContainerOpts, containerOpts) +} + +func TestInstanceNew_WithNotAllowedImage(t *testing.T) { + containerOpts := docker.CreateContainerOpts{} + dock := &mockDocker{} + dock.createContainer = func(opts docker.CreateContainerOpts) (string, error) { + containerOpts = opts + return "10.0.0.1", nil + } + + tasks := &mockTasks{} + broadcast := &mockBroadcast{} + storage := &mockStorage{} + + p := NewPWD(dock, tasks, broadcast, storage) + + session, err := p.SessionNew(time.Hour, "", "") + + assert.Nil(t, err) + + instance, err := p.InstanceNew(session, InstanceConfig{ImageName: "redis"}) + + assert.Nil(t, err) + + expectedInstance := Instance{ + Name: fmt.Sprintf("%s_node1", session.Id[:8]), + Hostname: "node1", + IP: "10.0.0.1", + Alias: "", + Image: "redis", + session: session, + } + + assert.Equal(t, expectedInstance, *instance) + + expectedContainerOpts := docker.CreateContainerOpts{ + Image: expectedInstance.Image, + SessionId: session.Id, + PwdIpAddress: session.PwdIpAddress, + ContainerName: expectedInstance.Name, + Hostname: expectedInstance.Hostname, + ServerCert: nil, + ServerKey: nil, + CACert: nil, + Privileged: false, + } + assert.Equal(t, expectedContainerOpts, containerOpts) +} + +func TestInstanceAllowedImages(t *testing.T) { + dock := &mockDocker{} + tasks := &mockTasks{} + broadcast := &mockBroadcast{} + storage := &mockStorage{} + + p := NewPWD(dock, tasks, broadcast, storage) + + expectedImages := []string{config.GetDindImageName(), "franela/dind:overlay2-dev"} + + assert.Equal(t, expectedImages, p.InstanceAllowedImages()) +} From 2e63e541f3a6f02d999e831a9820352cc5ed880c Mon Sep 17 00:00:00 2001 From: "Jonathan Leibiusky @xetorthio" Date: Mon, 29 May 2017 10:19:01 -0300 Subject: [PATCH 2/2] Make sure not to treat the instance as a docker host always, as it might not be one. --- pwd/check_swarm_status_task.go | 3 +++ pwd/check_swarm_used_ports.go | 3 +++ pwd/check_used_ports_task.go | 3 +++ pwd/instance.go | 43 ++++++++++++++++++---------------- pwd/instance_test.go | 26 ++++++++++---------- pwd/tasks.go | 2 +- 6 files changed, 47 insertions(+), 33 deletions(-) diff --git a/pwd/check_swarm_status_task.go b/pwd/check_swarm_status_task.go index 297a36b..3019a27 100644 --- a/pwd/check_swarm_status_task.go +++ b/pwd/check_swarm_status_task.go @@ -10,6 +10,9 @@ type checkSwarmStatusTask struct { } func (c checkSwarmStatusTask) Run(i *Instance) error { + if i.docker == nil { + return nil + } if info, err := i.docker.GetDaemonInfo(); err == nil { if info.Swarm.LocalNodeState != swarm.LocalNodeStateInactive && info.Swarm.LocalNodeState != swarm.LocalNodeStateLocked { i.IsManager = &info.Swarm.ControlAvailable diff --git a/pwd/check_swarm_used_ports.go b/pwd/check_swarm_used_ports.go index c01025b..7157240 100644 --- a/pwd/check_swarm_used_ports.go +++ b/pwd/check_swarm_used_ports.go @@ -9,6 +9,9 @@ type checkSwarmUsedPortsTask struct { } func (c checkSwarmUsedPortsTask) Run(i *Instance) error { + if i.docker == nil { + return nil + } if i.IsManager != nil && *i.IsManager { sessionPrefix := i.session.Id[:8] // This is a swarm manager instance, then check for ports diff --git a/pwd/check_used_ports_task.go b/pwd/check_used_ports_task.go index deae46a..74e3f3b 100644 --- a/pwd/check_used_ports_task.go +++ b/pwd/check_used_ports_task.go @@ -6,6 +6,9 @@ type checkUsedPortsTask struct { } func (c checkUsedPortsTask) Run(i *Instance) error { + if i.docker == nil { + return nil + } if ports, err := i.docker.GetPorts(); err == nil { for _, p := range ports { i.setUsedPort(uint16(p)) diff --git a/pwd/instance.go b/pwd/instance.go index 81ac9ca..a9ec36a 100644 --- a/pwd/instance.go +++ b/pwd/instance.go @@ -35,26 +35,27 @@ func (p UInt16Slice) Less(i, j int) bool { return p[i] < p[j] } func (p UInt16Slice) Swap(i, j int) { p[i], p[j] = p[j], p[i] } type Instance struct { - Image string `json:"image"` - Name string `json:"name"` - Hostname string `json:"hostname"` - IP string `json:"ip"` - IsManager *bool `json:"is_manager"` - Mem string `json:"mem"` - Cpu string `json:"cpu"` - Alias string `json:"alias"` - ServerCert []byte `json:"server_cert"` - ServerKey []byte `json:"server_key"` - CACert []byte `json:"ca_cert"` - Cert []byte `json:"cert"` - Key []byte `json:"key"` - session *Session `json:"-"` - conn net.Conn `json:"-"` - ctx context.Context `json:"-"` - docker docker.DockerApi `json:"-"` - tempPorts []uint16 `json:"-"` - Ports UInt16Slice - rw sync.Mutex + Image string `json:"image"` + Name string `json:"name"` + Hostname string `json:"hostname"` + IP string `json:"ip"` + IsManager *bool `json:"is_manager"` + Mem string `json:"mem"` + Cpu string `json:"cpu"` + Alias string `json:"alias"` + ServerCert []byte `json:"server_cert"` + ServerKey []byte `json:"server_key"` + CACert []byte `json:"ca_cert"` + Cert []byte `json:"cert"` + Key []byte `json:"key"` + IsDockerHost bool `json:"is_docker_host"` + session *Session `json:"-"` + conn net.Conn `json:"-"` + ctx context.Context `json:"-"` + docker docker.DockerApi `json:"-"` + tempPorts []uint16 `json:"-"` + Ports UInt16Slice + rw sync.Mutex } type InstanceConfig struct { ImageName string @@ -236,6 +237,8 @@ func (p *pwd) InstanceNew(session *Session, conf InstanceConfig) (*Instance, err instance.ServerKey = conf.ServerKey instance.CACert = conf.CACert instance.session = session + // For now this condition holds through. In the future we might need a more complex logic. + instance.IsDockerHost = opts.Privileged if session.Instances == nil { session.Instances = make(map[string]*Instance) diff --git a/pwd/instance_test.go b/pwd/instance_test.go index 4ff73ce..15a567a 100644 --- a/pwd/instance_test.go +++ b/pwd/instance_test.go @@ -61,12 +61,13 @@ func TestInstanceNew(t *testing.T) { assert.Nil(t, err) expectedInstance := Instance{ - Name: fmt.Sprintf("%s_node1", session.Id[:8]), - Hostname: "node1", - IP: "10.0.0.1", - Alias: "", - Image: config.GetDindImageName(), - session: session, + Name: fmt.Sprintf("%s_node1", session.Id[:8]), + Hostname: "node1", + IP: "10.0.0.1", + Alias: "", + Image: config.GetDindImageName(), + IsDockerHost: true, + session: session, } assert.Equal(t, expectedInstance, *instance) @@ -108,12 +109,13 @@ func TestInstanceNew_WithNotAllowedImage(t *testing.T) { assert.Nil(t, err) expectedInstance := Instance{ - Name: fmt.Sprintf("%s_node1", session.Id[:8]), - Hostname: "node1", - IP: "10.0.0.1", - Alias: "", - Image: "redis", - session: session, + Name: fmt.Sprintf("%s_node1", session.Id[:8]), + Hostname: "node1", + IP: "10.0.0.1", + Alias: "", + Image: "redis", + IsDockerHost: false, + session: session, } assert.Equal(t, expectedInstance, *instance) diff --git a/pwd/tasks.go b/pwd/tasks.go index 9a6071e..2b8ae57 100644 --- a/pwd/tasks.go +++ b/pwd/tasks.go @@ -45,7 +45,7 @@ func (sch *scheduler) Schedule(s *Session) { wg.Add(len(s.Instances)) for _, ins := range s.Instances { var i *Instance = ins - if i.docker == nil { + if i.docker == nil && i.IsDockerHost { // Need to create client to the DinD docker daemon // We check if the client needs to use TLS