loft-sh / devpod

Codespaces but open-source, client-only and unopinionated: Works with any IDE and lets you use any cloud, kubernetes or just localhost docker.
https://devpod.sh
Mozilla Public License 2.0
9.53k stars 345 forks source link

Other property of mounts should be serialized #1292

Closed gjhenrique closed 1 month ago

gjhenrique commented 1 month ago

What happened?
I tried to use the other property in the mounts

{
  "mounts": [
    {
      "source": "/home/host/filei",
      "target": "/home/container/fili",
      "type": "bind",
      "other": ["readonly"]
    }
  ],
}

What did you expect to happen instead?
The mount should be read-only

How can we reproduce the bug? (as minimally and precisely as possible)

Add other as mounts and check if the value is passed to docker run

Actually, the bug happens because there is no treatment for other property. I was going insane trying to find why go wasn't marshaling the struct properly, but the issue was

It worked with this code inside the UnmarshalJSON method

Unstaged changes (2)
modified   pkg/devcontainer/config/config.go
@@ -424,6 +424,16 @@ func (m *Mount) UnmarshalJSON(data []byte) error {
        if ok {
            m.External = externalStr
        }
+
+       valInterface, ok := obj["other"].([]interface{})
+       if ok {
+           other := make([]string, len(valInterface))
+           for i, _ := range valInterface {
+               other[i] = valInterface[i].(string)
+           }
+           m.Other = other
+       }

Local Environment:

DevPod Provider:

Anything else we need to know?
Not really. I can also open the PR if it's faster, since it works on my machine :tm:

bkneis commented 1 month ago

@gjhenrique thanks for reporting the issue! Looking at the code I can see your fix looks good to me. Can you please place a PR with these changes?