search

mr-smithers-excellent / docker-build-push

Docker Build & Push GitHub Action
MIT License
280 stars 62 forks source link

Action fails pipeline if credentials are not supplied #224

Open plgodin opened 6 months ago

plgodin commented 6 months ago

I'm using this action in a workflow that sets up the registry's credentials through a configuration file.

Because of that, I don't need to supply credentials to this action: Docker can authenticate with my registry just fine.

I can confirm that my image is built and pushed successfully, but because I didn't supply credentials to the action, the action raises an error and marks the step (and workflow) as failed, even though everything went fine.

mr-smithers-excellent commented 6 months ago

@plgodin - thanks for the feature request & PR. I did not think about this use case honestly. Let me review and ensure there are no unintended consequences.

mr-smithers-excellent commented 6 months ago

Can you give me a bit more info as to how you supply a credentials file to your workflow? I'm wondering if we can check for the existence of credentials or a credentials file to ensure a good debugging experience for users.

plgodin commented 5 months ago

I'm using the docker-credential-gcr credential helper to authenticate to GCR. As far as I can tell, it's adding entries to ~/.docker/config.json so that Docker automatically uses the GCR tool as a credential helper when it needs to authenticate with GCR.

mr-smithers-excellent commented 5 months ago

I'd rather not simply remove the credential check. Perhaps we can add some checks for GCR that would instead skip the docker.login() step.

plgodin commented 5 months ago

It doesn't have to be GCR-specific; checking for the presence of credential helpers in the Docker config file would be a better test.

mr-smithers-excellent commented 5 months ago

Agreed! Could we add that logic into the PR?

sirtuxalot commented 4 months ago

I see this is open an issue as I was going to add a similar (likely related) issue. While building a local test gitea server with a local registry with no authentication setup yet, specifically to test building of containers and sending them to a registry. While testing I had a similar experience as I was able to build and push a containers to my registry twice. The first with no secrets created or username/password options set and the second with username/password options set and secrets created. The first pass the action "failed" yet the freshly built container was in the registry and the second pass the action "succeeded" and a new container was also added to the registry.

If you are going to make login to registry mandatory, then I suspect you would want to check for the credentials and fail early if they are not supplied that way there isn't a false failure. Now I can certainly understand requiring credentials for registries that are public, however not for private registries...but that's just me. Sadly, I have proven with my second attempt that even requiring credentials in the action does not prove a registry requires login if I can build and push a container that doesn't have authentication setup. Now, if you can add verification that login succeeded before uploading the container, that would further enhance the requirement of credentials. Otherwise you may be better off adding a flag for when no authentication is used on the registry.

lancecmalibu commented 4 months ago

I would also like to add that I have similar issues with AWS.

I use OIDC to auth and thus don't have/need credentials to pass.

mr-smithers-excellent commented 4 months ago

@lancecmalibu - makes sense. We'll get this figured out. Thanks for the feedback here. I'll get some e2e tests for this scenario as well.