Merge pull request #89 from coderbirju/update-container-create-8

feat: Add devices option

Author: coderbirju

api/handlers/container/create.go

@@ -187,6 +187,17 @@ func (h *handler) create(w http.ResponseWriter, r *http.Request) {
 	if req.HostConfig.SecurityOpt != nil {
 		securityOpt = req.HostConfig.SecurityOpt
 	}
+	devices := []string{}
+	if req.HostConfig.Devices != nil {
+		// Validate device configurations
+		for _, device := range req.HostConfig.Devices {
+			if device.PathOnHost == "" {
+				response.JSON(w, http.StatusBadRequest, response.NewErrorFromMsg("invalid device configuration: PathOnHost cannot be empty"))
+				return
+			}
+		}
+		devices = translateDevices(req.HostConfig.Devices)
+	}
 
 	globalOpt := ncTypes.GlobalCommandOptions(*h.Config)
 	createOpt := ncTypes.ContainerCreateOptions{
@@ -237,6 +248,8 @@ func (h *handler) create(w http.ResponseWriter, r *http.Request) {
 		BlkioDeviceWriteIOps: throttleDevicesToStrings(req.HostConfig.BlkioDeviceWriteIOps),
 		IPC:                  req.HostConfig.IpcMode, // IPC namespace to use
 		ShmSize:              shmSize,
+		Device:               devices, // Device specifies add a host device to the container
+
 		// #endregion
 
 		// #region for user flags
@@ -430,3 +443,27 @@ func translateAnnotations(annotations map[string]string) []string {
 	}
 	return result
 }
+
+// translateDevices converts a slice of DeviceMapping to a slice of strings in the format "PATH_ON_HOST[:PATH_IN_CONTAINER][:CGROUP_PERMISSIONS]".
+func translateDevices(devices []types.DeviceMapping) []string {
+	if devices == nil {
+		return nil
+	}
+
+	var result []string
+	for _, deviceMap := range devices {
+		deviceString := deviceMap.PathOnHost
+
+		if deviceMap.PathInContainer != "" {
+			deviceString += ":" + deviceMap.PathInContainer
+			if deviceMap.CgroupPermissions != "" {
+				deviceString += ":" + deviceMap.CgroupPermissions
+			}
+		} else if deviceMap.CgroupPermissions != "" {
+			deviceString += ":" + deviceMap.CgroupPermissions
+		}
+
+		result = append(result, deviceString)
+	}
+	return result
+}

api/handlers/container/create_test.go

@@ -974,6 +974,42 @@ var _ = Describe("Container Create API ", func() {
 			Expect(rr.Body).Should(MatchJSON(jsonResponse))
 		})
 
+		It("should set Devices option", func() {
+			body := []byte(`{
+				"Image": "test-image",
+				"HostConfig": {
+					"Devices": [{"PathOnHost": "/dev/null", "PathInContainer": "/dev/null", "CgroupPermissions": "rwm"},{"PathOnHost": "/var/lib", "CgroupPermissions": "ro"}]
+				}
+			}`)
+			req, _ := http.NewRequest(http.MethodPost, "/containers/create", bytes.NewReader(body))
+
+			// expected create options
+			createOpt.Device = []string{"/dev/null:/dev/null:rwm", "/var/lib:ro"}
+
+			service.EXPECT().Create(gomock.Any(), "test-image", nil, equalTo(createOpt), equalTo(netOpt)).Return(
+				cid, nil)
+
+			// handler should return success message with 201 status code.
+			h.create(rr, req)
+			Expect(rr).Should(HaveHTTPStatus(http.StatusCreated))
+			Expect(rr.Body).Should(MatchJSON(jsonResponse))
+		})
+
+		It("should return 400 for invalid Devices configuration", func() {
+			body := []byte(`{
+				"Image": "test-image",
+				"HostConfig": {
+					"Devices": [{"PathOnHost": "", "PathInContainer": "/dev/null", "CgroupPermissions": "rwm"}]
+				}
+			}`)
+			req, _ := http.NewRequest(http.MethodPost, "/containers/create", bytes.NewReader(body))
+
+			// handler should return bad request message with 400 status code
+			h.create(rr, req)
+			Expect(rr).Should(HaveHTTPStatus(http.StatusBadRequest))
+			Expect(rr.Body.String()).Should(ContainSubstring("invalid device configuration"))
+		})
+
 		Context("translate port mappings", func() {
 			It("should return empty if port mappings is nil", func() {
 				Expect(translatePortMappings(nil)).Should(BeEmpty())
@@ -1170,6 +1206,7 @@ func getDefaultCreateOpt(conf config.Config) types.ContainerCreateOptions {
 		BlkioDeviceWriteBps:  []string{},
 		BlkioDeviceReadIOps:  []string{},
 		BlkioDeviceWriteIOps: []string{},
+		Device:               []string{},
 		// #endregion
 
 		// #region for user flags

api/types/container_types.go

@@ -121,8 +121,8 @@ type ContainerHostConfig struct {
 	BlkioDeviceWriteBps  []*blkiodev.ThrottleDevice
 	BlkioDeviceReadIOps  []*blkiodev.ThrottleDevice
 	BlkioDeviceWriteIOps []*blkiodev.ThrottleDevice
-	// TODO: Devices     []DeviceMapping // List of devices to map inside the container
-	PidsLimit int64 // Setting PIDs limit for a container; Set `0` or `-1` for unlimited, or `null` to not change.
+	Devices              []DeviceMapping // List of devices to map inside the container
+	PidsLimit            int64           // Setting PIDs limit for a container; Set `0` or `-1` for unlimited, or `null` to not change.
 	// Mounts specs used by the container
 	// TODO: Mounts []mount.Mount `json:",omitempty"`
 
@@ -271,3 +271,9 @@ type StatsJSON struct {
 }
 
 type Ulimit = units.Ulimit
+
+type DeviceMapping struct {
+	PathOnHost        string
+	PathInContainer   string
+	CgroupPermissions string
+}

e2e/e2e_test.go

@@ -59,7 +59,7 @@ func TestRun(t *testing.T) {
 	const description = "Finch Daemon Functional test"
 	ginkgo.Describe(description, func() {
 		// functional test for container APIs
-		tests.ContainerCreate(opt)
+		tests.ContainerCreate(opt, pOpt)
 		tests.ContainerStart(opt)
 		tests.ContainerStop(opt)
 		tests.ContainerRestart(opt)

e2e/tests/container_create.go

@@ -11,6 +11,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"time"
 
@@ -26,6 +27,7 @@ import (
 
 	"github.com/runfinch/finch-daemon/api/types"
 	"github.com/runfinch/finch-daemon/e2e/client"
+	"github.com/runfinch/finch-daemon/e2e/util"
 )
 
 type containerCreateResponse struct {
@@ -34,7 +36,7 @@ type containerCreateResponse struct {
 }
 
 // ContainerCreate tests the `POST containers/create` API.
-func ContainerCreate(opt *option.Option) {
+func ContainerCreate(opt *option.Option, pOpt util.NewOpt) {
 	Describe("create container", func() {
 		var (
 			uClient *http.Client
@@ -1412,6 +1414,113 @@ func ContainerCreate(opt *option.Option) {
 			Expect(ok).Should(BeTrue())
 			Expect(annotations["com.example.key"]).Should(Equal("test-value"))
 		})
+
+		It("should create a container with device mappings", func() {
+			// Create a temporary file to use as backing store
+			tmpFileOpt, _ := pOpt([]string{"touch", "/tmp/loopdev"})
+			command.Run(tmpFileOpt)
+			defer func() {
+				rmOpt, _ := pOpt([]string{"rm", "-f", "/tmp/loopdev"})
+				command.Run(rmOpt)
+			}()
+
+			// Write 4KB of data to the file
+			ddOpt, _ := pOpt([]string{"dd", "if=/dev/zero", "of=/tmp/loopdev", "bs=4096", "count=1"})
+			command.Run(ddOpt)
+
+			// Set up loop device
+			loopDevOpt, _ := pOpt([]string{"losetup", "-f", "--show", "/tmp/loopdev"})
+			loopDev := command.StdoutStr(loopDevOpt)
+			Expect(loopDev).ShouldNot(BeEmpty())
+			defer func() {
+				detachOpt, _ := pOpt([]string{"losetup", "-d", loopDev})
+				command.Run(detachOpt)
+			}()
+
+			// Write test content to the device
+			writeOpt, _ := pOpt([]string{"sh", "-c", "echo -n test-content > " + loopDev})
+			command.Run(writeOpt)
+
+			// Get device info to verify major/minor numbers
+			statOpt, _ := pOpt([]string{"stat", "-c", "%t,%T", loopDev})
+			devNums := command.StdoutStr(statOpt)
+			parts := strings.Split(devNums, ",")
+			major, _ := strconv.ParseUint(parts[0], 16, 64)
+			minor, _ := strconv.ParseUint(parts[1], 16, 64)
+
+			options.Cmd = []string{"sleep", "Infinity"}
+			options.HostConfig.Devices = []types.DeviceMapping{
+				{
+					PathOnHost:        loopDev,
+					PathInContainer:   loopDev,
+					CgroupPermissions: "rwm",
+				},
+			}
+
+			// Create container
+			statusCode, ctr := createContainer(uClient, url, testContainerName, options)
+			Expect(statusCode).Should(Equal(http.StatusCreated))
+			Expect(ctr.ID).ShouldNot(BeEmpty())
+
+			// Start container
+			command.Run(opt, "start", testContainerName)
+
+			// Inspect using native format
+			nativeResp := command.Stdout(opt, "inspect", "--mode=native", testContainerName)
+			var nativeInspect []map[string]interface{}
+			err := json.Unmarshal(nativeResp, &nativeInspect)
+			Expect(err).Should(BeNil())
+			Expect(nativeInspect).Should(HaveLen(1))
+
+			// Navigate to the linux section
+			spec, ok := nativeInspect[0]["Spec"].(map[string]interface{})
+			Expect(ok).Should(BeTrue())
+			linux, ok := spec["linux"].(map[string]interface{})
+			Expect(ok).Should(BeTrue())
+
+			// Verify device in linux.devices
+			devices, ok := linux["devices"].([]interface{})
+			Expect(ok).Should(BeTrue())
+
+			foundDevice := false
+			for _, device := range devices {
+				d := device.(map[string]interface{})
+				if d["path"] == loopDev {
+					foundDevice = true
+					Expect(d["type"]).Should(Equal("b")) // block device
+					Expect(d["major"].(float64)).Should(Equal(float64(major)))
+					Expect(d["minor"].(float64)).Should(Equal(float64(minor)))
+					break
+				}
+			}
+			Expect(foundDevice).Should(BeTrue())
+
+			// Verify device permissions in linux.resources.devices
+			resources, ok := linux["resources"].(map[string]interface{})
+			Expect(ok).Should(BeTrue())
+			resourceDevices, ok := resources["devices"].([]interface{})
+			Expect(ok).Should(BeTrue())
+
+			// First rule should be deny all
+			denyAll := resourceDevices[0].(map[string]interface{})
+			Expect(denyAll["allow"]).Should(BeFalse())
+			Expect(denyAll["access"]).Should(Equal("rwm"))
+
+			// Should find an allow rule for our device
+			foundAllowRule := false
+			for _, rule := range resourceDevices {
+				r := rule.(map[string]interface{})
+				if r["allow"] == true &&
+					r["type"] == "b" &&
+					r["major"].(float64) == float64(major) &&
+					r["minor"].(float64) == float64(minor) {
+					foundAllowRule = true
+					Expect(r["access"]).Should(Equal("rwm"))
+					break
+				}
+			}
+			Expect(foundAllowRule).Should(BeTrue())
+		})
 	})
 }