-
Notifications
You must be signed in to change notification settings - Fork 69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix error message on invalid scheme-prefixed ref #185
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a nit, othewise lgtm
7acdbb2
to
8f919d4
Compare
8f919d4
to
b7c65b7
Compare
controllers/scan_test.go
Outdated
Eventually(func() bool { | ||
_ = r.Get(ctx, objectName, &repo) | ||
ready = apimeta.FindStatusCondition(*repo.GetStatusConditions(), meta.ReadyCondition) | ||
fmt.Printf("%v", ready) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left-over debugging println?
@@ -143,6 +143,9 @@ func (r *ImageRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Requ | |||
|
|||
ref, err := name.ParseReference(imageRepo.Spec.Image) | |||
if err != nil { | |||
if u, _ := url.Parse(imageRepo.Spec.Image); u != nil && u.Scheme != "" { | |||
err = fmt.Errorf("image repository attribute should not be scheme-prefixed ('%s://')", u.Scheme) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
err = fmt.Errorf("image repository attribute should not be scheme-prefixed ('%s://')", u.Scheme) | |
err = fmt.Errorf(".spec.image value should not be scheme-prefixed ('%s://')", u.Scheme) |
(i.e., refer to the field by path)
b7c65b7
to
e46d73d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have only a suggestion on the language, which I leave to your discretion -- thanks Aurel 🍍
e46d73d
to
2f9119e
Compare
Adds a descriptive error when an invalid image ref containing a scheme is given to the ImageRepository. Fixes #146 Signed-off-by: Aurel Canciu <[email protected]>
2f9119e
to
b745ca8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Thanks.
Oh -- except parsing image repositories (no scheme, but port numbers with a colon) as though they are URLs (scheme with a colon expected) is going to give spurious errors: #220 |
Adds a descriptive error when an invalid image ref containing a scheme
is given to the ImageRepository.
Fixes #146