Simply returning no error when a caller would expect functionality to be triggered does not seem appropriate. Can you elaborate on the current use cases for these methods, to make review smoother?

Simply returning no error when a caller would expect functionality to be triggered does not seem appropriate. Can you elaborate on the current use cases for these methods, to make review smoother?

Agreed. The ECR credential helper is intended to be used instead of login and logout. I think silently returning no error is not enough here, I'd suggest keeping the error return and adding a warning log statement that this tool is intended to be set up as a credential helper in ~/.docker/config.json (or ~/.finch/config.json if using finch)

See https://github.com/awslabs/amazon-ecr-credential-helper#troubleshooting and https://github.com/awslabs/amazon-ecr-credential-helper#usage

Sorry I thought it's what prevented my ecr-login from working. It turns out to be a different issue. I noticed the error message during troubleshooting, then came to the conclusion too quickly.

As a side note, for anyone who by chance lands here, what prevented ecr-login from working for docker 24.0.4 with containerd-snapshotter enabled was because of the following:

time="2023-07-23T21:28:08.336275463Z" level=warning msg="Host doesn't match" cfgHost=registry-1.docker.io host=xxx.amazonaws.com
time="2023-07-23T21:28:08.342797167Z" level=error msg="Handler for POST /v1.43/images/create returned error: failed to resolve reference \"xxx.amazonaws.com/<image>:latest\": pulling from host xxx.amazonaws.com failed with status code [manifests latest]: 401 Unauthorized"

and what fixed it for me was with this in ~/.docker/config.json:

{
	"auths": {
		"xxx.amazonaws.com": {}  <=== has to add this to avoid the "Host doesn't match" warning
	},
	"credHelpers": {
		"xxx.amazonaws.com": "ecr-login"
	}
}