chore: add warning for users about short image names (podman-desktop#…

…9116) * chore: add warning for users about short image names Signed-off-by: Sonia Sandler <[email protected]> * chore: add shortname resolution API endpoint Signed-off-by: Sonia Sandler <[email protected]> * chore: show warning only for Podman FQN without docker.io Signed-off-by: Sonia Sandler <[email protected]> * chore: add tests and show warning only when Podman FQN is different Signed-off-by: Sonia Sandler <[email protected]>
SoniaSandler · Oct 10, 2024 · 63a62ea · 63a62ea
1 parent 6de369b
commit 63a62ea
Show file tree

Hide file tree

Showing 7 changed files with 257 additions and 15 deletions.
diff --git a/packages/main/src/plugin/container-registry.spec.ts b/packages/main/src/plugin/container-registry.spec.ts
@@ -5308,3 +5308,73 @@ describe('extractContainerEnvironment', () => {
     expect(env['SERVER_ARGS']).toBe('--host-config=host-secondary.xml --foo-=bar');
   });
 });
+
+test('resolve Podman image shortname to FQN', async () => {
+  const getMatchingContainerProviderMock = vi.spyOn(containerRegistry, 'getMatchingContainerProvider');
+  nock('http://localhost')
+    .get('/v5.0.0/libpod/images/shortname/resolve')
+    .reply(200, { Names: ['someregistry/shortname', 'docker.io/shortname', 'quay.io/shortname'] });
+
+  const dockerAPI = new Dockerode({ protocol: 'http', host: 'localhost' });
+
+  const libpod = new LibpodDockerode();
+  libpod.enhancePrototypeWithLibPod();
+
+  const internalContainerProviderMock = {
+    name: 'podman',
+    id: 'podman1',
+    api: dockerAPI,
+    libpodApi: dockerAPI,
+    connection: {
+      type: 'podman',
+    },
+  } as unknown as InternalContainerProvider;
+
+  containerRegistry.addInternalProvider('podman1', internalContainerProviderMock);
+
+  getMatchingContainerProviderMock.mockReturnValue(internalContainerProviderMock);
+
+  let imagesNames = await containerRegistry.resolveShortnameImage(
+    {} as unknown as ProviderContainerConnectionInfo,
+    'shortname',
+  );
+  expect(imagesNames.length).toBe(3);
+  expect(imagesNames[0]).toBe('someregistry/shortname');
+  expect(imagesNames[1]).toBe('docker.io/shortname');
+  expect(imagesNames[2]).toBe('quay.io/shortname');
+
+  nock('http://localhost').get('/v5.0.0/libpod/images/shortname/resolve').reply(200, { Names: [] });
+  imagesNames = await containerRegistry.resolveShortnameImage(
+    {} as unknown as ProviderContainerConnectionInfo,
+    'shortname',
+  );
+  expect(imagesNames.length).toBe(1);
+  expect(imagesNames[0]).toBe('shortname');
+});
+
+test('resolve Dokcer image shortname to FQN', async () => {
+  const getMatchingContainerProviderMock = vi.spyOn(containerRegistry, 'getMatchingContainerProvider');
+  const dockerAPI = new Dockerode({ protocol: 'http', host: 'localhost' });
+
+  const libpod = new LibpodDockerode();
+  libpod.enhancePrototypeWithLibPod();
+
+  const internalContainerProviderMock = {
+    name: 'docker1',
+    id: 'docker1',
+    api: dockerAPI,
+    connection: {
+      type: 'docker',
+    },
+  } as unknown as InternalContainerProvider;
+
+  containerRegistry.addInternalProvider('docker1', internalContainerProviderMock);
+
+  getMatchingContainerProviderMock.mockReturnValue(internalContainerProviderMock);
+  const imagesNames = await containerRegistry.resolveShortnameImage(
+    {} as unknown as ProviderContainerConnectionInfo,
+    'shortname',
+  );
+  expect(imagesNames.length).toBe(1);
+  expect(imagesNames[0]).toBe('shortname');
+});
diff --git a/packages/main/src/plugin/container-registry.ts b/packages/main/src/plugin/container-registry.ts
@@ -2715,4 +2715,17 @@ export class ContainerProviderRegistry {
       return Promise.reject(errors);
     }
   }
