Should ro: be prefix be documented somewhere?

Otherwise it looks good to me, but I don't have big knowledge of Koji code.

At least tests are failing now (simple update to exception texts). I'll test code more today.

This could be moved one level to right inside if, so it doesn't grow unnecessarily.

I would put it to sample config for now (we don't have any docs for runroot plugin :-( )

Isn't it easier to use getUserPerms once, instead of creating new call and going through cycle here?

@tkopecek: tests fixed and more added. I also went for the getUserPerms method as you suggested.

I am just having some trouble adding tests for the new Hub API call.

@tkopecek, @jkaluza: I've documented the ro: part in both the runroot.conf, and in the --help for the CLI plugin.

Sorry, I wasn't clear enough with getPerms. It could be dropped completely here, see https://pagure.io/fork/tkopecek/koji/c/191e21d479882809950c8a7545ca964f110ab079?branch=runroot-mount-policy2

Ah, cool. Sorry, I'd not noticed that call.

Concerns:

• I don't like that the caller has to specify the "ro:". It seems like optional mounts are going to be set up to be either ro or rw. If it's set up to be ro, then we don't need the caller to tell us
• I think "safe_roots" is an unfortunate option name, and I'd rather not propagate that

I'll remove the ro: call-time prefix and make sure that if a volume is defined as having the "ro" mount option in config, it cannot be mounted rw. I think that in that case, I can also remove the safe_ro_roots, since whether it's allowed ro or rw is already defined in the [pathN] entry.

I personally would still see a usecase for allowing a ro: call-time option specification, for clients that don't want to write to a volume, and also don't want to run the risk, even if the volume is allowed to be mounted rw. Would you be opposed to letting it in as an option, if it's not required to mount ro-only volumes?

I'm wondering whether the safe_roots is actually needed, or whether the fact that a volume is listed in [pathN] is enough to mark it as safe?

I'll remove the ro: call-time prefix and make sure that if a volume is defined as having the "ro" mount option in config, it cannot be mounted rw. I think that in that case, I can also remove the safe_ro_roots, since whether it's allowed ro or rw is already defined in the [pathN] entry.

We'll need some other indicator. A typical use case is for a key volume to be set up with a default ro mount, but with some subdirs listed in safe_roots. That is, you have the whole volume present, but only a subdir is rw.

For example, the runroot for composes might have the full compose volume mounted ro, but only the subdirectory used for the compose as rw.

I personally would still see a usecase for allowing a ro: call-time option specification, for clients that don't want to write to a volume, and also don't want to run the risk, even if the volume is allowed to be mounted rw. Would you be opposed to letting it in as an option, if it's not required to mount ro-only volumes?

Sure, I'm not opposed. I just don't want it to be required .

I'm wondering whether the safe_roots is actually needed, or whether the fact that a volume is listed in [pathN] is enough to mark it as safe?

It is, because of the use case above.

Sadly, runroot is kind of a mess

Issue #1536

Closing (see the #1536)