Skip to content

Commit

Permalink
fix: fix how to handle env with multiple = and spaces (#8342)
Browse files Browse the repository at this point in the history
  • Loading branch information
benoitf authored Aug 2, 2024
1 parent faef17d commit 72c1f79
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 5 deletions.
41 changes: 41 additions & 0 deletions packages/main/src/plugin/container-registry.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import { ImageRegistry } from '/@/plugin/image-registry.js';
import type { Proxy } from '/@/plugin/proxy.js';
import type { Telemetry } from '/@/plugin/telemetry/telemetry.js';
import type { ContainerCreateOptions } from '/@api/container-info.js';
import type { ContainerInspectInfo } from '/@api/container-inspect-info.js';
import type { ImageInfo } from '/@api/image-info.js';
import type { ProviderContainerConnectionInfo } from '/@api/provider-info.js';

Expand Down Expand Up @@ -312,6 +313,10 @@ const fakeContainerInspectInfoWithVolume = {
};

class TestContainerProviderRegistry extends ContainerProviderRegistry {
public extractContainerEnvironment(container: ContainerInspectInfo): { [key: string]: string } {
return super.extractContainerEnvironment(container);
}

public getMatchingEngine(engineId: string): Dockerode {
return super.getMatchingEngine(engineId);
}
Expand Down Expand Up @@ -5243,3 +5248,39 @@ describe('provider update', () => {
});
});
});

describe('extractContainerEnvironment', () => {
test('simple env', async () => {
// create a fake inspect info object with env
const inspectInfo = {
Config: {
Env: ['TERM=xterm', 'HOME=/root'],
},
} as unknown as ContainerInspectInfo;

const env = containerRegistry.extractContainerEnvironment(inspectInfo);

expect(env).toBeDefined();
expect(Object.keys(env)).toHaveLength(2);

expect(env['TERM']).toBe('xterm');
expect(env['HOME']).toBe('/root');
});

test('simple complex env', async () => {
// create a fake inspect info object with env
const inspectInfo = {
Config: {
Env: ['HOME=/root', 'SERVER_ARGS=--host-config=host-secondary.xml --foo-=bar'],
},
} as unknown as ContainerInspectInfo;

const env = containerRegistry.extractContainerEnvironment(inspectInfo);

expect(env).toBeDefined();
expect(Object.keys(env)).toHaveLength(2);

expect(env['HOME']).toBe('/root');
expect(env['SERVER_ARGS']).toBe('--host-config=host-secondary.xml --foo-=bar');
});
});
15 changes: 10 additions & 5 deletions packages/main/src/plugin/container-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1403,6 +1403,15 @@ export class ContainerProviderRegistry {
}
}

protected extractContainerEnvironment(container: ContainerInspectInfo): { [key: string]: string } {
return container.Config.Env.reduce((acc: { [key: string]: string }, env) => {
// should handle multiple values after the = sign
const [key, ...values] = env.split('=');
acc[key] = values.join('=');
return acc;
}, {});
}

async replicatePodmanContainer(
source: { engineId: string; id: string },
target: { engineId: string },
Expand All @@ -1417,11 +1426,7 @@ export class ContainerProviderRegistry {
const containerToReplicate = await this.getContainerInspect(source.engineId, source.id);

// convert env from array of string to an object with key being the env name
const updatedEnv = containerToReplicate.Config.Env.reduce((acc: { [key: string]: string }, env) => {
const [key, value] = env.split('=');
acc[key] = value;
return acc;
}, {});
const updatedEnv = this.extractContainerEnvironment(containerToReplicate);

// build create container configuration
const originalConfiguration = this.getCreateContainsOptionsFromOriginal(containerToReplicate, updatedEnv);
Expand Down

0 comments on commit 72c1f79

Please sign in to comment.