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

Add ECR credential provider #5

Merged
merged 9 commits into from
Jul 26, 2023
Merged

Add ECR credential provider #5

merged 9 commits into from
Jul 26, 2023

Conversation

yuzhouliu9
Copy link
Collaborator

@yuzhouliu9 yuzhouliu9 commented Jul 25, 2023

Adds the amazon-ecr-credential-helper to the container image.
ECR access-key and secret-key will be set as environment variables, and the credential helper will do the work when executing any buildah command. buildah login is not needed.

Testing

I spun up minikube to have a working registry.example.com

make local-push

docker run --rm --security-opt seccomp=/Users/yuzhou.liu/Downloads/work/buildah/seccomp.json --rm -e PLUGIN_MANIFEST="{\"sources\":[\"a\"],\"targets\":[\"1\"]}" -e PLUGIN_ACCESS_KEY="public_ecr_key" -e PLUGIN_SECRET_KEY="public_ecr_secret" -e PLUGIN_REGISTRY="public.ecr.aws" registry.example.com/buildah-plugin:latest

The seccomp.json is from here and is needed for docker because of this

Alternatively, testing with podman, you don't need seccomp.json

podman run --rm -e PLUGIN_MANIFEST="{\"sources\":[\"a\"],\"targets\":[\"1\"]}" -e PLUGIN_ACCESS_KEY="public_ecr_key" -e PLUGIN_SECRET_KEY="public_ecr_secret" -e PLUGIN_REGISTRY="public.ecr.aws" registry.example.com/buildah-plugin:latest

I'll post more details on how to test in slack

Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
@yuzhouliu9 yuzhouliu9 marked this pull request as ready for review July 26, 2023 13:27
@dskatz
Copy link
Member

dskatz commented Jul 26, 2023

An easier way to test this I think would be to adjust the drone pipeline to push a git-hash image to our registry. Then we can use the buildah-plugin within drone as normal.

@yuzhouliu9
Copy link
Collaborator Author

An easier way to test this I think would be to adjust the drone pipeline to push a git-hash image to our registry. Then we can use the buildah-plugin within drone as normal.

It was already doing this but only for pushes to main branch. I've changed it now to trigger publish git-hash image for all branches.

@yuzhouliu9 yuzhouliu9 requested a review from whwalter July 26, 2023 15:30
internal/cli/cli.go Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated
Comment on lines 24 to 26
ENV HOME=/buildah
RUN mkdir -m 777 -p $HOME/.docker
RUN chown -R app $HOME
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to manage HOME and these permissions would be bad anyway.

/home/build already exists for the build user and /home/app exists for the app user so just consume the default home of whichever user we settle on.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh thanks, I didn't bother looking at the upstream quay.io/buildah/stable Containerfile till now. Good catch.

Updated to use USER build because it seems the reference image already sets up all the necessary permissions subuid, subgid for that user so we don't need to repeat for another user.

Dockerfile Outdated Show resolved Hide resolved
pkg/buildah/manifest/manifest.go Outdated Show resolved Hide resolved
@yuzhouliu9 yuzhouliu9 requested a review from whwalter July 26, 2023 19:24
@yuzhouliu9 yuzhouliu9 merged commit 7aa8cf9 into main Jul 26, 2023
@yuzhouliu9 yuzhouliu9 deleted the ecr_auth branch July 26, 2023 19:52
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.

3 participants