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

Conversation

@g-linville
Copy link
Contributor

for #1427

This change ensures that apps with auto-upgrade enabled will never implicitly pull images from Docker Hub.

This is important in order to avoid dependency confusion. Since auto-upgrade apps, unlike normal apps, will give priority to remote images when resolving, it would be possible for an attacker to create a public image that matches the name of a private, local-only image, and Acorn would pick it up and auto-upgrade to that. This PR prevents that from being possible.

Unfortunately, there were three separate areas where I had to implement this logic:

  • Get image details
  • App validation in the apiserver
  • Pull app image

I wrote tests (two unit, one integration) to check for this logic in all three places.

Currently, prior to this PR, it is possible for auto-upgrade apps to implicitly use Docker Hub as long as they aren't using a tag pattern (e.g. acorn run -n nginx grantlinville/nginx:latest works and uses my image from Docker Hub). This will no longer work as soon as this is merged. For running apps that are like this, they will continue to run, but there will be an error in their status until their image is updated using acorn update (or until a local image that matches the auto-upgrade is built).

Checklist

  • The title of this PR would make a good line in Acorn's Release Note's Changelog
  • The title of this PR ends with a link to the main issue being address in parentheses, like: This is a title (#1216). Here's an example
  • All relevant issues are referenced in the PR description. NOTE: don't use GitHub keywords that auto-close issues
  • Commits follow contributing guidance
  • Automated tests added to cover the changes. If tests couldn't be added, an explanation is provided in the Verification and Testing section
  • Changes to user-facing functionality, API, CLI, and upgrade impacts are clearly called out in PR description
  • PR has at least two approvals before merging (or a reasonable exception, like it's just a docs change)

@g-linville g-linville requested review from cjellick and thedadams June 23, 2023 19:35
Signed-off-by: Grant Linville <grant@acorn.io>
Signed-off-by: Grant Linville <grant@acorn.io>
Signed-off-by: Grant Linville <grant@acorn.io>
Signed-off-by: Grant Linville <grant@acorn.io>
Signed-off-by: Grant Linville <grant@acorn.io>
Signed-off-by: Grant Linville <grant@acorn.io>
@g-linville g-linville force-pushed the autoupgrade-better-error branch from c8ddc80 to c67f3bd Compare June 23, 2023 21:34
@g-linville g-linville mentioned this pull request Jun 26, 2023
7 tasks
@g-linville g-linville merged commit 0be0a4c into acorn-io:main Jun 27, 2023
@g-linville g-linville deleted the autoupgrade-better-error branch June 27, 2023 00:23
cloudnautique pushed a commit to cloudnautique/runtime that referenced this pull request 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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants