Browse code

Merge pull request #51127 from thaJeztah/info_remove_deprecated

api/types/system: remove deprecated Commit.Expected field

Albin Kerouanton authored on 2025/10/08 18:08:06
Showing 9 changed files
... ...
@@ -11,9 +11,6 @@ type ServiceConfig struct {
11 11
 	InsecureRegistryCIDRs []netip.Prefix        `json:"InsecureRegistryCIDRs"`
12 12
 	IndexConfigs          map[string]*IndexInfo `json:"IndexConfigs"`
13 13
 	Mirrors               []string
14
-
15
-	// ExtraFields is for internal use to include deprecated fields on older API versions.
16
-	ExtraFields map[string]any `json:"-"`
17 14
 }
18 15
 
19 16
 // IndexInfo contains information about a registry
... ...
@@ -139,11 +139,6 @@ type PluginsInfo struct {
139 139
 type Commit struct {
140 140
 	// ID is the actual commit ID or version of external tool.
141 141
 	ID string
142
-
143
-	// Expected is the commit ID of external tool expected by dockerd as set at build time.
144
-	//
145
-	// Deprecated: this field is no longer used in API v1.49, but kept for backward-compatibility with older API versions.
146
-	Expected string `json:",omitempty"`
147 142
 }
148 143
 
149 144
 // NetworkAddressPool is a temp struct used by [Info] struct.
150 145
deleted file mode 100644
... ...
@@ -1,44 +0,0 @@
1
-package system
2
-
3
-import (
4
-	"encoding/json"
5
-	"maps"
6
-
7
-	"github.com/moby/moby/api/types/system"
8
-)
9
-
10
-// infoResponse is a wrapper around [system.Info] with a custom
11
-// marshal function for legacy fields.
12
-type infoResponse struct {
13
-	*system.Info
14
-
15
-	// extraFields is for internal use to include deprecated fields on older API versions.
16
-	extraFields map[string]any
17
-}
18
-
19
-// MarshalJSON implements a custom marshaler to include legacy fields
20
-// in API responses.
21
-func (ir *infoResponse) MarshalJSON() ([]byte, error) {
22
-	type tmp *system.Info
23
-	base, err := json.Marshal((tmp)(ir.Info))
24
-	if err != nil {
25
-		return nil, err
26
-	}
27
-	if len(ir.extraFields) == 0 && (ir.Info == nil || ir.Info.RegistryConfig == nil || len(ir.Info.RegistryConfig.ExtraFields) == 0) {
28
-		return base, nil
29
-	}
30
-	var merged map[string]any
31
-	_ = json.Unmarshal(base, &merged)
32
-
33
-	// Merge top-level extraFields
34
-	maps.Copy(merged, ir.extraFields)
35
-
36
-	// Merge RegistryConfig.ExtraFields if present
37
-	if ir.Info != nil && ir.Info.RegistryConfig != nil && len(ir.Info.RegistryConfig.ExtraFields) > 0 {
38
-		if rc, ok := merged["RegistryConfig"].(map[string]any); ok {
39
-			maps.Copy(rc, ir.Info.RegistryConfig.ExtraFields)
40
-		}
41
-	}
42
-
43
-	return json.Marshal(merged)
44
-}
45 1
deleted file mode 100644
... ...
@@ -1,104 +0,0 @@
1
-package system
2
-
3
-import (
4
-	"encoding/json"
5
-	"strings"
6
-	"testing"
7
-
8
-	"github.com/moby/moby/api/types/registry"
9
-	"github.com/moby/moby/api/types/system"
10
-	"gotest.tools/v3/assert"
11
-	is "gotest.tools/v3/assert/cmp"
12
-)
13
-
14
-func TestLegacyFields(t *testing.T) {
15
-	infoResp := &infoResponse{
16
-		Info: &system.Info{
17
-			Containers: 10,
18
-		},
19
-		extraFields: map[string]any{
20
-			"LegacyFoo": false,
21
-			"LegacyBar": true,
22
-		},
23
-	}
24
-
25
-	data, err := json.MarshalIndent(infoResp, "", "  ")
26
-	if err != nil {
27
-		t.Fatal(err)
28
-	}
29
-
30
-	if expected := `"LegacyFoo": false`; !strings.Contains(string(data), expected) {
31
-		t.Errorf("legacy fields should contain %s: %s", expected, string(data))
32
-	}
33
-	if expected := `"LegacyBar": true`; !strings.Contains(string(data), expected) {
34
-		t.Errorf("legacy fields should contain %s: %s", expected, string(data))
35
-	}
36
-}
37
-
38
-// TestMarshalRegistryConfigLegacyFields verifies extra fields in the registry config
39
-// field in the info response are serialized if they are not empty.
40
-// This is used for backwards compatibility for API versions < 1.47.
41
-func TestMarshalRegistryConfigLegacyFields(t *testing.T) {
42
-	expected := []string{"AllowNondistributableArtifactsCIDRs", "AllowNondistributableArtifactsHostnames"}
43
-
44
-	tests := []struct {
45
-		name   string
46
-		info   *infoResponse
47
-		assert func(t *testing.T, data []byte, err error)
48
-	}{
49
-		{
50
-			name: "without legacy fields",
51
-			info: &infoResponse{
52
-				Info: &system.Info{},
53
-			},
54
-			assert: func(t *testing.T, data []byte, err error) {
55
-				assert.NilError(t, err)
56
-
57
-				var resp map[string]any
58
-				err = json.Unmarshal(data, &resp)
59
-				assert.NilError(t, err)
60
-
61
-				rc, ok := resp["RegistryConfig"]
62
-				assert.Check(t, ok)
63
-
64
-				for _, v := range expected {
65
-					assert.Check(t, !is.Contains(rc, v)().Success())
66
-				}
67
-			},
68
-		},
69
-		{
70
-			name: "with legacy fields",
71
-			info: &infoResponse{
72
-				Info: &system.Info{
73
-					RegistryConfig: &registry.ServiceConfig{
74
-						ExtraFields: map[string]any{
75
-							"AllowNondistributableArtifactsCIDRs":     json.RawMessage(nil),
76
-							"AllowNondistributableArtifactsHostnames": json.RawMessage(nil),
77
-						},
78
-					},
79
-				},
80
-			},
81
-			assert: func(t *testing.T, data []byte, err error) {
82
-				assert.NilError(t, err)
83
-
84
-				var resp map[string]any
85
-				err = json.Unmarshal(data, &resp)
86
-				assert.NilError(t, err)
87
-
88
-				rc, ok := resp["RegistryConfig"]
89
-				assert.Check(t, ok)
90
-
91
-				for _, v := range expected {
92
-					assert.Check(t, is.Contains(rc, v))
93
-				}
94
-			},
95
-		},
96
-	}
97
-
98
-	for _, tc := range tests {
99
-		t.Run(tc.name, func(t *testing.T) {
100
-			data, err := json.MarshalIndent(tc.info, "", "  ")
101
-			tc.assert(t, data, err)
102
-		})
103
-	}
104
-}
... ...
@@ -1,6 +1,7 @@
1 1
 package system
2 2
 
3 3
 import (
4
+	"github.com/moby/moby/v2/daemon/internal/compat"
4 5
 	"github.com/moby/moby/v2/daemon/server/router"
5 6
 	"resenje.org/singleflight"
6 7
 )
... ...
@@ -17,7 +18,7 @@ type systemRouter struct {
17 17
 	// collectSystemInfo is a single-flight for the /info endpoint,
18 18
 	// unique per API version (as different API versions may return
19 19
 	// a different API response).
20
-	collectSystemInfo singleflight.Group[string, *infoResponse]
20
+	collectSystemInfo singleflight.Group[string, *compat.Wrapper]
21 21
 }
22 22
 
23 23
 // NewRouter initializes a new system router
... ...
@@ -64,7 +64,7 @@ func (s *systemRouter) swarmStatus() string {
64 64
 
65 65
 func (s *systemRouter) getInfo(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
66 66
 	version := httputils.VersionFromContext(ctx)
67
-	info, _, _ := s.collectSystemInfo.Do(ctx, version, func(ctx context.Context) (*infoResponse, error) {
67
+	info, _, _ := s.collectSystemInfo.Do(ctx, version, func(ctx context.Context) (*compat.Wrapper, error) {
68 68
 		info, err := s.backend.SystemInfo(ctx)
69 69
 		if err != nil {
70 70
 			return nil, err
... ...
@@ -75,6 +75,7 @@ func (s *systemRouter) getInfo(ctx context.Context, w http.ResponseWriter, r *ht
75 75
 			info.Warnings = append(info.Warnings, info.Swarm.Warnings...)
76 76
 		}
77 77
 
78
+		var legacyOptions []compat.Option
78 79
 		if versions.LessThan(version, "1.44") {
79 80
 			for k, rt := range info.Runtimes {
80 81
 				// Status field introduced in API v1.44.
... ...
@@ -88,35 +89,36 @@ func (s *systemRouter) getInfo(ctx context.Context, w http.ResponseWriter, r *ht
88 88
 		if versions.LessThan(version, "1.47") {
89 89
 			// Field is omitted in API 1.48 and up, but should still be included
90 90
 			// in older versions, even if no values are set.
91
-			info.RegistryConfig.ExtraFields = map[string]any{
92
-				"AllowNondistributableArtifactsCIDRs":     json.RawMessage(nil),
93
-				"AllowNondistributableArtifactsHostnames": json.RawMessage(nil),
94
-			}
91
+			legacyOptions = append(legacyOptions, compat.WithExtraFields(map[string]any{
92
+				"RegistryConfig": map[string]any{
93
+					"AllowNondistributableArtifactsCIDRs":     json.RawMessage(nil),
94
+					"AllowNondistributableArtifactsHostnames": json.RawMessage(nil),
95
+				},
96
+			}))
95 97
 		}
96 98
 		if versions.LessThan(version, "1.49") {
97 99
 			// FirewallBackend field introduced in API v1.49.
98 100
 			info.FirewallBackend = nil
99
-		}
100 101
 
101
-		extraFields := map[string]any{}
102
-		if versions.LessThan(version, "1.49") {
103 102
 			// Expected commits are omitted in API 1.49, but should still be
104 103
 			// included in older versions.
105
-			info.ContainerdCommit.Expected = info.ContainerdCommit.ID //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.49.
106
-			info.RuncCommit.Expected = info.RuncCommit.ID             //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.49.
107
-			info.InitCommit.Expected = info.InitCommit.ID             //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.49.
104
+			legacyOptions = append(legacyOptions, compat.WithExtraFields(map[string]any{
105
+				"ContainerdCommit": map[string]any{"Expected": info.ContainerdCommit.ID},
106
+				"RuncCommit":       map[string]any{"Expected": info.RuncCommit.ID},
107
+				"InitCommit":       map[string]any{"Expected": info.InitCommit.ID},
108
+			}))
108 109
 		}
109 110
 		if versions.LessThan(version, "1.50") {
110 111
 			info.DiscoveredDevices = nil
111 112
 
112 113
 			// These fields are omitted in > API 1.49, and always false
113 114
 			// older API versions.
114
-			extraFields = map[string]any{
115
+			legacyOptions = append(legacyOptions, compat.WithExtraFields(map[string]any{
115 116
 				"BridgeNfIptables":  json.RawMessage("false"),
116 117
 				"BridgeNfIp6tables": json.RawMessage("false"),
117
-			}
118
+			}))
118 119
 		}
119
-		return &infoResponse{Info: info, extraFields: extraFields}, nil
120
+		return compat.Wrap(info, legacyOptions...), nil
120 121
 	})
121 122
 
122 123
 	return httputils.WriteJSON(w, http.StatusOK, info)
... ...
@@ -8,7 +8,6 @@ import (
8 8
 	"net/http"
9 9
 	"testing"
10 10
 
11
-	"github.com/moby/moby/client"
12 11
 	"github.com/moby/moby/v2/internal/testutil/request"
13 12
 	"gotest.tools/v3/assert"
14 13
 	is "gotest.tools/v3/assert/cmp"
... ...
@@ -17,38 +16,63 @@ import (
17 17
 func TestInfoBinaryCommits(t *testing.T) {
18 18
 	ctx := setupTest(t)
19 19
 
20
+	// API v1.48 and lower returned both the "current" commit (ID) and "expected" commit.
21
+	// The "Expected" field has been removed in the API types, so define
22
+	// an ad-hoc type for this test.
23
+	type legacyCommit struct {
24
+		ID       string
25
+		Expected string
26
+	}
27
+
28
+	type legacyInfo struct {
29
+		ContainerdCommit legacyCommit
30
+		RuncCommit       legacyCommit
31
+		InitCommit       legacyCommit
32
+	}
33
+
20 34
 	t.Run("current", func(t *testing.T) {
21
-		apiClient := testEnv.APIClient()
35
+		res, body, err := request.Get(ctx, "/info", request.JSON)
36
+		assert.NilError(t, err)
37
+		assert.Equal(t, res.StatusCode, http.StatusOK)
22 38
 
23
-		info, err := apiClient.Info(ctx)
39
+		buf, err := request.ReadBody(body)
40
+		assert.NilError(t, err)
41
+
42
+		var info legacyInfo
43
+		err = json.Unmarshal(buf, &info)
24 44
 		assert.NilError(t, err)
25 45
 
26 46
 		assert.Check(t, info.ContainerdCommit.ID != "N/A")
27
-		assert.Check(t, is.Equal(info.ContainerdCommit.Expected, "")) //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.49.
47
+		assert.Check(t, is.Equal(info.ContainerdCommit.Expected, ""))
28 48
 
29 49
 		assert.Check(t, info.InitCommit.ID != "N/A")
30
-		assert.Check(t, is.Equal(info.InitCommit.Expected, "")) //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.49.
50
+		assert.Check(t, is.Equal(info.InitCommit.Expected, ""))
31 51
 
32 52
 		assert.Check(t, info.RuncCommit.ID != "N/A")
33
-		assert.Check(t, is.Equal(info.RuncCommit.Expected, "")) //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.49.
53
+		assert.Check(t, is.Equal(info.RuncCommit.Expected, ""))
34 54
 	})
35 55
 
36 56
 	// Expected commits are omitted in API 1.49, but should still be included in older versions.
37 57
 	t.Run("1.48", func(t *testing.T) {
38
-		apiClient, err := client.NewClientWithOpts(client.FromEnv, client.WithVersion("1.48"))
58
+		res, body, err := request.Get(ctx, "/v1.48/info", request.JSON)
59
+		assert.NilError(t, err)
60
+		assert.Equal(t, res.StatusCode, http.StatusOK)
61
+
62
+		buf, err := request.ReadBody(body)
39 63
 		assert.NilError(t, err)
40 64
 
41
-		info, err := apiClient.Info(ctx)
65
+		var info legacyInfo
66
+		err = json.Unmarshal(buf, &info)
42 67
 		assert.NilError(t, err)
43 68
 
44 69
 		assert.Check(t, info.ContainerdCommit.ID != "N/A")
45
-		assert.Check(t, is.Equal(info.ContainerdCommit.Expected, info.ContainerdCommit.ID)) //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.49.
70
+		assert.Check(t, is.Equal(info.ContainerdCommit.Expected, info.ContainerdCommit.ID))
46 71
 
47 72
 		assert.Check(t, info.InitCommit.ID != "N/A")
48
-		assert.Check(t, is.Equal(info.InitCommit.Expected, info.InitCommit.ID)) //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.49.
73
+		assert.Check(t, is.Equal(info.InitCommit.Expected, info.InitCommit.ID))
49 74
 
50 75
 		assert.Check(t, info.RuncCommit.ID != "N/A")
51
-		assert.Check(t, is.Equal(info.RuncCommit.Expected, info.RuncCommit.ID)) //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.49.
76
+		assert.Check(t, is.Equal(info.RuncCommit.Expected, info.RuncCommit.ID))
52 77
 	})
53 78
 }
54 79
 
... ...
@@ -11,9 +11,6 @@ type ServiceConfig struct {
11 11
 	InsecureRegistryCIDRs []netip.Prefix        `json:"InsecureRegistryCIDRs"`
12 12
 	IndexConfigs          map[string]*IndexInfo `json:"IndexConfigs"`
13 13
 	Mirrors               []string
14
-
15
-	// ExtraFields is for internal use to include deprecated fields on older API versions.
16
-	ExtraFields map[string]any `json:"-"`
17 14
 }
18 15
 
19 16
 // IndexInfo contains information about a registry
... ...
@@ -139,11 +139,6 @@ type PluginsInfo struct {
139 139
 type Commit struct {
140 140
 	// ID is the actual commit ID or version of external tool.
141 141
 	ID string
142
-
143
-	// Expected is the commit ID of external tool expected by dockerd as set at build time.
144
-	//
145
-	// Deprecated: this field is no longer used in API v1.49, but kept for backward-compatibility with older API versions.
146
-	Expected string `json:",omitempty"`
147 142
 }
148 143
 
149 144
 // NetworkAddressPool is a temp struct used by [Info] struct.