Bug 636525 - 'pulp repo upload' doesn't verify uploaded file to existing content
Summary: 'pulp repo upload' doesn't verify uploaded file to existing content
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Pulp
Classification: Retired
Component: z_other
Version: unspecified
Hardware: All
OS: Linux
high
high
Target Milestone: ---
: ---
Assignee: Pradeep Kilambi
QA Contact: wes hayutin
URL:
Whiteboard:
Depends On:
Blocks: 563609 verified-to-close
TreeView+ depends on / blocked
 
Reported: 2010-09-22 13:28 UTC by Daniel Mach
Modified: 2011-08-16 14:20 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-08-16 14:20:44 UTC
Embargoed:


Attachments (Terms of Use)

Description Daniel Mach 2010-09-22 13:28:03 UTC
$ pulp-admin repo upload --id=<repo> <foo>.rpm
 Successful uploaded [<foo>.rpm] to  Repo [ <repo> ] 

edit <foo>.rpm, remove some character from the end of the file
and run the same command again

$ pulp-admin repo upload --id=<repo> <foo>.rpm
 Successful uploaded [<foo>.rpm] to  Repo [ <repo> ] 

I'd expect it to fail due to different file content.

Comment 1 Todd Sanders 2010-09-22 16:04:19 UTC

*** This bug has been marked as a duplicate of bug 636564 ***

Comment 2 Daniel Mach 2010-09-23 06:34:45 UTC
Switching this bug back to assigned, I hope you don't mind...
Bug 636564 is a call for client, but this check has to be done on server as well.
Someone can use different client and possibly overwrite files existing on server.

Comment 3 Pradeep Kilambi 2010-10-27 19:10:43 UTC
fixed as part of sprint16 effort

Comment 4 Jay Dobies 2010-10-29 16:58:33 UTC
Fixed in 0.77.

Comment 5 Jay Dobies 2010-11-03 19:35:37 UTC
Fixed in build 0.78.

Comment 6 Daniel Mach 2010-11-04 07:56:39 UTC
current message:
Package [ConsoleKit-libs-0.4.1-5.fc13.i686.rpm] already exists on the server in repo rhel-i386-client-5

This message indicates that the file already exists on server (which is correct), but uploaded data is actually *different* -> another message should be printed.

suggested message:
Package [ConsoleKit-libs-0.4.1-5.fc13.i686.rpm] checksum doesn't match (file: <checksum>, server: <checksum>): 
Script should return any exit code but 0 in this case.

Comment 7 Pradeep Kilambi 2010-11-05 12:51:18 UTC
We only check if package exists on a per repo basis with nvrea check and return back true/false so there is no checksum coming into play as we decided not to support --force option to override the package. Only way to override is delete the package and re upload today.

So if the package exists and you wanna override it, just delete and re upload.

Comment 8 Pradeep Kilambi 2010-11-05 12:53:55 UTC
Another note, if you get the message that package exists and still wanna see what the checksum is, use package info call.

Comment 9 Daniel Mach 2010-11-05 15:18:31 UTC
I also consider --force harmful and avoid using it.
On the other hand, the NVREA check is not enough.
The package may be signed with a different key and we want to know if the content really matches.

Are you going to use the package info call in `pulp repo upload`?
Or will you stick with current NVREA check?

Comment 10 Pradeep Kilambi 2010-11-05 16:10:35 UTC
I can include the package info output as part of package already exists message if that helps.

Comment 11 Preethi Thomas 2010-11-10 17:02:04 UTC
fails_qa

1. I uploaded a repo with 2 packages
2. edit 1 package
3. try to upload the repo again.

The message mentioned only one of the package as already exists.


[root@preethi ~]# pulp-admin repo upload --id=repo-upload1 --dir=/root/rpm-dir/Successful uploaded [pulp-tools-0.0.52-1.fc13.noarch.rpm] to repo [ repo-upload1 ]
Successful uploaded [pulp-0.0.52-1.fc13.noarch.rpm] to repo [ repo-upload1 ]

