Skip to content

Commit

Permalink
fix(cdk-assets): remove https from endpoint, to work with podman (#92)
Browse files Browse the repository at this point in the history
Fixes aws/aws-cdk#16209

Redo of aws/aws-cdk#28926

podman does not allow prefixing the endpoint with https when calling
docker login:
```
fail: docker login --username AWS --password-stdin https://XXX.dkr.ecr.eu-west-1.amazonaws.com exited with error code 125: Error: credentials key has https[s]:// prefix
```
This PR removes the prefix, which works with both docker and podman

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

Co-authored-by: Kaizen Conroy <[email protected]>
Co-authored-by: Momo Kornher <[email protected]>
  • Loading branch information
3 people authored Feb 28, 2025
1 parent bd02059 commit a30b38d
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 151 deletions.
2 changes: 1 addition & 1 deletion packages/cdk-assets/lib/private/docker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ export class Docker {

// Use --password-stdin otherwise docker will complain. Loudly.
await this.execute(
['login', '--username', credentials.username, '--password-stdin', credentials.endpoint],
['login', '--username', credentials.username, '--password-stdin', credentials.endpoint.replace(/^https?:\/\/|\/$/g, '')],
{
input: credentials.password,

Expand Down
171 changes: 21 additions & 150 deletions packages/cdk-assets/test/docker-images.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,9 +280,7 @@ beforeEach(() => {

// Set consistent mocks
mockEcr.on(GetAuthorizationTokenCommand).resolves({
authorizationData: [
{ authorizationToken: 'dXNlcjpwYXNz', proxyEndpoint: 'https://proxy.com/' },
],
authorizationData: [{ authorizationToken: 'dXNlcjpwYXNz', proxyEndpoint: 'https://proxy.com/' }],
});
});

Expand Down Expand Up @@ -312,14 +310,10 @@ test('logging in twice for two repository domains (containing account id & regio

const responses: GetAuthorizationTokenResponse[] = [
{
authorizationData: [
{ authorizationToken: 'dXNlcjpwYXNz', proxyEndpoint: 'https://12345.proxy.com/' },
],
authorizationData: [{ authorizationToken: 'dXNlcjpwYXNz', proxyEndpoint: 'https://12345.proxy.com/' }],
},
{
authorizationData: [
{ authorizationToken: 'dXNlcjpwYXNz', proxyEndpoint: 'https://12346.proxy.com/' },
],
authorizationData: [{ authorizationToken: 'dXNlcjpwYXNz', proxyEndpoint: 'https://12346.proxy.com/' }],
},
];

Expand All @@ -331,14 +325,7 @@ test('logging in twice for two repository domains (containing account id & regio

const expectAllSpawns = mockSpawn(
{
commandLine: [
'docker',
'login',
'--username',
'user',
'--password-stdin',
'https://12345.proxy.com/',
],
commandLine: ['docker', 'login', '--username', 'user', '--password-stdin', '12345.proxy.com'],
},
{ commandLine: ['docker', 'inspect', 'cdkasset-theasset1'], exitCode: 1 },
{
Expand All @@ -350,14 +337,7 @@ test('logging in twice for two repository domains (containing account id & regio
},
{ commandLine: ['docker', 'push', '12345.amazonaws.com/repo:theAsset1'] },
{
commandLine: [
'docker',
'login',
'--username',
'user',
'--password-stdin',
'https://12346.proxy.com/',
],
commandLine: ['docker', 'login', '--username', 'user', '--password-stdin', '12346.proxy.com'],
},
{ commandLine: ['docker', 'inspect', 'cdkasset-theasset2'], exitCode: 1 },
{
Expand Down Expand Up @@ -421,14 +401,7 @@ describe('with a complete manifest', () => {
test('upload docker image if not uploaded yet but exists locally', async () => {
const expectAllSpawns = mockSpawn(
{
commandLine: [
'docker',
'login',
'--username',
'user',
'--password-stdin',
'https://proxy.com/',
],
commandLine: ['docker', 'login', '--username', 'user', '--password-stdin', 'proxy.com'],
},
{ commandLine: ['docker', 'inspect', 'cdkasset-theasset'] },
{ commandLine: ['docker', 'tag', 'cdkasset-theasset', '12345.amazonaws.com/repo:abcdef'] },
Expand All @@ -444,14 +417,7 @@ describe('with a complete manifest', () => {
test('build and upload docker image if not exists anywhere', async () => {
const expectAllSpawns = mockSpawn(
{
commandLine: [
'docker',
'login',
'--username',
'user',
'--password-stdin',
'https://proxy.com/',
],
commandLine: ['docker', 'login', '--username', 'user', '--password-stdin', 'proxy.com'],
},
{ commandLine: ['docker', 'inspect', 'cdkasset-theasset'], exitCode: 1 },
{
Expand All @@ -476,14 +442,7 @@ describe('with a complete manifest', () => {

const expectAllSpawns = mockSpawn(
{
commandLine: [
'docker',
'login',
'--username',
'user',
'--password-stdin',
'https://proxy.com/',
],
commandLine: ['docker', 'login', '--username', 'user', '--password-stdin', 'proxy.com'],
},
{ commandLine: ['docker', 'inspect', 'cdkasset-theasset'], exitCode: 1 },
{
Expand All @@ -508,26 +467,11 @@ describe('with a complete manifest', () => {

const expectAllSpawns = mockSpawn(
{
commandLine: [
'docker',
'login',
'--username',
'user',
'--password-stdin',
'https://proxy.com/',
],
commandLine: ['docker', 'login', '--username', 'user', '--password-stdin', 'proxy.com'],
},
{ commandLine: ['docker', 'inspect', 'cdkasset-theasset'], exitCode: 1 },
{
commandLine: [
'docker',
'build',
'--tag',
'cdkasset-theasset',
'--platform',
'linux/arm64',
'.',
],
commandLine: ['docker', 'build', '--tag', 'cdkasset-theasset', '--platform', 'linux/arm64', '.'],
cwd: defaultNetworkDockerpath,
},
{ commandLine: ['docker', 'tag', 'cdkasset-theasset', '12345.amazonaws.com/repo:nopqr'] },
Expand All @@ -546,14 +490,7 @@ describe('with a complete manifest', () => {

const expectAllSpawns = mockSpawn(
{
commandLine: [
'docker',
'login',
'--username',
'user',
'--password-stdin',
'https://proxy.com/',
],
commandLine: ['docker', 'login', '--username', 'user', '--password-stdin', 'proxy.com'],
},
{ commandLine: ['docker', 'inspect', 'cdkasset-theasset'], exitCode: 1 },
{
Expand Down Expand Up @@ -586,14 +523,7 @@ describe('with a complete manifest', () => {

const expectAllSpawns = mockSpawn(
{
commandLine: [
'docker',
'login',
'--username',
'user',
'--password-stdin',
'https://proxy.com/',
],
commandLine: ['docker', 'login', '--username', 'user', '--password-stdin', 'proxy.com'],
},
{ commandLine: ['docker', 'inspect', 'cdkasset-theasset'], exitCode: 1 },
{
Expand All @@ -618,14 +548,7 @@ describe('with a complete manifest', () => {

const expectAllSpawns = mockSpawn(
{
commandLine: [
'docker',
'login',
'--username',
'user',
'--password-stdin',
'https://proxy.com/',
],
commandLine: ['docker', 'login', '--username', 'user', '--password-stdin', 'proxy.com'],
},
{ commandLine: ['docker', 'inspect', 'cdkasset-theasset'], exitCode: 1 },
{
Expand Down Expand Up @@ -662,14 +585,7 @@ describe('with a complete manifest', () => {

const expectAllSpawns = mockSpawn(
{
commandLine: [
'docker',
'login',
'--username',
'user',
'--password-stdin',
'https://proxy.com/',
],
commandLine: ['docker', 'login', '--username', 'user', '--password-stdin', 'proxy.com'],
},
{ commandLine: ['docker', 'inspect', 'cdkasset-theasset'], exitCode: 1 },
{
Expand Down Expand Up @@ -705,14 +621,7 @@ describe('external assets', () => {
test('upload externally generated Docker image', async () => {
const expectAllSpawns = mockSpawn(
{
commandLine: [
'docker',
'login',
'--username',
'user',
'--password-stdin',
'https://proxy.com/',
],
commandLine: ['docker', 'login', '--username', 'user', '--password-stdin', 'proxy.com'],
},
{ commandLine: ['sometool'], stdout: externalTag, cwd: '/external/cdk.out' },
{ commandLine: ['docker', 'tag', externalTag, '12345.amazonaws.com/repo:ghijkl'] },
Expand Down Expand Up @@ -754,9 +663,7 @@ test('correctly identify Docker directory if path is absolute', async () => {

test('when external credentials are present, explicit Docker config directories are used', async () => {
// Setup -- Mock that we have CDK credentials, and mock fs operations.
jest
.spyOn(dockercreds, 'cdkCredentialsConfig')
.mockReturnValue({ version: '0.1', domainCredentials: {} });
jest.spyOn(dockercreds, 'cdkCredentialsConfig').mockReturnValue({ version: '0.1', domainCredentials: {} });
jest.spyOn(fs, 'mkdtempSync').mockImplementationOnce(() => '/tmp/mockedTempDir');
jest.spyOn(fs, 'writeFileSync').mockImplementation(jest.fn());

Expand All @@ -769,15 +676,7 @@ test('when external credentials are present, explicit Docker config directories
exitCode: 1,
},
{
commandLine: [
'docker',
'--config',
'/tmp/mockedTempDir',
'build',
'--tag',
'cdkasset-theasset',
'.',
],
commandLine: ['docker', '--config', '/tmp/mockedTempDir', 'build', '--tag', 'cdkasset-theasset', '.'],
cwd: absoluteDockerPath,
},
{
Expand Down Expand Up @@ -808,14 +707,7 @@ test('logging in only once for two assets', async () => {

const expectAllSpawns = mockSpawn(
{
commandLine: [
'docker',
'login',
'--username',
'user',
'--password-stdin',
'https://proxy.com/',
],
commandLine: ['docker', 'login', '--username', 'user', '--password-stdin', 'proxy.com'],
},
{ commandLine: ['docker', 'inspect', 'cdkasset-theasset1'], exitCode: 1 },
{
Expand Down Expand Up @@ -849,14 +741,7 @@ test('building only', async () => {

const expectAllSpawns = mockSpawn(
{
commandLine: [
'docker',
'login',
'--username',
'user',
'--password-stdin',
'https://proxy.com/',
],
commandLine: ['docker', 'login', '--username', 'user', '--password-stdin', 'proxy.com'],
},
{ commandLine: ['docker', 'inspect', 'cdkasset-theasset1'], exitCode: 1 },
{
Expand Down Expand Up @@ -888,14 +773,7 @@ test('publishing only', async () => {

const expectAllSpawns = mockSpawn(
{
commandLine: [
'docker',
'login',
'--username',
'user',
'--password-stdin',
'https://proxy.com/',
],
commandLine: ['docker', 'login', '--username', 'user', '--password-stdin', 'proxy.com'],
},
{ commandLine: ['docker', 'push', '12345.amazonaws.com/aws-cdk/assets:theAsset1'] },
{ commandLine: ['docker', 'push', '12345.amazonaws.com/aws-cdk/assets:theAsset2'] },
Expand All @@ -917,14 +795,7 @@ test('overriding the docker command', async () => {

const expectAllSpawns = mockSpawn(
{
commandLine: [
'custom',
'login',
'--username',
'user',
'--password-stdin',
'https://proxy.com/',
],
commandLine: ['custom', 'login', '--username', 'user', '--password-stdin', 'proxy.com'],
},
{ commandLine: ['custom', 'inspect', 'cdkasset-theasset'] },
{ commandLine: ['custom', 'tag', 'cdkasset-theasset', '12345.amazonaws.com/repo:abcdef'] },
Expand Down

0 comments on commit a30b38d

Please sign in to comment.