Repository: compose-spec/compose-go
Pull request #692 (2024-10-04 12:50:54)

prefer canonical transform vs DecodeMapStructure
Avoid use of DecodeMapstructure when we can just make yaml tree canonical 

ndeloof: I haven't removed DeviceCount#DecodeMapstructure which handle this case
glours: I just see that, and the `all` is converted to `-1` so it's fine for me 👍 
ndeloof: to be debated such moby-specific tweaks should live in this repository, or must move to docker specific implementation as docker/compose

Patch:
diff --git a/schema/compose-spec.json b/schema/compose-spec.json
index a95b941..abd564f 100644
--- a/schema/compose-spec.json
+++ b/schema/compose-spec.json
@@ -642,7 +642,10 @@
           "options":{"$ref": "#/definitions/list_or_dict"}
         },
         "additionalProperties": false,
-        "patternProperties": {"^x-": {}}
+        "patternProperties": {"^x-": {}},
+        "required": [
+          "capabilities"
+        ]
       }
     },
 
diff --git a/transform/canonical.go b/transform/canonical.go
index a248b4b..fa844e7 100644
--- a/transform/canonical.go
+++ b/transform/canonical.go
@@ -29,6 +29,7 @@ func init() {
 	transformers["services.*.build.secrets.*"] = transformFileMount
 	transformers["services.*.build.additional_contexts"] = transformKeyValue
 	transformers["services.*.depends_on"] = transformDependsOn
+	transformers["services.*.deploy.resources.reservations.devices.*"] = transformDeviceRequest
 	transformers["services.*.env_file"] = transformEnvFile
 	transformers["services.*.extends"] = transformExtends
 	transformers["services.*.networks"] = transformServiceNetworks
diff --git a/transform/devices.go b/transform/devices.go
new file mode 100644
index 0000000..558d47e
--- /dev/null
+++ b/transform/devices.go
@@ -0,0 +1,40 @@
+/*
+   Copyright 2020 The Compose Specification Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package transform
+
+import (
+	"fmt"
+
+	"github.com/compose-spec/compose-go/v2/tree"
+)
+
+func transformDeviceRequest(data any, p tree.Path, ignoreParseError bool) (any, error) {
+	switch v := data.(type) {
+	case map[string]any:
+		_, hasCount := v["count"]
+		_, hasIds := v["device_ids"]
+		if hasCount && hasIds {
+			return nil, fmt.Errorf(`%s: "count" and "device_ids" attributes are exclusive`, p)
+		}
+		if !hasCount && !hasIds {
+			v["count"] = "all"
+		}
+		return transformMapping(v, p, ignoreParseError)
+	default:
+		return data, fmt.Errorf("%s: invalid type %T for device request", p, v)
+	}
+}
diff --git a/types/device.go b/types/device.go
index 3d11094..5b30cc0 100644
--- a/types/device.go
+++ b/types/device.go
@@ -51,55 +51,3 @@ func (c *DeviceCount) DecodeMapstructure(value interface{}) error {
 	}
 	return nil
 }
-
-func (d *DeviceRequest) DecodeMapstructure(value interface{}) error {
-	v, ok := value.(map[string]any)
-	if !ok {
-		return fmt.Errorf("invalid device request type %T", value)
-	}
-	if _, okCaps := v["capabilities"]; !okCaps {
-		return fmt.Errorf(`"capabilities" attribute is mandatory for device request definition`)
-	}
-	if _, okCount := v["count"]; okCount {
-		if _, okDeviceIds := v["device_ids"]; okDeviceIds {
-			return fmt.Errorf(`invalid "count" and "device_ids" are attributes are exclusive`)
-		}
-	}
-	d.Count = DeviceCount(-1)
-
-	capabilities := v["capabilities"]
-	caps := StringList{}
-	if err := caps.DecodeMapstructure(capabilities); err != nil {
-		return err
-	}
-	d.Capabilities = caps
-	if driver, ok := v["driver"]; ok {
-		if val, ok := driver.(string); ok {
-			d.Driver = val
-		} else {
-			return fmt.Errorf("invalid type for driver value: %T", driver)
-		}
-	}
-	if count, ok := v["count"]; ok {
-		if err := d.Count.DecodeMapstructure(count); err != nil {
-			return err
-		}
-	}
-	if deviceIDs, ok := v["device_ids"]; ok {
-		ids := StringList{}
-		if err := ids.DecodeMapstructure(deviceIDs); err != nil {
-			return err
-		}
-		d.IDs = ids
-		d.Count = DeviceCount(len(ids))
-	}
-
-	d.Options = Mapping{}
-	if options, ok := v["options"].(map[string]any); ok {
-		for k, v := range options {
-			d.Options[k] = fmt.Sprint(v)
-		}
-	}
-	return nil
-
-}


Test patch:
diff --git a/loader/loader_test.go b/loader/loader_test.go
index d2bf189..c6cc932 100644
--- a/loader/loader_test.go
+++ b/loader/loader_test.go
@@ -2205,7 +2205,7 @@ services:
               count: 2
               device_ids: ["my-device-id"]
 `)
-	assert.ErrorContains(t, err, `invalid "count" and "device_ids" are attributes are exclusive`)
+	assert.ErrorContains(t, err, `"count" and "device_ids" attributes are exclusive`)
 }
 
 func TestServiceDeviceRequestCapabilitiesMandatory(t *testing.T) {
@@ -2221,7 +2221,7 @@ services:
             - driver: nvidia
               count: 2
 `)
-	assert.ErrorContains(t, err, `"capabilities" attribute is mandatory for device request definition`)
+	assert.ErrorContains(t, err, `capabilities is required`)
 }
 
 func TestServicePullPolicy(t *testing.T) {