[root@preethi ~]# cd /root/rpm-dir/
[root@preethi rpm-dir]# ls
pulp-0.0.52-1.fc13.noarch.rpm  pulp-tools-0.0.52-1.fc13.noarch.rpm
[root@preethi rpm-dir]# vi pulp-0.0.52-1.fc13.noarch.rpm 
[root@preethi rpm-dir]# 
[root@preethi rpm-dir]# 
[root@preethi rpm-dir]# 
[root@preethi rpm-dir]# pulp-admin repo upload --id=repo-upload1 --dir=/root/rpm-dir/
Package [pulp-tools-0.0.52-1.fc13.noarch.rpm] already exists on the server in repo repo-upload1

Comment 12 Pradeep Kilambi 2010-11-10 17:08:29 UTC
fixed!

commit 4f0752f084f0de758a1d42ec1f2db210913babe3
Author: Pradeep Kilambi <pkilambi>
Date:   Wed Nov 10 12:01:24 2010 -0500

$ sudo pulp-admin repo upload --id=testupload --dir=/home/pkilambi/Downloads/
Package [google-chrome-stable_current_i386.rpm] already exists on the server in repo testupload
Package [google-chrome-stable_current_i386(3).rpm] already exists on the server in repo testupload
Package [ypbind-1.31-3.fc13.x86_64.rpm] already exists on the server in repo testupload
Package [google-chrome-stable_current_i386(2).rpm] already exists on the server in repo testupload
Package [youtube-dl-2010.08.04-1.fc13.noarch.rpm] already exists on the server in repo testupload
Package [google-chrome-stable_current_x86_64.rpm] already exists on the server in repo testupload
Package [VirtualBox-3.2-3.2.8_64453_fedora13-1.x86_64.rpm] already exists on the server in repo testupload

Comment 13 Jay Dobies 2010-11-11 17:23:55 UTC
Fixed in 0.83.

Comment 14 Daniel Mach 2010-11-16 20:42:38 UTC
It doesn't seem to be fixed in pulp-0.0.92-1.fc14.noarch

still getting:
Package [...rpm] already exists on the server in repo ...
when I upload modified file with different checksum

Comment 15 Pradeep Kilambi 2010-11-16 20:57:57 UTC
Yea its the same as before. I dint hear back from you as to whether pkg info is preferred. I'll add some more details to the error message on existing package.

Comment 16 Daniel Mach 2010-11-17 13:37:47 UTC
(In reply to comment #15)
> Yea its the same as before. I dint hear back from you as to whether pkg info is
> preferred.
Ah, sorry for that.

> I'll add some more details to the error message on existing package.
Thanks.
It is essential for us to know whether packages on local disk really match those on server.

This bug is (sort of) related to bug #649765 - checksum is also used to identify the file, therefore can be used to avoid uploads when necessary and only link packages on server to requested repos.

I think rhnupload is well designed (but badly written) at this point and you should use it as a good example for the new code you're working on.

Comment 17 Daniel Mach 2010-12-01 11:20:23 UTC
still not fixed, moving back to ASSIGNED

Comment 18 Pradeep Kilambi 2010-12-03 16:44:11 UTC
I'll prioritize it for this sprint, bumping priority to high.

Comment 19 Pradeep Kilambi 2010-12-09 17:36:04 UTC
fixed! 

Now checksum is included as part of the exists message to compare,

$ sudo pulp-admin repo upload --id=testupload ~/Downloads/youtube-dl-2010.08.04-1.fc13.noarch.rpm
Package [youtube-dl-2010.08.04-1.fc13.noarch.rpm] already exists on the server with checksum [{u'sha256': u'c582eebedb24d8a9aeec1965c27f7ed758040bbbee3ec26556534378fb95175d'}] in repo testupload

If there is more than one checksum type stored per package all checksum types will be displayed with corresponding values.

Lemme know if this helps.

Comment 20 Daniel Mach 2010-12-09 17:49:18 UTC
Does it distinguish between these 2 cases?
a) files are the same (-> ok)
b) files differ (-> error)

