Skip to content

Commit f97ca2e

Browse files
committed
fix: address changes requested by @vknabel
1 parent cfe5e20 commit f97ca2e

2 files changed

Lines changed: 18 additions & 19 deletions

File tree

cmd/metal-api/internal/service/image-service.go

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -352,26 +352,25 @@ func (r *imageResource) createImage(request *restful.Request, response *restful.
352352
}
353353

354354
func checkImageURL(id, inputURL string, ociCredentials authn.Authenticator) error {
355-
parsedURL := strings.Split(inputURL, "://")
355+
parsedURL, err := url.Parse(inputURL)
356+
if err != nil {
357+
return fmt.Errorf("inputURL:%q could not be parsed, error:%w", inputURL, err)
358+
}
356359

357-
switch parsedURL[0] {
360+
switch parsedURL.Scheme {
358361
case "http", "https":
359-
_, err := url.ParseRequestURI(inputURL)
360-
if err != nil {
361-
return fmt.Errorf("image:%s could not be parsed. error:%w", inputURL, err)
362-
}
363-
364-
res, err := http.Head(inputURL)
362+
res, err := http.Head(parsedURL.String())
365363
if err != nil {
366-
return fmt.Errorf("image:%s is not accessible under:%s error:%w", id, inputURL, err)
364+
return fmt.Errorf("image:%q is not accessible under:%s, error:%w", id, inputURL, err)
367365
}
368366
if res.StatusCode >= 400 {
369-
return fmt.Errorf("image:%s is not accessible under:%s status:%s", id, inputURL, res.Status)
367+
return fmt.Errorf("image:%q is not accessible under:%s, status:%s", id, inputURL, res.Status)
370368
}
371369
case "oci":
372-
ref, err := name.ParseReference(parsedURL[1])
370+
parsedURLWithoutScheme := strings.TrimPrefix(parsedURL.String(), "oci://")
371+
ref, err := name.ParseReference(parsedURLWithoutScheme)
373372
if err != nil {
374-
return fmt.Errorf("image reference:%s could not be parsed. error:%w", inputURL, err)
373+
return fmt.Errorf("image reference:%q could not be parsed, error:%w", inputURL, err)
375374
}
376375

377376
if ociCredentials == nil {
@@ -380,10 +379,10 @@ func checkImageURL(id, inputURL string, ociCredentials authn.Authenticator) erro
380379

381380
_, err = remote.Image(ref, remote.WithAuth(ociCredentials))
382381
if err != nil {
383-
return fmt.Errorf("image:%s is not accessible under:%s error:%w", id, inputURL, err)
382+
return fmt.Errorf("image:%q is not accessible under:%s, error:%w", id, inputURL, err)
384383
}
385384
default:
386-
return fmt.Errorf("image:%s with url:%s has unkown protocol", id, inputURL)
385+
return fmt.Errorf("image:%q with url:%s has unknown protocol", id, inputURL)
387386
}
388387

389388
return nil

cmd/metal-api/internal/service/image-service_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ func TestDeleteImage(t *testing.T) {
111111
func TestCheckImageURL(t *testing.T) {
112112
t.Run("Invalid URL", func(t *testing.T) {
113113
err := checkImageURL("testID", "http://invalid url", nil)
114-
require.EqualError(t, err, "image:http://invalid url could not be parsed. error:parse \"http://invalid url\": invalid character \" \" in host name")
114+
require.EqualError(t, err, "inputURL:\"http://invalid url\" could not be parsed, error:parse \"http://invalid url\": invalid character \" \" in host name")
115115
})
116116

117117
t.Run("HTTP URL with successful HEAD request", func(t *testing.T) {
@@ -131,12 +131,12 @@ func TestCheckImageURL(t *testing.T) {
131131
defer server.Close()
132132

133133
err := checkImageURL("testID", server.URL, nil)
134-
require.EqualError(t, err, "image:testID is not accessible under:"+server.URL+" status:404 Not Found")
134+
require.EqualError(t, err, "image:\"testID\" is not accessible under:"+server.URL+", status:404 Not Found")
135135
})
136136

137137
t.Run("Unsupported scheme", func(t *testing.T) {
138138
err := checkImageURL("testID", "ftp://unsupported.url", nil)
139-
require.EqualError(t, err, "image:testID with url:ftp://unsupported.url has unkown protocol")
139+
require.EqualError(t, err, "image:\"testID\" with url:ftp://unsupported.url has unknown protocol")
140140
})
141141

142142
t.Run("valid OCI URL", func(t *testing.T) {
@@ -146,7 +146,7 @@ func TestCheckImageURL(t *testing.T) {
146146

147147
t.Run("OCI URL with invalid reference", func(t *testing.T) {
148148
err := checkImageURL("testID", "oci://inva lid", nil)
149-
require.EqualError(t, err, "image reference:oci://inva lid could not be parsed. error:could not parse reference: inva lid")
149+
require.EqualError(t, err, "inputURL:\"oci://inva lid\" could not be parsed, error:parse \"oci://inva lid\": invalid character \" \" in host name")
150150
})
151151
}
152152

@@ -244,7 +244,7 @@ func TestCreateImageWithBrokenURL(t *testing.T) {
244244

245245
err = json.NewDecoder(resp.Body).Decode(&result)
246246
require.NoError(t, err)
247-
require.Equal(t, "image:image-1 is not accessible under:http://images.metal-stack.io/this-file-does-not-exist status:404 Not Found", result.Message)
247+
require.Equal(t, "image:\"image-1\" is not accessible under:http://images.metal-stack.io/this-file-does-not-exist, status:404 Not Found", result.Message)
248248
}
249249

250250
func TestCreateImageWithClassification(t *testing.T) {

0 commit comments

Comments
 (0)