fix(pipelines): DockerCredential.dockerHub() silently fails auth

[kaizencc]

Problem:

DockerCredential.dockerHub() silently failed to authenticate users, resulting in
unexpected and intermittent throttling due to docker's policy for unauthenticated users.

Reason:

.dockerHub() added index.docker.io to the domain credentials, but the actual docker
command authenticated with https://index.docker.io/v1/ which it was unable to find
as a domain credential, thus failing to trigger docker-credential-cdk-assets
during the docker --config build call.

Furthermore, the credential DockerCredential.customRegistry('https://index.docker.io/v1/', secret)
alone does not work. This would successfully trigger docker-credential-cdk-assets
but fail to authenticate because of how cdk-assets handles credential lookup.
The command strips the endpoint into just a hostname so in this case we try
fetchDockerLoginCredentials(awsClient, config, 'index.docker.io') which fails:

aws-cdk/packages/cdk-assets/bin/docker-credential-cdk-assets.ts

Lines 32 to 38 in 4fb0309

	let rawDomain = fs.readFileSync(0, { encoding: 'utf-8' }).trim();
	// Paranoid handling to ensure new URL() doesn't throw if the schema is missing.
	// Not convinced docker will ever pass in a url like 'index.docker.io/v1', but just in case...
	rawDomain = rawDomain.includes('://') ? rawDomain : `https://${rawDomain}`;
	const domain = new URL(rawDomain).hostname;
	const credentials = await fetchDockerLoginCredentials(new DefaultAwsClient(), config, domain);

So the workaround for this bug was to specify both domains as credentials, each to
satisfy a separate step of the process:

dockerCredentials: [
                pipelines.DockerCredential.dockerHub(secret),
                pipelines.DockerCredential.customRegistry('https://index.docker.io/v1/', secret),
            ],

Solution:

This PR introduces two separate changes to address both problems. First, we change
the hardcoded domain in DockerCredential.dockerHub() to be https://index.docker.io/v1/.
This allows us to successfully trigger docker-credential-cdk-assets when the
docker --config build command is called.

Next, to make sure the credential lookup succeeds, we check for both the complete
endpoint and the domain name. In this case, we will check for both https://index.docker.io/v1/
as well as index.docker.io. Since https://index.docker.io/v1/ exists in the credentials helper,
authentication will succeed.

Why do we still check for the domain index.docker.io? I don't know how custom registries or
ecr works in this context and believe it to be beyond the scope of the PR. It's possible that they
require the domain only for lookup.

Testing:

The change to credential lookups is unit tested in docker-credentials.test.ts. I confirmed that
the change to DockerCredential.dockerHub() is successful by configuring a mock
cdk-docker-creds.json file and successfully cdk deploying a docker image that depends on
a private repository. This isn't a common use case but ensures that failure to authenticate
results in failure every time. Thanks @james-mathiesen for the suggestion.

Contributors:

Thanks to @nohack for the code in cdk-assets.

Fixes #15737.

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[gitpod-io]

[rix0rrr]

I like the new code, reads a lot more readably to me! Thanks

[mergify]

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
• Commit ID: f777f83
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[mergify]

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).