[Feature Request]: Report results of BYON URL validation in the modal

[bdattoma]

Feature description

Currently while importing a custom image from Dashboard, if the URL validation fails, the modal gets closed and user need to re-insert the data in a new Import session. That could impact user experience, especially if user inserted all the pkgs/software information.

Would it be possible to report the validation error in the form itself? Letting the user fixing the url format.

Describe alternatives you've considered

No response

Anything else?

No response

[andrewballantyne]

So this may be possible in time -- but it's not ideal today because the error is an async response from OpenShift. So we can "lock you in as we do it" but that's a whole UX flow in itself.

@DaoDaoNoCode you went through the testing scenarios and know more about this.
@vconzola is the designer on this and agreed to showing the error after the modal.

If this is possible, we would need a unique UX flow for this that sorta "sits and waits" -- may be a long wait. And then we'd need to support deletion of the image stream & tag if it is unsuccessful (which could also fail).

This may not be easily done nor do I expect a nice UX for this... but today the UX is "start over" -- so maybe there is room to improve. cc @kywalker-rh

I'm inclined to close this as won't do -- but let us give it the shake it deserves to see if we want to invest in some other mechanism to give a better experience in the end.

[shalberd]

Hi, I agree with @andrewballantyne . You should validate the url in dashboard code from a pure format-perspectice before the imagestream yaml gets submitted to the openshift server. So as to not have dependencies on Openshift. Which is I think what you already do, don't you? Regex-like.

Also, just tested this with the latest version of master: it is nice that source of a byon imagestream (tag.from.name) can also be in sha256 digest format myreposerver.com/myrepo/image@sha256s2929d91919, cool thing.

If the initiator of the request means just url format validation, then yes, you could just give back a modal response on the url field, while not deleting the other UI fields. Before pressing the button, i.e. after changing focus away from the url field in the GUI.

Now, whether the url is actually valid in the sense that it is externally correct and can be resolved to an image after the imagestream is applied is really up to the user, I would not check the status-section of the created imagestream, especially because you can have delay / async issues. Users have at least view rights on the namespace in openshift, they can look at the imagestream status themselves if they want; just my two cents. Over here, we say "that's not our beer (responsibility)" ;-)

[andrewballantyne]

Thanks for the input Sven, as well as the colloquial phrase haha.

There is definitely limitations here for what we can do. Not sure there is an easy way to validate an image tag without sending it through the OpenShift infra. But we can try to validate the image structure BEFORE submit, as you mentioned - we are using regexp on the backend to split it apart. Might be worth investigating if we can do all that on the frontend and send the parts broken to the backend to have all the logic in once place 🤔

This older area of the code is definitely unique and suffers a lot from our old (now deprecated) design of business logic in the backend.

[kywalker-rh]

I agree with the idea of making this experience slightly better, its not great that all the info you just entered could be deleted if you made a mistake in the url. If its especially difficult to validate the url, could we instead remember everything the user entered and allow them to go back to a pre-filled form if it fails? I could see that working as a link from a toast alert.

[vconzola]

I don't recall agreeing to the UX, because I didn't design it. Could we not save the image object in RHODS and just mark it as pending while it's validated by OpenShift. Then change it to either OK or error status once the validation is complete?

It looks like today we don't show any error. If I enter an invalid image repository in the Import modal the input gets accepted and it looks like the only way I know it's invalid is that the image name is disabled in the selection dropdown when creating a workbench.

[andrewballantyne]

I don't recall agreeing to the UX, because I didn't design it, but whatever.

@vconzola This is the conversation: slack link.

[vconzola]

OK. Thanks for reminding me - did not remember that conversation. I didn't do the original custom runtimes design, but I stand by the UX I recommended in Slack. Can we do that?

[andrewballantyne]

We have no real way to stop the modal -- we are creating the object -- if we want to wait some time for that resource to settle in the k8s system before closing the modal, that'll be a long time for the user.

We "hide" the image when it fails during that validation cycle as per the request of Vince in that slack thread.

So unless @kywalker-rh and/or @vconzola wants to lock up the modal (or something) to give the user a chance to "delete and recreate" the name... I think this is not a doable issue in the current infra of our backend (which uses OpenShift Console to have the resource to test it out).

[vconzola]

@andrewballantyne I'm not suggesting we hide the image in the notebook images table. I'm suggesting we hide invalid images from the end user when creating a workbench. Let me mock something up to show you what I'm talking about. I'm not doing a good job of describing it in words.

[bdattoma]

If its especially difficult to validate the url, could we instead remember everything the user entered and allow them to go back to a pre-filled form if it fails? I could see that working as a link from a toast alert.

