since pagure1866 is not fixed I am adding a comment so that I get notifications from this PR

watch

Sorry for spam, subscribing by commenting...

this probably should be removed

I believe this is to avoid overwriting the progress bar from the earlier upload.
However, looking at the kojid code, it doesn't look like this option is ever used as anything other than a boolean. I'll post an update in a few

Wait, nm, I see where the conf is used

Hmm, --multilib seems to be giving me some trouble

Yeah, it is trying to double link the noarch rpms. I've got an ugly workaround here, but I think it needs a proper fix.

Updated and rebased:

• fix multilib import
• avoid duplicate hard linking
• avoid noarch duplication

Generating a multilib signed repo now works for me locally. E.g.

lkoji signed-repo f24-repo  --multilib /etc/multilib.conf --allow-unsigned 4c8da725

This might be ready to merge

what about to not blow up koji more with yum code?

dnf is not everywhere yet. Plenty of folks running koji builders on rhel or centos, probably more than on Fedora actually.

@mikem what I wanted to say that if we don't support both, then we can't have any new features like rich dependencies.

I'm sure we'll support both eventually, but I'm starting with the original work that was submitted.

How would rich dependencies affect generating multilib repos?

@mikem I already have patch for python-multilib, so if signed repos are going to F26 cycle I would be happier if we would support DNF here as well.

@mikem basically, yum just crashes when it sees rich dependency.

I'm not going to hold this PR up over dnf, but I would certainly be happy to see a follow up that adds dnf support.

@mikem Then merge it so the work can be started.

I've been adding some unit tests for these changes and this revealed a design issue with the new signedRepoMove hub call. I'll need to rework that a bit.

python2-multilib requirement is missing in spec.

Filed #342 (hit with empty pkglist) It is questionable if task should fail or create empty repo, when there is nothing.

@mikem How is this looking now, against current master?

The delta rpms part of this is what's holding it back now. I just had a talk with Jay and Dennis about this and I think we have a plan for fixing that.

@mikem What "delta rpms part"?

I've modified the existing --delta-rpms arg to take a signed repo id (or the name of a tag with a signed repo). I think that is the last red flag for me.

Working locally for me.

The code looks alright to me. The only problem I have with it now is that I think it's inappropriately named. This code does everything but sign the repodata, so it's really a misnomer...

@ngompa It makes repos consisting of signed rpms. Can you suggest better terminology?

Actually, another issue is that the signed rpms are hardly the only difference between these repos and the other koji repos. Other differences are:

• multilib rpms
• delta rpms
• stored in different location
• hardlinked rpms (so the repo can easily be copied elsewhere)

Granted, several of these are optional, while the command insists on being given at least one key. Though, you can specify a bogus key and use --allow-unsigned.

Maybe "full repos" (Fedora definition), "enhanced repos" (repositories with extra bits), or "export repos" (repos suitable for external usage)?

cobrien just suggested "dist repo" or "distribution repo." Has a nice ring to it, and seems fairly accurate

rebranding this fully would involve changing:

• command name
• task method names
• task class names
• rpc method names
• the 'signed' field in the repo table in the db
• the PathInfo method
• some help text in the cli handler
• maybe a comment or two
• cli output
• some function names on the hub
• callback args
• option name for some calls
• the name of the permission used to govern access
• unit test names
• signed_repo_lifetime option in kojira
• a variable name or two

so... yeah....

coming in from left field here... just would like to say that if the current naming is "confusing" then doing a rename now, vs explaining things to people for years might be worth it.

I like "dist repo"/"distribution repo". It's very clear and allows the wide range of functionality to fall neatly under a single umbrella term.

Renaming wasn't too bad. Sed went a long way

Running it against build tag created broken link latest in repos/tag/latest as new repo was created in repos-dist/tag/60

$ koji dist-repo test-f25-build --skip-unsigned 81B46521
$ ls -l /mnt/koji/repos/test-f25-build
total 0
drwxr-xr-x 4 apache apache 34 Mar 20 13:09 56
lrwxrwxrwx 1 apache apache  2 Mar 27 12:00 latest -> 60
$ ls -l /mnt/koji/repos-dist/test-f25-build/
total 0
drwxr-xr-x 3 apache apache 20 Mar 27 11:59 60

Cosmetic: usage of deprecated py2 constructs (.has_key, print without parameters)

If taginfo['arches'] == None, split() will fail, so next line with message will not be reached.

Traceback (most recent call last):
File "/usr/bin/koji", line 7433, in
rv = locals()[command].call(options, session, args)
File "/usr/bin/koji", line 7131, in handle_dist_repo
task_opts.arch = taginfo['arches'].split()
AttributeError: 'NoneType' object has no attribute 'split'

Would it make sense to extend 'Unsigned packages found' message with signatures found in these rpms? It could be helpful in debugging (if packages really are not signed, or they have beta key, etc.)

Would it make sense to extend message with signatures found in these rpms? It could be helpful in debugging (if packages really are not signed, or they have beta key, etc.)

That would be helpful, but we should probably get all of this out of the exception text. There could easily be 1000s of rpms in this list if the wrong args are given.

subscribed

I would change this to --allow-unsigned-rpms, as we may also want to support signed repodata (as SUSE does), and so this could get confusing if we're not specific.

Like the other option, I'd clarify this. --skip-unsigned-rpms makes it more obvious.

@mikem Overall, it looks good to me, but the last bit about clarifying the options is because I fully expect that Koji will need to support signed repodata, too, since CentOS has been doing it since CentOS 7.3 and Fedora does it for the OpenH264 repository. The only reason we don't do it generally in Fedora is because it's not automatic.

Validated, that my previous issues are fixed.

What remains for me is non-clear usage of --allow-unsigned (and +1 for ngompas renaming) with differently signed rpms - maybe just extending help for the command, that it will skip rpm if there is no unsigned version?

And maybe some more clarification about sigkey's priority. It is not clear, that they are prioritized. Maybe it is not needed to be in command's help, but some section in docs?

If we later add support for signed metadata, that will work completely differently. Koji will have to generate that signature somehow during the process. There will be no corresponding --allow-X or --skip-X options for that. Koji will either sign the metadata or it won't. I'd expect something like --sign-metadata.

That said, the options are probably misnamed, but for a different reason. When a desired signature is missing, the task will fall back to the primary copy. While this is often the same as 'unsigned' that need not be the case. If an rpm is initially imported with a signature (with a manual import or perhaps a plugin that signs rpms at build time), then the primary copy will be signed. Unfortunately the data model doesn't track whether the primary copy is signed.

At any rate, I'm changing the options to --allow-missing-signatures and --skip-missing-signatures, to avoid the possibly incorrect term "unsigned".

Some fallout of this is that the code was dealing with these options all wrong (assuming that unsigned == primary copy), so I had to clean up the make_pkglist() quite a bit. Will update shortly.

@mikem Could you rebase this against master?

Commit c41f6cc8 fixes this pull-request

:beers:

:wine_glass: