Skip to content

Commit ee560a3

Browse files
corherecorhere
committed
api/types: fix Plugin.Config.Interface.Types def'n
The wire type of Plugin.Config.Interface.Types is an array of strings, not of objects with three properties. We just so happen to have a Go struct type to represent a plugin-interface-type value in memory with all the fields parsed out for convenience, but that is not part of the REST API contract documented by the Swager spec.U pdate the Swagger spec to correctly document that the Types property is an array of strings in the API, while still generating Go definitions that unmarshal into the convenient struct type. Move the definition and marshal/unmarshal methods for PluginInterfaceType into a more appropriate location than api/types. Rename the type to one that does not stutter or overload already heavily overloaded terminology. Modernize the parser and use property-based testing to assert that it behaves the same as the old parser for all well-formed inputs. Signed-off-by: Cory Snider <[email protected]>
1 parent 2783f80 commit ee560a3

23 files changed

Lines changed: 243 additions & 174 deletions

File tree

‎api/go.mod‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,5 @@ require (
1111
github.com/opencontainers/image-spec v1.1.1
1212
golang.org/x/time v0.11.0
1313
gotest.tools/v3 v3.5.2
14+
pgregory.net/rapid v1.2.0
1415
)

‎api/go.sum‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,5 @@ golang.org/x/time v0.11.0 h1:/bpjEDfN9tkoN/ryeYHnv5hcMlc8ncjMcM4XBk5NWV0=
1414
golang.org/x/time v0.11.0/go.mod h1:CDIdPxbZBQxdj6cxyCIdrNogrJKMJ7pr37NYpMcMDSg=
1515
gotest.tools/v3 v3.5.2 h1:7koQfIKdy+I8UTetycgUqXWSDwpgv193Ka+qRsmBY8Q=
1616
gotest.tools/v3 v3.5.2/go.mod h1:LtdLGcnqToBH83WByAAi/wiwSFCArdFIUV/xxN4pcjA=
17+
pgregory.net/rapid v1.2.0 h1:keKAYRcjm+e1F0oAuU5F5+YPAWcyxNNRK2wud503Gnk=
18+
pgregory.net/rapid v1.2.0/go.mod h1:PY5XlDGj0+V1FCq0o192FdRhpKHGTRIWBgqjDBTrq04=

‎api/swagger.yaml‎

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3021,21 +3021,6 @@ definitions:
30213021
Value:
30223022
type: "string"
30233023

3024-
PluginInterfaceType:
3025-
type: "object"
3026-
x-nullable: false
3027-
required: [Prefix, Capability, Version]
3028-
properties:
3029-
Prefix:
3030-
type: "string"
3031-
x-nullable: false
3032-
Capability:
3033-
type: "string"
3034-
x-nullable: false
3035-
Version:
3036-
type: "string"
3037-
x-nullable: false
3038-
30393024
PluginPrivilege:
30403025
description: |
30413026
Describes a permission the user has to accept upon installing
@@ -3144,7 +3129,11 @@ definitions:
31443129
Types:
31453130
type: "array"
31463131
items:
3147-
$ref: "#/definitions/PluginInterfaceType"
3132+
type: "string"
3133+
x-go-type:
3134+
type: "CapabilityID"
3135+
import:
3136+
package: "github.com/moby/moby/api/types/plugin"
31483137
example:
31493138
- "docker.volumedriver/1.0"
31503139
Socket:

‎api/types/plugin.go‎

Lines changed: 5 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎api/types/plugin/.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
testdata/rapid/**

‎api/types/plugin/capability.go‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
package plugin
2+
3+
import (
4+
"bytes"
5+
"encoding"
6+
"fmt"
7+
"strings"
8+
)
9+
10+
type CapabilityID struct {
11+
Capability string
12+
Prefix string
13+
Version string
14+
}
15+
16+
var (
17+
_ fmt.Stringer = CapabilityID{}
18+
_ encoding.TextUnmarshaler = (*CapabilityID)(nil)
19+
_ encoding.TextMarshaler = CapabilityID{}
20+
)
21+
22+
// String implements [fmt.Stringer] for CapabilityID
23+
func (t CapabilityID) String() string {
24+
return fmt.Sprintf("%s.%s/%s", t.Prefix, t.Capability, t.Version)
25+
}
26+
27+
// UnmarshalText implements [encoding.TextUnmarshaler] for CapabilityID
28+
func (t *CapabilityID) UnmarshalText(p []byte) error {
29+
fqcap, version, _ := bytes.Cut(p, []byte{'/'})
30+
idx := bytes.LastIndexByte(fqcap, '.')
31+
if idx < 0 {
32+
t.Prefix = ""
33+
t.Capability = string(fqcap)
34+
} else {
35+
t.Prefix = string(fqcap[:idx])
36+
t.Capability = string(fqcap[idx+1:])
37+
}
38+
t.Version = string(version)
39+
return nil
40+
}
41+
42+
// MarshalText implements [encoding.TextMarshaler] for CapabilityID
43+
func (t CapabilityID) MarshalText() ([]byte, error) {
44+
// Assert that the value can be round-tripped successfully.
45+
if strings.Contains(t.Capability, ".") {
46+
return nil, fmt.Errorf("capability %q cannot contain a dot", t.Capability)
47+
}
48+
if strings.Contains(t.Prefix, "/") {
49+
return nil, fmt.Errorf("prefix %q cannot contain a slash", t.Prefix)
50+
}
51+
if strings.Contains(t.Capability, "/") {
52+
return nil, fmt.Errorf("capability %q cannot contain a slash", t.Capability)
53+
}
54+
return []byte(t.String()), nil
55+
}

‎api/types/plugin/capability_test.go‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
package plugin
2+
3+
import (
4+
"encoding/json"
5+
"fmt"
6+
"testing"
7+
8+
"gotest.tools/v3/assert"
9+
is "gotest.tools/v3/assert/cmp"
10+
"pgregory.net/rapid"
11+
)
12+
13+
// unmarshalJSON is a copy of the original PluginInterfaceType.UnmarshalJSON
14+
// parser, used to test that the new parser produces the same results for
15+
// well-formed inputs.
16+
func (t *CapabilityID) unmarshalJSON(p []byte) error {
17+
versionIndex := len(p)
18+
prefixIndex := 0
19+
if len(p) < 2 || p[0] != '"' || p[len(p)-1] != '"' {
20+
return fmt.Errorf("%q is not a plugin interface type", p)
21+
}
22+
p = p[1 : len(p)-1]
23+
loop:
24+
for i, b := range p {
25+
switch b {
26+
case '.':
27+
prefixIndex = i
28+
case '/':
29+
versionIndex = i
30+
break loop
31+
}
32+
}
33+
t.Prefix = string(p[:prefixIndex])
34+
t.Capability = string(p[prefixIndex+1 : versionIndex])
35+
if versionIndex < len(p) {
36+
t.Version = string(p[versionIndex+1:])
37+
}
38+
return nil
39+
}
40+
41+
func TestCapabilityID_MarshalUnmarshal(t *testing.T) {
42+
stringgen := rapid.StringMatching(`[a-z0-9-./]*`)
43+
rapid.Check(t, func(t *rapid.T) {
44+
typ := CapabilityID{
45+
Capability: stringgen.Draw(t, "Capability"),
46+
Prefix: stringgen.Draw(t, "Prefix"),
47+
Version: stringgen.Draw(t, "Version"),
48+
}
49+
b, err := typ.MarshalText()
50+
if err != nil {
51+
t.Skipf("unmarshalable value: %v", err)
52+
}
53+
t.Logf("InterfaceType(%q)", b)
54+
55+
var roundtrip CapabilityID
56+
err = roundtrip.UnmarshalText(b)
57+
assert.Assert(t, err)
58+
assert.Assert(t, is.DeepEqual(typ, roundtrip))
59+
60+
jb, err := json.Marshal(string(b))
61+
assert.Assert(t, err)
62+
var oldparser CapabilityID
63+
err = oldparser.unmarshalJSON(jb)
64+
assert.Assert(t, err)
65+
assert.Assert(t, is.DeepEqual(typ, oldparser), "new parser does not match the old parser")
66+
})
67+
}
68+
69+
func TestCapabilityID_JSONMarshalUnmarshal(t *testing.T) {
70+
type rt struct {
71+
Type CapabilityID
72+
}
73+
a := rt{
74+
Type: CapabilityID{
75+
Capability: "foo",
76+
Prefix: "bar",
77+
Version: "baz",
78+
},
79+
}
80+
b, err := json.Marshal(a)
81+
assert.Assert(t, err)
82+
t.Logf("JSON: %s", b)
83+
84+
var roundtrip rt
85+
err = json.Unmarshal(b, &roundtrip)
86+
assert.Assert(t, err)
87+
assert.Assert(t, is.DeepEqual(a, roundtrip))
88+
}

‎api/types/plugin_interface_type.go‎

Lines changed: 0 additions & 24 deletions
This file was deleted.

‎api/types/plugin_responses.go‎

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,50 +1,12 @@
11
package types
22

33
import (
4-
"encoding/json"
5-
"fmt"
64
"sort"
75
)
86

97
// PluginsListResponse contains the response for the Engine API
108
type PluginsListResponse []*Plugin
119

12-
// UnmarshalJSON implements json.Unmarshaler for PluginInterfaceType
13-
func (t *PluginInterfaceType) UnmarshalJSON(p []byte) error {
14-
versionIndex := len(p)
15-
prefixIndex := 0
16-
if len(p) < 2 || p[0] != '"' || p[len(p)-1] != '"' {
17-
return fmt.Errorf("%q is not a plugin interface type", p)
18-
}
19-
p = p[1 : len(p)-1]
20-
loop:
21-
for i, b := range p {
22-
switch b {
23-
case '.':
24-
prefixIndex = i
25-
case '/':
26-
versionIndex = i
27-
break loop
28-
}
29-
}
30-
t.Prefix = string(p[:prefixIndex])
31-
t.Capability = string(p[prefixIndex+1 : versionIndex])
32-
if versionIndex < len(p) {
33-
t.Version = string(p[versionIndex+1:])
34-
}
35-
return nil
36-
}
37-
38-
// MarshalJSON implements json.Marshaler for PluginInterfaceType
39-
func (t *PluginInterfaceType) MarshalJSON() ([]byte, error) {
40-
return json.Marshal(t.String())
41-
}
42-
43-
// String implements fmt.Stringer for PluginInterfaceType
44-
func (t PluginInterfaceType) String() string {
45-
return fmt.Sprintf("%s.%s/%s", t.Prefix, t.Capability, t.Version)
46-
}
47-
4810
// PluginPrivilege describes a permission the user has to accept
4911
// upon installing a plugin.
5012
type PluginPrivilege struct {

‎daemon/pkg/plugin/manager_linux_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010

1111
"github.com/moby/moby/api/types"
1212
"github.com/moby/moby/api/types/events"
13+
"github.com/moby/moby/api/types/plugin"
1314
"github.com/moby/moby/v2/daemon/internal/containerfs"
1415
"github.com/moby/moby/v2/daemon/internal/stringid"
1516
v2 "github.com/moby/moby/v2/daemon/pkg/plugin/v2"
@@ -79,8 +80,8 @@ func newTestPlugin(t *testing.T, name, capability, root string) *v2.Plugin {
7980

8081
p := v2.Plugin{PluginObj: types.Plugin{ID: id, Name: name}}
8182
p.Rootfs = rootfs
82-
iType := types.PluginInterfaceType{Capability: capability, Prefix: "docker", Version: "1.0"}
83-
i := types.PluginConfigInterface{Socket: "plugin.sock", Types: []types.PluginInterfaceType{iType}}
83+
iType := plugin.CapabilityID{Capability: capability, Prefix: "docker", Version: "1.0"}
84+
i := types.PluginConfigInterface{Socket: "plugin.sock", Types: []plugin.CapabilityID{iType}}
8485
p.PluginObj.Config.Interface = i
8586
p.PluginObj.ID = id
8687

0 commit comments

Comments
 (0)