New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: add checks to see if manifest was renamed #6788
Conversation
10b2f2a
to
bacf386
Compare
@@ -39,13 +39,48 @@ const GUESSED_MANIFEST_SIZE = 50 * KB; | |||
// | |||
// We will check this within ImageInfo | |||
export function guessIsManifest(image: ImageInfo, connectionType: string): boolean { | |||
// There is an odd case that if the image has been renamed, we may not be able to safely detect |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering why we are not relying on the Names attributes:
C:\Users\Jeff\apps>podman image ls --format json
[
{
"Id": "00339ea1c0d1525fd93fc3aa9c926c75ad2ec32a15627c32bd3924a7dbe667cd",
"ParentId": "",
"RepoTags": null,
"RepoDigests": [
"localhost/mymanifest123@sha256:20b959ad5960230b65a77b746bdbf5d991ade4d7a129c2554e167acdcc990531",
"localhost/mymanifest@sha256:20b959ad5960230b65a77b746bdbf5d991ade4d7a129c2554e167acdcc990531"
],
"Size": 110,
"SharedSize": 0,
"VirtualSize": 110,
"Labels": null,
"Containers": 0,
"Names": [
"localhost/mymanifest123:latest",
"localhost/mymanifest:latest"
],
"Digest": "sha256:20b959ad5960230b65a77b746bdbf5d991ade4d7a129c2554e167acdcc990531",
"History": [
"localhost/mymanifest:latest",
"localhost/mymanifest123:latest"
],
"Created": 1713253600,
"CreatedAt": "2024-04-16T07:46:40Z"
},
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi! We do not use names as they can still be changed and do not contain the digest value. We use digests to more accurate detect if it's been renamed or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and worked fine
11692c7
to
39958a6
Compare
### What does this PR do? * Adds a check to see if a manifest was renamed by analyzing the digests * Adds a test for this as well. ### 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 containers#6787 ### 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 1. Create a manifest (it should not appear in PD) 2. Rename the manifest `podman image tag testmanifest testmanifest123` 3. It should **not** appear in PD. Signed-off-by: Charlie Drage <charlie@charliedrage.com>
39958a6
to
f1f0ab1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
chore: add checks to see if manifest was renamed
What does this PR do?
Screenshot / video of UI
N/A
What issues does this PR fix or reference?
Closes #6787
How to test this PR?
podman image tag testmanifest testmanifest123
Signed-off-by: Charlie Drage charlie@charliedrage.com