+
+  async resolveShortnameImage(
+    providerContainerConnectionInfo: ProviderContainerConnectionInfo,
+    shortName: string,
+  ): Promise<string[]> {
+    const provider = this.getMatchingContainerProvider(providerContainerConnectionInfo);
+    if (provider.libpodApi) {
+      const response = await provider.libpodApi.resolveShortnameImage(shortName);
+      return response.Names[0] ? response.Names : [shortName];
+    } else {
+      return [shortName];
+    }
+  }
 }
diff --git a/packages/main/src/plugin/dockerode/libpod-dockerode.ts b/packages/main/src/plugin/dockerode/libpod-dockerode.ts
@@ -364,6 +364,7 @@ export interface LibPod {
   startPod(podId: string): Promise<void>;
   stopPod(podId: string): Promise<void>;
   removePod(podId: string, options?: PodRemoveOptions): Promise<void>;
+  resolveShortnameImage(shortname: string): Promise<{ Names: string[] }>;
   restartPod(podId: string): Promise<void>;
   generateKube(names: string[]): Promise<string>;
   playKube(yamlContentFilePath: string): Promise<PlayKubeInfo>;
@@ -975,5 +976,26 @@ export class LibpodDockerode {
         });
       });
     };
+
+    prototypeOfDockerode.resolveShortnameImage = function (shortname: string): Promise<unknown> {
+      const optsf = {
+        path: `/v5.0.0/libpod/images/${shortname}/resolve`,
+        method: 'GET',
+        statusCodes: {
+          // in the documentation it says code 204, but only code 200 works as intended
+          200: true,
+          400: 'bad parameter',
+          500: 'server error',
+        },
+      };
+      return new Promise((resolve, reject) => {
+        this.modem.dial(optsf, (err: unknown, data: unknown) => {
+          if (err) {
+            return reject(err);
+          }
+          resolve(data);
+        });
+      });
+    };
   }
 }
diff --git a/packages/main/src/plugin/index.ts b/packages/main/src/plugin/index.ts
@@ -1055,6 +1055,16 @@ export class PluginSystem {
       },
     );
 
+    this.ipcHandle(
+      'container-provider-registry:resolveShortnameImage',
+      async (
+        _listener,
+        providerContainerConnectionInfo: ProviderContainerConnectionInfo,
+        shortName: string,
+      ): Promise<string[]> => {
+        return containerProviderRegistry.resolveShortnameImage(providerContainerConnectionInfo, shortName);
+      },
+    );
     this.ipcHandle(
       'container-provider-registry:pullImage',
       async (

diff --git a/packages/preload/src/index.ts b/packages/preload/src/index.ts
@@ -387,6 +387,13 @@ export function initExposure(): void {
     },
   );
 
+  contextBridge.exposeInMainWorld(
+    'resolveShortnameImage',
+    async (providerContainerConnectionInfo: ProviderContainerConnectionInfo, shortName: string): Promise<string[]> => {
+      return ipcInvoke('container-provider-registry:resolveShortnameImage', providerContainerConnectionInfo, shortName);
+    },
+  );
+
   let onDataCallbacksPullImageId = 0;
   const onDataCallbacksPullImage = new Map<number, (event: PullEvent) => void>();
   contextBridge.exposeInMainWorld(

diff --git a/packages/renderer/src/lib/image/PullImage.spec.ts b/packages/renderer/src/lib/image/PullImage.spec.ts
@@ -23,6 +23,7 @@ import '@testing-library/jest-dom/vitest';
 import type { ProviderStatus } from '@podman-desktop/api';
 import { render, screen } from '@testing-library/svelte';
 import userEvent from '@testing-library/user-event';
+import { tick } from 'svelte';
 import { beforeAll, beforeEach, describe, expect, test, vi } from 'vitest';
 
 import { providerInfos } from '/@/stores/providers';
@@ -32,6 +33,7 @@ import type { ProviderContainerConnectionInfo, ProviderInfo } from '/@api/provid
 import PullImage from './PullImage.svelte';
 
 const pullImageMock = vi.fn();
+const resolveShortnameImageMock = vi.fn();
 
 // fake the window.events object
 beforeAll(() => {
@@ -47,6 +49,7 @@ beforeAll(() => {
     addListener: vi.fn(),
   });
   (window as any).pullImage = pullImageMock;
+  (window as any).resolveShortnameImage = resolveShortnameImageMock.mockResolvedValue(['docker.io/test1']);
 
   Object.defineProperty(window, 'matchMedia', {
     value: () => {
@@ -261,3 +264,74 @@ describe('PullImage', () => {
     expect(proposal).toBeEnabled();
   });
 });
+
+test('Expect if no docker.io shortname to use Podman FQN', async () => {
+  resolveShortnameImageMock.mockResolvedValue(['someregistry/test1']);
+  setup();
+  render(PullImage);
+
+  const textbox = screen.getByRole('textbox', { name: 'Image to Pull' });
+  await userEvent.click(textbox);
+  await userEvent.paste('test1');
+
+  expect(resolveShortnameImageMock).toBeCalled();
+  await tick();
+  const FQNButton = screen.getByRole('checkbox', { name: 'Use Podman FQN' });
+
+  await userEvent.click(FQNButton);
+  const pullImagebutton = screen.getByRole('button', { name: 'Pull image' });
+  await userEvent.click(pullImagebutton);
+
+  const imageName = pullImageMock.mock.calls[0][1];
+  expect(imageName).toBe('someregistry/test1');
+});
+
+test('Expect if no docker.io shortname but checkbox not checked to use docker hub', async () => {
+  resolveShortnameImageMock.mockResolvedValue(['someregistry/test1']);
+  setup();
+  render(PullImage);
+
+  const textbox = screen.getByRole('textbox', { name: 'Image to Pull' });
+  await userEvent.click(textbox);
+  await userEvent.paste('test1');
+
+  expect(resolveShortnameImageMock).toBeCalled();
+  await tick();
+
+  const pullImagebutton = screen.getByRole('button', { name: 'Pull image' });
+  await userEvent.click(pullImagebutton);
+
+  const imageName = pullImageMock.mock.calls[0][1];
+  expect(imageName).toBe('docker.io/test1');
+});
+
+test('Expect if docker.io shortname exists to not use Podman FQN', async () => {
+  resolveShortnameImageMock.mockResolvedValue(['someregistry/test1', 'docker.io/test1']);
+  setup();
+  render(PullImage);
+
+  const textbox = screen.getByRole('textbox', { name: 'Image to Pull' });
+  await userEvent.click(textbox);
+  await userEvent.paste('test1');
+
+  expect(resolveShortnameImageMock).toBeCalled();
+  await tick();
+  expect(screen.queryByRole('checkbox', { name: 'Use Podman FQN' })).not.toBeInTheDocument();
+
+  const pullImagebutton = screen.getByRole('button', { name: 'Pull image' });
+  await userEvent.click(pullImagebutton);
+
+  const imageName = pullImageMock.mock.calls[0][1];
+  expect(imageName).toBe('test1');
+});
+
+test('Expect not to check not shortname images', async () => {
+  setup();
+  render(PullImage);
+
+  const textbox = screen.getByRole('textbox', { name: 'Image to Pull' });
+  await userEvent.click(textbox);
+  await userEvent.paste('test1/');
+
+  expect(resolveShortnameImageMock).not.toBeCalled();
+});
diff --git a/packages/renderer/src/lib/image/PullImage.svelte b/packages/renderer/src/lib/image/PullImage.svelte
@@ -1,8 +1,9 @@
 <script lang="ts">
-import { faArrowCircleDown, faCog } from '@fortawesome/free-solid-svg-icons';
-import { Button, ErrorMessage } from '@podman-desktop/ui-svelte';
+import { faArrowCircleDown, faCog, faTriangleExclamation } from '@fortawesome/free-solid-svg-icons';
+import { Button, Checkbox, ErrorMessage, Tooltip } from '@podman-desktop/ui-svelte';
 import type { Terminal } from '@xterm/xterm';
 import { onMount, tick } from 'svelte';
+import Fa from 'svelte-fa';
 import { router } from 'tinro';
 
 import type { ImageSearchOptions } from '/@api/image-registry';
@@ -21,6 +22,9 @@ let logsPull: Terminal;
 let pullError = '';
 let pullInProgress = false;
 let pullFinished = false;
+let shortnameImages: string[] = [];
+let podmanFQN = '';
+let usePodmanFQN = false;
 
 export let imageToPull: string | undefined = undefined;
 
@@ -34,6 +38,28 @@ let selectedProviderConnection: ProviderContainerConnectionInfo | undefined;
 const lineNumberPerId = new Map<string, number>();
 let lineIndex = 0;
 
+async function resolveShortname(): Promise<void> {
+  if (!selectedProviderConnection || selectedProviderConnection.type !== 'podman') {
+    return;
+  }
+  if (imageToPull && !imageToPull.includes('/')) {
+    shortnameImages = (await window.resolveShortnameImage(selectedProviderConnection, imageToPull)) ?? [];
+    // not a shortname
+  } else {
+    podmanFQN = '';
+    shortnameImages = [];
+    usePodmanFQN = false;
+  }
+  // checks if there is no FQN that is from dokcer hub
+  if (!shortnameImages.find(name => name.includes('docker.io'))) {
+    podmanFQN = shortnameImages[0];
+  } else {
+    podmanFQN = '';
+    shortnameImages = [];
+    usePodmanFQN = false;
+  }
+}
+
 function callback(event: PullEvent) {
   let lineIndexToWrite;
   if (event.status && event.id) {
@@ -96,7 +122,13 @@ async function pullImage() {
 
   pullInProgress = true;
   try {
-    await window.pullImage(selectedProviderConnection, imageToPull.trim(), callback);
+    if (podmanFQN) {
+      usePodmanFQN
+        ? await window.pullImage(selectedProviderConnection, podmanFQN.trim(), callback)
+        : await window.pullImage(selectedProviderConnection, `docker.io/${imageToPull.trim()}`, callback);
+    } else {
+      await window.pullImage(selectedProviderConnection, imageToPull.trim(), callback);
+    }
     pullInProgress = false;
     pullFinished = true;
   } catch (error: any) {
@@ -190,18 +222,32 @@ async function searchImages(value: string): Promise<string[]> {
     <div class="w-full">
       <label for="imageName" class="block mb-2 font-bold text-[var(--pd-content-card-header-text)]"
         >Image to Pull</label>
-      <Typeahead
-        id="imageName"
-        name="imageName"
-        placeholder="Image name"
-        searchFunction={searchImages}
-        onChange={(s: string) => {
-          validateImageName(s);
-        }}
-        onEnter={pullImage}
-        disabled={pullFinished || pullInProgress}
-        required
-        initialFocus />
+      <div class="flex flex-col">
+        <Typeahead
+          id="imageName"
+          name="imageName"
+          placeholder="Image name"
+          searchFunction={searchImages}
+          onChange={(s: string) => {
+            validateImageName(s);
+            resolveShortname();
+          }}
+          onEnter={pullImage}
+          disabled={pullFinished || pullInProgress}
+          required
+          initialFocus />
+        {#if selectedProviderConnection?.type === 'podman' && podmanFQN}
+          <div class="absolute mt-2 ml-[-18px] self-start">
+            <Tooltip tip="Shortname images will be pulled from Docker Hub" topRight>
+              <Fa id="shortname-warning" size="1.1x" class="text-amber-400" icon={faTriangleExclamation} />
+            </Tooltip>
+          </div>
+        {/if}
+      </div>
+      {#if selectedProviderConnection?.type === 'podman' && podmanFQN}
+        <Checkbox class="pt-2" bind:checked={usePodmanFQN} title="Use Podman FQN" disabled={podmanFQN === ''}
+          >Use Podman FQN for shortname image</Checkbox>
+      {/if}
       {#if imageNameInvalid}
         <ErrorMessage error={imageNameInvalid} />
       {/if}