Comment 21 Pradeep Kilambi 2010-12-09 18:04:45 UTC
you get the above message with checksum in either case. If the checksum ends up being different, you can use 'pulp-admin repo delete_package' functionality i added to remove that package and re-upload new one.

Comment 22 Daniel Mach 2010-12-09 20:17:33 UTC
So there's no information whether packages are the same or not.
That still makes the upload script unusable for RCM.
We need to distinguish these 2 states from logs printed by pulp-admin.
Also non-zero exit code on file mismatch would be appreciated.

Will you change it or should I rather write my own upload script?
I'll probably do it anyway, because AFAIK pulp-admin repo upload is not designed to upload really big quantities of files which requires working with hardlinks, precise error reporting etc.

Comment 23 Todd Sanders 2010-12-09 21:20:56 UTC
We are providing the necessary information in the return on the pulp-admin repo upload cmd to distinguish between the states:

1. successful upload
2. failed upload
3. package exists, with checksum info for file on server

#3 gives you the info you need to compare the checksums within your RCM integration code.  We are not going to compare checksums on the server, as we don't want to have to account for multiple checksum types. 

Yes, the server will not tell you the packages are the same, but we provide you with the information to determine that within your code. 

Unlike Satellite, we are not going to support a --force option.  This means that the package will need to be deleted (pulp-admin repo delete_package) before a new package (same nvrea) can be uploaded.

No reason pulp-admin repo upload can't be used with both large quantities of rpms, or large-sized (2GB) rpms.

"That still makes the upload script unusable for RCM."
Can you help us understand the specific use case and why what we have currently will not support it...other than the compare logic is on you.

Comment 24 Jay Dobies 2010-12-10 19:09:12 UTC
Fixed in build 0.114.

Comment 25 Preethi Thomas 2011-01-07 14:48:55 UTC
Moving to verified

[root@preethi ~]# rpm -q pulp
pulp-0.0.121-1.fc14.noarch


 1. Created an empty repo 
[root@preethi ~]# pulp-admin repo create --id=upload --name=upload 
Successfully created repository [ upload ]

2. uploaded 2 packages to it

[root@preethi ~]# pulp-admin repo upload --id=upload --dir=/root/repo-dir/
Successful uploaded [gofer-0.12-1.fc14.noarch.rpm] to repo [ upload ]
Successful uploaded [pulp-cds-0.0.121-1.fc14.noarch.rpm] to repo [ upload ]


3.  In /var/libpulp/repos/upload(thats my reponame)
I edit one of the packages

 vi /var/libpulp/repos/upload/gofer-0.12-1.fc14.noarch.rpm 

4. Now I run repo upload again

[root@preethi upload]# pulp-admin repo upload --id=upload --dir=/root/repo-dir/
Package [gofer-0.12-1.fc14.noarch.rpm] already exists on the server with checksum [{'sha256': '6191305c8c1e891fb17afaca4508d78939792d1848b9b7f77d534dd339b1806c'}] in repo upload
Package [pulp-cds-0.0.121-1.fc14.noarch.rpm] already exists on the server with checksum [{'sha256': '834fb545f765603e697ab016755f59afe48c22fb7a2e1411514a165b28ea36d8'}] in repo upload


[root@preethi upload]# pulp-admin repo delete_package --id=upload -p gofer-0.12-1.fc14.noarch.rpm 
Successfully removed package gofer-0.12-1.fc14.noarch.rpm from repo [upload].

[root@preethi upload]# pulp-admin repo upload --id=upload --dir=/root/repo-dir/
Successful uploaded [gofer-0.12-1.fc14.noarch.rpm] to repo [ upload ]
Package [pulp-cds-0.0.121-1.fc14.noarch.rpm] already exists on the server with checksum [{'sha256': '834fb545f765603e697ab016755f59afe48c22fb7a2e1411514a165b28ea36d8'}] in repo upload

Comment 26 Preethi Thomas 2011-08-16 14:20:44 UTC
Closing with Community Release 15

pulp-0.0.223-4.


Note You need to log in before you can comment on or make changes to this bug.