Browse code

client: keep image refs in canonical format where possible

Using "familiarname" (e.g. "ubuntu") should be mostly done for presenting
image refernces to the user, but internally, we should use the canonical
format where possible ("docker.io/library/ubuntu").

There's still many places where we use the familiar (short) form, but
let's start with not converting references in the client.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Sebastiaan van Stijn authored on 2025/03/10 00:20:26
Showing 10 changed files
... ...
@@ -32,7 +32,7 @@ func (cli *Client) ContainerCommit(ctx context.Context, containerID string, opti
32 32
 		if tagged, ok := ref.(reference.Tagged); ok {
33 33
 			tag = tagged.Tag()
34 34
 		}
35
-		repository = reference.FamiliarName(ref)
35
+		repository = ref.Name()
36 36
 	}
37 37
 
38 38
 	query := url.Values{}
... ...
@@ -33,13 +33,15 @@ func TestContainerCommitError(t *testing.T) {
33 33
 }
34 34
 
35 35
 func TestContainerCommit(t *testing.T) {
36
-	expectedURL := "/commit"
37
-	expectedContainerID := "container_id"
38
-	specifiedReference := "repository_name:tag"
39
-	expectedRepositoryName := "repository_name"
40
-	expectedTag := "tag"
41
-	expectedComment := "comment"
42
-	expectedAuthor := "author"
36
+	const (
37
+		expectedURL            = "/commit"
38
+		expectedContainerID    = "container_id"
39
+		specifiedReference     = "repository_name:tag"
40
+		expectedRepositoryName = "docker.io/library/repository_name"
41
+		expectedTag            = "tag"
42
+		expectedComment        = "comment"
43
+		expectedAuthor         = "author"
44
+	)
43 45
 	expectedChanges := []string{"change1", "change2"}
44 46
 
45 47
 	client := &Client{
... ...
@@ -21,7 +21,7 @@ func (cli *Client) ImageCreate(ctx context.Context, parentReference string, opti
21 21
 	}
22 22
 
23 23
 	query := url.Values{}
24
-	query.Set("fromImage", reference.FamiliarName(ref))
24
+	query.Set("fromImage", ref.Name())
25 25
 	query.Set("tag", getAPITagFromNamedRef(ref))
26 26
 	if options.Platform != "" {
27 27
 		query.Set("platform", strings.ToLower(options.Platform))
... ...
@@ -25,11 +25,14 @@ func TestImageCreateError(t *testing.T) {
25 25
 }
26 26
 
27 27
 func TestImageCreate(t *testing.T) {
28
-	expectedURL := "/images/create"
29
-	expectedImage := "test:5000/my_image"
30
-	expectedTag := "sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"
31
-	expectedReference := fmt.Sprintf("%s@%s", expectedImage, expectedTag)
32
-	expectedRegistryAuth := "eyJodHRwczovL2luZGV4LmRvY2tlci5pby92MS8iOnsiYXV0aCI6ImRHOTBid289IiwiZW1haWwiOiJqb2huQGRvZS5jb20ifX0="
28
+	const (
29
+		expectedURL          = "/images/create"
30
+		expectedImage        = "docker.io/test/my_image"
31
+		expectedTag          = "sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"
32
+		specifiedReference   = "test/my_image:latest@" + expectedTag
33
+		expectedRegistryAuth = "eyJodHRwczovL2luZGV4LmRvY2tlci5pby92MS8iOnsiYXV0aCI6ImRHOTBid289IiwiZW1haWwiOiJqb2huQGRvZS5jb20ifX0="
34
+	)
35
+
33 36
 	client := &Client{
34 37
 		client: newMockClient(func(r *http.Request) (*http.Response, error) {
35 38
 			if !strings.HasPrefix(r.URL.Path, expectedURL) {
... ...
@@ -58,7 +61,7 @@ func TestImageCreate(t *testing.T) {
58 58
 		}),
59 59
 	}
60 60
 
61
-	createResponse, err := client.ImageCreate(context.Background(), expectedReference, image.CreateOptions{
61
+	createResponse, err := client.ImageCreate(context.Background(), specifiedReference, image.CreateOptions{
62 62
 		RegistryAuth: expectedRegistryAuth,
63 63
 	})
64 64
 	if err != nil {
... ...
@@ -26,7 +26,7 @@ func (cli *Client) ImagePull(ctx context.Context, refStr string, options image.P
26 26
 	}
27 27
 
28 28
 	query := url.Values{}
29
-	query.Set("fromImage", reference.FamiliarName(ref))
29
+	query.Set("fromImage", ref.Name())
30 30
 	if !options.All {
31 31
 		query.Set("tag", getAPITagFromNamedRef(ref))
32 32
 	}
... ...
@@ -74,7 +74,7 @@ func TestImagePullWithUnauthorizedErrorAndAnotherUnauthorizedError(t *testing.T)
74 74
 }
75 75
 
76 76
 func TestImagePullWithPrivilegedFuncNoError(t *testing.T) {
77
-	expectedURL := "/images/create"
77
+	const expectedURL = "/images/create"
78 78
 	client := &Client{
79 79
 		client: newMockClient(func(req *http.Request) (*http.Response, error) {
80 80
 			if !strings.HasPrefix(req.URL.Path, expectedURL) {
... ...
@@ -92,8 +92,8 @@ func TestImagePullWithPrivilegedFuncNoError(t *testing.T) {
92 92
 			}
93 93
 			query := req.URL.Query()
94 94
 			fromImage := query.Get("fromImage")
95
-			if fromImage != "myimage" {
96
-				return nil, fmt.Errorf("fromimage not set in URL query properly. Expected '%s', got %s", "myimage", fromImage)
95
+			if fromImage != "docker.io/library/myimage" {
96
+				return nil, fmt.Errorf("fromimage not set in URL query properly. Expected '%s', got %s", "docker.io/library/myimage", fromImage)
97 97
 			}
98 98
 			tag := query.Get("tag")
99 99
 			if tag != "latest" {
... ...
@@ -125,8 +125,10 @@ func TestImagePullWithPrivilegedFuncNoError(t *testing.T) {
125 125
 }
126 126
 
127 127
 func TestImagePullWithoutErrors(t *testing.T) {
128
-	expectedURL := "/images/create"
129
-	expectedOutput := "hello world"
128
+	const (
129
+		expectedURL    = "/images/create"
130
+		expectedOutput = "hello world"
131
+	)
130 132
 	pullCases := []struct {
131 133
 		all           bool
132 134
 		reference     string
... ...
@@ -136,61 +138,88 @@ func TestImagePullWithoutErrors(t *testing.T) {
136 136
 		{
137 137
 			all:           false,
138 138
 			reference:     "myimage",
139
-			expectedImage: "myimage",
139
+			expectedImage: "docker.io/library/myimage",
140 140
 			expectedTag:   "latest",
141 141
 		},
142 142
 		{
143 143
 			all:           false,
144 144
 			reference:     "myimage:tag",
145
-			expectedImage: "myimage",
145
+			expectedImage: "docker.io/library/myimage",
146 146
 			expectedTag:   "tag",
147 147
 		},
148 148
 		{
149 149
 			all:           true,
150 150
 			reference:     "myimage",
151
-			expectedImage: "myimage",
151
+			expectedImage: "docker.io/library/myimage",
152 152
 			expectedTag:   "",
153 153
 		},
154 154
 		{
155 155
 			all:           true,
156 156
 			reference:     "myimage:anything",
157
-			expectedImage: "myimage",
157
+			expectedImage: "docker.io/library/myimage",
158 158
 			expectedTag:   "",
159 159
 		},
160
+		{
161
+			reference:     "myname/myimage",
162
+			expectedImage: "docker.io/myname/myimage",
163
+			expectedTag:   "latest",
164
+		},
165
+		{
166
+			reference:     "docker.io/myname/myimage",
167
+			expectedImage: "docker.io/myname/myimage",
168
+			expectedTag:   "latest",
169
+		},
170
+		{
171
+			reference:     "index.docker.io/myname/myimage:tag",
172
+			expectedImage: "docker.io/myname/myimage",
173
+			expectedTag:   "tag",
174
+		},
175
+		{
176
+			reference:     "localhost/myname/myimage",
177
+			expectedImage: "localhost/myname/myimage",
178
+			expectedTag:   "latest",
179
+		},
180
+		{
181
+			reference:     "registry.example.com:5000/myimage:tag",
182
+			expectedImage: "registry.example.com:5000/myimage",
183
+			expectedTag:   "tag",
184
+		},
160 185
 	}
161 186
 	for _, pullCase := range pullCases {
162
-		client := &Client{
163
-			client: newMockClient(func(req *http.Request) (*http.Response, error) {
164
-				if !strings.HasPrefix(req.URL.Path, expectedURL) {
165
-					return nil, fmt.Errorf("Expected URL '%s', got '%s'", expectedURL, req.URL)
166
-				}
167
-				query := req.URL.Query()
168
-				fromImage := query.Get("fromImage")
169
-				if fromImage != pullCase.expectedImage {
170
-					return nil, fmt.Errorf("fromimage not set in URL query properly. Expected '%s', got %s", pullCase.expectedImage, fromImage)
171
-				}
172
-				tag := query.Get("tag")
173
-				if tag != pullCase.expectedTag {
174
-					return nil, fmt.Errorf("tag not set in URL query properly. Expected '%s', got %s", pullCase.expectedTag, tag)
175
-				}
176
-				return &http.Response{
177
-					StatusCode: http.StatusOK,
178
-					Body:       io.NopCloser(bytes.NewReader([]byte(expectedOutput))),
179
-				}, nil
180
-			}),
181
-		}
182
-		resp, err := client.ImagePull(context.Background(), pullCase.reference, image.PullOptions{
183
-			All: pullCase.all,
187
+		t.Run(pullCase.reference, func(t *testing.T) {
188
+			client := &Client{
189
+				client: newMockClient(func(req *http.Request) (*http.Response, error) {
190
+					if !strings.HasPrefix(req.URL.Path, expectedURL) {
191
+						return nil, fmt.Errorf("Expected URL '%s', got '%s'", expectedURL, req.URL)
192
+					}
193
+					query := req.URL.Query()
194
+					fromImage := query.Get("fromImage")
195
+					if fromImage != pullCase.expectedImage {
196
+						return nil, fmt.Errorf("fromimage not set in URL query properly. Expected '%s', got %s", pullCase.expectedImage, fromImage)
197
+					}
198
+					tag := query.Get("tag")
199
+					if tag != pullCase.expectedTag {
200
+						return nil, fmt.Errorf("tag not set in URL query properly. Expected '%s', got %s", pullCase.expectedTag, tag)
201
+					}
202
+					return &http.Response{
203
+						StatusCode: http.StatusOK,
204
+						Body:       io.NopCloser(bytes.NewReader([]byte(expectedOutput))),
205
+					}, nil
206
+				}),
207
+			}
208
+			resp, err := client.ImagePull(context.Background(), pullCase.reference, image.PullOptions{
209
+				All: pullCase.all,
210
+			})
211
+			if err != nil {
212
+				t.Fatal(err)
213
+			}
214
+			body, err := io.ReadAll(resp)
215
+			if err != nil {
216
+				t.Fatal(err)
217
+			}
218
+			if string(body) != expectedOutput {
219
+				t.Fatalf("expected '%s', got %s", expectedOutput, string(body))
220
+			}
184 221
 		})
185
-		if err != nil {
186
-			t.Fatal(err)
187
-		}
188
-		body, err := io.ReadAll(resp)
189
-		if err != nil {
190
-			t.Fatal(err)
191
-		}
192
-		if string(body) != expectedOutput {
193
-			t.Fatalf("expected '%s', got %s", expectedOutput, string(body))
194
-		}
195 222
 	}
196 223
 }
... ...
@@ -29,7 +29,6 @@ func (cli *Client) ImagePush(ctx context.Context, image string, options image.Pu
29 29
 		return nil, errors.New("cannot push a digest reference")
30 30
 	}
31 31
 
32
-	name := reference.FamiliarName(ref)
33 32
 	query := url.Values{}
34 33
 	if !options.All {
35 34
 		ref = reference.TagNameOnly(ref)
... ...
@@ -52,13 +51,13 @@ func (cli *Client) ImagePush(ctx context.Context, image string, options image.Pu
52 52
 		query.Set("platform", string(pJson))
53 53
 	}
54 54
 
55
-	resp, err := cli.tryImagePush(ctx, name, query, options.RegistryAuth)
55
+	resp, err := cli.tryImagePush(ctx, ref.Name(), query, options.RegistryAuth)
56 56
 	if errdefs.IsUnauthorized(err) && options.PrivilegeFunc != nil {
57 57
 		newAuthHeader, privilegeErr := options.PrivilegeFunc(ctx)
58 58
 		if privilegeErr != nil {
59 59
 			return nil, privilegeErr
60 60
 		}
61
-		resp, err = cli.tryImagePush(ctx, name, query, newAuthHeader)
61
+		resp, err = cli.tryImagePush(ctx, ref.Name(), query, newAuthHeader)
62 62
 	}
63 63
 	if err != nil {
64 64
 		return nil, err
... ...
@@ -79,7 +79,7 @@ func TestImagePushWithUnauthorizedErrorAndAnotherUnauthorizedError(t *testing.T)
79 79
 }
80 80
 
81 81
 func TestImagePushWithPrivilegedFuncNoError(t *testing.T) {
82
-	expectedURL := "/images/myimage/push"
82
+	const expectedURL = "/images/docker.io/myname/myimage/push"
83 83
 	client := &Client{
84 84
 		client: newMockClient(func(req *http.Request) (*http.Response, error) {
85 85
 			if !strings.HasPrefix(req.URL.Path, expectedURL) {
... ...
@@ -109,7 +109,7 @@ func TestImagePushWithPrivilegedFuncNoError(t *testing.T) {
109 109
 	privilegeFunc := func(_ context.Context) (string, error) {
110 110
 		return "IAmValid", nil
111 111
 	}
112
-	resp, err := client.ImagePush(context.Background(), "myimage:tag", image.PushOptions{
112
+	resp, err := client.ImagePush(context.Background(), "myname/myimage:tag", image.PushOptions{
113 113
 		RegistryAuth:  "NotValid",
114 114
 		PrivilegeFunc: privilegeFunc,
115 115
 	})
... ...
@@ -126,8 +126,10 @@ func TestImagePushWithPrivilegedFuncNoError(t *testing.T) {
126 126
 }
127 127
 
128 128
 func TestImagePushWithoutErrors(t *testing.T) {
129
-	expectedOutput := "hello world"
130
-	expectedURLFormat := "/images/%s/push"
129
+	const (
130
+		expectedURLFormat = "/images/%s/push"
131
+		expectedOutput    = "hello world"
132
+	)
131 133
 	testCases := []struct {
132 134
 		all           bool
133 135
 		reference     string
... ...
@@ -137,27 +139,52 @@ func TestImagePushWithoutErrors(t *testing.T) {
137 137
 		{
138 138
 			all:           false,
139 139
 			reference:     "myimage",
140
-			expectedImage: "myimage",
140
+			expectedImage: "docker.io/library/myimage",
141 141
 			expectedTag:   "latest",
142 142
 		},
143 143
 		{
144 144
 			all:           false,
145 145
 			reference:     "myimage:tag",
146
-			expectedImage: "myimage",
146
+			expectedImage: "docker.io/library/myimage",
147 147
 			expectedTag:   "tag",
148 148
 		},
149 149
 		{
150 150
 			all:           true,
151 151
 			reference:     "myimage",
152
-			expectedImage: "myimage",
152
+			expectedImage: "docker.io/library/myimage",
153 153
 			expectedTag:   "",
154 154
 		},
155 155
 		{
156 156
 			all:           true,
157 157
 			reference:     "myimage:anything",
158
-			expectedImage: "myimage",
158
+			expectedImage: "docker.io/library/myimage",
159 159
 			expectedTag:   "",
160 160
 		},
161
+		{
162
+			reference:     "myname/myimage",
163
+			expectedImage: "docker.io/myname/myimage",
164
+			expectedTag:   "latest",
165
+		},
166
+		{
167
+			reference:     "docker.io/myname/myimage",
168
+			expectedImage: "docker.io/myname/myimage",
169
+			expectedTag:   "latest",
170
+		},
171
+		{
172
+			reference:     "index.docker.io/myname/myimage:tag",
173
+			expectedImage: "docker.io/myname/myimage",
174
+			expectedTag:   "tag",
175
+		},
176
+		{
177
+			reference:     "localhost/myname/myimage",
178
+			expectedImage: "localhost/myname/myimage",
179
+			expectedTag:   "latest",
180
+		},
181
+		{
182
+			reference:     "registry.example.com:5000/myimage:tag",
183
+			expectedImage: "registry.example.com:5000/myimage",
184
+			expectedTag:   "tag",
185
+		},
161 186
 	}
162 187
 	for _, tc := range testCases {
163 188
 		t.Run(fmt.Sprintf("%s,all-tags=%t", tc.reference, tc.all), func(t *testing.T) {
... ...
@@ -26,7 +26,7 @@ func (cli *Client) ImageTag(ctx context.Context, source, target string) error {
26 26
 	ref = reference.TagNameOnly(ref)
27 27
 
28 28
 	query := url.Values{}
29
-	query.Set("repo", reference.FamiliarName(ref))
29
+	query.Set("repo", ref.Name())
30 30
 	if tagged, ok := ref.(reference.Tagged); ok {
31 31
 		query.Set("tag", tagged.Tag())
32 32
 	}
... ...
@@ -95,7 +95,7 @@ func TestImageTagHexSource(t *testing.T) {
95 95
 }
96 96
 
97 97
 func TestImageTag(t *testing.T) {
98
-	expectedURL := "/images/image_id/tag"
98
+	const expectedURL = "/images/image_id/tag"
99 99
 	tagCases := []struct {
100 100
 		reference           string
101 101
 		expectedQueryParams map[string]string
... ...
@@ -103,37 +103,37 @@ func TestImageTag(t *testing.T) {
103 103
 		{
104 104
 			reference: "repository:tag1",
105 105
 			expectedQueryParams: map[string]string{
106
-				"repo": "repository",
106
+				"repo": "docker.io/library/repository",
107 107
 				"tag":  "tag1",
108 108
 			},
109 109
 		}, {
110 110
 			reference: "another_repository:latest",
111 111
 			expectedQueryParams: map[string]string{
112
-				"repo": "another_repository",
112
+				"repo": "docker.io/library/another_repository",
113 113
 				"tag":  "latest",
114 114
 			},
115 115
 		}, {
116 116
 			reference: "another_repository",
117 117
 			expectedQueryParams: map[string]string{
118
-				"repo": "another_repository",
118
+				"repo": "docker.io/library/another_repository",
119 119
 				"tag":  "latest",
120 120
 			},
121 121
 		}, {
122 122
 			reference: "test/another_repository",
123 123
 			expectedQueryParams: map[string]string{
124
-				"repo": "test/another_repository",
124
+				"repo": "docker.io/test/another_repository",
125 125
 				"tag":  "latest",
126 126
 			},
127 127
 		}, {
128 128
 			reference: "test/another_repository:tag1",
129 129
 			expectedQueryParams: map[string]string{
130
-				"repo": "test/another_repository",
130
+				"repo": "docker.io/test/another_repository",
131 131
 				"tag":  "tag1",
132 132
 			},
133 133
 		}, {
134 134
 			reference: "test/test/another_repository:tag1",
135 135
 			expectedQueryParams: map[string]string{
136
-				"repo": "test/test/another_repository",
136
+				"repo": "docker.io/test/test/another_repository",
137 137
 				"tag":  "tag1",
138 138
 			},
139 139
 		}, {