Skip to content

Commit 038541a

Browse files
committed
fix: Registry preflight check URL parse error is not an internal error
- If the URL cannot be parsed, fails the check, but does not use an internal error. - Removes redundant parsing, and removes related error handling.
1 parent 6cb777d commit 038541a

File tree

2 files changed

+53
-14
lines changed

2 files changed

+53
-14
lines changed

pkg/webhook/preflight/generic/registry.go

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ func (r *registryCheck) checkRegistry(
7676
registryURLParsed, err := url.ParseRequestURI(registryURL)
7777
if err != nil {
7878
result.Allowed = false
79-
result.Error = true
79+
result.Error = false
8080
result.Causes = append(result.Causes,
8181
preflight.Cause{
8282
Message: fmt.Sprintf("failed to parse registry url %s with error: %s", registryURL, err),
@@ -125,19 +125,15 @@ func (r *registryCheck) checkRegistry(
125125
regclient.WithConfigHost(mirrorHost),
126126
regclient.WithUserAgent("regclient/example"),
127127
)
128-
mirrorRef, err := ref.NewHost(registryURLParsed.Host)
129-
if err != nil {
130-
result.Allowed = false
131-
result.Error = true
132-
result.Causes = append(result.Causes,
133-
preflight.Cause{
134-
Message: fmt.Sprintf("failed to create a client to verify registry configuration %s", err.Error()),
135-
Field: r.field,
136-
},
137-
)
138-
return result
139-
}
140-
_, err = rc.Ping(ctx, mirrorRef) // ping will return an error for anything that's not 200
128+
_, err = rc.Ping(ctx,
129+
ref.Ref{
130+
// Because we ping the registry, we only need the "registry" part of the ref.
131+
Registry: registryURLParsed.Host,
132+
// The default scheme is "reg""
133+
Scheme: "reg",
134+
},
135+
)
136+
// Ping will return an error for anything that's not 200.
141137
if err != nil {
142138
result.Allowed = false
143139
result.Causes = append(result.Causes,

pkg/webhook/preflight/generic/registry_test.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,49 @@ func TestRegistryCheck(t *testing.T) {
199199
Allowed: true,
200200
},
201201
},
202+
{
203+
name: "image registry with invalid URL",
204+
field: "cluster.spec.topology.variables[.name=clusterConfig].value.imageRegistries[0]",
205+
imageRegistry: &carenv1.ImageRegistry{
206+
URL: "invalid-url",
207+
Credentials: &carenv1.RegistryCredentials{
208+
SecretRef: &carenv1.LocalObjectReference{
209+
Name: "test-secret",
210+
},
211+
},
212+
},
213+
kclient: &mockK8sClient{
214+
getSecretFunc: func(ctx context.Context,
215+
key types.NamespacedName,
216+
obj ctrlclient.Object,
217+
opts ...ctrlclient.GetOption,
218+
) error {
219+
secret := obj.(*corev1.Secret)
220+
secret.Data = map[string][]byte{
221+
"username": []byte("testuser"),
222+
"password": []byte("testpass"),
223+
"ca.crt": []byte("test-ca-cert"),
224+
}
225+
return nil
226+
},
227+
},
228+
mockRegClientPingerFactory: func(...regclient.Opt) regClientPinger {
229+
return &mockRegClient{
230+
pingFunc: func(ref.Ref) error { return nil },
231+
}
232+
},
233+
want: preflight.CheckResult{
234+
Allowed: false,
235+
Error: false,
236+
Causes: []preflight.Cause{
237+
{
238+
Message: fmt.Sprintf("failed to parse registry url %s with error: "+
239+
"parse \"invalid-url\": invalid URI for request", "invalid-url"),
240+
Field: "cluster.spec.topology.variables[.name=clusterConfig].value.imageRegistries[0].url",
241+
},
242+
},
243+
},
244+
},
202245
}
203246

204247
for _, tc := range testCases {

0 commit comments

Comments
 (0)