as alternative to remember form value which can bring to another issue (i.e., I create a new runtime but the modal get auto-filled with older values, like it was happening with Data connections in the past), we could make the "Edit" action able to change the URL. I kind of remember there is an existing issue for it downstream.

@andrewballantyne is the backend checking the image points to an existent and accessible domain? I thought it was just regex checking the URL (here). During a test I inserted a fake URL which met the expected regex and didn't get any issue in the dashboard. If you say it checks the accessibility of the URL, I didn't wait enough probably

[andrewballantyne]

@bdattoma it is my understand that it is validated by the image stream we create through the mechanisms of creating the tag in OpenShift Console. It's not done by our end.

If you specified something that did not create an error but you knew to not be valid, my only question is what did you test with so I can test and repeat? I was fairly certain that's what was happening.

[shalberd]

Ah, so you look at the imagestream status-section? Or you could do that, @bdattoma is suggesting? i.e. looking at whether dockerImageReference is resolved. If there were no value in there, then the original quai.io url (imagestream from.name) would be invalid. Nice to have, but not essential in my opinion. Admins of the namespace can view imagestreams and their status from the openshift web console, probably even those with View clusterrole.

BYON is probably not fully compatible with on-prem clusters that don't have an openshift-internal docker registry (dockerImageRepository empty), but that is fine by me. Our images in the third-party docker registry (Harbor, Artifactory, Nexus, you name it) are more curated and no interest in / firewall rule for importing them from the public internet :-) to be specific, on-prem customers would put their own, additional imagestream yamls in e.g. a gitops workflow and apply them.

[andrewballantyne]

@shalberd we already make use of the status message -- #1249

const getBYONImageErrorMessage = (imageStream: ImageStream) => {
  // there will be always only 1 tag in the spec for BYON images
  // status tags could be more than one
  const activeTag = imageStream.status?.tags?.find(
    (statusTag) => statusTag.tag === imageStream.spec.tags?.[0].name,
  );
  return activeTag?.conditions?.[0]?.message;
};

Looking back on this now, that seems somewhat odd logic. Maybe ImageStreams don't have conditions when they successfully find the source?

[shalberd]

@andrewballantyne @vconzola Mmh, yes, error relates to a string currently. Better would be, similar to not only looking at the boolean isBYON in your code, instead of looking at the condition's message, look at a combination of type and status

https://docs.openshift.com/container-platform/4.10/rest_api/image_apis/imagestream-image-openshift-io-v1.html#status-tags-conditions

.message does not have to mean an error necessarily.

conditions?.[0]?.type currently in OCP only string "ImportSuccess"
conditions?.[0]?.status i.e. for "ImportSuccess", there is a string here at status: Status of the condition, one of True, False, Unknown

I don't know, it could be that when the source (from.name external) is not found, that there is no such status transition, but to me that seems highly unlikely.

There should be ImportSuccess in condition type. Maybe someone can post here or look at imagestream status.tag[x}.conditions

• for a successful and valid import image

• vs one known to be false / wrong at quay.io here.

In the case of no internal docker registry

status:
  dockerImageRepository: ''

there definitely is no array or section tag[x].conditions with the fields above. That makes sense, though, since the ImageStream conditions (ImportSuccess) work only together with the internal openshift docker registry. Also, for airgapped, it is important that tag.from.name always be in digest-format. As mentioned, I think the BYON is really not for customer running airgapped environments with ImageContentSourcePolicy and without the internal openshift docker registry. In any case, in terms of code and behavior, it'd be interesting to have snippets of status.tag.conditions from other users here.

@lucferbux You currently only have (getBYONImageErrorMessage) that in the backend logic, correct?

[andrewballantyne]

@shalberd yes, that's the currently the only implemented logic. BYON is a bit of a patchwork of code. There definitely should be more logic around reading conditions... my best guess is @DaoDaoNoCode coded this originally when he found the conditions don't appear for successful image streams. We'll need to improve it.

[bdattoma]

was this solved in the last dev cycle of the custom notebook images? @andrewballantyne

[andrewballantyne]

@bdattoma no it was not.

[bdattoma]

@bdattoma no it was not.

ok thanks. Are you planning on migrating this ticket to jira?

[andrewballantyne]

@bdattoma it is your issue, please feel free to migrate it.

[bdattoma]

I give it another try, and I'm not able to reproduce the issue anymore. If the URL has a wrong format, it complains inside the modal. If the URL actually doesn't exist, it complains after I submit the modal, but it keeps the data I've submitted

[dgutride]

Closing based on the last comment - @bdattoma - please feel free to open in jira if this is a mistake.