chore: fix check is manifest logic

### What does this PR do?

* Updates the logic to pass in the connection type
* Update the tests to purposely not return engine id / engine name, and
  it should be pure json instead from the http endpoint
* Fixes the history check for checking object keys

### Screenshot / video of UI

<!-- If this PR is changing UI, please include
screenshots or screencasts showing the difference -->

N/A

### What issues does this PR fix or reference?

<!-- Include any related issues from Podman Desktop
repository (or from another issue tracker). -->

Closes https://github.com/containers/podman-desktop/issues/6731

### How to test this PR?

<!-- Please explain steps to verify the functionality,
do not forget to provide unit/component tests -->

- [X] Tests are covering the bug fix or the new feature

Signed-off-by: Charlie Drage <charlie@charliedrage.com>
This commit is contained in:
Charlie Drage 2024-04-09 11:21:49 -04:00 committed by Florent BENOIT
parent db072f7219
commit 37782bbce2
4 changed files with 32 additions and 17 deletions

View file

@ -4393,11 +4393,9 @@ describe('loadImages', () => {
});
test('manifest is listed as true with podmanListImages correctly', async () => {
const manifestImage: ImageInfo = {
const manifestImage = {
Id: 'manifestImage',
Labels: {},
engineId: 'engine1',
engineName: 'podman',
ParentId: '',
RepoTags: ['manifestTag'],
RepoDigests: ['manifestDigest'],
@ -4408,11 +4406,9 @@ test('manifest is listed as true with podmanListImages correctly', async () => {
Containers: 0,
};
const regularImage: ImageInfo = {
const regularImage = {
Id: 'ee301c921b8aadc002973b2e0c3da17d701dcd994b606769a7e6eaa100b81d44',
Labels: {},
engineId: 'engine5', // Assuming 'engineId' and 'engineName' are part of your ImageInfo but not relevant here
engineName: 'podman',
ParentId: '',
RepoTags: ['testdomain.io/library/hello:latest'],
RepoDigests: [

View file

@ -630,7 +630,7 @@ export class ContainerProviderRegistry {
// Using guessIsManifest, determine if the image is a manifest and set isManifest accordingly
// NOTE: This is a workaround until we have a better way to determine if an image is a manifest
// and may result in false positives until issue: https://github.com/containers/podman/issues/22184 is resolved
isManifest: guessIsManifest(image),
isManifest: guessIsManifest(image, provider.connection.type),
}));
}),
);

View file

@ -38,7 +38,7 @@ describe('guessIsManifest function', () => {
Containers: 0,
};
expect(guessIsManifest(manifestImage)).toBe(true);
expect(guessIsManifest(manifestImage, 'podman')).toBe(true);
});
test('returns false if VirtualSize is over 1MB', () => {
@ -57,7 +57,7 @@ describe('guessIsManifest function', () => {
Containers: 0,
};
expect(guessIsManifest(largeImage)).toBe(false);
expect(guessIsManifest(largeImage, 'podman')).toBe(false);
});
test('returns false if Labels is not empty', () => {
@ -76,7 +76,7 @@ describe('guessIsManifest function', () => {
Containers: 0,
};
expect(guessIsManifest(labeledImage)).toBe(false);
expect(guessIsManifest(labeledImage, 'podman')).toBe(false);
});
test('returns false if RepoTags is undefined or empty', () => {
@ -95,7 +95,7 @@ describe('guessIsManifest function', () => {
Containers: 0,
};
expect(guessIsManifest(noTagImage)).toBe(false);
expect(guessIsManifest(noTagImage, 'podman')).toBe(false);
});
test('returns false if RepoDigests is undefined or empty', () => {
@ -114,7 +114,7 @@ describe('guessIsManifest function', () => {
Containers: 0,
};
expect(guessIsManifest(noDigestImage)).toBe(false);
expect(guessIsManifest(noDigestImage, 'podman')).toBe(false);
});
test('return false for a typical helloworld image example, that may be small enough to trigger guessIsManifest as true', () => {
@ -138,6 +138,25 @@ describe('guessIsManifest function', () => {
};
// Should be false
expect(guessIsManifest(helloWorldImage)).toBe(false);
expect(guessIsManifest(helloWorldImage, 'podman')).toBe(false);
});
});
test('expect to fail even if engine name does not equal podman', () => {
const manifestImage: ImageInfo = {
Id: 'manifestImage',
Labels: {},
engineId: 'engine1',
engineName: 'podman',
ParentId: '',
RepoTags: ['manifestTag'],
RepoDigests: ['manifestDigest'],
Created: 0,
Size: 0,
VirtualSize: 40 * 1024, // 40KB (less than 50KB threshold)
SharedSize: 0,
Containers: 0,
};
expect(guessIsManifest(manifestImage, 'foobar')).toBe(false);
});

View file

@ -15,7 +15,6 @@
*
* SPDX-License-Identifier: Apache-2.0
***********************************************************************/
import type { ImageInfo } from '../api/image-info.js';
const KB = 1024;
@ -33,20 +32,21 @@ const GUESSED_MANIFEST_SIZE = 50 * KB;
// - Labels is null or empty, as the manifest usually doesn't have any labels
// - RepoTags always exists, as the manifest has a tag associated
// - RepoDigests always exists as well, as the manifest has a digest associated
// - Connection type is "podman"
// - History is null or empty
// - Engine type is "podman"
// all of these conditions must be met to (safely) determine if the image is a manifest
//
// We will check this within ImageInfo
export function guessIsManifest(image: ImageInfo): boolean {
export function guessIsManifest(image: ImageInfo, connectionType: string): boolean {
return Boolean(
image.RepoTags &&
image.RepoDigests &&
image.RepoTags.length > 0 &&
image.RepoDigests.length > 0 &&
(!image.Labels || Object.keys(image.Labels).length === 0) &&
(!image.History || image.History.length === 0) &&
image.engineName === 'podman' &&
(!image.History || Object.keys(image.History).length === 0) &&
connectionType === 'podman' &&
image.VirtualSize < GUESSED_MANIFEST_SIZE,
);
}