Skip to content
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

kamal app exec may fail if not logged in #1227

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

andrewjanssen
Copy link

Current state

The central Kamal command, kamal deploy, automatically logs you in because it requires access to the container registry.

Unlike the deploy command, kamal app exec does not log you in. Yet it sometimes requires registry access too. In these cases, when the host is not auth'd, it fails.

Why would kamal app exec need auth? When you tell it to use an image that's not currently on the host, as in #1163.

Note that containers don't stay authorized very long. Container registry session tokens expire pretty quickly:

Registry Session expiry
Amazon ECR 12 hours
Azure Container Registry 3 hours
Docker Hub 24 hours
GitHub Container Registry varies
Harbor 30 minutes?

Proposed state

It would be nice for all Kamal commands to have the same “it just works” auth behavior. I think this is the simplicity that attracts people to Kamal. So it would be nice if kamal exec took care of its own auth, like kamal deploy does.

Changes introduced by this PR

This PR makes kamal app exec log in before executing docker run.

Other comments

If this PR is approved I'd be happy to make a follow-up PR for kamal accessory exec.

Resolves #1163.

@djmb
Copy link
Collaborator

djmb commented Nov 21, 2024

@andrewjanssen, @cromega - like in #1223, we don't want to require secrets to be able to run kamal app exec. Logging into the registry would need them

Also I would expect the image to already be on the host - what's the situation where that's not the case?

@cromega
Copy link

cromega commented Nov 21, 2024

@djmb in our case, running a migration in a new build before we get to the deploy part. A secrets push command could potentially help work around this problem. Or an optional flag that would make the secrets sync up.

The real solution, of course, would be built-in support for migrations in a way that doesn't require working around base assumptions in the tool.

@djmb
Copy link
Collaborator

djmb commented Nov 21, 2024

@cromega - where are you running the migration? In a pre-deploy hook? In that case the secrets might be out of date, but the image should be there shouldn't it?

@cromega
Copy link

cromega commented Nov 22, 2024

@djmb our workflow is the following:

Assume v1 of app is running

  • CI pipeline builds image v2and pushes it to ECR, we don't use Kamal to build docker images
  • CI pipeline runs kamal app exec -r web --primary -d qa -V v2 rails db:migrate - the v2 image needs to be pulled and it fails if the ECR token has expired
  • CI pieline runs kamal deploy -d qa -P -V v2

@andrewjanssen
Copy link
Author

andrewjanssen commented Nov 30, 2024

@andrewjanssen, @cromega - like in #1223, we don't want to require secrets to be able to run kamal app exec. Logging into the registry would need them

That's a good point, I didn't know that when writing this PR. Could address that by changing this PR to only log in if the registry credentials are present. Not very important to me either way.

@djmb
Copy link
Collaborator

djmb commented Dec 2, 2024

@cromega - I think you should be able to ensure the pipeline works by adding kamal registry login just before your db:migrate command.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kamal app exec doesn't log in to docker
3 participants