Skip to content
This repository has been archived by the owner on Mar 16, 2024. It is now read-only.

Not able to deploy apps using dockerhub images in autoupgrade format without using "docker.io" in image name. #1427

Closed
sangee2004 opened this issue Apr 3, 2023 · 7 comments
Assignees
Labels
kind/bug Something isn't working
Milestone

Comments

@sangee2004
Copy link
Contributor

sangee2004 commented Apr 3, 2023

acorn version v0.6.0-106-g09f636a2+09f636a2

This behavior is also seen in older versions.

Steps to reproduce the problem:

1.  acorn run --interval 15s -n check1 docker.io/sangeetha/myfirstacorn:v#.#.# ==> works

2.  acorn run --interval 15s -n mytestautonew1 sangeetha/myfirstacorn:v#.#.# ==> does not work 

3.  acorn run  sangeetha/myfirstacorn:v6.0.0 ==> works (No autoupgrade format) 

Do we expect #2 work ? #1 working and #3 working but #2 not working is confusing behavior.

@sangee2004 sangee2004 added the kind/bug Something isn't working label Apr 3, 2023
@sangee2004 sangee2004 added this to the v0.7.0 milestone Apr 3, 2023
@iwilltry42
Copy link
Contributor

IIRC someone once said that we'll never assume some default registry when handling images? Maybe that should be popped up in a discussion round?

@cjellick cjellick modified the milestones: v0.7.0, v0.8.0 Apr 11, 2023
@g-linville
Copy link
Contributor

g-linville commented Jun 20, 2023

I have a draft PR for this here: #1817

However, this has a major consequence that I really do not like.

For auto-upgrade, the rule currently is that we will always prioritize/prefer remote images over local ones when resolving. If we allow the implicit use of Docker Hub, then we open up users to a dependency confusion attack, which could play out like this:

  1. I build a local image and tag it with myproject/myimage:v1.0.0
  2. I run the local image in an auto-upgrade app with acorn run -n myapp myproject/myimage:v#.#.#
  3. A threat actor uploads a public Docker Hub image called index.docker.io/myproject/myimage:v2.0.0
  4. Because Acorn prefers remote images over local images for auto-upgrade, the auto-upgrade daemon finds the image on Docker Hub and updates to it

Imo we should not support having an implicit Docker Hub registry for auto-upgrade. It opens up too many risks.

Curious to hear everyone's thoughts.

Edit: a possible workaround would be to note somewhere in the AppInstance that the image started as a local image and should never switch to a remote one when auto-upgrading. That sounds a little gross but could be doable. I want to get confirmation that that's actually something that we want before I proceed forward with it though.

@tylerslaton
Copy link
Contributor

tylerslaton commented Jun 20, 2023

@g-linville However, this has a major consequence that I really do not like.

I have two thoughts here:

  1. Whatever we do, should we make it consistent? As Sangeetha points out well here - there's a difference in behavior between the two modes. As a user I personally would expect both situations to act similarly and if they don't its clearly communicated.
  2. Just from what you've outline here, I'd say that requiring a fully-qualified (for lack of a better term) image during auto-upgrades to prevent the situation you're describing makes sense. Could we add (or does one already exist?) an API server error that rejects any auto-upgrade images that are ambiguous?

@g-linville
Copy link
Contributor

g-linville commented Jun 20, 2023

Whatever we do, should we make it consistent? As Sangeetha points out well here - there's a difference in behavior between the two modes. As a user I personally would expect both situations to act similarly and if they don't its clearly communicated.

There's still some inherent difference between the behavior of auto-upgrade and non-auto-upgrade, which is whether local images or remote images are preferred when resolving. We have to communicate that to the user somewhere, so imo it's not too much of an additional difference to say that users have to specify docker.io or index.docker.io when using auto-upgrade, if that's where their image lives.

Just from what you've outline here, I'd say that requiring a fully-qualified (for lack of a better term) image during auto-upgrades to prevent the situation you're describing makes sense. Could we add (or does one already exist?) an API server error that rejects any auto-upgrade images that are ambiguous?

I think this is the right idea.

@cjellick
Copy link
Member

Whatever we do, should we make it consistent? As Sangeetha points out well here - there's a difference in behavior between the two modes. As a user I personally would expect both situations to act similarly and if they don't its clearly communicated.

This was the crux of a debate/convo between Darren, Donnie, Grant, Sangeetha, and I. We concluded that consistency was not the most important thing here. I was originally arguing for consistency above all, but was convinced otherwise. Darren was the major proponent of auto-upgrade preferring remote and non-auto-upgrade preferring local. His argument being that they are two distinct use cases with different expectations. I think either way we go, we will eventually confuse some users.

All that said, I agree with Tyler's second point. Let's insist that auto-upgrade images be fully-qualified and just make sure the error is clear and explicit.

@g-linville
Copy link
Contributor

All of my auto-upgrade fixes have been merged. Ultimately we decided that the user does need to explicitly provide the image registry if they are using or referring to non-local images in auto-upgrade apps.

@sangee2004
Copy link
Contributor Author

Tested with acorn version v0.8.0-alpha7-98-gbb130866+bb130866

User is provided with following error message when trying to using images in autoupgrade format without providing registry info.

acorn run --interval 15s -n mytestautonew1 sangeetha/myfirstacorn:v#.#.# 
  ✗  ERROR:  unable to find an image for sangeetha/myfirstacorn:v#.#.# matching pattern v#.#.# - if you are trying to use a remote image, specify the full registry

cloudnautique pushed a commit to cloudnautique/runtime that referenced this issue Sep 28, 2023
…specified registry (acorn-io#1427) (acorn-io#1823)

Signed-off-by: Grant Linville <grant@acorn.io>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants