-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Failed to parse image ${inputs.params.DOCKER_IMAGE}: could not parse reference #885
Comments
Similar with #811. |
Like @gavinfish said, I think what's happening here is that we aren't applying the templating to all fields, and as discussed in #811 we might as well :D I'm going to close this in favor of #811. (Please re-open if we missed something @cmoulliard !) |
A few remarks:
|
It took me a few minutes to figure this out but the reason is actually pretty disappointing XD This specific error ("Failed to fetch remote image") is caused by an implementation detail: we override the 'entrypoint'/'cmd' provided in the 🙏
That part is definitely wrong (@sbwsg I think this is that same code that you were fixing up in #876 (comment)).
So this is happening b/c this is the first bit of code that tries to interact with the value of I think you're totally right though, we should catch that error earlier. The problem is that b/c we aren't applying any templating for those values, it's pretty hard to know that the user intended templating to work there and it didn't. I guess we could find values that look like
@cmoulliard can I ask a bit more about why you want to provide the images as params? Do you have a use case where you expect these to be swapped out? So far we have been assuming it would be find to hardcode image locations in Tasks ( + @dlorenc this might be relevant for Task entries in our catalog - maybe folks need to be able to swap out the images and/or the registry) |
This is because the URL of the image will change according to the registry where it will be published. when a local docker registry is available, then we will use the following by example URL Make sense @bobcatfish ? |
Makes sense @cmoulliard - that's an interesting use case. I think for local registries / images we might want to have some other top level solution to this (related to #235 maybe). |
Error
The following task when processed generates this error
Config
The text was updated successfully, but these errors were encountered: